Prefer ambiguous candidates when deduplicating traits in scope, so ambiguous_glob_imported_traits doesn't depend on import order

Project: rust-lang/rust | Language: Rust | Concepts: compiler-internals, error-handling, testing-strategy | Difficulty: senior
Link: https://github.com/rust-lang/rust/pull/161805

The problem

When multiple imports bring the same trait into scope—some ambiguous (e.g., via a glob import) and some not—the Rust compiler's method resolution would deduplicate these based solely on the trait's ID. This meant that if the ambiguous import appeared before the unambiguous one (or vice versa), only one would be considered, and the ambiguous_glob_imported_traits lint might not trigger, depending on import order. This made linting unreliable and order-dependent.

Why it's non-trivial

A naive fix would just deduplicate by trait ID, which ignores the distinction between ambiguous and unambiguous imports. This misses subtle cases where both forms are present, and the ambiguity should be reported regardless of order. The fix must ensure that both ambiguous and unambiguous candidates are tracked separately and that ambiguity is detected even if only one is present after deduplication.

The approach

1. Change deduplication to consider ambiguity:

Previously, deduplication was done with just the trait's def_id:

- if duplicates.insert(trait_did) {
+ // If we have the same trait in scope but one of them is ambiguous and the other
+ // is not, we should treat them differently and then handle them later on.
+ if duplicates.insert((trait_did, trait_candidate.lint_ambiguous)) {

Annotation:
This change makes the deduplication key a tuple of (def_id, lint_ambiguous), so ambiguous and unambiguous imports of the same trait are treated as distinct candidates. This ensures both are considered in later processing.

2. Mark the pick as ambiguous if any candidate is ambiguous:

Previously, only the first candidate's ambiguity was considered:

- let lint_ambiguous = match probes[0].0.kind {
-     TraitCandidate(_, lint) => lint,
-     _ => false,
- };
+ // They are all the same, so if any of them is ambiguous, we report the pick as ambiguous.
+ let lint_ambiguous = probes.iter().any(|(p, _)| match p.kind {
+     TraitCandidate(_, lint) => lint,
+     _ => false,
+ });

Annotation:
Now, if any candidate is ambiguous, the pick is marked as ambiguous. This ensures the lint is triggered whenever ambiguity is present, regardless of import order.

3. Add order-invariant regression tests:

Two test files are added, one for each import order:

• ambiguous-trait-in-scope-and-underscore.first.stderr
• ambiguous-trait-in-scope-and-underscore.last.stderr
• ambiguous-trait-in-scope-and-underscore.rs (with revisions for both orders)

Annotation:
These tests ensure that the lint is triggered regardless of whether the ambiguous or unambiguous import comes first.

From the review thread

petrochenkov:
The treatment of ambiguous glob sets needs to be reworked in general, and the answer on this question may depend on the results of the rework.

For now let's merge this, this PR makes the lint more conservative and (more importantly) order-independent, we can relax it later if it becomes possible with the glob set rework.

petrochenkov:

        fn into_sql(self);

It would be better to further minimize the test to remove all the distracting off-topic stuff like this.

Takeaways

• When deduplicating candidates, include all relevant distinguishing information (such as ambiguity flags) to avoid losing critical distinctions.
• Linting and error reporting should not depend on the order of imports or declarations; order-independence is essential for predictable diagnostics.
• Regression tests should explicitly cover both possible orders (or all relevant permutations) to ensure order-invariant behavior.