Rust: Take where clauses into account in path resolution#19193
Rust: Take where clauses into account in path resolution#19193hvitved merged 2 commits intogithub:mainfrom
where clauses into account in path resolution#19193Conversation
hvitved
force-pushed
the
rust/path-resolution-where-clause
branch
2 times, most recently
from
9117247 to
f17ffe3
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a stray whitespace issue in a comment marker in the type inference tests and adds a new test module to verify that Rust’s path resolution correctly takes into account where clauses.
- Fixed whitespace in a comment marker in the type inference test case
- Added a new module (m24) in path resolution tests to exercise trait bounds in where clauses
Reviewed Changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rust/ql/test/library-tests/type-inference/main.rs | Trimmed stray whitespace in the comment marker for a method call to ensure test annotations are correctly formatted. |
| rust/ql/test/library-tests/path-resolution/main.rs | Introduced module m24 with trait definitions and implementations to test path resolution logic involving where clauses. |
Files not reviewed (4)
- rust/ql/lib/codeql/rust/internal/PathResolution.qll: Language not supported
- rust/ql/lib/utils/test/PathResolutionInlineExpectationsTest.qll: Language not supported
- rust/ql/test/library-tests/path-resolution/path-resolution.expected: Language not supported
- rust/ql/test/library-tests/variables/variables.ql: Language not supported
| let impl_obj = Implementor; // $ item=I118 | ||
| let generic = GenericStruct { data: impl_obj }; // $ item=I115 | ||
|
|
||
| generic.call_trait_a(); // $ MISSING: item=I116 |
There was a problem hiding this comment.
The annotation '$ MISSING: item=I116' on this line is inconsistent with other markers in the file. If it is not intended, please remove or correct it to match the expected format.
| generic.call_trait_a(); // $ MISSING: item=I116 | |
| generic.call_trait_a(); // $ item=I116 |
hvitved
force-pushed
the
rust/path-resolution-where-clause
branch
from
f17ffe3 to
604e865
Compare
hvitved
force-pushed
the
rust/path-resolution-where-clause
branch
from
604e865 to
e26695f
Compare
paldepind
left a comment
paldepind
left a comment
There was a problem hiding this comment.
Looks good to me. I have one question, and based on the answer to that it might make the most sense to just merge as-is or make some tweaks.
|
|
||
| class TypeParamItemNode extends ItemNode instanceof TypeParam { | ||
| private WherePred getAWherePred() { | ||
| exists(ItemNode declaringItem | |
There was a problem hiding this comment.
The type of declaringItem is not very descriptive. Any item with result and this as descendants would fit. Could we reasonably spell out the type of items we expect here or does this cover to many things that that would be too tedious?
There was a problem hiding this comment.
I think it would be a bit tedious, type parameters can be declared in many different places: functions, type aliases, structs, enums, unions, consts, traits, impl blocks.
I used Copilot Agent to generate the test case, which mostly worked fine, except it inserted a stray whitespace in one of the comment markers, which took my quite a while to identify. To prevent this from happening again, I've inserted appropriate trimming of the markers.