Factor XML discard predicates into OverlayXml.qll

config/identical-files.json

@@ -276,5 +276,12 @@
   "Python model summaries test extension": [
     "python/ql/test/library-tests/dataflow/model-summaries/InlineTaintTest.ext.yml",
     "python/ql/test/library-tests/dataflow/model-summaries/NormalDataflowTest.ext.yml"
+  ],
+  "XML discard predicates": [
+    "javascript/ql/lib/semmle/javascript/internal/OverlayXml.qll",
+    "java/ql/lib/semmle/code/java/internal/OverlayXml.qll",
+    "go/ql/lib/semmle/go/internal/OverlayXml.qll",
+    "python/ql/lib/semmle/python/internal/OverlayXml.qll",
+    "csharp/ql/lib/semmle/code/csharp/internal/OverlayXml.qll"
   ]
 }

csharp/ql/lib/semmle/code/csharp/internal/Overlay.qll

@@ -2,6 +2,8 @@
  * Defines entity discard predicates for C# overlay analysis.
  */
 
+private import OverlayXml
+
 /**
  * Holds always for the overlay variant and never for the base variant.
  * This local predicate is used to define local predicates that behave
@@ -110,36 +112,6 @@ private predicate discardLocation(@location_default loc) {
   exists(string path | discardableLocation(loc, path) | overlayChangedFiles(path))
 }
 
-/**
- * A class of Xml locatables that can be discarded from the base.
- */
-overlay[local]
-private class DiscardableXmlEntity extends DiscardableEntityBase instanceof @xmllocatable {
-  /** Gets the path to the file in which this element occurs. */
-  override string getFilePath() {
-    exists(@location_default loc | result = getLocationFilePath(loc) | xmllocations(this, loc))
-  }
-}
-
-overlay[local]
-private predicate overlayXmlExtracted(string file) {
-  exists(DiscardableXmlEntity dxe |
-    dxe.existsInOverlay() and
-    file = dxe.getFilePath() and
-    not files(dxe, _) and
-    not xmlNs(dxe, _, _, _)
-  )
-}
-
-overlay[discard_entity]
-private predicate discardXmlEntity(@xmllocatable xml) {
-  overlayChangedFiles(xml.(DiscardableXmlEntity).getFilePath())
-  or
-  // The XML extractor is not incremental and may extract more
-  // XML files than those included in overlayChangedFiles.
-  overlayXmlExtracted(xml.(DiscardableXmlEntity).getFilePath())
-}
-
 overlay[local]
 private class DiscardableAspEntity extends DiscardableEntityBase instanceof @asp_element {
   /** Gets the path to the file in which this element occurs. */

csharp/ql/lib/semmle/code/csharp/internal/OverlayXml.qll

@@ -0,0 +1,45 @@
+overlay[local]
+module;
+
+/**
+ * A local predicate that always holds for the overlay variant and never holds for the base variant.
+ * This is used to define local predicates that behave differently for the base and overlay variant.
+ */
+private predicate isOverlay() { databaseMetadata("isOverlay", "true") }
+
+private @file getXmlFile(@xmllocatable locatable) {
+  exists(@location_default location | xmllocations(locatable, location) |
+    locations_default(location, result, _, _, _, _)
+  )
+}
+
+private @file getXmlFileInBase(@xmllocatable locatable) {
+  not isOverlay() and
+  result = getXmlFile(locatable)
+}
+
+/**
+ * Holds if the given `file` was extracted as part of the overlay and was extracted by the HTML/XML
+ * extractor.
+ */
+private predicate overlayXmlExtracted(@file file) {
+  isOverlay() and
+  exists(@xmllocatable locatable |
+    not files(locatable, _) and not xmlNs(locatable, _, _, _) and file = getXmlFile(locatable)
+  )
+}
+
+/**
+ * Holds if the given XML `locatable` should be discarded, because it is part of the overlay base
+ * and is in a file that was also extracted as part of the overlay database.
+ */
+overlay[discard_entity]
+private predicate discardXmlLocatable(@xmllocatable locatable) {
+  exists(@file file | file = getXmlFileInBase(locatable) |
+    exists(string path | files(file, path) | overlayChangedFiles(path))
+    or
+    // The HTML/XML extractor is currently not incremental and may extract more files than those
+    // included in overlayChangedFiles.
+    overlayXmlExtracted(file)
+  )
+}

go/ql/lib/semmle/go/Overlay.qll

@@ -4,6 +4,8 @@
 overlay[local]
 module;
 
+private import internal.OverlayXml
+
 /**
  * A local predicate that always holds for the overlay variant and never holds for the base variant.
  * This is used to define local predicates that behave differently for the base and overlay variant.
@@ -52,40 +54,3 @@ private predicate discardLocatable(@locatable locatable) {
     discardableLocatable(file, locatable) and discardableFile(path)
   )
 }
-
-private @file getXmlFile(@xmllocatable locatable) {
-  exists(@location_default location | xmllocations(locatable, location) |
-    locations_default(location, result, _, _, _, _)
-  )
-}
-
-private @file getXmlFileInBase(@xmllocatable locatable) {
-  not isOverlay() and
-  result = getXmlFile(locatable)
-}
-
-/**
- * Holds if the given `file` was extracted as part of the overlay and was extracted by the HTML/XML
- * extractor.
- */
-private predicate overlayXmlExtracted(@file file) {
-  isOverlay() and
-  exists(@xmllocatable locatable |
-    not files(locatable, _) and not xmlNs(locatable, _, _, _) and file = getXmlFile(locatable)
-  )
-}
-
-/**
- * Holds if the given XML `locatable` should be discarded, because it is part of the overlay base
- * and is in a file that was also extracted as part of the overlay database.
- */
-overlay[discard_entity]
-private predicate discardXmlLocatable(@xmllocatable locatable) {
-  exists(@file file | file = getXmlFileInBase(locatable) |
-    exists(string path | files(file, path) | overlayChangedFiles(path))
-    or
-    // The HTML/XML extractor is currently not incremental and may extract more files than those
-    // included in overlayChangedFiles.
-    overlayXmlExtracted(file)
-  )
-}

go/ql/lib/semmle/go/internal/OverlayXml.qll

@@ -0,0 +1,45 @@
+overlay[local]
+module;
+
+/**
+ * A local predicate that always holds for the overlay variant and never holds for the base variant.
+ * This is used to define local predicates that behave differently for the base and overlay variant.
+ */
+private predicate isOverlay() { databaseMetadata("isOverlay", "true") }
+
+private @file getXmlFile(@xmllocatable locatable) {
+  exists(@location_default location | xmllocations(locatable, location) |
+    locations_default(location, result, _, _, _, _)
+  )
+}
+
+private @file getXmlFileInBase(@xmllocatable locatable) {
+  not isOverlay() and
+  result = getXmlFile(locatable)
+}
+
+/**
+ * Holds if the given `file` was extracted as part of the overlay and was extracted by the HTML/XML
+ * extractor.
+ */
+private predicate overlayXmlExtracted(@file file) {
+  isOverlay() and
+  exists(@xmllocatable locatable |
+    not files(locatable, _) and not xmlNs(locatable, _, _, _) and file = getXmlFile(locatable)
+  )
+}
+
+/**
+ * Holds if the given XML `locatable` should be discarded, because it is part of the overlay base
+ * and is in a file that was also extracted as part of the overlay database.
+ */
+overlay[discard_entity]
+private predicate discardXmlLocatable(@xmllocatable locatable) {
+  exists(@file file | file = getXmlFileInBase(locatable) |
+    exists(string path | files(file, path) | overlayChangedFiles(path))
+    or
+    // The HTML/XML extractor is currently not incremental and may extract more files than those
+    // included in overlayChangedFiles.
+    overlayXmlExtracted(file)
+  )
+}

java/ql/lib/semmle/code/java/Overlay.qll

@@ -5,6 +5,7 @@ overlay[local?]
 module;
 
 import java
+private import internal.OverlayXml
 
 /**
  * A local predicate that always holds for the overlay variant and
@@ -18,7 +19,7 @@ predicate isOverlay() { databaseMetadata("isOverlay", "true") }
 overlay[local]
 string getRawFile(@locatable el) {
   exists(@location loc, @file file |
-    (hasLocation(el, loc) or xmllocations(el, loc)) and
+    hasLocation(el, loc) and
     locations_default(loc, file, _, _, _, _) and
     files(file, result)
   )
@@ -102,31 +103,3 @@ private predicate discardBaseConfigLocatable(@configLocatable el) {
   // property files than those included in overlayChangedFiles.
   overlayConfigExtracted(baseConfigLocatable(el))
 }
-
-/**
- * An `@xmllocatable` that should be discarded in the base variant if its file is
- * extracted in the overlay variant.
- */
-overlay[local]
-abstract class DiscardableXmlLocatable extends @xmllocatable {
-  /** Gets the raw file for an xmllocatable in base. */
-  string getRawFileInBase() { not isOverlay() and result = getRawFile(this) }
-
-  /** Gets a textual representation of this discardable xmllocatable. */
-  string toString() { none() }
-}
-
-overlay[local]
-private predicate overlayXmlExtracted(string file) {
-  isOverlay() and
-  exists(@xmllocatable el | not files(el, _) and not xmlNs(el, _, _, _) and file = getRawFile(el))
-}
-
-overlay[discard_entity]
-private predicate discardXmlLocatable(@xmllocatable el) {
-  overlayChangedFiles(el.(DiscardableXmlLocatable).getRawFileInBase())
-  or
-  // The XML extractor is currently not incremental and may extract more
-  // XML files than those included in overlayChangedFiles.
-  overlayXmlExtracted(el.(DiscardableXmlLocatable).getRawFileInBase())
-}

java/ql/lib/semmle/code/java/internal/OverlayXml.qll

@@ -0,0 +1,45 @@
+overlay[local]
+module;
+
+/**
+ * A local predicate that always holds for the overlay variant and never holds for the base variant.
+ * This is used to define local predicates that behave differently for the base and overlay variant.
+ */
+private predicate isOverlay() { databaseMetadata("isOverlay", "true") }
+
+private @file getXmlFile(@xmllocatable locatable) {
+  exists(@location_default location | xmllocations(locatable, location) |
+    locations_default(location, result, _, _, _, _)
+  )
+}
+
+private @file getXmlFileInBase(@xmllocatable locatable) {
+  not isOverlay() and
+  result = getXmlFile(locatable)
+}
+
+/**
+ * Holds if the given `file` was extracted as part of the overlay and was extracted by the HTML/XML
+ * extractor.
+ */
+private predicate overlayXmlExtracted(@file file) {
+  isOverlay() and
+  exists(@xmllocatable locatable |
+    not files(locatable, _) and not xmlNs(locatable, _, _, _) and file = getXmlFile(locatable)
+  )
+}
+
+/**
+ * Holds if the given XML `locatable` should be discarded, because it is part of the overlay base
+ * and is in a file that was also extracted as part of the overlay database.
+ */
+overlay[discard_entity]
+private predicate discardXmlLocatable(@xmllocatable locatable) {
+  exists(@file file | file = getXmlFileInBase(locatable) |
+    exists(string path | files(file, path) | overlayChangedFiles(path))
+    or
+    // The HTML/XML extractor is currently not incremental and may extract more files than those
+    // included in overlayChangedFiles.
+    overlayXmlExtracted(file)
+  )
+}

java/ql/lib/semmle/code/xml/XML.qll

@@ -6,7 +6,6 @@ module;
 
 import semmle.files.FileSystem
 private import codeql.xml.Xml
-private import semmle.code.java.Overlay
 
 private module Input implements InputSig<File, Location> {
   class XmlLocatableBase = @xmllocatable or @xmlnamespaceable;
@@ -70,13 +69,3 @@ private module Input implements InputSig<File, Location> {
 }
 
 import Make<File, Location, Input>
-
-private class DiscardableXmlAttribute extends DiscardableXmlLocatable, @xmlattribute { }
-
-private class DiscardableXmlElement extends DiscardableXmlLocatable, @xmlelement { }
-
-private class DiscardableXmlComment extends DiscardableXmlLocatable, @xmlcomment { }
-
-private class DiscardableXmlCharacters extends DiscardableXmlLocatable, @xmlcharacters { }
-
-private class DiscardableXmlDtd extends DiscardableXmlLocatable, @xmldtd { }

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

@@ -1,4 +1,5 @@
 private import javascript
+private import OverlayXml
 
 /** Holds if the database is an overlay. */
 overlay[local]
@@ -12,8 +13,6 @@ private string getFileFromEntity(@locatable node) {
     json_locations(node, loc)
     or
     yaml_locations(node, loc)
-    or
-    xmllocations(node, loc)
   |
     result = getFileFromLocation(loc)
   )

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

@@ -0,0 +1,45 @@
+overlay[local]
+module;
+
+/**
+ * A local predicate that always holds for the overlay variant and never holds for the base variant.
+ * This is used to define local predicates that behave differently for the base and overlay variant.
+ */
+private predicate isOverlay() { databaseMetadata("isOverlay", "true") }
+
+private @file getXmlFile(@xmllocatable locatable) {
+  exists(@location_default location | xmllocations(locatable, location) |
+    locations_default(location, result, _, _, _, _)
+  )
+}
+
+private @file getXmlFileInBase(@xmllocatable locatable) {
+  not isOverlay() and
+  result = getXmlFile(locatable)
+}
+
+/**
+ * Holds if the given `file` was extracted as part of the overlay and was extracted by the HTML/XML
+ * extractor.
+ */
+private predicate overlayXmlExtracted(@file file) {
+  isOverlay() and
+  exists(@xmllocatable locatable |
+    not files(locatable, _) and not xmlNs(locatable, _, _, _) and file = getXmlFile(locatable)
+  )
+}
+
+/**
+ * Holds if the given XML `locatable` should be discarded, because it is part of the overlay base
+ * and is in a file that was also extracted as part of the overlay database.
+ */
+overlay[discard_entity]
+private predicate discardXmlLocatable(@xmllocatable locatable) {
+  exists(@file file | file = getXmlFileInBase(locatable) |
+    exists(string path | files(file, path) | overlayChangedFiles(path))
+    or
+    // The HTML/XML extractor is currently not incremental and may extract more files than those
+    // included in overlayChangedFiles.
+    overlayXmlExtracted(file)
+  )
+}

python/ql/lib/semmle/python/Overlay.qll

@@ -2,6 +2,8 @@
  * Defines entity discard predicates for Python overlay analysis.
  */
 
+private import internal.OverlayXml
+
 /*- Predicates -*/
 /**
  * Holds always for the overlay variant and never for the base variant.
@@ -303,33 +305,6 @@ final private class DiscardableComment extends Discardable instanceof @py_commen
   }
 }
 
-/*- XML -*/
-overlay[local]
-final private class DiscardableXmlLocatable extends Discardable instanceof @xmllocatable {
-  override string getPath() {
-    exists(@location loc | xmllocations(this, loc) | result = getPathForLocation(loc))
-  }
-}
-
-overlay[local]
-private predicate overlayXmlExtracted(string path) {
-  exists(DiscardableXmlLocatable d | not files(d, _) and not xmlNs(d, _, _, _) |
-    d.existsInOverlay() and
-    path = d.getPath()
-  )
-}
-
-overlay[discard_entity]
-private predicate discardXmlLocatable(@xmllocatable el) {
-  exists(DiscardableXmlLocatable d | d = el |
-    // The XML extractor is currently not incremental and may extract more
-    // XML files than those included in `overlayChangedFiles`, so this discard predicate
-    // handles those files alongside the normal `discardStarEntity` logic.
-    overlayXmlExtracted(d.getPath()) and
-    d.existsInBase()
-  )
-}
-
 /*- YAML -*/
 overlay[local]
 final private class DiscardableYamlLocatable extends Discardable instanceof @yaml_locatable {

python/ql/lib/semmle/python/internal/OverlayXml.qll

@@ -0,0 +1,45 @@
+overlay[local]
+module;
+
+/**
+ * A local predicate that always holds for the overlay variant and never holds for the base variant.
+ * This is used to define local predicates that behave differently for the base and overlay variant.
+ */
+private predicate isOverlay() { databaseMetadata("isOverlay", "true") }
+
+private @file getXmlFile(@xmllocatable locatable) {
+  exists(@location_default location | xmllocations(locatable, location) |
+    locations_default(location, result, _, _, _, _)
+  )
+}
+
+private @file getXmlFileInBase(@xmllocatable locatable) {
+  not isOverlay() and
+  result = getXmlFile(locatable)
+}
+
+/**
+ * Holds if the given `file` was extracted as part of the overlay and was extracted by the HTML/XML
+ * extractor.
+ */
+private predicate overlayXmlExtracted(@file file) {
+  isOverlay() and
+  exists(@xmllocatable locatable |
+    not files(locatable, _) and not xmlNs(locatable, _, _, _) and file = getXmlFile(locatable)
+  )
+}
+
+/**
+ * Holds if the given XML `locatable` should be discarded, because it is part of the overlay base
+ * and is in a file that was also extracted as part of the overlay database.
+ */
+overlay[discard_entity]
+private predicate discardXmlLocatable(@xmllocatable locatable) {
+  exists(@file file | file = getXmlFileInBase(locatable) |
+    exists(string path | files(file, path) | overlayChangedFiles(path))
+    or
+    // The HTML/XML extractor is currently not incremental and may extract more files than those
+    // included in overlayChangedFiles.
+    overlayXmlExtracted(file)
+  )
+}