Rust: Path resolution before variable resolution

[hvitved]

Path resolution needs to happen before variable resolution, because otherwise we cannot know whether an identifier pattern binds a new variable or whether it refers to a constructor or constant:

let x = ...; // `x` is only a variable if it does not resolve to a constructor/constant

Even though variable names typically start with a lowercase letter and constructors/constants with an uppercase letter, this is not enforced by the Rust language.

Variables may shadow declarations, so variable resolution also needs to affect path resolution:

fn foo() {}        // (1)

fn bar() {
    let f = foo;   // `foo` here refers to (1) via path resolution
    let foo = f(); // (2)
    foo            // `foo` here refers to (2) via variable resolution
}

So it may seem that path resolution and variable resolution must happen in mutual recursion, but we would like to keep the inherently global path resolution logic separate from the inherently local variable resolution logic. We acheive this by

• First computing global path resolution, where variable shadowing is ignored, exposed as the internal predicate resolvePathIgnoreVariableShadowing.
• resolvePathIgnoreVariableShadowing is sufficient to determine whether an identifier pattern resolves to a constructor/constant, since if it does, it cannot be shadowed by a variable. We expose this as the predicate identPatIsResolvable.
• Variable resolution can then be computed as a local property, using only the global information from identPatIsResolvable.
• Finally, path resolution can be computed by restricting resolvePathIgnoreVariableShadowing to paths that are not resolvable via variable resolution.

DCA looks good: We get a (somewhat surprisingly) large increase in Percentage of calls with call target, and a (less surprisingly) increase in results for rust/unused-variable. I spot checked some of the new alerts on rust and rust-ffmpeg, and while they are FPs, opening those files in VS Code with Rust analyzer also results in warnings. So instead of attempting to filter away the results in the query, I would like to keep them in, as they are generally very useful for finding bugs/shortcomings in path resolution (such as this one).

[Copilot]

Pull Request Overview

This PR refactors the Rust path resolution logic to happen before variable resolution, addressing a fundamental ordering issue in the analysis. The change allows the CodeQL analyzer to correctly distinguish between identifier patterns that bind variables versus those that reference constructors or constants.

• Path resolution is now computed globally while ignoring variable shadowing, exposed via resolvePathIgnoreVariableShadowing
• A new identPatIsResolvable predicate determines if identifier patterns resolve to constructors/constants
• Variable resolution now uses path resolution information instead of relying on naming conventions
• The PathExt abstraction unifies handling of both Path and IdentPat nodes

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
rust/ql/lib/codeql/rust/internal/PathResolution.qll	Core refactoring: introduces PathExt abstraction, resolvePathIgnoreVariableShadowing, and identPatIsResolvable predicates; replaces RelevantPath with PathExt
rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll	Replaces uppercase naming heuristic with identPatIsResolvable check for determining variables
rust/ql/lib/codeql/rust/elements/internal/ConstImpl.qll	Adds IdentPatConstAccess to support constant references in identifier patterns
rust/ql/lib/codeql/rust/elements/internal/PathExprImpl.qll	Updates primary QL class handling for variable access distinction
rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll	Updates pattern matching logic to use variable declaration check instead of naming heuristic
rust/ql/lib/codeql/rust/elements/internal/FormatTemplateVariableAccessImpl.qll	Adds primary QL class identifier
rust/ql/lib/codeql/rust/internal/Definitions.qll	Minor optimization to macro call location check
rust/ql/lib/codeql/rust/internal/CachedStages.qll	Updates stage dependencies, removes unused trait bound check
rust/ql/lib/codeql/rust/elements/internal/CallImpl.qll	Updates type reference from RelevantPath to PathExt
rust/ql/test/library-tests/path-resolution/path-resolution.ql	Updates query signature to use PathExt
rust/ql/test/library-tests/path-resolution/main.rs	Adds comprehensive test cases for pattern resolution scenarios
rust/ql/test/library-tests/variables/main.rs	Adds test cases for variable shadowing and pattern matching
rust/ql/integration-tests/hello-workspace/path-resolution.ql	Updates query signature to use PathExt
Multiple .expected files	Updated test expectations reflecting improved path and variable resolution

---

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

[Copilot]

Spelling error: "acheive" should be "achieve".

Suggested change

	* separate from the inherently local variable resolution logic. We acheive this by
	* separate from the inherently local variable resolution logic. We achieve this by

[Copilot]

[nitpick] The comment "constz" appears to be unclear. Consider using a more descriptive comment like "constant z" to clarify the reference.

Suggested change

	= 0; // constz
	= 0; // constant z

[Copilot]

[nitpick] The comment says "constructors with an uppercase letter" but should say "constructors/constants with an uppercase letter" for consistency with the full description in lines 4-13 which mentions both constructors and constants.

Suggested change

	* Even though variable names typically start with a lowercase letter and constructors
	* Even though variable names typically start with a lowercase letter and constructors/constants