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
This commit is contained in:
Johannes Altmanninger 2024-01-14 08:48:41 +01:00 committed by Lukas Wirth
parent 60982dc8fc
commit 30b992e95a
2 changed files with 39 additions and 5 deletions

View file

@ -2,6 +2,7 @@
//! Protocol. This module specifically handles requests. //! Protocol. This module specifically handles requests.
use std::{ use std::{
collections::HashSet,
fs, fs,
io::Write as _, io::Write as _,
path::PathBuf, path::PathBuf,
@ -13,7 +14,8 @@ use anyhow::Context;
use ide::{ use ide::{
AnnotationConfig, AssistKind, AssistResolveStrategy, Cancellable, FilePosition, FileRange, AnnotationConfig, AssistKind, AssistResolveStrategy, Cancellable, FilePosition, FileRange,
HoverAction, HoverGotoTypeData, InlayFieldsToResolve, Query, RangeInfo, RangeLimit, HoverAction, HoverGotoTypeData, InlayFieldsToResolve, Query, RangeInfo, RangeLimit,
ReferenceCategory, Runnable, RunnableKind, SingleResolve, SourceChange, TextEdit, ReferenceCategory, ReferenceSearchResult, Runnable, RunnableKind, SingleResolve, SourceChange,
TextEdit,
}; };
use ide_db::SymbolKind; use ide_db::SymbolKind;
use lsp_server::ErrorCode; use lsp_server::ErrorCode;
@ -28,6 +30,8 @@ use lsp_types::{
}; };
use project_model::{ManifestPath, ProjectWorkspace, TargetKind}; use project_model::{ManifestPath, ProjectWorkspace, TargetKind};
use serde_json::json; use serde_json::json;
#[allow(unused_imports)]
use stdx::IsNoneOr;
use stdx::{format_to, never}; use stdx::{format_to, never};
use syntax::{algo, ast, AstNode, TextRange, TextSize}; use syntax::{algo, ast, AstNode, TextRange, TextSize};
use triomphe::Arc; use triomphe::Arc;
@ -1055,10 +1059,10 @@ pub(crate) fn handle_references(
let exclude_imports = snap.config.find_all_refs_exclude_imports(); let exclude_imports = snap.config.find_all_refs_exclude_imports();
let exclude_tests = snap.config.find_all_refs_exclude_tests(); let exclude_tests = snap.config.find_all_refs_exclude_tests();
let refs = match snap.analysis.find_all_refs(position, None)? { let Some(mut refs) = snap.analysis.find_all_refs(position, None)? else {
None => return Ok(None), return Ok(None);
Some(refs) => refs,
}; };
deduplicate_declarations(&mut refs);
let include_declaration = params.context.include_declaration; let include_declaration = params.context.include_declaration;
let locations = refs let locations = refs
@ -1090,6 +1094,17 @@ pub(crate) fn handle_references(
Ok(Some(locations)) Ok(Some(locations))
} }
fn deduplicate_declarations(refs: &mut Vec<ReferenceSearchResult>) {
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( pub(crate) fn handle_formatting(
snap: GlobalStateSnapshot, snap: GlobalStateSnapshot,
params: lsp_types::DocumentFormattingParams, params: lsp_types::DocumentFormattingParams,
@ -1794,7 +1809,10 @@ fn show_ref_command_link(
position: &FilePosition, position: &FilePosition,
) -> Option<lsp_ext::CommandLinkGroup> { ) -> Option<lsp_ext::CommandLinkGroup> {
if snap.config.hover_actions().references && snap.config.client_commands().show_reference { 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 uri = to_proto::url(snap, position.file_id);
let line_index = snap.file_line_index(position.file_id).ok()?; let line_index = snap.file_line_index(position.file_id).ok()?;
let position = to_proto::position(&line_index, position.offset); let position = to_proto::position(&line_index, position.offset);

View file

@ -302,6 +302,22 @@ pub fn slice_tails<T>(this: &[T]) -> impl Iterator<Item = &[T]> {
(0..this.len()).map(|i| &this[i..]) (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<T> IsNoneOr for Option<T> {
type Type = T;
fn is_none_or(self, f: impl FnOnce(T) -> bool) -> bool {
match self {
Some(v) => f(v),
None => true,
}
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;