Skip to content

Commit 605cf35

Browse files
committed
Rust: More path resolution improvements
1 parent f6ac82a commit 605cf35

File tree

4 files changed

+84
-23
lines changed

4 files changed

+84
-23
lines changed

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
@@ -613,11 +613,11 @@ private predicate fileModule(SourceFile f, string name, Folder folder) {
613613
}
614614

615615
/**
616-
* Holds if `m` is a `mod name;` module declaration happening in a file named
617-
* `fileName.rs`, inside the folder `parent`.
616+
* Holds if `m` is a `mod name;` module declaration, where the corresponding
617+
* module file needs to be looked up in `lookup` or one of its descandants.
618618
*/
619-
private predicate modImport(Module m, string fileName, string name, Folder parent) {
620-
exists(File f |
619+
private predicate modImport0(Module m, string name, Folder lookup) {
620+
exists(File f, Folder parent, string fileName |
621621
f = m.getFile() and
622622
not m.hasItemList() and
623623
// TODO: handle
@@ -629,17 +629,63 @@ private predicate modImport(Module m, string fileName, string name, Folder paren
629629
name = m.getName().getText() and
630630
parent = f.getParentContainer() and
631631
fileName = f.getStem()
632+
|
633+
// sibling import
634+
lookup = parent and
635+
(
636+
m.getFile() = any(CrateItemNode c).getModuleNode().(SourceFileItemNode).getFile()
637+
or
638+
m.getFile().getBaseName() = "mod.rs"
639+
)
640+
or
641+
// child import
642+
lookup = parent.getFolder(fileName)
643+
)
644+
}
645+
646+
/**
647+
* Holds if `m` is a `mod name;` module declaration, which happens inside a
648+
* nested module, and `pred -> succ` is a module edge leading to `m`.
649+
*/
650+
private predicate modImportNested(ModuleItemNode m, ModuleItemNode pred, ModuleItemNode succ) {
651+
pred.getAnItemInScope() = succ and
652+
(
653+
modImport0(m, _, _) and
654+
succ = m
655+
or
656+
modImportNested(m, succ, _)
657+
)
658+
}
659+
660+
/**
661+
* Holds if `m` is a `mod name;` module declaration, which happens inside a
662+
* nested module, where `ancestor` is a reflexive transitive ancestor module
663+
* of `m` with corresponding lookup folder `lookup`.
664+
*/
665+
private predicate modImportNestedLookup(Module m, ModuleItemNode ancestor, Folder lookup) {
666+
modImport0(m, _, lookup) and
667+
modImportNested(m, ancestor, _) and
668+
not modImportNested(m, _, ancestor)
669+
or
670+
exists(ModuleItemNode m1, Folder mid |
671+
modImportNestedLookup(m, m1, mid) and
672+
modImportNested(m, m1, ancestor) and
673+
lookup = mid.getFolder(m1.getName())
632674
)
633675
}
634676

635677
/** Holds if `m` is a `mod name;` item importing file `f`. */
636678
private predicate fileImport(Module m, SourceFile f) {
637-
exists(string fileName, string name, Folder parent | modImport(m, fileName, name, parent) |
638-
// sibling import
679+
exists(string name, Folder parent |
680+
modImport0(m, name, _) and
639681
fileModule(f, name, parent)
682+
|
683+
// `m` is not inside a nested module
684+
modImport0(m, name, parent) and
685+
not modImportNested(m, _, _)
640686
or
641-
// child import
642-
fileModule(f, name, parent.getFolder(fileName))
687+
// `m` is inside a nested module
688+
modImportNestedLookup(m, m, parent)
643689
)
644690
}
645691

@@ -651,7 +697,7 @@ pragma[nomagic]
651697
private predicate fileImportEdge(Module mod, string name, ItemNode item) {
652698
exists(SourceFileItemNode f |
653699
fileImport(mod, f) and
654-
item = f.getASuccessor(name)
700+
item = f.getASuccessorRec(name)
655701
)
656702
}
657703

@@ -660,7 +706,7 @@ private predicate fileImportEdge(Module mod, string name, ItemNode item) {
660706
*/
661707
pragma[nomagic]
662708
private predicate crateDefEdge(CrateItemNode c, string name, ItemNode i) {
663-
i = c.getModuleNode().getASuccessor(name) and
709+
i = c.getModuleNode().getASuccessorRec(name) and
664710
not i instanceof Crate
665711
}
666712

@@ -742,7 +788,16 @@ private predicate unqualifiedPathLookup(RelevantPath p, string name, Namespace n
742788
// lookup in an outer scope, but only if the item is not declared in inner scope
743789
exists(ItemNode mid |
744790
unqualifiedPathLookup(p, name, ns, mid) and
745-
not declares(mid, ns, name)
791+
not declares(mid, ns, name) and
792+
not name = ["super", "self"] and
793+
not (
794+
name = "Self" and
795+
mid = any(ImplOrTraitItemNode i).getAnItemInSelfScope()
796+
) and
797+
not (
798+
name = "crate" and
799+
mid = any(CrateItemNode i).getASourceFile()
800+
)
746801
|
747802
// nested modules do not have unqualified access to items from outer modules,
748803
// except for items declared at top-level in the source file
@@ -943,15 +998,19 @@ private predicate useImportEdge(Use use, string name, ItemNode item) {
943998
encl.getADescendant() = use and
944999
item = getASuccessor(used, name, ns) and
9451000
// glob imports can be shadowed
946-
not declares(encl, ns, name)
1001+
not declares(encl, ns, name) and
1002+
not name = ["super", "self", "Self", "crate"]
9471003
)
948-
else item = used
949-
|
950-
not tree.hasRename() and
951-
name = item.getName()
952-
or
953-
name = tree.getRename().getName().getText() and
954-
name != "_"
1004+
else (
1005+
item = used and
1006+
(
1007+
not tree.hasRename() and
1008+
name = item.getName()
1009+
or
1010+
name = tree.getRename().getName().getText() and
1011+
name != "_"
1012+
)
1013+
)
9551014
)
9561015
}
9571016

@@ -961,7 +1020,7 @@ private module Debug {
9611020
exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
9621021
result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
9631022
filepath.matches("%/main.rs") and
964-
startline = 1
1023+
startline = [1, 3]
9651024
)
9661025
}
9671026

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)