Skip to content

Commit

Permalink
Auto merge of rust-lang#13379 - DropDemBits:ide-assists-format-args-c…
Browse files Browse the repository at this point in the history
…apture, r=Veykril

internal: Migrate `ide_assists::utils` and `ide_assists::handlers` to use format arg captures (part 1)

This not only serves as making future migration to mutable syntax trees easier, it also finds out what needs to be migrated in the first place.

~~Aside from the first commit, subsequent commits are structured to only deal with one file/handler at a time.~~

This is the first of 3 PRs, migrating:

Utils:

- `gen_trait_fn_body`
- `render_snippet`
- `ReferenceConversion`
  - `convert_type`
  - `getter`

Handlers:

- `add_explicit_type`
- `add_return_type`
- `add_turbo_fish`
- `apply_demorgan`
- `auto_import`
- `convert_comment_block`
- `convert_integer_literal`
- `convert_into_to_from`
- `convert_iter_for_each_to_for`
- `convert_let_else_to_match`
- `convert_tuple_struct_to_named_struct`
- `convert_two_arm_bool_match_to_matches_macro`
- `destructure_tuple_binding`
- `extract_function`
- `extract_module`
- `extract_struct_from_enum_variant`
- `extract_type_alias`
- `extract_variable`
- `fix_visibility`
  • Loading branch information
bors committed Nov 5, 2022
2 parents 2c37e7d + d439fb2 commit afe8f6b
Show file tree
Hide file tree
Showing 21 changed files with 158 additions and 165 deletions.
4 changes: 2 additions & 2 deletions crates/ide-assists/src/handlers/add_explicit_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,14 @@ pub(crate) fn add_explicit_type(acc: &mut Assists, ctx: &AssistContext<'_>) -> O
let inferred_type = ty.display_source_code(ctx.db(), module.into()).ok()?;
acc.add(
AssistId("add_explicit_type", AssistKind::RefactorRewrite),
format!("Insert explicit type `{}`", inferred_type),
format!("Insert explicit type `{inferred_type}`"),
pat_range,
|builder| match ascribed_ty {
Some(ascribed_ty) => {
builder.replace(ascribed_ty.syntax().text_range(), inferred_type);
}
None => {
builder.insert(pat_range.end(), format!(": {}", inferred_type));
builder.insert(pat_range.end(), format!(": {inferred_type}"));
}
},
)
Expand Down
6 changes: 3 additions & 3 deletions crates/ide-assists/src/handlers/add_return_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,16 @@ pub(crate) fn add_return_type(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt
match builder_edit_pos {
InsertOrReplace::Insert(insert_pos, needs_whitespace) => {
let preceeding_whitespace = if needs_whitespace { " " } else { "" };
builder.insert(insert_pos, &format!("{}-> {} ", preceeding_whitespace, ty))
builder.insert(insert_pos, &format!("{preceeding_whitespace}-> {ty} "))
}
InsertOrReplace::Replace(text_range) => {
builder.replace(text_range, &format!("-> {}", ty))
builder.replace(text_range, &format!("-> {ty}"))
}
}
if let FnType::Closure { wrap_expr: true } = fn_type {
cov_mark::hit!(wrap_closure_non_block_expr);
// `|x| x` becomes `|x| -> T x` which is invalid, so wrap it in a block
builder.replace(tail_expr.syntax().text_range(), &format!("{{{}}}", tail_expr));
builder.replace(tail_expr.syntax().text_range(), &format!("{{{tail_expr}}}"));
}
},
)
Expand Down
7 changes: 4 additions & 3 deletions crates/ide-assists/src/handlers/add_turbo_fish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,13 @@ pub(crate) fn add_turbo_fish(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
builder.trigger_signature_help();
match ctx.config.snippet_cap {
Some(cap) => {
let snip = format!("::<{}>", get_snippet_fish_head(number_of_arguments));
let fish_head = get_snippet_fish_head(number_of_arguments);
let snip = format!("::<{fish_head}>");
builder.insert_snippet(cap, ident.text_range().end(), snip)
}
None => {
let fish_head = std::iter::repeat("_").take(number_of_arguments).format(", ");
let snip = format!("::<{}>", fish_head);
let snip = format!("::<{fish_head}>");
builder.insert(ident.text_range().end(), snip);
}
}
Expand All @@ -109,7 +110,7 @@ pub(crate) fn add_turbo_fish(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
/// This will create a snippet string with tabstops marked
fn get_snippet_fish_head(number_of_arguments: usize) -> String {
let mut fish_head = (1..number_of_arguments)
.format_with("", |i, f| f(&format_args!("${{{}:_}}, ", i)))
.format_with("", |i, f| f(&format_args!("${{{i}:_}}, ")))
.to_string();

// tabstop 0 is a special case and always the last one
Expand Down
6 changes: 3 additions & 3 deletions crates/ide-assists/src/handlers/apply_demorgan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,20 +123,20 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
let lhs_range = lhs.syntax().text_range();
let not_lhs = invert_boolean_expression(lhs);

edit.replace(lhs_range, format!("!({}", not_lhs.syntax().text()));
edit.replace(lhs_range, format!("!({not_lhs}"));
}

if let Some(rhs) = terms.pop_back() {
let rhs_range = rhs.syntax().text_range();
let not_rhs = invert_boolean_expression(rhs);

edit.replace(rhs_range, format!("{})", not_rhs.syntax().text()));
edit.replace(rhs_range, format!("{not_rhs})"));
}

for term in terms {
let term_range = term.syntax().text_range();
let not_term = invert_boolean_expression(term);
edit.replace(term_range, not_term.syntax().text());
edit.replace(term_range, not_term.to_string());
}
}
},
Expand Down
6 changes: 4 additions & 2 deletions crates/ide-assists/src/handlers/auto_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,20 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<
.sort_by_key(|import| Reverse(relevance_score(ctx, import, current_module.as_ref())));

for import in proposed_imports {
let import_path = import.import_path;

acc.add_group(
&group_label,
AssistId("auto_import", AssistKind::QuickFix),
format!("Import `{}`", import.import_path),
format!("Import `{import_path}`"),
range,
|builder| {
let scope = match scope.clone() {
ImportScope::File(it) => ImportScope::File(builder.make_mut(it)),
ImportScope::Module(it) => ImportScope::Module(builder.make_mut(it)),
ImportScope::Block(it) => ImportScope::Block(builder.make_mut(it)),
};
insert_use(&scope, mod_path_to_ast(&import.import_path), &ctx.config.insert_use);
insert_use(&scope, mod_path_to_ast(&import_path), &ctx.config.insert_use);
},
);
}
Expand Down
13 changes: 7 additions & 6 deletions crates/ide-assists/src/handlers/convert_comment_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,17 @@ fn block_to_line(acc: &mut Assists, comment: ast::Comment) -> Option<()> {

let indent_spaces = indentation.to_string();
let output = lines
.map(|l| l.trim_start_matches(&indent_spaces))
.map(|l| {
.map(|line| {
let line = line.trim_start_matches(&indent_spaces);

// Don't introduce trailing whitespace
if l.is_empty() {
if line.is_empty() {
line_prefix.to_string()
} else {
format!("{} {}", line_prefix, l.trim_start_matches(&indent_spaces))
format!("{line_prefix} {line}")
}
})
.join(&format!("\n{}", indent_spaces));
.join(&format!("\n{indent_spaces}"));

edit.replace(target, output)
},
Expand Down Expand Up @@ -96,7 +97,7 @@ fn line_to_block(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
let block_prefix =
CommentKind { shape: CommentShape::Block, ..comment.kind() }.prefix();

let output = format!("{}\n{}\n{}*/", block_prefix, block_comment_body, indentation);
let output = format!("{block_prefix}\n{block_comment_body}\n{indentation}*/");

edit.replace(target, output)
},
Expand Down
10 changes: 5 additions & 5 deletions crates/ide-assists/src/handlers/convert_integer_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,19 @@ pub(crate) fn convert_integer_literal(acc: &mut Assists, ctx: &AssistContext<'_>
}

let mut converted = match target_radix {
Radix::Binary => format!("0b{:b}", value),
Radix::Octal => format!("0o{:o}", value),
Radix::Binary => format!("0b{value:b}"),
Radix::Octal => format!("0o{value:o}"),
Radix::Decimal => value.to_string(),
Radix::Hexadecimal => format!("0x{:X}", value),
Radix::Hexadecimal => format!("0x{value:X}"),
};

let label = format!("Convert {} to {}{}", literal, converted, suffix.unwrap_or_default());

// Appends the type suffix back into the new literal if it exists.
if let Some(suffix) = suffix {
converted.push_str(suffix);
}

let label = format!("Convert {literal} to {converted}");

acc.add_group(
&group_id,
AssistId("convert_integer_literal", AssistKind::RefactorInline),
Expand Down
4 changes: 2 additions & 2 deletions crates/ide-assists/src/handlers/convert_into_to_from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ pub(crate) fn convert_into_to_from(acc: &mut Assists, ctx: &AssistContext<'_>) -
impl_.syntax().text_range(),
|builder| {
builder.replace(src_type.syntax().text_range(), dest_type.to_string());
builder.replace(ast_trait.syntax().text_range(), format!("From<{}>", src_type));
builder.replace(ast_trait.syntax().text_range(), format!("From<{src_type}>"));
builder.replace(into_fn_return.syntax().text_range(), "-> Self");
builder.replace(into_fn_params.syntax().text_range(), format!("(val: {})", src_type));
builder.replace(into_fn_params.syntax().text_range(), format!("(val: {src_type})"));
builder.replace(into_fn_name.syntax().text_range(), "from");

for s in selfs {
Expand Down
12 changes: 6 additions & 6 deletions crates/ide-assists/src/handlers/convert_iter_for_each_to_for.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,19 @@ pub(crate) fn convert_for_loop_with_for_each(
{
// We have either "for x in &col" and col implements a method called iter
// or "for x in &mut col" and col implements a method called iter_mut
format_to!(buf, "{}.{}()", expr_behind_ref, method);
format_to!(buf, "{expr_behind_ref}.{method}()");
} else if let ast::Expr::RangeExpr(..) = iterable {
// range expressions need to be parenthesized for the syntax to be correct
format_to!(buf, "({})", iterable);
format_to!(buf, "({iterable})");
} else if impls_core_iter(&ctx.sema, &iterable) {
format_to!(buf, "{}", iterable);
format_to!(buf, "{iterable}");
} else if let ast::Expr::RefExpr(_) = iterable {
format_to!(buf, "({}).into_iter()", iterable);
format_to!(buf, "({iterable}).into_iter()");
} else {
format_to!(buf, "{}.into_iter()", iterable);
format_to!(buf, "{iterable}.into_iter()");
}

format_to!(buf, ".for_each(|{}| {});", pat, body);
format_to!(buf, ".for_each(|{pat}| {body});");

builder.replace(for_loop.syntax().text_range(), buf)
},
Expand Down
6 changes: 3 additions & 3 deletions crates/ide-assists/src/handlers/convert_let_else_to_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fn binders_to_str(binders: &[(Name, bool)], addmut: bool) -> String {
.map(
|(ident, ismut)| {
if *ismut && addmut {
format!("mut {}", ident)
format!("mut {ident}")
} else {
ident.to_string()
}
Expand All @@ -93,7 +93,7 @@ fn binders_to_str(binders: &[(Name, bool)], addmut: bool) -> String {
} else if binders.len() == 1 {
vars
} else {
format!("({})", vars)
format!("({vars})")
}
}

Expand Down Expand Up @@ -153,7 +153,7 @@ pub(crate) fn convert_let_else_to_match(acc: &mut Assists, ctx: &AssistContext<'

let only_expr = let_else_block.statements().next().is_none();
let branch2 = match &let_else_block.tail_expr() {
Some(tail) if only_expr => format!("{},", tail.syntax().text()),
Some(tail) if only_expr => format!("{tail},"),
_ => let_else_block.syntax().text().to_string(),
};
let replace = if binders.is_empty() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,13 @@ fn edit_field_references(
}

fn generate_names(fields: impl Iterator<Item = ast::TupleField>) -> Vec<ast::Name> {
fields.enumerate().map(|(i, _)| ast::make::name(&format!("field{}", i + 1))).collect()
fields
.enumerate()
.map(|(i, _)| {
let idx = i + 1;
ast::make::name(&format!("field{idx}"))
})
.collect()
}

#[cfg(test)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,16 @@ pub(crate) fn convert_two_arm_bool_match_to_matches_macro(
target_range,
|builder| {
let mut arm_str = String::new();
if let Some(ref pat) = first_arm.pat() {
if let Some(pat) = &first_arm.pat() {
arm_str += &pat.to_string();
}
if let Some(ref guard) = first_arm.guard() {
arm_str += &format!(" {}", &guard.to_string());
if let Some(guard) = &first_arm.guard() {
arm_str += &format!(" {guard}");
}
if invert_matches {
builder.replace(target_range, format!("!matches!({}, {})", expr, arm_str));
builder.replace(target_range, format!("!matches!({expr}, {arm_str})"));
} else {
builder.replace(target_range, format!("matches!({}, {})", expr, arm_str));
builder.replace(target_range, format!("matches!({expr}, {arm_str})"));
}
},
)
Expand Down
12 changes: 6 additions & 6 deletions crates/ide-assists/src/handlers/destructure_tuple_binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ fn generate_name(
_usages: &Option<UsageSearchResult>,
) -> String {
// FIXME: detect if name already used
format!("_{}", index)
format!("_{index}")
}

enum RefType {
Expand Down Expand Up @@ -168,12 +168,12 @@ fn edit_tuple_assignment(
let add_cursor = |text: &str| {
// place cursor on first tuple item
let first_tuple = &data.field_names[0];
text.replacen(first_tuple, &format!("$0{}", first_tuple), 1)
text.replacen(first_tuple, &format!("$0{first_tuple}"), 1)
};

// with sub_pattern: keep original tuple and add subpattern: `tup @ (_0, _1)`
if in_sub_pattern {
let text = format!(" @ {}", tuple_pat);
let text = format!(" @ {tuple_pat}");
match ctx.config.snippet_cap {
Some(cap) => {
let snip = add_cursor(&text);
Expand Down Expand Up @@ -314,9 +314,9 @@ struct RefData {
impl RefData {
fn format(&self, field_name: &str) -> String {
match (self.needs_deref, self.needs_parentheses) {
(true, true) => format!("(*{})", field_name),
(true, false) => format!("*{}", field_name),
(false, true) => format!("({})", field_name),
(true, true) => format!("(*{field_name})"),
(true, false) => format!("*{field_name}"),
(false, true) => format!("({field_name})"),
(false, false) => field_name.to_string(),
}
}
Expand Down
Loading

0 comments on commit afe8f6b

Please sign in to comment.