Skip to content

Commit 970e4c6

Browse files
committed
unified: Fix handling of unscoped imports
An `import X` declaration now does two things: - X becomes a local name binding - X is bulk-imported into the local scope The AST mapping now maps it to X with a bulk-importing pattern as a sub-pattern. Module names can no longer be referenced anywhere except as the leading qualifier of an import statement.
1 parent af2f5a2 commit 970e4c6

11 files changed

Lines changed: 56 additions & 19 deletions

File tree

unified/extractor/ast_types.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ named:
413413
name_pattern:
414414
modifier*: modifier
415415
identifier: identifier
416+
sub_pattern?: pattern
416417

417418
# A pattern matching anything, binding no variables, usually using the syntax "_"
418419
ignore_pattern:

unified/extractor/src/languages/swift/swift.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,13 +1023,12 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
10231023
path: (importPathComponent name: @@parts)*)
10241024
=>
10251025
import_declaration {
1026-
let pattern = match kind {
1027-
Some(_) => {
1028-
let last = *parts.last().ok_or("import has no path")?;
1029-
tree!((name_pattern identifier: (identifier #{last})))
1030-
}
1031-
None => tree!((bulk_importing_pattern)),
1026+
let bulk_import = match kind {
1027+
None => Some(tree!((bulk_importing_pattern))),
1028+
Some(_) => None, // scoped import, no bulk import
10321029
};
1030+
let last = *parts.last().ok_or("import has no path")?;
1031+
let pattern = tree!((name_pattern identifier: (identifier #{last}) sub_pattern: {bulk_import}));
10331032
tree!((import_declaration
10341033
modifier: (modifier #{kind})?
10351034
modifier: {attrs}

unified/extractor/tests/corpus/swift/desugar/import-with-deeply-nested-path-three-parts.output

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,7 @@ top_level
3737
identifier: identifier "Foundation"
3838
member: identifier "Networking"
3939
member: identifier "URLSession"
40-
pattern: bulk_importing_pattern "import Foundation.Networking.URLSession"
40+
pattern:
41+
name_pattern
42+
identifier: identifier "URLSession"
43+
sub_pattern: bulk_importing_pattern "import Foundation.Networking.URLSession"

unified/extractor/tests/corpus/swift/desugar/import-with-dotted-path-two-parts.output

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,7 @@ top_level
3131
name_expr
3232
identifier: identifier "Foundation"
3333
member: identifier "Networking"
34-
pattern: bulk_importing_pattern "import Foundation.Networking"
34+
pattern:
35+
name_pattern
36+
identifier: identifier "Networking"
37+
sub_pattern: bulk_importing_pattern "import Foundation.Networking"

unified/extractor/tests/corpus/swift/desugar/simple-import-with-single-name.output

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,7 @@ top_level
2525
imported_expr:
2626
name_expr
2727
identifier: identifier "Foundation"
28-
pattern: bulk_importing_pattern "import Foundation"
28+
pattern:
29+
name_pattern
30+
identifier: identifier "Foundation"
31+
sub_pattern: bulk_importing_pattern "import Foundation"

unified/ql/lib/codeql/unified/internal/Ast.qll

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1078,9 +1078,14 @@ module Unified {
10781078
/** Gets the node corresponding to the field `modifier`. */
10791079
final F::Modifier getAModifier() { result = this.getModifier(_) }
10801080

1081+
/** Gets the node corresponding to the field `sub_pattern`. */
1082+
final F::Pattern getSubPattern() { unified_name_pattern_sub_pattern(this, result) }
1083+
10811084
/** Gets a field or child node of this node. */
10821085
final override F::AstNode getAFieldOrChild() {
1083-
unified_name_pattern_def(this, result) or unified_name_pattern_modifier(this, _, result)
1086+
unified_name_pattern_def(this, result) or
1087+
unified_name_pattern_modifier(this, _, result) or
1088+
unified_name_pattern_sub_pattern(this, result)
10841089
}
10851090
}
10861091

@@ -1895,6 +1900,8 @@ module Unified {
18951900
or
18961901
result = node.(NamePattern).getModifier(i) and name = "getModifier"
18971902
or
1903+
result = node.(NamePattern).getSubPattern() and i = -1 and name = "getSubPattern"
1904+
or
18981905
result = node.(NamedTypeExpr).getName() and i = -1 and name = "getName"
18991906
or
19001907
result = node.(NamedTypeExpr).getQualifier() and i = -1 and name = "getQualifier"

unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,6 @@ private module LocalNameBindingInput implements LocalNameBindingInputSig<Locatio
323323
// because the associated uncertain scope is a Member, from which we can check static-ness.
324324
scope = any(ClassLikeDeclaration cls).getAMember()
325325
or
326-
scope = any(TopLevel t) // Global module names are in scope here
327-
or
328326
scope = any(TopLevel t).getBody() // Imported names are in scope here
329327
or
330328
// Scopes with a bulk-import have uncertain members

unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ abstract class ModuleScopeRepr extends AstNode {
5454
predicate shouldInclude(Container c, string path) { none() }
5555

5656
/**
57-
* Holds if this module scope can be referenced by an unqualified identifier `name`.
57+
* Holds if this module scope can be referenced by an identifier `name`
58+
* appearing as the leading identifier of an import path.
5859
*/
5960
predicate hasImportableName(string name) { none() }
6061

unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ private newtype TNameBindingNode =
1212
TLocalName(LocalName local) or
1313
TExportedNamespace(ClassLikeDeclaration cls) or
1414
TLocalNamespace(AstNode n) {
15-
n = any(TopLevel t) or // Module names come in scope here
16-
n = any(TopLevel t).getBody() or // Imported names come in scope here (shadowing module names)
15+
n = any(TopLevel t).getBody() or // Imported names come in scope here
1716
n instanceof ClassLikeDeclaration
1817
} or
1918
TModuleScope(ModuleScopeRepr repr) or
@@ -129,6 +128,13 @@ predicate readStep(NameBindingNode node1, string name, NameBindingNode node2) {
129128
node1 = getNodeFromUncertainScope(LocalNameBindingOutput::getAnUncertainScope(access, name)) and
130129
node2.isIdentifier(access)
131130
)
131+
or
132+
exists(NameExpr expr |
133+
isImportPrefix(expr) and
134+
node1.isModuleRoot() and
135+
name = expr.getIdentifier().getValue() and
136+
node2 = getNodeFromRef(expr)
137+
)
132138
}
133139

134140
predicate storeStep(NameBindingNode node1, string name, NameBindingNode node2) {
@@ -181,9 +187,6 @@ predicate valueStep(NameBindingNode node1, NameBindingNode node2) {
181187
)
182188
or
183189
exists(TopLevel top |
184-
node1.isModuleRoot() and
185-
node2.isLocalNamespace(top) // module names in outermost scope
186-
or
187190
node1 = getModuleNodeFromFile(top.getFile()) and
188191
node2.isLocalNamespace(top.getBody()) // implicitly import own module
189192
)
@@ -206,6 +209,20 @@ predicate valueStep(NameBindingNode node1, NameBindingNode node2) {
206209
node2 = getModuleNodeFromFile(top.getFile())
207210
)
208211
)
212+
or
213+
exists(NamePattern p |
214+
node1 = getNodeFromRef(p) and
215+
node2 = getNodeFromRef(p.getSubPattern())
216+
)
217+
}
218+
219+
private predicate isImportPrefix(Expr e) {
220+
e = any(ImportDeclaration impr).getImportedExpr()
221+
or
222+
exists(MemberAccessExpr member |
223+
isImportPrefix(member) and
224+
e = member.getBase()
225+
)
209226
}
210227

211228
predicate inheritanceStep(NameBindingNode supertype, NameBindingNode subtype) {

unified/ql/lib/unified.dbscheme

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,11 @@ unified_name_pattern_modifier(
687687
unique int modifier: @unified_token_modifier ref
688688
);
689689

690+
unified_name_pattern_sub_pattern(
691+
unique int unified_name_pattern: @unified_name_pattern ref,
692+
unique int sub_pattern: @unified_pattern ref
693+
);
694+
690695
unified_name_pattern_def(
691696
unique int id: @unified_name_pattern,
692697
int identifier: @unified_token_identifier ref

0 commit comments

Comments
 (0)