Rust: Follow-up work to make path resolution and type inference tests pass again#19519
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the Rust QL tests and library modules to filter out macro-generated AST nodes, enforce source-only elements, and reflect real standard-library paths in test expectations. Key changes include:
- Adding
fromSource()andisFromMacroExpansion()filters in type inference and path resolution tests - Updating expected outputs to point to the real
core/resultdefinitions in the Rust stdlib - Implementing new predicates (
fromSource,startsBefore,startsStrictlyBefore, etc.) and adapting path-resolution and type-inference logic in the QL libraries
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/test/library-tests/type-inference/type-inference.ql | Added source/macro filters to inferType and related predicates |
| rust/ql/test/library-tests/type-inference/type-inference.expected | Updated expected test outputs to real stdlib Result and Arguments paths |
| rust/ql/test/library-tests/path-resolution/path-resolution.ql | Applied source/macro filters to resolvePath queries |
| rust/ql/test/library-tests/path-resolution/path-resolution.expected | Updated expected module paths to real stdlib locations |
| rust/ql/test/TestUtils.qll | Expanded toBeTested to only include source AST nodes |
| rust/ql/lib/utils/test/PathResolutionInlineExpectationsTest.qll | Added source/macro filters to inline-expectations test helpers |
| rust/ql/lib/codeql/rust/internal/TypeInference.qll | Adjusted hard-coded debug line number |
| rust/ql/lib/codeql/rust/internal/PathResolution.qll | Replaced getModuleNode with getSourceFile, removed outdated isRoot |
| rust/ql/lib/codeql/rust/frameworks/stdlib/Stdlib.qll | Refactored OptionEnum/ResultEnum to use getASuccessor |
| rust/ql/lib/codeql/rust/elements/internal/MacroCallImpl.qll | Introduced isInMacroExpansion and getATokenTreeNode |
| rust/ql/lib/codeql/rust/elements/internal/LocationImpl.qll | Renamed/extended location predicates (startsBefore, etc.) |
| rust/ql/lib/codeql/rust/elements/internal/LocatableImpl.qll | Changed fromSource to delegate to File.fromSource() |
| rust/ql/lib/codeql/rust/elements/internal/AstNodeImpl.qll | Hooked isInMacroExpansion into new MacroCallImpl predicates |
| rust/ql/lib/codeql/files/FileSystem.qll | Strengthened File.fromSource() by requiring a relative path |
Comments suppressed due to low confidence (1)
rust/ql/test/TestUtils.qll:17
- The
CrateElementpredicate no longer includesthis instanceof Crate, which may prevent actualCrateelements from being recognized. Consider restoringthis instanceof Cratein the predicate.
this instanceof NamedCrate
| result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and | ||
| filepath.matches("%/main.rs") and | ||
| startline = 28 | ||
| startline = 948 |
There was a problem hiding this comment.
[nitpick] Avoid hard-coding line numbers in production predicates. Consider computing this value dynamically or defining a named constant to explain its origin.
| f.getAPrecedingComment().getCommentText() = value and | ||
| f.fromSource() | ||
| or | ||
| not exists(f.getAPrecedingComment()) and | ||
| not any(f.getAPrecedingComment()).fromSource() and | ||
| // TODO: Default to canonical path once that is available | ||
| value = f.getName().getText() |
There was a problem hiding this comment.
[nitpick] To make the OR conditions clearer, wrap each branch in parentheses. For example: (f.getAPrecedingComment().getCommentText() = value and f.fromSource()) or (not any(f.getAPrecedingComment()).fromSource() and value = f.getName().getText()).
| | main.rs:658:19:658:22 | self | Fst | main.rs:656:10:656:12 | Fst | | ||
| | main.rs:658:19:658:22 | self | Snd | main.rs:656:15:656:17 | Snd | | ||
| | main.rs:659:43:659:82 | MacroExpr | | main.rs:656:15:656:17 | Snd | | ||
| | main.rs:659:50:659:81 | MacroExpr | | file:///Users/hvitved/.rustup/toolchains/1.85-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:549:1:584:1 | Arguments | |
There was a problem hiding this comment.
Here the home directory is leaking into expected test output.
There was a problem hiding this comment.
I believe those should be fixed when you merge latest main into your branch, which contains
f7f434b
into
github:aibaars/rust-extract-libs
#19506 follow-up.