Address review comment

javascript/ql/lib/semmle/javascript/SSA.qll

@@ -488,6 +488,14 @@ class SsaDefinition extends TSsaDefinition {
     string filepath, int startline, int startcolumn, int endline, int endcolumn
   );
 
+  /** Gets the location of this element. */
+  final Location getLocation() {
+    exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
+      this.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
+      result.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    )
+  }
+
   /** Gets the function or toplevel to which this definition belongs. */
   StmtContainer getContainer() { result = this.getBasicBlock().getContainer() }
 }

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

@@ -145,20 +145,15 @@ module DataFlow {
      * For more information, see
      * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
      */
-    cached
+    final predicate hasLocationInfo(
-    predicate hasLocationInfo(
       string filepath, int startline, int startcolumn, int endline, int endcolumn
     ) {
-      none()
+      this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
     }
 
     /** Gets the location of this node. */
-    Location getLocation() {
+    cached
-      exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
+    Location getLocation() { none() }
-        this.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
-        result.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-      )
-    }
 
     /** Gets the file this data flow node comes from. */
     File getFile() { none() } // overridden in subclasses
@@ -300,11 +295,9 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { astNode = result.getANode() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() {
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
       Stages::DataFlowStage::ref() and
-      astNode.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+      result = astNode.getLocation()
     }
 
     override File getFile() { result = astNode.getFile() }
@@ -325,11 +318,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = ssa.getBasicBlock() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = ssa.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      ssa.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override string toString() { result = ssa.getSourceVariable().getName() }
 
@@ -348,13 +337,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = prop.(ControlFlowNode).getBasicBlock() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = prop.(Locatable).getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      prop.(Locatable)
-          .getLocation()
-          .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override string toString() { result = prop.(AstNode).toString() }
 
@@ -375,11 +358,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = rest.getBasicBlock() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = rest.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      rest.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override string toString() { result = "..." + rest.toString() }
 
@@ -400,11 +379,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = elt.getBasicBlock() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = elt.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      elt.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override string toString() { result = elt.toString() }
 
@@ -429,11 +404,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = elt.getBasicBlock() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = elt.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      elt.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override string toString() { result = elt.toString() }
 
@@ -453,11 +424,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = call.getBasicBlock() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = call.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      call.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override string toString() { result = "reflective call" }
 
@@ -474,11 +441,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = imprt.getBasicBlock() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = imprt.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      imprt.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override string toString() { result = imprt.toString() }
 
@@ -968,11 +931,7 @@ module DataFlow {
 
     override string toString() { result = attr.toString() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = attr.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      attr.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     /** Gets the attribute corresponding to this data flow node. */
     HTML::Attribute getAttribute() { result = attr }
@@ -990,11 +949,7 @@ module DataFlow {
 
     override string toString() { result = attr.toString() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = attr.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      attr.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     /** Gets the attribute corresponding to this data flow node. */
     XmlAttribute getAttribute() { result = attr }
@@ -1012,11 +967,7 @@ module DataFlow {
 
     override string toString() { result = "exceptional return of " + function.describe() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = function.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      function.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override BasicBlock getBasicBlock() { result = function.getExit().getBasicBlock() }
 
@@ -1038,11 +989,7 @@ module DataFlow {
 
     override string toString() { result = "return of " + function.describe() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = function.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      function.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override BasicBlock getBasicBlock() { result = function.getExit().getBasicBlock() }
 
@@ -1064,11 +1011,7 @@ module DataFlow {
 
     override string toString() { result = "'arguments' object of " + function.describe() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = function.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      function.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override BasicBlock getBasicBlock() { result = function.getEntry().getBasicBlock() }
 
@@ -1090,11 +1033,7 @@ module DataFlow {
 
     override string toString() { result = "exceptional return of " + invoke.toString() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = invoke.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      invoke.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override BasicBlock getBasicBlock() { result = invoke.getBasicBlock() }
 
@@ -1366,15 +1305,10 @@ module DataFlow {
       exists(StmtContainer container | this = TThisNode(container) | result = container.getEntry())
     }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() {
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
       // Use the function entry as the location
       exists(StmtContainer container | this = TThisNode(container) |
-        container
+        result = container.getEntry().getLocation()
-            .getEntry()
-            .getLocation()
-            .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
       )
     }
 
@@ -1393,11 +1327,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = variable.getDeclaringContainer().getStartBB() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = variable.getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      variable.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override string toString() { result = variable.getName() }
   }
@@ -1409,13 +1339,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { none() }
 
-    override predicate hasLocationInfo(
+    override Location getLocation() { result = this.getTag().getLocation() }
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      this.getTag()
-          .getLocation()
-          .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
 
     override string toString() { result = this.getTag().toString() }
   }

javascript/ql/lib/semmle/javascript/internal/CachedStages.qll

@@ -136,8 +136,6 @@ module Stages {
       or
       exists(DataFlow::ssaDefinitionNode(_))
       or
-      any(DataFlow::Node node).hasLocationInfo(_, _, _, _, _)
-      or
       exists(any(DataFlow::Node node).getLocation())
       or
       exists(any(DataFlow::Node node).toString())

javascript/ql/lib/semmle/javascript/internal/Locations.qll

@@ -9,7 +9,7 @@ import javascript
 newtype TLocation =
   TDbLocation(@location loc) or
   TSynthLocation(string filepath, int startline, int startcolumn, int endline, int endcolumn) {
-    any(DataFlow::Node n).hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
+    any(SsaDefinition def).hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
     // avoid overlap with existing DB locations
     not exists(File f |
       locations_default(_, f, startline, startcolumn, endline, endcolumn) and