Rust: Refine Self resolution inside impl blocks

[hvitved]

Before this PR, we would resolve Self inside impl blocks to either (1) the type being implemented, if Self is not a qualifier of another path, or (2) the impl block itself, otherwise.

This PR changes the logic so we always resolve Self to the type being implemented, which enables us to correctly resolve more paths.

We also restrict the scope of default trait implementations in path resolution, similar to @paldepind 's #20723, and finally prevent type parameters from escaping their scope when resolving type mentions.

DCA looks great: A significant reduction in Nodes With Type At Length Limit, and a modest increase in resolved calls.

[Copilot]

Pull Request Overview

This PR refines the resolution of Self inside impl blocks to consistently resolve to the type being implemented rather than the impl block itself. This enables more accurate path resolution, trait method calls, and type inference. The changes include restricting the scope of default trait implementations, preventing type parameters from escaping their scope, and improving consistency in path resolution.

• Resolves Self to the implementing type in all contexts, not to the impl block
• Prevents type parameters from escaping their declared scope during type resolution
• Restricts default trait implementations to only be inherited from the directly implemented trait, not super traits

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
rust/ql/lib/codeql/rust/internal/PathResolution.qll	Updates Self resolution logic to always resolve to the implementing type; restricts default trait implementation inheritance to direct traits only; simplifies path resolution logic
rust/ql/lib/codeql/rust/internal/TypeMention.qll	Adds type parameter scoping logic to prevent parameters from escaping their declaring item's scope; refactors type resolution into resolvePathTypeAt
rust/ql/lib/codeql/rust/internal/Type.qll	Adds getDeclaringItem() method to TypeParameter to track the scope of type parameters
rust/ql/lib/codeql/rust/internal/TypeInferenceConsistency.qll	Filters out known limitations for escaping type parameters in consistency checks
rust/ql/test/library-tests/path-resolution/main.rs	Adds test cases for improved Self resolution in impl blocks; demonstrates resolution of methods from different traits
rust/ql/test/library-tests/type-inference/main.rs	Removes spurious target annotation that is now correctly resolved
rust/ql/test/library-tests/type-inference/type-inference.expected	Updates expected results reflecting improved type inference with fewer incorrect inferences
rust/ql/test/library-tests/type-inference/CONSISTENCY/PathResolutionConsistency.expected	Removes spurious call target and adds multiple path resolution entries for associated types
rust/ql/test/library-tests/sensitivedata/CONSISTENCY/PathResolutionConsistency.expected	Removes false positive for multiple call targets
rust/ql/test/library-tests/path-resolution/path-resolution.expected	Updates expected path resolutions reflecting improved Self resolution to implementing types
rust/ql/test/library-tests/path-resolution/CONSISTENCY/PathResolutionConsistency.expected	Updates line numbers for multiple call targets after code changes

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[paldepind]

Nice to get the spurious result fixed!

The new path resolution inconsistencies in the type inference tests look like a problem. The Self::Output paths resolve to all the Output associated types, including those from other impl blocks.

Since Self in Self::Output now resolves to the struct S there's not enough information after that to resolve Output to the associated type in the correct impl block.

If I try "unfolding" the Self in Self::Output only something like <S<T> as MyAdd<&'a T>>::Output (for the last case) is sufficient to make the compiler happy. And these Ty as Tr paths are basically pinning down a specific impl block. So maybe resolving Self to the impl block actually is the right thing to do?

Note also that there's a big relative increase in path resolution inconsistencies on some projects such as competitive-library and radiance. That might be related to the above, but if not, it would probably be worthwhile looking into that as well.

[hvitved]

The new path resolution inconsistencies in the type inference tests look like a problem. The Self::Output paths resolve to all the Output associated types, including those from other impl blocks.

As discussed offline, I have put in a fix specifically for disambiguating Self::AssocType paths.

[paldepind]

Looks great with the latest fix.

The comments for getASelfPath and getAnItemInSelfScope are somewhat outdated as Self paths no longer refer to the ImplItemNode.

[paldepind]

Looks really great! Thanks for addressing my comments.

[hvitved]

Looks really great! Thanks for addressing my comments.

Thanks for your comments. I just added a few more test cases.