Rust: crate_graph: generate 'use' statements for re-exported items #19113
Conversation
aibaars
force-pushed
the
aibaars/crate-graph-reexport
branch
from
f15b605 to
07767f3
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request adds pub use statements to re-export items in the crate graph and introduces a new helper function to emit these re-exports in the extractor.
- Adds explicit re-export statements in the test module
- Introduces a new helper function, emit_reexport, in the extractor to process and emit re-export code
- Updates module item emission to integrate the newly emitted re-exports
Reviewed Changes
Copilot reviewed 2 out of 7 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rust/ql/test/extractor-tests/crate_graph/module.rs | Adds re-export use statements for filesystem items and path types |
| rust/extractor/src/crate_graph.rs | Adds new function and updates logic to handle re-exported items during module item emission |
Files not reviewed (5)
- rust/ql/.generated.list: Language not supported
- rust/ql/.gitattributes: Language not supported
- rust/ql/lib/codeql/rust/elements/internal/UseImpl.qll: Language not supported
- rust/ql/lib/codeql/rust/elements/internal/UseTreeImpl.qll: Language not supported
- rust/ql/test/extractor-tests/crate_graph/modules.expected: Language not supported
Comments suppressed due to low confidence (1)
rust/extractor/src/crate_graph.rs:200
- [nitpick] Consider renaming the inner variable 'use_' to a clearer name (e.g., 'use_tree_item') to avoid potential confusion with the outer 'use_' from the match statement.
let use_ = &loc.id.item_tree(def_db)[loc.id.value];
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
aibaars
force-pushed
the
aibaars/crate-graph-reexport
branch
from
07767f3 to
cb6f48f
Compare
| result = | ||
| this.getPath().toStringImpl() + | ||
| any(string list | if this.hasUseTreeList() then list = "::{...}" else list = "") + | ||
| any(string glob | if this.isGlob() then glob = "::*" else glob = "") + | ||
| any(string rename | | ||
| rename = " as " + this.getRename().getName().getText() | ||
| or | ||
| rename = "" and not this.hasRename() | ||
| ) |
There was a problem hiding this comment.
Use concat and the toStringPart pattern instead.
| path_components.extend(path.segments().iter().map(|x| x.as_str().to_owned())); | ||
| match kind { | ||
| ImportKind::Plain => (), | ||
| ImportKind::Glob => path_components.push(name.to_owned()), |
There was a problem hiding this comment.
Can you elaborate on why we push name and not make a glob import instead?
There was a problem hiding this comment.
I think explicit imports are more clear and easier to interpret. However, if you prefer * imports then it is easy to change.
There was a problem hiding this comment.
Explicit imports are easier, I just wondered how a single name can make it up for a full glob import?
There was a problem hiding this comment.
We iterate over all types and values that are in the "item scope", for the types/values that are imported (possibly as part of a glob import) we generate a use statement.
There was a problem hiding this comment.
Just see the pub use statements in the test case and how they end up in the output of modules.ql.
aibaars
force-pushed
the
aibaars/crate-graph-reexport
branch
from
9851a54 to
3d20d33
Compare
aibaars
force-pushed
the
aibaars/crate-graph-reexport
branch
from
3d20d33 to
ec9fe80
Compare
This pull request adds
pub useimport statements to crate graph modules for items that are re-exported