Merge pull request #4012 from erik-krogh/getId

Approved by asgerf, esbena

javascript/ql/src/Declarations/ConflictingFunctions.ql

@@ -24,5 +24,6 @@ where
   // ignore ambient, abstract, and overloaded declarations in TypeScript
   f.hasBody() and
   g.hasBody()
-select f.getId(), "Declaration of " + f.describe() + " conflicts with $@ in the same scope.",
-  g.getId(), "another declaration"
+select f.getIdentifier(),
+  "Declaration of " + f.describe() + " conflicts with $@ in the same scope.", g.getIdentifier(),
+  "another declaration"

javascript/ql/src/Declarations/DeadStoreOfLocal.ql

@@ -42,11 +42,11 @@ where
     access.isAmbient()
   ) and
   // don't flag function expressions
-  not exists(FunctionExpr fe | dead = fe.getId()) and
+  not exists(FunctionExpr fe | dead = fe.getIdentifier()) and
   // don't flag function declarations nested inside blocks or other compound statements;
   // their meaning is only partially specified by the standard
   not exists(FunctionDeclStmt fd, StmtContainer outer |
-    dead = fd.getId() and outer = fd.getEnclosingContainer()
+    dead = fd.getIdentifier() and outer = fd.getEnclosingContainer()
   |
     not fd = outer.getBody().(BlockStmt).getAStmt()
   ) and

javascript/ql/src/Declarations/RedeclaredVariable.ql

@@ -22,6 +22,6 @@ where
   not decl.isAmbient() and
   not redecl.isAmbient() and
   // Redeclaring a namespace extends the previous definition.
-  not decl = any(NamespaceDeclaration ns).getId() and
-  not redecl = any(NamespaceDeclaration ns).getId()
+  not decl = any(NamespaceDeclaration ns).getIdentifier() and
+  not redecl = any(NamespaceDeclaration ns).getIdentifier()
 select redecl, "This variable has already been declared $@.", decl, "here"

javascript/ql/src/Declarations/UnusedVariable.ql

@@ -93,7 +93,7 @@ predicate isEnumMember(VarDecl decl) { decl = any(EnumMember member).getIdentifi
  * "function f", "variable v" or "class c".
  */
 string describeVarDecl(VarDecl vd) {
-  if vd = any(Function f).getId()
+  if vd = any(Function f).getIdentifier()
   then result = "function " + vd.getName()
   else
     if vd = any(ClassDefinition c).getIdentifier()
@@ -115,7 +115,7 @@ class ImportVarDeclProvider extends Stmt {
    */
   VarDecl getAVarDecl() {
     result = this.(ImportDeclaration).getASpecifier().getLocal() or
-    result = this.(ImportEqualsDeclaration).getId()
+    result = this.(ImportEqualsDeclaration).getIdentifier()
   }
 
   /**

javascript/ql/src/Expressions/SuspiciousPropAccess.ql

@@ -23,7 +23,7 @@ private import semmle.javascript.dataflow.InferredTypes
  * if it is part of a const enum access, so we conservatively silence the alert in that case.
  */
 predicate namespaceOrConstEnumAccess(VarAccess e) {
-  exists(NamespaceDeclaration decl | e.getVariable().getADeclaration() = decl.getId())
+  exists(NamespaceDeclaration decl | e.getVariable().getADeclaration() = decl.getIdentifier())
   or
   exists(EnumDeclaration decl | e.getVariable().getADeclaration() = decl.getIdentifier() |
     decl.isConst()

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

@@ -30,15 +30,17 @@ private predicate defn(ControlFlowNode def, Expr lhs, AST::ValueNode rhs) {
   or
   exists(VariableDeclarator vd | def = vd | lhs = vd.getBindingPattern() and rhs = vd.getInit())
   or
-  exists(Function f | def = f.getId() | lhs = def and rhs = f)
+  exists(Function f | def = f.getIdentifier() | lhs = def and rhs = f)
   or
   exists(ClassDefinition c | lhs = c.getIdentifier() | def = c and rhs = c and not c.isAmbient())
   or
-  exists(NamespaceDeclaration n | def = n | lhs = n.getId() and rhs = n)
+  exists(NamespaceDeclaration n | def = n | lhs = n.getIdentifier() and rhs = n)
   or
   exists(EnumDeclaration ed | def = ed.getIdentifier() | lhs = def and rhs = ed)
   or
-  exists(ImportEqualsDeclaration i | def = i | lhs = i.getId() and rhs = i.getImportedEntity())
+  exists(ImportEqualsDeclaration i | def = i |
+    lhs = i.getIdentifier() and rhs = i.getImportedEntity()
+  )
   or
   exists(ImportSpecifier i | def = i | lhs = i.getLocal() and rhs = i)
   or
@@ -149,7 +151,7 @@ class RValue extends RefExpr {
     or
     this = any(UpdateExpr u).getOperand().getUnderlyingReference()
     or
-    this = any(NamespaceDeclaration decl).getId()
+    this = any(NamespaceDeclaration decl).getIdentifier()
   }
 }
 

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

@@ -330,7 +330,7 @@ class ExportDefaultDeclaration extends ExportDeclaration, @exportdefaultdeclarat
   /** Gets the declaration, if any, exported by this default export. */
   VarDecl getADecl() {
     exists(ExprOrStmt op | op = getOperand() |
-      result = op.(FunctionDeclStmt).getId() or
+      result = op.(FunctionDeclStmt).getIdentifier() or
       result = op.(ClassDeclStmt).getIdentifier()
     )
   }
@@ -364,13 +364,13 @@ class ExportNamedDeclaration extends ExportDeclaration, @exportnameddeclaration
   Identifier getAnExportedDecl() {
     exists(ExprOrStmt op | op = getOperand() |
       result = op.(DeclStmt).getADecl().getBindingPattern().getABindingVarRef() or
-      result = op.(FunctionDeclStmt).getId() or
+      result = op.(FunctionDeclStmt).getIdentifier() or
       result = op.(ClassDeclStmt).getIdentifier() or
-      result = op.(NamespaceDeclaration).getId() or
+      result = op.(NamespaceDeclaration).getIdentifier() or
       result = op.(EnumDeclaration).getIdentifier() or
       result = op.(InterfaceDeclaration).getIdentifier() or
       result = op.(TypeAliasDeclaration).getIdentifier() or
-      result = op.(ImportEqualsDeclaration).getId()
+      result = op.(ImportEqualsDeclaration).getIdentifier()
     )
   }
 

javascript/ql/src/semmle/javascript/Functions.qll

@@ -77,8 +77,15 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine
     result = getDocumentation().getATagByTitle("this").getType()
   }
 
+  /**
+   * DEPRECATED: Use `getIdentifier()` instead.
+   *
+   * Gets the identifier specifying the name of this function, if any.
+   */
+  deprecated VarDecl getId() { result = getIdentifier() }
+
   /** Gets the identifier specifying the name of this function, if any. */
-  VarDecl getId() { result = getChildExpr(-1) }
+  VarDecl getIdentifier() { result = getChildExpr(-1) }
 
   /**
    * Gets the name of this function if it has one, or a name inferred from its context.
@@ -89,9 +96,9 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine
    * can be inferred, there is no result.
    */
   string getName() {
-    result = getId().getName()
+    result = getIdentifier().getName()
     or
-    not exists(getId()) and
+    not exists(getIdentifier()) and
     (
       exists(VarDef vd | this = vd.getSource() | result = vd.getTarget().(VarRef).getName())
       or
@@ -111,7 +118,7 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine
   }
 
   /** Gets the variable holding this function. */
-  Variable getVariable() { result = getId().getVariable() }
+  Variable getVariable() { result = getIdentifier().getVariable() }
 
   /** Gets the `arguments` variable of this function, if any. */
   ArgumentsVariable getArgumentsVariable() { result.getFunction() = this }
@@ -183,11 +190,11 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine
     not exists(getAParameter()) and
     (
       // if the function has a name, the opening parenthesis comes right after it
-      result = getId().getLastToken().getNextToken()
+      result = getIdentifier().getLastToken().getNextToken()
       or
       // otherwise this must be an arrow function with no parameters, so the opening
       // parenthesis is the very first token of the function
-      not exists(getId()) and result = getFirstToken()
+      not exists(getIdentifier()) and result = getFirstToken()
     )
   }
 
@@ -309,8 +316,8 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine
    */
   private string inferNameFromVarDef() {
     // in ambiguous cases like `var f = function g() {}`, prefer `g` to `f`
-    if exists(getId())
-    then result = "function " + getId().getName()
+    if exists(getIdentifier())
+    then result = "function " + getIdentifier().getName()
     else
       exists(VarDef vd | this = vd.getSource() |
         result = "function " + vd.getTarget().(VarRef).getName()

javascript/ql/src/semmle/javascript/GlobalAccessPaths.qll

@@ -267,7 +267,7 @@ module AccessPath {
     or
     exists(FunctionDeclStmt fun |
       node = DataFlow::valueNode(fun) and
-      result = fun.getId().(GlobalVarDecl).getName() and
+      result = fun.getIdentifier().(GlobalVarDecl).getName() and
       root.isGlobal()
     )
     or
@@ -285,7 +285,7 @@ module AccessPath {
     or
     exists(NamespaceDeclaration decl |
       node = DataFlow::valueNode(decl) and
-      result = decl.getId().(GlobalVarDecl).getName() and
+      result = decl.getIdentifier().(GlobalVarDecl).getName() and
       root.isGlobal()
     )
   }

javascript/ql/src/semmle/javascript/TypeScript.qll

@@ -8,13 +8,16 @@ import javascript
  */
 class NamespaceDefinition extends Stmt, @namespacedefinition, AST::ValueNode {
   /**
+   * DEPRECATED: Use `getIdentifier()` instead.
+   *
    * Gets the identifier naming the namespace.
    */
-  Identifier getId() {
-    result = this.(NamespaceDeclaration).getId()
-    or
-    result = this.(EnumDeclaration).getIdentifier()
-  }
+  deprecated Identifier getId() { result = getIdentifier() }
+
+  /**
+   * Gets the identifier naming the namespace.
+   */
+  Identifier getIdentifier() { none() } // Overridden in subtypes.
 
   /**
    * Gets unqualified name of the namespace being defined.
@@ -29,7 +32,7 @@ class NamespaceDefinition extends Stmt, @namespacedefinition, AST::ValueNode {
    * Gets the local namespace name induced by this namespace.
    */
   LocalNamespaceName getLocalNamespaceName() {
-    result = getId().(LocalNamespaceDecl).getLocalNamespaceName()
+    result = getIdentifier().(LocalNamespaceDecl).getLocalNamespaceName()
   }
 
   /**
@@ -55,10 +58,10 @@ class NamespaceDefinition extends Stmt, @namespacedefinition, AST::ValueNode {
  */
 class NamespaceDeclaration extends NamespaceDefinition, StmtContainer, @namespacedeclaration {
   /** Gets the name of this namespace. */
-  override Identifier getId() { result = getChildExpr(-1) }
+  override Identifier getIdentifier() { result = getChildExpr(-1) }
 
   /** Gets the name of this namespace as a string. */
-  override string getName() { result = getId().getName() }
+  override string getName() { result = getIdentifier().getName() }
 
   /** Gets the `i`th statement in this namespace. */
   Stmt getStmt(int i) {
@@ -83,7 +86,7 @@ class NamespaceDeclaration extends NamespaceDefinition, StmtContainer, @namespac
   predicate isInstantiated() { isInstantiated(this) }
 
   override ControlFlowNode getFirstControlFlowNode() {
-    if hasDeclareKeyword(this) then result = this else result = getId()
+    if hasDeclareKeyword(this) then result = this else result = getIdentifier()
   }
 }
 
@@ -178,13 +181,20 @@ class GlobalAugmentationDeclaration extends Stmt, StmtContainer, @globalaugmenta
 
 /** A TypeScript "import-equals" declaration. */
 class ImportEqualsDeclaration extends Stmt, @importequalsdeclaration {
+  /**
+   * DEPRECATED: Use `getIdentifier()` instead.
+   *
+   * Gets the name under which the imported entity is imported.
+   */
+  deprecated Identifier getId() { result = getIdentifier() }
+
   /** Gets the name under which the imported entity is imported. */
-  Identifier getId() { result = getChildExpr(0) }
+  Identifier getIdentifier() { result = getChildExpr(0) }
 
   /** Gets the expression specifying the imported module or entity. */
   Expr getImportedEntity() { result = getChildExpr(1) }
 
-  override ControlFlowNode getFirstControlFlowNode() { result = getId() }
+  override ControlFlowNode getFirstControlFlowNode() { result = getIdentifier() }
 }
 
 /**
@@ -348,7 +358,7 @@ class TypeDecl extends Identifier, TypeRef, LexicalDecl {
     this = any(ClassOrInterface ci).getIdentifier() or
     this = any(TypeParameter tp).getIdentifier() or
     this = any(ImportSpecifier im).getLocal() or
-    this = any(ImportEqualsDeclaration im).getId() or
+    this = any(ImportEqualsDeclaration im).getIdentifier() or
     this = any(TypeAliasDeclaration td).getIdentifier() or
     this = any(EnumDeclaration ed).getIdentifier() or
     this = any(EnumMember member).getIdentifier()
@@ -1226,8 +1236,8 @@ abstract class NamespaceRef extends ASTNode { }
  */
 class LocalNamespaceDecl extends VarDecl, NamespaceRef {
   LocalNamespaceDecl() {
-    any(NamespaceDeclaration nd).getId() = this or
-    any(ImportEqualsDeclaration im).getId() = this or
+    any(NamespaceDeclaration nd).getIdentifier() = this or
+    any(ImportEqualsDeclaration im).getIdentifier() = this or
     any(ImportSpecifier im).getLocal() = this or
     any(EnumDeclaration ed).getIdentifier() = this
   }
@@ -1325,7 +1335,7 @@ class ImportVarTypeAccess extends VarTypeAccess, ImportTypeExpr, @importvartypea
  */
 class EnumDeclaration extends NamespaceDefinition, @enumdeclaration, AST::ValueNode {
   /** Gets the name of this enum, such as `E` in `enum E { A, B }`. */
-  Identifier getIdentifier() { result = getChildExpr(0) }
+  override Identifier getIdentifier() { result = getChildExpr(0) }
 
   /** Gets the name of this enum as a string. */
   override string getName() { result = getIdentifier().getName() }

javascript/ql/src/semmle/javascript/Variables.qll

@@ -312,8 +312,11 @@ class LocalVariable extends Variable {
     this = result.getScope().getAVariable()
     or
     exists(VarDecl d | d = getADeclaration() |
-      if d = any(FunctionDeclStmt fds).getId()
-      then exists(FunctionDeclStmt fds | d = fds.getId() | result = fds.getEnclosingContainer())
+      if d = any(FunctionDeclStmt fds).getIdentifier()
+      then
+        exists(FunctionDeclStmt fds | d = fds.getIdentifier() |
+          result = fds.getEnclosingContainer()
+        )
       else result = d.getContainer()
     )
   }

javascript/ql/src/semmle/javascript/dataflow/internal/BasicExprTypeInference.qll

@@ -111,7 +111,7 @@ private class AnalyzedNamespaceDeclaration extends DataFlow::AnalyzedValueNode {
 
   AbstractValue getPreviousValue() {
     exists(AnalyzedSsaDefinition def |
-      def.getVariable().getAUse() = astNode.getId() and
+      def.getVariable().getAUse() = astNode.getIdentifier() and
       result = def.getAnRhsValue()
     )
   }

javascript/ql/test/library-tests/Functions/getId.qll

@@ -1,5 +1,5 @@
 import javascript
 
 query predicate test_getId(Function f, VarDecl res0, string res1) {
-  res0 = f.getId() and res1 = f.getName()
+  res0 = f.getIdentifier() and res1 = f.getName()
 }

javascript/ql/test/library-tests/TypeScript/Namespaces/CheckBindings.ql

@@ -10,7 +10,7 @@ class ResolveCall extends CallExpr {
   string getDeclaredValue() {
     result = getVariable().getAnAssignedExpr().getStringValue()
     or
-    exists(NamespaceDeclaration decl | decl.getId() = getVariable().getADeclaration() |
+    exists(NamespaceDeclaration decl | decl.getIdentifier() = getVariable().getADeclaration() |
       result = getNamespaceName(decl)
     )
   }
@@ -21,7 +21,8 @@ string getNamespaceName(NamespaceDeclaration decl) {
   or
   not decl.getStmt(0).(ExprStmt).getExpr() instanceof ConstantString and
   result =
-    "Namespace " + decl.getId() + " on line " + decl.getFirstToken().getLocation().getStartLine()
+    "Namespace " + decl.getIdentifier() + " on line " +
+      decl.getFirstToken().getLocation().getStartLine()
 }
 
 from ResolveCall resolve