From 28ddddd09109982566dbec62b7c04f71f23d6b02 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Wed, 22 May 2024 13:43:07 -0700 Subject: [PATCH 001/124] Feat: hide double underscored symbols from symbol search --- crates/ide-db/src/symbol_index.rs | 12 +++++++++++- crates/ide/src/navigation_target.rs | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/ide-db/src/symbol_index.rs b/crates/ide-db/src/symbol_index.rs index 12085f9ebd..e14cf0eb1f 100644 --- a/crates/ide-db/src/symbol_index.rs +++ b/crates/ide-db/src/symbol_index.rs @@ -53,6 +53,7 @@ pub struct Query { case_sensitive: bool, only_types: bool, libs: bool, + include_hidden: bool, } impl Query { @@ -66,9 +67,14 @@ impl Query { mode: SearchMode::Fuzzy, assoc_mode: AssocSearchMode::Include, case_sensitive: false, + include_hidden: false, } } + pub fn include_hidden(&mut self) { + self.include_hidden = true; + } + pub fn only_types(&mut self) { self.only_types = true; } @@ -192,7 +198,8 @@ impl std::ops::Deref for Snap { // Note that filtering does not currently work in VSCode due to the editor never // sending the special symbols to the language server. Instead, you can configure // the filtering via the `rust-analyzer.workspace.symbol.search.scope` and -// `rust-analyzer.workspace.symbol.search.kind` settings. +// `rust-analyzer.workspace.symbol.search.kind` settings. Symbols prefixed +// with `__` are hidden from the search results unless configured otherwise. // // |=== // | Editor | Shortcut @@ -374,6 +381,9 @@ impl Query { if non_type_for_type_only_query || !self.matches_assoc_mode(symbol.is_assoc) { continue; } + if !self.include_hidden && symbol.name.starts_with("__") { + continue; + } if self.mode.check(&self.query, self.case_sensitive, &symbol.name) { cb(symbol); } diff --git a/crates/ide/src/navigation_target.rs b/crates/ide/src/navigation_target.rs index fc836d5540..e40c7ecef0 100644 --- a/crates/ide/src/navigation_target.rs +++ b/crates/ide/src/navigation_target.rs @@ -926,4 +926,25 @@ struct Foo; let navs = analysis.symbol_search(Query::new("foo".to_owned()), !0).unwrap(); assert_eq!(navs.len(), 2) } + + #[test] + fn test_ensure_hidden_symbols_are_not_returned() { + let (analysis, _) = fixture::file( + r#" +fn foo() {} +struct Foo; +static __FOO_CALLSITE: () = (); +"#, + ); + + // It doesn't show the hidden symbol + let navs = analysis.symbol_search(Query::new("foo".to_owned()), !0).unwrap(); + assert_eq!(navs.len(), 2); + + // Unless we configure a query to show hidden symbols + let mut query = Query::new("foo".to_owned()); + query.include_hidden(); + let navs = analysis.symbol_search(query, !0).unwrap(); + assert_eq!(navs.len(), 3); + } } From bdfcae556d3c54b0c138382c295207fe706230ef Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Wed, 22 May 2024 13:47:05 -0700 Subject: [PATCH 002/124] Allow searching with prefix --- crates/ide-db/src/symbol_index.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/ide-db/src/symbol_index.rs b/crates/ide-db/src/symbol_index.rs index e14cf0eb1f..5e1930e602 100644 --- a/crates/ide-db/src/symbol_index.rs +++ b/crates/ide-db/src/symbol_index.rs @@ -381,7 +381,7 @@ impl Query { if non_type_for_type_only_query || !self.matches_assoc_mode(symbol.is_assoc) { continue; } - if !self.include_hidden && symbol.name.starts_with("__") { + if self.should_hide_query(&symbol) { continue; } if self.mode.check(&self.query, self.case_sensitive, &symbol.name) { @@ -392,6 +392,11 @@ impl Query { } } + fn should_hide_query(&self, symbol: &FileSymbol) -> bool { + // Hide symbols that start with `__` unless the query starts with `__` + !self.include_hidden && symbol.name.starts_with("__") && !self.query.starts_with("__") + } + fn matches_assoc_mode(&self, is_trait_assoc_item: bool) -> bool { !matches!( (is_trait_assoc_item, self.assoc_mode), From 042bd0b78d84498e53362d9bcce3a7db4c5326c5 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Wed, 22 May 2024 14:39:16 -0700 Subject: [PATCH 003/124] Fix: clippy --- crates/ide-db/src/symbol_index.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ide-db/src/symbol_index.rs b/crates/ide-db/src/symbol_index.rs index 5e1930e602..365a727245 100644 --- a/crates/ide-db/src/symbol_index.rs +++ b/crates/ide-db/src/symbol_index.rs @@ -381,7 +381,7 @@ impl Query { if non_type_for_type_only_query || !self.matches_assoc_mode(symbol.is_assoc) { continue; } - if self.should_hide_query(&symbol) { + if self.should_hide_query(symbol) { continue; } if self.mode.check(&self.query, self.case_sensitive, &symbol.name) { From 4bf51eb49633e556799ceddd146a520cad964788 Mon Sep 17 00:00:00 2001 From: Mathew Horner Date: Tue, 28 May 2024 23:05:27 -0500 Subject: [PATCH 004/124] Add preference modifier for workspace-local crates when using auto import. --- .../ide-assists/src/handlers/auto_import.rs | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/crates/ide-assists/src/handlers/auto_import.rs b/crates/ide-assists/src/handlers/auto_import.rs index 3bd003a267..4ae7ff4a33 100644 --- a/crates/ide-assists/src/handlers/auto_import.rs +++ b/crates/ide-assists/src/handlers/auto_import.rs @@ -272,8 +272,10 @@ fn module_distance_heuristic(db: &dyn HirDatabase, current: &Module, item: &Modu // cost of importing from another crate let crate_boundary_cost = if current.krate() == item.krate() { 0 - } else if item.krate().is_builtin(db) { + } else if item.krate().origin(db).is_local() { 2 + } else if item.krate().is_builtin(db) { + 3 } else { 4 }; @@ -365,6 +367,49 @@ pub struct HashMap; ) } + #[test] + fn prefer_workspace() { + let before = r" +//- /main.rs crate:main deps:foo,bar +HashMap$0::new(); + +//- /lib.rs crate:foo +pub mod module { + pub struct HashMap; +} + +//- /lib.rs crate:bar library +pub struct HashMap; + "; + + check_auto_import_order(before, &["Import `foo::module::HashMap`", "Import `bar::HashMap`"]) + } + + #[test] + fn prefer_non_local_over_long_path() { + let before = r" +//- /main.rs crate:main deps:foo,bar +HashMap$0::new(); + +//- /lib.rs crate:foo +pub mod deeply { + pub mod nested { + pub mod module { + pub struct HashMap; + } + } +} + +//- /lib.rs crate:bar library +pub struct HashMap; + "; + + check_auto_import_order( + before, + &["Import `bar::HashMap`", "Import `foo::deeply::nested::module::HashMap`"], + ) + } + #[test] fn not_applicable_if_scope_inside_macro() { check_assist_not_applicable( From 7b75f1113c849783713146be796c7d627f4144ca Mon Sep 17 00:00:00 2001 From: Vincent Esche Date: Wed, 29 May 2024 19:41:55 +0200 Subject: [PATCH 005/124] =?UTF-8?q?Add=20`Function::fn=5Fptr=5Ftype(?= =?UTF-8?q?=E2=80=A6)`=20for=20obtaining=20name-erased=20function=20type?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/hir/src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 777be711a5..963ad6ca99 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1884,6 +1884,14 @@ impl Function { Type::from_value_def(db, self.id) } + pub fn fn_ptr_type(self, db: &dyn HirDatabase) -> Type { + let resolver = self.id.resolver(db.upcast()); + let substs = TyBuilder::placeholder_subst(db, self.id); + let callable_sig = db.callable_item_signature(self.id.into()).substitute(Interner, &substs); + let ty = TyKind::Function(callable_sig.to_fn_ptr()).intern(Interner); + Type::new_with_resolver_inner(db, &resolver, ty) + } + /// Get this function's return type pub fn ret_type(self, db: &dyn HirDatabase) -> Type { let resolver = self.id.resolver(db.upcast()); From 7c34eb38807f86df2fb3b344a2dbdfa90b223259 Mon Sep 17 00:00:00 2001 From: Hamir Mahal Date: Thu, 30 May 2024 16:18:49 -0700 Subject: [PATCH 006/124] style: simplify string interpolation --- crates/flycheck/src/lib.rs | 7 +++---- crates/hir-ty/src/consteval/tests.rs | 6 +++--- crates/hir-ty/src/display.rs | 2 +- crates/hir-ty/src/mir/eval.rs | 4 ++-- crates/hir-ty/src/mir/eval/tests.rs | 2 +- crates/hir-ty/src/mir/lower.rs | 2 +- crates/hir/src/lib.rs | 4 ++-- crates/ide-assists/src/handlers/auto_import.rs | 4 ++-- .../ide-assists/src/handlers/bool_to_enum.rs | 4 ++-- .../src/handlers/destructure_struct_binding.rs | 2 +- .../src/handlers/generate_delegate_trait.rs | 2 +- .../src/handlers/generate_getter_or_setter.rs | 2 +- .../src/handlers/generate_mut_trait_impl.rs | 2 +- .../src/handlers/into_to_qualified_from.rs | 4 ++-- .../src/handlers/merge_nested_if.rs | 2 +- .../src/handlers/remove_parentheses.rs | 2 +- .../src/completions/postfix/format_like.rs | 4 ++-- .../src/handlers/json_is_not_rust.rs | 2 +- .../trait_impl_redundant_assoc_item.rs | 6 +++--- .../src/handlers/unresolved_method.rs | 7 +++---- crates/ide/src/interpret_function.rs | 2 +- crates/ide/src/view_memory_layout.rs | 2 +- crates/parser/src/grammar.rs | 2 +- crates/paths/src/lib.rs | 2 +- crates/proc-macro-api/src/process.rs | 3 +-- crates/project-model/src/sysroot.rs | 3 +-- crates/project-model/src/tests.rs | 2 +- crates/rust-analyzer/src/cli/analysis_stats.rs | 2 +- crates/rust-analyzer/src/cli/run_tests.rs | 4 ++-- crates/rust-analyzer/src/cli/rustc_tests.rs | 4 ++-- crates/rust-analyzer/src/tracing/hprof.rs | 2 +- crates/salsa/salsa-macros/src/query_group.rs | 18 ++++++++---------- crates/salsa/tests/cycles.rs | 2 +- crates/salsa/tests/incremental/constants.rs | 4 ++-- .../salsa/tests/incremental/implementation.rs | 8 ++++---- .../parallel/parallel_cycle_none_recover.rs | 2 +- crates/salsa/tests/parallel/race.rs | 2 +- crates/span/src/lib.rs | 2 +- crates/syntax/src/ast/make.rs | 4 ++-- crates/test-fixture/src/lib.rs | 2 +- crates/test-utils/src/fixture.rs | 2 +- lib/lsp-server/src/lib.rs | 3 +-- xtask/src/metrics.rs | 2 +- 43 files changed, 71 insertions(+), 78 deletions(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index 6d5ca8321e..434e0e4d70 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -288,7 +288,7 @@ impl FlycheckActor { Some(c) => c, None => continue, }; - let formatted_command = format!("{:?}", command); + let formatted_command = format!("{command:?}"); tracing::debug!(?command, "will restart flycheck"); let (sender, receiver) = unbounded(); @@ -301,8 +301,7 @@ impl FlycheckActor { } Err(error) => { self.report_progress(Progress::DidFailToRestart(format!( - "Failed to run the following command: {} error={}", - formatted_command, error + "Failed to run the following command: {formatted_command} error={error}" ))); } } @@ -313,7 +312,7 @@ impl FlycheckActor { // Watcher finished let command_handle = self.command_handle.take().unwrap(); self.command_receiver.take(); - let formatted_handle = format!("{:?}", command_handle); + let formatted_handle = format!("{command_handle:?}"); let res = command_handle.join(); if let Err(error) = &res { diff --git a/crates/hir-ty/src/consteval/tests.rs b/crates/hir-ty/src/consteval/tests.rs index d1ffd5046c..1b4584a18d 100644 --- a/crates/hir-ty/src/consteval/tests.rs +++ b/crates/hir-ty/src/consteval/tests.rs @@ -73,7 +73,7 @@ fn check_answer(ra_fixture: &str, check: impl FnOnce(&[u8], &MemoryMap)) { Ok(t) => t, Err(e) => { let err = pretty_print_err(e, db); - panic!("Error in evaluating goal: {}", err); + panic!("Error in evaluating goal: {err}"); } }; match &r.data(Interner).value { @@ -81,7 +81,7 @@ fn check_answer(ra_fixture: &str, check: impl FnOnce(&[u8], &MemoryMap)) { ConstScalar::Bytes(b, mm) => { check(b, mm); } - x => panic!("Expected number but found {:?}", x), + x => panic!("Expected number but found {x:?}"), }, _ => panic!("result of const eval wasn't a concrete const"), } @@ -89,7 +89,7 @@ fn check_answer(ra_fixture: &str, check: impl FnOnce(&[u8], &MemoryMap)) { fn pretty_print_err(e: ConstEvalError, db: TestDB) -> String { let mut err = String::new(); - let span_formatter = |file, range| format!("{:?} {:?}", file, range); + let span_formatter = |file, range| format!("{file:?} {range:?}"); match e { ConstEvalError::MirLowerError(e) => e.pretty_print(&mut err, &db, span_formatter), ConstEvalError::MirEvalError(e) => e.pretty_print(&mut err, &db, span_formatter), diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs index 5a9621bb69..0027eb56a8 100644 --- a/crates/hir-ty/src/display.rs +++ b/crates/hir-ty/src/display.rs @@ -670,7 +670,7 @@ fn render_const_scalar( TyKind::FnDef(..) => ty.hir_fmt(f), TyKind::Function(_) | TyKind::Raw(_, _) => { let it = u128::from_le_bytes(pad16(b, false)); - write!(f, "{:#X} as ", it)?; + write!(f, "{it:#X} as ")?; ty.hir_fmt(f) } TyKind::Array(ty, len) => { diff --git a/crates/hir-ty/src/mir/eval.rs b/crates/hir-ty/src/mir/eval.rs index 2de1aa30c9..4ee96a66a3 100644 --- a/crates/hir-ty/src/mir/eval.rs +++ b/crates/hir-ty/src/mir/eval.rs @@ -363,7 +363,7 @@ impl MirEvalError { )?; } Either::Right(closure) => { - writeln!(f, "In {:?}", closure)?; + writeln!(f, "In {closure:?}")?; } } let source_map = db.body_with_source_map(*def).1; @@ -424,7 +424,7 @@ impl MirEvalError { | MirEvalError::StackOverflow | MirEvalError::CoerceUnsizedError(_) | MirEvalError::InternalError(_) - | MirEvalError::InvalidVTableId(_) => writeln!(f, "{:?}", err)?, + | MirEvalError::InvalidVTableId(_) => writeln!(f, "{err:?}")?, } Ok(()) } diff --git a/crates/hir-ty/src/mir/eval/tests.rs b/crates/hir-ty/src/mir/eval/tests.rs index 4abbda56cb..c3b35cd553 100644 --- a/crates/hir-ty/src/mir/eval/tests.rs +++ b/crates/hir-ty/src/mir/eval/tests.rs @@ -77,7 +77,7 @@ fn check_panic(ra_fixture: &str, expected_panic: &str) { let (db, file_ids) = TestDB::with_many_files(ra_fixture); let file_id = *file_ids.last().unwrap(); let e = eval_main(&db, file_id).unwrap_err(); - assert_eq!(e.is_panic().unwrap_or_else(|| panic!("unexpected error: {:?}", e)), expected_panic); + assert_eq!(e.is_panic().unwrap_or_else(|| panic!("unexpected error: {e:?}")), expected_panic); } #[test] diff --git a/crates/hir-ty/src/mir/lower.rs b/crates/hir-ty/src/mir/lower.rs index 151f65cfbb..95b16ca8bc 100644 --- a/crates/hir-ty/src/mir/lower.rs +++ b/crates/hir-ty/src/mir/lower.rs @@ -213,7 +213,7 @@ impl MirLowerError { | MirLowerError::LangItemNotFound(_) | MirLowerError::MutatingRvalue | MirLowerError::UnresolvedLabel - | MirLowerError::UnresolvedUpvar(_) => writeln!(f, "{:?}", self)?, + | MirLowerError::UnresolvedUpvar(_) => writeln!(f, "{self:?}")?, } Ok(()) } diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 777be711a5..d28c45edd7 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -2414,9 +2414,9 @@ impl Const { let value_signed = i128::from_le_bytes(mir::pad16(b, matches!(s, Scalar::Int(_)))); if value >= 10 { - return Ok(format!("{} ({:#X})", value_signed, value)); + return Ok(format!("{value_signed} ({value:#X})")); } else { - return Ok(format!("{}", value_signed)); + return Ok(format!("{value_signed}")); } } } diff --git a/crates/ide-assists/src/handlers/auto_import.rs b/crates/ide-assists/src/handlers/auto_import.rs index 3bd003a267..650b4279ea 100644 --- a/crates/ide-assists/src/handlers/auto_import.rs +++ b/crates/ide-assists/src/handlers/auto_import.rs @@ -140,7 +140,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option< acc.add_group( &group_label, assist_id, - format!("Import `{}`", import_name), + format!("Import `{import_name}`"), range, |builder| { let scope = match scope.clone() { @@ -165,7 +165,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option< acc.add_group( &group_label, assist_id, - format!("Import `{} as _`", import_name), + format!("Import `{import_name} as _`"), range, |builder| { let scope = match scope.clone() { diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs index 71436e6580..c95e24693d 100644 --- a/crates/ide-assists/src/handlers/bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -228,7 +228,7 @@ fn replace_usages( edit.replace( prefix_expr.syntax().text_range(), - format!("{} == Bool::False", inner_expr), + format!("{inner_expr} == Bool::False"), ); } else if let Some((record_field, initializer)) = name .as_name_ref() @@ -275,7 +275,7 @@ fn replace_usages( } else if let Some(receiver) = find_method_call_expr_usage(&name) { edit.replace( receiver.syntax().text_range(), - format!("({} == Bool::True)", receiver), + format!("({receiver} == Bool::True)"), ); } else if 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 diff --git a/crates/ide-assists/src/handlers/destructure_struct_binding.rs b/crates/ide-assists/src/handlers/destructure_struct_binding.rs index 9adbdd220c..7618871552 100644 --- a/crates/ide-assists/src/handlers/destructure_struct_binding.rs +++ b/crates/ide-assists/src/handlers/destructure_struct_binding.rs @@ -242,7 +242,7 @@ fn generate_field_names(ctx: &AssistContext<'_>, data: &StructEditData) -> Vec<( .iter() .enumerate() .map(|(index, _)| { - let new_name = new_field_name((format!("_{}", index)).into(), &data.names_in_scope); + let new_name = new_field_name((format!("_{index}")).into(), &data.names_in_scope); (index.to_string().into(), new_name) }) .collect(), diff --git a/crates/ide-assists/src/handlers/generate_delegate_trait.rs b/crates/ide-assists/src/handlers/generate_delegate_trait.rs index 748acb46ef..19521b8a4b 100644 --- a/crates/ide-assists/src/handlers/generate_delegate_trait.rs +++ b/crates/ide-assists/src/handlers/generate_delegate_trait.rs @@ -758,7 +758,7 @@ fn ty_assoc_item(item: syntax::ast::TypeAlias, qual_path_ty: Path) -> Option ast::Path { - make::path_from_text(&format!("{}::{}", qual_path_ty, path_expr_seg)) + make::path_from_text(&format!("{qual_path_ty}::{path_expr_seg}")) } #[cfg(test)] diff --git a/crates/ide-assists/src/handlers/generate_getter_or_setter.rs b/crates/ide-assists/src/handlers/generate_getter_or_setter.rs index e90a032f1c..60214aaaf6 100644 --- a/crates/ide-assists/src/handlers/generate_getter_or_setter.rs +++ b/crates/ide-assists/src/handlers/generate_getter_or_setter.rs @@ -47,7 +47,7 @@ pub(crate) fn generate_setter(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt } // Prepend set_ to fn names. - fn_names.iter_mut().for_each(|name| *name = format!("set_{}", name)); + fn_names.iter_mut().for_each(|name| *name = format!("set_{name}")); // Return early if we've found an existing fn let impl_def = find_struct_impl(ctx, &ast::Adt::Struct(strukt.clone()), &fn_names)?; diff --git a/crates/ide-assists/src/handlers/generate_mut_trait_impl.rs b/crates/ide-assists/src/handlers/generate_mut_trait_impl.rs index 91eaa96b6c..6aa561ad7f 100644 --- a/crates/ide-assists/src/handlers/generate_mut_trait_impl.rs +++ b/crates/ide-assists/src/handlers/generate_mut_trait_impl.rs @@ -105,7 +105,7 @@ pub(crate) fn generate_mut_trait_impl(acc: &mut Assists, ctx: &AssistContext<'_> "Generate `IndexMut` impl from this `Index` trait", target, |edit| { - edit.insert(target.start(), format!("$0{}\n\n", impl_def)); + edit.insert(target.start(), format!("$0{impl_def}\n\n")); }, ) } diff --git a/crates/ide-assists/src/handlers/into_to_qualified_from.rs b/crates/ide-assists/src/handlers/into_to_qualified_from.rs index dee74afcbe..e405af5533 100644 --- a/crates/ide-assists/src/handlers/into_to_qualified_from.rs +++ b/crates/ide-assists/src/handlers/into_to_qualified_from.rs @@ -67,9 +67,9 @@ pub(crate) fn into_to_qualified_from(acc: &mut Assists, ctx: &AssistContext<'_>) edit.replace( method_call.syntax().text_range(), if sc.chars().all(|c| c.is_alphanumeric() || c == ':') { - format!("{}::from({})", sc, receiver) + format!("{sc}::from({receiver})") } else { - format!("<{}>::from({})", sc, receiver) + format!("<{sc}>::from({receiver})") }, ); }, diff --git a/crates/ide-assists/src/handlers/merge_nested_if.rs b/crates/ide-assists/src/handlers/merge_nested_if.rs index 2f3136f027..7a0037fa20 100644 --- a/crates/ide-assists/src/handlers/merge_nested_if.rs +++ b/crates/ide-assists/src/handlers/merge_nested_if.rs @@ -86,7 +86,7 @@ pub(crate) fn merge_nested_if(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt nested_if_cond.syntax().text().to_string() }; - let replace_cond = format!("{} && {}", cond_text, nested_if_cond_text); + let replace_cond = format!("{cond_text} && {nested_if_cond_text}"); edit.replace(cond_range, replace_cond); edit.replace(then_branch_range, nested_if_then_branch.syntax().text()); diff --git a/crates/ide-assists/src/handlers/remove_parentheses.rs b/crates/ide-assists/src/handlers/remove_parentheses.rs index 99c55e9ff7..799d36be93 100644 --- a/crates/ide-assists/src/handlers/remove_parentheses.rs +++ b/crates/ide-assists/src/handlers/remove_parentheses.rs @@ -48,7 +48,7 @@ pub(crate) fn remove_parentheses(acc: &mut Assists, ctx: &AssistContext<'_>) -> } None => false, }; - let expr = if need_to_add_ws { format!(" {}", expr) } else { expr.to_string() }; + let expr = if need_to_add_ws { format!(" {expr}") } else { expr.to_string() }; builder.replace(parens.syntax().text_range(), expr) }, diff --git a/crates/ide-completion/src/completions/postfix/format_like.rs b/crates/ide-completion/src/completions/postfix/format_like.rs index fd50fd4e8c..2755329bb3 100644 --- a/crates/ide-completion/src/completions/postfix/format_like.rs +++ b/crates/ide-completion/src/completions/postfix/format_like.rs @@ -65,7 +65,7 @@ pub(crate) fn add_format_like_completions( let exprs = with_placeholders(exprs); for (label, macro_name) in KINDS { let snippet = if exprs.is_empty() { - format!(r#"{}({})"#, macro_name, out) + format!(r#"{macro_name}({out})"#) } else { format!(r#"{}({}, {})"#, macro_name, out, exprs.join(", ")) }; @@ -108,7 +108,7 @@ mod tests { for (kind, input, output) in test_vector { let (parsed_string, _exprs) = parse_format_exprs(input).unwrap(); - let snippet = format!(r#"{}("{}")"#, kind, parsed_string); + let snippet = format!(r#"{kind}("{parsed_string}")"#); assert_eq!(&snippet, output); } } diff --git a/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs b/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs index 132b93df10..2b8779044f 100644 --- a/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs +++ b/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs @@ -37,7 +37,7 @@ impl State { self.names.insert(name.clone(), 1); 1 }; - make::name(&format!("{}{}", name, count)) + make::name(&format!("{name}{count}")) } fn serde_derive(&self) -> String { diff --git a/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs b/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs index 00710ef507..be1e6ed572 100644 --- a/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs +++ b/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs @@ -27,7 +27,7 @@ pub(crate) fn trait_impl_redundant_assoc_item( hir::AssocItem::Function(id) => { let function = id; ( - format!("`fn {}`", redundant_assoc_item_name), + format!("`fn {redundant_assoc_item_name}`"), function .source(db) .map(|it| it.syntax().value.text_range()) @@ -38,7 +38,7 @@ pub(crate) fn trait_impl_redundant_assoc_item( hir::AssocItem::Const(id) => { let constant = id; ( - format!("`const {}`", redundant_assoc_item_name), + format!("`const {redundant_assoc_item_name}`"), constant .source(db) .map(|it| it.syntax().value.text_range()) @@ -49,7 +49,7 @@ pub(crate) fn trait_impl_redundant_assoc_item( hir::AssocItem::TypeAlias(id) => { let type_alias = id; ( - format!("`type {}`", redundant_assoc_item_name), + format!("`type {redundant_assoc_item_name}`"), type_alias .source(db) .map(|it| it.syntax().value.text_range()) diff --git a/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/crates/ide-diagnostics/src/handlers/unresolved_method.rs index 0614fdc551..4f5ad685a4 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_method.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_method.rs @@ -162,11 +162,11 @@ fn assoc_func_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) - if !need_to_take_receiver_as_first_arg && !generic_parameters.is_empty() { let generic_parameters = generic_parameters.join(", "); receiver_type_adt_name = - format!("{}::<{}>", receiver_type_adt_name, generic_parameters); + format!("{receiver_type_adt_name}::<{generic_parameters}>"); } let method_name = call.name_ref()?; - let assoc_func_call = format!("{}::{}()", receiver_type_adt_name, method_name); + let assoc_func_call = format!("{receiver_type_adt_name}::{method_name}()"); let assoc_func_call = make::expr_path(make::path_from_text(&assoc_func_call)); @@ -184,8 +184,7 @@ fn assoc_func_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) - Some(Assist { id: AssistId("method_call_to_assoc_func_call_fix", AssistKind::QuickFix), label: Label::new(format!( - "Use associated func call instead: `{}`", - assoc_func_call_expr_string + "Use associated func call instead: `{assoc_func_call_expr_string}`" )), group: None, target: range, diff --git a/crates/ide/src/interpret_function.rs b/crates/ide/src/interpret_function.rs index df444a3f4d..7bd2da9f88 100644 --- a/crates/ide/src/interpret_function.rs +++ b/crates/ide/src/interpret_function.rs @@ -43,7 +43,7 @@ fn find_and_interpret(db: &RootDatabase, position: FilePosition) -> Option"); match db.line_index(file_id).try_line_col(text_range.start()) { Some(line_col) => format!("file://{path}#{}:{}", line_col.line + 1, line_col.col), - None => format!("file://{path} range {:?}", text_range), + None => format!("file://{path} range {text_range:?}"), } }; Some(def.eval(db, span_formatter)) diff --git a/crates/ide/src/view_memory_layout.rs b/crates/ide/src/view_memory_layout.rs index a229bc87c8..826447d058 100644 --- a/crates/ide/src/view_memory_layout.rs +++ b/crates/ide/src/view_memory_layout.rs @@ -40,7 +40,7 @@ impl fmt::Display for RecursiveMemoryLayout { "{}: {} (size: {}, align: {}, field offset: {})\n", node.item_name, node.typename, node.size, node.alignment, node.offset ); - write!(fmt, "{}", out)?; + write!(fmt, "{out}")?; if node.children_start != -1 { for j in nodes[idx].children_start ..(nodes[idx].children_start + nodes[idx].children_len as i64) diff --git a/crates/parser/src/grammar.rs b/crates/parser/src/grammar.rs index 4e5837312f..b342e35488 100644 --- a/crates/parser/src/grammar.rs +++ b/crates/parser/src/grammar.rs @@ -418,7 +418,7 @@ fn delimited( } if !p.eat(delim) { if p.at_ts(first_set) { - p.error(format!("expected {:?}", delim)); + p.error(format!("expected {delim:?}")); } else { break; } diff --git a/crates/paths/src/lib.rs b/crates/paths/src/lib.rs index 2d3653401d..4c79d67d5b 100644 --- a/crates/paths/src/lib.rs +++ b/crates/paths/src/lib.rs @@ -106,7 +106,7 @@ impl AbsPathBuf { /// Panics if `path` is not absolute. pub fn assert(path: Utf8PathBuf) -> AbsPathBuf { AbsPathBuf::try_from(path) - .unwrap_or_else(|path| panic!("expected absolute path, got {}", path)) + .unwrap_or_else(|path| panic!("expected absolute path, got {path}")) } /// Wrap the given absolute path in `AbsPathBuf` diff --git a/crates/proc-macro-api/src/process.rs b/crates/proc-macro-api/src/process.rs index dce086d429..718a96dc80 100644 --- a/crates/proc-macro-api/src/process.rs +++ b/crates/proc-macro-api/src/process.rs @@ -50,8 +50,7 @@ impl ProcMacroProcessSrv { Ok(v) if v > CURRENT_API_VERSION => Err(io::Error::new( io::ErrorKind::Other, format!( - "proc-macro server's api version ({}) is newer than rust-analyzer's ({})", - v, CURRENT_API_VERSION + "proc-macro server's api version ({v}) is newer than rust-analyzer's ({CURRENT_API_VERSION})" ), )), Ok(v) => { diff --git a/crates/project-model/src/sysroot.rs b/crates/project-model/src/sysroot.rs index 653e7157bc..1eeec4cede 100644 --- a/crates/project-model/src/sysroot.rs +++ b/crates/project-model/src/sysroot.rs @@ -219,8 +219,7 @@ impl Sysroot { ", try running `rustup component add rust-src` to possibly fix this" }; sysroot.error = Some(format!( - "sysroot at `{}` is missing a `core` library{var_note}", - src_root, + "sysroot at `{src_root}` is missing a `core` library{var_note}", )); } } diff --git a/crates/project-model/src/tests.rs b/crates/project-model/src/tests.rs index a6730863d6..2762de5997 100644 --- a/crates/project-model/src/tests.rs +++ b/crates/project-model/src/tests.rs @@ -126,7 +126,7 @@ fn replace_fake_sys_root(s: &mut String) { let fake_sysroot_path = get_test_path("fake-sysroot"); let fake_sysroot_path = if cfg!(windows) { let normalized_path = fake_sysroot_path.as_str().replace('\\', r#"\\"#); - format!(r#"{}\\"#, normalized_path) + format!(r#"{normalized_path}\\"#) } else { format!("{}/", fake_sysroot_path.as_str()) }; diff --git a/crates/rust-analyzer/src/cli/analysis_stats.rs b/crates/rust-analyzer/src/cli/analysis_stats.rs index bded41932c..932f2e70b3 100644 --- a/crates/rust-analyzer/src/cli/analysis_stats.rs +++ b/crates/rust-analyzer/src/cli/analysis_stats.rs @@ -479,7 +479,7 @@ impl flags::AnalysisStats { .or_insert(1); } else { acc.syntax_errors += 1; - bar.println(format!("Syntax error: \n{}", err)); + bar.println(format!("Syntax error: \n{err}")); } } } diff --git a/crates/rust-analyzer/src/cli/run_tests.rs b/crates/rust-analyzer/src/cli/run_tests.rs index a2d0dcc599..10cb2d5ad6 100644 --- a/crates/rust-analyzer/src/cli/run_tests.rs +++ b/crates/rust-analyzer/src/cli/run_tests.rs @@ -49,7 +49,7 @@ impl flags::RunTests { let mut sw_all = StopWatch::start(); for test in tests { let full_name = full_name_of_item(db, test.module(db), test.name(db)); - println!("test {}", full_name); + println!("test {full_name}"); if test.is_ignore(db) { println!("ignored"); ignore_count += 1; @@ -62,7 +62,7 @@ impl flags::RunTests { } else { fail_count += 1; } - println!("{}", result); + println!("{result}"); eprintln!("{:<20} {}", format!("test {}", full_name), sw_one.elapsed()); } println!("{pass_count} passed, {fail_count} failed, {ignore_count} ignored"); diff --git a/crates/rust-analyzer/src/cli/rustc_tests.rs b/crates/rust-analyzer/src/cli/rustc_tests.rs index e9a4db7a2b..31565878d8 100644 --- a/crates/rust-analyzer/src/cli/rustc_tests.rs +++ b/crates/rust-analyzer/src/cli/rustc_tests.rs @@ -220,8 +220,8 @@ impl Tester { self.pass_count += 1; } else { println!("{p:?} FAIL"); - println!("actual (r-a) = {:?}", actual); - println!("expected (rustc) = {:?}", expected); + println!("actual (r-a) = {actual:?}"); + println!("expected (rustc) = {expected:?}"); self.fail_count += 1; } } diff --git a/crates/rust-analyzer/src/tracing/hprof.rs b/crates/rust-analyzer/src/tracing/hprof.rs index 73f94671f2..978dcd67fa 100644 --- a/crates/rust-analyzer/src/tracing/hprof.rs +++ b/crates/rust-analyzer/src/tracing/hprof.rs @@ -199,7 +199,7 @@ impl Node { let _ = write!(out, " ({} calls)", self.count); } - eprintln!("{}", out); + eprintln!("{out}"); for child in &self.children { child.go(level + 1, filter) diff --git a/crates/salsa/salsa-macros/src/query_group.rs b/crates/salsa/salsa-macros/src/query_group.rs index 659797d6d4..c04b306916 100644 --- a/crates/salsa/salsa-macros/src/query_group.rs +++ b/crates/salsa/salsa-macros/src/query_group.rs @@ -20,7 +20,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream let input_span = input.span(); let (trait_attrs, salsa_attrs) = filter_attrs(input.attrs); if !salsa_attrs.is_empty() { - return Error::new(input_span, format!("unsupported attributes: {:?}", salsa_attrs)) + return Error::new(input_span, format!("unsupported attributes: {salsa_attrs:?}")) .to_compile_error() .into(); } @@ -78,7 +78,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream num_storages += 1; } _ => { - return Error::new(span, format!("unknown salsa attribute `{}`", name)) + return Error::new(span, format!("unknown salsa attribute `{name}`")) .to_compile_error() .into(); } @@ -111,7 +111,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream _ => { return Error::new( sig_span, - format!("first argument of query `{}` must be `&self`", query_name), + format!("first argument of query `{query_name}` must be `&self`"), ) .to_compile_error() .into(); @@ -130,7 +130,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream arg => { return Error::new( arg.span(), - format!("unsupported argument `{:?}` of `{}`", arg, query_name,), + format!("unsupported argument `{arg:?}` of `{query_name}`",), ) .to_compile_error() .into(); @@ -144,7 +144,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream ref ret => { return Error::new( ret.span(), - format!("unsupported return type `{:?}` of `{}`", ret, query_name), + format!("unsupported return type `{ret:?}` of `{query_name}`"), ) .to_compile_error() .into(); @@ -169,7 +169,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream let lookup_keys = vec![(parse_quote! { key }, value.clone())]; Some(Query { query_type: lookup_query_type, - query_name: format!("{}", lookup_fn_name), + query_name: format!("{lookup_fn_name}"), fn_name: lookup_fn_name, receiver: self_receiver.clone(), attrs: vec![], // FIXME -- some automatically generated docs on this method? @@ -274,8 +274,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream *Note:* Setting values will trigger cancellation of any ongoing queries; this method blocks until those queries have been cancelled. - ", - fn_name = fn_name + " ); let set_constant_fn_docs = format!( @@ -290,8 +289,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream *Note:* Setting values will trigger cancellation of any ongoing queries; this method blocks until those queries have been cancelled. - ", - fn_name = fn_name + " ); query_fn_declarations.extend(quote! { diff --git a/crates/salsa/tests/cycles.rs b/crates/salsa/tests/cycles.rs index ea5d15a250..e9bddfc630 100644 --- a/crates/salsa/tests/cycles.rs +++ b/crates/salsa/tests/cycles.rs @@ -162,7 +162,7 @@ fn extract_cycle(f: impl FnOnce() + UnwindSafe) -> salsa::Cycle { return cycle.clone(); } } - panic!("unexpected value: {:?}", v) + panic!("unexpected value: {v:?}") } #[test] diff --git a/crates/salsa/tests/incremental/constants.rs b/crates/salsa/tests/incremental/constants.rs index ea0eb81978..32bfbc4564 100644 --- a/crates/salsa/tests/incremental/constants.rs +++ b/crates/salsa/tests/incremental/constants.rs @@ -13,12 +13,12 @@ pub(crate) trait ConstantsDatabase: TestContext { } fn add(db: &dyn ConstantsDatabase, key1: char, key2: char) -> usize { - db.log().add(format!("add({}, {})", key1, key2)); + db.log().add(format!("add({key1}, {key2})")); db.input(key1) + db.input(key2) } fn add3(db: &dyn ConstantsDatabase, key1: char, key2: char, key3: char) -> usize { - db.log().add(format!("add3({}, {}, {})", key1, key2, key3)); + db.log().add(format!("add3({key1}, {key2}, {key3})")); db.add(key1, key2) + db.input(key3) } diff --git a/crates/salsa/tests/incremental/implementation.rs b/crates/salsa/tests/incremental/implementation.rs index 19752bba00..8434913441 100644 --- a/crates/salsa/tests/incremental/implementation.rs +++ b/crates/salsa/tests/incremental/implementation.rs @@ -26,7 +26,7 @@ pub(crate) struct TestContextImpl { impl TestContextImpl { #[track_caller] pub(crate) fn assert_log(&self, expected_log: &[&str]) { - let expected_text = &format!("{:#?}", expected_log); + let expected_text = &format!("{expected_log:#?}"); let actual_text = &format!("{:#?}", self.log().take()); if expected_text == actual_text { @@ -36,9 +36,9 @@ impl TestContextImpl { #[allow(clippy::print_stdout)] for diff in dissimilar::diff(expected_text, actual_text) { match diff { - dissimilar::Chunk::Delete(l) => println!("-{}", l), - dissimilar::Chunk::Equal(l) => println!(" {}", l), - dissimilar::Chunk::Insert(r) => println!("+{}", r), + dissimilar::Chunk::Delete(l) => println!("-{l}"), + dissimilar::Chunk::Equal(l) => println!(" {l}"), + dissimilar::Chunk::Insert(r) => println!("+{r}"), } } diff --git a/crates/salsa/tests/parallel/parallel_cycle_none_recover.rs b/crates/salsa/tests/parallel/parallel_cycle_none_recover.rs index 2930c4e379..3c73852eaf 100644 --- a/crates/salsa/tests/parallel/parallel_cycle_none_recover.rs +++ b/crates/salsa/tests/parallel/parallel_cycle_none_recover.rs @@ -33,7 +33,7 @@ fn parallel_cycle_none_recover() { "#]] .assert_debug_eq(&c.unexpected_participants(&db)); } else { - panic!("b failed in an unexpected way: {:?}", err_b); + panic!("b failed in an unexpected way: {err_b:?}"); } // We expect A to propagate a panic, which causes us to use the sentinel diff --git a/crates/salsa/tests/parallel/race.rs b/crates/salsa/tests/parallel/race.rs index e875de998f..c53d4b464e 100644 --- a/crates/salsa/tests/parallel/race.rs +++ b/crates/salsa/tests/parallel/race.rs @@ -28,7 +28,7 @@ fn in_par_get_set_race() { // cancellation, it'll unwind. let result1 = thread1.join().unwrap(); if let Ok(value1) = result1 { - assert!(value1 == 111 || value1 == 1011, "illegal result {}", value1); + assert!(value1 == 111 || value1 == 1011, "illegal result {value1}"); } // thread2 can not observe a cancellation because it performs a diff --git a/crates/span/src/lib.rs b/crates/span/src/lib.rs index 8ca7bc2d38..74693a270b 100644 --- a/crates/span/src/lib.rs +++ b/crates/span/src/lib.rs @@ -218,7 +218,7 @@ impl From for HirFileId { fn from(MacroFileId { macro_call_id: MacroCallId(id) }: MacroFileId) -> Self { _ = Self::ASSERT_MAX_FILE_ID_IS_SAME; let id = id.as_u32(); - assert!(id <= Self::MAX_HIR_FILE_ID, "MacroCallId index {} is too large", id); + assert!(id <= Self::MAX_HIR_FILE_ID, "MacroCallId index {id} is too large"); HirFileId(id | Self::MACRO_FILE_TAG_MASK) } } diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 186f1b01da..92a53c0f9e 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -172,7 +172,7 @@ pub fn ty_alias( assignment: Option<(ast::Type, Option)>, ) -> ast::TypeAlias { let mut s = String::new(); - s.push_str(&format!("type {}", ident)); + s.push_str(&format!("type {ident}")); if let Some(list) = generic_param_list { s.push_str(&list.to_string()); @@ -297,7 +297,7 @@ pub fn impl_trait( }; let where_clause = merge_where_clause(ty_where_clause, trait_where_clause) - .map_or_else(|| " ".to_owned(), |wc| format!("\n{}\n", wc)); + .map_or_else(|| " ".to_owned(), |wc| format!("\n{wc}\n")); let body = match body { Some(bd) => bd.iter().map(|elem| elem.to_string()).join(""), diff --git a/crates/test-fixture/src/lib.rs b/crates/test-fixture/src/lib.rs index be9961120d..e65186d377 100644 --- a/crates/test-fixture/src/lib.rs +++ b/crates/test-fixture/src/lib.rs @@ -196,7 +196,7 @@ impl ChangeFixture { origin, ); let prev = crates.insert(crate_name.clone(), crate_id); - assert!(prev.is_none(), "multiple crates with same name: {}", crate_name); + assert!(prev.is_none(), "multiple crates with same name: {crate_name}"); for dep in meta.deps { let prelude = match &meta.extern_prelude { Some(v) => v.contains(&dep), diff --git a/crates/test-utils/src/fixture.rs b/crates/test-utils/src/fixture.rs index aafe4fb5b1..9871fd2cf0 100644 --- a/crates/test-utils/src/fixture.rs +++ b/crates/test-utils/src/fixture.rs @@ -450,7 +450,7 @@ impl MiniCore { } if !active_regions.is_empty() { - panic!("unclosed regions: {:?} Add an `endregion` comment", active_regions); + panic!("unclosed regions: {active_regions:?} Add an `endregion` comment"); } for flag in &self.valid_flags { diff --git a/lib/lsp-server/src/lib.rs b/lib/lsp-server/src/lib.rs index 5dc052b587..4069e6f2c0 100644 --- a/lib/lsp-server/src/lib.rs +++ b/lib/lsp-server/src/lib.rs @@ -433,8 +433,7 @@ mod tests { initialize_start_test(TestCase { test_messages: vec![notification_msg.clone()], expected_resp: Err(ProtocolError::new(format!( - "expected initialize request, got {:?}", - notification_msg + "expected initialize request, got {notification_msg:?}" ))), }); } diff --git a/xtask/src/metrics.rs b/xtask/src/metrics.rs index 285abb9efc..9a7785dd43 100644 --- a/xtask/src/metrics.rs +++ b/xtask/src/metrics.rs @@ -64,7 +64,7 @@ impl flags::Metrics { }; let mut file = - fs::File::options().write(true).create(true).open(format!("target/{}.json", name))?; + fs::File::options().write(true).create(true).open(format!("target/{name}.json"))?; writeln!(file, "{}", metrics.json())?; eprintln!("{metrics:#?}"); Ok(()) From 7fa555f8ba1ad93523e4e42149fd2879b1aaa64f Mon Sep 17 00:00:00 2001 From: Hamir Mahal Date: Thu, 30 May 2024 16:39:41 -0700 Subject: [PATCH 007/124] fix: formatting in `handlers/unresolved_method.rs` --- crates/ide-diagnostics/src/handlers/unresolved_method.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/crates/ide-diagnostics/src/handlers/unresolved_method.rs index 4f5ad685a4..42211cdbe5 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_method.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_method.rs @@ -161,8 +161,7 @@ fn assoc_func_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) - // we could omit generic parameters cause compiler can deduce it automatically if !need_to_take_receiver_as_first_arg && !generic_parameters.is_empty() { let generic_parameters = generic_parameters.join(", "); - receiver_type_adt_name = - format!("{receiver_type_adt_name}::<{generic_parameters}>"); + receiver_type_adt_name = format!("{receiver_type_adt_name}::<{generic_parameters}>"); } let method_name = call.name_ref()?; From 0e1353bebdf4e152751c21b3b86b622944e135fa Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sun, 2 Jun 2024 14:56:38 +0200 Subject: [PATCH 008/124] Don't mark `#[rustc_deprecated_safe_2024]` functions as unsafe `std::env::set_var` will be unsafe in edition 2024, but not before it. I couldn't quite figure out how to check for the span properly, so for now we just turn the false positives into false negatives, which are less bad. --- crates/hir-def/src/attr/builtin.rs | 4 ++++ crates/hir-ty/src/utils.rs | 5 +++++ .../ide-diagnostics/src/handlers/missing_unsafe.rs | 14 ++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/crates/hir-def/src/attr/builtin.rs b/crates/hir-def/src/attr/builtin.rs index f4564c94bb..7b649496c4 100644 --- a/crates/hir-def/src/attr/builtin.rs +++ b/crates/hir-def/src/attr/builtin.rs @@ -628,6 +628,10 @@ pub const INERT_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_safe_intrinsic, Normal, template!(Word), WarnFollowing, "the `#[rustc_safe_intrinsic]` attribute is used internally to mark intrinsics as safe" ), + rustc_attr!( + rustc_deprecated_safe_2024, Normal, template!(Word), WarnFollowing, + "the `#[rustc_safe_intrinsic]` marks functions as unsafe in Rust 2024", + ), // ========================================================================== // Internal attributes, Testing: diff --git a/crates/hir-ty/src/utils.rs b/crates/hir-ty/src/utils.rs index 42c7a84032..cff8e6b036 100644 --- a/crates/hir-ty/src/utils.rs +++ b/crates/hir-ty/src/utils.rs @@ -534,6 +534,11 @@ fn parent_generic_def(db: &dyn DefDatabase, def: GenericDefId) -> Option bool { let data = db.function_data(func); if data.has_unsafe_kw() { + // Functions that are `#[rustc_deprecated_safe_2024]` are safe to call before 2024. + if db.attrs(func.into()).by_key("rustc_deprecated_safe_2024").exists() { + // FIXME: Properly check the caller span and mark it as unsafe after 2024. + return false; + } return true; } diff --git a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs index 1b29e0a374..30dd26a118 100644 --- a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs +++ b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs @@ -182,6 +182,20 @@ fn main() { ); } + #[test] + fn no_missing_unsafe_diagnostic_with_deprecated_safe_2024() { + check_diagnostics( + r#" +#[rustc_deprecated_safe_2024] +fn set_var() {} + +fn main() { + set_var(); +} +"#, + ); + } + #[test] fn add_unsafe_block_when_dereferencing_a_raw_pointer() { check_fix( From 3b763a847a1ca565c80d543978ccfc9eac25cd61 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 4 Mar 2024 20:18:27 -0500 Subject: [PATCH 009/124] Add `tt_from_syntax` Used for inserting syntax nodes into existing token trees --- crates/ide-assists/src/utils.rs | 47 +++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index bc0c9b79c7..b42779a902 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -14,9 +14,9 @@ use syntax::{ edit_in_place::{AttrsOwnerEdit, Indent, Removable}, make, HasArgList, HasAttrs, HasGenericParams, HasName, HasTypeBounds, Whitespace, }, - ted, AstNode, AstToken, Direction, SourceFile, + ted, AstNode, AstToken, Direction, NodeOrToken, SourceFile, SyntaxKind::*, - SyntaxNode, TextRange, TextSize, T, + SyntaxNode, SyntaxToken, TextRange, TextSize, T, }; use crate::assist_context::{AssistContext, SourceChangeBuilder}; @@ -916,3 +916,46 @@ pub(crate) fn replace_record_field_expr( edit.replace(file_range.range, initializer.syntax().text()); } } + +/// Creates a token tree list from a syntax node, creating the needed delimited sub token trees. +/// Assumes that the input syntax node is a valid syntax tree. +pub(crate) fn tt_from_syntax(node: SyntaxNode) -> Vec> { + let mut tt_stack = vec![(None, vec![])]; + + for element in node.descendants_with_tokens() { + let NodeOrToken::Token(token) = element else { continue }; + + match token.kind() { + T!['('] | T!['{'] | T!['['] => { + // Found an opening delimeter, start a new sub token tree + tt_stack.push((Some(token.kind()), vec![])); + } + T![')'] | T!['}'] | T![']'] => { + // Closing a subtree + let (delimiter, tt) = tt_stack.pop().expect("unbalanced delimeters"); + let (_, parent_tt) = tt_stack + .last_mut() + .expect("parent token tree was closed before it was completed"); + let closing_delimiter = delimiter.map(|it| match it { + T!['('] => T![')'], + T!['{'] => T!['}'], + T!['['] => T![']'], + _ => unreachable!(), + }); + stdx::always!( + closing_delimiter == Some(token.kind()), + "mismatched opening and closing delimiters" + ); + + let sub_tt = make::token_tree(delimiter.expect("unbalanced delimiters"), tt); + parent_tt.push(NodeOrToken::Node(sub_tt)); + } + _ => { + let (_, current_tt) = tt_stack.last_mut().expect("unmatched delimiters"); + current_tt.push(NodeOrToken::Token(token)) + } + } + } + + tt_stack.pop().expect("parent token tree was closed before it was completed").1 +} From c4573b26f68c093ceb05ee521d4803623e15bd29 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Fri, 8 Mar 2024 21:45:01 -0500 Subject: [PATCH 010/124] minor: tidy up `Parse` a little bit - Add doc comments to some `Parse` methods - Uses `Parse::new` more --- crates/syntax/src/lib.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/crates/syntax/src/lib.rs b/crates/syntax/src/lib.rs index 3a9ebafe87..b8a8103cc6 100644 --- a/crates/syntax/src/lib.rs +++ b/crates/syntax/src/lib.rs @@ -107,14 +107,22 @@ impl Parse { } impl Parse { + /// Converts this parse result into a parse result for an untyped syntax tree. pub fn to_syntax(self) -> Parse { Parse { green: self.green, errors: self.errors, _ty: PhantomData } } + /// Gets the parsed syntax tree as a typed ast node. + /// + /// # Panics + /// + /// Panics if the root node cannot be casted into the typed ast node + /// (e.g. if it's an `ERROR` node). pub fn tree(&self) -> T { T::cast(self.syntax_node()).unwrap() } + /// Converts from `Parse` to [`Result>`]. pub fn ok(self) -> Result> { match self.errors() { errors if !errors.is_empty() => Err(errors), @@ -177,11 +185,7 @@ impl SourceFile { let root = SyntaxNode::new_root(green.clone()); assert_eq!(root.kind(), SyntaxKind::SOURCE_FILE); - Parse { - green, - errors: if errors.is_empty() { None } else { Some(errors.into()) }, - _ty: PhantomData, - } + Parse::new(green, errors) } } @@ -290,12 +294,7 @@ impl ast::TokenTree { } let (green, errors) = builder.finish_raw(); - - Parse { - green, - errors: if errors.is_empty() { None } else { Some(errors.into()) }, - _ty: PhantomData, - } + Parse::new(green, errors) } } From e989f220157fee547e98f8fc009a4ad2c68eb3da Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Fri, 8 Mar 2024 21:45:35 -0500 Subject: [PATCH 011/124] Add `ast::Expr::parse` --- crates/syntax/src/lib.rs | 23 +++++++++++++++++++++++ crates/syntax/src/parsing.rs | 9 +++++++++ 2 files changed, 32 insertions(+) diff --git a/crates/syntax/src/lib.rs b/crates/syntax/src/lib.rs index b8a8103cc6..58f59c384b 100644 --- a/crates/syntax/src/lib.rs +++ b/crates/syntax/src/lib.rs @@ -175,6 +175,29 @@ impl Parse { } } +impl ast::Expr { + /// Parses an `ast::Expr` from `text`. + /// + /// Note that if the parsed root node is not a valid expression, [`Parse::tree`] will panic. + /// For example: + /// ```rust,should_panic + /// # use syntax::{ast, Edition}; + /// ast::Expr::parse("let fail = true;", Edition::CURRENT).tree(); + /// ``` + pub fn parse(text: &str, edition: Edition) -> Parse { + let _p = tracing::span!(tracing::Level::INFO, "Expr::parse").entered(); + let (green, errors) = parsing::parse_text_at(text, parser::TopEntryPoint::Expr, edition); + let root = SyntaxNode::new_root(green.clone()); + + assert!( + ast::Expr::can_cast(root.kind()) || root.kind() == SyntaxKind::ERROR, + "{:?} isn't an expression", + root.kind() + ); + Parse::new(green, errors) + } +} + /// `SourceFile` represents a parse tree for a single Rust file. pub use crate::ast::SourceFile; diff --git a/crates/syntax/src/parsing.rs b/crates/syntax/src/parsing.rs index 420f4938e5..165109029f 100644 --- a/crates/syntax/src/parsing.rs +++ b/crates/syntax/src/parsing.rs @@ -18,6 +18,15 @@ pub(crate) fn parse_text(text: &str, edition: parser::Edition) -> (GreenNode, Ve (node, errors) } +pub(crate) fn parse_text_at(text: &str, entry: parser::TopEntryPoint, edition: parser::Edition) -> (GreenNode, Vec) { + let _p = tracing::span!(tracing::Level::INFO, "parse_text_at").entered(); + let lexed = parser::LexedStr::new(text); + let parser_input = lexed.to_input(); + let parser_output = entry.parse(&parser_input, edition); + let (node, errors, _eof) = build_tree(lexed, parser_output); + (node, errors) +} + pub(crate) fn build_tree( lexed: parser::LexedStr<'_>, parser_output: parser::Output, From cf9401049cb9795bebf9d1a39654d09644486d38 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 4 Mar 2024 20:33:15 -0500 Subject: [PATCH 012/124] Make `extract_expressions_from_format_string` only use snippets when available --- .../extract_expressions_from_format_string.rs | 145 +++++++++++------- crates/ide-db/src/source_change.rs | 6 + crates/syntax/src/ast/make.rs | 2 +- 3 files changed, 98 insertions(+), 55 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_expressions_from_format_string.rs b/crates/ide-assists/src/handlers/extract_expressions_from_format_string.rs index 2725a97de8..28f645171c 100644 --- a/crates/ide-assists/src/handlers/extract_expressions_from_format_string.rs +++ b/crates/ide-assists/src/handlers/extract_expressions_from_format_string.rs @@ -1,4 +1,4 @@ -use crate::{AssistContext, Assists}; +use crate::{utils, AssistContext, Assists}; use hir::DescendPreference; use ide_db::{ assists::{AssistId, AssistKind}, @@ -8,8 +8,12 @@ use ide_db::{ }, }; use itertools::Itertools; -use stdx::format_to; -use syntax::{ast, AstNode, AstToken, NodeOrToken, SyntaxKind::COMMA, TextRange}; +use syntax::{ + ast::{self, make}, + ted, AstNode, AstToken, NodeOrToken, + SyntaxKind::WHITESPACE, + T, +}; // Assist: extract_expressions_from_format_string // @@ -34,6 +38,7 @@ pub(crate) fn extract_expressions_from_format_string( ) -> Option<()> { let fmt_string = ctx.find_token_at_offset::()?; let tt = fmt_string.syntax().parent().and_then(ast::TokenTree::cast)?; + let tt_delimiter = tt.left_delimiter_token()?.kind(); let expanded_t = ast::String::cast( ctx.sema @@ -61,72 +66,63 @@ pub(crate) fn extract_expressions_from_format_string( "Extract format expressions", tt.syntax().text_range(), |edit| { - let fmt_range = fmt_string.syntax().text_range(); - - // Replace old format string with new format string whose arguments have been extracted - edit.replace(fmt_range, new_fmt); - - // Insert cursor at end of format string - edit.insert(fmt_range.end(), "$0"); + let tt = edit.make_mut(tt); // Extract existing arguments in macro - let tokens = - tt.token_trees_and_tokens().collect_vec(); + let tokens = tt.token_trees_and_tokens().collect_vec(); - let mut existing_args: Vec = vec![]; - - let mut current_arg = String::new(); - if let [_opening_bracket, NodeOrToken::Token(format_string), _args_start_comma, tokens @ .., NodeOrToken::Token(end_bracket)] = + let existing_args = if let [_opening_bracket, NodeOrToken::Token(_format_string), _args_start_comma, tokens @ .., NodeOrToken::Token(_end_bracket)] = tokens.as_slice() { - for t in tokens { - match t { - NodeOrToken::Node(n) => { - format_to!(current_arg, "{n}"); - }, - NodeOrToken::Token(t) if t.kind() == COMMA => { - existing_args.push(current_arg.trim().into()); - current_arg.clear(); - }, - NodeOrToken::Token(t) => { - current_arg.push_str(t.text()); - }, - } - } - existing_args.push(current_arg.trim().into()); + let args = tokens.split(|it| matches!(it, NodeOrToken::Token(t) if t.kind() == T![,])).map(|arg| { + // Strip off leading and trailing whitespace tokens + let arg = match arg.split_first() { + Some((NodeOrToken::Token(t), rest)) if t.kind() == WHITESPACE => rest, + _ => arg, + }; + let arg = match arg.split_last() { + Some((NodeOrToken::Token(t), rest)) if t.kind() == WHITESPACE => rest, + _ => arg, + }; + arg + }); - // delete everything after the format string till end bracket - // we're going to insert the new arguments later - edit.delete(TextRange::new( - format_string.text_range().end(), - end_bracket.text_range().start(), - )); - } + args.collect() + } else { + vec![] + }; // Start building the new args let mut existing_args = existing_args.into_iter(); - let mut args = String::new(); + let mut new_tt_bits = vec![NodeOrToken::Token(make::tokens::literal(&new_fmt))]; + let mut placeholder_indexes = vec![]; - let mut placeholder_idx = 1; + for arg in extracted_args { + if matches!(arg, Arg::Expr(_) | Arg::Placeholder) { + // insert ", " before each arg + new_tt_bits.extend_from_slice(&[ + NodeOrToken::Token(make::token(T![,])), + NodeOrToken::Token(make::tokens::single_space()), + ]); + } - for extracted_args in extracted_args { - match extracted_args { - Arg::Expr(s)=> { - args.push_str(", "); + match arg { + Arg::Expr(s) => { // insert arg - args.push_str(&s); + // FIXME: use the crate's edition for parsing + let expr = ast::Expr::parse(&s, syntax::Edition::CURRENT).syntax_node(); + let mut expr_tt = utils::tt_from_syntax(expr); + new_tt_bits.append(&mut expr_tt); } Arg::Placeholder => { - args.push_str(", "); // try matching with existing argument match existing_args.next() { - Some(ea) => { - args.push_str(&ea); + Some(arg) => { + new_tt_bits.extend_from_slice(arg); } None => { - // insert placeholder - args.push_str(&format!("${placeholder_idx}")); - placeholder_idx += 1; + placeholder_indexes.push(new_tt_bits.len()); + new_tt_bits.push(NodeOrToken::Token(make::token(T![_]))); } } } @@ -134,8 +130,31 @@ pub(crate) fn extract_expressions_from_format_string( } } + // Insert new args - edit.insert(fmt_range.end(), args); + let new_tt = make::token_tree(tt_delimiter, new_tt_bits).clone_for_update(); + ted::replace(tt.syntax(), new_tt.syntax()); + + if let Some(cap) = ctx.config.snippet_cap { + // Add placeholder snippets over placeholder args + for pos in placeholder_indexes { + // Skip the opening delimiter + let Some(NodeOrToken::Token(placeholder)) = + new_tt.token_trees_and_tokens().skip(1).nth(pos) + else { + continue; + }; + + if stdx::always!(placeholder.kind() == T![_]) { + edit.add_placeholder_snippet_token(cap, placeholder); + } + } + + // Add the final tabstop after the format literal + if let Some(NodeOrToken::Token(literal)) = new_tt.token_trees_and_tokens().nth(1) { + edit.add_tabstop_after_token(cap, literal); + } + } }, ); @@ -145,7 +164,7 @@ pub(crate) fn extract_expressions_from_format_string( #[cfg(test)] mod tests { use super::*; - use crate::tests::check_assist; + use crate::tests::{check_assist, check_assist_no_snippet_cap}; #[test] fn multiple_middle_arg() { @@ -195,7 +214,7 @@ fn main() { "#, r#" fn main() { - print!("{} {:b} {} {}"$0, y + 2, x + 1, 2, $1); + print!("{} {:b} {} {}"$0, y + 2, x + 1, 2, ${1:_}); } "#, ); @@ -292,4 +311,22 @@ fn main() { "#, ); } + + #[test] + fn without_snippets() { + check_assist_no_snippet_cap( + extract_expressions_from_format_string, + r#" +//- minicore: fmt +fn main() { + print!("{} {x + 1:b} {} {}$0", y + 2, 2); +} +"#, + r#" +fn main() { + print!("{} {:b} {} {}", y + 2, x + 1, 2, _); +} +"#, + ); + } } diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs index f59d8d08c8..7ef7b7ae1d 100644 --- a/crates/ide-db/src/source_change.rs +++ b/crates/ide-db/src/source_change.rs @@ -338,6 +338,12 @@ impl SourceChangeBuilder { self.add_snippet(PlaceSnippet::Over(node.syntax().clone().into())) } + /// Adds a snippet to move the cursor selected over `token` + pub fn add_placeholder_snippet_token(&mut self, _cap: SnippetCap, token: SyntaxToken) { + assert!(token.parent().is_some()); + self.add_snippet(PlaceSnippet::Over(token.into())) + } + /// Adds a snippet to move the cursor selected over `nodes` /// /// This allows for renaming newly generated items without having to go diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 186f1b01da..bf5310c082 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -1159,7 +1159,7 @@ pub mod tokens { pub(super) static SOURCE_FILE: Lazy> = Lazy::new(|| { SourceFile::parse( - "const C: <()>::Item = ( true && true , true || true , 1 != 1, 2 == 2, 3 < 3, 4 <= 4, 5 > 5, 6 >= 6, !true, *p, &p , &mut p, { let a @ [] })\n;\n\nimpl A for B where: {}", Edition::CURRENT, + "const C: <()>::Item = ( true && true , true || true , 1 != 1, 2 == 2, 3 < 3, 4 <= 4, 5 > 5, 6 >= 6, !true, *p, &p , &mut p, { let _ @ [] })\n;\n\nimpl A for B where: {}", Edition::CURRENT, ) }); From a741bb20550e3e86f924f63d18eac1f86d5b3059 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Sun, 2 Jun 2024 14:09:36 -0400 Subject: [PATCH 013/124] fix typos & formatting --- crates/ide-assists/src/utils.rs | 4 ++-- crates/syntax/src/parsing.rs | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index b42779a902..ba6ef1921a 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -927,12 +927,12 @@ pub(crate) fn tt_from_syntax(node: SyntaxNode) -> Vec { - // Found an opening delimeter, start a new sub token tree + // Found an opening delimiter, start a new sub token tree tt_stack.push((Some(token.kind()), vec![])); } T![')'] | T!['}'] | T![']'] => { // Closing a subtree - let (delimiter, tt) = tt_stack.pop().expect("unbalanced delimeters"); + let (delimiter, tt) = tt_stack.pop().expect("unbalanced delimiters"); let (_, parent_tt) = tt_stack .last_mut() .expect("parent token tree was closed before it was completed"); diff --git a/crates/syntax/src/parsing.rs b/crates/syntax/src/parsing.rs index 165109029f..a1ca3b3279 100644 --- a/crates/syntax/src/parsing.rs +++ b/crates/syntax/src/parsing.rs @@ -18,7 +18,11 @@ pub(crate) fn parse_text(text: &str, edition: parser::Edition) -> (GreenNode, Ve (node, errors) } -pub(crate) fn parse_text_at(text: &str, entry: parser::TopEntryPoint, edition: parser::Edition) -> (GreenNode, Vec) { +pub(crate) fn parse_text_at( + text: &str, + entry: parser::TopEntryPoint, + edition: parser::Edition, +) -> (GreenNode, Vec) { let _p = tracing::span!(tracing::Level::INFO, "parse_text_at").entered(); let lexed = parser::LexedStr::new(text); let parser_input = lexed.to_input(); From cdb4f9631c88be66fc0ea84700f334600ace4a67 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 2 Jun 2024 21:28:25 +0200 Subject: [PATCH 014/124] Add path info to `AbsPathBuf::assert`'s assert --- crates/paths/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/paths/src/lib.rs b/crates/paths/src/lib.rs index 2d3653401d..b1cefe9ade 100644 --- a/crates/paths/src/lib.rs +++ b/crates/paths/src/lib.rs @@ -197,7 +197,7 @@ impl AbsPath { /// /// Panics if `path` is not absolute. pub fn assert(path: &Utf8Path) -> &AbsPath { - assert!(path.is_absolute()); + assert!(path.is_absolute(), "{path} is not absolute"); unsafe { &*(path as *const Utf8Path as *const AbsPath) } } From 60fa981df1242004b5cccdaa99335e0798cc91c8 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 3 Jun 2024 19:04:33 +0200 Subject: [PATCH 015/124] Simplify --- crates/hir-def/src/find_path.rs | 177 +++++++----------- crates/hir-def/src/item_tree.rs | 12 +- crates/hir-def/src/lib.rs | 38 ++-- crates/hir-def/src/nameres/path_resolution.rs | 4 +- crates/hir-def/src/path/lower.rs | 4 +- crates/hir-def/src/pretty.rs | 2 +- crates/hir-def/src/visibility.rs | 7 +- crates/hir-expand/src/mod_path.rs | 12 +- crates/hir-ty/src/display.rs | 2 +- crates/hir/src/attrs.rs | 2 +- crates/ide-db/src/helpers.rs | 2 +- 11 files changed, 113 insertions(+), 149 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index d9495d36c0..86eac6e97c 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -23,12 +23,29 @@ pub fn find_path( db: &dyn DefDatabase, item: ItemInNs, from: ModuleId, - prefix_kind: PrefixKind, + mut prefix_kind: PrefixKind, ignore_local_imports: bool, - cfg: ImportPathConfig, + mut cfg: ImportPathConfig, ) -> Option { let _p = tracing::span!(tracing::Level::INFO, "find_path").entered(); - find_path_inner(FindPathCtx { db, prefix: prefix_kind, cfg, ignore_local_imports }, item, from) + + // - if the item is a builtin, it's in scope + if let ItemInNs::Types(ModuleDefId::BuiltinType(builtin)) = item { + return Some(ModPath::from_segments(PathKind::Plain, iter::once(builtin.as_name()))); + } + + // within block modules, forcing a `self` or `crate` prefix will not allow using inner items, so + // default to plain paths. + if item.module(db).is_some_and(ModuleId::is_within_block) { + prefix_kind = PrefixKind::Plain; + } + cfg.prefer_no_std = cfg.prefer_no_std || db.crate_supports_no_std(from.krate()); + + find_path_inner( + &FindPathCtx { db, prefix: prefix_kind, cfg, ignore_local_imports, from }, + item, + MAX_PATH_LEN, + ) } #[derive(Copy, Clone, Debug)] @@ -63,79 +80,52 @@ impl PrefixKind { #[inline] fn path_kind(self) -> PathKind { match self { - PrefixKind::BySelf => PathKind::Super(0), + PrefixKind::BySelf => PathKind::SELF, PrefixKind::Plain => PathKind::Plain, PrefixKind::ByCrate => PathKind::Crate, } } } -#[derive(Copy, Clone)] struct FindPathCtx<'db> { db: &'db dyn DefDatabase, prefix: PrefixKind, cfg: ImportPathConfig, ignore_local_imports: bool, + from: ModuleId, } /// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId -fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Option { - // - if the item is a builtin, it's in scope - if let ItemInNs::Types(ModuleDefId::BuiltinType(builtin)) = item { - return Some(ModPath::from_segments(PathKind::Plain, iter::once(builtin.as_name()))); - } - - let def_map = from.def_map(ctx.db); - let crate_root = from.derive_crate_root(); +fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Option { + let def_map = ctx.from.def_map(ctx.db); // - if the item is a module, jump straight to module search if let ItemInNs::Types(ModuleDefId::ModuleId(module_id)) = item { let mut visited_modules = FxHashSet::default(); - return find_path_for_module( - FindPathCtx { - cfg: ImportPathConfig { - prefer_no_std: ctx.cfg.prefer_no_std - || ctx.db.crate_supports_no_std(crate_root.krate), - ..ctx.cfg - }, - ..ctx - }, - &def_map, - &mut visited_modules, - from, - module_id, - MAX_PATH_LEN, - ) - .map(|(item, _)| item); + return find_path_for_module(ctx, &def_map, &mut visited_modules, module_id, max_len) + .map(|(item, _)| item); } - let prefix = if item.module(ctx.db).is_some_and(|it| it.is_within_block()) { - PrefixKind::Plain - } else { - ctx.prefix - }; - let may_be_in_scope = match prefix { + let may_be_in_scope = match ctx.prefix { PrefixKind::Plain | PrefixKind::BySelf => true, - PrefixKind::ByCrate => from.is_crate_root(), + PrefixKind::ByCrate => ctx.from.is_crate_root(), }; if may_be_in_scope { // - if the item is already in scope, return the name under which it is - let scope_name = find_in_scope(ctx.db, &def_map, from, item, ctx.ignore_local_imports); + let scope_name = find_in_scope(ctx.db, &def_map, ctx.from, item, ctx.ignore_local_imports); if let Some(scope_name) = scope_name { - return Some(ModPath::from_segments(prefix.path_kind(), iter::once(scope_name))); + return Some(ModPath::from_segments(ctx.prefix.path_kind(), iter::once(scope_name))); } } // - if the item is in the prelude, return the name from there - if let value @ Some(_) = - find_in_prelude(ctx.db, &crate_root.def_map(ctx.db), &def_map, item, from) - { - return value; + if let Some(value) = find_in_prelude(ctx.db, &def_map, item, ctx.from) { + return Some(value); } if let Some(ModuleDefId::EnumVariantId(variant)) = item.as_module_def_id() { // - if the item is an enum variant, refer to it via the enum if let Some(mut path) = - find_path_inner(ctx, ItemInNs::Types(variant.lookup(ctx.db).parent.into()), from) + find_path_inner(ctx, ItemInNs::Types(variant.lookup(ctx.db).parent.into()), max_len) { path.push_segment(ctx.db.enum_variant_data(variant).name.clone()); return Some(path); @@ -147,30 +137,14 @@ fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Opti let mut visited_modules = FxHashSet::default(); - calculate_best_path( - FindPathCtx { - cfg: ImportPathConfig { - prefer_no_std: ctx.cfg.prefer_no_std - || ctx.db.crate_supports_no_std(crate_root.krate), - ..ctx.cfg - }, - ..ctx - }, - &def_map, - &mut visited_modules, - MAX_PATH_LEN, - item, - from, - ) - .map(|(item, _)| item) + calculate_best_path(ctx, &def_map, &mut visited_modules, item, max_len).map(|(item, _)| item) } #[tracing::instrument(skip_all)] fn find_path_for_module( - ctx: FindPathCtx<'_>, + ctx: &FindPathCtx<'_>, def_map: &DefMap, visited_modules: &mut FxHashSet, - from: ModuleId, module_id: ModuleId, max_len: usize, ) -> Option<(ModPath, Stability)> { @@ -180,20 +154,20 @@ fn find_path_for_module( let is_crate_root = module_id.as_crate_root(); // - if the item is the crate root, return `crate` - if is_crate_root.is_some_and(|it| it == from.derive_crate_root()) { + if is_crate_root == Some(ctx.from.derive_crate_root()) { return Some((ModPath::from_segments(PathKind::Crate, None), Stable)); } - let root_def_map = from.derive_crate_root().def_map(ctx.db); // - if the item is the crate root of a dependency crate, return the name from the extern prelude if let Some(crate_root) = is_crate_root { + let root_def_map = ctx.from.derive_crate_root().def_map(ctx.db); // rev here so we prefer looking at renamed extern decls first for (name, (def_id, _extern_crate)) in root_def_map.extern_prelude().rev() { if crate_root != def_id { continue; } let name_already_occupied_in_type_ns = def_map - .with_ancestor_maps(ctx.db, from.local_id, &mut |def_map, local_id| { + .with_ancestor_maps(ctx.db, ctx.from.local_id, &mut |def_map, local_id| { def_map[local_id] .scope .type_(name) @@ -209,30 +183,30 @@ fn find_path_for_module( return Some((ModPath::from_segments(kind, iter::once(name.clone())), Stable)); } } - let prefix = if module_id.is_within_block() { PrefixKind::Plain } else { ctx.prefix }; - let may_be_in_scope = match prefix { + + let may_be_in_scope = match ctx.prefix { PrefixKind::Plain | PrefixKind::BySelf => true, - PrefixKind::ByCrate => from.is_crate_root(), + PrefixKind::ByCrate => ctx.from.is_crate_root(), }; if may_be_in_scope { let scope_name = find_in_scope( ctx.db, def_map, - from, + ctx.from, ItemInNs::Types(module_id.into()), ctx.ignore_local_imports, ); if let Some(scope_name) = scope_name { // - if the item is already in scope, return the name under which it is return Some(( - ModPath::from_segments(prefix.path_kind(), iter::once(scope_name)), + ModPath::from_segments(ctx.prefix.path_kind(), iter::once(scope_name)), Stable, )); } } // - if the module can be referenced as self, super or crate, do that - if let Some(mod_path) = is_kw_kind_relative_to_from(def_map, module_id, from) { + if let Some(mod_path) = is_kw_kind_relative_to_from(def_map, module_id, ctx.from) { if ctx.prefix != PrefixKind::ByCrate || mod_path.kind == PathKind::Crate { return Some((mod_path, Stable)); } @@ -240,18 +214,11 @@ fn find_path_for_module( // - if the module is in the prelude, return it by that path if let Some(mod_path) = - find_in_prelude(ctx.db, &root_def_map, def_map, ItemInNs::Types(module_id.into()), from) + find_in_prelude(ctx.db, def_map, ItemInNs::Types(module_id.into()), ctx.from) { return Some((mod_path, Stable)); } - calculate_best_path( - ctx, - def_map, - visited_modules, - max_len, - ItemInNs::Types(module_id.into()), - from, - ) + calculate_best_path(ctx, def_map, visited_modules, ItemInNs::Types(module_id.into()), max_len) } // FIXME: Do we still need this now that we record import origins, and hence aliases? @@ -274,12 +241,11 @@ fn find_in_scope( /// name doesn't clash in current scope. fn find_in_prelude( db: &dyn DefDatabase, - root_def_map: &DefMap, local_def_map: &DefMap, item: ItemInNs, from: ModuleId, ) -> Option { - let (prelude_module, _) = root_def_map.prelude()?; + let (prelude_module, _) = local_def_map.prelude()?; // Preludes in block DefMaps are ignored, only the crate DefMap is searched let prelude_def_map = prelude_module.def_map(db); let prelude_scope = &prelude_def_map[prelude_module.local_id].scope; @@ -319,7 +285,7 @@ fn is_kw_kind_relative_to_from( let from = from.local_id; if item == from { // - if the item is the module we're in, use `self` - Some(ModPath::from_segments(PathKind::Super(0), None)) + Some(ModPath::from_segments(PathKind::SELF, None)) } else if let Some(parent_id) = def_map[from].parent { if item == parent_id { // - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly) @@ -337,12 +303,11 @@ fn is_kw_kind_relative_to_from( #[tracing::instrument(skip_all)] fn calculate_best_path( - ctx: FindPathCtx<'_>, + ctx: &FindPathCtx<'_>, def_map: &DefMap, visited_modules: &mut FxHashSet, - max_len: usize, item: ItemInNs, - from: ModuleId, + max_len: usize, ) -> Option<(ModPath, Stability)> { if max_len <= 1 { return None; @@ -356,24 +321,21 @@ fn calculate_best_path( } None => *best_path = Some(new_path), }; - // Recursive case: - // - otherwise, look for modules containing (reexporting) it and import it from one of those - if item.krate(ctx.db) == Some(from.krate) { + + if item.krate(ctx.db) == Some(ctx.from.krate) { let mut best_path_len = max_len; // Item was defined in the same crate that wants to import it. It cannot be found in any // dependency in this case. - for (module_id, name) in find_local_import_locations(ctx.db, item, from) { + // FIXME: cache the `find_local_import_locations` output? + for (module_id, name) in find_local_import_locations(ctx.db, item, ctx.from) { if !visited_modules.insert(module_id) { continue; } - if let Some(mut path) = find_path_for_module( - ctx, - def_map, - visited_modules, - from, - module_id, - best_path_len - 1, - ) { + // we are looking for paths of length up to best_path_len, any longer will make it be + // less optimal. The -1 is due to us pushing name onto it afterwards. + if let Some(mut path) = + find_path_for_module(ctx, def_map, visited_modules, module_id, best_path_len - 1) + { path.0.push_segment(name); let new_path = match best_path.take() { @@ -389,7 +351,7 @@ fn calculate_best_path( // too (unless we can't name it at all). It could *also* be (re)exported by the same crate // that wants to import it here, but we always prefer to use the external path here. - for dep in &ctx.db.crate_graph()[from.krate].dependencies { + for dep in &ctx.db.crate_graph()[ctx.from.krate].dependencies { let import_map = ctx.db.import_map(dep.crate_id); let Some(import_info_for) = import_map.import_info_for(item) else { continue }; for info in import_info_for { @@ -400,14 +362,15 @@ fn calculate_best_path( // Determine best path for containing module and append last segment from `info`. // FIXME: we should guide this to look up the path locally, or from the same crate again? - let Some((mut path, path_stability)) = find_path_for_module( + let path = find_path_for_module( ctx, def_map, visited_modules, - from, info.container, max_len - 1, - ) else { + // fixme shouldnt we consider the best path length here? + ); + let Some((mut path, path_stability)) = path else { continue; }; cov_mark::hit!(partially_imported); @@ -633,15 +596,13 @@ mod tests { .into_iter() .cartesian_product([false, true]) { - let found_path = find_path_inner( - FindPathCtx { - db: &db, - prefix, - cfg: ImportPathConfig { prefer_no_std: false, prefer_prelude }, - ignore_local_imports, - }, + let found_path = find_path( + &db, resolved, module, + prefix, + ignore_local_imports, + ImportPathConfig { prefer_no_std: false, prefer_prelude }, ); format_to!( res, diff --git a/crates/hir-def/src/item_tree.rs b/crates/hir-def/src/item_tree.rs index acda64c41f..52147e4a54 100644 --- a/crates/hir-def/src/item_tree.rs +++ b/crates/hir-def/src/item_tree.rs @@ -242,11 +242,11 @@ impl ItemVisibilities { match &vis { RawVisibility::Public => RawVisibilityId::PUB, RawVisibility::Module(path, explicitiy) if path.segments().is_empty() => { - match (&path.kind, explicitiy) { - (PathKind::Super(0), VisibilityExplicitness::Explicit) => { + match (path.kind, explicitiy) { + (PathKind::SELF, VisibilityExplicitness::Explicit) => { RawVisibilityId::PRIV_EXPLICIT } - (PathKind::Super(0), VisibilityExplicitness::Implicit) => { + (PathKind::SELF, VisibilityExplicitness::Implicit) => { RawVisibilityId::PRIV_IMPLICIT } (PathKind::Crate, _) => RawVisibilityId::PUB_CRATE, @@ -586,11 +586,11 @@ impl Index for ItemTree { fn index(&self, index: RawVisibilityId) -> &Self::Output { static VIS_PUB: RawVisibility = RawVisibility::Public; static VIS_PRIV_IMPLICIT: RawVisibility = RawVisibility::Module( - ModPath::from_kind(PathKind::Super(0)), + ModPath::from_kind(PathKind::SELF), VisibilityExplicitness::Implicit, ); static VIS_PRIV_EXPLICIT: RawVisibility = RawVisibility::Module( - ModPath::from_kind(PathKind::Super(0)), + ModPath::from_kind(PathKind::SELF), VisibilityExplicitness::Explicit, ); static VIS_PUB_CRATE: RawVisibility = RawVisibility::Module( @@ -928,7 +928,7 @@ impl UseTree { _ => None, } } - (Some(prefix), PathKind::Super(0)) if path.segments().is_empty() => { + (Some(prefix), PathKind::SELF) if path.segments().is_empty() => { // `some::path::self` == `some::path` Some((prefix, ImportKind::TypeOnly)) } diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index 682d169adb..b86703c3cb 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -396,6 +396,23 @@ impl PartialEq for CrateRootModuleId { other.block.is_none() && other.local_id == DefMap::ROOT && self.krate == other.krate } } +impl PartialEq for ModuleId { + fn eq(&self, other: &CrateRootModuleId) -> bool { + other == self + } +} + +impl From for ModuleId { + fn from(CrateRootModuleId { krate }: CrateRootModuleId) -> Self { + ModuleId { krate, block: None, local_id: DefMap::ROOT } + } +} + +impl From for ModuleDefId { + fn from(value: CrateRootModuleId) -> Self { + ModuleDefId::ModuleId(value.into()) + } +} impl From for CrateRootModuleId { fn from(krate: CrateId) -> Self { @@ -472,6 +489,7 @@ impl ModuleId { self.block.is_some() } + /// Returns the [`CrateRootModuleId`] for this module if it is the crate root module. pub fn as_crate_root(&self) -> Option { if self.local_id == DefMap::ROOT && self.block.is_none() { Some(CrateRootModuleId { krate: self.krate }) @@ -480,33 +498,17 @@ impl ModuleId { } } + /// Returns the [`CrateRootModuleId`] for this module. pub fn derive_crate_root(&self) -> CrateRootModuleId { CrateRootModuleId { krate: self.krate } } + /// Whether this module represents the crate root module fn is_crate_root(&self) -> bool { self.local_id == DefMap::ROOT && self.block.is_none() } } -impl PartialEq for ModuleId { - fn eq(&self, other: &CrateRootModuleId) -> bool { - other == self - } -} - -impl From for ModuleId { - fn from(CrateRootModuleId { krate }: CrateRootModuleId) -> Self { - ModuleId { krate, block: None, local_id: DefMap::ROOT } - } -} - -impl From for ModuleDefId { - fn from(value: CrateRootModuleId) -> Self { - ModuleDefId::ModuleId(value.into()) - } -} - /// An ID of a module, **local** to a `DefMap`. pub type LocalModuleId = Idx; diff --git a/crates/hir-def/src/nameres/path_resolution.rs b/crates/hir-def/src/nameres/path_resolution.rs index d621f3a360..863ccec885 100644 --- a/crates/hir-def/src/nameres/path_resolution.rs +++ b/crates/hir-def/src/nameres/path_resolution.rs @@ -283,7 +283,7 @@ impl DefMap { // If we have a different `DefMap` from `self` (the original `DefMap` we started // with), resolve the remaining path segments in that `DefMap`. let path = - ModPath::from_segments(PathKind::Super(0), path.segments().iter().cloned()); + ModPath::from_segments(PathKind::SELF, path.segments().iter().cloned()); return def_map.resolve_path_fp_with_macro( db, mode, @@ -333,7 +333,7 @@ impl DefMap { ModuleDefId::ModuleId(module) => { if module.krate != self.krate { let path = ModPath::from_segments( - PathKind::Super(0), + PathKind::SELF, path.segments()[i..].iter().cloned(), ); tracing::debug!("resolving {:?} in other crate", path); diff --git a/crates/hir-def/src/path/lower.rs b/crates/hir-def/src/path/lower.rs index 6af5261411..2b555b3998 100644 --- a/crates/hir-def/src/path/lower.rs +++ b/crates/hir-def/src/path/lower.rs @@ -122,7 +122,7 @@ pub(super) fn lower_path(ctx: &LowerCtx<'_>, mut path: ast::Path) -> Option, mut path: ast::Path) -> Option match path.kind() { PathKind::Plain => {} - PathKind::Super(0) => write!(buf, "self")?, + &PathKind::SELF => write!(buf, "self")?, PathKind::Super(n) => { for i in 0..*n { if i == 0 { diff --git a/crates/hir-def/src/visibility.rs b/crates/hir-def/src/visibility.rs index 1ef8fa772a..e08718fc83 100644 --- a/crates/hir-def/src/visibility.rs +++ b/crates/hir-def/src/visibility.rs @@ -27,10 +27,7 @@ pub enum RawVisibility { impl RawVisibility { pub(crate) const fn private() -> RawVisibility { - RawVisibility::Module( - ModPath::from_kind(PathKind::Super(0)), - VisibilityExplicitness::Implicit, - ) + RawVisibility::Module(ModPath::from_kind(PathKind::SELF), VisibilityExplicitness::Implicit) } pub(crate) fn from_ast( @@ -60,7 +57,7 @@ impl RawVisibility { } ast::VisibilityKind::PubCrate => ModPath::from_kind(PathKind::Crate), ast::VisibilityKind::PubSuper => ModPath::from_kind(PathKind::Super(1)), - ast::VisibilityKind::PubSelf => ModPath::from_kind(PathKind::Super(0)), + ast::VisibilityKind::PubSelf => ModPath::from_kind(PathKind::SELF), ast::VisibilityKind::Pub => return RawVisibility::Public, }; RawVisibility::Module(path, VisibilityExplicitness::Explicit) diff --git a/crates/hir-expand/src/mod_path.rs b/crates/hir-expand/src/mod_path.rs index 46f8c2b9d8..12fdf88a2a 100644 --- a/crates/hir-expand/src/mod_path.rs +++ b/crates/hir-expand/src/mod_path.rs @@ -44,6 +44,10 @@ pub enum PathKind { DollarCrate(CrateId), } +impl PathKind { + pub const SELF: PathKind = PathKind::Super(0); +} + impl ModPath { pub fn from_src( db: &dyn ExpandDatabase, @@ -96,7 +100,7 @@ impl ModPath { pub fn textual_len(&self) -> usize { let base = match self.kind { PathKind::Plain => 0, - PathKind::Super(0) => "self".len(), + PathKind::SELF => "self".len(), PathKind::Super(i) => "super".len() * i as usize, PathKind::Crate => "crate".len(), PathKind::Abs => 0, @@ -113,7 +117,7 @@ impl ModPath { } pub fn is_self(&self) -> bool { - self.kind == PathKind::Super(0) && self.segments.is_empty() + self.kind == PathKind::SELF && self.segments.is_empty() } #[allow(non_snake_case)] @@ -193,7 +197,7 @@ fn display_fmt_path( }; match path.kind { PathKind::Plain => {} - PathKind::Super(0) => add_segment("self")?, + PathKind::SELF => add_segment("self")?, PathKind::Super(n) => { for _ in 0..n { add_segment("super")?; @@ -316,7 +320,7 @@ fn convert_path_tt(db: &dyn ExpandDatabase, tt: &[tt::TokenTree]) -> Option { resolve_crate_root(db, span.ctx).map(PathKind::DollarCrate).unwrap_or(PathKind::Crate) } - tt::Leaf::Ident(tt::Ident { text, .. }) if text == "self" => PathKind::Super(0), + tt::Leaf::Ident(tt::Ident { text, .. }) if text == "self" => PathKind::SELF, tt::Leaf::Ident(tt::Ident { text, .. }) if text == "super" => { let mut deg = 1; while let Some(tt::Leaf::Ident(tt::Ident { text, .. })) = leaves.next() { diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs index 0027eb56a8..f035dd11e1 100644 --- a/crates/hir-ty/src/display.rs +++ b/crates/hir-ty/src/display.rs @@ -1942,7 +1942,7 @@ impl HirDisplay for Path { (_, PathKind::Plain) => {} (_, PathKind::Abs) => {} (_, PathKind::Crate) => write!(f, "crate")?, - (_, PathKind::Super(0)) => write!(f, "self")?, + (_, &PathKind::SELF) => write!(f, "self")?, (_, PathKind::Super(n)) => { for i in 0..*n { if i > 0 { diff --git a/crates/hir/src/attrs.rs b/crates/hir/src/attrs.rs index c7502890ef..7b3ff7b064 100644 --- a/crates/hir/src/attrs.rs +++ b/crates/hir/src/attrs.rs @@ -307,7 +307,7 @@ fn doc_modpath_from_str(link: &str) -> Option { let kind = match parts.next()? { "" => PathKind::Abs, "crate" => PathKind::Crate, - "self" => PathKind::Super(0), + "self" => PathKind::SELF, "super" => { let mut deg = 1; for segment in parts.by_ref() { diff --git a/crates/ide-db/src/helpers.rs b/crates/ide-db/src/helpers.rs index db44b1e723..063e366d4b 100644 --- a/crates/ide-db/src/helpers.rs +++ b/crates/ide-db/src/helpers.rs @@ -41,7 +41,7 @@ pub fn mod_path_to_ast(path: &hir::ModPath) -> ast::Path { let mut is_abs = false; match path.kind { hir::PathKind::Plain => {} - hir::PathKind::Super(0) => segments.push(make::path_segment_self()), + hir::PathKind::SELF => segments.push(make::path_segment_self()), hir::PathKind::Super(n) => segments.extend((0..n).map(|_| make::path_segment_super())), hir::PathKind::DollarCrate(_) | hir::PathKind::Crate => { segments.push(make::path_segment_crate()) From db80216dac3d972612d8e2d12ade3c28a1826ac2 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 3 Jun 2024 19:21:19 +0200 Subject: [PATCH 016/124] Fix find_path search not reducing scope appropriately for foreign items --- crates/hir-def/src/find_path.rs | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index 86eac6e97c..d386601d57 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -48,7 +48,7 @@ pub fn find_path( ) } -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] enum Stability { Unstable, Stable, @@ -322,12 +322,14 @@ fn calculate_best_path( None => *best_path = Some(new_path), }; - if item.krate(ctx.db) == Some(ctx.from.krate) { - let mut best_path_len = max_len; + let db = ctx.db; + + let mut best_path_len = max_len; + if item.krate(db) == Some(ctx.from.krate) { // Item was defined in the same crate that wants to import it. It cannot be found in any // dependency in this case. // FIXME: cache the `find_local_import_locations` output? - for (module_id, name) in find_local_import_locations(ctx.db, item, ctx.from) { + for (module_id, name) in find_local_import_locations(db, item, ctx.from) { if !visited_modules.insert(module_id) { continue; } @@ -342,7 +344,9 @@ fn calculate_best_path( Some(best_path) => select_best_path(best_path, path, ctx.cfg), None => path, }; - best_path_len = new_path.0.len(); + if new_path.1 == Stable { + best_path_len = new_path.0.len(); + } update_best_path(&mut best_path, new_path); } } @@ -351,8 +355,8 @@ fn calculate_best_path( // too (unless we can't name it at all). It could *also* be (re)exported by the same crate // that wants to import it here, but we always prefer to use the external path here. - for dep in &ctx.db.crate_graph()[ctx.from.krate].dependencies { - let import_map = ctx.db.import_map(dep.crate_id); + for dep in &db.crate_graph()[ctx.from.krate].dependencies { + let import_map = db.import_map(dep.crate_id); let Some(import_info_for) = import_map.import_info_for(item) else { continue }; for info in import_info_for { if info.is_doc_hidden { @@ -367,8 +371,7 @@ fn calculate_best_path( def_map, visited_modules, info.container, - max_len - 1, - // fixme shouldnt we consider the best path length here? + best_path_len - 1, ); let Some((mut path, path_stability)) = path else { continue; @@ -381,11 +384,14 @@ fn calculate_best_path( zip_stability(path_stability, if info.is_unstable { Unstable } else { Stable }), ); - let new_path_with_stab = match best_path.take() { + let new_path = match best_path.take() { Some(best_path) => select_best_path(best_path, path_with_stab, ctx.cfg), None => path_with_stab, }; - update_best_path(&mut best_path, new_path_with_stab); + if new_path.1 == Stable { + best_path_len = new_path.0.len(); + } + update_best_path(&mut best_path, new_path); } } } @@ -393,7 +399,7 @@ fn calculate_best_path( } /// Select the best (most relevant) path between two paths. -/// This accounts for stability, path length whether std should be chosen over alloc/core paths as +/// This accounts for stability, path length whether, std should be chosen over alloc/core paths as /// well as ignoring prelude like paths or not. fn select_best_path( old_path @ (_, old_stability): (ModPath, Stability), From 426d01eab01c88482fa5922e89cc4f2473552477 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 3 Jun 2024 19:26:11 +0200 Subject: [PATCH 017/124] Deduplicate --- crates/hir-def/src/find_path.rs | 50 +++++++++++++-------------------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index d386601d57..b64a62e8fa 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -312,19 +312,27 @@ fn calculate_best_path( if max_len <= 1 { return None; } + let mut best_path = None; - let update_best_path = - |best_path: &mut Option<_>, new_path: (ModPath, Stability)| match best_path { + let mut best_path_len = max_len; + let mut process = |mut path: (ModPath, Stability), name, best_path_len: &mut _| { + path.0.push_segment(name); + let new_path = match best_path.take() { + Some(best_path) => select_best_path(best_path, path, ctx.cfg), + None => path, + }; + if new_path.1 == Stable { + *best_path_len = new_path.0.len(); + } + match &mut best_path { Some((old_path, old_stability)) => { *old_path = new_path.0; *old_stability = zip_stability(*old_stability, new_path.1); } - None => *best_path = Some(new_path), - }; - + None => best_path = Some(new_path), + } + }; let db = ctx.db; - - let mut best_path_len = max_len; if item.krate(db) == Some(ctx.from.krate) { // Item was defined in the same crate that wants to import it. It cannot be found in any // dependency in this case. @@ -335,19 +343,10 @@ fn calculate_best_path( } // we are looking for paths of length up to best_path_len, any longer will make it be // less optimal. The -1 is due to us pushing name onto it afterwards. - if let Some(mut path) = + if let Some(path) = find_path_for_module(ctx, def_map, visited_modules, module_id, best_path_len - 1) { - path.0.push_segment(name); - - let new_path = match best_path.take() { - Some(best_path) => select_best_path(best_path, path, ctx.cfg), - None => path, - }; - if new_path.1 == Stable { - best_path_len = new_path.0.len(); - } - update_best_path(&mut best_path, new_path); + process(path, name, &mut best_path_len); } } } else { @@ -373,25 +372,16 @@ fn calculate_best_path( info.container, best_path_len - 1, ); - let Some((mut path, path_stability)) = path else { + let Some((path, path_stability)) = path else { continue; }; cov_mark::hit!(partially_imported); - path.push_segment(info.name.clone()); - - let path_with_stab = ( + let path = ( path, zip_stability(path_stability, if info.is_unstable { Unstable } else { Stable }), ); - let new_path = match best_path.take() { - Some(best_path) => select_best_path(best_path, path_with_stab, ctx.cfg), - None => path_with_stab, - }; - if new_path.1 == Stable { - best_path_len = new_path.0.len(); - } - update_best_path(&mut best_path, new_path); + process(path, info.name.clone(), &mut best_path_len); } } } From 48822e0941fce994b0e81f82ce8426b19187e5b0 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 3 Jun 2024 19:46:40 +0200 Subject: [PATCH 018/124] Simplify --- crates/hir-def/src/find_path.rs | 64 ++++++++++++++++----------------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index b64a62e8fa..9d252aac1e 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -42,7 +42,14 @@ pub fn find_path( cfg.prefer_no_std = cfg.prefer_no_std || db.crate_supports_no_std(from.krate()); find_path_inner( - &FindPathCtx { db, prefix: prefix_kind, cfg, ignore_local_imports, from }, + &FindPathCtx { + db, + prefix: prefix_kind, + cfg, + ignore_local_imports, + from, + from_def_map: &from.def_map(db), + }, item, MAX_PATH_LEN, ) @@ -93,15 +100,15 @@ struct FindPathCtx<'db> { cfg: ImportPathConfig, ignore_local_imports: bool, from: ModuleId, + from_def_map: &'db DefMap, } /// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Option { - let def_map = ctx.from.def_map(ctx.db); // - if the item is a module, jump straight to module search if let ItemInNs::Types(ModuleDefId::ModuleId(module_id)) = item { let mut visited_modules = FxHashSet::default(); - return find_path_for_module(ctx, &def_map, &mut visited_modules, module_id, max_len) + return find_path_for_module(ctx, &mut visited_modules, module_id, max_len) .map(|(item, _)| item); } @@ -111,14 +118,15 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt }; if may_be_in_scope { // - if the item is already in scope, return the name under which it is - let scope_name = find_in_scope(ctx.db, &def_map, ctx.from, item, ctx.ignore_local_imports); + let scope_name = + find_in_scope(ctx.db, ctx.from_def_map, ctx.from, item, ctx.ignore_local_imports); if let Some(scope_name) = scope_name { return Some(ModPath::from_segments(ctx.prefix.path_kind(), iter::once(scope_name))); } } // - if the item is in the prelude, return the name from there - if let Some(value) = find_in_prelude(ctx.db, &def_map, item, ctx.from) { + if let Some(value) = find_in_prelude(ctx.db, ctx.from_def_map, item, ctx.from) { return Some(value); } @@ -137,13 +145,12 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt let mut visited_modules = FxHashSet::default(); - calculate_best_path(ctx, &def_map, &mut visited_modules, item, max_len).map(|(item, _)| item) + calculate_best_path(ctx, &mut visited_modules, item, max_len).map(|(item, _)| item) } #[tracing::instrument(skip_all)] fn find_path_for_module( ctx: &FindPathCtx<'_>, - def_map: &DefMap, visited_modules: &mut FxHashSet, module_id: ModuleId, max_len: usize, @@ -152,21 +159,21 @@ fn find_path_for_module( return None; } - let is_crate_root = module_id.as_crate_root(); - // - if the item is the crate root, return `crate` - if is_crate_root == Some(ctx.from.derive_crate_root()) { - return Some((ModPath::from_segments(PathKind::Crate, None), Stable)); - } + if let Some(crate_root) = module_id.as_crate_root() { + if crate_root == ctx.from.derive_crate_root() { + // - if the item is the crate root, return `crate` + return Some((ModPath::from_segments(PathKind::Crate, None), Stable)); + } + // - otherwise if the item is the crate root of a dependency crate, return the name from the extern prelude - // - if the item is the crate root of a dependency crate, return the name from the extern prelude - if let Some(crate_root) = is_crate_root { let root_def_map = ctx.from.derive_crate_root().def_map(ctx.db); // rev here so we prefer looking at renamed extern decls first for (name, (def_id, _extern_crate)) in root_def_map.extern_prelude().rev() { if crate_root != def_id { continue; } - let name_already_occupied_in_type_ns = def_map + let name_already_occupied_in_type_ns = ctx + .from_def_map .with_ancestor_maps(ctx.db, ctx.from.local_id, &mut |def_map, local_id| { def_map[local_id] .scope @@ -191,7 +198,7 @@ fn find_path_for_module( if may_be_in_scope { let scope_name = find_in_scope( ctx.db, - def_map, + ctx.from_def_map, ctx.from, ItemInNs::Types(module_id.into()), ctx.ignore_local_imports, @@ -206,7 +213,7 @@ fn find_path_for_module( } // - if the module can be referenced as self, super or crate, do that - if let Some(mod_path) = is_kw_kind_relative_to_from(def_map, module_id, ctx.from) { + if let Some(mod_path) = is_kw_kind_relative_to_from(ctx.from_def_map, module_id, ctx.from) { if ctx.prefix != PrefixKind::ByCrate || mod_path.kind == PathKind::Crate { return Some((mod_path, Stable)); } @@ -214,14 +221,13 @@ fn find_path_for_module( // - if the module is in the prelude, return it by that path if let Some(mod_path) = - find_in_prelude(ctx.db, def_map, ItemInNs::Types(module_id.into()), ctx.from) + find_in_prelude(ctx.db, ctx.from_def_map, ItemInNs::Types(module_id.into()), ctx.from) { return Some((mod_path, Stable)); } - calculate_best_path(ctx, def_map, visited_modules, ItemInNs::Types(module_id.into()), max_len) + calculate_best_path(ctx, visited_modules, ItemInNs::Types(module_id.into()), max_len) } -// FIXME: Do we still need this now that we record import origins, and hence aliases? fn find_in_scope( db: &dyn DefDatabase, def_map: &DefMap, @@ -246,7 +252,6 @@ fn find_in_prelude( from: ModuleId, ) -> Option { let (prelude_module, _) = local_def_map.prelude()?; - // Preludes in block DefMaps are ignored, only the crate DefMap is searched let prelude_def_map = prelude_module.def_map(db); let prelude_scope = &prelude_def_map[prelude_module.local_id].scope; let (name, vis, _declared) = prelude_scope.name_of(item)?; @@ -304,7 +309,6 @@ fn is_kw_kind_relative_to_from( #[tracing::instrument(skip_all)] fn calculate_best_path( ctx: &FindPathCtx<'_>, - def_map: &DefMap, visited_modules: &mut FxHashSet, item: ItemInNs, max_len: usize, @@ -337,14 +341,14 @@ fn calculate_best_path( // Item was defined in the same crate that wants to import it. It cannot be found in any // dependency in this case. // FIXME: cache the `find_local_import_locations` output? - for (module_id, name) in find_local_import_locations(db, item, ctx.from) { + for (module_id, name) in find_local_import_locations(db, item, ctx.from, ctx.from_def_map) { if !visited_modules.insert(module_id) { continue; } // we are looking for paths of length up to best_path_len, any longer will make it be // less optimal. The -1 is due to us pushing name onto it afterwards. if let Some(path) = - find_path_for_module(ctx, def_map, visited_modules, module_id, best_path_len - 1) + find_path_for_module(ctx, visited_modules, module_id, best_path_len - 1) { process(path, name, &mut best_path_len); } @@ -365,13 +369,8 @@ fn calculate_best_path( // Determine best path for containing module and append last segment from `info`. // FIXME: we should guide this to look up the path locally, or from the same crate again? - let path = find_path_for_module( - ctx, - def_map, - visited_modules, - info.container, - best_path_len - 1, - ); + let path = + find_path_for_module(ctx, visited_modules, info.container, best_path_len - 1); let Some((path, path_stability)) = path else { continue; }; @@ -461,6 +460,7 @@ fn find_local_import_locations( db: &dyn DefDatabase, item: ItemInNs, from: ModuleId, + def_map: &DefMap, ) -> Vec<(ModuleId, Name)> { let _p = tracing::span!(tracing::Level::INFO, "find_local_import_locations").entered(); @@ -468,8 +468,6 @@ fn find_local_import_locations( // above `from` with any visibility. That means we do not need to descend into private siblings // of `from` (and similar). - let def_map = from.def_map(db); - // Compute the initial worklist. We start with all direct child modules of `from` as well as all // of its (recursive) parent modules. let data = &def_map[from.local_id]; From f94d34bd727867b7189bc60cf6a979b0df36917b Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 3 Jun 2024 19:57:49 +0200 Subject: [PATCH 019/124] Remove an allocation in `find_path::find_local_import_locations` --- crates/hir-def/src/find_path.rs | 40 ++++++++++++++------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index 9d252aac1e..e68f0d45cd 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -155,10 +155,6 @@ fn find_path_for_module( module_id: ModuleId, max_len: usize, ) -> Option<(ModPath, Stability)> { - if max_len == 0 { - return None; - } - if let Some(crate_root) = module_id.as_crate_root() { if crate_root == ctx.from.derive_crate_root() { // - if the item is the crate root, return `crate` @@ -314,6 +310,8 @@ fn calculate_best_path( max_len: usize, ) -> Option<(ModPath, Stability)> { if max_len <= 1 { + // recursive base case, we can't find a path prefix of length 0, one segment is occupied by + // the item's name itself. return None; } @@ -341,18 +339,18 @@ fn calculate_best_path( // Item was defined in the same crate that wants to import it. It cannot be found in any // dependency in this case. // FIXME: cache the `find_local_import_locations` output? - for (module_id, name) in find_local_import_locations(db, item, ctx.from, ctx.from_def_map) { + find_local_import_locations(db, item, ctx.from, ctx.from_def_map, |name, module_id| { if !visited_modules.insert(module_id) { - continue; + return; } // we are looking for paths of length up to best_path_len, any longer will make it be // less optimal. The -1 is due to us pushing name onto it afterwards. if let Some(path) = find_path_for_module(ctx, visited_modules, module_id, best_path_len - 1) { - process(path, name, &mut best_path_len); + process(path, name.clone(), &mut best_path_len); } - } + }) } else { // Item was defined in some upstream crate. This means that it must be exported from one, // too (unless we can't name it at all). It could *also* be (re)exported by the same crate @@ -454,14 +452,14 @@ fn select_best_path( } } -// FIXME: Remove allocations /// Finds locations in `from.krate` from which `item` can be imported by `from`. fn find_local_import_locations( db: &dyn DefDatabase, item: ItemInNs, from: ModuleId, def_map: &DefMap, -) -> Vec<(ModuleId, Name)> { + mut cb: impl FnMut(&Name, ModuleId), +) { let _p = tracing::span!(tracing::Level::INFO, "find_local_import_locations").entered(); // `from` can import anything below `from` with visibility of at least `from`, and anything @@ -470,19 +468,17 @@ fn find_local_import_locations( // Compute the initial worklist. We start with all direct child modules of `from` as well as all // of its (recursive) parent modules. - let data = &def_map[from.local_id]; - let mut worklist = - data.children.values().map(|child| def_map.module_id(*child)).collect::>(); - // FIXME: do we need to traverse out of block expressions here? - for ancestor in iter::successors(from.containing_module(db), |m| m.containing_module(db)) { - worklist.push(ancestor); - } + let mut worklist = def_map[from.local_id] + .children + .values() + .map(|child| def_map.module_id(*child)) + // FIXME: do we need to traverse out of block expressions here? + .chain(iter::successors(from.containing_module(db), |m| m.containing_module(db))) + .collect::>(); + let mut seen: FxHashSet<_> = FxHashSet::default(); let def_map = def_map.crate_root().def_map(db); - let mut seen: FxHashSet<_> = FxHashSet::default(); - - let mut locations = Vec::new(); while let Some(module) = worklist.pop() { if !seen.insert(module) { continue; // already processed this module @@ -523,7 +519,7 @@ fn find_local_import_locations( // the item and we're a submodule of it, so can we. // Also this keeps the cached data smaller. if declared || is_pub_or_explicit { - locations.push((module, name.clone())); + cb(name, module); } } } @@ -535,8 +531,6 @@ fn find_local_import_locations( } } } - - locations } #[cfg(test)] From f1dbb958c8ef53900f77f6e55bd0aad1b2885454 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 3 Jun 2024 20:07:00 +0200 Subject: [PATCH 020/124] Add fuel to `find_path` --- crates/hir-def/src/find_path.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index e68f0d45cd..365003acef 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -1,6 +1,6 @@ //! An algorithm to find a path to refer to a certain item. -use std::{cmp::Ordering, iter}; +use std::{cell::Cell, cmp::Ordering, iter}; use hir_expand::{ name::{known, AsName, Name}, @@ -49,6 +49,7 @@ pub fn find_path( ignore_local_imports, from, from_def_map: &from.def_map(db), + fuel: Cell::new(FIND_PATH_FUEL), }, item, MAX_PATH_LEN, @@ -70,6 +71,7 @@ fn zip_stability(a: Stability, b: Stability) -> Stability { } const MAX_PATH_LEN: usize = 15; +const FIND_PATH_FUEL: usize = 10000; #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum PrefixKind { @@ -101,6 +103,7 @@ struct FindPathCtx<'db> { ignore_local_imports: bool, from: ModuleId, from_def_map: &'db DefMap, + fuel: Cell, } /// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId @@ -314,6 +317,17 @@ fn calculate_best_path( // the item's name itself. return None; } + let fuel = ctx.fuel.get(); + if fuel == 0 { + // we ran out of fuel, so we stop searching here + tracing::warn!( + "ran out of fuel while searching for a path for item {item:?} of krate {:?} from krate {:?}", + item.krate(ctx.db), + ctx.from.krate() + ); + return None; + } + ctx.fuel.set(fuel - 1); let mut best_path = None; let mut best_path_len = max_len; From 0110cfcae0e18627ab2d6fc8e60654d63991524b Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 4 Jun 2024 10:36:04 +0200 Subject: [PATCH 021/124] Recognize `__` prefixes for symbol search query --- crates/ide-db/src/symbol_index.rs | 15 +++------------ crates/ide/src/navigation_target.rs | 9 +++++---- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/crates/ide-db/src/symbol_index.rs b/crates/ide-db/src/symbol_index.rs index 365a727245..3539c7fbf8 100644 --- a/crates/ide-db/src/symbol_index.rs +++ b/crates/ide-db/src/symbol_index.rs @@ -53,7 +53,6 @@ pub struct Query { case_sensitive: bool, only_types: bool, libs: bool, - include_hidden: bool, } impl Query { @@ -67,14 +66,9 @@ impl Query { mode: SearchMode::Fuzzy, assoc_mode: AssocSearchMode::Include, case_sensitive: false, - include_hidden: false, } } - pub fn include_hidden(&mut self) { - self.include_hidden = true; - } - pub fn only_types(&mut self) { self.only_types = true; } @@ -363,6 +357,7 @@ impl Query { mut stream: fst::map::Union<'_>, mut cb: impl FnMut(&'sym FileSymbol), ) { + let ignore_underscore_prefixed = !self.query.starts_with("__"); while let Some((_, indexed_values)) = stream.next() { for &IndexedValue { index, value } in indexed_values { let symbol_index = &indices[index]; @@ -381,7 +376,8 @@ impl Query { if non_type_for_type_only_query || !self.matches_assoc_mode(symbol.is_assoc) { continue; } - if self.should_hide_query(symbol) { + // Hide symbols that start with `__` unless the query starts with `__` + if ignore_underscore_prefixed && symbol.name.starts_with("__") { continue; } if self.mode.check(&self.query, self.case_sensitive, &symbol.name) { @@ -392,11 +388,6 @@ impl Query { } } - fn should_hide_query(&self, symbol: &FileSymbol) -> bool { - // Hide symbols that start with `__` unless the query starts with `__` - !self.include_hidden && symbol.name.starts_with("__") && !self.query.starts_with("__") - } - fn matches_assoc_mode(&self, is_trait_assoc_item: bool) -> bool { !matches!( (is_trait_assoc_item, self.assoc_mode), diff --git a/crates/ide/src/navigation_target.rs b/crates/ide/src/navigation_target.rs index e40c7ecef0..a93a8da57e 100644 --- a/crates/ide/src/navigation_target.rs +++ b/crates/ide/src/navigation_target.rs @@ -940,11 +940,12 @@ static __FOO_CALLSITE: () = (); // It doesn't show the hidden symbol let navs = analysis.symbol_search(Query::new("foo".to_owned()), !0).unwrap(); assert_eq!(navs.len(), 2); + let navs = analysis.symbol_search(Query::new("_foo".to_owned()), !0).unwrap(); + assert_eq!(navs.len(), 0); - // Unless we configure a query to show hidden symbols - let mut query = Query::new("foo".to_owned()); - query.include_hidden(); + // Unless we explicitly search for a `__` prefix + let query = Query::new("__foo".to_owned()); let navs = analysis.symbol_search(query, !0).unwrap(); - assert_eq!(navs.len(), 3); + assert_eq!(navs.len(), 1); } } From 6f0207d594167be10584f8ade082b0975a53b303 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 4 Jun 2024 12:31:04 +0200 Subject: [PATCH 022/124] Cleanup some inert attribute stuff --- crates/hir-def/src/attr.rs | 57 +++++++++++++++++-- crates/hir-def/src/attr/tests.rs | 48 ---------------- crates/hir-def/src/data.rs | 2 +- crates/hir-def/src/db.rs | 10 ++-- crates/hir-def/src/nameres/attr_resolution.rs | 11 ++-- crates/hir-def/src/nameres/collector.rs | 52 +++++++---------- crates/hir-expand/src/builtin_attr_macro.rs | 2 - crates/hir-expand/src/cfg_process.rs | 4 +- crates/hir-expand/src/db.rs | 38 ++++++------- .../src/inert_attr_macro.rs} | 5 -- crates/hir-expand/src/lib.rs | 47 +++++++-------- crates/hir/src/lib.rs | 13 +++-- crates/hir/src/semantics.rs | 14 +---- .../src/handlers/macro_error.rs | 2 - crates/mbe/src/lib.rs | 6 ++ crates/salsa/src/lib.rs | 4 ++ crates/tt/src/lib.rs | 2 +- 17 files changed, 155 insertions(+), 162 deletions(-) delete mode 100644 crates/hir-def/src/attr/tests.rs rename crates/{hir-def/src/attr/builtin.rs => hir-expand/src/inert_attr_macro.rs} (99%) diff --git a/crates/hir-def/src/attr.rs b/crates/hir-def/src/attr.rs index d9eeffd798..369277633e 100644 --- a/crates/hir-def/src/attr.rs +++ b/crates/hir-def/src/attr.rs @@ -1,10 +1,5 @@ //! A higher level attributes based on TokenTree, with also some shortcuts. -pub mod builtin; - -#[cfg(test)] -mod tests; - use std::{borrow::Cow, hash::Hash, ops, slice::Iter as SliceIter}; use base_db::CrateId; @@ -646,3 +641,55 @@ pub(crate) fn fields_attrs_source_map( Arc::new(res) } + +#[cfg(test)] +mod tests { + //! This module contains tests for doc-expression parsing. + //! Currently, it tests `#[doc(hidden)]` and `#[doc(alias)]`. + + use triomphe::Arc; + + use base_db::FileId; + use hir_expand::span_map::{RealSpanMap, SpanMap}; + use mbe::{syntax_node_to_token_tree, DocCommentDesugarMode}; + use syntax::{ast, AstNode, TextRange}; + + use crate::attr::{DocAtom, DocExpr}; + + fn assert_parse_result(input: &str, expected: DocExpr) { + let source_file = ast::SourceFile::parse(input, span::Edition::CURRENT).ok().unwrap(); + let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap(); + let map = SpanMap::RealSpanMap(Arc::new(RealSpanMap::absolute(FileId::from_raw(0)))); + let tt = syntax_node_to_token_tree( + tt.syntax(), + map.as_ref(), + map.span_for_range(TextRange::empty(0.into())), + DocCommentDesugarMode::ProcMacro, + ); + let cfg = DocExpr::parse(&tt); + assert_eq!(cfg, expected); + } + + #[test] + fn test_doc_expr_parser() { + assert_parse_result("#![doc(hidden)]", DocAtom::Flag("hidden".into()).into()); + + assert_parse_result( + r#"#![doc(alias = "foo")]"#, + DocAtom::KeyValue { key: "alias".into(), value: "foo".into() }.into(), + ); + + assert_parse_result(r#"#![doc(alias("foo"))]"#, DocExpr::Alias(["foo".into()].into())); + assert_parse_result( + r#"#![doc(alias("foo", "bar", "baz"))]"#, + DocExpr::Alias(["foo".into(), "bar".into(), "baz".into()].into()), + ); + + assert_parse_result( + r#" + #[doc(alias("Bar", "Qux"))] + struct Foo;"#, + DocExpr::Alias(["Bar".into(), "Qux".into()].into()), + ); + } +} diff --git a/crates/hir-def/src/attr/tests.rs b/crates/hir-def/src/attr/tests.rs deleted file mode 100644 index 727f442980..0000000000 --- a/crates/hir-def/src/attr/tests.rs +++ /dev/null @@ -1,48 +0,0 @@ -//! This module contains tests for doc-expression parsing. -//! Currently, it tests `#[doc(hidden)]` and `#[doc(alias)]`. - -use triomphe::Arc; - -use base_db::FileId; -use hir_expand::span_map::{RealSpanMap, SpanMap}; -use mbe::{syntax_node_to_token_tree, DocCommentDesugarMode}; -use syntax::{ast, AstNode, TextRange}; - -use crate::attr::{DocAtom, DocExpr}; - -fn assert_parse_result(input: &str, expected: DocExpr) { - let source_file = ast::SourceFile::parse(input, span::Edition::CURRENT).ok().unwrap(); - let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap(); - let map = SpanMap::RealSpanMap(Arc::new(RealSpanMap::absolute(FileId::from_raw(0)))); - let tt = syntax_node_to_token_tree( - tt.syntax(), - map.as_ref(), - map.span_for_range(TextRange::empty(0.into())), - DocCommentDesugarMode::ProcMacro, - ); - let cfg = DocExpr::parse(&tt); - assert_eq!(cfg, expected); -} - -#[test] -fn test_doc_expr_parser() { - assert_parse_result("#![doc(hidden)]", DocAtom::Flag("hidden".into()).into()); - - assert_parse_result( - r#"#![doc(alias = "foo")]"#, - DocAtom::KeyValue { key: "alias".into(), value: "foo".into() }.into(), - ); - - assert_parse_result(r#"#![doc(alias("foo"))]"#, DocExpr::Alias(["foo".into()].into())); - assert_parse_result( - r#"#![doc(alias("foo", "bar", "baz"))]"#, - DocExpr::Alias(["foo".into(), "bar".into(), "baz".into()].into()), - ); - - assert_parse_result( - r#" - #[doc(alias("Bar", "Qux"))] - struct Foo;"#, - DocExpr::Alias(["Bar".into(), "Qux".into()].into()), - ); -} diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index 51a4dd6f42..0a7ca19910 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -642,7 +642,7 @@ impl<'a> AssocItemCollector<'a> { continue 'attrs; } let loc = self.db.lookup_intern_macro_call(call_id); - if let MacroDefKind::ProcMacro(exp, ..) = loc.def.kind { + if let MacroDefKind::ProcMacro(_, exp, _) = loc.def.kind { // If there's no expander for the proc macro (e.g. the // proc macro is ignored, or building the proc macro // crate failed), skip expansion like we would if it was diff --git a/crates/hir-def/src/db.rs b/crates/hir-def/src/db.rs index 55ecabdc38..61fed71218 100644 --- a/crates/hir-def/src/db.rs +++ b/crates/hir-def/src/db.rs @@ -294,10 +294,10 @@ fn macro_def(db: &dyn DefDatabase, id: MacroId) -> MacroDefId { let in_file = InFile::new(file_id, m); match expander { MacroExpander::Declarative => MacroDefKind::Declarative(in_file), - MacroExpander::BuiltIn(it) => MacroDefKind::BuiltIn(it, in_file), - MacroExpander::BuiltInAttr(it) => MacroDefKind::BuiltInAttr(it, in_file), - MacroExpander::BuiltInDerive(it) => MacroDefKind::BuiltInDerive(it, in_file), - MacroExpander::BuiltInEager(it) => MacroDefKind::BuiltInEager(it, in_file), + MacroExpander::BuiltIn(it) => MacroDefKind::BuiltIn(in_file, it), + MacroExpander::BuiltInAttr(it) => MacroDefKind::BuiltInAttr(in_file, it), + MacroExpander::BuiltInDerive(it) => MacroDefKind::BuiltInDerive(in_file, it), + MacroExpander::BuiltInEager(it) => MacroDefKind::BuiltInEager(in_file, it), } }; @@ -338,9 +338,9 @@ fn macro_def(db: &dyn DefDatabase, id: MacroId) -> MacroDefId { MacroDefId { krate: loc.container.krate, kind: MacroDefKind::ProcMacro( + InFile::new(loc.id.file_id(), makro.ast_id), loc.expander, loc.kind, - InFile::new(loc.id.file_id(), makro.ast_id), ), local_inner: false, allow_internal_unsafe: false, diff --git a/crates/hir-def/src/nameres/attr_resolution.rs b/crates/hir-def/src/nameres/attr_resolution.rs index 3cb0666edf..5829887c45 100644 --- a/crates/hir-def/src/nameres/attr_resolution.rs +++ b/crates/hir-def/src/nameres/attr_resolution.rs @@ -3,6 +3,7 @@ use base_db::CrateId; use hir_expand::{ attrs::{Attr, AttrId, AttrInput}, + inert_attr_macro::find_builtin_attr_idx, MacroCallId, MacroCallKind, MacroDefId, }; use span::SyntaxContextId; @@ -10,7 +11,6 @@ use syntax::{ast, SmolStr}; use triomphe::Arc; use crate::{ - attr::builtin::find_builtin_attr_idx, db::DefDatabase, item_scope::BuiltinShadowMode, nameres::path_resolution::ResolveMode, @@ -89,9 +89,12 @@ impl DefMap { } if segments.len() == 1 { - let mut registered = self.data.registered_attrs.iter().map(SmolStr::as_str); - let is_inert = find_builtin_attr_idx(&name).is_some() || registered.any(pred); - return is_inert; + if find_builtin_attr_idx(&name).is_some() { + return true; + } + if self.data.registered_attrs.iter().map(SmolStr::as_str).any(pred) { + return true; + } } } false diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 587997c473..ba8c6ba645 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -10,7 +10,7 @@ use cfg::{CfgExpr, CfgOptions}; use either::Either; use hir_expand::{ attrs::{Attr, AttrId}, - builtin_attr_macro::{find_builtin_attr, BuiltinAttrExpander}, + builtin_attr_macro::find_builtin_attr, builtin_derive_macro::find_builtin_derive, builtin_fn_macro::find_builtin_macro, name::{name, AsName, Name}, @@ -270,6 +270,7 @@ struct DefCollector<'a> { /// /// This also stores the attributes to skip when we resolve derive helpers and non-macro /// non-builtin attributes in general. + // FIXME: There has to be a better way to do this skip_attrs: FxHashMap, AttrId>, } @@ -1255,17 +1256,23 @@ impl DefCollector<'_> { _ => return Resolved::No, }; - let call_id = - attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def); - if let MacroDefId { - kind: - MacroDefKind::BuiltInAttr( - BuiltinAttrExpander::Derive | BuiltinAttrExpander::DeriveConst, - _, - ), - .. - } = def - { + // Skip #[test]/#[bench] expansion, which would merely result in more memory usage + // due to duplicating functions into macro expansions + if matches!( + def.kind, + MacroDefKind::BuiltInAttr(_, expander) + if expander.is_test() || expander.is_bench() + ) { + return recollect_without(self); + } + + let call_id = || { + attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def) + }; + if matches!(def, + MacroDefId { kind: MacroDefKind::BuiltInAttr(_, exp), .. } + if exp.is_derive() + ) { // Resolved to `#[derive]`, we don't actually expand this attribute like // normal (as that would just be an identity expansion with extra output) // Instead we treat derive attributes special and apply them separately. @@ -1290,6 +1297,7 @@ impl DefCollector<'_> { match attr.parse_path_comma_token_tree(self.db.upcast()) { Some(derive_macros) => { + let call_id = call_id(); let mut len = 0; for (idx, (path, call_site)) in derive_macros.enumerate() { let ast_id = AstIdWithPath::new(file_id, ast_id.value, path); @@ -1312,13 +1320,6 @@ impl DefCollector<'_> { // This is just a trick to be able to resolve the input to derives // as proper paths in `Semantics`. // Check the comment in [`builtin_attr_macro`]. - let call_id = attr_macro_as_call_id( - self.db, - file_ast_id, - attr, - self.def_map.krate, - def, - ); self.def_map.modules[directive.module_id] .scope .init_derive_attribute(ast_id, attr.id, call_id, len + 1); @@ -1336,17 +1337,8 @@ impl DefCollector<'_> { return recollect_without(self); } - // Skip #[test]/#[bench] expansion, which would merely result in more memory usage - // due to duplicating functions into macro expansions - if matches!( - def.kind, - MacroDefKind::BuiltInAttr(expander, _) - if expander.is_test() || expander.is_bench() - ) { - return recollect_without(self); - } - - if let MacroDefKind::ProcMacro(exp, ..) = def.kind { + let call_id = call_id(); + if let MacroDefKind::ProcMacro(_, exp, _) = def.kind { // If proc attribute macro expansion is disabled, skip expanding it here if !self.db.expand_proc_attr_macros() { self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro( diff --git a/crates/hir-expand/src/builtin_attr_macro.rs b/crates/hir-expand/src/builtin_attr_macro.rs index 9ff29b484d..2e115f4793 100644 --- a/crates/hir-expand/src/builtin_attr_macro.rs +++ b/crates/hir-expand/src/builtin_attr_macro.rs @@ -52,8 +52,6 @@ impl BuiltinAttrExpander { register_builtin! { (bench, Bench) => dummy_attr_expand, - (cfg, Cfg) => dummy_attr_expand, - (cfg_attr, CfgAttr) => dummy_attr_expand, (cfg_accessible, CfgAccessible) => dummy_attr_expand, (cfg_eval, CfgEval) => dummy_attr_expand, (derive, Derive) => derive_expand, diff --git a/crates/hir-expand/src/cfg_process.rs b/crates/hir-expand/src/cfg_process.rs index 9dd44262ba..55ae19068f 100644 --- a/crates/hir-expand/src/cfg_process.rs +++ b/crates/hir-expand/src/cfg_process.rs @@ -189,8 +189,8 @@ pub(crate) fn process_cfg_attrs( // FIXME: #[cfg_eval] is not implemented. But it is not stable yet let is_derive = match loc.def.kind { MacroDefKind::BuiltInDerive(..) - | MacroDefKind::ProcMacro(_, ProcMacroKind::CustomDerive, _) => true, - MacroDefKind::BuiltInAttr(expander, _) => expander.is_derive(), + | MacroDefKind::ProcMacro(_, _, ProcMacroKind::CustomDerive) => true, + MacroDefKind::BuiltInAttr(_, expander) => expander.is_derive(), _ => false, }; if !is_derive { diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index 12421bbe70..8b49a6877a 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -252,7 +252,7 @@ pub fn expand_speculative( // Otherwise the expand query will fetch the non speculative attribute args and pass those instead. let mut speculative_expansion = match loc.def.kind { - MacroDefKind::ProcMacro(expander, _, ast) => { + MacroDefKind::ProcMacro(ast, expander, _) => { let span = db.proc_macro_span(ast); tt.delimiter = tt::Delimiter::invisible_spanned(span); expander.expand( @@ -266,22 +266,22 @@ pub fn expand_speculative( span_with_mixed_site_ctxt(db, span, actual_macro_call), ) } - MacroDefKind::BuiltInAttr(BuiltinAttrExpander::Derive, _) => { + MacroDefKind::BuiltInAttr(_, it) if it.is_derive() => { pseudo_derive_attr_expansion(&tt, attr_arg.as_ref()?, span) } MacroDefKind::Declarative(it) => db .decl_macro_expander(loc.krate, it) .expand_unhygienic(db, tt, loc.def.krate, span, loc.def.edition), - MacroDefKind::BuiltIn(it, _) => { + MacroDefKind::BuiltIn(_, it) => { it.expand(db, actual_macro_call, &tt, span).map_err(Into::into) } - MacroDefKind::BuiltInDerive(it, ..) => { + MacroDefKind::BuiltInDerive(_, it) => { it.expand(db, actual_macro_call, &tt, span).map_err(Into::into) } - MacroDefKind::BuiltInEager(it, _) => { + MacroDefKind::BuiltInEager(_, it) => { it.expand(db, actual_macro_call, &tt, span).map_err(Into::into) } - MacroDefKind::BuiltInAttr(it, _) => it.expand(db, actual_macro_call, &tt, span), + MacroDefKind::BuiltInAttr(_, it) => it.expand(db, actual_macro_call, &tt, span), }; let expand_to = loc.expand_to(); @@ -493,7 +493,7 @@ fn macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult { .map_or_else(|| node.syntax().text_range(), |it| it.syntax().text_range()), ); // If derive attribute we need to censor the derive input - if matches!(loc.def.kind, MacroDefKind::BuiltInAttr(expander, ..) if expander.is_derive()) + if matches!(loc.def.kind, MacroDefKind::BuiltInAttr(_, expander) if expander.is_derive()) && ast::Adt::can_cast(node.syntax().kind()) { let adt = ast::Adt::cast(node.syntax().clone()).unwrap(); @@ -569,11 +569,11 @@ impl TokenExpander { MacroDefKind::Declarative(ast_id) => { TokenExpander::DeclarativeMacro(db.decl_macro_expander(id.krate, ast_id)) } - MacroDefKind::BuiltIn(expander, _) => TokenExpander::BuiltIn(expander), - MacroDefKind::BuiltInAttr(expander, _) => TokenExpander::BuiltInAttr(expander), - MacroDefKind::BuiltInDerive(expander, _) => TokenExpander::BuiltInDerive(expander), - MacroDefKind::BuiltInEager(expander, ..) => TokenExpander::BuiltInEager(expander), - MacroDefKind::ProcMacro(expander, ..) => TokenExpander::ProcMacro(expander), + MacroDefKind::BuiltIn(_, expander) => TokenExpander::BuiltIn(expander), + MacroDefKind::BuiltInAttr(_, expander) => TokenExpander::BuiltInAttr(expander), + MacroDefKind::BuiltInDerive(_, expander) => TokenExpander::BuiltInDerive(expander), + MacroDefKind::BuiltInEager(_, expander) => TokenExpander::BuiltInEager(expander), + MacroDefKind::ProcMacro(_, expander, _) => TokenExpander::ProcMacro(expander), } } } @@ -604,13 +604,13 @@ fn macro_expand( MacroDefKind::Declarative(id) => db .decl_macro_expander(loc.def.krate, id) .expand(db, arg.clone(), macro_call_id, span), - MacroDefKind::BuiltIn(it, _) => { + MacroDefKind::BuiltIn(_, it) => { it.expand(db, macro_call_id, arg, span).map_err(Into::into).zip_val(None) } - MacroDefKind::BuiltInDerive(it, _) => { + MacroDefKind::BuiltInDerive(_, it) => { it.expand(db, macro_call_id, arg, span).map_err(Into::into).zip_val(None) } - MacroDefKind::BuiltInEager(it, _) => { + MacroDefKind::BuiltInEager(_, it) => { // This might look a bit odd, but we do not expand the inputs to eager macros here. // Eager macros inputs are expanded, well, eagerly when we collect the macro calls. // That kind of expansion uses the ast id map of an eager macros input though which goes through @@ -634,12 +634,12 @@ fn macro_expand( } res.zip_val(None) } - MacroDefKind::BuiltInAttr(it, _) => { + MacroDefKind::BuiltInAttr(_, it) => { let mut res = it.expand(db, macro_call_id, arg, span); fixup::reverse_fixups(&mut res.value, &undo_info); res.zip_val(None) } - _ => unreachable!(), + MacroDefKind::ProcMacro(_, _, _) => unreachable!(), }; (ExpandResult { value: res.value, err: res.err }, span) } @@ -678,8 +678,8 @@ fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult (expander, ast), + let (ast, expander) = match loc.def.kind { + MacroDefKind::ProcMacro(ast, expander, _) => (ast, expander), _ => unreachable!(), }; diff --git a/crates/hir-def/src/attr/builtin.rs b/crates/hir-expand/src/inert_attr_macro.rs similarity index 99% rename from crates/hir-def/src/attr/builtin.rs rename to crates/hir-expand/src/inert_attr_macro.rs index 7b649496c4..35fd85bf45 100644 --- a/crates/hir-def/src/attr/builtin.rs +++ b/crates/hir-expand/src/inert_attr_macro.rs @@ -36,11 +36,6 @@ pub fn find_builtin_attr_idx(name: &str) -> Option { .copied() } -// impl AttributeTemplate { -// const DEFAULT: AttributeTemplate = -// AttributeTemplate { word: false, list: None, name_value_str: None }; -// } - /// A convenience macro for constructing attribute templates. /// E.g., `template!(Word, List: "description")` means that the attribute /// supports forms `#[attr]` and `#[attr(description)]`. diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index 83e92565f4..b34649d972 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -16,6 +16,7 @@ pub mod declarative; pub mod eager; pub mod files; pub mod hygiene; +pub mod inert_attr_macro; pub mod mod_path; pub mod name; pub mod proc_macro; @@ -186,11 +187,11 @@ pub struct MacroDefId { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum MacroDefKind { Declarative(AstId), - BuiltIn(BuiltinFnLikeExpander, AstId), - BuiltInAttr(BuiltinAttrExpander, AstId), - BuiltInDerive(BuiltinDeriveExpander, AstId), - BuiltInEager(EagerExpander, AstId), - ProcMacro(CustomProcMacroExpander, ProcMacroKind, AstId), + BuiltIn(AstId, BuiltinFnLikeExpander), + BuiltInAttr(AstId, BuiltinAttrExpander), + BuiltInDerive(AstId, BuiltinDeriveExpander), + BuiltInEager(AstId, EagerExpander), + ProcMacro(AstId, CustomProcMacroExpander, ProcMacroKind), } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -379,7 +380,7 @@ impl MacroFileIdExt for MacroFileId { fn is_custom_derive(&self, db: &dyn ExpandDatabase) -> bool { matches!( db.lookup_intern_macro_call(self.macro_call_id).def.kind, - MacroDefKind::ProcMacro(_, ProcMacroKind::CustomDerive, _) + MacroDefKind::ProcMacro(_, _, ProcMacroKind::CustomDerive) ) } @@ -440,13 +441,13 @@ impl MacroDefId { pub fn definition_range(&self, db: &dyn ExpandDatabase) -> InFile { match self.kind { MacroDefKind::Declarative(id) - | MacroDefKind::BuiltIn(_, id) - | MacroDefKind::BuiltInAttr(_, id) - | MacroDefKind::BuiltInDerive(_, id) - | MacroDefKind::BuiltInEager(_, id) => { + | MacroDefKind::BuiltIn(id, _) + | MacroDefKind::BuiltInAttr(id, _) + | MacroDefKind::BuiltInDerive(id, _) + | MacroDefKind::BuiltInEager(id, _) => { id.with_value(db.ast_id_map(id.file_id).get(id.value).text_range()) } - MacroDefKind::ProcMacro(_, _, id) => { + MacroDefKind::ProcMacro(id, _, _) => { id.with_value(db.ast_id_map(id.file_id).get(id.value).text_range()) } } @@ -454,12 +455,12 @@ impl MacroDefId { pub fn ast_id(&self) -> Either, AstId> { match self.kind { - MacroDefKind::ProcMacro(.., id) => Either::Right(id), + MacroDefKind::ProcMacro(id, ..) => Either::Right(id), MacroDefKind::Declarative(id) - | MacroDefKind::BuiltIn(_, id) - | MacroDefKind::BuiltInAttr(_, id) - | MacroDefKind::BuiltInDerive(_, id) - | MacroDefKind::BuiltInEager(_, id) => Either::Left(id), + | MacroDefKind::BuiltIn(id, _) + | MacroDefKind::BuiltInAttr(id, _) + | MacroDefKind::BuiltInDerive(id, _) + | MacroDefKind::BuiltInEager(id, _) => Either::Left(id), } } @@ -470,7 +471,7 @@ impl MacroDefId { pub fn is_attribute(&self) -> bool { matches!( self.kind, - MacroDefKind::BuiltInAttr(..) | MacroDefKind::ProcMacro(_, ProcMacroKind::Attr, _) + MacroDefKind::BuiltInAttr(..) | MacroDefKind::ProcMacro(_, _, ProcMacroKind::Attr) ) } @@ -478,7 +479,7 @@ impl MacroDefId { matches!( self.kind, MacroDefKind::BuiltInDerive(..) - | MacroDefKind::ProcMacro(_, ProcMacroKind::CustomDerive, _) + | MacroDefKind::ProcMacro(_, _, ProcMacroKind::CustomDerive) ) } @@ -486,26 +487,26 @@ impl MacroDefId { matches!( self.kind, MacroDefKind::BuiltIn(..) - | MacroDefKind::ProcMacro(_, ProcMacroKind::Bang, _) + | MacroDefKind::ProcMacro(_, _, ProcMacroKind::Bang) | MacroDefKind::BuiltInEager(..) | MacroDefKind::Declarative(..) ) } pub fn is_attribute_derive(&self) -> bool { - matches!(self.kind, MacroDefKind::BuiltInAttr(expander, ..) if expander.is_derive()) + matches!(self.kind, MacroDefKind::BuiltInAttr(_, expander) if expander.is_derive()) } pub fn is_include(&self) -> bool { - matches!(self.kind, MacroDefKind::BuiltInEager(expander, ..) if expander.is_include()) + matches!(self.kind, MacroDefKind::BuiltInEager(_, expander) if expander.is_include()) } pub fn is_include_like(&self) -> bool { - matches!(self.kind, MacroDefKind::BuiltInEager(expander, ..) if expander.is_include_like()) + matches!(self.kind, MacroDefKind::BuiltInEager(_, expander) if expander.is_include_like()) } pub fn is_env_or_option_env(&self) -> bool { - matches!(self.kind, MacroDefKind::BuiltInEager(expander, ..) if expander.is_env_or_option_env()) + matches!(self.kind, MacroDefKind::BuiltInEager(_, expander) if expander.is_env_or_option_env()) } } diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 8a91179cd4..79321df6b6 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -113,7 +113,7 @@ pub use hir_ty::method_resolution::TyFingerprint; pub use { cfg::{CfgAtom, CfgExpr, CfgOptions}, hir_def::{ - attr::{builtin::AttributeTemplate, AttrSourceMap, Attrs, AttrsWithOwner}, + attr::{AttrSourceMap, Attrs, AttrsWithOwner}, data::adt::StructKind, find_path::PrefixKind, import_map, @@ -132,6 +132,7 @@ pub use { attrs::{Attr, AttrId}, change::ChangeWithProcMacros, hygiene::{marks_rev, SyntaxContextExt}, + inert_attr_macro::AttributeTemplate, name::{known, Name}, proc_macro::ProcMacros, tt, ExpandResult, HirFileId, HirFileIdExt, InFile, InMacroFile, InRealFile, MacroFileId, @@ -3389,7 +3390,7 @@ impl BuiltinAttr { } fn builtin(name: &str) -> Option { - hir_def::attr::builtin::find_builtin_attr_idx(name) + hir_expand::inert_attr_macro::find_builtin_attr_idx(name) .map(|idx| BuiltinAttr { krate: None, idx: idx as u32 }) } @@ -3397,14 +3398,18 @@ impl BuiltinAttr { // FIXME: Return a `Name` here match self.krate { Some(krate) => db.crate_def_map(krate).registered_attrs()[self.idx as usize].clone(), - None => SmolStr::new(hir_def::attr::builtin::INERT_ATTRIBUTES[self.idx as usize].name), + None => { + SmolStr::new(hir_expand::inert_attr_macro::INERT_ATTRIBUTES[self.idx as usize].name) + } } } pub fn template(&self, _: &dyn HirDatabase) -> Option { match self.krate { Some(_) => None, - None => Some(hir_def::attr::builtin::INERT_ATTRIBUTES[self.idx as usize].template), + None => { + Some(hir_expand::inert_attr_macro::INERT_ATTRIBUTES[self.idx as usize].template) + } } } } diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 43de2a6ee7..0cde3f000a 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -1617,17 +1617,9 @@ fn macro_call_to_macro_id( macro_call_id: MacroCallId, ) -> Option { let loc = db.lookup_intern_macro_call(macro_call_id); - match loc.def.kind { - hir_expand::MacroDefKind::Declarative(it) - | hir_expand::MacroDefKind::BuiltIn(_, it) - | hir_expand::MacroDefKind::BuiltInAttr(_, it) - | hir_expand::MacroDefKind::BuiltInDerive(_, it) - | hir_expand::MacroDefKind::BuiltInEager(_, it) => { - ctx.macro_to_def(InFile::new(it.file_id, it.to_node(db))) - } - hir_expand::MacroDefKind::ProcMacro(_, _, it) => { - ctx.proc_macro_to_def(InFile::new(it.file_id, it.to_node(db))) - } + match loc.def.ast_id() { + Either::Left(it) => ctx.macro_to_def(InFile::new(it.file_id, it.to_node(db))), + Either::Right(it) => ctx.proc_macro_to_def(InFile::new(it.file_id, it.to_node(db))), } } diff --git a/crates/ide-diagnostics/src/handlers/macro_error.rs b/crates/ide-diagnostics/src/handlers/macro_error.rs index 6a957ac1c9..f8780fc0da 100644 --- a/crates/ide-diagnostics/src/handlers/macro_error.rs +++ b/crates/ide-diagnostics/src/handlers/macro_error.rs @@ -11,7 +11,6 @@ pub(crate) fn macro_error(ctx: &DiagnosticsContext<'_>, d: &hir::MacroError) -> d.message.clone(), display_range, ) - .experimental() } // Diagnostic: macro-error @@ -26,7 +25,6 @@ pub(crate) fn macro_def_error(ctx: &DiagnosticsContext<'_>, d: &hir::MacroDefErr d.message.clone(), display_range, ) - .experimental() } #[cfg(test)] diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs index 6920832dd2..ed3200964d 100644 --- a/crates/mbe/src/lib.rs +++ b/crates/mbe/src/lib.rs @@ -317,6 +317,12 @@ pub struct ValueResult { pub err: Option, } +impl Default for ValueResult { + fn default() -> Self { + Self { value: Default::default(), err: Default::default() } + } +} + impl ValueResult { pub fn new(value: T, err: E) -> Self { Self { value, err: Some(err) } diff --git a/crates/salsa/src/lib.rs b/crates/salsa/src/lib.rs index 5dde0d560f..9219a55634 100644 --- a/crates/salsa/src/lib.rs +++ b/crates/salsa/src/lib.rs @@ -513,6 +513,10 @@ where { self.storage.purge(); } + + pub fn storage(&self) -> &::Storage { + self.storage + } } /// Return value from [the `query_mut` method] on `Database`. diff --git a/crates/tt/src/lib.rs b/crates/tt/src/lib.rs index ab0efff651..e9de3f97b0 100644 --- a/crates/tt/src/lib.rs +++ b/crates/tt/src/lib.rs @@ -147,7 +147,7 @@ pub struct Punct { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum Spacing { Alone, - /// Whether the following token is joint to the current one. + /// Whether the following token is joint to this one. Joint, } From 97b58f28467c853b2eecca840906079877deb9ab Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 3 Apr 2024 15:23:56 +0200 Subject: [PATCH 023/124] Try caching macro calls more aggressively --- crates/hir-def/src/body.rs | 6 + crates/hir-def/src/body/lower.rs | 10 +- crates/hir-def/src/child_by_source.rs | 52 ++-- crates/hir-def/src/dyn_map/keys.rs | 13 +- crates/hir-expand/src/builtin_fn_macro.rs | 4 + crates/hir-expand/src/files.rs | 27 +- crates/hir-expand/src/lib.rs | 75 ++--- crates/hir/src/semantics.rs | 182 +++++++---- crates/hir/src/semantics/source_to_def.rs | 293 +++++++++++------- crates/hir/src/source_analyzer.rs | 18 +- .../src/integrated_benchmarks.rs | 2 +- 11 files changed, 382 insertions(+), 300 deletions(-) diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index d2f4d7b7e5..a796ef33c6 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -395,6 +395,12 @@ impl BodySourceMap { self.expr_map.get(&src).copied() } + pub fn expansions( + &self, + ) -> impl Iterator>, &MacroFileId)> { + self.expansions.iter() + } + pub fn implicit_format_args( &self, node: InFile<&ast::FormatArgsExpr>, diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index c6d9ba6cfe..5a5a8d302b 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -12,6 +12,7 @@ use intern::Interned; use rustc_hash::FxHashMap; use smallvec::SmallVec; use span::AstIdMap; +use stdx::never; use syntax::{ ast::{ self, ArrayExprKind, AstChildren, BlockExpr, HasArgList, HasAttrs, HasLoopBody, HasName, @@ -480,7 +481,8 @@ impl ExprCollector<'_> { } else if e.const_token().is_some() { Mutability::Shared } else { - unreachable!("parser only remaps to raw_token() if matching mutability token follows") + never!("parser only remaps to raw_token() if matching mutability token follows"); + Mutability::Shared } } else { Mutability::from_mutable(e.mut_token().is_some()) @@ -1006,9 +1008,9 @@ impl ExprCollector<'_> { Some((mark, expansion)) => { // Keep collecting even with expansion errors so we can provide completions and // other services in incomplete macro expressions. - self.source_map - .expansions - .insert(macro_call_ptr, self.expander.current_file_id().macro_file().unwrap()); + if let Some(macro_file) = self.expander.current_file_id().macro_file() { + self.source_map.expansions.insert(macro_call_ptr, macro_file); + } let prev_ast_id_map = mem::replace( &mut self.ast_id_map, self.db.ast_id_map(self.expander.current_file_id()), diff --git a/crates/hir-def/src/child_by_source.rs b/crates/hir-def/src/child_by_source.rs index 0b41984bdd..106109eb18 100644 --- a/crates/hir-def/src/child_by_source.rs +++ b/crates/hir-def/src/child_by_source.rs @@ -6,7 +6,7 @@ use either::Either; use hir_expand::{attrs::collect_attrs, HirFileId}; -use syntax::ast; +use syntax::{ast, AstPtr}; use crate::{ db::DefDatabase, @@ -38,7 +38,7 @@ impl ChildBySource for TraitId { data.attribute_calls().filter(|(ast_id, _)| ast_id.file_id == file_id).for_each( |(ast_id, call_id)| { - res[keys::ATTR_MACRO_CALL].insert(ast_id.to_node(db.upcast()), call_id); + res[keys::ATTR_MACRO_CALL].insert(ast_id.to_ptr(db.upcast()), call_id); }, ); data.items.iter().for_each(|&(_, item)| { @@ -50,9 +50,10 @@ impl ChildBySource for TraitId { impl ChildBySource for ImplId { fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap, file_id: HirFileId) { let data = db.impl_data(*self); + // FIXME: Macro calls data.attribute_calls().filter(|(ast_id, _)| ast_id.file_id == file_id).for_each( |(ast_id, call_id)| { - res[keys::ATTR_MACRO_CALL].insert(ast_id.to_node(db.upcast()), call_id); + res[keys::ATTR_MACRO_CALL].insert(ast_id.to_ptr(db.upcast()), call_id); }, ); data.items.iter().for_each(|&item| { @@ -80,7 +81,7 @@ impl ChildBySource for ItemScope { .for_each(|konst| insert_item_loc(db, res, file_id, konst, keys::CONST)); self.attr_macro_invocs().filter(|(id, _)| id.file_id == file_id).for_each( |(ast_id, call_id)| { - res[keys::ATTR_MACRO_CALL].insert(ast_id.to_node(db.upcast()), call_id); + res[keys::ATTR_MACRO_CALL].insert(ast_id.to_ptr(db.upcast()), call_id); }, ); self.legacy_macros().for_each(|(_, ids)| { @@ -88,7 +89,7 @@ impl ChildBySource for ItemScope { if let MacroId::MacroRulesId(id) = id { let loc = id.lookup(db); if loc.id.file_id() == file_id { - res[keys::MACRO_RULES].insert(loc.source(db).value, id); + res[keys::MACRO_RULES].insert(loc.ast_ptr(db).value, id); } } }) @@ -100,12 +101,18 @@ impl ChildBySource for ItemScope { if let Some((_, Either::Left(attr))) = collect_attrs(&adt).nth(attr_id.ast_index()) { - res[keys::DERIVE_MACRO_CALL].insert(attr, (attr_id, call_id, calls.into())); + res[keys::DERIVE_MACRO_CALL] + .insert(AstPtr::new(&attr), (attr_id, call_id, calls.into())); } }); }, ); - + self.iter_macro_invoc().filter(|(id, _)| id.file_id == file_id).for_each( + |(ast_id, &call)| { + let ast = ast_id.to_ptr(db.upcast()); + res[keys::MACRO_CALL].insert(ast, call); + }, + ); fn add_module_def( db: &dyn DefDatabase, map: &mut DynMap, @@ -155,8 +162,8 @@ impl ChildBySource for VariantId { for (local_id, source) in arena_map.value.iter() { let id = FieldId { parent, local_id }; match source.clone() { - Either::Left(source) => res[keys::TUPLE_FIELD].insert(source, id), - Either::Right(source) => res[keys::RECORD_FIELD].insert(source, id), + Either::Left(source) => res[keys::TUPLE_FIELD].insert(AstPtr::new(&source), id), + Either::Right(source) => res[keys::RECORD_FIELD].insert(AstPtr::new(&source), id), } } } @@ -171,29 +178,30 @@ impl ChildBySource for EnumId { let tree = loc.id.item_tree(db); let ast_id_map = db.ast_id_map(loc.id.file_id()); - let root = db.parse_or_expand(loc.id.file_id()); db.enum_data(*self).variants.iter().for_each(|&(variant, _)| { - res[keys::ENUM_VARIANT].insert( - ast_id_map.get(tree[variant.lookup(db).id.value].ast_id).to_node(&root), - variant, - ); + res[keys::ENUM_VARIANT] + .insert(ast_id_map.get(tree[variant.lookup(db).id.value].ast_id), variant); }); } } impl ChildBySource for DefWithBodyId { fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap, file_id: HirFileId) { - let body = db.body(*self); + let (body, sm) = db.body_with_source_map(*self); if let &DefWithBodyId::VariantId(v) = self { VariantId::EnumVariantId(v).child_by_source_to(db, res, file_id) } + sm.expansions().filter(|(ast, _)| ast.file_id == file_id).for_each(|(ast, &exp_id)| { + res[keys::MACRO_CALL].insert(ast.value, exp_id.macro_call_id); + }); + for (block, def_map) in body.blocks(db) { // All block expressions are merged into the same map, because they logically all add // inner items to the containing `DefWithBodyId`. def_map[DefMap::ROOT].scope.child_by_source_to(db, res, file_id); - res[keys::BLOCK].insert(block.lookup(db).ast_id.to_node(db.upcast()), block); + res[keys::BLOCK].insert(block.lookup(db).ast_id.to_ptr(db.upcast()), block); } } } @@ -220,13 +228,17 @@ impl ChildBySource for GenericDefId { { let id = TypeOrConstParamId { parent: *self, local_id }; match ast_param { - ast::TypeOrConstParam::Type(a) => res[keys::TYPE_PARAM].insert(a, id), - ast::TypeOrConstParam::Const(a) => res[keys::CONST_PARAM].insert(a, id), + ast::TypeOrConstParam::Type(a) => { + res[keys::TYPE_PARAM].insert(AstPtr::new(&a), id) + } + ast::TypeOrConstParam::Const(a) => { + res[keys::CONST_PARAM].insert(AstPtr::new(&a), id) + } } } for (local_id, ast_param) in lts_idx_iter.zip(generic_params_list.lifetime_params()) { let id = LifetimeParamId { parent: *self, local_id }; - res[keys::LIFETIME_PARAM].insert(ast_param, id); + res[keys::LIFETIME_PARAM].insert(AstPtr::new(&ast_param), id); } } } @@ -246,7 +258,7 @@ fn insert_item_loc( { let loc = id.lookup(db); if loc.item_tree_id().file_id() == file_id { - res[key].insert(loc.source(db).value, id) + res[key].insert(loc.ast_ptr(db).value, id) } } diff --git a/crates/hir-def/src/dyn_map/keys.rs b/crates/hir-def/src/dyn_map/keys.rs index f83ab1e1a0..9d330a7bf1 100644 --- a/crates/hir-def/src/dyn_map/keys.rs +++ b/crates/hir-def/src/dyn_map/keys.rs @@ -13,7 +13,7 @@ use crate::{ TraitId, TypeAliasId, TypeOrConstParamId, UnionId, UseId, }; -pub type Key = crate::dyn_map::Key>; +pub type Key = crate::dyn_map::Key, V, AstPtrPolicy>; pub const BLOCK: Key = Key::new(); pub const FUNCTION: Key = Key::new(); @@ -39,6 +39,7 @@ pub const LIFETIME_PARAM: Key = Key::new(); pub const MACRO_RULES: Key = Key::new(); pub const MACRO2: Key = Key::new(); pub const PROC_MACRO: Key = Key::new(); +pub const MACRO_CALL: Key = Key::new(); pub const ATTR_MACRO_CALL: Key = Key::new(); pub const DERIVE_MACRO_CALL: Key]>)> = Key::new(); @@ -54,18 +55,16 @@ pub struct AstPtrPolicy { } impl Policy for AstPtrPolicy { - type K = AST; + type K = AstPtr; type V = ID; - fn insert(map: &mut DynMap, key: AST, value: ID) { - let key = AstPtr::new(&key); + fn insert(map: &mut DynMap, key: AstPtr, value: ID) { map.map .entry::, ID>>() .or_insert_with(Default::default) .insert(key, value); } - fn get<'a>(map: &'a DynMap, key: &AST) -> Option<&'a ID> { - let key = AstPtr::new(key); - map.map.get::, ID>>()?.get(&key) + fn get<'a>(map: &'a DynMap, key: &AstPtr) -> Option<&'a ID> { + map.map.get::, ID>>()?.get(key) } fn is_empty(map: &DynMap) -> bool { map.map.get::, ID>>().map_or(true, |it| it.is_empty()) diff --git a/crates/hir-expand/src/builtin_fn_macro.rs b/crates/hir-expand/src/builtin_fn_macro.rs index ba96ab6cc2..02fd431e4e 100644 --- a/crates/hir-expand/src/builtin_fn_macro.rs +++ b/crates/hir-expand/src/builtin_fn_macro.rs @@ -67,6 +67,10 @@ impl BuiltinFnLikeExpander { let span = span_with_def_site_ctxt(db, span, id); self.expander()(db, id, tt, span) } + + pub fn is_asm(&self) -> bool { + matches!(self, Self::Asm | Self::GlobalAsm) + } } impl EagerExpander { diff --git a/crates/hir-expand/src/files.rs b/crates/hir-expand/src/files.rs index 1ba85c5c7e..743fac50f4 100644 --- a/crates/hir-expand/src/files.rs +++ b/crates/hir-expand/src/files.rs @@ -1,6 +1,4 @@ //! Things to wrap other things in file ids. -use std::iter; - use either::Either; use span::{ AstIdNode, ErasedFileAstId, FileAstId, FileId, FileRange, HirFileId, HirFileIdRepr, @@ -150,27 +148,16 @@ impl InFileWrapper { } } +impl InFileWrapper { + // unfortunately `syntax` collides with the impl above, because `&_` is fundamental + pub fn syntax_ref(&self) -> InFileWrapper { + self.with_value(self.value.syntax()) + } +} + // region:specific impls impl InFile<&SyntaxNode> { - /// Traverse up macro calls and skips the macro invocation node - pub fn ancestors_with_macros( - self, - db: &dyn db::ExpandDatabase, - ) -> impl Iterator> + '_ { - let succ = move |node: &InFile| match node.value.parent() { - Some(parent) => Some(node.with_value(parent)), - None => db - .lookup_intern_macro_call(node.file_id.macro_file()?.macro_call_id) - .to_node_item(db) - .syntax() - .cloned() - .map(|node| node.parent()) - .transpose(), - }; - iter::successors(succ(&self.cloned()), succ) - } - /// Falls back to the macro call range if the node cannot be mapped up fully. /// /// For attributes and derives, this will point back to the attribute only. diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index b34649d972..131625a96a 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -47,7 +47,7 @@ use crate::{ builtin_attr_macro::BuiltinAttrExpander, builtin_derive_macro::BuiltinDeriveExpander, builtin_fn_macro::{BuiltinFnLikeExpander, EagerExpander}, - db::{ExpandDatabase, TokenExpander}, + db::ExpandDatabase, mod_path::ModPath, proc_macro::{CustomProcMacroExpander, ProcMacroKind}, span_map::{ExpansionSpanMap, SpanMap}, @@ -253,9 +253,6 @@ pub trait HirFileIdExt { /// If this is a macro call, returns the syntax node of the very first macro call this file resides in. fn original_call_node(self, db: &dyn ExpandDatabase) -> Option>; - /// Return expansion information if it is a macro-expansion file - fn expansion_info(self, db: &dyn ExpandDatabase) -> Option; - fn as_builtin_derive_attr_node(&self, db: &dyn ExpandDatabase) -> Option>; } @@ -309,11 +306,6 @@ impl HirFileIdExt for HirFileId { } } - /// Return expansion information if it is a macro-expansion file - fn expansion_info(self, db: &dyn ExpandDatabase) -> Option { - Some(ExpansionInfo::new(db, self.macro_file()?)) - } - fn as_builtin_derive_attr_node(&self, db: &dyn ExpandDatabase) -> Option> { let macro_file = self.macro_file()?; let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id); @@ -417,8 +409,10 @@ impl MacroFileIdExt for MacroFileId { } fn is_attr_macro(&self, db: &dyn ExpandDatabase) -> bool { - let loc = db.lookup_intern_macro_call(self.macro_call_id); - matches!(loc.kind, MacroCallKind::Attr { .. }) + matches!( + db.lookup_intern_macro_call(self.macro_call_id).def.kind, + MacroDefKind::BuiltInAttr(..) | MacroDefKind::ProcMacro(_, ProcMacroKind::Attr, _) + ) } fn is_derive_attr_pseudo_expansion(&self, db: &dyn ExpandDatabase) -> bool { @@ -703,16 +697,12 @@ impl MacroCallKind { // simpler function calls if the map is only used once #[derive(Clone, Debug, PartialEq, Eq)] pub struct ExpansionInfo { - pub expanded: InMacroFile, + expanded: InMacroFile, /// The argument TokenTree or item for attributes arg: InFile>, - /// The `macro_rules!` or attribute input. - attr_input_or_mac_def: Option>, - - macro_def: TokenExpander, - macro_arg: Arc, - pub exp_map: Arc, + exp_map: Arc, arg_map: SpanMap, + loc: MacroCallLoc, } impl ExpansionInfo { @@ -720,14 +710,21 @@ impl ExpansionInfo { self.expanded.clone() } - pub fn call_node(&self) -> Option> { - Some(self.arg.with_value(self.arg.value.as_ref()?.parent()?)) + pub fn call_node(&self) -> InFile> { + self.arg.with_value(self.arg.value.as_ref().and_then(SyntaxNode::parent)) } pub fn call_file(&self) -> HirFileId { self.arg.file_id } + pub fn is_attr(&self) -> bool { + matches!( + self.loc.def.kind, + MacroDefKind::BuiltInAttr(..) | MacroDefKind::ProcMacro(_, ProcMacroKind::Attr, _) + ) + } + /// Maps the passed in file range down into a macro expansion if it is the input to a macro call. /// /// Note this does a linear search through the entire backing vector of the spanmap. @@ -812,49 +809,15 @@ impl ExpansionInfo { } pub fn new(db: &dyn ExpandDatabase, macro_file: MacroFileId) -> ExpansionInfo { - let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id); + let loc = db.lookup_intern_macro_call(macro_file.macro_call_id); let arg_tt = loc.kind.arg(db); let arg_map = db.span_map(arg_tt.file_id); - let macro_def = db.macro_expander(loc.def); let (parse, exp_map) = db.parse_macro_expansion(macro_file).value; let expanded = InMacroFile { file_id: macro_file, value: parse.syntax_node() }; - let (macro_arg, _, _) = - db.macro_arg_considering_derives(macro_file.macro_call_id, &loc.kind); - - let def = loc.def.ast_id().left().and_then(|id| { - let def_tt = match id.to_node(db) { - ast::Macro::MacroRules(mac) => mac.token_tree()?, - ast::Macro::MacroDef(_) if matches!(macro_def, TokenExpander::BuiltInAttr(_)) => { - return None - } - ast::Macro::MacroDef(mac) => mac.body()?, - }; - Some(InFile::new(id.file_id, def_tt)) - }); - let attr_input_or_mac_def = def.or_else(|| match loc.kind { - MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => { - // FIXME: handle `cfg_attr` - let tt = collect_attrs(&ast_id.to_node(db)) - .nth(invoc_attr_index.ast_index()) - .and_then(|x| Either::left(x.1))? - .token_tree()?; - Some(InFile::new(ast_id.file_id, tt)) - } - _ => None, - }); - - ExpansionInfo { - expanded, - arg: arg_tt, - attr_input_or_mac_def, - macro_arg, - macro_def, - exp_map, - arg_map, - } + ExpansionInfo { expanded, loc, arg: arg_tt, exp_map, arg_map } } } diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 0cde3f000a..1eab509b7b 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -19,8 +19,8 @@ use hir_def::{ AsMacroCall, DefWithBodyId, FunctionId, MacroId, TraitId, VariantId, }; use hir_expand::{ - attrs::collect_attrs, db::ExpandDatabase, files::InRealFile, name::AsName, ExpansionInfo, - InMacroFile, MacroCallId, MacroFileId, MacroFileIdExt, + attrs::collect_attrs, db::ExpandDatabase, files::InRealFile, name::AsName, InMacroFile, + MacroCallId, MacroFileId, MacroFileIdExt, }; use itertools::Itertools; use rustc_hash::{FxHashMap, FxHashSet}; @@ -129,12 +129,9 @@ pub struct Semantics<'db, DB> { pub struct SemanticsImpl<'db> { pub db: &'db dyn HirDatabase, - s2d_cache: RefCell, + s2d_cache: RefCell<(SourceToDefCache, FxHashMap)>, /// Rootnode to HirFileId cache root_to_file_cache: RefCell>, - // These 2 caches are mainly useful for semantic highlighting as nothing else descends a lot of tokens - // So we might wanna move them out into something specific for semantic highlighting - expansion_info_cache: RefCell>, /// MacroCall to its expansion's MacroFileId cache macro_call_cache: RefCell, MacroFileId>>, } @@ -295,7 +292,6 @@ impl<'db> SemanticsImpl<'db> { db, s2d_cache: Default::default(), root_to_file_cache: Default::default(), - expansion_info_cache: Default::default(), macro_call_cache: Default::default(), } } @@ -314,7 +310,16 @@ impl<'db> SemanticsImpl<'db> { pub fn expand(&self, macro_call: &ast::MacroCall) -> Option { let sa = self.analyze_no_infer(macro_call.syntax())?; - let file_id = sa.expand(self.db, InFile::new(sa.file_id, macro_call))?; + + let macro_call = InFile::new(sa.file_id, macro_call); + let file_id = if let Some(call) = + ::to_def(self, macro_call) + { + call.as_macro_file() + } else { + sa.expand(self.db, macro_call)? + }; + let node = self.parse_or_expand(file_id.into()); Some(node) } @@ -322,7 +327,7 @@ impl<'db> SemanticsImpl<'db> { /// If `item` has an attribute macro attached to it, expands it. pub fn expand_attr_macro(&self, item: &ast::Item) -> Option { let src = self.wrap_node_infile(item.clone()); - let macro_call_id = self.with_ctx(|ctx| ctx.item_to_macro_call(src))?; + let macro_call_id = self.with_ctx(|ctx| ctx.item_to_macro_call(src.as_ref()))?; Some(self.parse_or_expand(macro_call_id.as_file())) } @@ -341,9 +346,7 @@ impl<'db> SemanticsImpl<'db> { Some( calls .into_iter() - .map(|call| { - macro_call_to_macro_id(ctx, self.db.upcast(), call?).map(|id| Macro { id }) - }) + .map(|call| macro_call_to_macro_id(ctx, call?).map(|id| Macro { id })) .collect(), ) }) @@ -403,7 +406,7 @@ impl<'db> SemanticsImpl<'db> { pub fn is_attr_macro_call(&self, item: &ast::Item) -> bool { let file_id = self.find_file(item.syntax()).file_id; - let src = InFile::new(file_id, item.clone()); + let src = InFile::new(file_id, item); self.with_ctx(|ctx| ctx.item_to_macro_call(src).is_some()) } @@ -453,7 +456,7 @@ impl<'db> SemanticsImpl<'db> { token_to_map: SyntaxToken, ) -> Option<(SyntaxNode, SyntaxToken)> { let macro_call = self.wrap_node_infile(actual_macro_call.clone()); - let macro_call_id = self.with_ctx(|ctx| ctx.item_to_macro_call(macro_call))?; + let macro_call_id = self.with_ctx(|ctx| ctx.item_to_macro_call(macro_call.as_ref()))?; hir_expand::db::expand_speculative( self.db.upcast(), macro_call_id, @@ -705,8 +708,6 @@ impl<'db> SemanticsImpl<'db> { let parent = token.parent()?; let file_id = self.find_file(&parent).file_id.file_id()?; - let mut cache = self.expansion_info_cache.borrow_mut(); - // iterate related crates and find all include! invocations that include_file_id matches for (invoc, _) in self .db @@ -716,18 +717,31 @@ impl<'db> SemanticsImpl<'db> { .filter(|&(_, include_file_id)| include_file_id == file_id) { let macro_file = invoc.as_macro_file(); - let expansion_info = cache.entry(macro_file).or_insert_with(|| { - let exp_info = macro_file.expansion_info(self.db.upcast()); + let expansion_info = { + self.with_ctx(|ctx| { + ctx.expansion_info_cache + .entry(macro_file) + .or_insert_with(|| { + let exp_info = macro_file.expansion_info(self.db.upcast()); - let InMacroFile { file_id, value } = exp_info.expanded(); - self.cache(value, file_id.into()); + let InMacroFile { file_id, value } = exp_info.expanded(); + if let InFile { file_id, value: Some(value) } = exp_info.call_node() { + self.cache(value.ancestors().last().unwrap(), file_id); + } + self.cache(value, file_id.into()); - exp_info - }); + exp_info + }) + .clone() + }) + }; // FIXME: uncached parse // Create the source analyzer for the macro call scope - let Some(sa) = self.analyze_no_infer(&self.parse_or_expand(expansion_info.call_file())) + let Some(sa) = expansion_info + .call_node() + .value + .and_then(|it| self.analyze_no_infer(&it.ancestors().last().unwrap())) else { continue; }; @@ -785,23 +799,27 @@ impl<'db> SemanticsImpl<'db> { } }; - let mut cache = self.expansion_info_cache.borrow_mut(); - let mut mcache = self.macro_call_cache.borrow_mut(); + let mut m_cache = self.macro_call_cache.borrow_mut(); let def_map = sa.resolver.def_map(); let mut stack: Vec<(_, SmallVec<[_; 2]>)> = vec![(file_id, smallvec![token])]; - let mut process_expansion_for_token = |stack: &mut Vec<_>, macro_file| { - let exp_info = cache.entry(macro_file).or_insert_with(|| { - let exp_info = macro_file.expansion_info(self.db.upcast()); + let process_expansion_for_token = |stack: &mut Vec<_>, macro_file| { + let InMacroFile { file_id, value: mapped_tokens } = self.with_ctx(|ctx| { + Some( + ctx.expansion_info_cache + .entry(macro_file) + .or_insert_with(|| { + let exp_info = macro_file.expansion_info(self.db.upcast()); - let InMacroFile { file_id, value } = exp_info.expanded(); - self.cache(value, file_id.into()); + let InMacroFile { file_id, value } = exp_info.expanded(); + self.cache(value, file_id.into()); - exp_info - }); - - let InMacroFile { file_id, value: mapped_tokens } = exp_info.map_range_down(span)?; - let mapped_tokens: SmallVec<[_; 2]> = mapped_tokens.collect(); + exp_info + }) + .map_range_down(span)? + .map(SmallVec::<[_; 2]>::from_iter), + ) + })?; // we have found a mapping for the token if the vec is non-empty let res = mapped_tokens.is_empty().not().then_some(()); @@ -818,10 +836,7 @@ impl<'db> SemanticsImpl<'db> { token.parent_ancestors().filter_map(ast::Item::cast).find_map(|item| { // Don't force populate the dyn cache for items that don't have an attribute anyways item.attrs().next()?; - Some(( - ctx.item_to_macro_call(InFile::new(file_id, item.clone()))?, - item, - )) + Some((ctx.item_to_macro_call(InFile::new(file_id, &item))?, item)) }) }); if let Some((call_id, item)) = containing_attribute_macro_call { @@ -874,13 +889,20 @@ impl<'db> SemanticsImpl<'db> { return None; } let macro_call = tt.syntax().parent().and_then(ast::MacroCall::cast)?; - let mcall: hir_expand::files::InFileWrapper = - InFile::new(file_id, macro_call); - let file_id = match mcache.get(&mcall) { + let mcall = InFile::new(file_id, macro_call); + let file_id = match m_cache.get(&mcall) { Some(&it) => it, None => { - let it = sa.expand(self.db, mcall.as_ref())?; - mcache.insert(mcall, it); + let it = if let Some(call) = + ::to_def( + self, + mcall.as_ref(), + ) { + call.as_macro_file() + } else { + sa.expand(self.db, mcall.as_ref())? + }; + m_cache.insert(mcall, it); it } }; @@ -1056,16 +1078,19 @@ impl<'db> SemanticsImpl<'db> { node: SyntaxNode, ) -> impl Iterator + Clone + '_ { let node = self.find_file(&node); - let db = self.db.upcast(); iter::successors(Some(node.cloned()), move |&InFile { file_id, ref value }| { match value.parent() { Some(parent) => Some(InFile::new(file_id, parent)), None => { - let call_node = file_id.macro_file()?.call_node(db); - // cache the node - // FIXME: uncached parse - self.parse_or_expand(call_node.file_id); - Some(call_node) + let macro_file = file_id.macro_file()?; + + self.with_ctx(|ctx| { + let expansion_info = ctx + .expansion_info_cache + .entry(macro_file) + .or_insert_with(|| macro_file.expansion_info(self.db.upcast())); + expansion_info.call_node().transpose() + }) } } }) @@ -1090,7 +1115,7 @@ impl<'db> SemanticsImpl<'db> { .find(|tp| tp.lifetime().as_ref().map(|lt| lt.text()).as_ref() == Some(&text)) })?; let src = self.wrap_node_infile(lifetime_param); - ToDef::to_def(self, src) + ToDef::to_def(self, src.as_ref()) } pub fn resolve_label(&self, lifetime: &ast::Lifetime) -> Option