Hmm, this won't fix the linked issue though. In that issue, all the occurences are different Definitions! We already do deduplicate the results, its just the the resulting NavigationTargets differ in properties that are not relevant to the shown results, so we need to do another deduplication pass when we convert to the requests response.

The deduplication should happen on the references before we return the ReferenceSearchResult here:

That is we need to go through all value Vecs of the map, sort, and deduplicate them there.

Hmm, this won't fix the linked issue though.

Good catch - that's because in that case the definitions are actually different, so unique() doesn't eliminate them, and because of the macro weirdness producing a cartesian product of references we also get redundant references along the technically-unique definitions.

The deduplication should happen on the references before we return the ReferenceSearchResult here:

The results of the lambda created by make_searcher are already deduplicated. The problem is that we call said lambda multiple times and concatenate the results.

Apparently our descend_into_macros() stanza returns one definition for
each time a name is used in a macro.

Sorry this was misleading. It's not about definitions but all references.

Given

macro_rules! use_name {
    ($name:ident) => {
        println!("{}", $name);
        println!("{}", $name);
        println!("{}", $name);
    };
}
fn test() {
    let name = 123;
    use_name!(name);
    println!("{}", name);
    println!("{}", name);
}

so 3 references inside the macro, and 4 outside;
this will produce 12 references; it's the product. Not sure if this is expected but I guess I haven't seen another problem with it.

It only surfaced when there was a definition inside a macro because we already deduplicate the per-definition references.

I think your suggestion makes sense; I'll try it and maybe get rid of the other two unique() calls.

This may still end up with multiple overlapping links, we need to do the filtering after we turned the ReferenceSearchResults into Locations, then dedup on those locations (this transformation, to_proto::location is where we lose the unnecessary data that prevented the deduplication from working as intended in the first place).

That's why I linked to specify lines in my earlier review, sorry that I wasn't clear enough with that.

we should probably find a test case. My variable name seen_declarations is misleading because we actually insert navigation targets here, which should be what we want to duplicate on?

No we do not want to deduplicate NavigationTargets, we want to deduplicate lsp Locations. The nav targets haave a lot of info that differs even though the pointed to ranges are the same hence why deduplicating them is non trivial (and also wrong!). once we transformed them to locations we lose all the differing info of the nav targets that are irrelevant for the feature so now the deduplication does meaningful work (and is trivial to implement).

We should probably do the same for goto_definition_response