From 16cef92106caebbbfed72734df00cf64391063ef Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Tue, 12 Mar 2024 11:08:18 +0100
Subject: [PATCH 1/2] JS: Add `DataFlow::Node.getLocation`

---
 config/identical-files.json                   |   3 +-
 javascript/ql/lib/semmle/javascript/AST.qll   |  16 +-
 javascript/ql/lib/semmle/javascript/CFG.qll   |   4 +-
 .../ql/lib/semmle/javascript/Comments.qll     |   2 -
 .../ql/lib/semmle/javascript/Errors.qll       |   2 -
 javascript/ql/lib/semmle/javascript/Files.qll |   3 +-
 javascript/ql/lib/semmle/javascript/HTML.qll  |   8 -
 javascript/ql/lib/semmle/javascript/JSDoc.qll |   4 -
 javascript/ql/lib/semmle/javascript/JSON.qll  |  12 +-
 javascript/ql/lib/semmle/javascript/Lines.qll |   2 -
 .../ql/lib/semmle/javascript/Locations.qll    |  53 +++---
 .../ql/lib/semmle/javascript/Regexp.qll       |   2 -
 .../semmle/javascript/RestrictedLocations.qll |   2 +-
 .../ql/lib/semmle/javascript/Tokens.qll       |   2 -
 .../ql/lib/semmle/javascript/Variables.qll    |   4 +-
 javascript/ql/lib/semmle/javascript/XML.qll   |   8 +-
 javascript/ql/lib/semmle/javascript/YAML.qll  |   8 +-
 .../semmle/javascript/dataflow/DataFlow.qll   |   8 +
 .../javascript/frameworks/Templating.qll      |   9 +-
 .../javascript/internal/CachedStages.qll      |   2 +
 .../semmle/javascript/internal/Locations.qll  | 171 ++++++++++++++++++
 .../internal/InlineExpectationsTestImpl.qll   |   7 +-
 22 files changed, 235 insertions(+), 97 deletions(-)
 create mode 100644 javascript/ql/lib/semmle/javascript/internal/Locations.qll

diff --git a/config/identical-files.json b/config/identical-files.json
index ce0e3a67f35d..017fdd11481d 100644
--- a/config/identical-files.json
+++ b/config/identical-files.json
@@ -255,7 +255,6 @@
     "cpp/ql/lib/semmle/code/cpp/XML.qll",
     "csharp/ql/lib/semmle/code/csharp/XML.qll",
     "java/ql/lib/semmle/code/xml/XML.qll",
-    "javascript/ql/lib/semmle/javascript/XML.qll",
     "python/ql/lib/semmle/python/xml/XML.qll"
   ],
   "DuplicationProblems.inc.qhelp": [
@@ -372,4 +371,4 @@
     "python/ql/test/experimental/dataflow/model-summaries/InlineTaintTest.ext.yml",
     "python/ql/test/experimental/dataflow/model-summaries/NormalDataflowTest.ext.yml"
   ]
-}
+}
\ No newline at end of file
diff --git a/javascript/ql/lib/semmle/javascript/AST.qll b/javascript/ql/lib/semmle/javascript/AST.qll
index e4a1cf944c46..412f2036280e 100644
--- a/javascript/ql/lib/semmle/javascript/AST.qll
+++ b/javascript/ql/lib/semmle/javascript/AST.qll
@@ -23,31 +23,27 @@ private import semmle.javascript.internal.CachedStages
  * ```
  */
 class AstNode extends @ast_node, NodeInStmtContainer {
-  override Location getLocation() { hasLocation(this, result) }
-
   override File getFile() {
     result = this.getLocation().getFile() // Specialized for performance reasons
   }
 
   /** Gets the first token belonging to this element. */
   Token getFirstToken() {
-    exists(Location l1, Location l2 |
+    exists(DbLocation l1, DbLocation l2, string filepath, int startline, int startcolumn |
       l1 = this.getLocation() and
       l2 = result.getLocation() and
-      l1.getFile() = l2.getFile() and
-      l1.getStartLine() = l2.getStartLine() and
-      l1.getStartColumn() = l2.getStartColumn()
+      l1.hasLocationInfo(filepath, startline, startcolumn, _, _) and
+      l2.hasLocationInfo(filepath, startline, startcolumn, _, _)
     )
   }
 
   /** Gets the last token belonging to this element. */
   Token getLastToken() {
-    exists(Location l1, Location l2 |
+    exists(DbLocation l1, DbLocation l2, string filepath, int endline, int endcolumn |
       l1 = this.getLocation() and
       l2 = result.getLocation() and
-      l1.getFile() = l2.getFile() and
-      l1.getEndLine() = l2.getEndLine() and
-      l1.getEndColumn() = l2.getEndColumn()
+      l1.hasLocationInfo(filepath, _, _, endline, endcolumn) and
+      l2.hasLocationInfo(filepath, _, _, endline, endcolumn)
     ) and
     // exclude empty EOF token
     not result instanceof EOFToken
diff --git a/javascript/ql/lib/semmle/javascript/CFG.qll b/javascript/ql/lib/semmle/javascript/CFG.qll
index 81bbef4c6d22..95e1e9aef72a 100644
--- a/javascript/ql/lib/semmle/javascript/CFG.qll
+++ b/javascript/ql/lib/semmle/javascript/CFG.qll
@@ -356,9 +356,7 @@ class ControlFlowNode extends @cfg_node, Locatable, NodeInStmtContainer {
  * A synthetic CFG node that does not correspond to a statement or expression;
  * examples include guard nodes and entry/exit nodes.
  */
-class SyntheticControlFlowNode extends @synthetic_cfg_node, ControlFlowNode {
-  override Location getLocation() { hasLocation(this, result) }
-}
+class SyntheticControlFlowNode extends @synthetic_cfg_node, ControlFlowNode { }
 
 /** A synthetic CFG node marking the entry point of a function or toplevel script. */
 class ControlFlowEntryNode extends SyntheticControlFlowNode, @entry_node {
diff --git a/javascript/ql/lib/semmle/javascript/Comments.qll b/javascript/ql/lib/semmle/javascript/Comments.qll
index 4888aae0b6d5..889843728a25 100644
--- a/javascript/ql/lib/semmle/javascript/Comments.qll
+++ b/javascript/ql/lib/semmle/javascript/Comments.qll
@@ -15,8 +15,6 @@ import javascript
  * </pre>
  */
 class Comment extends @comment, Locatable {
-  override Location getLocation() { hasLocation(this, result) }
-
   /** Gets the toplevel element this comment belongs to. */
   TopLevel getTopLevel() { comments(this, _, result, _, _) }
 
diff --git a/javascript/ql/lib/semmle/javascript/Errors.qll b/javascript/ql/lib/semmle/javascript/Errors.qll
index 729965029971..6a5d73566a44 100644
--- a/javascript/ql/lib/semmle/javascript/Errors.qll
+++ b/javascript/ql/lib/semmle/javascript/Errors.qll
@@ -4,8 +4,6 @@ import javascript
 
 /** An error encountered during extraction. */
 abstract class Error extends Locatable {
-  override Location getLocation() { hasLocation(this, result) }
-
   /** Gets the message associated with this error. */
   abstract string getMessage();
 
diff --git a/javascript/ql/lib/semmle/javascript/Files.qll b/javascript/ql/lib/semmle/javascript/Files.qll
index b384febb9a1a..88513f087ae0 100644
--- a/javascript/ql/lib/semmle/javascript/Files.qll
+++ b/javascript/ql/lib/semmle/javascript/Files.qll
@@ -3,6 +3,7 @@
 import javascript
 private import NodeModuleResolutionImpl
 private import codeql.util.FileSystem
+private import internal.Locations
 
 private module FsInput implements InputSig {
   abstract class ContainerBase extends @container {
@@ -83,7 +84,7 @@ class File extends Container, Impl::File {
    *
    * Note that files have special locations starting and ending at line zero, column zero.
    */
-  Location getLocation() { hasLocation(this, result) }
+  DbLocation getLocation() { result = getLocatableLocation(this) }
 
   /** Gets the number of lines in this file. */
   int getNumberOfLines() { result = sum(int loc | numlines(this, loc, _, _) | loc) }
diff --git a/javascript/ql/lib/semmle/javascript/HTML.qll b/javascript/ql/lib/semmle/javascript/HTML.qll
index 5ba02cba7cb9..01ce54cef529 100644
--- a/javascript/ql/lib/semmle/javascript/HTML.qll
+++ b/javascript/ql/lib/semmle/javascript/HTML.qll
@@ -43,8 +43,6 @@ module HTML {
   class Element extends Locatable, @xmlelement {
     Element() { exists(FileContainingHtml f | xmlElements(this, _, _, _, f)) }
 
-    override Location getLocation() { xmllocations(this, result) }
-
     /**
      * Gets the name of this HTML element.
      *
@@ -122,8 +120,6 @@ module HTML {
   class Attribute extends Locatable, @xmlattribute {
     Attribute() { exists(FileContainingHtml f | xmlAttrs(this, _, _, _, _, f)) }
 
-    override Location getLocation() { xmllocations(this, result) }
-
     /**
      * Gets the inline script of this attribute, if any.
      */
@@ -326,8 +322,6 @@ module HTML {
      * Holds if this text node is inside a `CDATA` tag.
      */
     predicate isCData() { xmlChars(this, _, _, _, 1, _) }
-
-    override Location getLocation() { xmllocations(this, result) }
   }
 
   /**
@@ -349,7 +343,5 @@ module HTML {
     string getText() { result = this.toString().regexpCapture("(?s)<!--(.*)-->", 1) }
 
     override string toString() { xmlComments(this, result, _, _) }
-
-    override Location getLocation() { xmllocations(this, result) }
   }
 }
diff --git a/javascript/ql/lib/semmle/javascript/JSDoc.qll b/javascript/ql/lib/semmle/javascript/JSDoc.qll
index 44ec09f34e43..6e1ea5caecb6 100644
--- a/javascript/ql/lib/semmle/javascript/JSDoc.qll
+++ b/javascript/ql/lib/semmle/javascript/JSDoc.qll
@@ -18,8 +18,6 @@ private import semmle.javascript.internal.CachedStages
  * </pre>
  */
 class JSDoc extends @jsdoc, Locatable {
-  override Location getLocation() { hasLocation(this, result) }
-
   /** Gets the description text of this JSDoc comment. */
   string getDescription() { jsdoc(this, result, _) }
 
@@ -75,8 +73,6 @@ abstract class Documentable extends AstNode {
  * ```
  */
 class JSDocTypeExprParent extends @jsdoc_type_expr_parent, Locatable {
-  override Location getLocation() { hasLocation(this, result) }
-
   /** Gets the JSDoc comment to which this element belongs. */
   JSDoc getJSDocComment() { none() }
 }
diff --git a/javascript/ql/lib/semmle/javascript/JSON.qll b/javascript/ql/lib/semmle/javascript/JSON.qll
index 1e56fc00657e..714228e52b65 100644
--- a/javascript/ql/lib/semmle/javascript/JSON.qll
+++ b/javascript/ql/lib/semmle/javascript/JSON.qll
@@ -3,6 +3,7 @@
  */
 
 import javascript
+private import semmle.javascript.internal.Locations
 
 /**
  * A JSON-encoded value, which may be a primitive value, an array or an object.
@@ -20,8 +21,6 @@ import javascript
  * ```
  */
 class JsonValue extends @json_value, Locatable {
-  override Location getLocation() { json_locations(this, result) }
-
   /** Gets the parent value to which this value belongs, if any. */
   JsonValue getParent() { json(this, _, result, _, _) }
 
@@ -34,12 +33,7 @@ class JsonValue extends @json_value, Locatable {
   override string toString() { json(this, _, _, _, result) }
 
   /** Gets the JSON file containing this value. */
-  File getJsonFile() {
-    exists(Location loc |
-      json_locations(this, loc) and
-      result = loc.getFile()
-    )
-  }
+  File getJsonFile() { result = getLocatableLocation(this).getFile() }
 
   /** If this is an object, gets the value of property `name`. */
   JsonValue getPropValue(string name) { json_properties(this, name, result) }
@@ -172,7 +166,5 @@ class JsonObject extends @json_object, JsonValue {
  * An error reported by the JSON parser.
  */
 class JsonParseError extends @json_parse_error, Error {
-  override Location getLocation() { json_locations(this, result) }
-
   override string getMessage() { json_errors(this, result) }
 }
diff --git a/javascript/ql/lib/semmle/javascript/Lines.qll b/javascript/ql/lib/semmle/javascript/Lines.qll
index 08a013e52e88..1db9187008a1 100644
--- a/javascript/ql/lib/semmle/javascript/Lines.qll
+++ b/javascript/ql/lib/semmle/javascript/Lines.qll
@@ -14,8 +14,6 @@ import javascript
  * extracted with the `--extract-program-text` flag.
  */
 class Line extends @line, Locatable {
-  override Location getLocation() { hasLocation(this, result) }
-
   /** Gets the toplevel element this line belongs to. */
   TopLevel getTopLevel() { lines(this, result, _, _) }
 
diff --git a/javascript/ql/lib/semmle/javascript/Locations.qll b/javascript/ql/lib/semmle/javascript/Locations.qll
index c0748f7b3e71..ce323dfc14db 100644
--- a/javascript/ql/lib/semmle/javascript/Locations.qll
+++ b/javascript/ql/lib/semmle/javascript/Locations.qll
@@ -1,38 +1,41 @@
 /** Provides classes for working with locations and program elements that have locations. */
 
 import javascript
+private import internal.Locations
 
 /**
  * A location as given by a file, a start line, a start column,
  * an end line, and an end column.
  *
+ * This class is restricted to locations created by the extractor.
+ *
  * For more information about locations see [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
  */
-class Location extends @location {
+class DbLocation extends TDbLocation {
   /** Gets the file for this location. */
-  File getFile() { locations_default(this, result, _, _, _, _) }
+  File getFile() { dbLocationInfo(this, result, _, _, _, _) }
 
   /** Gets the 1-based line number (inclusive) where this location starts. */
-  int getStartLine() { locations_default(this, _, result, _, _, _) }
+  int getStartLine() { dbLocationInfo(this, _, result, _, _, _) }
 
   /** Gets the 1-based column number (inclusive) where this location starts. */
-  int getStartColumn() { locations_default(this, _, _, result, _, _) }
+  int getStartColumn() { dbLocationInfo(this, _, _, result, _, _) }
 
   /** Gets the 1-based line number (inclusive) where this location ends. */
-  int getEndLine() { locations_default(this, _, _, _, result, _) }
+  int getEndLine() { dbLocationInfo(this, _, _, _, result, _) }
 
   /** Gets the 1-based column number (inclusive) where this location ends. */
-  int getEndColumn() { locations_default(this, _, _, _, _, result) }
+  int getEndColumn() { dbLocationInfo(this, _, _, _, _, result) }
 
   /** Gets the number of lines covered by this location. */
   int getNumLines() { result = this.getEndLine() - this.getStartLine() + 1 }
 
   /** Holds if this location starts before location `that`. */
   pragma[inline]
-  predicate startsBefore(Location that) {
+  predicate startsBefore(DbLocation that) {
     exists(File f, int sl1, int sc1, int sl2, int sc2 |
-      locations_default(this, f, sl1, sc1, _, _) and
-      locations_default(that, f, sl2, sc2, _, _)
+      dbLocationInfo(this, f, sl1, sc1, _, _) and
+      dbLocationInfo(that, f, sl2, sc2, _, _)
     |
       sl1 < sl2
       or
@@ -42,10 +45,10 @@ class Location extends @location {
 
   /** Holds if this location ends after location `that`. */
   pragma[inline]
-  predicate endsAfter(Location that) {
+  predicate endsAfter(DbLocation that) {
     exists(File f, int el1, int ec1, int el2, int ec2 |
-      locations_default(this, f, _, _, el1, ec1) and
-      locations_default(that, f, _, _, el2, ec2)
+      dbLocationInfo(this, f, _, _, el1, ec1) and
+      dbLocationInfo(that, f, _, _, el2, ec2)
     |
       el1 > el2
       or
@@ -57,10 +60,10 @@ class Location extends @location {
    * Holds if this location contains location `that`, meaning that it starts
    * before and ends after it.
    */
-  predicate contains(Location that) { this.startsBefore(that) and this.endsAfter(that) }
+  predicate contains(DbLocation that) { this.startsBefore(that) and this.endsAfter(that) }
 
   /** Holds if this location is empty. */
-  predicate isEmpty() { exists(int l, int c | locations_default(this, _, l, c, l, c - 1)) }
+  predicate isEmpty() { exists(int l, int c | dbLocationInfo(this, _, l, c, l, c - 1)) }
 
   /** Gets a textual representation of this element. */
   string toString() { result = this.getFile().getBaseName() + ":" + this.getStartLine().toString() }
@@ -76,22 +79,21 @@ class Location extends @location {
     string filepath, int startline, int startcolumn, int endline, int endcolumn
   ) {
     exists(File f |
-      locations_default(this, f, startline, startcolumn, endline, endcolumn) and
+      dbLocationInfo(this, f, startline, startcolumn, endline, endcolumn) and
       filepath = f.getAbsolutePath()
     )
   }
 }
 
+final class Location = LocationImpl;
+
 /** A program element with a location. */
 class Locatable extends @locatable {
   /** Gets the file this program element comes from. */
   File getFile() { result = this.getLocation().getFile() }
 
   /** Gets this element's location. */
-  Location getLocation() {
-    // overridden by subclasses
-    none()
-  }
+  final DbLocation getLocation() { result = getLocatableLocation(this) }
 
   /**
    * Gets the line on which this element starts.
@@ -142,16 +144,3 @@ class Locatable extends @locatable {
    */
   string getAPrimaryQlClass() { result = "???" }
 }
-
-/**
- * A `File`, considered as a `Locatable`.
- *
- * For reasons of backwards compatibility, @file is a subtype of @locatable. This class exists to
- * provide an override of `Locatable.getLocation()` for @files, since it would otherwise default
- * to `none()`, which is unhelpful.
- */
-private class FileLocatable extends File, Locatable {
-  override Location getLocation() { result = File.super.getLocation() }
-
-  override string toString() { result = File.super.toString() }
-}
diff --git a/javascript/ql/lib/semmle/javascript/Regexp.qll b/javascript/ql/lib/semmle/javascript/Regexp.qll
index 3266f1527a27..3c190af44764 100644
--- a/javascript/ql/lib/semmle/javascript/Regexp.qll
+++ b/javascript/ql/lib/semmle/javascript/Regexp.qll
@@ -43,8 +43,6 @@ class RegExpParent extends Locatable, @regexpparent { }
  * ```
  */
 class RegExpTerm extends Locatable, @regexpterm {
-  override Location getLocation() { hasLocation(this, result) }
-
   /** Gets the `i`th child term of this term. */
   RegExpTerm getChild(int i) { regexpterm(result, _, this, i, _) }
 
diff --git a/javascript/ql/lib/semmle/javascript/RestrictedLocations.qll b/javascript/ql/lib/semmle/javascript/RestrictedLocations.qll
index 47ee41a42357..05bcd8b3dddc 100644
--- a/javascript/ql/lib/semmle/javascript/RestrictedLocations.qll
+++ b/javascript/ql/lib/semmle/javascript/RestrictedLocations.qll
@@ -26,7 +26,7 @@ class FirstLineOf extends Locatable {
       then endcolumn = xc
       else
         endcolumn =
-          max(int c | any(Location l).hasLocationInfo(filepath, startline, _, startline, c))
+          max(int c | any(DbLocation l).hasLocationInfo(filepath, startline, _, startline, c))
     )
   }
 }
diff --git a/javascript/ql/lib/semmle/javascript/Tokens.qll b/javascript/ql/lib/semmle/javascript/Tokens.qll
index 52659f444c4a..c6a9b05a3d1a 100644
--- a/javascript/ql/lib/semmle/javascript/Tokens.qll
+++ b/javascript/ql/lib/semmle/javascript/Tokens.qll
@@ -17,8 +17,6 @@ import javascript
  * ```
  */
 class Token extends Locatable, @token {
-  override Location getLocation() { hasLocation(this, result) }
-
   /** Gets the toplevel syntactic structure to which this token belongs. */
   TopLevel getTopLevel() { tokeninfo(this, _, result, _, _) }
 
diff --git a/javascript/ql/lib/semmle/javascript/Variables.qll b/javascript/ql/lib/semmle/javascript/Variables.qll
index 00b463f8a9bb..1eeb735124b5 100644
--- a/javascript/ql/lib/semmle/javascript/Variables.qll
+++ b/javascript/ql/lib/semmle/javascript/Variables.qll
@@ -329,9 +329,9 @@ class LocalVariable extends Variable {
    * If the variable has one or more declarations, the location of the first declaration is used.
    * If the variable has no declaration, the entry point of its declaring container is used.
    */
-  Location getLocation() {
+  DbLocation getLocation() {
     result =
-      min(Location loc |
+      min(DbLocation loc |
         loc = this.getADeclaration().getLocation()
       |
         loc order by loc.getStartLine(), loc.getStartColumn()
diff --git a/javascript/ql/lib/semmle/javascript/XML.qll b/javascript/ql/lib/semmle/javascript/XML.qll
index 65bdd7b7cc16..1a27c9a1ef3a 100644
--- a/javascript/ql/lib/semmle/javascript/XML.qll
+++ b/javascript/ql/lib/semmle/javascript/XML.qll
@@ -3,6 +3,7 @@
  */
 
 import semmle.files.FileSystem
+private import semmle.javascript.internal.Locations
 
 private class TXmlLocatable =
   @xmldtd or @xmlelement or @xmlattribute or @xmlnamespace or @xmlcomment or @xmlcharacters;
@@ -10,7 +11,7 @@ private class TXmlLocatable =
 /** An XML element that has a location. */
 class XmlLocatable extends @xmllocatable, TXmlLocatable {
   /** Gets the source location for this element. */
-  Location getLocation() { xmllocations(this, result) }
+  DbLocation getLocation() { result = getLocatableLocation(this) }
 
   /**
    * Holds if this element is at the specified location.
@@ -22,10 +23,7 @@ class XmlLocatable extends @xmllocatable, TXmlLocatable {
   predicate hasLocationInfo(
     string filepath, int startline, int startcolumn, int endline, int endcolumn
   ) {
-    exists(File f, Location l | l = this.getLocation() |
-      locations_default(l, f, startline, startcolumn, endline, endcolumn) and
-      filepath = f.getAbsolutePath()
-    )
+    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
   }
 
   /** Gets a textual representation of this element. */
diff --git a/javascript/ql/lib/semmle/javascript/YAML.qll b/javascript/ql/lib/semmle/javascript/YAML.qll
index 38bca7779002..1ab562b9524d 100644
--- a/javascript/ql/lib/semmle/javascript/YAML.qll
+++ b/javascript/ql/lib/semmle/javascript/YAML.qll
@@ -9,9 +9,9 @@ import javascript
 private import codeql.yaml.Yaml as LibYaml
 
 private module YamlSig implements LibYaml::InputSig {
-  class LocatableBase extends @yaml_locatable, Locatable {
-    override Location getLocation() { yaml_locations(this, result) }
-  }
+  class Location = DbLocation;
+
+  class LocatableBase extends @yaml_locatable, Locatable { }
 
   import javascript
 
@@ -52,8 +52,6 @@ import LibYaml::Make<YamlSig>
 private class MyYmlNode extends Locatable instanceof YamlNode {
   override string getAPrimaryQlClass() { result = YamlNode.super.getAPrimaryQlClass() }
 
-  override Location getLocation() { result = YamlNode.super.getLocation() }
-
   override string toString() { result = YamlNode.super.toString() }
 }
 
diff --git a/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll b/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll
index c098c60816e4..6d091a720af5 100644
--- a/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll
+++ b/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll
@@ -152,6 +152,14 @@ module DataFlow {
       none()
     }
 
+    /** Gets the location of this node. */
+    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 file this data flow node comes from. */
     File getFile() { none() } // overridden in subclasses
 
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Templating.qll b/javascript/ql/lib/semmle/javascript/frameworks/Templating.qll
index 097003c5ab8f..a7286c7a1999 100644
--- a/javascript/ql/lib/semmle/javascript/frameworks/Templating.qll
+++ b/javascript/ql/lib/semmle/javascript/frameworks/Templating.qll
@@ -36,8 +36,6 @@ module Templating {
 
   /** A placeholder tag for a templating engine. */
   class TemplatePlaceholderTag extends @template_placeholder_tag, Locatable {
-    override Location getLocation() { hasLocation(this, result) }
-
     override string toString() { template_placeholder_tag_info(this, _, result) }
 
     /** Gets the full text of the template tag, including delimiters. */
@@ -107,7 +105,12 @@ module Templating {
      * Gets the innermost JavaScript expression containing this template tag, if any.
      */
     pragma[nomagic]
-    Expr getEnclosingExpr() { expr_contains_template_tag_location(result, this.getLocation()) }
+    Expr getEnclosingExpr() {
+      exists(@location loc |
+        hasLocation(this, loc) and
+        expr_contains_template_tag_location(result, loc)
+      )
+    }
   }
 
   /**
diff --git a/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll b/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
index 459b83f2b996..09d52e89ee04 100644
--- a/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
+++ b/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
@@ -138,6 +138,8 @@ module Stages {
       or
       any(DataFlow::Node node).hasLocationInfo(_, _, _, _, _)
       or
+      exists(any(DataFlow::Node node).getLocation())
+      or
       exists(any(DataFlow::Node node).toString())
       or
       exists(any(AccessPath a).getAnInstanceIn(_))
diff --git a/javascript/ql/lib/semmle/javascript/internal/Locations.qll b/javascript/ql/lib/semmle/javascript/internal/Locations.qll
new file mode 100644
index 000000000000..4a21f4a6b980
--- /dev/null
+++ b/javascript/ql/lib/semmle/javascript/internal/Locations.qll
@@ -0,0 +1,171 @@
+/** Provides classes for working with locations and program elements that have locations. */
+
+import javascript
+
+// Should _not_ be cached, as that would require the data flow stage to be evaluated
+// in order to evaluate the AST stage. Ideally, we would cache each injector separately,
+// but that's not possible. Instead, we cache all predicates that need the injectors
+// to be tuple numbered.
+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
+    // avoid overlap with existing DB locations
+    not exists(File f |
+      locations_default(_, f, startline, startcolumn, endline, endcolumn) and
+      f.getAbsolutePath() = filepath
+    )
+  }
+
+/**
+ * A location as given by a file, a start line, a start column,
+ * an end line, and an end column.
+ *
+ * For more information about locations see [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+ */
+abstract class LocationImpl extends TLocation {
+  /** Gets the file for this location. */
+  abstract File getFile();
+
+  /** Gets the 1-based line number (inclusive) where this location starts. */
+  abstract int getStartLine();
+
+  /** Gets the 1-based column number (inclusive) where this location starts. */
+  abstract int getStartColumn();
+
+  /** Gets the 1-based line number (inclusive) where this location ends. */
+  abstract int getEndLine();
+
+  /** Gets the 1-based column number (inclusive) where this location ends. */
+  abstract int getEndColumn();
+
+  /** Gets the number of lines covered by this location. */
+  int getNumLines() { result = this.getEndLine() - this.getStartLine() + 1 }
+
+  /** Holds if this location starts before location `that`. */
+  pragma[inline]
+  predicate startsBefore(Location that) {
+    exists(string f, int sl1, int sc1, int sl2, int sc2 |
+      this.hasLocationInfo(f, sl1, sc1, _, _) and
+      that.hasLocationInfo(f, sl2, sc2, _, _)
+    |
+      sl1 < sl2
+      or
+      sl1 = sl2 and sc1 < sc2
+    )
+  }
+
+  /** Holds if this location ends after location `that`. */
+  pragma[inline]
+  predicate endsAfter(Location that) {
+    exists(string f, int el1, int ec1, int el2, int ec2 |
+      this.hasLocationInfo(f, _, _, el1, ec1) and
+      that.hasLocationInfo(f, _, _, el2, ec2)
+    |
+      el1 > el2
+      or
+      el1 = el2 and ec1 > ec2
+    )
+  }
+
+  /**
+   * Holds if this location contains location `that`, meaning that it starts
+   * before and ends after it.
+   */
+  predicate contains(Location that) { this.startsBefore(that) and this.endsAfter(that) }
+
+  /** Holds if this location is empty. */
+  predicate isEmpty() { exists(int l, int c | this.hasLocationInfo(_, l, c, l, c - 1)) }
+
+  /** Gets a textual representation of this element. */
+  string toString() { result = this.getFile().getBaseName() + ":" + this.getStartLine().toString() }
+
+  /**
+   * Holds if this element is at the specified location.
+   * The location spans column `startcolumn` of line `startline` to
+   * column `endcolumn` of line `endline` in file `filepath`.
+   * For more information, see
+   * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+   */
+  abstract predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  );
+}
+
+class DbLocationImpl extends LocationImpl instanceof DbLocation {
+  override File getFile() { result = DbLocation.super.getFile() }
+
+  override int getStartLine() { result = DbLocation.super.getStartLine() }
+
+  override int getStartColumn() { result = DbLocation.super.getStartColumn() }
+
+  override int getEndLine() { result = DbLocation.super.getEndLine() }
+
+  override int getEndColumn() { result = DbLocation.super.getEndColumn() }
+
+  override predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    DbLocation.super.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+  }
+}
+
+class SynthLocationImpl extends LocationImpl, TSynthLocation {
+  override File getFile() { synthLocationInfo(this, result.getAbsolutePath(), _, _, _, _) }
+
+  override int getStartLine() { synthLocationInfo(this, _, result, _, _, _) }
+
+  override int getStartColumn() { synthLocationInfo(this, _, _, result, _, _) }
+
+  override int getEndLine() { synthLocationInfo(this, _, _, _, result, _) }
+
+  override int getEndColumn() { synthLocationInfo(this, _, _, _, _, result) }
+
+  override predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    synthLocationInfo(this, filepath, startline, startcolumn, endline, endcolumn)
+  }
+}
+
+cached
+private module Cached {
+  cached
+  DbLocation getLocatableLocation(@locatable l) {
+    exists(@location loc |
+      hasLocation(l, loc) or
+      xmllocations(l, loc) or
+      json_locations(l, loc) or
+      yaml_locations(l, loc)
+    |
+      result = TDbLocation(loc)
+    )
+  }
+
+  cached
+  predicate dbLocationInfo(
+    DbLocation l, File f, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    exists(@location loc |
+      l = TDbLocation(loc) and
+      locations_default(loc, f, startline, startcolumn, endline, endcolumn)
+    )
+  }
+}
+
+import Cached
+
+cached
+private module CachedInDataFlowStage {
+  private import semmle.javascript.internal.CachedStages
+
+  cached
+  predicate synthLocationInfo(
+    SynthLocationImpl l, string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    Stages::DataFlowStage::ref() and
+    l = TSynthLocation(filepath, startline, startcolumn, endline, endcolumn)
+  }
+}
+
+private import CachedInDataFlowStage
diff --git a/javascript/ql/test/testUtilities/internal/InlineExpectationsTestImpl.qll b/javascript/ql/test/testUtilities/internal/InlineExpectationsTestImpl.qll
index d1de2866b105..9e92f70af69f 100644
--- a/javascript/ql/test/testUtilities/internal/InlineExpectationsTestImpl.qll
+++ b/javascript/ql/test/testUtilities/internal/InlineExpectationsTestImpl.qll
@@ -4,8 +4,13 @@ private import codeql.util.test.InlineExpectationsTest
 module Impl implements InlineExpectationsTestSig {
   private import javascript
 
-  class ExpectationComment extends LineComment {
+  final private class LineCommentFinal = LineComment;
+
+  class ExpectationComment extends LineCommentFinal {
     string getContents() { result = this.getText() }
+
+    /** Gets this element's location. */
+    Location getLocation() { result = super.getLocation() }
   }
 
   class Location = JS::Location;

From 54fa8181da3527974a8ca0ce6572dbb23783dba8 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Wed, 13 Mar 2024 20:03:01 +0100
Subject: [PATCH 2/2] Address review comment

---
 javascript/ql/lib/semmle/javascript/SSA.qll   |   8 ++
 .../semmle/javascript/dataflow/DataFlow.qll   | 122 ++++--------------
 .../javascript/internal/CachedStages.qll      |   2 -
 .../semmle/javascript/internal/Locations.qll  |   2 +-
 4 files changed, 32 insertions(+), 102 deletions(-)

diff --git a/javascript/ql/lib/semmle/javascript/SSA.qll b/javascript/ql/lib/semmle/javascript/SSA.qll
index a505cf5ff484..2de42193743f 100644
--- a/javascript/ql/lib/semmle/javascript/SSA.qll
+++ b/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() }
 }
diff --git a/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll b/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll
index 6d091a720af5..79fede61b8f7 100644
--- a/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll
+++ b/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
-    predicate hasLocationInfo(
+    final 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() {
-      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)
-      )
-    }
+    cached
+    Location getLocation() { none() }
 
     /** 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(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
+    override Location getLocation() {
       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(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      ssa.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = ssa.getLocation() }
 
     override string toString() { result = ssa.getSourceVariable().getName() }
 
@@ -348,13 +337,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = prop.(ControlFlowNode).getBasicBlock() }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      prop.(Locatable)
-          .getLocation()
-          .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = prop.(Locatable).getLocation() }
 
     override string toString() { result = prop.(AstNode).toString() }
 
@@ -375,11 +358,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = rest.getBasicBlock() }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      rest.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = rest.getLocation() }
 
     override string toString() { result = "..." + rest.toString() }
 
@@ -400,11 +379,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = elt.getBasicBlock() }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      elt.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = elt.getLocation() }
 
     override string toString() { result = elt.toString() }
 
@@ -429,11 +404,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = elt.getBasicBlock() }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      elt.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = elt.getLocation() }
 
     override string toString() { result = elt.toString() }
 
@@ -453,11 +424,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = call.getBasicBlock() }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      call.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = call.getLocation() }
 
     override string toString() { result = "reflective call" }
 
@@ -474,11 +441,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = imprt.getBasicBlock() }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      imprt.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = imprt.getLocation() }
 
     override string toString() { result = imprt.toString() }
 
@@ -968,11 +931,7 @@ module DataFlow {
 
     override string toString() { result = attr.toString() }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      attr.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = attr.getLocation() }
 
     /** 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(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      attr.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = attr.getLocation() }
 
     /** 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(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      function.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = function.getLocation() }
 
     override BasicBlock getBasicBlock() { result = function.getExit().getBasicBlock() }
 
@@ -1038,11 +989,7 @@ module DataFlow {
 
     override string toString() { result = "return of " + function.describe() }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      function.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = function.getLocation() }
 
     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(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      function.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = function.getLocation() }
 
     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(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      invoke.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = invoke.getLocation() }
 
     override BasicBlock getBasicBlock() { result = invoke.getBasicBlock() }
 
@@ -1366,15 +1305,10 @@ module DataFlow {
       exists(StmtContainer container | this = TThisNode(container) | result = container.getEntry())
     }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
+    override Location getLocation() {
       // Use the function entry as the location
       exists(StmtContainer container | this = TThisNode(container) |
-        container
-            .getEntry()
-            .getLocation()
-            .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+        result = container.getEntry().getLocation()
       )
     }
 
@@ -1393,11 +1327,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { result = variable.getDeclaringContainer().getStartBB() }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      variable.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = variable.getLocation() }
 
     override string toString() { result = variable.getName() }
   }
@@ -1409,13 +1339,7 @@ module DataFlow {
 
     override BasicBlock getBasicBlock() { none() }
 
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      this.getTag()
-          .getLocation()
-          .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
+    override Location getLocation() { result = this.getTag().getLocation() }
 
     override string toString() { result = this.getTag().toString() }
   }
diff --git a/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll b/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
index 09d52e89ee04..39da790b6b94 100644
--- a/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
+++ b/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())
diff --git a/javascript/ql/lib/semmle/javascript/internal/Locations.qll b/javascript/ql/lib/semmle/javascript/internal/Locations.qll
index 4a21f4a6b980..d1dc8d403f75 100644
--- a/javascript/ql/lib/semmle/javascript/internal/Locations.qll
+++ b/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