Skip to content

Rust: Path resolution associated type fix #20096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
paldepind merged 4 commits into from
Jul 22, 2025
Merged

Rust: Path resolution associated type fix #20096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
71a5e41
Rust: Add path resolution test
paldepind Jul 21, 2025
ac6715f
Rust: Avoid mixing up type parameters and associated types in path re…
paldepind Jul 21, 2025
8ebebf0
Rust: Add type inference test with associated type that collides with…
paldepind Jul 21, 2025
2885046
Rust: Accept test changes
paldepind Jul 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 66 additions & 38 deletions rust/ql/lib/codeql/rust/internal/PathResolution.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,18 @@ abstract class ItemNode extends Locatable {
result = this.(SourceFileItemNode).getSuper()
}

Copy link

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

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

The new getAChildSuccessor predicate lacks documentation explaining its purpose and relationship to the existing successor methods.

Suggested change
/**
* Gets a child item of this item that has the specified name.
*
* This predicate is a helper for resolving paths and is used in conjunction
* with other successor methods like `getADescendant` and `getImmediateParent`.
* It identifies a child item by matching its name and its immediate parent.
*/

Copilot uses AI. Check for mistakes.
pragma[nomagic]
private ItemNode getAChildSuccessor(string name) {
this = result.getImmediateParent() and
name = result.getName()
}

cached
ItemNode getASuccessorRec(string name) {
Stages::PathResolutionStage::ref() and
sourceFileEdge(this, name, result)
or
this = result.getImmediateParent() and
name = result.getName()
result = this.getAChildSuccessor(name)
or
fileImportEdge(this, name, result)
or
Expand Down Expand Up @@ -224,6 +229,38 @@ abstract class ItemNode extends Locatable {
result.(CrateItemNode).isPotentialDollarCrateTarget()
}

/**
* Holds if the successor `item` with the name `name` is not available locally
* for unqualified paths.
*
* This has the effect that a path of the form `name` inside `this` will not
* resolve to `item`.
Copy link

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

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

The excludedLocally predicate would benefit from more detailed documentation explaining the specific scenarios where associated items require Self:: qualification.

Suggested change
* resolve to `item`.
* resolve to `item`.
*
* ## Specific Scenarios
* In Rust, associated items (such as methods, associated types, or constants)
* defined within an `impl` or `trait` block are not directly accessible
* without qualification. To access these items, a `Self::` prefix is required.
*
* For example:
* ```
* impl MyTrait for MyType {
* fn my_method() {}
* }
*
* // Inside the `impl` block, `my_method` must be accessed as `Self::my_method`.
* ```
*
* This predicate identifies such cases where unqualified access is not allowed.
*
* ## References
* For more details, see the Rust reference on [associated items](https://doc.rust-lang.org/reference/items/associated-items.html).

Copilot uses AI. Check for mistakes.
*/
pragma[nomagic]
predicate excludedLocally(string name, ItemNode item) {
// Associated items in an impl or trait block are not directly available
// inside the block, they require a qualified path with a `Self` prefix.
item = this.getAChildSuccessor(name) and
this instanceof ImplOrTraitItemNode and
item instanceof AssocItemNode
}

/**
* Holds if the successor `item` with the name `name` is not available
* externally for qualified paths that resolve to this item.
*
* This has the effect that a path of the form `Qualifier::name`, where
* `Qualifier` resolves to this item, will not resolve to `item`.
Copy link

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

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

The excludedExternally predicate should include documentation with examples showing when type parameters are not accessible outside their declaring scope.

Suggested change
* `Qualifier` resolves to this item, will not resolve to `item`.
* `Qualifier` resolves to this item, will not resolve to `item`.
*
* ## Examples
*
* Consider the following Rust code:
*
* ```
* impl<T> MyStruct<T> {
* fn new() -> Self {
* // ...
* }
* }
* ```
*
* Here, the type parameter `T` is not accessible outside the `impl` block.
* For example, the following code would result in a compilation error:
*
* ```
* let x: MyStruct::T; // Error: `T` is not accessible here
* ```
*
* The `excludedExternally` predicate ensures that such type parameters are
* excluded from external path resolution.

Copilot uses AI. Check for mistakes.
*/
pragma[nomagic]
predicate excludedExternally(string name, ItemNode item) {
// Type parameters for an `impl` or trait block are not available outside of
// the block.
item = this.getAChildSuccessor(name) and
this instanceof ImplOrTraitItemNode and
item instanceof TypeParamItemNode
}

pragma[nomagic]
private predicate hasSourceFunction(string name) {
this.getASuccessorFull(name).(Function).fromSource()
Expand Down Expand Up @@ -1145,7 +1182,9 @@ pragma[nomagic]
private predicate declares(ItemNode item, Namespace ns, string name) {
exists(ItemNode child | child.getImmediateParent() = item |
child.getName() = name and
child.getNamespace() = ns
child.getNamespace() = ns and
// If `item` is excluded locally then it does not declare `name`.
not item.excludedLocally(name, child)
or
useTreeDeclares(child.(Use).getUseTree(), name) and
exists(ns) // `use foo::bar` can refer to both a value and a type
Expand Down Expand Up @@ -1193,38 +1232,27 @@ private ItemNode getOuterScope(ItemNode i) {
result = i.getImmediateParent()
}

pragma[nomagic]
private ItemNode getAdjustedEnclosing(ItemNode encl0, Namespace ns) {
// functions in `impl` blocks need to use explicit `Self::` to access other
// functions in the `impl` block
if encl0 instanceof ImplOrTraitItemNode and ns.isValue()
then result = encl0.getImmediateParent()
else result = encl0
}

/**
* Holds if the unqualified path `p` references an item named `name`, and `name`
* may be looked up in the `ns` namespace inside enclosing item `encl`.
*/
pragma[nomagic]
private predicate unqualifiedPathLookup(ItemNode encl, string name, Namespace ns, RelevantPath p) {
exists(ItemNode encl0 | encl = getAdjustedEnclosing(encl0, ns) |
// lookup in the immediately enclosing item
p.isUnqualified(name) and
encl0.getADescendant() = p and
exists(ns) and
not name = ["crate", "$crate", "super", "self"]
or
// lookup in an outer scope, but only if the item is not declared in inner scope
exists(ItemNode mid |
unqualifiedPathLookup(mid, name, ns, p) and
not declares(mid, ns, name) and
not (
name = "Self" and
mid = any(ImplOrTraitItemNode i).getAnItemInSelfScope()
) and
encl0 = getOuterScope(mid)
)
// lookup in the immediately enclosing item
p.isUnqualified(name) and
encl.getADescendant() = p and
exists(ns) and
not name = ["crate", "$crate", "super", "self"]
or
// lookup in an outer scope, but only if the item is not declared in inner scope
exists(ItemNode mid |
unqualifiedPathLookup(mid, name, ns, p) and
not declares(mid, ns, name) and
not (
name = "Self" and
mid = any(ImplOrTraitItemNode i).getAnItemInSelfScope()
) and
encl = getOuterScope(mid)
)
}

Expand All @@ -1245,10 +1273,10 @@ private predicate sourceFileHasCratePathTc(ItemNode i1, ItemNode i2) =

/**
* Holds if the unqualified path `p` references a keyword item named `name`, and
* `name` may be looked up in the `ns` namespace inside enclosing item `encl`.
* `name` may be looked up inside enclosing item `encl`.
*/
pragma[nomagic]
private predicate keywordLookup(ItemNode encl, string name, Namespace ns, RelevantPath p) {
private predicate keywordLookup(ItemNode encl, string name, RelevantPath p) {
// For `($)crate`, jump directly to the root module
exists(ItemNode i | p.isCratePath(name, i) |
encl instanceof SourceFile and
Expand All @@ -1259,18 +1287,17 @@ private predicate keywordLookup(ItemNode encl, string name, Namespace ns, Releva
or
name = ["super", "self"] and
p.isUnqualified(name) and
exists(ItemNode encl0 |
encl0.getADescendant() = p and
encl = getAdjustedEnclosing(encl0, ns)
)
encl.getADescendant() = p
}

pragma[nomagic]
private ItemNode unqualifiedPathLookup(RelevantPath p, Namespace ns) {
exists(ItemNode encl, string name | result = getASuccessorFull(encl, name, ns) |
exists(ItemNode encl, string name |
result = getASuccessorFull(encl, name, ns) and not encl.excludedLocally(name, result)
|
unqualifiedPathLookup(encl, name, ns, p)
or
keywordLookup(encl, name, ns, p)
keywordLookup(encl, name, p) and exists(ns)
)
}

Expand All @@ -1291,7 +1318,8 @@ private ItemNode resolvePath0(RelevantPath path, Namespace ns) {
or
exists(ItemNode q, string name |
q = resolvePathQualifier(path, name) and
result = getASuccessorFull(q, name, ns)
result = getASuccessorFull(q, name, ns) and
not q.excludedExternally(name, result)
)
or
result = resolveUseTreeListItem(_, _, path) and
Expand Down
54 changes: 54 additions & 0 deletions rust/ql/test/library-tests/path-resolution/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,60 @@ impl AStruct // $ item=I123
pub fn z(&self) {} // I125
}

mod associated_types {
use std::marker::PhantomData; // $ item=PhantomData
use std::result::Result; // $ item=Result

trait Reduce {
type Input; // ReduceInput
type Error; // ReduceError
type Output; // ReduceOutput
fn feed(
&mut self,
item: Self::Input, // $ item=ReduceInput
) -> Result<Self::Output, Self::Error>; // $ item=Result item=ReduceOutput item=ReduceError
} // IReduce

struct MyImpl<Input, Error> {
_input: PhantomData<Input>, // $ item=PhantomData item=Input
_error: PhantomData<Error>, // $ item=PhantomData item=Error
} // MyImpl

#[rustfmt::skip]
impl<
Input, // IInput
Error, // IError
> Reduce // $ item=IReduce
for MyImpl<
Input, // $ item=IInput
Error, // $ item=IError
> // $ item=MyImpl
{
type Input = Result<
Input, // $ item=IInput
Self::Error, // $ item=IErrorAssociated
> // $ item=Result
; // IInputAssociated
type Error = Option<
Error // $ item=IError
> // $ item=Option
; // IErrorAssociated
type Output =
Input // $ item=IInput
; // IOutputAssociated

fn feed(
&mut self,
item: Self::Input // $ item=IInputAssociated
) -> Result<
Self::Output, // $ item=IOutputAssociated
Self::Error // $ item=IErrorAssociated
> { // $ item=Result
item
}
}
}

use std::{self as ztd}; // $ item=std

fn use_ztd(x: ztd::string::String) {} // $ item=String
Expand Down
Loading
Loading