From 30b992e95a1e437b3e96b0e86373427f0fe2b121 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 14 Jan 2024 08:48:41 +0100 Subject: [PATCH 1/2] Deduplicate references to macro argument Commit 6a06f6f72 (Deduplicate reference search results, 2022-11-07) deduplicates references within each definition. There is an edge case when requesting references of a macro argument. Apparently, our descend_into_macros() stanza in references.rs produces a cartesian product of - references inside the macro times - times references outside the macro. Since the above deduplication only applies to the references within a single definition, we return them all, leading to many redundant references. Work around this by deduplicating definitions as well. Perhaps there is a better fix to not produce this cartesian product in the first place; but I think at least for definitions the problem would remain; a macro can contain multiple definitions of the same name, but since the navigation target will be the unresolved location, it's the same for all of them. We can't use unique() because we don't want to drop references that don't have a declaration (though I dont' have an example for this case). I discovered this working with the "bitflags" macro from the crate of the same name. Fixes #16357 --- crates/rust-analyzer/src/handlers/request.rs | 28 ++++++++++++++++---- crates/stdx/src/lib.rs | 16 +++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index eb9d4bf0f02d..a677cea31b50 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -2,6 +2,7 @@ //! Protocol. This module specifically handles requests. use std::{ + collections::HashSet, fs, io::Write as _, path::PathBuf, @@ -13,7 +14,8 @@ use anyhow::Context; use ide::{ AnnotationConfig, AssistKind, AssistResolveStrategy, Cancellable, FilePosition, FileRange, HoverAction, HoverGotoTypeData, InlayFieldsToResolve, Query, RangeInfo, RangeLimit, - ReferenceCategory, Runnable, RunnableKind, SingleResolve, SourceChange, TextEdit, + ReferenceCategory, ReferenceSearchResult, Runnable, RunnableKind, SingleResolve, SourceChange, + TextEdit, }; use ide_db::SymbolKind; use lsp_server::ErrorCode; @@ -28,6 +30,8 @@ use lsp_types::{ }; use project_model::{ManifestPath, ProjectWorkspace, TargetKind}; use serde_json::json; +#[allow(unused_imports)] +use stdx::IsNoneOr; use stdx::{format_to, never}; use syntax::{algo, ast, AstNode, TextRange, TextSize}; use triomphe::Arc; @@ -1055,10 +1059,10 @@ pub(crate) fn handle_references( let exclude_imports = snap.config.find_all_refs_exclude_imports(); let exclude_tests = snap.config.find_all_refs_exclude_tests(); - let refs = match snap.analysis.find_all_refs(position, None)? { - None => return Ok(None), - Some(refs) => refs, + let Some(mut refs) = snap.analysis.find_all_refs(position, None)? else { + return Ok(None); }; + deduplicate_declarations(&mut refs); let include_declaration = params.context.include_declaration; let locations = refs @@ -1090,6 +1094,17 @@ pub(crate) fn handle_references( Ok(Some(locations)) } +fn deduplicate_declarations(refs: &mut Vec) { + if refs.iter().filter(|decl| decl.declaration.is_some()).take(2).count() > 1 { + let mut seen_navigation_targets = HashSet::new(); + refs.retain(|res| { + res.declaration + .as_ref() + .is_none_or(|decl| seen_navigation_targets.insert(decl.nav.clone())) + }); + } +} + pub(crate) fn handle_formatting( snap: GlobalStateSnapshot, params: lsp_types::DocumentFormattingParams, @@ -1794,7 +1809,10 @@ fn show_ref_command_link( position: &FilePosition, ) -> Option { if snap.config.hover_actions().references && snap.config.client_commands().show_reference { - if let Some(ref_search_res) = snap.analysis.find_all_refs(*position, None).unwrap_or(None) { + if let Some(mut ref_search_res) = + snap.analysis.find_all_refs(*position, None).unwrap_or(None) + { + deduplicate_declarations(&mut ref_search_res); let uri = to_proto::url(snap, position.file_id); let line_index = snap.file_line_index(position.file_id).ok()?; let position = to_proto::position(&line_index, position.offset); diff --git a/crates/stdx/src/lib.rs b/crates/stdx/src/lib.rs index 9a9ebae74e8c..0504ca50b882 100644 --- a/crates/stdx/src/lib.rs +++ b/crates/stdx/src/lib.rs @@ -302,6 +302,22 @@ pub fn slice_tails(this: &[T]) -> impl Iterator { (0..this.len()).map(|i| &this[i..]) } +pub trait IsNoneOr { + type Type; + #[allow(clippy::wrong_self_convention)] + fn is_none_or(self, s: impl FnOnce(Self::Type) -> bool) -> bool; +} +#[allow(unstable_name_collisions)] +impl IsNoneOr for Option { + type Type = T; + fn is_none_or(self, f: impl FnOnce(T) -> bool) -> bool { + match self { + Some(v) => f(v), + None => true, + } + } +} + #[cfg(test)] mod tests { use super::*; From 91a8f34aeed075427ad4f6c0c6f58f247ac7de42 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 19 Feb 2024 12:22:27 +0100 Subject: [PATCH 2/2] Deduplicate lsp locations --- crates/rust-analyzer/src/handlers/request.rs | 33 +++++--------------- crates/rust-analyzer/src/lsp/to_proto.rs | 7 +++-- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index a677cea31b50..04a043954299 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -2,7 +2,6 @@ //! Protocol. This module specifically handles requests. use std::{ - collections::HashSet, fs, io::Write as _, path::PathBuf, @@ -14,10 +13,10 @@ use anyhow::Context; use ide::{ AnnotationConfig, AssistKind, AssistResolveStrategy, Cancellable, FilePosition, FileRange, HoverAction, HoverGotoTypeData, InlayFieldsToResolve, Query, RangeInfo, RangeLimit, - ReferenceCategory, ReferenceSearchResult, Runnable, RunnableKind, SingleResolve, SourceChange, - TextEdit, + ReferenceCategory, Runnable, RunnableKind, SingleResolve, SourceChange, TextEdit, }; use ide_db::SymbolKind; +use itertools::Itertools; use lsp_server::ErrorCode; use lsp_types::{ CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem, @@ -30,8 +29,6 @@ use lsp_types::{ }; use project_model::{ManifestPath, ProjectWorkspace, TargetKind}; use serde_json::json; -#[allow(unused_imports)] -use stdx::IsNoneOr; use stdx::{format_to, never}; use syntax::{algo, ast, AstNode, TextRange, TextSize}; use triomphe::Arc; @@ -1059,10 +1056,9 @@ pub(crate) fn handle_references( let exclude_imports = snap.config.find_all_refs_exclude_imports(); let exclude_tests = snap.config.find_all_refs_exclude_tests(); - let Some(mut refs) = snap.analysis.find_all_refs(position, None)? else { + let Some(refs) = snap.analysis.find_all_refs(position, None)? else { return Ok(None); }; - deduplicate_declarations(&mut refs); let include_declaration = params.context.include_declaration; let locations = refs @@ -1088,23 +1084,13 @@ pub(crate) fn handle_references( }) .chain(decl) }) + .unique() .filter_map(|frange| to_proto::location(&snap, frange).ok()) .collect(); Ok(Some(locations)) } -fn deduplicate_declarations(refs: &mut Vec) { - if refs.iter().filter(|decl| decl.declaration.is_some()).take(2).count() > 1 { - let mut seen_navigation_targets = HashSet::new(); - refs.retain(|res| { - res.declaration - .as_ref() - .is_none_or(|decl| seen_navigation_targets.insert(decl.nav.clone())) - }); - } -} - pub(crate) fn handle_formatting( snap: GlobalStateSnapshot, params: lsp_types::DocumentFormattingParams, @@ -1809,10 +1795,7 @@ fn show_ref_command_link( position: &FilePosition, ) -> Option { if snap.config.hover_actions().references && snap.config.client_commands().show_reference { - if let Some(mut ref_search_res) = - snap.analysis.find_all_refs(*position, None).unwrap_or(None) - { - deduplicate_declarations(&mut ref_search_res); + if let Some(ref_search_res) = snap.analysis.find_all_refs(*position, None).unwrap_or(None) { let uri = to_proto::url(snap, position.file_id); let line_index = snap.file_line_index(position.file_id).ok()?; let position = to_proto::position(&line_index, position.offset); @@ -1820,10 +1803,10 @@ fn show_ref_command_link( .into_iter() .flat_map(|res| res.references) .flat_map(|(file_id, ranges)| { - ranges.into_iter().filter_map(move |(range, _)| { - to_proto::location(snap, FileRange { file_id, range }).ok() - }) + ranges.into_iter().map(move |(range, _)| FileRange { file_id, range }) }) + .unique() + .filter_map(|range| to_proto::location(snap, range).ok()) .collect(); let title = to_proto::reference_title(locations.len()); let command = to_proto::command::show_references(title, &uri, position, locations); diff --git a/crates/rust-analyzer/src/lsp/to_proto.rs b/crates/rust-analyzer/src/lsp/to_proto.rs index 727007bba083..4101d476cd30 100644 --- a/crates/rust-analyzer/src/lsp/to_proto.rs +++ b/crates/rust-analyzer/src/lsp/to_proto.rs @@ -904,15 +904,16 @@ pub(crate) fn goto_definition_response( if snap.config.location_link() { let links = targets .into_iter() + .unique_by(|nav| (nav.file_id, nav.full_range, nav.focus_range)) .map(|nav| location_link(snap, src, nav)) .collect::>>()?; Ok(links.into()) } else { let locations = targets .into_iter() - .map(|nav| { - location(snap, FileRange { file_id: nav.file_id, range: nav.focus_or_full_range() }) - }) + .map(|nav| FileRange { file_id: nav.file_id, range: nav.focus_or_full_range() }) + .unique() + .map(|range| location(snap, range)) .collect::>>()?; Ok(locations.into()) }