Merge pull request #20931 from owen-mc/go/fix-misc-ql

owen-mc · web-flow · commit c43b03ba34e2 · 2025-11-28T09:42:00.000Z
Go/Java: fix miscellaneous trivial issues highlighted by ql-for-ql
diff --git a/go/ql/lib/semmle/go/dataflow/GlobalValueNumbering.qll b/go/ql/lib/semmle/go/dataflow/GlobalValueNumbering.qll
@@ -255,8 +255,6 @@ private predicate globalValueNumbers(DataFlow::CallNode ce, int start, GVN head,
  * methods.
  */
 class GVN extends GvnBase {
-  GVN() { this instanceof GvnBase }
-
   /** Gets a data-flow node that has this GVN. */
   DataFlow::Node getANode() { this = globalValueNumber(result) }
 
diff --git a/go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll b/go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll
@@ -1347,7 +1347,6 @@ module Public {
   }
 }
 
-private import Private
 private import Public
 
 class SummaryPostUpdateNode extends FlowSummaryNode, PostUpdateNode {
diff --git a/go/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.ql b/go/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.ql
@@ -6,7 +6,8 @@
  * @precision low
  * @problem.severity error
  * @security-severity 7.8
- * @tags security external/cwe/cwe-020
+ * @tags security
+ *       external/cwe/cwe-020
  */
 
 import go
diff --git a/go/ql/src/Security/CWE-020/UntrustedDataToUnknownExternalAPI.ql b/go/ql/src/Security/CWE-020/UntrustedDataToUnknownExternalAPI.ql
@@ -6,7 +6,8 @@
  * @precision low
  * @problem.severity error
  * @security-severity 7.8
- * @tags security external/cwe/cwe-020
+ * @tags security
+ *       external/cwe/cwe-020
  */
 
 import go
diff --git a/go/ql/src/experimental/CWE-918/validator.qll b/go/ql/src/experimental/CWE-918/validator.qll
@@ -24,7 +24,7 @@ class FieldWithTags extends FieldDecl {
    * For example: the tag `json:"word" binding:"required,alpha"` yields `key: "json", value: "word"`
    * and `key: "binding" values: "required","alpha"`.
    */
-  predicate getTagByKeyValue(string key, string value) {
+  predicate hasTagKeyValue(string key, string value) {
     exists(string tag, string key_value, string values |
       this.getTag().toString() = tag and
       // Each key_value is like key:"value1,value2"
@@ -50,7 +50,7 @@ class AlphanumericStructFieldRead extends DataFlow::Node {
     exists(FieldWithTags decl, Field field, string tag |
       this = field.getARead() and
       field.getDeclaration() = decl.getNameExpr(0) and
-      decl.getTagByKeyValue(key, tag) and
+      decl.hasTagKeyValue(key, tag) and
       isAlphanumericValidationKind(tag)
     )
   }
diff --git a/go/ql/src/experimental/frameworks/DecompressionBombsCustomizations.qll b/go/ql/src/experimental/frameworks/DecompressionBombsCustomizations.qll
@@ -511,7 +511,7 @@ module DecompressionBombs {
   }
 
   /**
-   * Provides decompression bomb sinks for packages that use some standard IO interfaces/methods for reading decompressed data
+   * A standard IO function for reading decompressed data.
    */
   class GeneralReadIoSink extends Sink {
     GeneralReadIoSink() {
diff --git a/go/ql/test/library-tests/semmle/go/dataflow/GlobalVariableSideEffects/Flows.ql b/go/ql/test/library-tests/semmle/go/dataflow/GlobalVariableSideEffects/Flows.ql
@@ -2,7 +2,6 @@ import go
 import utils.test.InlineFlowTest
 
 string getArgString(DataFlow::Node src, DataFlow::Node sink) {
-  exists(src) and
   result =
     "\"" + sink.toString() + " (from source " +
       src.(DataFlow::CallNode).getArgument(0).getExactValue() + ")\""
diff --git a/java/ql/lib/semmle/code/java/JDK.qll b/java/ql/lib/semmle/code/java/JDK.qll
@@ -321,12 +321,7 @@ class WriteObjectMethod extends Method {
 class ReadObjectMethod extends Method {
   ReadObjectMethod() {
     this.getDeclaringType() instanceof TypeObjectInputStream and
-    (
-      this.hasName("readObject") or
-      this.hasName("readObjectOverride") or
-      this.hasName("readUnshared") or
-      this.hasName("resolveObject")
-    )
+    this.hasName(["readObject", "readObjectOverride", "readUnshared", "resolveObject"])
   }
 }
 
diff --git a/java/ql/lib/semmle/code/java/NumberFormatException.qll b/java/ql/lib/semmle/code/java/NumberFormatException.qll
@@ -46,12 +46,7 @@ private class SpecialClassInstanceExpr extends ClassInstanceExpr {
   }
 
   predicate throwsNfe() {
-    this.isStringConstructor("Byte") or
-    this.isStringConstructor("Short") or
-    this.isStringConstructor("Integer") or
-    this.isStringConstructor("Long") or
-    this.isStringConstructor("Float") or
-    this.isStringConstructor("Double")
+    this.isStringConstructor(["Byte", "Short", "Integer", "Long", "Float", "Double"])
   }
 }
 
diff --git a/java/ql/lib/semmle/code/java/frameworks/JAXB.qll b/java/ql/lib/semmle/code/java/frameworks/JAXB.qll
@@ -107,10 +107,7 @@ class XmlAccessType extends EnumConstant {
  */
 class JaxbMemberAnnotation extends JaxbAnnotationType {
   JaxbMemberAnnotation() {
-    this.hasName("XmlElement") or
-    this.hasName("XmlAttribute") or
-    this.hasName("XmlElementRefs") or
-    this.hasName("XmlElements")
+    this.hasName(["XmlElement", "XmlAttribute", "XmlElementRefs", "XmlElements"])
   }
 }
 
diff --git a/java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJB.qll b/java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJB.qll
@@ -677,7 +677,7 @@ Type inheritsMatchingMethodExceptThrows(SessionEjb ejb, Method m) {
 }
 
 /**
- * Holds if `ejb` inherits an `ejbCreate` or `@Init` method matching `create` method `m`.
+ * Holds if `ejb` inherits an `ejbCreate` or `@Init` method matching `create` method `icm`.
  * (Ignores `throws` clauses.)
  */
 predicate inheritsMatchingCreateMethodIgnoreThrows(
@@ -704,7 +704,7 @@ predicate inheritsMatchingCreateMethodIgnoreThrows(
 }
 
 /**
- * If `ejb` inherits an `ejbCreate` or `@Init` method matching `create` method `m` except for the `throws` clause,
+ * If `ejb` inherits an `ejbCreate` or `@Init` method matching `create` method `icm` except for the `throws` clause,
  * then return any type in the `throws` clause that does not match.
  */
 Type inheritsMatchingCreateMethodExceptThrows(StatefulSessionEjb ejb, EjbInterfaceCreateMethod icm) {
diff --git a/java/ql/lib/semmle/code/java/frameworks/spring/SpringController.qll b/java/ql/lib/semmle/code/java/frameworks/spring/SpringController.qll
@@ -187,13 +187,10 @@ class SpringServletInputAnnotation extends Annotation {
       a = this.getType() and
       a.getPackage().getName() = "org.springframework.web.bind.annotation"
     |
-      a.hasName("MatrixVariable") or
-      a.hasName("RequestParam") or
-      a.hasName("RequestHeader") or
-      a.hasName("CookieValue") or
-      a.hasName("RequestPart") or
-      a.hasName("PathVariable") or
-      a.hasName("RequestBody")
+      a.hasName([
+          "MatrixVariable", "RequestParam", "RequestHeader", "CookieValue", "RequestPart",
+          "PathVariable", "RequestBody"
+        ])
     )
   }
 }
diff --git a/java/ql/lib/semmle/code/java/frameworks/struts/StrutsActions.qll b/java/ql/lib/semmle/code/java/frameworks/struts/StrutsActions.qll
@@ -40,26 +40,15 @@ class Struts2ActionClass extends Class {
       getStrutsMapperClass(this) = "org.apache.struts2.dispatcher.mapper.RestfulActionMapper"
     then
       // The "Restful" action mapper maps rest APIs to specific methods
-      result.hasName("index") or
-      result.hasName("create") or
-      result.hasName("editNew") or
-      result.hasName("view") or
-      result.hasName("remove") or
-      result.hasName("update")
+      result.hasName(["index", "create", "editNew", "view", "remove", "update"])
     else
       if
         getStrutsMapperClass(this) = "org.apache.struts2.rest.RestActionMapper" or
         getStrutsMapperClass(this) = "rest"
       then
         // The "Rest" action mapper is provided with the rest plugin, and maps rest APIs to specific
         // methods based on a "ruby-on-rails" style.
-        result.hasName("index") or
-        result.hasName("show") or
-        result.hasName("edit") or
-        result.hasName("editNew") or
-        result.hasName("create") or
-        result.hasName("update") or
-        result.hasName("destroy")
+        result.hasName(["index", "show", "edit", "editNew", "create", "update", "destroy"])
       else
         if exists(getStrutsMapperClass(this))
         then
diff --git a/java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql b/java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql
@@ -6,7 +6,8 @@
  * @precision low
  * @problem.severity error
  * @security-severity 7.8
- * @tags security external/cwe/cwe-020
+ * @tags security
+ *       external/cwe/cwe-020
  */
 
 import java

Original file line number	Diff line number	Diff line change
`@@ -1347,7 +1347,6 @@ module Public {`
`1347`	`1347`	`}`
`1348`	`1348`	`}`
`1349`	`1349`
`1350`		`-private import Private`
`1351`	`1350`	`private import Public`
`1352`	`1351`
`1353`	`1352`	`class SummaryPostUpdateNode extends FlowSummaryNode, PostUpdateNode {`
Original file line number	Diff line number	Diff line change
`@@ -511,7 +511,7 @@ module DecompressionBombs {`
`511`	`511`	`}`
`512`	`512`
`513`	`513`	`/**`
`514`		`- * Provides decompression bomb sinks for packages that use some standard IO interfaces/methods for reading decompressed data`
	`514`	`+ * A standard IO function for reading decompressed data.`
`515`	`515`	`*/`
`516`	`516`	`class GeneralReadIoSink extends Sink {`
`517`	`517`	`GeneralReadIoSink() {`
Original file line number	Diff line number	Diff line change
`@@ -321,12 +321,7 @@ class WriteObjectMethod extends Method {`
`321`	`321`	`class ReadObjectMethod extends Method {`
`322`	`322`	`ReadObjectMethod() {`
`323`	`323`	`this.getDeclaringType() instanceof TypeObjectInputStream and`
`324`		`- (`
`325`		`- this.hasName("readObject") or`
`326`		`- this.hasName("readObjectOverride") or`
`327`		`- this.hasName("readUnshared") or`
`328`		`- this.hasName("resolveObject")`
`329`		`- )`
	`324`	`+ this.hasName(["readObject", "readObjectOverride", "readUnshared", "resolveObject"])`
`330`	`325`	`}`
`331`	`326`	`}`
`332`	`327`
Original file line number	Diff line number	Diff line change
`@@ -46,12 +46,7 @@ private class SpecialClassInstanceExpr extends ClassInstanceExpr {`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`predicate throwsNfe() {`
`49`		`- this.isStringConstructor("Byte") or`
`50`		`- this.isStringConstructor("Short") or`
`51`		`- this.isStringConstructor("Integer") or`
`52`		`- this.isStringConstructor("Long") or`
`53`		`- this.isStringConstructor("Float") or`
`54`		`- this.isStringConstructor("Double")`
	`49`	`+ this.isStringConstructor(["Byte", "Short", "Integer", "Long", "Float", "Double"])`
`55`	`50`	`}`
`56`	`51`	`}`
`57`	`52`
Original file line number	Diff line number	Diff line change
`@@ -107,10 +107,7 @@ class XmlAccessType extends EnumConstant {`
`107`	`107`	`*/`
`108`	`108`	`class JaxbMemberAnnotation extends JaxbAnnotationType {`
`109`	`109`	`JaxbMemberAnnotation() {`
`110`		`- this.hasName("XmlElement") or`
`111`		`- this.hasName("XmlAttribute") or`
`112`		`- this.hasName("XmlElementRefs") or`
`113`		`- this.hasName("XmlElements")`
	`110`	`+ this.hasName(["XmlElement", "XmlAttribute", "XmlElementRefs", "XmlElements"])`
`114`	`111`	`}`
`115`	`112`	`}`
`116`	`113`