Merge pull request github#9266 from asgerf/js/madman-prep

JS: Some fixes to support proper analysis of d.ts files

Author: asgerf

.gitignore

@@ -55,3 +55,6 @@ go/tools/win64
 go/tools/tokenizer.jar
 go/main
 
+# node_modules folders except in the JS test suite
+node_modules/
+!/javascript/ql/test/**/node_modules/

javascript/extractor/lib/typescript/src/main.ts

@@ -670,6 +670,12 @@ function handleOpenProjectCommand(command: OpenProjectCommand) {
         if (file.endsWith(".d.ts")) {
             return pathlib.basename(file, ".d.ts");
         }
+        if (file.endsWith(".d.mts") || file.endsWith(".d.cts")) {
+            // We don't extract d.mts or d.cts files, but their symbol can coincide with that of a d.ts file,
+            // which means any module bindings we generate for it will ultimately be visible in QL.
+            let base = pathlib.basename(file);
+            return base.substring(0, base.length - '.d.mts'.length);
+        }
         let base = pathlib.basename(file);
         let dot = base.lastIndexOf('.');
         return dot === -1 || dot === 0 ? base : base.substring(0, dot);

javascript/extractor/src/com/semmle/js/extractor/ASTExtractor.java

@@ -590,7 +590,7 @@ public Label visit(Literal nd, Context c) {
       trapwriter.addTuple("literals", valueString, source, key);
       Position start = nd.getLoc().getStart();
       com.semmle.util.locations.Position startPos = new com.semmle.util.locations.Position(start.getLine(), start.getColumn() + 1 /* Convert from 0-based to 1-based. */, start.getOffset());
-      
+
       if (nd.isRegExp()) {
         OffsetTranslation offsets = new OffsetTranslation();
         offsets.set(0, 1); // skip the initial '/'
@@ -622,7 +622,7 @@ private boolean isOctalDigit(char ch) {
     /**
      * Constant-folds simple string concatenations in `exp` while keeping an offset translation
      * that tracks back to the original source.
-     */ 
+     */
     private Pair<String, OffsetTranslation> getStringConcatResult(Expression exp) {
       if (exp instanceof BinaryExpression) {
         BinaryExpression be = (BinaryExpression) exp;
@@ -636,7 +636,7 @@ private Pair<String, OffsetTranslation> getStringConcatResult(Expression exp) {
           if (str.length() > 1000) {
             return null;
           }
-            
+
           int delta = be.getRight().getLoc().getStart().getOffset() - be.getLeft().getLoc().getStart().getOffset();
           int offset = left.fst().length();
           return Pair.make(str, left.snd().append(right.snd(), offset, delta));
@@ -747,7 +747,7 @@ public Label visit(MemberExpression nd, Context c) {
         visit(nd.getProperty(), key, 1, IdContext.TYPE_LABEL);
       } else {
         IdContext baseIdContext =
-            c.idcontext == IdContext.EXPORT ? IdContext.EXPORT_BASE : IdContext.VAR_BIND;
+            (c.idcontext == IdContext.EXPORT || c.idcontext == IdContext.EXPORT_BASE) ? IdContext.EXPORT_BASE : IdContext.VAR_BIND;
         visit(nd.getObject(), key, 0, baseIdContext);
         visit(nd.getProperty(), key, 1, nd.isComputed() ? IdContext.VAR_BIND : IdContext.LABEL);
       }
@@ -848,14 +848,14 @@ public Label visit(AssignmentExpression nd, Context c) {
     public Label visit(BinaryExpression nd, Context c) {
       Label key = super.visit(nd, c);
       if (nd.getOperator().equals("in") && nd.getLeft() instanceof Identifier && ((Identifier)nd.getLeft()).getName().startsWith("#")) {
-        // this happens with Ergonomic brand checks for Private Fields (see https://github.com/tc39/proposal-private-fields-in-in). 
+        // this happens with Ergonomic brand checks for Private Fields (see https://github.com/tc39/proposal-private-fields-in-in).
         // it's the only case where private field identifiers are used not as a field.
         visit(nd.getLeft(), key, 0, IdContext.LABEL, true);
       } else {
         visit(nd.getLeft(), key, 0, true);
       }
       visit(nd.getRight(), key, 1, true);
-      
+
       extractRegxpFromBinop(nd, c);
       return key;
     }
@@ -1815,7 +1815,7 @@ public Label visit(ImportSpecifier nd, Context c) {
       visit(nd.getLocal(), lbl, 1, nd.hasTypeKeyword() ? IdContext.TYPE_ONLY_IMPORT : c.idcontext);
       if (nd.hasTypeKeyword()) {
         trapwriter.addTuple("has_type_keyword", lbl);
-      } 
+      }
       return lbl;
     }
 

javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java

@@ -40,7 +40,8 @@ public JavaScriptHTMLElementHandler(TextualExtractor textualExtractor) {
       this.textualExtractor = textualExtractor;
 
       this.scopeManager =
-          new ScopeManager(textualExtractor.getTrapwriter(), config.getEcmaVersion(), true);
+          new ScopeManager(textualExtractor.getTrapwriter(), config.getEcmaVersion(),
+            ScopeManager.FileKind.TEMPLATE);
     }
 
     /*
@@ -425,7 +426,7 @@ private void extractTemplateTags(
           extractSnippet(
               TopLevelKind.ANGULAR_STYLE_TEMPLATE,
               config.withSourceType(SourceType.ANGULAR_STYLE_TEMPLATE),
-              new ScopeManager(textualExtractor.getTrapwriter(), ECMAVersion.ECMA2020, true),
+              new ScopeManager(textualExtractor.getTrapwriter(), ECMAVersion.ECMA2020, ScopeManager.FileKind.TEMPLATE),
               textualExtractor,
               m.group(bodyGroup),
               m.start(bodyGroup),

javascript/extractor/src/com/semmle/js/extractor/ScopeManager.java

@@ -97,28 +97,43 @@ public Label lookupNamespace(String name) {
     }
   }
 
+  public static enum FileKind {
+    /** Any file not specific to one of the other file kinds. */
+    PLAIN,
+
+    /** A file potentially containing template tags. */
+    TEMPLATE,
+
+    /** A d.ts file, containing TypeScript ambient declarations. */
+    TYPESCRIPT_DECLARATION,
+  }
+
   private final TrapWriter trapWriter;
   private Scope curScope;
   private final Scope toplevelScope;
   private final ECMAVersion ecmaVersion;
   private final Set<String> implicitGlobals = new LinkedHashSet<String>();
   private Scope implicitVariableScope;
-  private final boolean isInTemplateScope;
+  private final FileKind fileKind;
 
-  public ScopeManager(TrapWriter trapWriter, ECMAVersion ecmaVersion, boolean isInTemplateScope) {
+  public ScopeManager(TrapWriter trapWriter, ECMAVersion ecmaVersion, FileKind fileKind) {
     this.trapWriter = trapWriter;
     this.toplevelScope = enterScope(ScopeKind.GLOBAL, trapWriter.globalID("global_scope"), null);
     this.ecmaVersion = ecmaVersion;
-    this.implicitVariableScope = toplevelScope; 
-    this.isInTemplateScope = isInTemplateScope;
+    this.implicitVariableScope = toplevelScope;
+    this.fileKind = fileKind;
   }
 
   /**
    * Returns true the current scope is potentially in a template file, and may contain
    * relevant template tags.
    */
   public boolean isInTemplateFile() {
-    return isInTemplateScope;
+    return this.fileKind == FileKind.TEMPLATE;
+  }
+
+  public boolean isInTypeScriptDeclarationFile() {
+    return this.fileKind == FileKind.TYPESCRIPT_DECLARATION;
   }
 
   /**
@@ -221,7 +236,7 @@ public Scope getToplevelScope() {
 
   /**
    * Get the label for a given variable in the current scope; if it cannot be found, add it to the
-   * implicit variable scope (usually the global scope). 
+   * implicit variable scope (usually the global scope).
    */
   public Label getVarKey(String name) {
     Label lbl = curScope.lookupVariable(name);
@@ -411,15 +426,15 @@ public Void visit(Identifier nd, Void v) {
       // cases where we turn on the 'declKind' flags
       @Override
       public Void visit(FunctionDeclaration nd, Void v) {
-        if (nd.hasDeclareKeyword()) return null;
+        if (nd.hasDeclareKeyword() && !isInTypeScriptDeclarationFile()) return null;
         // strict mode functions are block-scoped, non-strict mode ones aren't
         if (blockscope == isStrict) visit(nd.getId(), DeclKind.var);
         return null;
       }
 
       @Override
       public Void visit(ClassDeclaration nd, Void c) {
-        if (nd.hasDeclareKeyword()) return null;
+        if (nd.hasDeclareKeyword() && !isInTypeScriptDeclarationFile()) return null;
         if (blockscope) visit(nd.getClassDef().getId(), DeclKind.varAndType);
         return null;
       }
@@ -468,7 +483,7 @@ public Void visit(EnhancedForStatement nd, Void v) {
 
       @Override
       public Void visit(VariableDeclaration nd, Void v) {
-        if (nd.hasDeclareKeyword()) return null;
+        if (nd.hasDeclareKeyword() && !isInTypeScriptDeclarationFile()) return null;
         // in block scoping mode, only process 'let'; in non-block scoping
         // mode, only process non-'let'
         if (blockscope == nd.isBlockScoped(ecmaVersion)) visit(nd.getDeclarations());
@@ -503,7 +518,8 @@ public Void visit(ClassBody nd, Void c) {
       @Override
       public Void visit(NamespaceDeclaration nd, Void c) {
         if (blockscope) return null;
-        boolean hasVariable = nd.isInstantiated() && !nd.hasDeclareKeyword();
+        boolean isAmbientOutsideDtsFile = nd.hasDeclareKeyword() && !isInTypeScriptDeclarationFile();
+        boolean hasVariable = nd.isInstantiated() && !isAmbientOutsideDtsFile;
         visit(nd.getName(), hasVariable ? DeclKind.varAndNamespace : DeclKind.namespace);
         return null;
       }

javascript/extractor/src/com/semmle/js/extractor/ScriptExtractor.java

@@ -77,7 +77,7 @@ public LoCInfo extract(TextualExtractor textualExtractor) {
     }
 
     ScopeManager scopeManager =
-        new ScopeManager(textualExtractor.getTrapwriter(), config.getEcmaVersion(), false);
+        new ScopeManager(textualExtractor.getTrapwriter(), config.getEcmaVersion(), ScopeManager.FileKind.PLAIN);
     Label toplevelLabel = null;
     LoCInfo loc;
     try {

javascript/extractor/src/com/semmle/js/extractor/TypeScriptExtractor.java

@@ -22,8 +22,10 @@ public LoCInfo extract(TextualExtractor textualExtractor) {
     String source = textualExtractor.getSource();
     File sourceFile = textualExtractor.getExtractedFile();
     Result res = state.getTypeScriptParser().parse(sourceFile, source, textualExtractor.getMetrics());
-    ScopeManager scopeManager =
-        new ScopeManager(textualExtractor.getTrapwriter(), ECMAVersion.ECMA2017, false);
+    ScopeManager.FileKind fileKind = sourceFile.getName().endsWith(".d.ts")
+        ? ScopeManager.FileKind.TYPESCRIPT_DECLARATION
+        : ScopeManager.FileKind.PLAIN;
+    ScopeManager scopeManager = new ScopeManager(textualExtractor.getTrapwriter(), ECMAVersion.ECMA2017, fileKind);
     try {
       FileSnippet snippet = state.getSnippets().get(sourceFile.toPath());
       SourceType sourceType = snippet != null ? snippet.getSourceType() : jsExtractor.establishSourceType(source, false);

javascript/extractor/tests/ts/output/trap/exportasnamespace.d.ts.trap

@@ -97,28 +97,28 @@ scopenodes(#20001,#20033)
 scopenesting(#20033,#20000)
 is_module(#20001)
 is_es2015_module(#20001)
-#20034=*
-stmts(#20034,30,#20001,0,"export  ...  foo();")
-hasLocation(#20034,#20003)
-stmt_containers(#20034,#20001)
+#20034=@"var;{foo};{#20033}"
+variables(#20034,"foo",#20033)
 #20035=*
-stmts(#20035,17,#20034,-1,"declare ...  foo();")
-#20036=@"loc,{#10000},1,8,1,30"
-locations_default(#20036,#10000,1,8,1,30)
-hasLocation(#20035,#20036)
+stmts(#20035,30,#20001,0,"export  ...  foo();")
+hasLocation(#20035,#20003)
 stmt_containers(#20035,#20001)
-has_declare_keyword(#20035)
-#20037=*
-exprs(#20037,78,#20035,-1,"foo")
-hasLocation(#20037,#20013)
-expr_containers(#20037,#20035)
-literals("foo","foo",#20037)
-#20038=@"var;{foo};{#20000}"
-variables(#20038,"foo",#20000)
-decl(#20037,#20038)
+#20036=*
+stmts(#20036,17,#20035,-1,"declare ...  foo();")
+#20037=@"loc,{#10000},1,8,1,30"
+locations_default(#20037,#10000,1,8,1,30)
+hasLocation(#20036,#20037)
+stmt_containers(#20036,#20001)
+has_declare_keyword(#20036)
+#20038=*
+exprs(#20038,78,#20036,-1,"foo")
+hasLocation(#20038,#20013)
+expr_containers(#20038,#20036)
+literals("foo","foo",#20038)
+decl(#20038,#20034)
 #20039=*
 scopes(#20039,1)
-scopenodes(#20035,#20039)
+scopenodes(#20036,#20039)
 scopenesting(#20039,#20033)
 #20040=@"var;{arguments};{#20039}"
 variables(#20040,"arguments",#20039)
@@ -142,8 +142,8 @@ hasLocation(#20043,#20044)
 exit_cfg_node(#20045,#20001)
 hasLocation(#20045,#20031)
 successor(#20041,#20045)
-successor(#20034,#20035)
-successor(#20035,#20041)
-successor(#20043,#20034)
+successor(#20035,#20036)
+successor(#20036,#20041)
+successor(#20043,#20035)
 numlines(#10000,2,2,0)
 filetype(#10000,"typescript")

javascript/extractor/tests/ts/output/trap/namespaces.ts.trap

@@ -305,11 +305,12 @@ hasLocation(#20096,#20097)
 enclosing_stmt(#20096,#20092)
 expr_containers(#20096,#20001)
 #20098=*
-exprs(#20098,79,#20096,0,"M")
+exprs(#20098,103,#20096,0,"M")
 hasLocation(#20098,#20052)
 enclosing_stmt(#20098,#20092)
 expr_containers(#20098,#20001)
 literals("M","M",#20098)
+namespacebind(#20098,#20069)
 bind(#20098,#20066)
 #20099=*
 exprs(#20099,0,#20096,1,"N")

javascript/ql/lib/semmle/javascript/Files.qll

@@ -175,6 +175,15 @@ class Folder extends Container, @folder {
     result.getExtension() = extension
   }
 
+  /** Like `getFile` except `d.ts` is treated as a single extension. */
+  private File getFileLongExtension(string stem, string extension) {
+    not (stem.matches("%.d") and extension = "ts") and
+    result = this.getFile(stem, extension)
+    or
+    extension = "d.ts" and
+    result = this.getFile(stem + ".d", "ts")
+  }
+
   /**
    * Gets the file in this folder that has the given `stem` and any of the supported JavaScript extensions.
    *
@@ -188,7 +197,11 @@ class Folder extends Container, @folder {
    */
   File getJavaScriptFile(string stem) {
     result =
-      min(int p, string ext | p = getFileExtensionPriority(ext) | this.getFile(stem, ext) order by p)
+      min(int p, string ext |
+        p = getFileExtensionPriority(ext)
+      |
+        this.getFileLongExtension(stem, ext) order by p
+      )
   }
 
   /** Gets a subfolder contained in this folder. */