Skip to content

Commit 2283195

Browse files
authored
Merge pull request #1871 from asger-semmle/type-tracking-through-imports
Approved by xiemaisi
2 parents 22e1715 + 65862c9 commit 2283195

File tree

16 files changed

+93
-27
lines changed

16 files changed

+93
-27
lines changed

javascript/ql/src/Declarations/DeadStoreOfLocal.ql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ where
3636
exists(v.getAnAccess()) and
3737
// don't flag ambient variable definitions
3838
not dead.(ASTNode).isAmbient() and
39+
// don't flag variables with ambient uses
40+
not exists(LexicalAccess access |
41+
access.getALexicalName() = v.getADeclaration().getALexicalName() and
42+
access.isAmbient()
43+
) and
3944
// don't flag function expressions
4045
not exists(FunctionExpr fe | dead = fe.getId()) and
4146
// don't flag function declarations nested inside blocks or other compound statements;

javascript/ql/src/semmle/javascript/DefUse.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ private predicate defn(ControlFlowNode def, Expr lhs, AST::ValueNode rhs) {
4040
or
4141
exists(ImportEqualsDeclaration i | def = i | lhs = i.getId() and rhs = i.getImportedEntity())
4242
or
43+
exists(ImportSpecifier i | def = i | lhs = i.getLocal() and rhs = i)
44+
or
4345
exists(EnumMember member | def = member.getIdentifier() |
4446
lhs = def and rhs = member.getInitializer()
4547
)
@@ -71,8 +73,6 @@ private predicate defn(ControlFlowNode def, Expr lhs) {
7173
or
7274
lhs = def.(UpdateExpr).getOperand().getUnderlyingReference()
7375
or
74-
lhs = def.(ImportSpecifier).getLocal()
75-
or
7676
exists(EnhancedForLoop efl | def = efl.getIteratorExpr() |
7777
lhs = def.(Expr).stripParens() or
7878
lhs = def.(VariableDeclarator).getBindingPattern()

javascript/ql/src/semmle/javascript/ES2015Modules.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class ImportDeclaration extends Stmt, Import, @importdeclaration {
6565
// `import * as http from 'http'` or `import http from `http`'
6666
exists(ImportSpecifier is |
6767
is = getASpecifier() and
68-
result = DataFlow::ssaDefinitionNode(SSA::definition(is))
68+
result = DataFlow::valueNode(is)
6969
|
7070
is instanceof ImportNamespaceSpecifier and
7171
count(getASpecifier()) = 1

javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -799,21 +799,21 @@ module DataFlow {
799799
/**
800800
* A named import specifier seen as a property read on the imported module.
801801
*/
802-
private class ImportSpecifierAsPropRead extends PropRead {
802+
private class ImportSpecifierAsPropRead extends PropRead, ValueNode {
803+
override ImportSpecifier astNode;
804+
803805
ImportDeclaration imprt;
804-
ImportSpecifier spec;
805806

806807
ImportSpecifierAsPropRead() {
807-
spec = imprt.getASpecifier() and
808-
exists(spec.getImportedName()) and
809-
this = ssaDefinitionNode(SSA::definition(spec))
808+
astNode = imprt.getASpecifier() and
809+
exists(astNode.getImportedName())
810810
}
811811

812812
override Node getBase() { result = TDestructuredModuleImportNode(imprt) }
813813

814-
override Expr getPropertyNameExpr() { result = spec.getImported() }
814+
override Expr getPropertyNameExpr() { result = astNode.getImported() }
815815

816-
override string getPropertyName() { result = spec.getImportedName() }
816+
override string getPropertyName() { result = astNode.getImportedName() }
817817
}
818818

819819
/**

javascript/ql/src/semmle/javascript/dataflow/Sources.qll

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,11 +234,10 @@ module SourceNode {
234234
astNode instanceof AwaitExpr or
235235
astNode instanceof FunctionSentExpr or
236236
astNode instanceof FunctionBindExpr or
237-
astNode instanceof DynamicImportExpr
237+
astNode instanceof DynamicImportExpr or
238+
astNode instanceof ImportSpecifier
238239
)
239240
or
240-
this = DataFlow::ssaDefinitionNode(SSA::definition(any(ImportSpecifier imp)))
241-
or
242241
DataFlow::parameterNode(this, _)
243242
or
244243
this instanceof DataFlow::Impl::InvokeNodeDef

javascript/ql/test/library-tests/DataFlow/flowStep.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
| sources.js:5:4:5:5 | 23 | sources.js:3:11:3:11 | x |
1010
| tst.js:1:1:1:1 | x | tst.js:28:2:28:1 | x |
1111
| tst.js:1:1:1:1 | x | tst.js:32:1:32:0 | x |
12+
| tst.js:1:10:1:11 | fs | tst.js:1:10:1:11 | fs |
1213
| tst.js:1:10:1:11 | fs | tst.js:7:1:7:2 | fs |
1314
| tst.js:1:10:1:11 | fs | tst.js:22:24:22:25 | fs |
1415
| tst.js:3:5:3:10 | x | tst.js:8:1:8:1 | x |

javascript/ql/test/library-tests/DefUse/VarDefSource.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
| classes.js:7:5:8:5 | class L ... {\\n } | classes.js:7:5:8:5 | class L ... {\\n } |
44
| classes.js:10:5:10:31 | LocalFo ... ar = 42 | classes.js:10:30:10:31 | 42 |
55
| es2015.js:4:10:4:19 | getSquares | es2015.js:4:1:6:1 | functio ... i*i];\\n} |
6+
| es2015modules.js:1:10:1:12 | foo | es2015modules.js:1:10:1:12 | foo |
7+
| es2015modules.js:1:15:1:24 | bar as baz | es2015modules.js:1:15:1:24 | bar as baz |
8+
| es2015modules.js:10:10:10:13 | quux | es2015modules.js:10:10:10:13 | quux |
69
| es2015modules.js:15:17:15:17 | f | es2015modules.js:15:8:15:22 | function f() {} |
710
| es2015modules.js:16:25:16:25 | g | es2015modules.js:16:16:16:30 | function g() {} |
811
| fundecls.js:1:10:1:10 | s | fundecls.js:1:1:5:1 | functio ... f();\\n} |

javascript/ql/test/library-tests/ModuleImportNodes/TSGlobalImport.expected

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,14 @@
66
| client2_lazy.ts:4:28:4:29 | F2 | framework2 |
77
| declare-module-client2.ts:3:1:3:22 | import ... 'foo'; | foo |
88
| declare-module-client.ts:3:1:3:22 | import ... 'foo'; | foo |
9+
| decls.ts:2:8:2:20 | * as F1_outer | framework1 |
10+
| decls.ts:3:8:3:20 | * as F2_outer | framework2 |
11+
| decls.ts:4:8:4:21 | * as net_outer | net |
912
| destructuringES6.js:1:1:1:41 | import ... ctron'; | electron |
1013
| destructuringRequire.js:1:27:1:45 | require('electron') | electron |
1114
| instanceThroughDefaultImport.js:1:1:1:82 | import ... tance'; | myDefaultImportedModuleInstance |
12-
| instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName | myDefaultImportedModuleInstance |
13-
| instanceThroughNamespaceImport.js:1:8:1:49 | myNamespaceImportedModuleInstanceName | myNamespaceImportedModuleInstance |
15+
| instanceThroughDefaultImport.js:1:8:1:42 | myDefau ... nceName | myDefaultImportedModuleInstance |
16+
| instanceThroughNamespaceImport.js:1:8:1:49 | * as my ... nceName | myNamespaceImportedModuleInstance |
1417
| instanceThroughRequire.js:1:36:1:70 | require ... tance') | myRequiredModuleInstance |
1518
| moduleUses.js:1:11:1:24 | require('mod') | mod |
1619
| process2.js:1:1:1:13 | require('fs') | fs |

javascript/ql/test/library-tests/ModuleImportNodes/tests.expected

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ test_ModuleImportNode
55
| client1.ts:6:9:6:11 | net | net | client1.ts:6:9:6:11 | net | net |
66
| client2.ts:4:28:4:29 | F2 | framework2 | client2.ts:4:28:4:29 | F2 | F2 |
77
| client2_lazy.ts:4:28:4:29 | F2 | framework2 | client2_lazy.ts:4:28:4:29 | F2 | F2 |
8-
| instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName | myDefaultImportedModuleInstance | instanceThroughDefaultImport.js:2:1:2:35 | myDefau ... nceName | myDefaultImportedModuleInstanceName |
9-
| instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName | myDefaultImportedModuleInstance | instanceThroughDefaultImport.js:4:5:4:39 | myDefau ... nceName | myDefaultImportedModuleInstanceName |
10-
| instanceThroughNamespaceImport.js:1:8:1:49 | myNamespaceImportedModuleInstanceName | myNamespaceImportedModuleInstance | instanceThroughNamespaceImport.js:2:1:2:37 | myNames ... nceName | myNamespaceImportedModuleInstanceName |
11-
| instanceThroughNamespaceImport.js:1:8:1:49 | myNamespaceImportedModuleInstanceName | myNamespaceImportedModuleInstance | instanceThroughNamespaceImport.js:4:5:4:41 | myNames ... nceName | myNamespaceImportedModuleInstanceName |
8+
| instanceThroughDefaultImport.js:1:8:1:42 | myDefau ... nceName | myDefaultImportedModuleInstance | instanceThroughDefaultImport.js:2:1:2:35 | myDefau ... nceName | myDefaultImportedModuleInstanceName |
9+
| instanceThroughDefaultImport.js:1:8:1:42 | myDefau ... nceName | myDefaultImportedModuleInstance | instanceThroughDefaultImport.js:4:5:4:39 | myDefau ... nceName | myDefaultImportedModuleInstanceName |
10+
| instanceThroughNamespaceImport.js:1:8:1:49 | * as my ... nceName | myNamespaceImportedModuleInstance | instanceThroughNamespaceImport.js:2:1:2:37 | myNames ... nceName | myNamespaceImportedModuleInstanceName |
11+
| instanceThroughNamespaceImport.js:1:8:1:49 | * as my ... nceName | myNamespaceImportedModuleInstance | instanceThroughNamespaceImport.js:4:5:4:41 | myNames ... nceName | myNamespaceImportedModuleInstanceName |
1212
| instanceThroughRequire.js:1:36:1:70 | require ... tance') | myRequiredModuleInstance | instanceThroughRequire.js:2:1:2:28 | myRequi ... nceName | myRequiredModuleInstanceName |
1313
| instanceThroughRequire.js:1:36:1:70 | require ... tance') | myRequiredModuleInstance | instanceThroughRequire.js:4:5:4:32 | myRequi ... nceName | myRequiredModuleInstanceName |
1414
| moduleUses.js:1:11:1:24 | require('mod') | mod | moduleUses.js:3:1:3:3 | mod | mod |
@@ -31,22 +31,27 @@ test_moduleImport
3131
| foo | declare-module-client2.ts:3:1:3:22 | import ... 'foo'; |
3232
| foo | declare-module-client.ts:3:1:3:22 | import ... 'foo'; |
3333
| framework1 | client1.ts:4:28:4:29 | F1 |
34+
| framework1 | decls.ts:2:8:2:20 | * as F1_outer |
3435
| framework2 | client2.ts:4:28:4:29 | F2 |
3536
| framework2 | client2_lazy.ts:4:28:4:29 | F2 |
37+
| framework2 | decls.ts:3:8:3:20 | * as F2_outer |
3638
| fs | amd1.js:1:25:1:26 | fs |
3739
| fs | amd2.js:2:12:2:24 | require('fs') |
3840
| fs | process2.js:1:1:1:13 | require('fs') |
3941
| mod | moduleUses.js:1:11:1:24 | require('mod') |
4042
| myDefaultImportedModuleInstance | instanceThroughDefaultImport.js:1:1:1:82 | import ... tance'; |
41-
| myDefaultImportedModuleInstance | instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName |
42-
| myNamespaceImportedModuleInstance | instanceThroughNamespaceImport.js:1:8:1:49 | myNamespaceImportedModuleInstanceName |
43+
| myDefaultImportedModuleInstance | instanceThroughDefaultImport.js:1:8:1:42 | myDefau ... nceName |
44+
| myNamespaceImportedModuleInstance | instanceThroughNamespaceImport.js:1:8:1:49 | * as my ... nceName |
4345
| myRequiredModuleInstance | instanceThroughRequire.js:1:36:1:70 | require ... tance') |
4446
| net | client1.ts:6:9:6:11 | net |
47+
| net | decls.ts:4:8:4:21 | * as net_outer |
4548
| process | process2.js:2:10:2:16 | process |
4649
| process | process.js:1:10:1:27 | require('process') |
4750
test_moduleMember
4851
| electron | BrowserWindow | destructuringES6.js:1:10:1:22 | BrowserWindow |
4952
| electron | BrowserWindow | destructuringRequire.js:1:9:1:21 | BrowserWindow |
53+
| foo | C | declare-module-client2.ts:3:9:3:9 | C |
54+
| foo | C | declare-module-client.ts:3:9:3:9 | C |
5055
| framework1 | Component | client1.ts:4:28:4:39 | F1.Component |
5156
| framework2 | Component | client2.ts:4:28:4:39 | F2.Component |
5257
| framework2 | Component | client2_lazy.ts:4:28:4:39 | F2.Component |
@@ -56,7 +61,7 @@ test_moduleMember
5661
| mod | moduleField | moduleUses.js:11:1:11:15 | mod.moduleField |
5762
| mod | moduleFunction | moduleUses.js:5:9:5:26 | mod.moduleFunction |
5863
| mod | moduleMethod | moduleUses.js:3:1:3:16 | mod.moduleMethod |
59-
| myDefaultImportedModuleInstance | default | instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName |
64+
| myDefaultImportedModuleInstance | default | instanceThroughDefaultImport.js:1:8:1:42 | myDefau ... nceName |
6065
| net | createServer | client1.ts:6:9:6:24 | net.createServer |
6166
test_ModuleImportNode_getPath
6267
| amd1.js:1:25:1:26 | fs | fs |
@@ -67,11 +72,14 @@ test_ModuleImportNode_getPath
6772
| client2_lazy.ts:4:28:4:29 | F2 | framework2 |
6873
| declare-module-client2.ts:3:1:3:22 | import ... 'foo'; | foo |
6974
| declare-module-client.ts:3:1:3:22 | import ... 'foo'; | foo |
75+
| decls.ts:2:8:2:20 | * as F1_outer | framework1 |
76+
| decls.ts:3:8:3:20 | * as F2_outer | framework2 |
77+
| decls.ts:4:8:4:21 | * as net_outer | net |
7078
| destructuringES6.js:1:1:1:41 | import ... ctron'; | electron |
7179
| destructuringRequire.js:1:27:1:45 | require('electron') | electron |
7280
| instanceThroughDefaultImport.js:1:1:1:82 | import ... tance'; | myDefaultImportedModuleInstance |
73-
| instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName | myDefaultImportedModuleInstance |
74-
| instanceThroughNamespaceImport.js:1:8:1:49 | myNamespaceImportedModuleInstanceName | myNamespaceImportedModuleInstance |
81+
| instanceThroughDefaultImport.js:1:8:1:42 | myDefau ... nceName | myDefaultImportedModuleInstance |
82+
| instanceThroughNamespaceImport.js:1:8:1:49 | * as my ... nceName | myNamespaceImportedModuleInstance |
7583
| instanceThroughRequire.js:1:36:1:70 | require ... tance') | myRequiredModuleInstance |
7684
| moduleUses.js:1:11:1:24 | require('mod') | mod |
7785
| process2.js:1:1:1:13 | require('fs') | fs |
@@ -97,6 +105,8 @@ test_ModuleImportNode_getAMemberInvocation
97105
test_moduleImportProp
98106
| electron | BrowserWindow | destructuringES6.js:1:10:1:22 | BrowserWindow |
99107
| electron | BrowserWindow | destructuringRequire.js:1:9:1:21 | BrowserWindow |
108+
| foo | C | declare-module-client2.ts:3:9:3:9 | C |
109+
| foo | C | declare-module-client.ts:3:9:3:9 | C |
100110
| framework1 | Component | client1.ts:4:28:4:39 | F1.Component |
101111
| framework2 | Component | client2.ts:4:28:4:39 | F2.Component |
102112
| framework2 | Component | client2_lazy.ts:4:28:4:39 | F2.Component |
@@ -106,7 +116,7 @@ test_moduleImportProp
106116
| mod | moduleField | moduleUses.js:11:1:11:15 | mod.moduleField |
107117
| mod | moduleFunction | moduleUses.js:5:9:5:26 | mod.moduleFunction |
108118
| mod | moduleMethod | moduleUses.js:3:1:3:16 | mod.moduleMethod |
109-
| myDefaultImportedModuleInstance | default | instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName |
119+
| myDefaultImportedModuleInstance | default | instanceThroughDefaultImport.js:1:8:1:42 | myDefau ... nceName |
110120
| net | createServer | client1.ts:6:9:6:24 | net.createServer |
111121
test_ModuleImportNode_getAMemberCall
112122
| amd1.js:1:25:1:26 | fs | amd1.js:2:3:2:29 | fs.read ... a.txt") |
@@ -121,10 +131,12 @@ test_ModuleImportNode_getAPropertyRead
121131
| client1.ts:6:9:6:11 | net | client1.ts:6:9:6:24 | net.createServer |
122132
| client2.ts:4:28:4:29 | F2 | client2.ts:4:28:4:39 | F2.Component |
123133
| client2_lazy.ts:4:28:4:29 | F2 | client2_lazy.ts:4:28:4:39 | F2.Component |
134+
| declare-module-client2.ts:3:1:3:22 | import ... 'foo'; | declare-module-client2.ts:3:9:3:9 | C |
135+
| declare-module-client.ts:3:1:3:22 | import ... 'foo'; | declare-module-client.ts:3:9:3:9 | C |
124136
| destructuringES6.js:1:1:1:41 | import ... ctron'; | destructuringES6.js:1:10:1:22 | BrowserWindow |
125137
| destructuringRequire.js:1:27:1:45 | require('electron') | destructuringRequire.js:1:9:1:21 | BrowserWindow |
126-
| instanceThroughDefaultImport.js:1:1:1:82 | import ... tance'; | instanceThroughDefaultImport.js:1:8:1:42 | myDefaultImportedModuleInstanceName |
138+
| instanceThroughDefaultImport.js:1:1:1:82 | import ... tance'; | instanceThroughDefaultImport.js:1:8:1:42 | myDefau ... nceName |
127139
| moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:3:1:3:16 | mod.moduleMethod |
128140
| moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:5:9:5:26 | mod.moduleFunction |
129141
| moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:8:9:8:31 | mod.con ... unction |
130-
| moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:11:1:11:15 | mod.moduleField |
142+
| moduleUses.js:1:11:1:24 | require('mod') | moduleUses.js:11:1:11:15 | mod.moduleField |

javascript/ql/test/library-tests/TypeTracking/ClassStyle.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ test_ApiObject
1010
| tst_conflict.js:6:38:6:49 | api.chain1() |
1111
| tst_conflict.js:6:38:6:58 | api.cha ... hain2() |
1212
test_Connection
13+
| client.js:1:10:1:27 | exportedConnection |
1314
| tst.js:7:15:7:18 | conn |
1415
| tst.js:11:5:11:19 | this.connection |
1516
| tst.js:16:10:16:49 | api.cha ... ction() |
@@ -22,8 +23,10 @@ test_Connection
2223
| tst.js:62:40:62:79 | api.cha ... ction() |
2324
| tst.js:63:38:63:77 | api.cha ... ction() |
2425
| tst.js:67:14:67:47 | MyAppli ... nection |
26+
| tst.js:78:35:78:49 | getConnection() |
2527
| tst_conflict.js:6:38:6:77 | api.cha ... ction() |
2628
test_DataCallback
29+
| client.js:3:28:3:34 | x => {} |
2730
| tst.js:10:11:10:12 | cb |
2831
| tst.js:21:1:23:1 | functio ... ata);\\n} |
2932
| tst.js:30:26:30:27 | cb |
@@ -35,6 +38,7 @@ test_DataCallback
3538
| tst.js:58:16:58:22 | x => {} |
3639
| tst.js:68:16:70:3 | data => ... a);\\n } |
3740
test_DataValue
41+
| client.js:3:28:3:28 | x |
3842
| tst.js:21:18:21:21 | data |
3943
| tst.js:25:19:25:22 | data |
4044
| tst.js:33:17:33:20 | data |

0 commit comments

Comments
 (0)