From 940d87b2d05dcf074bfe5461999f244ca82f6403 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 21 Aug 2023 10:41:49 +0200 Subject: [PATCH 01/49] extend check.overrideCommand and buildScripts.overrideCommand docs regarding invocation strategy and location --- crates/rust-analyzer/src/config.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 40c50f6d1768f..dc678dae6ebc7 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -90,6 +90,12 @@ config_data! { /// and should therefore include `--message-format=json` or a similar /// option. /// + /// If there are multiple linked projects, this command is invoked for + /// each of them, with the working directory being the project root + /// (i.e., the folder containing the `Cargo.toml`). This can be overwritten + /// by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and + /// `#rust-analyzer.cargo.buildScripts.invocationLocation#`. + /// /// By default, a cargo invocation will be constructed for the configured /// targets and features, with the following base command line: /// @@ -183,7 +189,9 @@ config_data! { /// /// If there are multiple linked projects, this command is invoked for /// each of them, with the working directory being the project root - /// (i.e., the folder containing the `Cargo.toml`). + /// (i.e., the folder containing the `Cargo.toml`). This can be overwritten + /// by changing `#rust-analyzer.cargo.check.invocationStrategy#` and + /// `#rust-analyzer.cargo.check.invocationLocation#`. /// /// An example command would be: /// From 2de62be09b33a86f5f9f23bdc88c589dde2f767e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 21 Aug 2023 10:51:58 +0200 Subject: [PATCH 02/49] projects/workspaces --- crates/rust-analyzer/src/config.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index dc678dae6ebc7..a6803b1b48039 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -90,8 +90,8 @@ config_data! { /// and should therefore include `--message-format=json` or a similar /// option. /// - /// If there are multiple linked projects, this command is invoked for - /// each of them, with the working directory being the project root + /// If there are multiple linked projects/workspaces, this command is invoked for + /// each of them, with the working directory being the workspace root /// (i.e., the folder containing the `Cargo.toml`). This can be overwritten /// by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and /// `#rust-analyzer.cargo.buildScripts.invocationLocation#`. @@ -187,8 +187,8 @@ config_data! { /// Cargo, you might also want to change /// `#rust-analyzer.cargo.buildScripts.overrideCommand#`. /// - /// If there are multiple linked projects, this command is invoked for - /// each of them, with the working directory being the project root + /// If there are multiple linked projects/workspaces, this command is invoked for + /// each of them, with the working directory being the workspace root /// (i.e., the folder containing the `Cargo.toml`). This can be overwritten /// by changing `#rust-analyzer.cargo.check.invocationStrategy#` and /// `#rust-analyzer.cargo.check.invocationLocation#`. From 23ffda1a97726ccdb70e43910ae4363f3e5b8448 Mon Sep 17 00:00:00 2001 From: vxpm Date: Sun, 3 Sep 2023 22:42:56 -0300 Subject: [PATCH 03/49] full function signatures option --- crates/ide-completion/src/config.rs | 1 + crates/ide-completion/src/render/function.rs | 8 ++++++-- crates/rust-analyzer/src/config.rs | 3 +++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/ide-completion/src/config.rs b/crates/ide-completion/src/config.rs index 8f6a97e1e09d8..3d025f284bbb7 100644 --- a/crates/ide-completion/src/config.rs +++ b/crates/ide-completion/src/config.rs @@ -14,6 +14,7 @@ pub struct CompletionConfig { pub enable_imports_on_the_fly: bool, pub enable_self_on_the_fly: bool, pub enable_private_editable: bool, + pub full_function_signatures: bool, pub callable: Option, pub snippet_cap: Option, pub insert_use: InsertUseConfig, diff --git a/crates/ide-completion/src/render/function.rs b/crates/ide-completion/src/render/function.rs index 8afce8db5ea86..1672826f3306b 100644 --- a/crates/ide-completion/src/render/function.rs +++ b/crates/ide-completion/src/render/function.rs @@ -100,7 +100,7 @@ fn render( item.set_documentation(ctx.docs(func)) .set_deprecated(ctx.is_deprecated(func) || ctx.is_deprecated_assoc_item(func)) - .detail(detail(db, func)) + .detail(detail(db, func, ctx.completion.config.full_function_signatures)) .lookup_by(name.unescaped().to_smol_str()); match ctx.completion.config.snippet_cap { @@ -239,7 +239,11 @@ fn ref_of_param(ctx: &CompletionContext<'_>, arg: &str, ty: &hir::Type) -> &'sta "" } -fn detail(db: &dyn HirDatabase, func: hir::Function) -> String { +fn detail(db: &dyn HirDatabase, func: hir::Function, full_function_signature: bool) -> String { + if full_function_signature { + return format!("{}", func.display(db)).replace("\n", " "); + } + let mut ret_ty = func.ret_type(db); let mut detail = String::new(); diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index ea3a21241cb6e..07c6457b9c759 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -215,6 +215,8 @@ config_data! { completion_postfix_enable: bool = "true", /// Enables completions of private items and fields that are defined in the current workspace even if they are not visible at the current position. completion_privateEditable_enable: bool = "false", + /// Whether to show full function/method signatures in completion docs. + completion_fullFunctionSignatures_enable: bool = "false", /// Custom completion snippets. // NOTE: Keep this list in sync with the feature docs of user snippets. completion_snippets_custom: FxHashMap = r#"{ @@ -1444,6 +1446,7 @@ impl Config { && completion_item_edit_resolve(&self.caps), enable_self_on_the_fly: self.data.completion_autoself_enable, enable_private_editable: self.data.completion_privateEditable_enable, + full_function_signatures: self.data.completion_fullFunctionSignatures_enable, callable: match self.data.completion_callable_snippets { CallableCompletionDef::FillArguments => Some(CallableSnippets::FillArguments), CallableCompletionDef::AddParentheses => Some(CallableSnippets::AddParentheses), From 6afa5b0ba28c46905c0d0ac3022d4e1af0f1de36 Mon Sep 17 00:00:00 2001 From: vxpm Date: Sun, 3 Sep 2023 23:41:13 -0300 Subject: [PATCH 04/49] better handling of spaces & newlines --- crates/ide-completion/src/render/function.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/ide-completion/src/render/function.rs b/crates/ide-completion/src/render/function.rs index 1672826f3306b..dd7de72190d1e 100644 --- a/crates/ide-completion/src/render/function.rs +++ b/crates/ide-completion/src/render/function.rs @@ -241,7 +241,18 @@ fn ref_of_param(ctx: &CompletionContext<'_>, arg: &str, ty: &hir::Type) -> &'sta fn detail(db: &dyn HirDatabase, func: hir::Function, full_function_signature: bool) -> String { if full_function_signature { - return format!("{}", func.display(db)).replace("\n", " "); + let signature = format!("{}", func.display(db)); + let mut singleline = String::with_capacity(signature.len()); + + for segment in signature.split_whitespace() { + if !singleline.is_empty() { + singleline.push(' '); + } + + singleline.push_str(segment); + } + + return singleline; } let mut ret_ty = func.ret_type(db); From 6b487ed4be81e723e0ed2035f79b27d9efb93b8a Mon Sep 17 00:00:00 2001 From: vxpm Date: Mon, 4 Sep 2023 00:02:08 -0300 Subject: [PATCH 05/49] fix & run tests --- crates/ide-completion/src/tests.rs | 1 + crates/rust-analyzer/src/config.rs | 4 ++-- crates/rust-analyzer/src/integrated_benchmarks.rs | 2 ++ docs/user/generated_config.adoc | 5 +++++ editors/code/package.json | 5 +++++ 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/crates/ide-completion/src/tests.rs b/crates/ide-completion/src/tests.rs index 2464e8d5f8175..284bdd8af21f8 100644 --- a/crates/ide-completion/src/tests.rs +++ b/crates/ide-completion/src/tests.rs @@ -64,6 +64,7 @@ pub(crate) const TEST_CONFIG: CompletionConfig = CompletionConfig { enable_imports_on_the_fly: true, enable_self_on_the_fly: true, enable_private_editable: false, + full_function_signatures: false, callable: Some(CallableSnippets::FillArguments), snippet_cap: SnippetCap::new(true), prefer_no_std: false, diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 07c6457b9c759..9382f4ea19efe 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -209,14 +209,14 @@ config_data! { completion_autoself_enable: bool = "true", /// Whether to add parenthesis and argument snippets when completing function. completion_callable_snippets: CallableCompletionDef = "\"fill_arguments\"", + /// Whether to show full function/method signatures in completion docs. + completion_fullFunctionSignatures_enable: bool = "false", /// Maximum number of completions to return. If `None`, the limit is infinite. completion_limit: Option = "null", /// Whether to show postfix snippets like `dbg`, `if`, `not`, etc. completion_postfix_enable: bool = "true", /// Enables completions of private items and fields that are defined in the current workspace even if they are not visible at the current position. completion_privateEditable_enable: bool = "false", - /// Whether to show full function/method signatures in completion docs. - completion_fullFunctionSignatures_enable: bool = "false", /// Custom completion snippets. // NOTE: Keep this list in sync with the feature docs of user snippets. completion_snippets_custom: FxHashMap = r#"{ diff --git a/crates/rust-analyzer/src/integrated_benchmarks.rs b/crates/rust-analyzer/src/integrated_benchmarks.rs index 5a11012b93cf1..2c402552919f0 100644 --- a/crates/rust-analyzer/src/integrated_benchmarks.rs +++ b/crates/rust-analyzer/src/integrated_benchmarks.rs @@ -134,6 +134,7 @@ fn integrated_completion_benchmark() { enable_imports_on_the_fly: true, enable_self_on_the_fly: true, enable_private_editable: true, + full_function_signatures: false, callable: Some(CallableSnippets::FillArguments), snippet_cap: SnippetCap::new(true), insert_use: InsertUseConfig { @@ -173,6 +174,7 @@ fn integrated_completion_benchmark() { enable_imports_on_the_fly: true, enable_self_on_the_fly: true, enable_private_editable: true, + full_function_signatures: false, callable: Some(CallableSnippets::FillArguments), snippet_cap: SnippetCap::new(true), insert_use: InsertUseConfig { diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index 71feed0f72ca0..cf549565fe1db 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -244,6 +244,11 @@ with `self` prefixed to them when inside a method. -- Whether to add parenthesis and argument snippets when completing function. -- +[[rust-analyzer.completion.fullFunctionSignatures.enable]]rust-analyzer.completion.fullFunctionSignatures.enable (default: `false`):: ++ +-- +Whether to show full function/method signatures in completion docs. +-- [[rust-analyzer.completion.limit]]rust-analyzer.completion.limit (default: `null`):: + -- diff --git a/editors/code/package.json b/editors/code/package.json index 233e7bf44b166..e075e227625ab 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -799,6 +799,11 @@ "Do no snippet completions for callables." ] }, + "rust-analyzer.completion.fullFunctionSignatures.enable": { + "markdownDescription": "Whether to show full function/method signatures in completion docs.", + "default": false, + "type": "boolean" + }, "rust-analyzer.completion.limit": { "markdownDescription": "Maximum number of completions to return. If `None`, the limit is infinite.", "default": null, From d7a8e800c9270023d5a19f793f28054b936f7c5d Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Sat, 19 Aug 2023 16:49:26 -0600 Subject: [PATCH 06/49] feat: initial version of bool_to_enum assist --- .../ide-assists/src/handlers/bool_to_enum.rs | 724 ++++++++++++++++++ crates/ide-assists/src/lib.rs | 2 + crates/syntax/src/ast/make.rs | 28 + 3 files changed, 754 insertions(+) create mode 100644 crates/ide-assists/src/handlers/bool_to_enum.rs diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs new file mode 100644 index 0000000000000..dd11824b992d7 --- /dev/null +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -0,0 +1,724 @@ +use ide_db::{ + assists::{AssistId, AssistKind}, + defs::Definition, + search::{FileReference, SearchScope, UsageSearchResult}, + source_change::SourceChangeBuilder, +}; +use syntax::{ + ast::{ + self, + edit::IndentLevel, + edit_in_place::{AttrsOwnerEdit, Indent}, + make, HasName, + }, + ted, AstNode, NodeOrToken, SyntaxNode, T, +}; + +use crate::assist_context::{AssistContext, Assists}; + +// Assist: bool_to_enum +// +// This converts boolean local variables, fields, constants, and statics into a new +// enum with two variants `Bool::True` and `Bool::False`, as well as replacing +// all assignments with the variants and replacing all usages with `== Bool::True` or +// `== Bool::False`. +// +// ``` +// fn main() { +// let $0bool = true; +// +// if bool { +// println!("foo"); +// } +// } +// ``` +// -> +// ``` +// fn main() { +// #[derive(PartialEq, Eq)] +// enum Bool { True, False } +// +// let bool = Bool::True; +// +// if bool == Bool::True { +// println!("foo"); +// } +// } +// ``` +pub(crate) fn bool_to_enum(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { + let BoolNodeData { target_node, name, ty_annotation, initializer, definition } = + find_bool_node(ctx)?; + + let target = name.syntax().text_range(); + acc.add( + AssistId("bool_to_enum", AssistKind::RefactorRewrite), + "Convert boolean to enum", + target, + |edit| { + if let Some(ty) = &ty_annotation { + cov_mark::hit!(replaces_ty_annotation); + edit.replace(ty.syntax().text_range(), "Bool"); + } + + if let Some(initializer) = initializer { + replace_bool_expr(edit, initializer); + } + + let usages = definition + .usages(&ctx.sema) + .in_scope(&SearchScope::single_file(ctx.file_id())) + .all(); + replace_usages(edit, &usages); + + add_enum_def(edit, ctx, &usages, target_node); + }, + ) +} + +struct BoolNodeData { + target_node: SyntaxNode, + name: ast::Name, + ty_annotation: Option, + initializer: Option, + definition: Definition, +} + +/// Attempts to find an appropriate node to apply the action to. +fn find_bool_node(ctx: &AssistContext<'_>) -> Option { + if let Some(let_stmt) = ctx.find_node_at_offset::() { + let bind_pat = match let_stmt.pat()? { + ast::Pat::IdentPat(pat) => pat, + _ => { + cov_mark::hit!(not_applicable_in_non_ident_pat); + return None; + } + }; + let def = ctx.sema.to_def(&bind_pat)?; + if !def.ty(ctx.db()).is_bool() { + cov_mark::hit!(not_applicable_non_bool_local); + return None; + } + + Some(BoolNodeData { + target_node: let_stmt.syntax().clone(), + name: bind_pat.name()?, + ty_annotation: let_stmt.ty(), + initializer: let_stmt.initializer(), + definition: Definition::Local(def), + }) + } else if let Some(const_) = ctx.find_node_at_offset::() { + let def = ctx.sema.to_def(&const_)?; + if !def.ty(ctx.db()).is_bool() { + cov_mark::hit!(not_applicable_non_bool_const); + return None; + } + + Some(BoolNodeData { + target_node: const_.syntax().clone(), + name: const_.name()?, + ty_annotation: const_.ty(), + initializer: const_.body(), + definition: Definition::Const(def), + }) + } else if let Some(static_) = ctx.find_node_at_offset::() { + let def = ctx.sema.to_def(&static_)?; + if !def.ty(ctx.db()).is_bool() { + cov_mark::hit!(not_applicable_non_bool_static); + return None; + } + + Some(BoolNodeData { + target_node: static_.syntax().clone(), + name: static_.name()?, + ty_annotation: static_.ty(), + initializer: static_.body(), + definition: Definition::Static(def), + }) + } else if let Some(field_name) = ctx.find_node_at_offset::() { + let field = field_name.syntax().ancestors().find_map(ast::RecordField::cast)?; + if field.name()? != field_name { + return None; + } + + let strukt = field.syntax().ancestors().find_map(ast::Struct::cast)?; + let def = ctx.sema.to_def(&field)?; + if !def.ty(ctx.db()).is_bool() { + cov_mark::hit!(not_applicable_non_bool_field); + return None; + } + Some(BoolNodeData { + target_node: strukt.syntax().clone(), + name: field_name, + ty_annotation: field.ty(), + initializer: None, + definition: Definition::Field(def), + }) + } else { + None + } +} + +fn replace_bool_expr(edit: &mut SourceChangeBuilder, expr: ast::Expr) { + let expr_range = expr.syntax().text_range(); + let enum_expr = bool_expr_to_enum_expr(expr); + edit.replace(expr_range, enum_expr.syntax().text()) +} + +/// Converts an expression of type `bool` to one of the new enum type. +fn bool_expr_to_enum_expr(expr: ast::Expr) -> ast::Expr { + let true_expr = make::expr_path(make::path_from_text("Bool::True")).clone_for_update(); + let false_expr = make::expr_path(make::path_from_text("Bool::False")).clone_for_update(); + + if let ast::Expr::Literal(literal) = &expr { + match literal.kind() { + ast::LiteralKind::Bool(true) => true_expr, + ast::LiteralKind::Bool(false) => false_expr, + _ => expr, + } + } else { + make::expr_if( + expr, + make::tail_only_block_expr(true_expr), + Some(ast::ElseBranch::Block(make::tail_only_block_expr(false_expr))), + ) + .clone_for_update() + } +} + +/// Replaces all usages of the target identifier, both when read and written to. +fn replace_usages(edit: &mut SourceChangeBuilder, usages: &UsageSearchResult) { + for (_, references) in usages.iter() { + references + .into_iter() + .filter_map(|FileReference { range, name, .. }| match name { + ast::NameLike::NameRef(name) => Some((*range, name)), + _ => None, + }) + .for_each(|(range, name_ref)| { + if let Some(initializer) = find_assignment_usage(name_ref) { + cov_mark::hit!(replaces_assignment); + + replace_bool_expr(edit, initializer); + } else if let Some((prefix_expr, expr)) = find_negated_usage(name_ref) { + cov_mark::hit!(replaces_negation); + + edit.replace( + prefix_expr.syntax().text_range(), + format!("{} == Bool::False", expr), + ); + } else if let Some((record_field, initializer)) = find_record_expr_usage(name_ref) { + cov_mark::hit!(replaces_record_expr); + + let record_field = edit.make_mut(record_field); + let enum_expr = bool_expr_to_enum_expr(initializer); + record_field.replace_expr(enum_expr); + } else if name_ref.syntax().ancestors().find_map(ast::Expr::cast).is_some() { + // for any other usage in an expression, replace it with a check that it is the true variant + edit.replace(range, format!("{} == Bool::True", name_ref.text())); + } + }) + } +} + +fn find_assignment_usage(name_ref: &ast::NameRef) -> Option { + let bin_expr = name_ref.syntax().ancestors().find_map(ast::BinExpr::cast)?; + + if let Some(ast::BinaryOp::Assignment { op: None }) = bin_expr.op_kind() { + bin_expr.rhs() + } else { + None + } +} + +fn find_negated_usage(name_ref: &ast::NameRef) -> Option<(ast::PrefixExpr, ast::Expr)> { + let prefix_expr = name_ref.syntax().ancestors().find_map(ast::PrefixExpr::cast)?; + + if let Some(ast::UnaryOp::Not) = prefix_expr.op_kind() { + let initializer = prefix_expr.expr()?; + Some((prefix_expr, initializer)) + } else { + None + } +} + +fn find_record_expr_usage(name_ref: &ast::NameRef) -> Option<(ast::RecordExprField, ast::Expr)> { + let record_field = name_ref.syntax().ancestors().find_map(ast::RecordExprField::cast)?; + let initializer = record_field.expr()?; + + Some((record_field, initializer)) +} + +/// Adds the definition of the new enum before the target node. +fn add_enum_def( + edit: &mut SourceChangeBuilder, + ctx: &AssistContext<'_>, + usages: &UsageSearchResult, + target_node: SyntaxNode, +) { + let make_enum_pub = usages.iter().any(|(file_id, _)| file_id != &ctx.file_id()); + let enum_def = make_bool_enum(make_enum_pub); + + let indent = IndentLevel::from_node(&target_node); + enum_def.reindent_to(indent); + + ted::insert_all( + ted::Position::before(&edit.make_syntax_mut(target_node)), + vec![ + enum_def.syntax().clone().into(), + make::tokens::whitespace(&format!("\n\n{indent}")).into(), + ], + ); +} + +fn make_bool_enum(make_pub: bool) -> ast::Enum { + let enum_def = make::enum_( + if make_pub { Some(make::visibility_pub()) } else { None }, + make::name("Bool"), + make::variant_list(vec![ + make::variant(make::name("True"), None), + make::variant(make::name("False"), None), + ]), + ) + .clone_for_update(); + + let derive_eq = make::attr_outer(make::meta_token_tree( + make::ext::ident_path("derive"), + make::token_tree( + T!['('], + vec![ + NodeOrToken::Token(make::tokens::ident("PartialEq")), + NodeOrToken::Token(make::token(T![,])), + NodeOrToken::Token(make::tokens::single_space()), + NodeOrToken::Token(make::tokens::ident("Eq")), + ], + ), + )) + .clone_for_update(); + enum_def.add_attr(derive_eq); + + enum_def +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::tests::{check_assist, check_assist_not_applicable}; + + #[test] + fn local_variable_with_usage() { + check_assist( + bool_to_enum, + r#" +fn main() { + let $0foo = true; + + if foo { + println!("foo"); + } +} +"#, + r#" +fn main() { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + let foo = Bool::True; + + if foo == Bool::True { + println!("foo"); + } +} +"#, + ) + } + + #[test] + fn local_variable_with_usage_negated() { + cov_mark::check!(replaces_negation); + check_assist( + bool_to_enum, + r#" +fn main() { + let $0foo = true; + + if !foo { + println!("foo"); + } +} +"#, + r#" +fn main() { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + let foo = Bool::True; + + if foo == Bool::False { + println!("foo"); + } +} +"#, + ) + } + + #[test] + fn local_variable_with_type_annotation() { + cov_mark::check!(replaces_ty_annotation); + check_assist( + bool_to_enum, + r#" +fn main() { + let $0foo: bool = false; +} +"#, + r#" +fn main() { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + let foo: Bool = Bool::False; +} +"#, + ) + } + + #[test] + fn local_variable_with_non_literal_initializer() { + check_assist( + bool_to_enum, + r#" +fn main() { + let $0foo = 1 == 2; +} +"#, + r#" +fn main() { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + let foo = if 1 == 2 { Bool::True } else { Bool::False }; +} +"#, + ) + } + + #[test] + fn local_variable_binexpr_usage() { + check_assist( + bool_to_enum, + r#" +fn main() { + let $0foo = false; + let bar = true; + + if !foo && bar { + println!("foobar"); + } +} +"#, + r#" +fn main() { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + let foo = Bool::False; + let bar = true; + + if foo == Bool::False && bar { + println!("foobar"); + } +} +"#, + ) + } + + #[test] + fn local_variable_unop_usage() { + check_assist( + bool_to_enum, + r#" +fn main() { + let $0foo = true; + + if *&foo { + println!("foobar"); + } +} +"#, + r#" +fn main() { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + let foo = Bool::True; + + if *&foo == Bool::True { + println!("foobar"); + } +} +"#, + ) + } + + #[test] + fn local_variable_assigned_later() { + cov_mark::check!(replaces_assignment); + check_assist( + bool_to_enum, + r#" +fn main() { + let $0foo: bool; + foo = true; +} +"#, + r#" +fn main() { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + let foo: Bool; + foo = Bool::True; +} +"#, + ) + } + + #[test] + fn local_variable_does_not_apply_recursively() { + check_assist( + bool_to_enum, + r#" +fn main() { + let $0foo = true; + let bar = !foo; + + if bar { + println!("bar"); + } +} +"#, + r#" +fn main() { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + let foo = Bool::True; + let bar = foo == Bool::False; + + if bar { + println!("bar"); + } +} +"#, + ) + } + + #[test] + fn local_variable_non_bool() { + cov_mark::check!(not_applicable_non_bool_local); + check_assist_not_applicable( + bool_to_enum, + r#" +fn main() { + let $0foo = 1; +} +"#, + ) + } + + #[test] + fn local_variable_non_ident_pat() { + cov_mark::check!(not_applicable_in_non_ident_pat); + check_assist_not_applicable( + bool_to_enum, + r#" +fn main() { + let ($0foo, bar) = (true, false); +} +"#, + ) + } + + #[test] + fn field_basic() { + cov_mark::check!(replaces_record_expr); + check_assist( + bool_to_enum, + r#" +struct Foo { + $0bar: bool, + baz: bool, +} + +fn main() { + let foo = Foo { bar: true, baz: false }; + + if foo.bar { + println!("foo"); + } +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +struct Foo { + bar: Bool, + baz: bool, +} + +fn main() { + let foo = Foo { bar: Bool::True, baz: false }; + + if foo.bar == Bool::True { + println!("foo"); + } +} +"#, + ) + } + + #[test] + fn field_in_mod_properly_indented() { + check_assist( + bool_to_enum, + r#" +mod foo { + struct Bar { + $0baz: bool, + } + + impl Bar { + fn new(baz: bool) -> Self { + Self { baz } + } + } +} +"#, + r#" +mod foo { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + struct Bar { + baz: Bool, + } + + impl Bar { + fn new(baz: bool) -> Self { + Self { baz: if baz { Bool::True } else { Bool::False } } + } + } +} +"#, + ) + } + + #[test] + fn field_non_bool() { + cov_mark::check!(not_applicable_non_bool_field); + check_assist_not_applicable( + bool_to_enum, + r#" +struct Foo { + $0bar: usize, +} + +fn main() { + let foo = Foo { bar: 1 }; +} +"#, + ) + } + + #[test] + fn const_basic() { + check_assist( + bool_to_enum, + r#" +const $0FOO: bool = false; + +fn main() { + if FOO { + println!("foo"); + } +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +const FOO: Bool = Bool::False; + +fn main() { + if FOO == Bool::True { + println!("foo"); + } +} +"#, + ) + } + + #[test] + fn const_non_bool() { + cov_mark::check!(not_applicable_non_bool_const); + check_assist_not_applicable( + bool_to_enum, + r#" +const $0FOO: &str = "foo"; + +fn main() { + println!("{FOO}"); +} +"#, + ) + } + + #[test] + fn static_basic() { + check_assist( + bool_to_enum, + r#" +static mut $0BOOL: bool = true; + +fn main() { + unsafe { BOOL = false }; + if unsafe { BOOL } { + println!("foo"); + } +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +static mut BOOL: Bool = Bool::True; + +fn main() { + unsafe { BOOL = Bool::False }; + if unsafe { BOOL == Bool::True } { + println!("foo"); + } +} +"#, + ) + } + + #[test] + fn static_non_bool() { + cov_mark::check!(not_applicable_non_bool_static); + check_assist_not_applicable( + bool_to_enum, + r#" +static mut $0FOO: usize = 0; + +fn main() { + if unsafe { FOO } == 0 { + println!("foo"); + } +} +"#, + ) + } +} diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index 6f973ab53eec1..a17ce93e928c8 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -115,6 +115,7 @@ mod handlers { mod apply_demorgan; mod auto_import; mod bind_unused_param; + mod bool_to_enum; mod change_visibility; mod convert_bool_then; mod convert_comment_block; @@ -227,6 +228,7 @@ mod handlers { apply_demorgan::apply_demorgan, auto_import::auto_import, bind_unused_param::bind_unused_param, + bool_to_enum::bool_to_enum, change_visibility::change_visibility, convert_bool_then::convert_bool_then_to_if, convert_bool_then::convert_if_to_bool_then, diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 17e311c0c502f..e0055be6e69e2 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -973,6 +973,11 @@ pub fn tuple_field(visibility: Option, ty: ast::Type) -> ast::T ast_from_text(&format!("struct f({visibility}{ty});")) } +pub fn variant_list(variants: impl IntoIterator) -> ast::VariantList { + let variants = variants.into_iter().join(", "); + ast_from_text(&format!("enum f {{ {variants} }}")) +} + pub fn variant(name: ast::Name, field_list: Option) -> ast::Variant { let field_list = match field_list { None => String::new(), @@ -1037,6 +1042,19 @@ pub fn struct_( ast_from_text(&format!("{visibility}struct {strukt_name}{type_params}{field_list}{semicolon}",)) } +pub fn enum_( + visibility: Option, + enum_name: ast::Name, + variant_list: ast::VariantList, +) -> ast::Enum { + let visibility = match visibility { + None => String::new(), + Some(it) => format!("{it} "), + }; + + ast_from_text(&format!("{visibility}enum {enum_name} {variant_list}")) +} + pub fn attr_outer(meta: ast::Meta) -> ast::Attr { ast_from_text(&format!("#[{meta}]")) } @@ -1149,6 +1167,16 @@ pub mod tokens { lit.syntax().first_child_or_token().unwrap().into_token().unwrap() } + pub fn ident(text: &str) -> SyntaxToken { + assert_eq!(text.trim(), text); + let path: ast::Path = super::ext::ident_path(text); + path.syntax() + .descendants_with_tokens() + .filter_map(|it| it.into_token()) + .find(|it| it.kind() == IDENT) + .unwrap() + } + pub fn single_newline() -> SyntaxToken { let res = SOURCE_FILE .tree() From 59738d5fd5f868cec69e0ff30e27a6b80fc81ee4 Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Sat, 19 Aug 2023 17:41:44 -0600 Subject: [PATCH 07/49] fix: add generated doctest --- crates/ide-assists/src/tests/generated.rs | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index dfaa53449f42d..63a08a0e5697b 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -280,6 +280,34 @@ fn some_function(x: i32) { ) } +#[test] +fn doctest_bool_to_enum() { + check_doc_test( + "bool_to_enum", + r#####" +fn main() { + let $0bool = true; + + if bool { + println!("foo"); + } +} +"#####, + r#####" +fn main() { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + let bool = Bool::True; + + if bool == Bool::True { + println!("foo"); + } +} +"#####, + ) +} + #[test] fn doctest_change_visibility() { check_doc_test( From 83196fd4d9ed8544410fc82fee5d54830163f248 Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Sat, 19 Aug 2023 17:45:16 -0600 Subject: [PATCH 08/49] fix: remove trailing whitespace --- crates/ide-assists/src/handlers/bool_to_enum.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs index dd11824b992d7..279e5583624fc 100644 --- a/crates/ide-assists/src/handlers/bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -369,7 +369,7 @@ fn main() { bool_to_enum, r#" fn main() { - let $0foo: bool = false; + let $0foo: bool = false; } "#, r#" @@ -377,7 +377,7 @@ fn main() { #[derive(PartialEq, Eq)] enum Bool { True, False } - let foo: Bool = Bool::False; + let foo: Bool = Bool::False; } "#, ) @@ -389,7 +389,7 @@ fn main() { bool_to_enum, r#" fn main() { - let $0foo = 1 == 2; + let $0foo = 1 == 2; } "#, r#" @@ -397,7 +397,7 @@ fn main() { #[derive(PartialEq, Eq)] enum Bool { True, False } - let foo = if 1 == 2 { Bool::True } else { Bool::False }; + let foo = if 1 == 2 { Bool::True } else { Bool::False }; } "#, ) @@ -468,7 +468,7 @@ fn main() { bool_to_enum, r#" fn main() { - let $0foo: bool; + let $0foo: bool; foo = true; } "#, @@ -477,7 +477,7 @@ fn main() { #[derive(PartialEq, Eq)] enum Bool { True, False } - let foo: Bool; + let foo: Bool; foo = Bool::True; } "#, From 91ac1d619475e1b61bf4ae8d318c4740a0adce66 Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Fri, 8 Sep 2023 07:45:23 -0700 Subject: [PATCH 09/49] fix: initializing struct multiple times --- .../ide-assists/src/handlers/bool_to_enum.rs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs index 279e5583624fc..4158b75dc00c9 100644 --- a/crates/ide-assists/src/handlers/bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -194,6 +194,7 @@ fn replace_usages(edit: &mut SourceChangeBuilder, usages: &UsageSearchResult) { ast::NameLike::NameRef(name) => Some((*range, name)), _ => None, }) + .rev() .for_each(|(range, name_ref)| { if let Some(initializer) = find_assignment_usage(name_ref) { cov_mark::hit!(replaces_assignment); @@ -615,6 +616,46 @@ mod foo { ) } + #[test] + fn field_multiple_initializations() { + check_assist( + bool_to_enum, + r#" +struct Foo { + $0bar: bool, + baz: bool, +} + +fn main() { + let foo1 = Foo { bar: true, baz: false }; + let foo2 = Foo { bar: false, baz: false }; + + if foo1.bar && foo2.bar { + println!("foo"); + } +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum $0Bool { True, False } + +struct Foo { + bar: Bool, + baz: bool, +} + +fn main() { + let foo1 = Foo { bar: Bool::True, baz: false }; + let foo2 = Foo { bar: Bool::False, baz: false }; + + if foo1.bar == Bool::True && foo2.bar == Bool::True { + println!("foo"); + } +} +"#, + ) + } + #[test] fn field_non_bool() { cov_mark::check!(not_applicable_non_bool_field); From 455dacfd3b5387bcf2854f2a88edb9b69361e69f Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Fri, 8 Sep 2023 10:06:17 -0700 Subject: [PATCH 10/49] fix: only trigger assist on Name --- .../ide-assists/src/handlers/bool_to_enum.rs | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs index 4158b75dc00c9..56749edf4636b 100644 --- a/crates/ide-assists/src/handlers/bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -85,7 +85,9 @@ struct BoolNodeData { /// Attempts to find an appropriate node to apply the action to. fn find_bool_node(ctx: &AssistContext<'_>) -> Option { - if let Some(let_stmt) = ctx.find_node_at_offset::() { + let name: ast::Name = ctx.find_node_at_offset()?; + + if let Some(let_stmt) = name.syntax().ancestors().find_map(ast::LetStmt::cast) { let bind_pat = match let_stmt.pat()? { ast::Pat::IdentPat(pat) => pat, _ => { @@ -101,12 +103,12 @@ fn find_bool_node(ctx: &AssistContext<'_>) -> Option { Some(BoolNodeData { target_node: let_stmt.syntax().clone(), - name: bind_pat.name()?, + name, ty_annotation: let_stmt.ty(), initializer: let_stmt.initializer(), definition: Definition::Local(def), }) - } else if let Some(const_) = ctx.find_node_at_offset::() { + } else if let Some(const_) = name.syntax().ancestors().find_map(ast::Const::cast) { let def = ctx.sema.to_def(&const_)?; if !def.ty(ctx.db()).is_bool() { cov_mark::hit!(not_applicable_non_bool_const); @@ -115,12 +117,12 @@ fn find_bool_node(ctx: &AssistContext<'_>) -> Option { Some(BoolNodeData { target_node: const_.syntax().clone(), - name: const_.name()?, + name, ty_annotation: const_.ty(), initializer: const_.body(), definition: Definition::Const(def), }) - } else if let Some(static_) = ctx.find_node_at_offset::() { + } else if let Some(static_) = name.syntax().ancestors().find_map(ast::Static::cast) { let def = ctx.sema.to_def(&static_)?; if !def.ty(ctx.db()).is_bool() { cov_mark::hit!(not_applicable_non_bool_static); @@ -129,14 +131,14 @@ fn find_bool_node(ctx: &AssistContext<'_>) -> Option { Some(BoolNodeData { target_node: static_.syntax().clone(), - name: static_.name()?, + name, ty_annotation: static_.ty(), initializer: static_.body(), definition: Definition::Static(def), }) - } else if let Some(field_name) = ctx.find_node_at_offset::() { - let field = field_name.syntax().ancestors().find_map(ast::RecordField::cast)?; - if field.name()? != field_name { + } else { + let field = name.syntax().ancestors().find_map(ast::RecordField::cast)?; + if field.name()? != name { return None; } @@ -148,13 +150,11 @@ fn find_bool_node(ctx: &AssistContext<'_>) -> Option { } Some(BoolNodeData { target_node: strukt.syntax().clone(), - name: field_name, + name, ty_annotation: field.ty(), initializer: None, definition: Definition::Field(def), }) - } else { - None } } @@ -528,6 +528,18 @@ fn main() { ) } + #[test] + fn local_variable_cursor_not_on_ident() { + check_assist_not_applicable( + bool_to_enum, + r#" +fn main() { + let foo = $0true; +} +"#, + ) + } + #[test] fn local_variable_non_ident_pat() { cov_mark::check!(not_applicable_in_non_ident_pat); @@ -762,4 +774,9 @@ fn main() { "#, ) } + + #[test] + fn not_applicable_to_other_names() { + check_assist_not_applicable(bool_to_enum, "fn $0main() {}") + } } From 136a9dbe36606cb00b546c3562088c462d8a0926 Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Fri, 8 Sep 2023 10:54:30 -0700 Subject: [PATCH 11/49] style: rename some locals --- crates/ide-assists/src/handlers/bool_to_enum.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs index 56749edf4636b..975226484403c 100644 --- a/crates/ide-assists/src/handlers/bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -200,12 +200,12 @@ fn replace_usages(edit: &mut SourceChangeBuilder, usages: &UsageSearchResult) { cov_mark::hit!(replaces_assignment); replace_bool_expr(edit, initializer); - } else if let Some((prefix_expr, expr)) = find_negated_usage(name_ref) { + } else if let Some((prefix_expr, inner_expr)) = find_negated_usage(name_ref) { cov_mark::hit!(replaces_negation); edit.replace( prefix_expr.syntax().text_range(), - format!("{} == Bool::False", expr), + format!("{} == Bool::False", inner_expr), ); } else if let Some((record_field, initializer)) = find_record_expr_usage(name_ref) { cov_mark::hit!(replaces_record_expr); @@ -235,8 +235,8 @@ fn find_negated_usage(name_ref: &ast::NameRef) -> Option<(ast::PrefixExpr, ast:: let prefix_expr = name_ref.syntax().ancestors().find_map(ast::PrefixExpr::cast)?; if let Some(ast::UnaryOp::Not) = prefix_expr.op_kind() { - let initializer = prefix_expr.expr()?; - Some((prefix_expr, initializer)) + let inner_expr = prefix_expr.expr()?; + Some((prefix_expr, inner_expr)) } else { None } From 2e13aed3bc235d47d92f9ce3b8fd4fa3c5f87939 Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Sat, 9 Sep 2023 11:40:29 -0700 Subject: [PATCH 12/49] feat: support cross module imports --- .../ide-assists/src/handlers/bool_to_enum.rs | 226 +++++++++++++++++- 1 file changed, 214 insertions(+), 12 deletions(-) diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs index 975226484403c..f59b0528131e4 100644 --- a/crates/ide-assists/src/handlers/bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -1,9 +1,13 @@ +use hir::ModuleDef; use ide_db::{ assists::{AssistId, AssistKind}, defs::Definition, - search::{FileReference, SearchScope, UsageSearchResult}, + helpers::mod_path_to_ast, + imports::insert_use::{insert_use, ImportScope}, + search::{FileReference, UsageSearchResult}, source_change::SourceChangeBuilder, }; +use itertools::Itertools; use syntax::{ ast::{ self, @@ -48,6 +52,7 @@ use crate::assist_context::{AssistContext, Assists}; pub(crate) fn bool_to_enum(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let BoolNodeData { target_node, name, ty_annotation, initializer, definition } = find_bool_node(ctx)?; + let target_module = ctx.sema.scope(&target_node)?.module(); let target = name.syntax().text_range(); acc.add( @@ -64,13 +69,10 @@ pub(crate) fn bool_to_enum(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option replace_bool_expr(edit, initializer); } - let usages = definition - .usages(&ctx.sema) - .in_scope(&SearchScope::single_file(ctx.file_id())) - .all(); - replace_usages(edit, &usages); + let usages = definition.usages(&ctx.sema).all(); - add_enum_def(edit, ctx, &usages, target_node); + add_enum_def(edit, ctx, &usages, target_node, &target_module); + replace_usages(edit, ctx, &usages, &target_module); }, ) } @@ -186,8 +188,45 @@ fn bool_expr_to_enum_expr(expr: ast::Expr) -> ast::Expr { } /// Replaces all usages of the target identifier, both when read and written to. -fn replace_usages(edit: &mut SourceChangeBuilder, usages: &UsageSearchResult) { - for (_, references) in usages.iter() { +fn replace_usages( + edit: &mut SourceChangeBuilder, + ctx: &AssistContext<'_>, + usages: &UsageSearchResult, + target_module: &hir::Module, +) { + for (file_id, references) in usages.iter() { + edit.edit_file(*file_id); + + // add imports across modules where needed + references + .iter() + .filter_map(|FileReference { name, .. }| { + ctx.sema.scope(name.syntax()).map(|scope| (name, scope.module())) + }) + .unique_by(|name_and_module| name_and_module.1) + .filter(|(_, module)| module != target_module) + .filter_map(|(name, module)| { + let import_scope = ImportScope::find_insert_use_container(name.syntax(), &ctx.sema); + let mod_path = module.find_use_path_prefixed( + ctx.sema.db, + ModuleDef::Module(*target_module), + ctx.config.insert_use.prefix_kind, + ctx.config.prefer_no_std, + ); + import_scope.zip(mod_path) + }) + .for_each(|(import_scope, mod_path)| { + let import_scope = match import_scope { + ImportScope::File(it) => ImportScope::File(edit.make_mut(it)), + ImportScope::Module(it) => ImportScope::Module(edit.make_mut(it)), + ImportScope::Block(it) => ImportScope::Block(edit.make_mut(it)), + }; + let path = + make::path_concat(mod_path_to_ast(&mod_path), make::path_from_text("Bool")); + insert_use(&import_scope, path, &ctx.config.insert_use); + }); + + // replace the usages in expressions references .into_iter() .filter_map(|FileReference { range, name, .. }| match name { @@ -213,7 +252,7 @@ fn replace_usages(edit: &mut SourceChangeBuilder, usages: &UsageSearchResult) { let record_field = edit.make_mut(record_field); let enum_expr = bool_expr_to_enum_expr(initializer); record_field.replace_expr(enum_expr); - } else if name_ref.syntax().ancestors().find_map(ast::Expr::cast).is_some() { + } else if name_ref.syntax().ancestors().find_map(ast::UseTree::cast).is_none() { // for any other usage in an expression, replace it with a check that it is the true variant edit.replace(range, format!("{} == Bool::True", name_ref.text())); } @@ -255,8 +294,15 @@ fn add_enum_def( ctx: &AssistContext<'_>, usages: &UsageSearchResult, target_node: SyntaxNode, + target_module: &hir::Module, ) { - let make_enum_pub = usages.iter().any(|(file_id, _)| file_id != &ctx.file_id()); + let make_enum_pub = usages + .iter() + .flat_map(|(_, refs)| refs) + .filter_map(|FileReference { name, .. }| { + ctx.sema.scope(name.syntax()).map(|scope| scope.module()) + }) + .any(|module| &module != target_module); let enum_def = make_bool_enum(make_enum_pub); let indent = IndentLevel::from_node(&target_node); @@ -649,7 +695,7 @@ fn main() { "#, r#" #[derive(PartialEq, Eq)] -enum $0Bool { True, False } +enum Bool { True, False } struct Foo { bar: Bool, @@ -713,6 +759,162 @@ fn main() { ) } + #[test] + fn const_in_module() { + check_assist( + bool_to_enum, + r#" +fn main() { + if foo::FOO { + println!("foo"); + } +} + +mod foo { + pub const $0FOO: bool = true; +} +"#, + r#" +use foo::Bool; + +fn main() { + if foo::FOO == Bool::True { + println!("foo"); + } +} + +mod foo { + #[derive(PartialEq, Eq)] + pub enum Bool { True, False } + + pub const FOO: Bool = Bool::True; +} +"#, + ) + } + + #[test] + fn const_in_module_with_import() { + check_assist( + bool_to_enum, + r#" +fn main() { + use foo::FOO; + + if FOO { + println!("foo"); + } +} + +mod foo { + pub const $0FOO: bool = true; +} +"#, + r#" +use crate::foo::Bool; + +fn main() { + use foo::FOO; + + if FOO == Bool::True { + println!("foo"); + } +} + +mod foo { + #[derive(PartialEq, Eq)] + pub enum Bool { True, False } + + pub const FOO: Bool = Bool::True; +} +"#, + ) + } + + #[test] + fn const_cross_file() { + check_assist( + bool_to_enum, + r#" +//- /main.rs +mod foo; + +fn main() { + if foo::FOO { + println!("foo"); + } +} + +//- /foo.rs +pub const $0FOO: bool = true; +"#, + r#" +//- /main.rs +use foo::Bool; + +mod foo; + +fn main() { + if foo::FOO == Bool::True { + println!("foo"); + } +} + +//- /foo.rs +#[derive(PartialEq, Eq)] +pub enum Bool { True, False } + +pub const FOO: Bool = Bool::True; +"#, + ) + } + + #[test] + fn const_cross_file_and_module() { + check_assist( + bool_to_enum, + r#" +//- /main.rs +mod foo; + +fn main() { + use foo::bar; + + if bar::BAR { + println!("foo"); + } +} + +//- /foo.rs +pub mod bar { + pub const $0BAR: bool = false; +} +"#, + r#" +//- /main.rs +use crate::foo::bar::Bool; + +mod foo; + +fn main() { + use foo::bar; + + if bar::BAR == Bool::True { + println!("foo"); + } +} + +//- /foo.rs +pub mod bar { + #[derive(PartialEq, Eq)] + pub enum Bool { True, False } + + pub const BAR: Bool = Bool::False; +} +"#, + ) + } + #[test] fn const_non_bool() { cov_mark::check!(not_applicable_non_bool_const); From 7ba2e130b975f62906fff6ea82aacff883a9e528 Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Sat, 9 Sep 2023 23:54:25 -0700 Subject: [PATCH 13/49] fix: add checks for overwriting incorrect ancestor --- .../ide-assists/src/handlers/bool_to_enum.rs | 166 +++++++++++++++++- 1 file changed, 165 insertions(+), 1 deletion(-) diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs index f59b0528131e4..784a0d3559970 100644 --- a/crates/ide-assists/src/handlers/bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -263,6 +263,11 @@ fn replace_usages( fn find_assignment_usage(name_ref: &ast::NameRef) -> Option { let bin_expr = name_ref.syntax().ancestors().find_map(ast::BinExpr::cast)?; + if !bin_expr.lhs()?.syntax().descendants().contains(name_ref.syntax()) { + cov_mark::hit!(dont_assign_incorrect_ref); + return None; + } + if let Some(ast::BinaryOp::Assignment { op: None }) = bin_expr.op_kind() { bin_expr.rhs() } else { @@ -273,6 +278,11 @@ fn find_assignment_usage(name_ref: &ast::NameRef) -> Option { fn find_negated_usage(name_ref: &ast::NameRef) -> Option<(ast::PrefixExpr, ast::Expr)> { let prefix_expr = name_ref.syntax().ancestors().find_map(ast::PrefixExpr::cast)?; + if !matches!(prefix_expr.expr()?, ast::Expr::PathExpr(_) | ast::Expr::FieldExpr(_)) { + cov_mark::hit!(dont_overwrite_expression_inside_negation); + return None; + } + if let Some(ast::UnaryOp::Not) = prefix_expr.op_kind() { let inner_expr = prefix_expr.expr()?; Some((prefix_expr, inner_expr)) @@ -285,7 +295,12 @@ fn find_record_expr_usage(name_ref: &ast::NameRef) -> Option<(ast::RecordExprFie let record_field = name_ref.syntax().ancestors().find_map(ast::RecordExprField::cast)?; let initializer = record_field.expr()?; - Some((record_field, initializer)) + if record_field.field_name()?.syntax().descendants().contains(name_ref.syntax()) { + Some((record_field, initializer)) + } else { + cov_mark::hit!(dont_overwrite_wrong_record_field); + None + } } /// Adds the definition of the new enum before the target node. @@ -561,6 +576,37 @@ fn main() { ) } + #[test] + fn local_variable_nested_in_negation() { + cov_mark::check!(dont_overwrite_expression_inside_negation); + check_assist( + bool_to_enum, + r#" +fn main() { + if !"foo".chars().any(|c| { + let $0foo = true; + foo + }) { + println!("foo"); + } +} +"#, + r#" +fn main() { + if !"foo".chars().any(|c| { + #[derive(PartialEq, Eq)] + enum Bool { True, False } + + let foo = Bool::True; + foo == Bool::True + }) { + println!("foo"); + } +} +"#, + ) + } + #[test] fn local_variable_non_bool() { cov_mark::check!(not_applicable_non_bool_local); @@ -638,6 +684,42 @@ fn main() { ) } + #[test] + fn field_negated() { + check_assist( + bool_to_enum, + r#" +struct Foo { + $0bar: bool, +} + +fn main() { + let foo = Foo { bar: false }; + + if !foo.bar { + println!("foo"); + } +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +struct Foo { + bar: Bool, +} + +fn main() { + let foo = Foo { bar: Bool::False }; + + if foo.bar == Bool::False { + println!("foo"); + } +} +"#, + ) + } + #[test] fn field_in_mod_properly_indented() { check_assist( @@ -714,6 +796,88 @@ fn main() { ) } + #[test] + fn field_assigned_to_another() { + cov_mark::check!(dont_assign_incorrect_ref); + check_assist( + bool_to_enum, + r#" +struct Foo { + $0foo: bool, +} + +struct Bar { + bar: bool, +} + +fn main() { + let foo = Foo { foo: true }; + let mut bar = Bar { bar: true }; + + bar.bar = foo.foo; +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +struct Foo { + foo: Bool, +} + +struct Bar { + bar: bool, +} + +fn main() { + let foo = Foo { foo: Bool::True }; + let mut bar = Bar { bar: true }; + + bar.bar = foo.foo == Bool::True; +} +"#, + ) + } + + #[test] + fn field_initialized_with_other() { + cov_mark::check!(dont_overwrite_wrong_record_field); + check_assist( + bool_to_enum, + r#" +struct Foo { + $0foo: bool, +} + +struct Bar { + bar: bool, +} + +fn main() { + let foo = Foo { foo: true }; + let bar = Bar { bar: foo.foo }; +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +struct Foo { + foo: Bool, +} + +struct Bar { + bar: bool, +} + +fn main() { + let foo = Foo { foo: Bool::True }; + let bar = Bar { bar: foo.foo == Bool::True }; +} +"#, + ) + } + #[test] fn field_non_bool() { cov_mark::check!(not_applicable_non_bool_field); From 5683df2965f9577ca1307f10af766aeecd6b560e Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Thu, 10 Aug 2023 01:18:05 +0200 Subject: [PATCH 14/49] Deunwrap inline call --- .../ide-assists/src/handlers/inline_call.rs | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/crates/ide-assists/src/handlers/inline_call.rs b/crates/ide-assists/src/handlers/inline_call.rs index ffab58509b182..b7787e4c33a41 100644 --- a/crates/ide-assists/src/handlers/inline_call.rs +++ b/crates/ide-assists/src/handlers/inline_call.rs @@ -116,7 +116,8 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) -> .into_iter() .map(|(call_info, mut_node)| { let replacement = - inline(&ctx.sema, def_file, function, &func_body, ¶ms, &call_info); + inline(&ctx.sema, def_file, function, &func_body, ¶ms, &call_info) + .unwrap(); ted::replace(mut_node, replacement.syntax()); }) .count(); @@ -218,13 +219,12 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option< } let syntax = call_info.node.syntax().clone(); + let replacement = inline(&ctx.sema, file_id, function, &fn_body, ¶ms, &call_info)?; acc.add( AssistId("inline_call", AssistKind::RefactorInline), label, syntax.text_range(), |builder| { - let replacement = inline(&ctx.sema, file_id, function, &fn_body, ¶ms, &call_info); - builder.replace_ast( match call_info.node { ast::CallableExpr::Call(it) => ast::Expr::CallExpr(it), @@ -305,7 +305,7 @@ fn inline( fn_body: &ast::BlockExpr, params: &[(ast::Pat, Option, hir::Param)], CallInfo { node, arguments, generic_arg_list }: &CallInfo, -) -> ast::Expr { +) -> Option { let mut body = if sema.hir_file_for(fn_body.syntax()).is_macro() { cov_mark::hit!(inline_call_defined_in_macro); if let Some(body) = ast::BlockExpr::cast(insert_ws_into(fn_body.syntax().clone())) { @@ -363,16 +363,17 @@ fn inline( .collect(); if function.self_param(sema.db).is_some() { - let this = || make::name_ref("this").syntax().clone_for_update().first_token().unwrap(); + let this = || make::name_ref("this").syntax().clone_for_update().first_token(); if let Some(self_local) = params[0].2.as_local(sema.db) { - usages_for_locals(self_local) - .filter_map(|FileReference { name, range, .. }| match name { + let usages = usages_for_locals(self_local).filter_map( + |FileReference { name, range, .. }| match name { ast::NameLike::NameRef(_) => Some(body.syntax().covering_element(range)), _ => None, - }) - .for_each(|it| { - ted::replace(it, &this()); - }) + }, + ); + for usage in usages { + ted::replace(usage, &this()?); + } } } @@ -470,7 +471,7 @@ fn inline( } } else if let Some(stmt_list) = body.stmt_list() { ted::insert_all( - ted::Position::after(stmt_list.l_curly_token().unwrap()), + ted::Position::after(stmt_list.l_curly_token()?), let_stmts.into_iter().map(|stmt| stmt.syntax().clone().into()).collect(), ); } @@ -481,7 +482,7 @@ fn inline( }; body.reindent_to(original_indentation); - match body.tail_expr() { + Some(match body.tail_expr() { Some(expr) if !is_async_fn && body.statements().next().is_none() => expr, _ => match node .syntax() @@ -494,7 +495,7 @@ fn inline( } _ => ast::Expr::BlockExpr(body), }, - } + }) } fn path_expr_as_record_field(usage: &PathExpr) -> Option { From 68d24b69d45f357e7309c497f683c8b1e6ad691a Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sat, 9 Sep 2023 14:35:26 +0200 Subject: [PATCH 15/49] Deunwrap inline call v2 --- .../ide-assists/src/handlers/inline_call.rs | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/crates/ide-assists/src/handlers/inline_call.rs b/crates/ide-assists/src/handlers/inline_call.rs index b7787e4c33a41..4751de3125595 100644 --- a/crates/ide-assists/src/handlers/inline_call.rs +++ b/crates/ide-assists/src/handlers/inline_call.rs @@ -117,7 +117,7 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) -> .map(|(call_info, mut_node)| { let replacement = inline(&ctx.sema, def_file, function, &func_body, ¶ms, &call_info) - .unwrap(); + .expect("inline() should return an Expr"); ted::replace(mut_node, replacement.syntax()); }) .count(); @@ -363,17 +363,23 @@ fn inline( .collect(); if function.self_param(sema.db).is_some() { - let this = || make::name_ref("this").syntax().clone_for_update().first_token(); + let this = || { + make::name_ref("this") + .syntax() + .clone_for_update() + .first_token() + .expect("NameRef should have had a token.") + }; if let Some(self_local) = params[0].2.as_local(sema.db) { - let usages = usages_for_locals(self_local).filter_map( - |FileReference { name, range, .. }| match name { + usages_for_locals(self_local) + .filter_map(|FileReference { name, range, .. }| match name { ast::NameLike::NameRef(_) => Some(body.syntax().covering_element(range)), _ => None, - }, - ); - for usage in usages { - ted::replace(usage, &this()?); - } + }) + .into_iter() + .for_each(|usage| { + ted::replace(usage, &this()); + }); } } @@ -471,7 +477,9 @@ fn inline( } } else if let Some(stmt_list) = body.stmt_list() { ted::insert_all( - ted::Position::after(stmt_list.l_curly_token()?), + ted::Position::after( + stmt_list.l_curly_token().expect("L_CURLY for StatementList is missing."), + ), let_stmts.into_iter().map(|stmt| stmt.syntax().clone().into()).collect(), ); } From 38491fcf0785a33720ccfd746d2ebe2ad2686207 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sun, 10 Sep 2023 22:14:58 +0200 Subject: [PATCH 16/49] v3 --- crates/ide-assists/src/handlers/inline_call.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/crates/ide-assists/src/handlers/inline_call.rs b/crates/ide-assists/src/handlers/inline_call.rs index 4751de3125595..9b4ac8a02a350 100644 --- a/crates/ide-assists/src/handlers/inline_call.rs +++ b/crates/ide-assists/src/handlers/inline_call.rs @@ -116,8 +116,7 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) -> .into_iter() .map(|(call_info, mut_node)| { let replacement = - inline(&ctx.sema, def_file, function, &func_body, ¶ms, &call_info) - .expect("inline() should return an Expr"); + inline(&ctx.sema, def_file, function, &func_body, ¶ms, &call_info); ted::replace(mut_node, replacement.syntax()); }) .count(); @@ -219,7 +218,7 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option< } let syntax = call_info.node.syntax().clone(); - let replacement = inline(&ctx.sema, file_id, function, &fn_body, ¶ms, &call_info)?; + let replacement = inline(&ctx.sema, file_id, function, &fn_body, ¶ms, &call_info); acc.add( AssistId("inline_call", AssistKind::RefactorInline), label, @@ -305,7 +304,7 @@ fn inline( fn_body: &ast::BlockExpr, params: &[(ast::Pat, Option, hir::Param)], CallInfo { node, arguments, generic_arg_list }: &CallInfo, -) -> Option { +) -> ast::Expr { let mut body = if sema.hir_file_for(fn_body.syntax()).is_macro() { cov_mark::hit!(inline_call_defined_in_macro); if let Some(body) = ast::BlockExpr::cast(insert_ws_into(fn_body.syntax().clone())) { @@ -376,7 +375,6 @@ fn inline( ast::NameLike::NameRef(_) => Some(body.syntax().covering_element(range)), _ => None, }) - .into_iter() .for_each(|usage| { ted::replace(usage, &this()); }); @@ -490,7 +488,7 @@ fn inline( }; body.reindent_to(original_indentation); - Some(match body.tail_expr() { + match body.tail_expr() { Some(expr) if !is_async_fn && body.statements().next().is_none() => expr, _ => match node .syntax() @@ -503,7 +501,7 @@ fn inline( } _ => ast::Expr::BlockExpr(body), }, - }) + } } fn path_expr_as_record_field(usage: &PathExpr) -> Option { From 9c6257138de5f093b2dd03893aa694b6165ea157 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Wed, 9 Aug 2023 17:09:57 +0200 Subject: [PATCH 17/49] Deunwrap convert_comment_block --- .../src/handlers/convert_comment_block.rs | 29 +++++++++------- .../src/handlers/desugar_doc_comment.rs | 33 +++++++++++-------- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/crates/ide-assists/src/handlers/convert_comment_block.rs b/crates/ide-assists/src/handlers/convert_comment_block.rs index 1acd5ee97283f..b6ad2dc0b65df 100644 --- a/crates/ide-assists/src/handlers/convert_comment_block.rs +++ b/crates/ide-assists/src/handlers/convert_comment_block.rs @@ -78,21 +78,26 @@ fn line_to_block(acc: &mut Assists, comment: ast::Comment) -> Option<()> { // Establish the target of our edit based on the comments we found let target = TextRange::new( comments[0].syntax().text_range().start(), - comments.last().unwrap().syntax().text_range().end(), + comments.last()?.syntax().text_range().end(), ); + // We pick a single indentation level for the whole block comment based on the + // comment where the assist was invoked. This will be prepended to the + // contents of each line comment when they're put into the block comment. + let indentation = IndentLevel::from_token(comment.syntax()); + + let mut cms: Vec = Vec::new(); + for cm in comments { + let lcm = line_comment_text(indentation, cm)?; + cms.push(lcm); + } + acc.add( AssistId("line_to_block", AssistKind::RefactorRewrite), "Replace line comments with a single block comment", target, |edit| { - // We pick a single indentation level for the whole block comment based on the - // comment where the assist was invoked. This will be prepended to the - // contents of each line comment when they're put into the block comment. - let indentation = IndentLevel::from_token(comment.syntax()); - - let block_comment_body = - comments.into_iter().map(|c| line_comment_text(indentation, c)).join("\n"); + let block_comment_body = cms.into_iter().join("\n"); let block_prefix = CommentKind { shape: CommentShape::Block, ..comment.kind() }.prefix(); @@ -159,15 +164,15 @@ pub(crate) fn relevant_line_comments(comment: &ast::Comment) -> Vec { // */ // // But since such comments aren't idiomatic we're okay with this. -pub(crate) fn line_comment_text(indentation: IndentLevel, comm: ast::Comment) -> String { - let contents_without_prefix = comm.text().strip_prefix(comm.prefix()).unwrap(); +pub(crate) fn line_comment_text(indentation: IndentLevel, comm: ast::Comment) -> Option { + let contents_without_prefix = comm.text().strip_prefix(comm.prefix())?; let contents = contents_without_prefix.strip_prefix(' ').unwrap_or(contents_without_prefix); // Don't add the indentation if the line is empty if contents.is_empty() { - contents.to_owned() + Some(contents.to_owned()) } else { - indentation.to_string() + contents + Some(indentation.to_string() + contents) } } diff --git a/crates/ide-assists/src/handlers/desugar_doc_comment.rs b/crates/ide-assists/src/handlers/desugar_doc_comment.rs index ddc8a50ed4006..daa2c1df0cc48 100644 --- a/crates/ide-assists/src/handlers/desugar_doc_comment.rs +++ b/crates/ide-assists/src/handlers/desugar_doc_comment.rs @@ -57,25 +57,30 @@ pub(crate) fn desugar_doc_comment(acc: &mut Assists, ctx: &AssistContext<'_>) -> } }; + let text = match comments { + Either::Left(comment) => { + let text = comment.text(); + text[comment.prefix().len()..(text.len() - "*/".len())] + .trim() + .lines() + .map(|l| l.strip_prefix(&indentation).unwrap_or(l)) + .join("\n") + } + Either::Right(comments) => { + let mut cms: Vec = Vec::new(); + for cm in comments { + let lcm = line_comment_text(IndentLevel(0), cm)?; + cms.push(lcm); + } + cms.into_iter().join("\n") + } + }; + acc.add( AssistId("desugar_doc_comment", AssistKind::RefactorRewrite), "Desugar doc-comment to attribute macro", target, |edit| { - let text = match comments { - Either::Left(comment) => { - let text = comment.text(); - text[comment.prefix().len()..(text.len() - "*/".len())] - .trim() - .lines() - .map(|l| l.strip_prefix(&indentation).unwrap_or(l)) - .join("\n") - } - Either::Right(comments) => { - comments.into_iter().map(|c| line_comment_text(IndentLevel(0), c)).join("\n") - } - }; - let hashes = "#".repeat(required_hashes(&text)); let prefix = match placement { From b316bccc9716dc058120b4cf73c753973e2cd802 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Wed, 9 Aug 2023 23:34:30 +0200 Subject: [PATCH 18/49] replace for loops with sth more idiomatic --- .../src/handlers/convert_comment_block.rs | 9 ++++----- .../src/handlers/desugar_doc_comment.rs | 15 ++++++--------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/crates/ide-assists/src/handlers/convert_comment_block.rs b/crates/ide-assists/src/handlers/convert_comment_block.rs index b6ad2dc0b65df..4cbb30ec15c5e 100644 --- a/crates/ide-assists/src/handlers/convert_comment_block.rs +++ b/crates/ide-assists/src/handlers/convert_comment_block.rs @@ -86,11 +86,10 @@ fn line_to_block(acc: &mut Assists, comment: ast::Comment) -> Option<()> { // contents of each line comment when they're put into the block comment. let indentation = IndentLevel::from_token(comment.syntax()); - let mut cms: Vec = Vec::new(); - for cm in comments { - let lcm = line_comment_text(indentation, cm)?; - cms.push(lcm); - } + let cms = comments + .into_iter() + .map(|c| line_comment_text(indentation, c)) + .collect::>>()?; acc.add( AssistId("line_to_block", AssistKind::RefactorRewrite), diff --git a/crates/ide-assists/src/handlers/desugar_doc_comment.rs b/crates/ide-assists/src/handlers/desugar_doc_comment.rs index daa2c1df0cc48..2f8cef1e4a7a0 100644 --- a/crates/ide-assists/src/handlers/desugar_doc_comment.rs +++ b/crates/ide-assists/src/handlers/desugar_doc_comment.rs @@ -50,7 +50,7 @@ pub(crate) fn desugar_doc_comment(acc: &mut Assists, ctx: &AssistContext<'_>) -> ( TextRange::new( comments[0].syntax().text_range().start(), - comments.last().unwrap().syntax().text_range().end(), + comments.last()?.syntax().text_range().end(), ), Either::Right(comments), ) @@ -66,14 +66,11 @@ pub(crate) fn desugar_doc_comment(acc: &mut Assists, ctx: &AssistContext<'_>) -> .map(|l| l.strip_prefix(&indentation).unwrap_or(l)) .join("\n") } - Either::Right(comments) => { - let mut cms: Vec = Vec::new(); - for cm in comments { - let lcm = line_comment_text(IndentLevel(0), cm)?; - cms.push(lcm); - } - cms.into_iter().join("\n") - } + Either::Right(comments) => comments + .into_iter() + .map(|cm| line_comment_text(IndentLevel(0), cm)) + .collect::>>()? + .join("\n"), }; acc.add( From a66dbd11ed1501c0efbea265f76bc92147554ab7 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Tue, 15 Aug 2023 19:29:09 +0200 Subject: [PATCH 19/49] v2 --- crates/ide-assists/src/handlers/convert_comment_block.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ide-assists/src/handlers/convert_comment_block.rs b/crates/ide-assists/src/handlers/convert_comment_block.rs index 4cbb30ec15c5e..a850d28e918fa 100644 --- a/crates/ide-assists/src/handlers/convert_comment_block.rs +++ b/crates/ide-assists/src/handlers/convert_comment_block.rs @@ -164,7 +164,8 @@ pub(crate) fn relevant_line_comments(comment: &ast::Comment) -> Vec { // // But since such comments aren't idiomatic we're okay with this. pub(crate) fn line_comment_text(indentation: IndentLevel, comm: ast::Comment) -> Option { - let contents_without_prefix = comm.text().strip_prefix(comm.prefix())?; + let text = comm.text(); + let contents_without_prefix = text.strip_prefix(comm.prefix()).unwrap_or(text); let contents = contents_without_prefix.strip_prefix(' ').unwrap_or(contents_without_prefix); // Don't add the indentation if the line is empty From 2fdf7e4b752c8ac50585879163a3aebccfa82d4f Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sun, 10 Sep 2023 23:15:37 +0200 Subject: [PATCH 20/49] v3 --- .../src/handlers/convert_comment_block.rs | 16 ++++++---------- .../src/handlers/desugar_doc_comment.rs | 6 ++---- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/crates/ide-assists/src/handlers/convert_comment_block.rs b/crates/ide-assists/src/handlers/convert_comment_block.rs index a850d28e918fa..1f048ac10fa55 100644 --- a/crates/ide-assists/src/handlers/convert_comment_block.rs +++ b/crates/ide-assists/src/handlers/convert_comment_block.rs @@ -25,9 +25,7 @@ pub(crate) fn convert_comment_block(acc: &mut Assists, ctx: &AssistContext<'_>) let comment = ctx.find_token_at_offset::()?; // Only allow comments which are alone on their line if let Some(prev) = comment.syntax().prev_token() { - if Whitespace::cast(prev).filter(|w| w.text().contains('\n')).is_none() { - return None; - } + Whitespace::cast(prev).filter(|w| w.text().contains('\n'))?; } match comment.kind().shape { @@ -86,10 +84,8 @@ fn line_to_block(acc: &mut Assists, comment: ast::Comment) -> Option<()> { // contents of each line comment when they're put into the block comment. let indentation = IndentLevel::from_token(comment.syntax()); - let cms = comments - .into_iter() - .map(|c| line_comment_text(indentation, c)) - .collect::>>()?; + let cms = + comments.into_iter().map(|c| line_comment_text(indentation, c)).collect::>(); acc.add( AssistId("line_to_block", AssistKind::RefactorRewrite), @@ -163,16 +159,16 @@ pub(crate) fn relevant_line_comments(comment: &ast::Comment) -> Vec { // */ // // But since such comments aren't idiomatic we're okay with this. -pub(crate) fn line_comment_text(indentation: IndentLevel, comm: ast::Comment) -> Option { +pub(crate) fn line_comment_text(indentation: IndentLevel, comm: ast::Comment) -> String { let text = comm.text(); let contents_without_prefix = text.strip_prefix(comm.prefix()).unwrap_or(text); let contents = contents_without_prefix.strip_prefix(' ').unwrap_or(contents_without_prefix); // Don't add the indentation if the line is empty if contents.is_empty() { - Some(contents.to_owned()) + contents.to_owned() } else { - Some(indentation.to_string() + contents) + indentation.to_string() + contents } } diff --git a/crates/ide-assists/src/handlers/desugar_doc_comment.rs b/crates/ide-assists/src/handlers/desugar_doc_comment.rs index 2f8cef1e4a7a0..b7919bd1502cb 100644 --- a/crates/ide-assists/src/handlers/desugar_doc_comment.rs +++ b/crates/ide-assists/src/handlers/desugar_doc_comment.rs @@ -33,9 +33,7 @@ pub(crate) fn desugar_doc_comment(acc: &mut Assists, ctx: &AssistContext<'_>) -> // Only allow comments which are alone on their line if let Some(prev) = comment.syntax().prev_token() { - if Whitespace::cast(prev).filter(|w| w.text().contains('\n')).is_none() { - return None; - } + Whitespace::cast(prev).filter(|w| w.text().contains('\n'))?; } let indentation = IndentLevel::from_token(comment.syntax()).to_string(); @@ -69,7 +67,7 @@ pub(crate) fn desugar_doc_comment(acc: &mut Assists, ctx: &AssistContext<'_>) -> Either::Right(comments) => comments .into_iter() .map(|cm| line_comment_text(IndentLevel(0), cm)) - .collect::>>()? + .collect::>() .join("\n"), }; From 25b1b3e753c4cb6fa75b2004c8d753daf240b1c6 Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Sun, 10 Sep 2023 22:21:12 -0700 Subject: [PATCH 21/49] feat: add support for other ADT types and destructuring patterns --- .../ide-assists/src/handlers/bool_to_enum.rs | 494 +++++++++++++++--- 1 file changed, 430 insertions(+), 64 deletions(-) diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs index 784a0d3559970..b9dbd6e98fcbf 100644 --- a/crates/ide-assists/src/handlers/bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -6,6 +6,7 @@ use ide_db::{ imports::insert_use::{insert_use, ImportScope}, search::{FileReference, UsageSearchResult}, source_change::SourceChangeBuilder, + FxHashSet, }; use itertools::Itertools; use syntax::{ @@ -17,6 +18,7 @@ use syntax::{ }, ted, AstNode, NodeOrToken, SyntaxNode, T, }; +use text_edit::TextRange; use crate::assist_context::{AssistContext, Assists}; @@ -52,7 +54,7 @@ use crate::assist_context::{AssistContext, Assists}; pub(crate) fn bool_to_enum(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let BoolNodeData { target_node, name, ty_annotation, initializer, definition } = find_bool_node(ctx)?; - let target_module = ctx.sema.scope(&target_node)?.module(); + let target_module = ctx.sema.scope(&target_node)?.module().nearest_non_block_module(ctx.db()); let target = name.syntax().text_range(); acc.add( @@ -70,9 +72,8 @@ pub(crate) fn bool_to_enum(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option } let usages = definition.usages(&ctx.sema).all(); - add_enum_def(edit, ctx, &usages, target_node, &target_module); - replace_usages(edit, ctx, &usages, &target_module); + replace_usages(edit, ctx, &usages, definition, &target_module); }, ) } @@ -144,14 +145,14 @@ fn find_bool_node(ctx: &AssistContext<'_>) -> Option { return None; } - let strukt = field.syntax().ancestors().find_map(ast::Struct::cast)?; + let adt = field.syntax().ancestors().find_map(ast::Adt::cast)?; let def = ctx.sema.to_def(&field)?; if !def.ty(ctx.db()).is_bool() { cov_mark::hit!(not_applicable_non_bool_field); return None; } Some(BoolNodeData { - target_node: strukt.syntax().clone(), + target_node: adt.syntax().clone(), name, ty_annotation: field.ty(), initializer: None, @@ -192,78 +193,171 @@ fn replace_usages( edit: &mut SourceChangeBuilder, ctx: &AssistContext<'_>, usages: &UsageSearchResult, + target_definition: Definition, target_module: &hir::Module, ) { for (file_id, references) in usages.iter() { edit.edit_file(*file_id); - // add imports across modules where needed - references - .iter() - .filter_map(|FileReference { name, .. }| { - ctx.sema.scope(name.syntax()).map(|scope| (name, scope.module())) - }) - .unique_by(|name_and_module| name_and_module.1) - .filter(|(_, module)| module != target_module) - .filter_map(|(name, module)| { - let import_scope = ImportScope::find_insert_use_container(name.syntax(), &ctx.sema); - let mod_path = module.find_use_path_prefixed( - ctx.sema.db, - ModuleDef::Module(*target_module), - ctx.config.insert_use.prefix_kind, - ctx.config.prefer_no_std, - ); - import_scope.zip(mod_path) - }) - .for_each(|(import_scope, mod_path)| { - let import_scope = match import_scope { - ImportScope::File(it) => ImportScope::File(edit.make_mut(it)), - ImportScope::Module(it) => ImportScope::Module(edit.make_mut(it)), - ImportScope::Block(it) => ImportScope::Block(edit.make_mut(it)), - }; - let path = - make::path_concat(mod_path_to_ast(&mod_path), make::path_from_text("Bool")); - insert_use(&import_scope, path, &ctx.config.insert_use); - }); - - // replace the usages in expressions - references - .into_iter() - .filter_map(|FileReference { range, name, .. }| match name { - ast::NameLike::NameRef(name) => Some((*range, name)), - _ => None, - }) - .rev() - .for_each(|(range, name_ref)| { - if let Some(initializer) = find_assignment_usage(name_ref) { + let refs_with_imports = + augment_references_with_imports(edit, ctx, references, target_module); + + refs_with_imports.into_iter().rev().for_each( + |FileReferenceWithImport { range, old_name, new_name, import_data }| { + // replace the usages in patterns and expressions + if let Some(ident_pat) = old_name.syntax().ancestors().find_map(ast::IdentPat::cast) + { + cov_mark::hit!(replaces_record_pat_shorthand); + + let definition = ctx.sema.to_def(&ident_pat).map(Definition::Local); + if let Some(def) = definition { + replace_usages( + edit, + ctx, + &def.usages(&ctx.sema).all(), + target_definition, + target_module, + ) + } + } else if let Some(initializer) = find_assignment_usage(&new_name) { cov_mark::hit!(replaces_assignment); replace_bool_expr(edit, initializer); - } else if let Some((prefix_expr, inner_expr)) = find_negated_usage(name_ref) { + } else if let Some((prefix_expr, inner_expr)) = find_negated_usage(&new_name) { cov_mark::hit!(replaces_negation); edit.replace( prefix_expr.syntax().text_range(), format!("{} == Bool::False", inner_expr), ); - } else if let Some((record_field, initializer)) = find_record_expr_usage(name_ref) { + } else if let Some((record_field, initializer)) = old_name + .as_name_ref() + .and_then(ast::RecordExprField::for_field_name) + .and_then(|record_field| ctx.sema.resolve_record_field(&record_field)) + .and_then(|(got_field, _, _)| { + find_record_expr_usage(&new_name, got_field, target_definition) + }) + { cov_mark::hit!(replaces_record_expr); let record_field = edit.make_mut(record_field); let enum_expr = bool_expr_to_enum_expr(initializer); record_field.replace_expr(enum_expr); - } else if name_ref.syntax().ancestors().find_map(ast::UseTree::cast).is_none() { + } else if let Some(pat) = find_record_pat_field_usage(&old_name) { + match pat { + ast::Pat::IdentPat(ident_pat) => { + cov_mark::hit!(replaces_record_pat); + + let definition = ctx.sema.to_def(&ident_pat).map(Definition::Local); + if let Some(def) = definition { + replace_usages( + edit, + ctx, + &def.usages(&ctx.sema).all(), + target_definition, + target_module, + ) + } + } + ast::Pat::LiteralPat(literal_pat) => { + cov_mark::hit!(replaces_literal_pat); + + if let Some(expr) = literal_pat.literal().and_then(|literal| { + literal.syntax().ancestors().find_map(ast::Expr::cast) + }) { + replace_bool_expr(edit, expr); + } + } + _ => (), + } + } else if new_name.syntax().ancestors().find_map(ast::UseTree::cast).is_none() { // for any other usage in an expression, replace it with a check that it is the true variant - edit.replace(range, format!("{} == Bool::True", name_ref.text())); + if let Some((record_field, expr)) = new_name + .as_name_ref() + .and_then(ast::RecordExprField::for_field_name) + .and_then(|record_field| { + record_field.expr().map(|expr| (record_field, expr)) + }) + { + record_field.replace_expr( + make::expr_bin_op( + expr, + ast::BinaryOp::CmpOp(ast::CmpOp::Eq { negated: false }), + make::expr_path(make::path_from_text("Bool::True")), + ) + .clone_for_update(), + ); + } else { + edit.replace(range, format!("{} == Bool::True", new_name.text())); + } + } + + // add imports across modules where needed + if let Some((import_scope, path)) = import_data { + insert_use(&import_scope, path, &ctx.config.insert_use); } - }) + }, + ) } } -fn find_assignment_usage(name_ref: &ast::NameRef) -> Option { - let bin_expr = name_ref.syntax().ancestors().find_map(ast::BinExpr::cast)?; +struct FileReferenceWithImport { + range: TextRange, + old_name: ast::NameLike, + new_name: ast::NameLike, + import_data: Option<(ImportScope, ast::Path)>, +} - if !bin_expr.lhs()?.syntax().descendants().contains(name_ref.syntax()) { +fn augment_references_with_imports( + edit: &mut SourceChangeBuilder, + ctx: &AssistContext<'_>, + references: &[FileReference], + target_module: &hir::Module, +) -> Vec { + let mut visited_modules = FxHashSet::default(); + + references + .iter() + .filter_map(|FileReference { range, name, .. }| { + ctx.sema.scope(name.syntax()).map(|scope| (*range, name, scope.module())) + }) + .map(|(range, name, ref_module)| { + let old_name = name.clone(); + let new_name = edit.make_mut(name.clone()); + + // if the referenced module is not the same as the target one and has not been seen before, add an import + let import_data = if ref_module.nearest_non_block_module(ctx.db()) != *target_module + && !visited_modules.contains(&ref_module) + { + visited_modules.insert(ref_module); + + let import_scope = + ImportScope::find_insert_use_container(new_name.syntax(), &ctx.sema); + let path = ref_module + .find_use_path_prefixed( + ctx.sema.db, + ModuleDef::Module(*target_module), + ctx.config.insert_use.prefix_kind, + ctx.config.prefer_no_std, + ) + .map(|mod_path| { + make::path_concat(mod_path_to_ast(&mod_path), make::path_from_text("Bool")) + }); + + import_scope.zip(path) + } else { + None + }; + + FileReferenceWithImport { range, old_name, new_name, import_data } + }) + .collect() +} + +fn find_assignment_usage(name: &ast::NameLike) -> Option { + let bin_expr = name.syntax().ancestors().find_map(ast::BinExpr::cast)?; + + if !bin_expr.lhs()?.syntax().descendants().contains(name.syntax()) { cov_mark::hit!(dont_assign_incorrect_ref); return None; } @@ -275,8 +369,8 @@ fn find_assignment_usage(name_ref: &ast::NameRef) -> Option { } } -fn find_negated_usage(name_ref: &ast::NameRef) -> Option<(ast::PrefixExpr, ast::Expr)> { - let prefix_expr = name_ref.syntax().ancestors().find_map(ast::PrefixExpr::cast)?; +fn find_negated_usage(name: &ast::NameLike) -> Option<(ast::PrefixExpr, ast::Expr)> { + let prefix_expr = name.syntax().ancestors().find_map(ast::PrefixExpr::cast)?; if !matches!(prefix_expr.expr()?, ast::Expr::PathExpr(_) | ast::Expr::FieldExpr(_)) { cov_mark::hit!(dont_overwrite_expression_inside_negation); @@ -291,15 +385,31 @@ fn find_negated_usage(name_ref: &ast::NameRef) -> Option<(ast::PrefixExpr, ast:: } } -fn find_record_expr_usage(name_ref: &ast::NameRef) -> Option<(ast::RecordExprField, ast::Expr)> { - let record_field = name_ref.syntax().ancestors().find_map(ast::RecordExprField::cast)?; +fn find_record_expr_usage( + name: &ast::NameLike, + got_field: hir::Field, + target_definition: Definition, +) -> Option<(ast::RecordExprField, ast::Expr)> { + let name_ref = name.as_name_ref()?; + let record_field = ast::RecordExprField::for_field_name(name_ref)?; let initializer = record_field.expr()?; - if record_field.field_name()?.syntax().descendants().contains(name_ref.syntax()) { - Some((record_field, initializer)) - } else { - cov_mark::hit!(dont_overwrite_wrong_record_field); - None + if let Definition::Field(expected_field) = target_definition { + if got_field != expected_field { + return None; + } + } + + Some((record_field, initializer)) +} + +fn find_record_pat_field_usage(name: &ast::NameLike) -> Option { + let record_pat_field = name.syntax().parent().and_then(ast::RecordPatField::cast)?; + let pat = record_pat_field.pat()?; + + match pat { + ast::Pat::IdentPat(_) | ast::Pat::LiteralPat(_) | ast::Pat::WildcardPat(_) => Some(pat), + _ => None, } } @@ -317,7 +427,7 @@ fn add_enum_def( .filter_map(|FileReference { name, .. }| { ctx.sema.scope(name.syntax()).map(|scope| scope.module()) }) - .any(|module| &module != target_module); + .any(|module| module.nearest_non_block_module(ctx.db()) != *target_module); let enum_def = make_bool_enum(make_enum_pub); let indent = IndentLevel::from_node(&target_node); @@ -646,7 +756,7 @@ fn main() { } #[test] - fn field_basic() { + fn field_struct_basic() { cov_mark::check!(replaces_record_expr); check_assist( bool_to_enum, @@ -684,6 +794,263 @@ fn main() { ) } + #[test] + fn field_enum_basic() { + cov_mark::check!(replaces_record_pat); + check_assist( + bool_to_enum, + r#" +enum Foo { + Foo, + Bar { $0bar: bool }, +} + +fn main() { + let foo = Foo::Bar { bar: true }; + + if let Foo::Bar { bar: baz } = foo { + if baz { + println!("foo"); + } + } +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +enum Foo { + Foo, + Bar { bar: Bool }, +} + +fn main() { + let foo = Foo::Bar { bar: Bool::True }; + + if let Foo::Bar { bar: baz } = foo { + if baz == Bool::True { + println!("foo"); + } + } +} +"#, + ) + } + + #[test] + fn field_enum_cross_file() { + check_assist( + bool_to_enum, + r#" +//- /foo.rs +pub enum Foo { + Foo, + Bar { $0bar: bool }, +} + +fn foo() { + let foo = Foo::Bar { bar: true }; +} + +//- /main.rs +use foo::Foo; + +mod foo; + +fn main() { + let foo = Foo::Bar { bar: false }; +} +"#, + r#" +//- /foo.rs +#[derive(PartialEq, Eq)] +pub enum Bool { True, False } + +pub enum Foo { + Foo, + Bar { bar: Bool }, +} + +fn foo() { + let foo = Foo::Bar { bar: Bool::True }; +} + +//- /main.rs +use foo::{Foo, Bool}; + +mod foo; + +fn main() { + let foo = Foo::Bar { bar: Bool::False }; +} +"#, + ) + } + + #[test] + fn field_enum_shorthand() { + cov_mark::check!(replaces_record_pat_shorthand); + check_assist( + bool_to_enum, + r#" +enum Foo { + Foo, + Bar { $0bar: bool }, +} + +fn main() { + let foo = Foo::Bar { bar: true }; + + match foo { + Foo::Bar { bar } => { + if bar { + println!("foo"); + } + } + _ => (), + } +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +enum Foo { + Foo, + Bar { bar: Bool }, +} + +fn main() { + let foo = Foo::Bar { bar: Bool::True }; + + match foo { + Foo::Bar { bar } => { + if bar == Bool::True { + println!("foo"); + } + } + _ => (), + } +} +"#, + ) + } + + #[test] + fn field_enum_replaces_literal_patterns() { + cov_mark::check!(replaces_literal_pat); + check_assist( + bool_to_enum, + r#" +enum Foo { + Foo, + Bar { $0bar: bool }, +} + +fn main() { + let foo = Foo::Bar { bar: true }; + + if let Foo::Bar { bar: true } = foo { + println!("foo"); + } +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +enum Foo { + Foo, + Bar { bar: Bool }, +} + +fn main() { + let foo = Foo::Bar { bar: Bool::True }; + + if let Foo::Bar { bar: Bool::True } = foo { + println!("foo"); + } +} +"#, + ) + } + + #[test] + fn field_enum_keeps_wildcard_patterns() { + check_assist( + bool_to_enum, + r#" +enum Foo { + Foo, + Bar { $0bar: bool }, +} + +fn main() { + let foo = Foo::Bar { bar: true }; + + if let Foo::Bar { bar: _ } = foo { + println!("foo"); + } +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +enum Foo { + Foo, + Bar { bar: Bool }, +} + +fn main() { + let foo = Foo::Bar { bar: Bool::True }; + + if let Foo::Bar { bar: _ } = foo { + println!("foo"); + } +} +"#, + ) + } + + #[test] + fn field_union_basic() { + check_assist( + bool_to_enum, + r#" +union Foo { + $0foo: bool, + bar: usize, +} + +fn main() { + let foo = Foo { foo: true }; + + if unsafe { foo.foo } { + println!("foo"); + } +} +"#, + r#" +#[derive(PartialEq, Eq)] +enum Bool { True, False } + +union Foo { + foo: Bool, + bar: usize, +} + +fn main() { + let foo = Foo { foo: Bool::True }; + + if unsafe { foo.foo == Bool::True } { + println!("foo"); + } +} +"#, + ) + } + #[test] fn field_negated() { check_assist( @@ -841,7 +1208,6 @@ fn main() { #[test] fn field_initialized_with_other() { - cov_mark::check!(dont_overwrite_wrong_record_field); check_assist( bool_to_enum, r#" From 0863024b1a03b61dbdff518eca52e550efe67b8f Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Mon, 11 Sep 2023 13:31:42 +0200 Subject: [PATCH 22/49] Make assist lazy again --- .../src/handlers/convert_comment_block.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/ide-assists/src/handlers/convert_comment_block.rs b/crates/ide-assists/src/handlers/convert_comment_block.rs index 1f048ac10fa55..ef914cdb2cdf4 100644 --- a/crates/ide-assists/src/handlers/convert_comment_block.rs +++ b/crates/ide-assists/src/handlers/convert_comment_block.rs @@ -79,19 +79,21 @@ fn line_to_block(acc: &mut Assists, comment: ast::Comment) -> Option<()> { comments.last()?.syntax().text_range().end(), ); - // We pick a single indentation level for the whole block comment based on the - // comment where the assist was invoked. This will be prepended to the - // contents of each line comment when they're put into the block comment. - let indentation = IndentLevel::from_token(comment.syntax()); - - let cms = - comments.into_iter().map(|c| line_comment_text(indentation, c)).collect::>(); - acc.add( AssistId("line_to_block", AssistKind::RefactorRewrite), "Replace line comments with a single block comment", target, |edit| { + // We pick a single indentation level for the whole block comment based on the + // comment where the assist was invoked. This will be prepended to the + // contents of each line comment when they're put into the block comment. + let indentation = IndentLevel::from_token(comment.syntax()); + + let cms = comments + .into_iter() + .map(|c| line_comment_text(indentation, c)) + .collect::>(); + let block_comment_body = cms.into_iter().join("\n"); let block_prefix = From 893e19137e85e0fa11187a100b7fc716f7a99c76 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Mon, 11 Sep 2023 13:33:26 +0200 Subject: [PATCH 23/49] Make assist lazy again --- crates/ide-assists/src/handlers/inline_call.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ide-assists/src/handlers/inline_call.rs b/crates/ide-assists/src/handlers/inline_call.rs index 9b4ac8a02a350..3c5a0be775e23 100644 --- a/crates/ide-assists/src/handlers/inline_call.rs +++ b/crates/ide-assists/src/handlers/inline_call.rs @@ -218,12 +218,12 @@ pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option< } let syntax = call_info.node.syntax().clone(); - let replacement = inline(&ctx.sema, file_id, function, &fn_body, ¶ms, &call_info); acc.add( AssistId("inline_call", AssistKind::RefactorInline), label, syntax.text_range(), |builder| { + let replacement = inline(&ctx.sema, file_id, function, &fn_body, ¶ms, &call_info); builder.replace_ast( match call_info.node { ast::CallableExpr::Call(it) => ast::Expr::CallExpr(it), From 145a101fe86f570f4a7b236c8a4818f4ce7873c8 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Mon, 11 Sep 2023 14:09:19 +0200 Subject: [PATCH 24/49] Deunwrap add_missing_match_arms --- .../src/handlers/add_missing_match_arms.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/crates/ide-assists/src/handlers/add_missing_match_arms.rs b/crates/ide-assists/src/handlers/add_missing_match_arms.rs index 3b162d7c4d82f..5376ece2f584b 100644 --- a/crates/ide-assists/src/handlers/add_missing_match_arms.rs +++ b/crates/ide-assists/src/handlers/add_missing_match_arms.rs @@ -273,9 +273,10 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>) syntax::SyntaxElement::Token(it) => { // Don't have a way to make tokens mut, so instead make the parent mut // and find the token again - let parent = edit.make_syntax_mut(it.parent().unwrap()); + let parent = + edit.make_syntax_mut(it.parent().expect("Token must have a parent.")); let mut_token = - parent.covering_element(it.text_range()).into_token().unwrap(); + parent.covering_element(it.text_range()).into_token().expect("Covering element cannot be found. Range may be beyond the current node's range"); syntax::SyntaxElement::from(mut_token) } @@ -446,21 +447,23 @@ fn build_pat( mod_path_to_ast(&module.find_use_path(db, ModuleDef::from(var), prefer_no_std)?); // FIXME: use HIR for this; it doesn't currently expose struct vs. tuple vs. unit variants though - let pat: ast::Pat = match var.source(db)?.value.kind() { + Some(match var.source(db)?.value.kind() { ast::StructKind::Tuple(field_list) => { let pats = iter::repeat(make::wildcard_pat().into()).take(field_list.fields().count()); make::tuple_struct_pat(path, pats).into() } ast::StructKind::Record(field_list) => { - let pats = field_list - .fields() - .map(|f| make::ext::simple_ident_pat(f.name().unwrap()).into()); + let pats = field_list.fields().map(|f| { + make::ext::simple_ident_pat( + f.name().expect("Record field must have a name"), + ) + .into() + }); make::record_pat(path, pats).into() } ast::StructKind::Unit => make::path_pat(path), - }; - Some(pat) + }) } ExtendedVariant::True => Some(ast::Pat::from(make::literal_pat("true"))), ExtendedVariant::False => Some(ast::Pat::from(make::literal_pat("false"))), From d79486529e5f39216f202971c076997486a0b1c5 Mon Sep 17 00:00:00 2001 From: dfireBird Date: Sat, 22 Jul 2023 19:51:34 +0530 Subject: [PATCH 25/49] remove `as _` on auto importing on trait that is aliased with `_` --- crates/ide-db/src/imports/merge_imports.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/ide-db/src/imports/merge_imports.rs b/crates/ide-db/src/imports/merge_imports.rs index 27b6321f3a7a5..ff84e9ffaeec7 100644 --- a/crates/ide-db/src/imports/merge_imports.rs +++ b/crates/ide-db/src/imports/merge_imports.rs @@ -78,6 +78,10 @@ fn try_merge_trees_mut(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehav { lhs.split_prefix(&lhs_prefix); rhs.split_prefix(&rhs_prefix); + } else { + ted::replace(lhs.syntax(), rhs.syntax()); + // we can safely return here, in this case `recursive_merge` doesn't do anything + return Some(()); } recursive_merge(lhs, rhs, merge) } @@ -123,6 +127,13 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior) // so they need to be handled explicitly .or_else(|| tree.star_token().map(|_| false)) }; + + if lhs_t.rename().and_then(|x| x.underscore_token()).is_some() { + ted::replace(lhs_t.syntax(), rhs_t.syntax()); + *lhs_t = rhs_t; + continue; + } + match (tree_contains_self(lhs_t), tree_contains_self(&rhs_t)) { (Some(true), None) => continue, (None, Some(true)) => { From df1239bf9246423f1006e4affb38ee504ef3dc79 Mon Sep 17 00:00:00 2001 From: dfireBird Date: Mon, 11 Sep 2023 17:36:09 +0530 Subject: [PATCH 26/49] add tests for insert use with renamed imports Tested for two cases: 1. Simple Use 2. Complex Use --- crates/ide-db/src/imports/insert_use/tests.rs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/crates/ide-db/src/imports/insert_use/tests.rs b/crates/ide-db/src/imports/insert_use/tests.rs index b92e367f7e12a..01d2f1970c38e 100644 --- a/crates/ide-db/src/imports/insert_use/tests.rs +++ b/crates/ide-db/src/imports/insert_use/tests.rs @@ -993,6 +993,46 @@ use foo::bar::qux; ); } +#[test] +fn insert_with_renamed_import_simple_use() { + check_with_config( + "use self::foo::Foo", + r#" +use self::foo::Foo as _; +"#, + r#" +use self::foo::Foo; +"#, + &InsertUseConfig { + granularity: ImportGranularity::Crate, + prefix_kind: hir::PrefixKind::BySelf, + enforce_granularity: true, + group: true, + skip_glob_imports: true, + }, + ); +} + +#[test] +fn insert_with_renamed_import_complex_use() { + check_with_config( + "use self::foo::Foo;", + r#" +use self::foo::{self, Foo as _, Bar}; +"#, + r#" +use self::foo::{self, Foo, Bar}; +"#, + &InsertUseConfig { + granularity: ImportGranularity::Crate, + prefix_kind: hir::PrefixKind::BySelf, + enforce_granularity: true, + group: true, + skip_glob_imports: true, + }, + ); +} + fn check_with_config( path: &str, ra_fixture_before: &str, From ebbbaaa90f4dd111b1bb9df7250925f612110925 Mon Sep 17 00:00:00 2001 From: shogo-nakano-desu <61229807+shogo-nakano-desu@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:43:21 +0900 Subject: [PATCH 27/49] refactor: fix clippy lints --- crates/intern/src/lib.rs | 14 +++------- crates/parser/src/shortcuts.rs | 36 ++++++++++++-------------- crates/syntax/src/ast/edit_in_place.rs | 14 +++++++--- crates/syntax/src/lib.rs | 36 ++++++++++++-------------- crates/syntax/src/tests.rs | 4 +-- 5 files changed, 50 insertions(+), 54 deletions(-) diff --git a/crates/intern/src/lib.rs b/crates/intern/src/lib.rs index 2934d26674d7e..d784321c7c7af 100644 --- a/crates/intern/src/lib.rs +++ b/crates/intern/src/lib.rs @@ -33,13 +33,10 @@ impl Interned { // - if not, box it up, insert it, and return a clone // This needs to be atomic (locking the shard) to avoid races with other thread, which could // insert the same object between us looking it up and inserting it. - match shard.raw_entry_mut().from_key_hashed_nocheck(hash as u64, &obj) { + match shard.raw_entry_mut().from_key_hashed_nocheck(hash, &obj) { RawEntryMut::Occupied(occ) => Self { arc: occ.key().clone() }, RawEntryMut::Vacant(vac) => Self { - arc: vac - .insert_hashed_nocheck(hash as u64, Arc::new(obj), SharedValue::new(())) - .0 - .clone(), + arc: vac.insert_hashed_nocheck(hash, Arc::new(obj), SharedValue::new(())).0.clone(), }, } } @@ -54,13 +51,10 @@ impl Interned { // - if not, box it up, insert it, and return a clone // This needs to be atomic (locking the shard) to avoid races with other thread, which could // insert the same object between us looking it up and inserting it. - match shard.raw_entry_mut().from_key_hashed_nocheck(hash as u64, s) { + match shard.raw_entry_mut().from_key_hashed_nocheck(hash, s) { RawEntryMut::Occupied(occ) => Self { arc: occ.key().clone() }, RawEntryMut::Vacant(vac) => Self { - arc: vac - .insert_hashed_nocheck(hash as u64, Arc::from(s), SharedValue::new(())) - .0 - .clone(), + arc: vac.insert_hashed_nocheck(hash, Arc::from(s), SharedValue::new(())).0.clone(), }, } } diff --git a/crates/parser/src/shortcuts.rs b/crates/parser/src/shortcuts.rs index 2c47e3d086d61..57005a6834c90 100644 --- a/crates/parser/src/shortcuts.rs +++ b/crates/parser/src/shortcuts.rs @@ -32,29 +32,27 @@ impl LexedStr<'_> { let kind = self.kind(i); if kind.is_trivia() { was_joint = false + } else if kind == SyntaxKind::IDENT { + let token_text = self.text(i); + let contextual_kw = + SyntaxKind::from_contextual_keyword(token_text).unwrap_or(SyntaxKind::IDENT); + res.push_ident(contextual_kw); } else { - if kind == SyntaxKind::IDENT { - let token_text = self.text(i); - let contextual_kw = SyntaxKind::from_contextual_keyword(token_text) - .unwrap_or(SyntaxKind::IDENT); - res.push_ident(contextual_kw); - } else { - if was_joint { + if was_joint { + res.was_joint(); + } + res.push(kind); + // Tag the token as joint if it is float with a fractional part + // we use this jointness to inform the parser about what token split + // event to emit when we encounter a float literal in a field access + if kind == SyntaxKind::FLOAT_NUMBER { + if !self.text(i).ends_with('.') { res.was_joint(); - } - res.push(kind); - // Tag the token as joint if it is float with a fractional part - // we use this jointness to inform the parser about what token split - // event to emit when we encounter a float literal in a field access - if kind == SyntaxKind::FLOAT_NUMBER { - if !self.text(i).ends_with('.') { - res.was_joint(); - } else { - was_joint = false; - } } else { - was_joint = true; + was_joint = false; } + } else { + was_joint = true; } } } diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index a150d9e6c07d8..a85e1d1d9d0eb 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -224,7 +224,7 @@ pub trait AttrsOwnerEdit: ast::HasAttrs { let after_attrs_and_comments = node .children_with_tokens() .find(|it| !matches!(it.kind(), WHITESPACE | COMMENT | ATTR)) - .map_or(Position::first_child_of(node), |it| Position::before(it)); + .map_or(Position::first_child_of(node), Position::before); ted::insert_all( after_attrs_and_comments, @@ -433,7 +433,9 @@ impl ast::UseTree { if &path == prefix && self.use_tree_list().is_none() { if self.star_token().is_some() { // path$0::* -> * - self.coloncolon_token().map(ted::remove); + if let Some(a) = self.coloncolon_token() { + ted::remove(a) + } ted::remove(prefix.syntax()); } else { // path$0 -> self @@ -460,7 +462,9 @@ impl ast::UseTree { for p in successors(parent.parent_path(), |it| it.parent_path()) { p.segment()?; } - prefix.parent_path().and_then(|p| p.coloncolon_token()).map(ted::remove); + if let Some(a) = prefix.parent_path().and_then(|p| p.coloncolon_token()) { + ted::remove(a) + } ted::remove(prefix.syntax()); Some(()) } @@ -976,7 +980,9 @@ enum Foo { fn check_add_variant(before: &str, expected: &str, variant: ast::Variant) { let enum_ = ast_mut_from_text::(before); - enum_.variant_list().map(|it| it.add_variant(variant)); + if let Some(it) = enum_.variant_list() { + it.add_variant(variant) + } let after = enum_.to_string(); assert_eq_text!(&trim_indent(expected.trim()), &trim_indent(after.trim())); } diff --git a/crates/syntax/src/lib.rs b/crates/syntax/src/lib.rs index 27c8a13e58d66..73eff358462a3 100644 --- a/crates/syntax/src/lib.rs +++ b/crates/syntax/src/lib.rs @@ -181,29 +181,27 @@ impl ast::TokenTree { let kind = t.kind(); if kind.is_trivia() { was_joint = false + } else if kind == SyntaxKind::IDENT { + let token_text = t.text(); + let contextual_kw = + SyntaxKind::from_contextual_keyword(token_text).unwrap_or(SyntaxKind::IDENT); + parser_input.push_ident(contextual_kw); } else { - if kind == SyntaxKind::IDENT { - let token_text = t.text(); - let contextual_kw = SyntaxKind::from_contextual_keyword(token_text) - .unwrap_or(SyntaxKind::IDENT); - parser_input.push_ident(contextual_kw); - } else { - if was_joint { + if was_joint { + parser_input.was_joint(); + } + parser_input.push(kind); + // Tag the token as joint if it is float with a fractional part + // we use this jointness to inform the parser about what token split + // event to emit when we encounter a float literal in a field access + if kind == SyntaxKind::FLOAT_NUMBER { + if !t.text().ends_with('.') { parser_input.was_joint(); - } - parser_input.push(kind); - // Tag the token as joint if it is float with a fractional part - // we use this jointness to inform the parser about what token split - // event to emit when we encounter a float literal in a field access - if kind == SyntaxKind::FLOAT_NUMBER { - if !t.text().ends_with('.') { - parser_input.was_joint(); - } else { - was_joint = false; - } } else { - was_joint = true; + was_joint = false; } + } else { + was_joint = true; } } } diff --git a/crates/syntax/src/tests.rs b/crates/syntax/src/tests.rs index 168439053c27a..3010d77d827e1 100644 --- a/crates/syntax/src/tests.rs +++ b/crates/syntax/src/tests.rs @@ -17,11 +17,11 @@ use crate::{ast, fuzz, AstNode, SourceFile, SyntaxError}; #[test] fn parse_smoke_test() { - let code = r##" + let code = r#" fn main() { println!("Hello, world!") } - "##; + "#; let parse = SourceFile::parse(code); // eprintln!("{:#?}", parse.syntax_node()); From 0bb2298ac604e0654652418e63a9eb15a82f155c Mon Sep 17 00:00:00 2001 From: shogo-nakano-desu <61229807+shogo-nakano-desu@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:43:31 +0900 Subject: [PATCH 28/49] refactor: remove TODO with no explanation --- crates/syntax/src/ast/make.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 17e311c0c502f..20cd6abb5a812 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -433,7 +433,6 @@ pub fn record_field( ast_from_text(&format!("struct S {{ {visibility}{name}: {ty}, }}")) } -// TODO pub fn block_expr( stmts: impl IntoIterator, tail_expr: Option, From 96c333262a51d98a11fb0fed949e5ef55e40cd87 Mon Sep 17 00:00:00 2001 From: shogo-nakano-desu <61229807+shogo-nakano-desu@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:47:39 +0900 Subject: [PATCH 29/49] refactor: fix clippy lint --- crates/flycheck/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index 2de719af92ce9..61433313921a7 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -488,7 +488,9 @@ impl CargoActor { // Skip certain kinds of messages to only spend time on what's useful JsonMessage::Cargo(message) => match message { cargo_metadata::Message::CompilerArtifact(artifact) if !artifact.fresh => { - self.sender.send(CargoMessage::CompilerArtifact(artifact)).unwrap(); + self.sender + .send(CargoMessage::CompilerArtifact(Box::new(artifact))) + .unwrap(); } cargo_metadata::Message::CompilerMessage(msg) => { self.sender.send(CargoMessage::Diagnostic(msg.message)).unwrap(); @@ -533,7 +535,7 @@ impl CargoActor { } enum CargoMessage { - CompilerArtifact(cargo_metadata::Artifact), + CompilerArtifact(Box), Diagnostic(Diagnostic), } From f4704bc8ae92d150b2801e391692d9503f0c6126 Mon Sep 17 00:00:00 2001 From: hkalbasi Date: Fri, 15 Sep 2023 18:10:11 +0330 Subject: [PATCH 30/49] Switch to in-tree rustc dependencies with a cfg flag --- Cargo.lock | 22 ++++++++++----- Cargo.toml | 18 +++++-------- crates/hir-def/Cargo.toml | 8 +++--- crates/hir-def/src/data/adt.rs | 2 +- crates/hir-def/src/hir/format_args.rs | 2 +- crates/hir-def/src/lib.rs | 3 ++- crates/hir-ty/Cargo.toml | 5 +++- crates/hir-ty/src/layout.rs | 2 +- crates/parser/Cargo.toml | 5 +++- crates/parser/src/lexed_str.rs | 1 + crates/parser/src/lib.rs | 1 + crates/rust-analyzer/Cargo.toml | 12 ++++++++- crates/rustc-dependencies/Cargo.toml | 20 ++++++++++++++ crates/rustc-dependencies/src/lib.rs | 39 +++++++++++++++++++++++++++ crates/syntax/Cargo.toml | 4 +-- crates/syntax/src/ast/token_ext.rs | 2 ++ crates/syntax/src/lib.rs | 1 + crates/syntax/src/validation.rs | 2 +- 18 files changed, 118 insertions(+), 31 deletions(-) create mode 100644 crates/rustc-dependencies/Cargo.toml create mode 100644 crates/rustc-dependencies/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index bd6554bf8899a..e506aa834eba8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -531,8 +531,6 @@ dependencies = [ "fst", "hashbrown 0.12.3", "hir-expand", - "hkalbasi-rustc-ap-rustc_abi", - "hkalbasi-rustc-ap-rustc_index", "indexmap 2.0.0", "intern", "itertools", @@ -541,7 +539,7 @@ dependencies = [ "mbe", "once_cell", "profile", - "ra-ap-rustc_parse_format", + "rustc-dependencies", "rustc-hash", "smallvec", "stdx", @@ -594,7 +592,6 @@ dependencies = [ "expect-test", "hir-def", "hir-expand", - "hkalbasi-rustc-ap-rustc_index", "intern", "itertools", "la-arena 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -604,6 +601,7 @@ dependencies = [ "oorandom", "profile", "project-model", + "rustc-dependencies", "rustc-hash", "scoped-tls", "smallvec", @@ -1277,7 +1275,7 @@ dependencies = [ "drop_bomb", "expect-test", "limit", - "ra-ap-rustc_lexer", + "rustc-dependencies", "sourcegen", "stdx", ] @@ -1594,10 +1592,12 @@ dependencies = [ "oorandom", "parking_lot 0.12.1", "parking_lot_core 0.9.6", + "parser", "proc-macro-api", "profile", "project-model", "rayon", + "rustc-dependencies", "rustc-hash", "scip", "serde", @@ -1626,6 +1626,16 @@ version = "0.1.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" +[[package]] +name = "rustc-dependencies" +version = "0.0.0" +dependencies = [ + "hkalbasi-rustc-ap-rustc_abi", + "hkalbasi-rustc-ap-rustc_index", + "ra-ap-rustc_lexer", + "ra-ap-rustc_parse_format", +] + [[package]] name = "rustc-hash" version = "1.1.0" @@ -1853,9 +1863,9 @@ dependencies = [ "proc-macro2", "profile", "quote", - "ra-ap-rustc_lexer", "rayon", "rowan", + "rustc-dependencies", "rustc-hash", "smol_str", "sourcegen", diff --git a/Cargo.toml b/Cargo.toml index cab88fc18cec6..ffac946b18f97 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -79,6 +79,7 @@ toolchain = { path = "./crates/toolchain", version = "0.0.0" } tt = { path = "./crates/tt", version = "0.0.0" } vfs-notify = { path = "./crates/vfs-notify", version = "0.0.0" } vfs = { path = "./crates/vfs", version = "0.0.0" } +rustc-dependencies = { path = "./crates/rustc-dependencies", version = "0.0.0" } # local crates that aren't published to crates.io. These should not have versions. proc-macro-test = { path = "./crates/proc-macro-test" } @@ -90,9 +91,9 @@ lsp-server = { version = "0.7.4" } # non-local crates smallvec = { version = "1.10.0", features = [ - "const_new", - "union", - "const_generics", + "const_new", + "union", + "const_generics", ] } smol_str = "0.2.0" nohash-hasher = "0.2.0" @@ -101,11 +102,6 @@ serde = { version = "1.0.156", features = ["derive"] } serde_json = "1.0.96" triomphe = { version = "0.1.8", default-features = false, features = ["std"] } # can't upgrade due to dashmap depending on 0.12.3 currently -hashbrown = { version = "0.12.3", features = ["inline-more"], default-features = false } - -rustc_lexer = { version = "0.10.0", package = "ra-ap-rustc_lexer" } -rustc_parse_format = { version = "0.10.0", package = "ra-ap-rustc_parse_format", default-features = false } - -# Upstream broke this for us so we can't update it -rustc_abi = { version = "0.0.20221221", package = "hkalbasi-rustc-ap-rustc_abi", default-features = false } -rustc_index = { version = "0.0.20221221", package = "hkalbasi-rustc-ap-rustc_index", default-features = false } +hashbrown = { version = "0.12.3", features = [ + "inline-more", +], default-features = false } diff --git a/crates/hir-def/Cargo.toml b/crates/hir-def/Cargo.toml index 8cf61ee04d4e6..092aab011205a 100644 --- a/crates/hir-def/Cargo.toml +++ b/crates/hir-def/Cargo.toml @@ -31,10 +31,7 @@ smallvec.workspace = true hashbrown.workspace = true triomphe.workspace = true -rustc_abi.workspace = true -rustc_index.workspace = true -rustc_parse_format.workspace = true - +rustc-dependencies.workspace = true # local deps stdx.workspace = true @@ -53,3 +50,6 @@ expect-test = "1.4.0" # local deps test-utils.workspace = true + +[features] +in-rust-tree = ["rustc-dependencies/in-rust-tree"] diff --git a/crates/hir-def/src/data/adt.rs b/crates/hir-def/src/data/adt.rs index 224f7328f8cdc..b163112db9184 100644 --- a/crates/hir-def/src/data/adt.rs +++ b/crates/hir-def/src/data/adt.rs @@ -11,7 +11,7 @@ use hir_expand::{ }; use intern::Interned; use la_arena::{Arena, ArenaMap}; -use rustc_abi::{Align, Integer, IntegerType, ReprFlags, ReprOptions}; +use rustc_dependencies::abi::{Align, Integer, IntegerType, ReprFlags, ReprOptions}; use syntax::ast::{self, HasName, HasVisibility}; use triomphe::Arc; diff --git a/crates/hir-def/src/hir/format_args.rs b/crates/hir-def/src/hir/format_args.rs index 75025a984fcd2..46d24bd4a6142 100644 --- a/crates/hir-def/src/hir/format_args.rs +++ b/crates/hir-def/src/hir/format_args.rs @@ -2,7 +2,7 @@ use std::mem; use hir_expand::name::Name; -use rustc_parse_format as parse; +use rustc_dependencies::parse_format as parse; use syntax::{ ast::{self, IsString}, AstToken, SmolStr, TextRange, diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index 3f87fe62b83cd..1abcb1835fd5b 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -8,6 +8,7 @@ //! actually true. #![warn(rust_2018_idioms, unused_lifetimes, semicolon_in_expressions_from_macros)] +#![cfg_attr(feature = "in-rust-tree", feature(rustc_private))] #[allow(unused)] macro_rules! eprintln { @@ -48,7 +49,7 @@ pub mod visibility; pub mod find_path; pub mod import_map; -pub use rustc_abi as layout; +pub use rustc_dependencies::abi as layout; use triomphe::Arc; #[cfg(test)] diff --git a/crates/hir-ty/Cargo.toml b/crates/hir-ty/Cargo.toml index b95ae05ccd458..6a0c26a8bc092 100644 --- a/crates/hir-ty/Cargo.toml +++ b/crates/hir-ty/Cargo.toml @@ -33,7 +33,7 @@ triomphe.workspace = true nohash-hasher.workspace = true typed-arena = "2.0.1" -rustc_index.workspace = true +rustc-dependencies.workspace = true # local deps stdx.workspace = true @@ -56,3 +56,6 @@ project-model = { path = "../project-model" } # local deps test-utils.workspace = true + +[features] +in-rust-tree = ["rustc-dependencies/in-rust-tree"] diff --git a/crates/hir-ty/src/layout.rs b/crates/hir-ty/src/layout.rs index 1a6106c0244c6..ee558956a761c 100644 --- a/crates/hir-ty/src/layout.rs +++ b/crates/hir-ty/src/layout.rs @@ -34,7 +34,7 @@ mod target; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct RustcEnumVariantIdx(pub LocalEnumVariantId); -impl rustc_index::vec::Idx for RustcEnumVariantIdx { +impl rustc_dependencies::index::vec::Idx for RustcEnumVariantIdx { fn new(idx: usize) -> Self { RustcEnumVariantIdx(Idx::from_raw(RawIdx::from(idx as u32))) } diff --git a/crates/parser/Cargo.toml b/crates/parser/Cargo.toml index 09e62c35278e9..efb326323f915 100644 --- a/crates/parser/Cargo.toml +++ b/crates/parser/Cargo.toml @@ -13,7 +13,7 @@ doctest = false [dependencies] drop_bomb = "0.1.5" -rustc_lexer.workspace = true +rustc-dependencies.workspace = true limit.workspace = true @@ -22,3 +22,6 @@ expect-test = "1.4.0" stdx.workspace = true sourcegen.workspace = true + +[features] +in-rust-tree = ["rustc-dependencies/in-rust-tree"] diff --git a/crates/parser/src/lexed_str.rs b/crates/parser/src/lexed_str.rs index 36c52953a0246..30c1c4f8c75be 100644 --- a/crates/parser/src/lexed_str.rs +++ b/crates/parser/src/lexed_str.rs @@ -8,6 +8,7 @@ //! Note that these tokens, unlike the tokens we feed into the parser, do //! include info about comments and whitespace. +use rustc_dependencies::lexer as rustc_lexer; use std::ops; use crate::{ diff --git a/crates/parser/src/lib.rs b/crates/parser/src/lib.rs index c155e8aaf67b3..fcfd1a50719bd 100644 --- a/crates/parser/src/lib.rs +++ b/crates/parser/src/lib.rs @@ -19,6 +19,7 @@ #![warn(rust_2018_idioms, unused_lifetimes, semicolon_in_expressions_from_macros)] #![allow(rustdoc::private_intra_doc_links)] +#![cfg_attr(feature = "in-rust-tree", feature(rustc_private))] mod lexed_str; mod token_set; diff --git a/crates/rust-analyzer/Cargo.toml b/crates/rust-analyzer/Cargo.toml index 7410f0a3a668e..0a5412c638c3f 100644 --- a/crates/rust-analyzer/Cargo.toml +++ b/crates/rust-analyzer/Cargo.toml @@ -57,6 +57,7 @@ flycheck.workspace = true hir-def.workspace = true hir-ty.workspace = true hir.workspace = true +rustc-dependencies.workspace = true ide-db.workspace = true # This should only be used in CLI ide-ssr.workspace = true @@ -67,6 +68,7 @@ profile.workspace = true project-model.workspace = true stdx.workspace = true syntax.workspace = true +parser.workspace = true toolchain.workspace = true vfs-notify.workspace = true vfs.workspace = true @@ -89,4 +91,12 @@ mbe.workspace = true jemalloc = ["jemallocator", "profile/jemalloc"] force-always-assert = ["always-assert/force"] sysroot-abi = [] -in-rust-tree = ["sysroot-abi", "ide/in-rust-tree", "syntax/in-rust-tree"] +in-rust-tree = [ + "sysroot-abi", + "ide/in-rust-tree", + "syntax/in-rust-tree", + "parser/in-rust-tree", + "rustc-dependencies/in-rust-tree", + "hir-def/in-rust-tree", + "hir-ty/in-rust-tree", +] diff --git a/crates/rustc-dependencies/Cargo.toml b/crates/rustc-dependencies/Cargo.toml new file mode 100644 index 0000000000000..901706d3d95f3 --- /dev/null +++ b/crates/rustc-dependencies/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "rustc-dependencies" +version = "0.0.0" +rust-version.workspace = true +edition.workspace = true +license.workspace = true +authors.workspace = true + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +ra-ap-rustc_lexer = { version = "0.10.0" } +ra-ap-rustc_parse_format = { version = "0.10.0", default-features = false } + +# Upstream broke this for us so we can't update it +hkalbasi-rustc-ap-rustc_abi = { version = "0.0.20221221", default-features = false } +hkalbasi-rustc-ap-rustc_index = { version = "0.0.20221221", default-features = false } + +[features] +in-rust-tree = [] diff --git a/crates/rustc-dependencies/src/lib.rs b/crates/rustc-dependencies/src/lib.rs new file mode 100644 index 0000000000000..c1d3f05f34e7f --- /dev/null +++ b/crates/rustc-dependencies/src/lib.rs @@ -0,0 +1,39 @@ +//! A wrapper around rustc internal crates, which enables switching between compiler provided +//! ones and stable ones published in crates.io + +#![cfg_attr(feature = "in-rust-tree", feature(rustc_private))] + +#[cfg(feature = "in-rust-tree")] +extern crate rustc_lexer; + +#[cfg(feature = "in-rust-tree")] +pub mod lexer { + pub use ::rustc_lexer::*; +} + +#[cfg(not(feature = "in-rust-tree"))] +pub mod lexer { + pub use ::ra_ap_rustc_lexer::*; +} + +#[cfg(feature = "in-rust-tree")] +extern crate rustc_parse_format; + +#[cfg(feature = "in-rust-tree")] +pub mod parse_format { + pub use ::rustc_parse_format::*; +} + +#[cfg(not(feature = "in-rust-tree"))] +pub mod parse_format { + pub use ::ra_ap_rustc_parse_format::*; +} + +// Upstream broke this for us so we can't update it +pub mod abi { + pub use ::hkalbasi_rustc_ap_rustc_abi::*; +} + +pub mod index { + pub use ::hkalbasi_rustc_ap_rustc_index::*; +} diff --git a/crates/syntax/Cargo.toml b/crates/syntax/Cargo.toml index 5ee0c4792846a..dc92366d1c7a2 100644 --- a/crates/syntax/Cargo.toml +++ b/crates/syntax/Cargo.toml @@ -23,7 +23,7 @@ indexmap = "2.0.0" smol_str.workspace = true triomphe.workspace = true -rustc_lexer.workspace = true +rustc-dependencies.workspace = true parser.workspace = true profile.workspace = true @@ -41,4 +41,4 @@ test-utils.workspace = true sourcegen.workspace = true [features] -in-rust-tree = [] +in-rust-tree = ["rustc-dependencies/in-rust-tree"] diff --git a/crates/syntax/src/ast/token_ext.rs b/crates/syntax/src/ast/token_ext.rs index 87fd51d703cff..8cc271d226c4e 100644 --- a/crates/syntax/src/ast/token_ext.rs +++ b/crates/syntax/src/ast/token_ext.rs @@ -2,6 +2,8 @@ use std::borrow::Cow; +use rustc_dependencies::lexer as rustc_lexer; + use rustc_lexer::unescape::{ unescape_byte, unescape_c_string, unescape_char, unescape_literal, CStrUnit, Mode, }; diff --git a/crates/syntax/src/lib.rs b/crates/syntax/src/lib.rs index 27c8a13e58d66..2cd82e3762e62 100644 --- a/crates/syntax/src/lib.rs +++ b/crates/syntax/src/lib.rs @@ -19,6 +19,7 @@ //! [RFC]: //! [Swift]: +#![cfg_attr(feature = "in-rust-tree", feature(rustc_private))] #![warn(rust_2018_idioms, unused_lifetimes, semicolon_in_expressions_from_macros)] #[allow(unused)] diff --git a/crates/syntax/src/validation.rs b/crates/syntax/src/validation.rs index e0ec6a242ffa7..2b1bbac08e528 100644 --- a/crates/syntax/src/validation.rs +++ b/crates/syntax/src/validation.rs @@ -5,7 +5,7 @@ mod block; use rowan::Direction; -use rustc_lexer::unescape::{self, unescape_literal, Mode}; +use rustc_dependencies::lexer::unescape::{self, unescape_literal, Mode}; use crate::{ algo, From 24b6922957dbe0b9f887c69508fc1192663019ca Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Sat, 16 Sep 2023 10:58:53 -0600 Subject: [PATCH 31/49] triagebot exclude_labels -> exclude_titles --- triagebot.toml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/triagebot.toml b/triagebot.toml index f0cd35399752f..95eed3ee172c0 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -11,5 +11,10 @@ allow-unauthenticated = [ new_pr = true [no-merges] -exclude_labels = ["sync"] +exclude_titles = [ # exclude syncs from subtree in rust-lang/rust + "Sync from downstream", + "sync from downstream", + "Sync from rust", + "sync from rust", +] labels = ["has-merge-commits", "S-waiting-on-author"] From cac796acb30d3af9a2eb9aff02acfae43a6da537 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Sat, 16 Sep 2023 12:59:17 -0700 Subject: [PATCH 32/49] Give `unmerge_use` a label explaining what it will affect. --- .../ide-assists/src/handlers/unmerge_use.rs | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/crates/ide-assists/src/handlers/unmerge_use.rs b/crates/ide-assists/src/handlers/unmerge_use.rs index dac216b69b727..52df30d9623fe 100644 --- a/crates/ide-assists/src/handlers/unmerge_use.rs +++ b/crates/ide-assists/src/handlers/unmerge_use.rs @@ -36,29 +36,25 @@ pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option< let old_parent_range = use_.syntax().parent()?.text_range(); let new_parent = use_.syntax().parent()?; + // If possible, explain what is going to be done. + let label = match tree.path().and_then(|path| path.first_segment()) { + Some(name) => format!("Unmerge use of `{name}`"), + None => "Unmerge use".into(), + }; + let target = tree.syntax().text_range(); - acc.add( - AssistId("unmerge_use", AssistKind::RefactorRewrite), - "Unmerge use", - target, - |builder| { - let new_use = make::use_( - use_.visibility(), - make::use_tree( - path, - tree.use_tree_list(), - tree.rename(), - tree.star_token().is_some(), - ), - ) - .clone_for_update(); - - tree.remove(); - ted::insert(Position::after(use_.syntax()), new_use.syntax()); - - builder.replace(old_parent_range, new_parent.to_string()); - }, - ) + acc.add(AssistId("unmerge_use", AssistKind::RefactorRewrite), label, target, |builder| { + let new_use = make::use_( + use_.visibility(), + make::use_tree(path, tree.use_tree_list(), tree.rename(), tree.star_token().is_some()), + ) + .clone_for_update(); + + tree.remove(); + ted::insert(Position::after(use_.syntax()), new_use.syntax()); + + builder.replace(old_parent_range, new_parent.to_string()); + }) } fn resolve_full_path(tree: &ast::UseTree) -> Option { From c3724311236916264adaaede2e05bf4ebbf08f5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 19 Sep 2023 13:48:05 +0200 Subject: [PATCH 33/49] scip: Use load_workspace_at. This honors the build script config, and is also simpler. --- crates/rust-analyzer/src/cli/scip.rs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/crates/rust-analyzer/src/cli/scip.rs b/crates/rust-analyzer/src/cli/scip.rs index 8c056fff000e6..875b724bd892e 100644 --- a/crates/rust-analyzer/src/cli/scip.rs +++ b/crates/rust-analyzer/src/cli/scip.rs @@ -11,10 +11,9 @@ use ide::{ TokenStaticData, }; use ide_db::LineIndexDatabase; -use load_cargo::{load_workspace, LoadCargoConfig, ProcMacroServerChoice}; -use project_model::{CargoConfig, ProjectManifest, ProjectWorkspace, RustLibSource}; +use load_cargo::{load_workspace_at, LoadCargoConfig, ProcMacroServerChoice}; +use project_model::{CargoConfig, RustLibSource}; use scip::types as scip_types; -use std::env; use crate::{ cli::flags, @@ -34,14 +33,13 @@ impl flags::Scip { with_proc_macro_server: ProcMacroServerChoice::Sysroot, prefill_caches: true, }; - let path = vfs::AbsPathBuf::assert(env::current_dir()?.join(&self.path)); - let rootpath = path.normalize(); - let manifest = ProjectManifest::discover_single(&path)?; - - let workspace = ProjectWorkspace::load(manifest, &cargo_config, no_progress)?; - - let (host, vfs, _) = - load_workspace(workspace, &cargo_config.extra_env, &load_cargo_config)?; + let root = vfs::AbsPathBuf::assert(std::env::current_dir()?.join(&self.path)).normalize(); + let (host, vfs, _) = load_workspace_at( + root.as_path().as_ref(), + &cargo_config, + &load_cargo_config, + &no_progress, + )?; let db = host.raw_database(); let analysis = host.analysis(); @@ -58,8 +56,7 @@ impl flags::Scip { .into(), project_root: format!( "file://{}", - path.normalize() - .as_os_str() + root.as_os_str() .to_str() .ok_or(anyhow::format_err!("Unable to normalize project_root path"))? ), @@ -80,7 +77,7 @@ impl flags::Scip { new_symbol }; - let relative_path = match get_relative_filepath(&vfs, &rootpath, file_id) { + let relative_path = match get_relative_filepath(&vfs, &root, file_id) { Some(relative_path) => relative_path, None => continue, }; From 184119258e256b656f30d89ea5e1512fe1d927db Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 19 Sep 2023 21:34:43 +0300 Subject: [PATCH 34/49] Do not resolve inlayHint.textEdit for VSCode client VSCode behaves strangely, allowing to navigate into label location, but not allowing to apply hint's text edit, after hint is resolved. See https://github.com/microsoft/vscode/issues/193124 for details. For now, stub hint resolution for VSCode specifically. --- crates/rust-analyzer/src/bin/main.rs | 12 ++++--- crates/rust-analyzer/src/config.rs | 33 +++++++++++++++---- .../rust-analyzer/src/diagnostics/to_proto.rs | 7 +++- crates/rust-analyzer/src/lsp/to_proto.rs | 3 +- .../rust-analyzer/tests/slow-tests/support.rs | 1 + 5 files changed, 43 insertions(+), 13 deletions(-) diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index 2fa14fc7e283f..c1d0d24b96d2f 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -190,6 +190,12 @@ fn run_server() -> anyhow::Result<()> { } }; + let mut is_visual_studio = false; + if let Some(client_info) = client_info { + tracing::info!("Client '{}' {}", client_info.name, client_info.version.unwrap_or_default()); + is_visual_studio = client_info.name == "Visual Studio Code"; + } + let workspace_roots = workspace_folders .map(|workspaces| { workspaces @@ -201,7 +207,7 @@ fn run_server() -> anyhow::Result<()> { }) .filter(|workspaces| !workspaces.is_empty()) .unwrap_or_else(|| vec![root_path.clone()]); - let mut config = Config::new(root_path, capabilities, workspace_roots); + let mut config = Config::new(root_path, capabilities, workspace_roots, is_visual_studio); if let Some(json) = initialization_options { if let Err(e) = config.update(json) { use lsp_types::{ @@ -231,10 +237,6 @@ fn run_server() -> anyhow::Result<()> { connection.initialize_finish(initialize_id, initialize_result)?; - if let Some(client_info) = client_info { - tracing::info!("Client '{}' {}", client_info.name, client_info.version.unwrap_or_default()); - } - if !config.has_linked_projects() && config.detached_files().is_empty() { config.rediscover_workspaces(); } diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index ea3a21241cb6e..683b3deb6ee6b 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -565,6 +565,7 @@ pub struct Config { data: ConfigData, detached_files: Vec, snippets: Vec, + is_visual_studio: bool, } type ParallelCachePrimingNumThreads = u8; @@ -760,6 +761,7 @@ impl Config { root_path: AbsPathBuf, caps: ClientCapabilities, workspace_roots: Vec, + is_visual_studio: bool, ) -> Self { Config { caps, @@ -769,6 +771,7 @@ impl Config { root_path, snippets: Default::default(), workspace_roots, + is_visual_studio, } } @@ -1667,6 +1670,12 @@ impl Config { pub fn typing_autoclose_angle(&self) -> bool { self.data.typing_autoClosingAngleBrackets_enable } + + // FIXME: VSCode seems to work wrong sometimes, see https://github.com/microsoft/vscode/issues/193124 + // hence, distinguish it for now. + pub fn is_visual_studio(&self) -> bool { + self.is_visual_studio + } } // Deserialization definitions @@ -2555,8 +2564,12 @@ mod tests { #[test] fn proc_macro_srv_null() { - let mut config = - Config::new(AbsPathBuf::try_from(project_root()).unwrap(), Default::default(), vec![]); + let mut config = Config::new( + AbsPathBuf::try_from(project_root()).unwrap(), + Default::default(), + vec![], + false, + ); config .update(serde_json::json!({ "procMacro_server": null, @@ -2567,8 +2580,12 @@ mod tests { #[test] fn proc_macro_srv_abs() { - let mut config = - Config::new(AbsPathBuf::try_from(project_root()).unwrap(), Default::default(), vec![]); + let mut config = Config::new( + AbsPathBuf::try_from(project_root()).unwrap(), + Default::default(), + vec![], + false, + ); config .update(serde_json::json!({ "procMacro": {"server": project_root().display().to_string()} @@ -2579,8 +2596,12 @@ mod tests { #[test] fn proc_macro_srv_rel() { - let mut config = - Config::new(AbsPathBuf::try_from(project_root()).unwrap(), Default::default(), vec![]); + let mut config = Config::new( + AbsPathBuf::try_from(project_root()).unwrap(), + Default::default(), + vec![], + false, + ); config .update(serde_json::json!({ "procMacro": {"server": "./server"} diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs index 731580557c29a..f8bc66ff8e74a 100644 --- a/crates/rust-analyzer/src/diagnostics/to_proto.rs +++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs @@ -538,7 +538,12 @@ mod tests { let (sender, _) = crossbeam_channel::unbounded(); let state = GlobalState::new( sender, - Config::new(workspace_root.to_path_buf(), ClientCapabilities::default(), Vec::new()), + Config::new( + workspace_root.to_path_buf(), + ClientCapabilities::default(), + Vec::new(), + false, + ), ); let snap = state.snapshot(); let mut actual = map_rust_diagnostic_to_lsp(&config, &diagnostic, workspace_root, &snap); diff --git a/crates/rust-analyzer/src/lsp/to_proto.rs b/crates/rust-analyzer/src/lsp/to_proto.rs index 23074493aeeea..9190c203146ea 100644 --- a/crates/rust-analyzer/src/lsp/to_proto.rs +++ b/crates/rust-analyzer/src/lsp/to_proto.rs @@ -443,10 +443,11 @@ pub(crate) fn inlay_hint( file_id: FileId, inlay_hint: InlayHint, ) -> Cancellable { + let is_visual_studio = snap.config.is_visual_studio(); let needs_resolve = inlay_hint.needs_resolve; let (label, tooltip, mut something_to_resolve) = inlay_hint_label(snap, fields_to_resolve, needs_resolve, inlay_hint.label)?; - let text_edits = if needs_resolve && fields_to_resolve.resolve_text_edits { + let text_edits = if !is_visual_studio && needs_resolve && fields_to_resolve.resolve_text_edits { something_to_resolve |= inlay_hint.text_edit.is_some(); None } else { diff --git a/crates/rust-analyzer/tests/slow-tests/support.rs b/crates/rust-analyzer/tests/slow-tests/support.rs index e49b5768fa46d..106b99cb9352f 100644 --- a/crates/rust-analyzer/tests/slow-tests/support.rs +++ b/crates/rust-analyzer/tests/slow-tests/support.rs @@ -150,6 +150,7 @@ impl Project<'_> { ..Default::default() }, roots, + false, ); config.update(self.config).expect("invalid config"); config.rediscover_workspaces(); From f9fac02c571a4f5520246ab80f5c4b825ca492a5 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 19 Sep 2023 23:34:43 +0300 Subject: [PATCH 35/49] Use proper editor name --- crates/rust-analyzer/src/bin/main.rs | 6 +++--- crates/rust-analyzer/src/config.rs | 10 +++++----- crates/rust-analyzer/src/lsp/to_proto.rs | 15 ++++++++------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index c1d0d24b96d2f..b8875ef87a4ca 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -190,10 +190,10 @@ fn run_server() -> anyhow::Result<()> { } }; - let mut is_visual_studio = false; + let mut is_visual_studio_code = false; if let Some(client_info) = client_info { tracing::info!("Client '{}' {}", client_info.name, client_info.version.unwrap_or_default()); - is_visual_studio = client_info.name == "Visual Studio Code"; + is_visual_studio_code = client_info.name == "Visual Studio Code"; } let workspace_roots = workspace_folders @@ -207,7 +207,7 @@ fn run_server() -> anyhow::Result<()> { }) .filter(|workspaces| !workspaces.is_empty()) .unwrap_or_else(|| vec![root_path.clone()]); - let mut config = Config::new(root_path, capabilities, workspace_roots, is_visual_studio); + let mut config = Config::new(root_path, capabilities, workspace_roots, is_visual_studio_code); if let Some(json) = initialization_options { if let Err(e) = config.update(json) { use lsp_types::{ diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 683b3deb6ee6b..af2a8436efb93 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -565,7 +565,7 @@ pub struct Config { data: ConfigData, detached_files: Vec, snippets: Vec, - is_visual_studio: bool, + is_visual_studio_code: bool, } type ParallelCachePrimingNumThreads = u8; @@ -761,7 +761,7 @@ impl Config { root_path: AbsPathBuf, caps: ClientCapabilities, workspace_roots: Vec, - is_visual_studio: bool, + is_visual_studio_code: bool, ) -> Self { Config { caps, @@ -771,7 +771,7 @@ impl Config { root_path, snippets: Default::default(), workspace_roots, - is_visual_studio, + is_visual_studio_code, } } @@ -1673,8 +1673,8 @@ impl Config { // FIXME: VSCode seems to work wrong sometimes, see https://github.com/microsoft/vscode/issues/193124 // hence, distinguish it for now. - pub fn is_visual_studio(&self) -> bool { - self.is_visual_studio + pub fn is_visual_studio_code(&self) -> bool { + self.is_visual_studio_code } } // Deserialization definitions diff --git a/crates/rust-analyzer/src/lsp/to_proto.rs b/crates/rust-analyzer/src/lsp/to_proto.rs index 9190c203146ea..aca91570f7c27 100644 --- a/crates/rust-analyzer/src/lsp/to_proto.rs +++ b/crates/rust-analyzer/src/lsp/to_proto.rs @@ -443,16 +443,17 @@ pub(crate) fn inlay_hint( file_id: FileId, inlay_hint: InlayHint, ) -> Cancellable { - let is_visual_studio = snap.config.is_visual_studio(); + let is_visual_studio_code = snap.config.is_visual_studio_code(); let needs_resolve = inlay_hint.needs_resolve; let (label, tooltip, mut something_to_resolve) = inlay_hint_label(snap, fields_to_resolve, needs_resolve, inlay_hint.label)?; - let text_edits = if !is_visual_studio && needs_resolve && fields_to_resolve.resolve_text_edits { - something_to_resolve |= inlay_hint.text_edit.is_some(); - None - } else { - inlay_hint.text_edit.map(|it| text_edit_vec(line_index, it)) - }; + let text_edits = + if !is_visual_studio_code && needs_resolve && fields_to_resolve.resolve_text_edits { + something_to_resolve |= inlay_hint.text_edit.is_some(); + None + } else { + inlay_hint.text_edit.map(|it| text_edit_vec(line_index, it)) + }; let data = if needs_resolve && something_to_resolve { Some(to_value(lsp_ext::InlayHintResolveData { file_id: file_id.0 }).unwrap()) } else { From 3a63255d2a16fd56e08915da0dbaa7228a41cb8f Mon Sep 17 00:00:00 2001 From: Wilfred Hughes Date: Tue, 19 Sep 2023 16:56:59 -0700 Subject: [PATCH 36/49] Update chalk version --- Cargo.lock | 16 ++++++++-------- crates/hir-ty/Cargo.toml | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e506aa834eba8..fa7b6a2fa3784 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -177,9 +177,9 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "chalk-derive" -version = "0.92.0" +version = "0.93.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff5053a8a42dbff5279a82423946fc56dc1253b76cf211b2b3c14b3aad4e1281" +checksum = "264726159011fc7f22c23eb51f49021ece6e71bc358b96e7f2e842db0b14162b" dependencies = [ "proc-macro2", "quote", @@ -189,9 +189,9 @@ dependencies = [ [[package]] name = "chalk-ir" -version = "0.92.0" +version = "0.93.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a56de2146a8ed0fcd54f4bd50db852f1de4eac9e1efe568494f106c21b77d2a" +checksum = "d65c17407d4c756b8f7f84344acb0fb96364d0298822743219bb25769b6d00df" dependencies = [ "bitflags 1.3.2", "chalk-derive", @@ -200,9 +200,9 @@ dependencies = [ [[package]] name = "chalk-recursive" -version = "0.92.0" +version = "0.93.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5cc09e6e9531f3544989ef89b189e80fbc7ad9e2f73f1c5e03ddc9ffb0527463" +checksum = "80e2cf7b70bedaaf3a8cf3c93b6120c2bb65be89389124028e724d19e209686e" dependencies = [ "chalk-derive", "chalk-ir", @@ -213,9 +213,9 @@ dependencies = [ [[package]] name = "chalk-solve" -version = "0.92.0" +version = "0.93.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b392e02b4c81ec76d3748da839fc70a5539b83d27c9030668463d34d5110b860" +checksum = "afc67c548d3854f64e97e67dc5b7c88513425c5bfa347cff96b7992ae6379288" dependencies = [ "chalk-derive", "chalk-ir", diff --git a/crates/hir-ty/Cargo.toml b/crates/hir-ty/Cargo.toml index 6a0c26a8bc092..c30807ad88490 100644 --- a/crates/hir-ty/Cargo.toml +++ b/crates/hir-ty/Cargo.toml @@ -23,10 +23,10 @@ oorandom = "11.1.3" tracing = "0.1.35" rustc-hash = "1.1.0" scoped-tls = "1.0.0" -chalk-solve = { version = "0.92.0", default-features = false } -chalk-ir = "0.92.0" -chalk-recursive = { version = "0.92.0", default-features = false } -chalk-derive = "0.92.0" +chalk-solve = { version = "0.93.0", default-features = false } +chalk-ir = "0.93.0" +chalk-recursive = { version = "0.93.0", default-features = false } +chalk-derive = "0.93.0" la-arena.workspace = true once_cell = "1.17.0" triomphe.workspace = true From dd843060f9ebff3862dc48c033c3ddbabb3b6a84 Mon Sep 17 00:00:00 2001 From: shogo-nakano-desu <61229807+shogo-nakano-desu@users.noreply.github.com> Date: Wed, 20 Sep 2023 23:02:52 +0900 Subject: [PATCH 37/49] refactor: remove boxing --- crates/flycheck/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index 61433313921a7..2de719af92ce9 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -488,9 +488,7 @@ impl CargoActor { // Skip certain kinds of messages to only spend time on what's useful JsonMessage::Cargo(message) => match message { cargo_metadata::Message::CompilerArtifact(artifact) if !artifact.fresh => { - self.sender - .send(CargoMessage::CompilerArtifact(Box::new(artifact))) - .unwrap(); + self.sender.send(CargoMessage::CompilerArtifact(artifact)).unwrap(); } cargo_metadata::Message::CompilerMessage(msg) => { self.sender.send(CargoMessage::Diagnostic(msg.message)).unwrap(); @@ -535,7 +533,7 @@ impl CargoActor { } enum CargoMessage { - CompilerArtifact(Box), + CompilerArtifact(cargo_metadata::Artifact), Diagnostic(Diagnostic), } From 91b012f91d625eeedd3831e8968174a3f2d33a63 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Thu, 21 Sep 2023 14:58:24 -0400 Subject: [PATCH 38/49] Documentation: Add parenthesis to the list of on-typing assists. --- crates/ide/src/typing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ide/src/typing.rs b/crates/ide/src/typing.rs index b40509715bafd..d21850bcff3ec 100644 --- a/crates/ide/src/typing.rs +++ b/crates/ide/src/typing.rs @@ -47,7 +47,7 @@ struct ExtendedTextEdit { // - typing `=` between two expressions adds `;` when in statement position // - typing `=` to turn an assignment into an equality comparison removes `;` when in expression position // - typing `.` in a chain method call auto-indents -// - typing `{` in front of an expression inserts a closing `}` after the expression +// - typing `{` or `(` in front of an expression inserts a closing `}` or `)` after the expression // - typing `{` in a use item adds a closing `}` in the right place // // VS Code:: From 60f7473c997a33cd59d8530f8aa1588bd622d296 Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Thu, 21 Sep 2023 21:31:15 -0700 Subject: [PATCH 39/49] fix parens when inlining closure local variables --- .../src/handlers/inline_local_variable.rs | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/ide-assists/src/handlers/inline_local_variable.rs b/crates/ide-assists/src/handlers/inline_local_variable.rs index e69d1a29677a9..49dcde75d2b31 100644 --- a/crates/ide-assists/src/handlers/inline_local_variable.rs +++ b/crates/ide-assists/src/handlers/inline_local_variable.rs @@ -96,8 +96,7 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext<'_>) ); let parent = matches!( usage_parent, - ast::Expr::CallExpr(_) - | ast::Expr::TupleExpr(_) + ast::Expr::TupleExpr(_) | ast::Expr::ArrayExpr(_) | ast::Expr::ParenExpr(_) | ast::Expr::ForExpr(_) @@ -949,6 +948,24 @@ fn f() { let S$0 = S; S; } +"#, + ); + } + + #[test] + fn test_inline_closure() { + check_assist( + inline_local_variable, + r#" +fn main() { + let $0f = || 2; + let _ = f(); +} +"#, + r#" +fn main() { + let _ = (|| 2)(); +} "#, ); } From ea118464908589db0293b7ba458a58db2f13db83 Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Thu, 21 Sep 2023 21:55:10 -0700 Subject: [PATCH 40/49] fix parens when inlining closure in body of function --- .../ide-assists/src/handlers/inline_call.rs | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/ide-assists/src/handlers/inline_call.rs b/crates/ide-assists/src/handlers/inline_call.rs index ffab58509b182..a80c1e23941f2 100644 --- a/crates/ide-assists/src/handlers/inline_call.rs +++ b/crates/ide-assists/src/handlers/inline_call.rs @@ -481,8 +481,12 @@ fn inline( }; body.reindent_to(original_indentation); + let no_stmts = body.statements().next().is_none(); match body.tail_expr() { - Some(expr) if !is_async_fn && body.statements().next().is_none() => expr, + Some(expr) if matches!(expr, ast::Expr::ClosureExpr(_)) && no_stmts => { + make::expr_paren(expr).clone_for_update() + } + Some(expr) if !is_async_fn && no_stmts => expr, _ => match node .syntax() .parent() @@ -1471,6 +1475,31 @@ fn main() { } }); } +"#, + ); + } + + #[test] + fn inline_call_closure_body() { + check_assist( + inline_call, + r#" +fn f() -> impl Fn() -> i32 { + || 2 +} + +fn main() { + let _ = $0f()(); +} +"#, + r#" +fn f() -> impl Fn() -> i32 { + || 2 +} + +fn main() { + let _ = (|| 2)(); +} "#, ); } From 93562dd5bdec82c70b8eff05c59badb4314c90c8 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 22 Sep 2023 08:53:24 +0200 Subject: [PATCH 41/49] Use parent + and_then instead of ancestors --- crates/ide-assists/src/handlers/bool_to_enum.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs index b9dbd6e98fcbf..85b0b87d0c95e 100644 --- a/crates/ide-assists/src/handlers/bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -111,7 +111,7 @@ fn find_bool_node(ctx: &AssistContext<'_>) -> Option { initializer: let_stmt.initializer(), definition: Definition::Local(def), }) - } else if let Some(const_) = name.syntax().ancestors().find_map(ast::Const::cast) { + } else if let Some(const_) = name.syntax().parent().and_then(ast::Const::cast) { let def = ctx.sema.to_def(&const_)?; if !def.ty(ctx.db()).is_bool() { cov_mark::hit!(not_applicable_non_bool_const); @@ -125,7 +125,7 @@ fn find_bool_node(ctx: &AssistContext<'_>) -> Option { initializer: const_.body(), definition: Definition::Const(def), }) - } else if let Some(static_) = name.syntax().ancestors().find_map(ast::Static::cast) { + } else if let Some(static_) = name.syntax().parent().and_then(ast::Static::cast) { let def = ctx.sema.to_def(&static_)?; if !def.ty(ctx.db()).is_bool() { cov_mark::hit!(not_applicable_non_bool_static); @@ -140,7 +140,7 @@ fn find_bool_node(ctx: &AssistContext<'_>) -> Option { definition: Definition::Static(def), }) } else { - let field = name.syntax().ancestors().find_map(ast::RecordField::cast)?; + let field = name.syntax().parent().and_then(ast::RecordField::cast)?; if field.name()? != name { return None; } From 556f0c67049517c98e89ca26055b513f5a35746b Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 22 Sep 2023 08:08:00 +0200 Subject: [PATCH 42/49] Various small fixes --- crates/hir-def/src/body/scope.rs | 11 +++++------ crates/hir-def/src/import_map.rs | 13 +++++++------ crates/hir-def/src/lib.rs | 5 +---- crates/hir/src/source_analyzer.rs | 4 ++-- crates/stdx/src/macros.rs | 7 ++++++- crates/stdx/src/process.rs | 2 +- 6 files changed, 22 insertions(+), 20 deletions(-) diff --git a/crates/hir-def/src/body/scope.rs b/crates/hir-def/src/body/scope.rs index 2a90a09f25e8c..f694666313553 100644 --- a/crates/hir-def/src/body/scope.rs +++ b/crates/hir-def/src/body/scope.rs @@ -1,7 +1,6 @@ //! Name resolution for expressions. use hir_expand::name::Name; -use la_arena::{Arena, Idx, IdxRange, RawIdx}; -use rustc_hash::FxHashMap; +use la_arena::{Arena, ArenaMap, Idx, IdxRange, RawIdx}; use triomphe::Arc; use crate::{ @@ -17,7 +16,7 @@ pub type ScopeId = Idx; pub struct ExprScopes { scopes: Arena, scope_entries: Arena, - scope_by_expr: FxHashMap, + scope_by_expr: ArenaMap, } #[derive(Debug, PartialEq, Eq)] @@ -77,10 +76,10 @@ impl ExprScopes { } pub fn scope_for(&self, expr: ExprId) -> Option { - self.scope_by_expr.get(&expr).copied() + self.scope_by_expr.get(expr).copied() } - pub fn scope_by_expr(&self) -> &FxHashMap { + pub fn scope_by_expr(&self) -> &ArenaMap { &self.scope_by_expr } } @@ -94,7 +93,7 @@ impl ExprScopes { let mut scopes = ExprScopes { scopes: Arena::default(), scope_entries: Arena::default(), - scope_by_expr: FxHashMap::default(), + scope_by_expr: ArenaMap::with_capacity(body.exprs.len()), }; let mut root = scopes.root_scope(); scopes.add_params_bindings(body, root, &body.params); diff --git a/crates/hir-def/src/import_map.rs b/crates/hir-def/src/import_map.rs index eb32c76b066d9..90763d4c3dfe7 100644 --- a/crates/hir-def/src/import_map.rs +++ b/crates/hir-def/src/import_map.rs @@ -1,7 +1,6 @@ //! A map of all publicly exported items in a crate. -use std::collections::hash_map::Entry; -use std::{fmt, hash::BuildHasherDefault}; +use std::{collections::hash_map::Entry, fmt, hash::BuildHasherDefault}; use base_db::CrateId; use fst::{self, Streamer}; @@ -11,10 +10,12 @@ use itertools::Itertools; use rustc_hash::{FxHashMap, FxHashSet, FxHasher}; use triomphe::Arc; -use crate::item_scope::ImportOrExternCrate; use crate::{ - db::DefDatabase, item_scope::ItemInNs, nameres::DefMap, visibility::Visibility, AssocItemId, - ModuleDefId, ModuleId, TraitId, + db::DefDatabase, + item_scope::{ImportOrExternCrate, ItemInNs}, + nameres::DefMap, + visibility::Visibility, + AssocItemId, ModuleDefId, ModuleId, TraitId, }; type FxIndexMap = IndexMap>; @@ -94,7 +95,7 @@ fn collect_import_map(db: &dyn DefDatabase, krate: CrateId) -> FxIndexMap bool { - match self { - MacroId::ProcMacroId(it) => it.lookup(db).kind == ProcMacroKind::Attr, - _ => false, - } + matches!(self, MacroId::ProcMacroId(it) if it.lookup(db).kind == ProcMacroKind::Attr) } } diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index f29fb1edf00bf..8d8ba48ad923d 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -888,7 +888,7 @@ fn scope_for_offset( .scope_by_expr() .iter() .filter_map(|(id, scope)| { - let InFile { file_id, value } = source_map.expr_syntax(*id).ok()?; + let InFile { file_id, value } = source_map.expr_syntax(id).ok()?; if from_file == file_id { return Some((value.text_range(), scope)); } @@ -923,7 +923,7 @@ fn adjust( .scope_by_expr() .iter() .filter_map(|(id, scope)| { - let source = source_map.expr_syntax(*id).ok()?; + let source = source_map.expr_syntax(id).ok()?; // FIXME: correctly handle macro expansion if source.file_id != from_file { return None; diff --git a/crates/stdx/src/macros.rs b/crates/stdx/src/macros.rs index 1a9982fa8b2a7..d71e418c89bc6 100644 --- a/crates/stdx/src/macros.rs +++ b/crates/stdx/src/macros.rs @@ -15,7 +15,12 @@ macro_rules! eprintln { macro_rules! format_to { ($buf:expr) => (); ($buf:expr, $lit:literal $($arg:tt)*) => { - { use ::std::fmt::Write as _; let _ = ::std::write!($buf, $lit $($arg)*); } + { + use ::std::fmt::Write as _; + // We can't do ::std::fmt::Write::write_fmt($buf, format_args!($lit $($arg)*)) + // unfortunately, as that loses out on autoref behavior. + _ = $buf.write_fmt(format_args!($lit $($arg)*)) + } }; } diff --git a/crates/stdx/src/process.rs b/crates/stdx/src/process.rs index e5aa343651876..bca0cbc36d1a7 100644 --- a/crates/stdx/src/process.rs +++ b/crates/stdx/src/process.rs @@ -23,7 +23,7 @@ pub fn streaming_output( let idx = if eof { data.len() } else { - match data.iter().rposition(|b| *b == b'\n') { + match data.iter().rposition(|&b| b == b'\n') { Some(i) => i + 1, None => return, } From 0a91a54794cfd45bc927de4760332c6ac5559ac8 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Fri, 22 Sep 2023 13:32:20 +0200 Subject: [PATCH 43/49] v4 --- .../src/handlers/convert_comment_block.rs | 8 ++--- .../src/handlers/desugar_doc_comment.rs | 32 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/crates/ide-assists/src/handlers/convert_comment_block.rs b/crates/ide-assists/src/handlers/convert_comment_block.rs index ef914cdb2cdf4..3f478ee7d39ad 100644 --- a/crates/ide-assists/src/handlers/convert_comment_block.rs +++ b/crates/ide-assists/src/handlers/convert_comment_block.rs @@ -89,12 +89,12 @@ fn line_to_block(acc: &mut Assists, comment: ast::Comment) -> Option<()> { // contents of each line comment when they're put into the block comment. let indentation = IndentLevel::from_token(comment.syntax()); - let cms = comments + let block_comment_body = comments .into_iter() .map(|c| line_comment_text(indentation, c)) - .collect::>(); - - let block_comment_body = cms.into_iter().join("\n"); + .collect::>() + .into_iter() + .join("\n"); let block_prefix = CommentKind { shape: CommentShape::Block, ..comment.kind() }.prefix(); diff --git a/crates/ide-assists/src/handlers/desugar_doc_comment.rs b/crates/ide-assists/src/handlers/desugar_doc_comment.rs index b7919bd1502cb..c859e98524e85 100644 --- a/crates/ide-assists/src/handlers/desugar_doc_comment.rs +++ b/crates/ide-assists/src/handlers/desugar_doc_comment.rs @@ -55,27 +55,27 @@ pub(crate) fn desugar_doc_comment(acc: &mut Assists, ctx: &AssistContext<'_>) -> } }; - let text = match comments { - Either::Left(comment) => { - let text = comment.text(); - text[comment.prefix().len()..(text.len() - "*/".len())] - .trim() - .lines() - .map(|l| l.strip_prefix(&indentation).unwrap_or(l)) - .join("\n") - } - Either::Right(comments) => comments - .into_iter() - .map(|cm| line_comment_text(IndentLevel(0), cm)) - .collect::>() - .join("\n"), - }; - acc.add( AssistId("desugar_doc_comment", AssistKind::RefactorRewrite), "Desugar doc-comment to attribute macro", target, |edit| { + let text = match comments { + Either::Left(comment) => { + let text = comment.text(); + text[comment.prefix().len()..(text.len() - "*/".len())] + .trim() + .lines() + .map(|l| l.strip_prefix(&indentation).unwrap_or(l)) + .join("\n") + } + Either::Right(comments) => comments + .into_iter() + .map(|cm| line_comment_text(IndentLevel(0), cm)) + .collect::>() + .join("\n"), + }; + let hashes = "#".repeat(required_hashes(&text)); let prefix = match placement { From 622e1a8d882204601d8e12c464d27cf3a7d96d44 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Fri, 22 Sep 2023 13:51:19 +0200 Subject: [PATCH 44/49] Add a test case to `add_missing_match_arms` Although it doesn't panic now, further changes to how we recover from incomplete syntax may cause this assist to panic. To mitigate this a test case has been added. --- .../src/handlers/add_missing_match_arms.rs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/crates/ide-assists/src/handlers/add_missing_match_arms.rs b/crates/ide-assists/src/handlers/add_missing_match_arms.rs index 5376ece2f584b..c8b78b0941694 100644 --- a/crates/ide-assists/src/handlers/add_missing_match_arms.rs +++ b/crates/ide-assists/src/handlers/add_missing_match_arms.rs @@ -1944,4 +1944,35 @@ fn main() { "#, ); } + + /// See [`discussion`](https://github.com/rust-lang/rust-analyzer/pull/15594#discussion_r1322960614) + #[test] + fn missing_field_name() { + check_assist( + add_missing_match_arms, + r#" +enum A { + A, + Missing { a: u32, : u32, c: u32 } +} + +fn a() { + let b = A::A; + match b$0 {} +}"#, + r#" +enum A { + A, + Missing { a: u32, : u32, c: u32 } +} + +fn a() { + let b = A::A; + match b { + $0A::A => todo!(), + A::Missing { a, u32, c } => todo!(), + } +}"#, + ) + } } From ba7f2bfb85dffb0d300fedf8f9adcebb6666c8a0 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 22 Sep 2023 17:46:17 +0200 Subject: [PATCH 45/49] Update config docs --- docs/user/generated_config.adoc | 14 +++++++++++--- editors/code/package.json | 4 ++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index 71feed0f72ca0..bde1c03bef7f0 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -57,6 +57,12 @@ build procedural macros. The command is required to output json and should therefore include `--message-format=json` or a similar option. +If there are multiple linked projects/workspaces, this command is invoked for +each of them, with the working directory being the workspace root +(i.e., the folder containing the `Cargo.toml`). This can be overwritten +by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and +`#rust-analyzer.cargo.buildScripts.invocationLocation#`. + By default, a cargo invocation will be constructed for the configured targets and features, with the following base command line: @@ -206,9 +212,11 @@ If you're changing this because you're using some tool wrapping Cargo, you might also want to change `#rust-analyzer.cargo.buildScripts.overrideCommand#`. -If there are multiple linked projects, this command is invoked for -each of them, with the working directory being the project root -(i.e., the folder containing the `Cargo.toml`). +If there are multiple linked projects/workspaces, this command is invoked for +each of them, with the working directory being the workspace root +(i.e., the folder containing the `Cargo.toml`). This can be overwritten +by changing `#rust-analyzer.cargo.check.invocationStrategy#` and +`#rust-analyzer.cargo.check.invocationLocation#`. An example command would be: diff --git a/editors/code/package.json b/editors/code/package.json index 44f1b81675a5b..406846fa59dcf 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -560,7 +560,7 @@ ] }, "rust-analyzer.cargo.buildScripts.overrideCommand": { - "markdownDescription": "Override the command rust-analyzer uses to run build scripts and\nbuild procedural macros. The command is required to output json\nand should therefore include `--message-format=json` or a similar\noption.\n\nBy default, a cargo invocation will be constructed for the configured\ntargets and features, with the following base command line:\n\n```bash\ncargo check --quiet --workspace --message-format=json --all-targets\n```\n.", + "markdownDescription": "Override the command rust-analyzer uses to run build scripts and\nbuild procedural macros. The command is required to output json\nand should therefore include `--message-format=json` or a similar\noption.\n\nIf there are multiple linked projects/workspaces, this command is invoked for\neach of them, with the working directory being the workspace root\n(i.e., the folder containing the `Cargo.toml`). This can be overwritten\nby changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and\n`#rust-analyzer.cargo.buildScripts.invocationLocation#`.\n\nBy default, a cargo invocation will be constructed for the configured\ntargets and features, with the following base command line:\n\n```bash\ncargo check --quiet --workspace --message-format=json --all-targets\n```\n.", "default": null, "type": [ "null", @@ -749,7 +749,7 @@ ] }, "rust-analyzer.check.overrideCommand": { - "markdownDescription": "Override the command rust-analyzer uses instead of `cargo check` for\ndiagnostics on save. The command is required to output json and\nshould therefore include `--message-format=json` or a similar option\n(if your client supports the `colorDiagnosticOutput` experimental\ncapability, you can use `--message-format=json-diagnostic-rendered-ansi`).\n\nIf you're changing this because you're using some tool wrapping\nCargo, you might also want to change\n`#rust-analyzer.cargo.buildScripts.overrideCommand#`.\n\nIf there are multiple linked projects, this command is invoked for\neach of them, with the working directory being the project root\n(i.e., the folder containing the `Cargo.toml`).\n\nAn example command would be:\n\n```bash\ncargo check --workspace --message-format=json --all-targets\n```\n.", + "markdownDescription": "Override the command rust-analyzer uses instead of `cargo check` for\ndiagnostics on save. The command is required to output json and\nshould therefore include `--message-format=json` or a similar option\n(if your client supports the `colorDiagnosticOutput` experimental\ncapability, you can use `--message-format=json-diagnostic-rendered-ansi`).\n\nIf you're changing this because you're using some tool wrapping\nCargo, you might also want to change\n`#rust-analyzer.cargo.buildScripts.overrideCommand#`.\n\nIf there are multiple linked projects/workspaces, this command is invoked for\neach of them, with the working directory being the workspace root\n(i.e., the folder containing the `Cargo.toml`). This can be overwritten\nby changing `#rust-analyzer.cargo.check.invocationStrategy#` and\n`#rust-analyzer.cargo.check.invocationLocation#`.\n\nAn example command would be:\n\n```bash\ncargo check --workspace --message-format=json --all-targets\n```\n.", "default": null, "type": [ "null", From 9f3d627681e069ea313076ce65cbd28a8dfe0974 Mon Sep 17 00:00:00 2001 From: vxpm Date: Sat, 23 Sep 2023 19:39:42 -0300 Subject: [PATCH 46/49] add tests for full signatures --- crates/ide-completion/src/tests/special.rs | 73 +++++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/crates/ide-completion/src/tests/special.rs b/crates/ide-completion/src/tests/special.rs index e80a289049f1d..83888e08f1c7b 100644 --- a/crates/ide-completion/src/tests/special.rs +++ b/crates/ide-completion/src/tests/special.rs @@ -2,10 +2,15 @@ use expect_test::{expect, Expect}; -use crate::tests::{ - check_edit, completion_list, completion_list_no_kw, completion_list_with_trigger_character, +use crate::{ + tests::{ + check_edit, completion_list, completion_list_no_kw, completion_list_with_trigger_character, + }, + CompletionItemKind, }; +use super::{do_completion_with_config, TEST_CONFIG}; + fn check_no_kw(ra_fixture: &str, expect: Expect) { let actual = completion_list_no_kw(ra_fixture); expect.assert_eq(&actual) @@ -1303,3 +1308,67 @@ struct Foo(x: &'x mut T) -> u8 where T: Clone, { 0u8 } +fn main() { fo$0 } +"#, + CompletionItemKind::SymbolKind(ide_db::SymbolKind::Function), + expect!("fn(&mut T) -> u8"), + expect!("pub fn foo<'x, T>(x: &'x mut T) -> u8 where T: Clone,"), + ); + + check_signatures( + r#" +struct Foo; +struct Bar; +impl Bar { + pub const fn baz(x: Foo) -> ! { loop {} }; +} + +fn main() { Bar::b$0 } +"#, + CompletionItemKind::SymbolKind(ide_db::SymbolKind::Function), + expect!("const fn(Foo) -> !"), + expect!("pub const fn baz(x: Foo) -> !"), + ); + + check_signatures( + r#" +struct Foo; +struct Bar; +impl Bar { + pub const fn baz<'foo>(&'foo mut self, x: &'foo Foo) -> ! { loop {} }; +} + +fn main() { + let mut bar = Bar; + bar.b$0 +} +"#, + CompletionItemKind::Method, + expect!("const fn(&'foo mut self, &Foo) -> !"), + expect!("pub const fn baz<'foo>(&'foo mut self, x: &'foo Foo) -> !"), + ); +} From 10fae6282070945531853901cc19155f59758bbc Mon Sep 17 00:00:00 2001 From: vxpm Date: Sat, 23 Sep 2023 19:43:19 -0300 Subject: [PATCH 47/49] split detail function --- crates/ide-completion/src/render/function.rs | 39 +++++++++++--------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/crates/ide-completion/src/render/function.rs b/crates/ide-completion/src/render/function.rs index dd7de72190d1e..dfae715afe36d 100644 --- a/crates/ide-completion/src/render/function.rs +++ b/crates/ide-completion/src/render/function.rs @@ -98,9 +98,14 @@ fn render( _ => (), } + let detail = if ctx.completion.config.full_function_signatures { + detail_full(db, func) + } else { + detail(db, func) + }; item.set_documentation(ctx.docs(func)) .set_deprecated(ctx.is_deprecated(func) || ctx.is_deprecated_assoc_item(func)) - .detail(detail(db, func, ctx.completion.config.full_function_signatures)) + .detail(detail) .lookup_by(name.unescaped().to_smol_str()); match ctx.completion.config.snippet_cap { @@ -239,22 +244,7 @@ fn ref_of_param(ctx: &CompletionContext<'_>, arg: &str, ty: &hir::Type) -> &'sta "" } -fn detail(db: &dyn HirDatabase, func: hir::Function, full_function_signature: bool) -> String { - if full_function_signature { - let signature = format!("{}", func.display(db)); - let mut singleline = String::with_capacity(signature.len()); - - for segment in signature.split_whitespace() { - if !singleline.is_empty() { - singleline.push(' '); - } - - singleline.push_str(segment); - } - - return singleline; - } - +fn detail(db: &dyn HirDatabase, func: hir::Function) -> String { let mut ret_ty = func.ret_type(db); let mut detail = String::new(); @@ -278,6 +268,21 @@ fn detail(db: &dyn HirDatabase, func: hir::Function, full_function_signature: bo detail } +fn detail_full(db: &dyn HirDatabase, func: hir::Function) -> String { + let signature = format!("{}", func.display(db)); + let mut detail = String::with_capacity(signature.len()); + + for segment in signature.split_whitespace() { + if !detail.is_empty() { + detail.push(' '); + } + + detail.push_str(segment); + } + + detail +} + fn params_display(db: &dyn HirDatabase, func: hir::Function) -> String { if let Some(self_param) = func.self_param(db) { let assoc_fn_params = func.assoc_fn_params(db); From 588c7d9182de9748f87d99f5f26c0f840fe59e16 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sun, 24 Sep 2023 22:47:29 +0200 Subject: [PATCH 48/49] minor: hover_simple refactor --- crates/base-db/src/input.rs | 1 + crates/ide-db/src/defs.rs | 4 ++-- crates/ide/src/hover.rs | 32 +++++++++++++++----------------- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index b75c7079be787..65db5c0fc7d31 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -257,6 +257,7 @@ pub trait ProcMacroExpander: fmt::Debug + Send + Sync + RefUnwindSafe { ) -> Result; } +#[derive(Debug)] pub enum ProcMacroExpansionError { Panic(String), /// Things like "proc macro server was killed by OOM". diff --git a/crates/ide-db/src/defs.rs b/crates/ide-db/src/defs.rs index 4ce80532e8efa..ef72fc3861a7f 100644 --- a/crates/ide-db/src/defs.rs +++ b/crates/ide-db/src/defs.rs @@ -161,8 +161,8 @@ impl IdentClass { ast::AwaitExpr(await_expr) => OperatorClass::classify_await(sema, &await_expr).map(IdentClass::Operator), ast::BinExpr(bin_expr) => OperatorClass::classify_bin(sema, &bin_expr).map(IdentClass::Operator), ast::IndexExpr(index_expr) => OperatorClass::classify_index(sema, &index_expr).map(IdentClass::Operator), - ast::PrefixExpr(prefix_expr) => OperatorClass::classify_prefix(sema,&prefix_expr).map(IdentClass::Operator), - ast::TryExpr(try_expr) => OperatorClass::classify_try(sema,&try_expr).map(IdentClass::Operator), + ast::PrefixExpr(prefix_expr) => OperatorClass::classify_prefix(sema, &prefix_expr).map(IdentClass::Operator), + ast::TryExpr(try_expr) => OperatorClass::classify_try(sema, &try_expr).map(IdentClass::Operator), _ => None, } } diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 21934b9480efb..e0b64fe7988e5 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -180,26 +180,24 @@ fn hover_simple( descended() .filter_map(|token| { let node = token.parent()?; - let class = IdentClass::classify_token(sema, token)?; - if let IdentClass::Operator(OperatorClass::Await(_)) = class { + match IdentClass::classify_node(sema, &node)? { // It's better for us to fall back to the keyword hover here, // rendering poll is very confusing - return None; + IdentClass::Operator(OperatorClass::Await(_)) => None, + + IdentClass::NameRefClass(NameRefClass::ExternCrateShorthand { + decl, + .. + }) => Some(vec![(Definition::ExternCrateDecl(decl), node)]), + + class => Some( + class + .definitions() + .into_iter() + .zip(iter::repeat(node)) + .collect::>(), + ), } - if let IdentClass::NameRefClass(NameRefClass::ExternCrateShorthand { - decl, - .. - }) = class - { - return Some(vec![(Definition::ExternCrateDecl(decl), node)]); - } - Some( - class - .definitions() - .into_iter() - .zip(iter::once(node).cycle()) - .collect::>(), - ) }) .flatten() .unique_by(|&(def, _)| def) From 2678cd3d7af7b753f54fc0e45e54b42bc974ebce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauren=C8=9Biu=20Nicola?= Date: Mon, 25 Sep 2023 11:42:56 +0300 Subject: [PATCH 49/49] Allow rustc_private for RustAnalyzer --- src/bootstrap/tool.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index f094dd9d7c90b..8fa7cc4309321 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -602,8 +602,7 @@ pub struct RustAnalyzer { } impl RustAnalyzer { - pub const ALLOW_FEATURES: &'static str = - "proc_macro_internals,proc_macro_diagnostic,proc_macro_span,proc_macro_span_shrink"; + pub const ALLOW_FEATURES: &'static str = "rustc_private,proc_macro_internals,proc_macro_diagnostic,proc_macro_span,proc_macro_span_shrink"; } impl Step for RustAnalyzer {