diff --git a/change-notes/1.19/analysis-javascript.md b/change-notes/1.19/analysis-javascript.md index 13fbc13a0b1b..a3ca0461a1f7 100644 --- a/change-notes/1.19/analysis-javascript.md +++ b/change-notes/1.19/analysis-javascript.md @@ -9,6 +9,7 @@ * Support for popular libraries has been improved. Consequently, queries may produce more results on code bases that use the following features: - file system access, for example through [fs-extra](https://github.com/jprichardson/node-fs-extra) or [globby](https://www.npmjs.com/package/globby) +* The type inference now handles nested imports (that is, imports not appearing at the toplevel). This may yield fewer false-positive results on projects that use this non-standard language feature. ## New queries diff --git a/javascript/ql/src/semmle/javascript/CFG.qll b/javascript/ql/src/semmle/javascript/CFG.qll index faec8deca6d8..c70b0dd15116 100644 --- a/javascript/ql/src/semmle/javascript/CFG.qll +++ b/javascript/ql/src/semmle/javascript/CFG.qll @@ -281,7 +281,7 @@ import javascript */ class ControlFlowNode extends @cfg_node, Locatable { /** Gets a node succeeding this node in the CFG. */ - ControlFlowNode getASuccessor() { + cached ControlFlowNode getASuccessor() { successor(this, result) } @@ -457,3 +457,47 @@ class ConcreteControlFlowNode extends ControlFlowNode { not this instanceof SyntheticControlFlowNode } } + +/** + * A CFG node corresponding to a nested (that is, non-toplevel) import declaration. + * + * This is a non-standard language feature that is not currently supported very well + * by the extractor, in particular such imports do not appear in the control flow graph + * generated by the extractor. We patch them in by overriding `getASuccessor`; once an + * extractor fix becomes available, this class can be removed. + */ +private class NestedImportDeclaration extends ImportDeclaration { + NestedImportDeclaration() { + exists (ASTNode p | p = getParent() | + not p instanceof TopLevel + ) and + // if there are no specifiers, the default control flow graph is fine + exists(getASpecifier()) + } + + override ControlFlowNode getASuccessor() { + result = getSpecifier(0).getFirstControlFlowNode() + } +} + +/** + * A CFG node corresponding to an import specifier in a nested import declaration. + * + * As for `NestedImportDeclaration` above, this is a temporary workaround that will be + * removed once extractor support for this non-standard language feature becomes available. + */ +private class NestedImportSpecifier extends ImportSpecifier { + NestedImportDeclaration decl; + int i; + + NestedImportSpecifier() { + this = decl.getSpecifier(i) + } + + override ControlFlowNode getASuccessor() { + result = decl.getSpecifier(i+1).getFirstControlFlowNode() + or + not exists(decl.getSpecifier(i+1)) and + successor(decl, result) + } +} diff --git a/javascript/ql/src/semmle/javascript/ES2015Modules.qll b/javascript/ql/src/semmle/javascript/ES2015Modules.qll index f5cf676f3d50..4cfa9dcb04d8 100644 --- a/javascript/ql/src/semmle/javascript/ES2015Modules.qll +++ b/javascript/ql/src/semmle/javascript/ES2015Modules.qll @@ -27,7 +27,7 @@ class ES2015Module extends Module { /** Gets an export declaration in this module. */ ExportDeclaration getAnExport() { - result.getContainer() = this + result.getTopLevel() = this } override Module getAnImportedModule() { @@ -55,7 +55,7 @@ class ES2015Module extends Module { /** An import declaration. */ class ImportDeclaration extends Stmt, Import, @importdeclaration { override ES2015Module getEnclosingModule() { - result = getContainer() + result = getTopLevel() } override PathExprInModule getImportedPath() { @@ -254,7 +254,7 @@ class BulkReExportDeclaration extends ReExportDeclaration, @exportalldeclaration * but we ignore this subtlety. */ private predicate isShadowedFromBulkExport(BulkReExportDeclaration reExport, string name) { - exists (ExportNamedDeclaration other | other.getContainer() = reExport.getEnclosingModule() | + exists (ExportNamedDeclaration other | other.getTopLevel() = reExport.getEnclosingModule() | other.getAnExportedDecl().getName() = name or other.getASpecifier().getExportedName() = name) diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll index 6efa6f9b0cf9..77359efb216d 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll @@ -301,7 +301,7 @@ private class AnalyzedExportAssign extends AnalyzedPropertyWrite, DataFlow::Valu } override predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source) { - baseVal = TAbstractModuleObject(exportAssign.getContainer()) and + baseVal = TAbstractModuleObject(exportAssign.getTopLevel()) and propName = "exports" and source = this } diff --git a/javascript/ql/test/library-tests/Flow/AbstractValues.expected b/javascript/ql/test/library-tests/Flow/AbstractValues.expected index 0f3244a73cbd..77911a902153 100644 --- a/javascript/ql/test/library-tests/Flow/AbstractValues.expected +++ b/javascript/ql/test/library-tests/Flow/AbstractValues.expected @@ -180,6 +180,10 @@ | n.js:3:16:3:23 | object literal | | namespace-reexport.js:1:1:4:0 | exports object of module namespace-reexport | | namespace-reexport.js:1:1:4:0 | module object of module namespace-reexport | +| nestedImport.js:1:1:13:0 | exports object of module nestedImport | +| nestedImport.js:1:1:13:0 | module object of module nestedImport | +| nestedImport.js:9:1:12:1 | function tst | +| nestedImport.js:9:1:12:1 | instance of function tst | | nodeJsClient.js:1:1:6:0 | exports object of module nodeJsClient | | nodeJsClient.js:1:1:6:0 | module object of module nodeJsClient | | nodeJsLib.js:1:1:4:0 | exports object of module nodeJsLib | diff --git a/javascript/ql/test/library-tests/Flow/abseval.expected b/javascript/ql/test/library-tests/Flow/abseval.expected index dbc1a06ebfe4..86e8334adc79 100644 --- a/javascript/ql/test/library-tests/Flow/abseval.expected +++ b/javascript/ql/test/library-tests/Flow/abseval.expected @@ -54,6 +54,7 @@ | b.js:42:5:42:7 | z11 | b.js:42:11:42:18 | toString | file://:0:0:0:0 | indefinite value (import) | | b.js:45:5:45:7 | z12 | b.js:45:11:45:12 | f2 | ts2.ts:1:1:6:0 | exports object of module ts2 | | b.js:45:5:45:7 | z12 | b.js:45:11:45:12 | f2 | ts2.ts:1:10:1:22 | anonymous function | +| b.js:45:5:45:7 | z12 | b.js:45:11:45:12 | f2 | ts2.ts:4:12:4:13 | object literal | | b.js:48:5:48:7 | z13 | b.js:48:11:48:11 | w | file://:0:0:0:0 | non-empty, non-numeric string | | b.js:51:5:51:7 | z14 | b.js:51:11:51:24 | foo_reexported | file://:0:0:0:0 | indefinite value (import) | | b.js:54:5:54:7 | z15 | b.js:54:11:54:19 | something | file://:0:0:0:0 | indefinite value (import) | @@ -171,6 +172,10 @@ | mixed.js:9:5:9:6 | fn | mixed.js:9:10:9:19 | __filename | file://:0:0:0:0 | numeric string | | mixed.js:10:5:10:6 | dn | mixed.js:10:10:10:18 | __dirname | file://:0:0:0:0 | non-empty, non-numeric string | | mixed.js:10:5:10:6 | dn | mixed.js:10:10:10:18 | __dirname | file://:0:0:0:0 | numeric string | +| nestedImport.js:2:5:2:6 | x1 | nestedImport.js:2:10:2:12 | foo | esLib.js:3:8:3:24 | function foo | +| nestedImport.js:6:7:6:8 | x2 | nestedImport.js:6:12:6:14 | foo | file://:0:0:0:0 | indefinite value (import) | +| nestedImport.js:6:7:6:8 | x2 | nestedImport.js:6:12:6:14 | foo | nodeJsLib.js:3:15:3:37 | function nodeJsFoo | +| nestedImport.js:11:7:11:8 | x3 | nestedImport.js:11:12:11:14 | foo | esLib.js:3:8:3:24 | function foo | | nodeJsClient.js:1:5:1:6 | nj | nodeJsClient.js:1:10:1:31 | require ... JsLib') | file://:0:0:0:0 | indefinite value (call) | | nodeJsClient.js:1:5:1:6 | nj | nodeJsClient.js:1:10:1:31 | require ... JsLib') | nodeJsLib.js:1:1:4:0 | exports object of module nodeJsLib | | nodeJsClient.js:1:5:1:6 | nj | nodeJsClient.js:1:10:1:31 | require ... JsLib') | nodeJsLib.js:1:18:1:43 | function nodeJsModule | diff --git a/javascript/ql/test/library-tests/Flow/nestedImport.js b/javascript/ql/test/library-tests/Flow/nestedImport.js new file mode 100644 index 000000000000..c52953db2cfb --- /dev/null +++ b/javascript/ql/test/library-tests/Flow/nestedImport.js @@ -0,0 +1,12 @@ +import { foo } from './esLib'; +let x1 = foo; + +if (!foo) { + import { foo } from './nodeJsLib'; + let x2 = foo; +} + +function tst() { + import { foo } from './esLib'; + let x3 = foo; +} diff --git a/javascript/ql/test/library-tests/Flow/types.expected b/javascript/ql/test/library-tests/Flow/types.expected index 786fef8fcb96..f0aef176a107 100644 --- a/javascript/ql/test/library-tests/Flow/types.expected +++ b/javascript/ql/test/library-tests/Flow/types.expected @@ -98,6 +98,9 @@ | mixed.js:8:5:8:7 | exp | mixed.js:8:11:8:17 | exports | object | | mixed.js:9:5:9:6 | fn | mixed.js:9:10:9:19 | __filename | string | | mixed.js:10:5:10:6 | dn | mixed.js:10:10:10:18 | __dirname | string | +| nestedImport.js:2:5:2:6 | x1 | nestedImport.js:2:10:2:12 | foo | function | +| nestedImport.js:6:7:6:8 | x2 | nestedImport.js:6:12:6:14 | foo | boolean, class, date, function, null, number, object, regular expression,string or undefined | +| nestedImport.js:11:7:11:8 | x3 | nestedImport.js:11:12:11:14 | foo | function | | nodeJsClient.js:1:5:1:6 | nj | nodeJsClient.js:1:10:1:31 | require ... JsLib') | boolean, class, date, function, null, number, object, regular expression,string or undefined | | nodeJsClient.js:2:5:2:6 | es | nodeJsClient.js:2:10:2:27 | require('./esLib') | boolean, class, date, function, null, number, object, regular expression,string or undefined | | nodeJsClient.js:4:5:4:6 | x1 | nodeJsClient.js:4:10:4:15 | nj.foo | boolean, class, date, function, null, number, object, regular expression,string or undefined |