Rust: Path resolution for extern crates#19614
Merged
hvitved merged 2 commits intogithub:mainfrom Jun 10, 2025
Merged
Conversation
65edd1c to
521b69d
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for resolving items reexported via extern crate, updates core resolution logic, and adjusts test expectations accordingly
- Introduces
externCrateEdgeandExternCrateItemNodeinPathResolution.qllto handleextern crateimports - Extends library path-resolution tests (
main.rs,my.rs) to validate resolvingStringand other items throughextern cratealiases - Updates numerous expected outputs and diagnostics (including
SummaryStatsReduced.expected) to reflect new path resolutions
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| rust/ql/lib/codeql/rust/internal/PathResolution.qll | Added externCrateEdge, ExternCrateItemNode, and stub overrides |
| rust/ql/test/library-tests/path-resolution/main.rs | Added extern crate self as zelf and updated use to test aliasing |
| rust/ql/test/library-tests/path-resolution/my.rs | Extended test to assert String resolution via extern crate |
| rust/ql/test/diagnostics/SummaryStatsReduced.expected | Incremented path resolution inconsistency count |
Comments suppressed due to low confidence (3)
rust/ql/lib/codeql/rust/internal/PathResolution.qll:382
- [nitpick] The ExternCrateItemNode.getNamespace override returns none, preventing extern crate aliases from providing a namespace; consider returning an appropriate namespace or documenting why none() is correct.
override Namespace getNamespace() { none() }
rust/ql/lib/codeql/rust/internal/PathResolution.qll:388
- [nitpick] The ExternCrateItemNode.hasCanonicalPath override returns none, which may unintentionally disable canonical path mappings for extern crates; ensure this behavior is intended or implement proper canonical path logic.
override predicate hasCanonicalPath(Crate c) { none() }
rust/ql/lib/codeql/rust/internal/PathResolution.qll:390
- [nitpick] The ExternCrateItemNode.getCanonicalPath override returns none, which may prevent resolution of canonical paths for extern crate items; consider providing a canonical path or clarifying why none() is appropriate.
override string getCanonicalPath(Crate c) { none() }
521b69d to
721ffb1
Compare
paldepind
approved these changes
Jun 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I noticed that we were not able to resolve the
Stringstruct. This happens because it is reexported using anextern cratedefinition. This PR adds path resolution support forextern crates, and as the test output witnesses we are now able to resolveString.DCA shows a whopping 4 percentage point increase in resolved calls (up 54 % from 50 %).