Merge pull request #20326 from michaelnebel/csharp/ql4ql

michaelnebel · web-flow · commit 39d3741a9141 · 2025-09-02T11:44:22.000+02:00
C#: Fix some Ql4Ql violations.
diff --git a/csharp/ql/lib/printAst.ql b/csharp/ql/lib/printAst.ql
@@ -18,8 +18,8 @@ external string selectedSourceFile();
 
 class PrintAstConfigurationOverride extends PrintAstConfiguration {
   /**
-   * Holds if the location matches the selected file in the VS Code extension and
-   * the element is `fromSource`.
+   * Holds if the location `l` matches the selected file in the VS Code extension and
+   * the element is `e` is `fromSource`.
    */
   override predicate shouldPrint(Element e, Location l) {
     super.shouldPrint(e, l) and
diff --git a/csharp/ql/lib/semmle/code/csharp/Assignable.qll b/csharp/ql/lib/semmle/code/csharp/Assignable.qll
@@ -583,7 +583,7 @@ module AssignableDefinitions {
   }
 
   /**
-   * Holds if the `ref` assignment to `aa` via call `c` is uncertain.
+   * Holds if the `ref` assignment to `arg` via call `c` is uncertain.
    */
   // Not in the cached module `Cached`, as that would introduce a dependency
   // on the CFG construction, and effectively collapse too many stages into one
diff --git a/csharp/ql/lib/semmle/code/csharp/Callable.qll b/csharp/ql/lib/semmle/code/csharp/Callable.qll
@@ -708,7 +708,7 @@ class TrueOperator extends UnaryOperator {
  *
  * Either an addition operator (`AddOperator`), a checked addition operator
  * (`CheckedAddOperator`) a subtraction operator (`SubOperator`), a checked
- * substraction operator (`CheckedSubOperator`), a multiplication operator
+ * subtraction operator (`CheckedSubOperator`), a multiplication operator
  * (`MulOperator`), a checked multiplication operator (`CheckedMulOperator`),
  * a division operator (`DivOperator`), a checked division operator
  * (`CheckedDivOperator`), a remainder operator (`RemOperator`), an and
diff --git a/csharp/ql/lib/semmle/code/csharp/Member.qll b/csharp/ql/lib/semmle/code/csharp/Member.qll
@@ -491,7 +491,7 @@ class Parameterizable extends Declaration, @parameterizable {
   final Parameter getARawParameter() { result = this.getRawParameter(_) }
 
   /**
-   * Gets the type of the parameter, possibly prefixed
+   * Gets the type of the `i`th parameter, possibly prefixed
    * with `out`, `ref`, or `params`, where appropriate.
    */
   private string parameterTypeToString(int i) {
diff --git a/csharp/ql/lib/semmle/code/csharp/PrintAst.qll b/csharp/ql/lib/semmle/code/csharp/PrintAst.qll
@@ -523,11 +523,9 @@ final class AttributeNode extends ElementNode {
  * A node representing a `TypeParameter`.
  */
 final class TypeParameterNode extends ElementNode {
-  TypeParameter typeParameter;
-
   TypeParameterNode() {
-    typeParameter = element and
-    not isNotNeeded(typeParameter.getDeclaringGeneric())
+    element =
+      any(TypeParameter typeParameter | not isNotNeeded(typeParameter.getDeclaringGeneric()))
   }
 
   override ElementNode getChild(int childIndex) { none() }
diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/Completion.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/Completion.qll
@@ -310,10 +310,8 @@ private class Overflowable extends UnaryOperation {
 
 /** A control flow element that is inside a `try` block. */
 private class TriedControlFlowElement extends ControlFlowElement {
-  TryStmt try;
-
   TriedControlFlowElement() {
-    this = try.getATriedElement() and
+    this = any(TryStmt try).getATriedElement() and
     not this instanceof NonReturningCall
   }
 
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll
@@ -451,10 +451,9 @@ module Ssa {
    * An SSA definition that corresponds to an explicit assignable definition.
    */
   class ExplicitDefinition extends Definition, SsaImpl::WriteDefinition {
-    SourceVariable sv;
     AssignableDefinition ad;
 
-    ExplicitDefinition() { SsaImpl::explicitDefinition(this, sv, ad) }
+    ExplicitDefinition() { SsaImpl::explicitDefinition(this, _, ad) }
 
     /**
      * Gets an underlying assignable definition. The result is always unique,
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll
@@ -143,7 +143,7 @@ private module SourceVariableImpl {
   }
 
   /**
-   * Gets an `out`/`ref` definition of the same source variable as the `out`/`ref`
+   * Gets an `out`/`ref` definition of the same source variable `v` as the `out`/`ref`
    * definition `def`, belonging to the same call, at a position after `def`.
    */
   OutRefDefinition getASameOutRefDefAfter(Ssa::SourceVariable v, OutRefDefinition def) {
diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/UnsafeDeserializationQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/UnsafeDeserializationQuery.qll
@@ -874,7 +874,7 @@ private predicate isStrongTypeFsPicklerCall(MethodCall mc, Method m) {
   (
     m instanceof FsPicklerSerializerClassDeserializeMethod or
     m instanceof FsPicklerSerializerClassDeserializeSequenceMethod or
-    m instanceof FsPicklerSerializerClasDeserializeSiftedMethod or
+    m instanceof FsPicklerSerializerClassDeserializeSiftedMethod or
     m instanceof FsPicklerSerializerClassUnPickleMethod or
     m instanceof FsPicklerSerializerClassUnPickleSiftedMethod or
     m instanceof CsPicklerSerializerClassDeserializeMethod or
diff --git a/csharp/ql/lib/semmle/code/csharp/serialization/Deserializers.qll b/csharp/ql/lib/semmle/code/csharp/serialization/Deserializers.qll
@@ -560,9 +560,15 @@ class FsPicklerSerializerClassDeserializeSequenceMethod extends Method, UnsafeDe
   }
 }
 
+/**
+ * DEPRECATED: Use `FsPicklerSerializerClassDeserializeSiftedMethod` instead.
+ */
+deprecated class FsPicklerSerializerClasDeserializeSiftedMethod =
+  FsPicklerSerializerClassDeserializeSiftedMethod;
+
 /** `MBrace.FsPickler.FsPicklerSerializer.DeserializeSifted` method */
-class FsPicklerSerializerClasDeserializeSiftedMethod extends Method, UnsafeDeserializer {
-  FsPicklerSerializerClasDeserializeSiftedMethod() {
+class FsPicklerSerializerClassDeserializeSiftedMethod extends Method, UnsafeDeserializer {
+  FsPicklerSerializerClassDeserializeSiftedMethod() {
     this.getDeclaringType().getBaseClass*() instanceof FsPicklerSerializerClass and
     this.hasUndecoratedName("DeserializeSifted")
   }
diff --git a/csharp/ql/src/Bad Practices/Magic Constants/MagicConstants.qll b/csharp/ql/src/Bad Practices/Magic Constants/MagicConstants.qll
@@ -113,7 +113,7 @@ private predicate valueOccurrenceCount(string value, int n) {
   n > 20
 }
 
-private predicate occurenceCount(Literal lit, string value, int n) {
+private predicate occurrenceCount(Literal lit, string value, int n) {
   valueOccurrenceCount(value, n) and
   value = lit.getValue() and
   relevantLiteral(lit, value)
@@ -127,7 +127,7 @@ private predicate check(Literal lit, string value, int n, File f) {
   // Check that the literal is nontrivial
   not trivial(lit) and
   // Check that it is repeated a number of times
-  occurenceCount(lit, value, n) and
+  occurrenceCount(lit, value, n) and
   n > 20 and
   f = lit.getFile()
 }
diff --git a/csharp/ql/src/Language Abuse/UselessUpcast.ql b/csharp/ql/src/Language Abuse/UselessUpcast.ql
@@ -75,15 +75,16 @@ private class ConstructorCall extends Call {
 
 /** An explicit upcast. */
 class ExplicitUpcast extends ExplicitCast {
-  ValueOrRefType src;
   ValueOrRefType dest;
 
   ExplicitUpcast() {
-    src = this.getSourceType() and
-    dest = this.getTargetType() and
-    (src instanceof RefType or src instanceof Struct) and
-    src.isImplicitlyConvertibleTo(dest) and
-    src != dest // Handled by `cs/useless-cast-to-self`
+    exists(ValueOrRefType src |
+      src = this.getSourceType() and
+      dest = this.getTargetType() and
+      (src instanceof RefType or src instanceof Struct) and
+      src.isImplicitlyConvertibleTo(dest) and
+      src != dest // Handled by `cs/useless-cast-to-self`
+    )
   }
 
   pragma[nomagic]
diff --git a/csharp/ql/src/Telemetry/DatabaseQuality.qll b/csharp/ql/src/Telemetry/DatabaseQuality.qll
@@ -12,7 +12,7 @@ module CallTargetStats implements StatsSig {
   private predicate isNoSetterPropertyCallInConstructor(PropertyCall c) {
     exists(Property p, Constructor ctor |
       p = c.getProperty() and
-      not exists(Setter a | a = p.getAnAccessor()) and
+      not p.getAnAccessor() instanceof Setter and
       c.getEnclosingCallable() = ctor and
       (
         c.hasThisQualifier()
@@ -25,7 +25,7 @@ module CallTargetStats implements StatsSig {
   private predicate isNoSetterPropertyInitialization(PropertyCall c) {
     exists(Property p, AssignExpr assign |
       p = c.getProperty() and
-      not exists(Setter a | a = p.getAnAccessor()) and
+      not p.getAnAccessor() instanceof Setter and
       assign = c.getParent() and
       assign.getLValue() = c and
       assign.getParent() instanceof Property

Original file line number	Diff line number	Diff line change
`@@ -583,7 +583,7 @@ module AssignableDefinitions {`
`583`	`583`	`}`
`584`	`584`
`585`	`585`	`/**`
`586`		- * Holds if the `ref` assignment to `aa` via call `c` is uncertain.
	`586`	+ * Holds if the `ref` assignment to `arg` via call `c` is uncertain.
`587`	`587`	`*/`
`588`	`588`	// Not in the cached module `Cached`, as that would introduce a dependency
`589`	`589`	`// on the CFG construction, and effectively collapse too many stages into one`
Original file line number	Diff line number	Diff line change
`@@ -708,7 +708,7 @@ class TrueOperator extends UnaryOperator {`
`708`	`708`	`*`
`709`	`709`	* Either an addition operator (`AddOperator`), a checked addition operator
`710`	`710`	* (`CheckedAddOperator`) a subtraction operator (`SubOperator`), a checked
`711`		- * substraction operator (`CheckedSubOperator`), a multiplication operator
	`711`	+ * subtraction operator (`CheckedSubOperator`), a multiplication operator
`712`	`712`	* (`MulOperator`), a checked multiplication operator (`CheckedMulOperator`),
`713`	`713`	* a division operator (`DivOperator`), a checked division operator
`714`	`714`	* (`CheckedDivOperator`), a remainder operator (`RemOperator`), an and
Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,7 @@ private module SourceVariableImpl {`
`143`	`143`	`}`
`144`	`144`
`145`	`145`	`/**`
`146`		- * Gets an `out`/`ref` definition of the same source variable as the `out`/`ref`
	`146`	+ * Gets an `out`/`ref` definition of the same source variable `v` as the `out`/`ref`
`147`	`147`	* definition `def`, belonging to the same call, at a position after `def`.
`148`	`148`	`*/`
`149`	`149`	`OutRefDefinition getASameOutRefDefAfter(Ssa::SourceVariable v, OutRefDefinition def) {`
Original file line number	Diff line number	Diff line change
`@@ -874,7 +874,7 @@ private predicate isStrongTypeFsPicklerCall(MethodCall mc, Method m) {`
`874`	`874`	`(`
`875`	`875`	`m instanceof FsPicklerSerializerClassDeserializeMethod or`
`876`	`876`	`m instanceof FsPicklerSerializerClassDeserializeSequenceMethod or`
`877`		`- m instanceof FsPicklerSerializerClasDeserializeSiftedMethod or`
	`877`	`+ m instanceof FsPicklerSerializerClassDeserializeSiftedMethod or`
`878`	`878`	`m instanceof FsPicklerSerializerClassUnPickleMethod or`
`879`	`879`	`m instanceof FsPicklerSerializerClassUnPickleSiftedMethod or`
`880`	`880`	`m instanceof CsPicklerSerializerClassDeserializeMethod or`
Original file line number	Diff line number	Diff line change
`@@ -560,9 +560,15 @@ class FsPicklerSerializerClassDeserializeSequenceMethod extends Method, UnsafeDe`
`560`	`560`	`}`
`561`	`561`	`}`
`562`	`562`
	`563`	`+/**`
	`564`	+ * DEPRECATED: Use `FsPicklerSerializerClassDeserializeSiftedMethod` instead.
	`565`	`+ */`
	`566`	`+deprecated class FsPicklerSerializerClasDeserializeSiftedMethod =`
	`567`	`+ FsPicklerSerializerClassDeserializeSiftedMethod;`
	`568`	`+`
`563`	`569`	/** `MBrace.FsPickler.FsPicklerSerializer.DeserializeSifted` method */
`564`		`-class FsPicklerSerializerClasDeserializeSiftedMethod extends Method, UnsafeDeserializer {`
`565`		`- FsPicklerSerializerClasDeserializeSiftedMethod() {`
	`570`	`+class FsPicklerSerializerClassDeserializeSiftedMethod extends Method, UnsafeDeserializer {`
	`571`	`+ FsPicklerSerializerClassDeserializeSiftedMethod() {`
`566`	`572`	`this.getDeclaringType().getBaseClass*() instanceof FsPicklerSerializerClass and`
`567`	`573`	`this.hasUndecoratedName("DeserializeSifted")`
`568`	`574`	`}`