From 6bcae5e7c2bf8235f2ef39b35604f249ea8d02d4 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 24 Jan 2019 11:43:38 +0000 Subject: [PATCH] JS: address comments --- .../com/semmle/js/extractor/ASTExtractor.java | 8 +++---- .../semmle/js/extractor/ExtractorConfig.java | 21 ++++++++++++++++++- .../semmle/js/extractor/HTMLExtractor.java | 4 ++-- .../com/semmle/js/extractor/JSExtractor.java | 4 ++-- .../semmle/js/extractor/ScriptExtractor.java | 2 +- .../ql/src/semmle/javascript/Closure.qll | 11 +++++----- .../internal/InterModuleTypeInference.qll | 5 +++-- 7 files changed, 37 insertions(+), 18 deletions(-) diff --git a/javascript/extractor/src/com/semmle/js/extractor/ASTExtractor.java b/javascript/extractor/src/com/semmle/js/extractor/ASTExtractor.java index ddba9c50157..d5a18d24a6f 100644 --- a/javascript/extractor/src/com/semmle/js/extractor/ASTExtractor.java +++ b/javascript/extractor/src/com/semmle/js/extractor/ASTExtractor.java @@ -306,7 +306,7 @@ public class ASTExtractor { public V(Platform platform, SourceType sourceType) { this.platform = platform; this.sourceType = sourceType; - this.isStrict = sourceType == SourceType.ES6_MODULE || sourceType == SourceType.CLOSURE_MODULE; + this.isStrict = sourceType == SourceType.MODULE || sourceType == SourceType.CLOSURE_MODULE; } private Label visit(INode child, Label parent, int childIndex) { @@ -557,7 +557,7 @@ public class ASTExtractor { if (!".mjs".equals(locationManager.getSourceFileExtension())) scopeManager.addVariables("require", "module", "exports", "__filename", "__dirname", "arguments"); trapwriter.addTuple("isModule", toplevelLabel); - } else if (sourceType == SourceType.ES6_MODULE || sourceType == SourceType.CLOSURE_MODULE) { + } else if (sourceType == SourceType.MODULE || sourceType == SourceType.CLOSURE_MODULE) { Label moduleScopeKey = trapwriter.globalID("module;{" + locationManager.getFileLabel() + "}," + locationManager.getStartLine() + "," + locationManager.getStartColumn()); scopeManager.enterScope(3, moduleScopeKey, toplevelLabel); if (sourceType == SourceType.CLOSURE_MODULE) { @@ -572,8 +572,8 @@ public class ASTExtractor { visitAll(nd.getBody(), toplevelLabel); - // if we're extracting a Node.js/ES2015 module, leave its scope - if (platform == Platform.NODE || sourceType == SourceType.ES6_MODULE || sourceType == SourceType.CLOSURE_MODULE) + // if we're extracting a module, leave its scope + if (platform == Platform.NODE || sourceType == SourceType.MODULE || sourceType == SourceType.CLOSURE_MODULE) scopeManager.leaveScope(); contextManager.leaveContainer(); diff --git a/javascript/extractor/src/com/semmle/js/extractor/ExtractorConfig.java b/javascript/extractor/src/com/semmle/js/extractor/ExtractorConfig.java index f7e0e46b184..b12b479720c 100644 --- a/javascript/extractor/src/com/semmle/js/extractor/ExtractorConfig.java +++ b/javascript/extractor/src/com/semmle/js/extractor/ExtractorConfig.java @@ -5,6 +5,7 @@ import java.nio.charset.IllegalCharsetNameException; import java.nio.charset.StandardCharsets; import java.nio.charset.UnsupportedCharsetException; +import com.semmle.js.parser.JcornWrapper; import com.semmle.util.data.StringUtil; import com.semmle.util.exception.UserError; @@ -50,8 +51,26 @@ public class ExtractorConfig { } }; + /** + * The type of a source file, which together with the {@link Platform} + * determines how the top-level scope of the file behaves, and whether ES2015 + * module syntax should be allowed. + *

+ * Note that the names of these enum members are depended on by {@link Main}, + * {@link AutoBuild}, and {@link JcornWrapper}. + */ public static enum SourceType { - SCRIPT, ES6_MODULE, CLOSURE_MODULE, AUTO; + /** A script executed in the global scope. */ + SCRIPT, + + /** An ES2015 module. */ + MODULE, + + /** A Closure-Library module, defined using `goog.module()`. */ + CLOSURE_MODULE, + + /** Automatically determined source type. */ + AUTO; @Override public String toString() { diff --git a/javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java b/javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java index f7413a7383b..5d47fff222d 100644 --- a/javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java +++ b/javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java @@ -140,14 +140,14 @@ public class HTMLExtractor implements IExtractor { if ("text/babel".equals(scriptType)) { String plugins = getAttributeValueLC(script, "data-plugins"); if (plugins != null && plugins.contains("transform-es2015-modules-umd")) { - return SourceType.ES6_MODULE; + return SourceType.MODULE; } return config.getSourceType(); } // if `type` is "module", extract as module if ("module".equals(scriptType)) - return SourceType.ES6_MODULE; + return SourceType.MODULE; return null; } diff --git a/javascript/extractor/src/com/semmle/js/extractor/JSExtractor.java b/javascript/extractor/src/com/semmle/js/extractor/JSExtractor.java index f00eefa04fc..f6f6928a1e5 100644 --- a/javascript/extractor/src/com/semmle/js/extractor/JSExtractor.java +++ b/javascript/extractor/src/com/semmle/js/extractor/JSExtractor.java @@ -71,7 +71,7 @@ public class JSExtractor { if (config.getEcmaVersion().compareTo(ECMAVersion.ECMA2015) >= 0) { Matcher m = containsModuleIndicator.matcher(source); if (m.find() && (allowLeadingWS || m.group(1).isEmpty())) { - return m.group(2).startsWith("goog") ? SourceType.CLOSURE_MODULE : SourceType.ES6_MODULE; + return m.group(2).startsWith("goog") ? SourceType.CLOSURE_MODULE : SourceType.MODULE; } } return SourceType.SCRIPT; @@ -126,7 +126,7 @@ public class JSExtractor { if (config.isExterns()) textualExtractor.getTrapwriter().addTuple("isExterns", toplevelLabel); - if (platform == Platform.NODE && sourceType != SourceType.ES6_MODULE && sourceType != SourceType.CLOSURE_MODULE) + if (platform == Platform.NODE && sourceType != SourceType.MODULE && sourceType != SourceType.CLOSURE_MODULE) textualExtractor.getTrapwriter().addTuple("isNodejs", toplevelLabel); return Pair.make(toplevelLabel, loc); diff --git a/javascript/extractor/src/com/semmle/js/extractor/ScriptExtractor.java b/javascript/extractor/src/com/semmle/js/extractor/ScriptExtractor.java index d90e83fbea1..d526a8fae2d 100644 --- a/javascript/extractor/src/com/semmle/js/extractor/ScriptExtractor.java +++ b/javascript/extractor/src/com/semmle/js/extractor/ScriptExtractor.java @@ -52,7 +52,7 @@ public class ScriptExtractor implements IExtractor { // Some file extensions are interpreted as modules by default. if (isAlwaysModule(locationManager.getSourceFileExtension())) { if (config.getSourceType() == SourceType.AUTO) - config = config.withSourceType(SourceType.ES6_MODULE); + config = config.withSourceType(SourceType.MODULE); } ScopeManager scopeManager = new ScopeManager(textualExtractor.getTrapwriter(), config.getEcmaVersion()); diff --git a/javascript/ql/src/semmle/javascript/Closure.qll b/javascript/ql/src/semmle/javascript/Closure.qll index 503197c52c9..efb30038f70 100644 --- a/javascript/ql/src/semmle/javascript/Closure.qll +++ b/javascript/ql/src/semmle/javascript/Closure.qll @@ -48,7 +48,7 @@ class GoogProvide extends GoogFunctionCallStmt, GoogNamespaceRef { GoogProvide() { getFunctionName() = "provide" } /** Gets the identifier of the namespace created by this call. */ - override string getNamespaceId() { result = getArgument(0).(ConstantString).getStringValue() } + override string getNamespaceId() { result = getArgument(0).getStringValue() } } /** @@ -58,7 +58,7 @@ class GoogRequire extends GoogFunctionCall, GoogNamespaceRef { GoogRequire() { getFunctionName() = "require" } /** Gets the identifier of the namespace imported by this call. */ - override string getNamespaceId() { result = getArgument(0).(ConstantString).getStringValue() } + override string getNamespaceId() { result = getArgument(0).getStringValue() } } private class GoogRequireImport extends GoogRequire, Import { @@ -81,7 +81,7 @@ class GoogModuleDeclaration extends GoogFunctionCallStmt, GoogNamespaceRef { } /** Gets the identifier of the namespace imported by this call. */ - override string getNamespaceId() { result = getArgument(0).(ConstantString).getStringValue() } + override string getNamespaceId() { result = getArgument(0).getStringValue() } } /** @@ -93,7 +93,7 @@ class ClosureModule extends Module { } /** - * Gets the call to `goog.module()` or `goog.declareModuleId` in this module. + * Gets the call to `goog.module` or `goog.declareModuleId` in this module. */ GoogModuleDeclaration getModuleDeclaration() { result = getAChildStmt() @@ -121,8 +121,7 @@ class ClosureModule extends Module { */ Variable getExportsVariable() { getModuleDeclaration().getFunctionName() = "module" and - result.getScope() = this.getScope() and - result.getName() = "exports" + result = getScope().getVariable("exports") } override predicate exports(string name, ASTNode export) { diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll index f771228c418..93488c06967 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll @@ -338,16 +338,17 @@ private class AnalyzedExportAssign extends AnalyzedPropertyWrite, DataFlow::Valu * Flow analysis for assignments to the `exports` variable in a Closure module. */ private class AnalyzedClosureExportAssign extends AnalyzedPropertyWrite, DataFlow::ValueNode { + override AssignExpr astNode; ClosureModule mod; AnalyzedClosureExportAssign() { - astNode.(AssignExpr).getLhs() = mod.getExportsVariable().getAReference() + astNode.getLhs() = mod.getExportsVariable().getAReference() } override predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source) { baseVal = TAbstractModuleObject(astNode.getTopLevel()) and propName = "exports" and - source = astNode.(AssignExpr).getRhs().flow() + source = astNode.getRhs().flow() } }