Skip to content

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Nov 17, 2025

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.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Nov 17, 2025
@hvitved hvitved force-pushed the rust/path-resolution-impl-self branch 2 times, most recently from 104b715 to 6f83525 Compare November 18, 2025 13:50
@hvitved hvitved force-pushed the rust/path-resolution-impl-self branch from 6f83525 to 63926e6 Compare November 19, 2025 07:51
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Nov 21, 2025
@hvitved hvitved marked this pull request as ready for review November 21, 2025 12:04
@hvitved hvitved requested a review from a team as a code owner November 21, 2025 12:04
@hvitved hvitved requested review from Copilot and paldepind November 21, 2025 12:04
Copilot finished reviewing on behalf of hvitved November 21, 2025 12:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant