Skip to content

Commit f091b36

Browse files
committed
Rust: More path resolution improvements
1 parent 25dc03a commit f091b36

4 files changed

Lines changed: 84 additions & 23 deletions

File tree

rust/ql/lib/codeql/rust/internal/PathResolution.qll

Lines changed: 80 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ abstract class ItemNode extends Locatable {
116116
}
117117

118118
pragma[nomagic]
119-
private ItemNode getASuccessorRec(string name) {
119+
ItemNode getASuccessorRec(string name) {
120120
sourceFileEdge(this, name, result)
121121
or
122122
this = result.getImmediateParent() and
@@ -618,11 +618,11 @@ private predicate fileModule(SourceFile f, string name, Folder folder) {
618618
}
619619

620620
/**
621-
* Holds if `m` is a `mod name;` module declaration happening in a file named
622-
* `fileName.rs`, inside the folder `parent`.
621+
* Holds if `m` is a `mod name;` module declaration, where the corresponding
622+
* module file needs to be looked up in `lookup` or one of its descandants.
623623
*/
624-
private predicate modImport(Module m, string fileName, string name, Folder parent) {
625-
exists(File f |
624+
private predicate modImport0(Module m, string name, Folder lookup) {
625+
exists(File f, Folder parent, string fileName |
626626
f = m.getFile() and
627627
not m.hasItemList() and
628628
// TODO: handle
@@ -634,17 +634,63 @@ private predicate modImport(Module m, string fileName, string name, Folder paren
634634
name = m.getName().getText() and
635635
parent = f.getParentContainer() and
636636
fileName = f.getStem()
637+
|
638+
// sibling import
639+
lookup = parent and
640+
(
641+
m.getFile() = any(CrateItemNode c).getModuleNode().(SourceFileItemNode).getFile()
642+
or
643+
m.getFile().getBaseName() = "mod.rs"
644+
)
645+
or
646+
// child import
647+
lookup = parent.getFolder(fileName)
648+
)
649+
}
650+
651+
/**
652+
* Holds if `m` is a `mod name;` module declaration, which happens inside a
653+
* nested module, and `pred -> succ` is a module edge leading to `m`.
654+
*/
655+
private predicate modImportNested(ModuleItemNode m, ModuleItemNode pred, ModuleItemNode succ) {
656+
pred.getAnItemInScope() = succ and
657+
(
658+
modImport0(m, _, _) and
659+
succ = m
660+
or
661+
modImportNested(m, succ, _)
662+
)
663+
}
664+
665+
/**
666+
* Holds if `m` is a `mod name;` module declaration, which happens inside a
667+
* nested module, where `ancestor` is a reflexive transitive ancestor module
668+
* of `m` with corresponding lookup folder `lookup`.
669+
*/
670+
private predicate modImportNestedLookup(Module m, ModuleItemNode ancestor, Folder lookup) {
671+
modImport0(m, _, lookup) and
672+
modImportNested(m, ancestor, _) and
673+
not modImportNested(m, _, ancestor)
674+
or
675+
exists(ModuleItemNode m1, Folder mid |
676+
modImportNestedLookup(m, m1, mid) and
677+
modImportNested(m, m1, ancestor) and
678+
lookup = mid.getFolder(m1.getName())
637679
)
638680
}
639681

640682
/** Holds if `m` is a `mod name;` item importing file `f`. */
641683
private predicate fileImport(Module m, SourceFile f) {
642-
exists(string fileName, string name, Folder parent | modImport(m, fileName, name, parent) |
643-
// sibling import
684+
exists(string name, Folder parent |
685+
modImport0(m, name, _) and
644686
fileModule(f, name, parent)
687+
|
688+
// `m` is not inside a nested module
689+
modImport0(m, name, parent) and
690+
not modImportNested(m, _, _)
645691
or
646-
// child import
647-
fileModule(f, name, parent.getFolder(fileName))
692+
// `m` is inside a nested module
693+
modImportNestedLookup(m, m, parent)
648694
)
649695
}
650696

@@ -656,7 +702,7 @@ pragma[nomagic]
656702
private predicate fileImportEdge(Module mod, string name, ItemNode item) {
657703
exists(SourceFileItemNode f |
658704
fileImport(mod, f) and
659-
item = f.getASuccessor(name)
705+
item = f.getASuccessorRec(name)
660706
)
661707
}
662708

@@ -665,7 +711,7 @@ private predicate fileImportEdge(Module mod, string name, ItemNode item) {
665711
*/
666712
pragma[nomagic]
667713
private predicate crateDefEdge(CrateItemNode c, string name, ItemNode i) {
668-
i = c.getModuleNode().getASuccessor(name) and
714+
i = c.getModuleNode().getASuccessorRec(name) and
669715
not i instanceof Crate
670716
}
671717

@@ -747,7 +793,16 @@ private predicate unqualifiedPathLookup(RelevantPath p, string name, Namespace n
747793
// lookup in an outer scope, but only if the item is not declared in inner scope
748794
exists(ItemNode mid |
749795
unqualifiedPathLookup(p, name, ns, mid) and
750-
not declares(mid, ns, name)
796+
not declares(mid, ns, name) and
797+
not name = ["super", "self"] and
798+
not (
799+
name = "Self" and
800+
mid = any(ImplOrTraitItemNode i).getAnItemInSelfScope()
801+
) and
802+
not (
803+
name = "crate" and
804+
mid = any(CrateItemNode i).getASourceFile()
805+
)
751806
|
752807
// nested modules do not have unqualified access to items from outer modules,
753808
// except for items declared at top-level in the source file
@@ -948,15 +1003,19 @@ private predicate useImportEdge(Use use, string name, ItemNode item) {
9481003
encl.getADescendant() = use and
9491004
item = getASuccessor(used, name, ns) and
9501005
// glob imports can be shadowed
951-
not declares(encl, ns, name)
1006+
not declares(encl, ns, name) and
1007+
not name = ["super", "self", "Self", "crate"]
9521008
)
953-
else item = used
954-
|
955-
not tree.hasRename() and
956-
name = item.getName()
957-
or
958-
name = tree.getRename().getName().getText() and
959-
name != "_"
1009+
else (
1010+
item = used and
1011+
(
1012+
not tree.hasRename() and
1013+
name = item.getName()
1014+
or
1015+
name = tree.getRename().getName().getText() and
1016+
name != "_"
1017+
)
1018+
)
9601019
)
9611020
}
9621021

@@ -966,7 +1025,7 @@ private module Debug {
9661025
exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
9671026
result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
9681027
filepath.matches("%/main.rs") and
969-
startline = 1
1028+
startline = [1, 3]
9701029
)
9711030
}
9721031

rust/ql/test/library-tests/path-resolution/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,6 @@ fn main() {
518518
nested6::f(); // $ item=I116
519519
nested8::f(); // $ item=I119
520520
my3::f(); // $ item=I200
521-
nested_f(); // $ MISSING: item=I201
521+
nested_f(); // $ item=I201
522522
m18::m19::m20::g(); // $ item=I103
523523
}

rust/ql/test/library-tests/path-resolution/my.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,4 @@ mod my4 {
1515
pub mod my5;
1616
}
1717

18-
pub use my4::my5::f as nested_f; // $ MISSING: item=I201
18+
pub use my4::my5::f as nested_f; // $ item=I201

rust/ql/test/library-tests/path-resolution/path-resolution.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ resolvePath
270270
| main.rs:519:5:519:14 | ...::f | my2/nested2.rs:23:9:25:9 | fn f |
271271
| main.rs:520:5:520:7 | my3 | my2/mod.rs:12:1:12:12 | mod my3 |
272272
| main.rs:520:5:520:10 | ...::f | my2/my3/mod.rs:1:1:5:1 | fn f |
273+
| main.rs:521:5:521:12 | nested_f | my/my4/my5/mod.rs:1:1:3:1 | fn f |
273274
| main.rs:522:5:522:7 | m18 | main.rs:476:1:494:1 | mod m18 |
274275
| main.rs:522:5:522:12 | ...::m19 | main.rs:481:5:493:5 | mod m19 |
275276
| main.rs:522:5:522:17 | ...::m20 | main.rs:486:9:492:9 | mod m20 |
@@ -296,6 +297,7 @@ resolvePath
296297
| my.rs:11:5:11:5 | g | my/nested.rs:19:1:22:1 | fn g |
297298
| my.rs:18:9:18:11 | my4 | my.rs:14:1:16:1 | mod my4 |
298299
| my.rs:18:9:18:16 | ...::my5 | my.rs:15:5:15:16 | mod my5 |
300+
| my.rs:18:9:18:19 | ...::f | my/my4/my5/mod.rs:1:1:3:1 | fn f |
299301
| my/nested.rs:9:13:9:13 | f | my/nested.rs:3:9:5:9 | fn f |
300302
| my/nested.rs:15:9:15:15 | nested2 | my/nested.rs:2:5:11:5 | mod nested2 |
301303
| my/nested.rs:15:9:15:18 | ...::f | my/nested.rs:3:9:5:9 | fn f |

0 commit comments

Comments
 (0)