Address comments

Author: aibaars

ruby/ql/lib/codeql/ruby/InclusionTests.qll

@@ -19,18 +19,18 @@ private import codeql.ruby.controlflow.CfgNodes
  */
 class InclusionTest extends DataFlow::Node instanceof InclusionTest::Range {
   /** Gets the `A` in `A.include?(B)`. */
-  DataFlow::Node getContainerNode() { result = super.getContainerNode() }
+  final DataFlow::Node getContainerNode() { result = super.getContainerNode() }
 
   /** Gets the `B` in `A.include?(B)`. */
-  DataFlow::Node getContainedNode() { result = super.getContainedNode() }
+  final DataFlow::Node getContainedNode() { result = super.getContainedNode() }
 
   /**
    * Gets the polarity of the check.
    *
    * If the polarity is `false` the check returns `true` if the container does not contain
    * the given element.
    */
-  boolean getPolarity() { result = super.getPolarity() }
+  final boolean getPolarity() { result = super.getPolarity() }
 }
 
 /**
@@ -65,7 +65,7 @@ module InclusionTest {
   private class Includes_Native extends Range, DataFlow::CallNode {
     Includes_Native() {
       this.getMethodName() = "include?" and
-      count(this.getArgument(_)) = 1
+      strictcount(this.getArgument(_)) = 1
     }
 
     override DataFlow::Node getContainerNode() { result = this.getReceiver() }
@@ -91,7 +91,7 @@ module InclusionTest {
         (
           value = index.getConstantValue().getInt() and value = 0
           or
-          index.getExpr() instanceof NilLiteral and value = -1
+          index.getConstantValue().isNil() and value = -1
         )
       |
         value = -1 and polarity = false and comparison.getExpr() instanceof CaseEqExpr

ruby/ql/lib/codeql/ruby/StringOps.qll

@@ -18,20 +18,20 @@ module StringOps {
     /**
      * Gets the `A` in `A.start_with?(B)`.
      */
-    DataFlow::Node getBaseString() { result = super.getBaseString() }
+    final DataFlow::Node getBaseString() { result = super.getBaseString() }
 
     /**
      * Gets the `B` in `A.start_with?(B)`.
      */
-    DataFlow::Node getSubstring() { result = super.getSubstring() }
+    final DataFlow::Node getSubstring() { result = super.getSubstring() }
 
     /**
      * Gets the polarity of the check.
      *
      * If the polarity is `false` the check returns `true` if the string does not start
      * with the given substring.
      */
-    boolean getPolarity() { result = super.getPolarity() }
+    final boolean getPolarity() { result = super.getPolarity() }
   }
 
   /**
@@ -75,15 +75,15 @@ module StringOps {
     /**
      * An expression of form `A.index(B) == 0` or `A.index(B) != 0`.
      */
-    private class StartsWith_IndexOfEquals extends Range, DataFlow::Node {
+    private class StartsWith_IndexOfEquals extends Range {
       private DataFlow::CallNode indexOf;
       private boolean polarity;
 
       StartsWith_IndexOfEquals() {
         exists(ExprNodes::ComparisonOperationCfgNode comparison |
           this.asExpr() = comparison and
           indexOf.getMethodName() = "index" and
-          count(indexOf.getArgument(_)) = 1 and
+          strictcount(indexOf.getArgument(_)) = 1 and
           indexOf.flowsTo(any(DataFlow::Node n | n.asExpr() = comparison.getAnOperand())) and
           comparison.getAnOperand().getConstantValue().getInt() = 0
         |
@@ -110,10 +110,10 @@ module StringOps {
    */
   class Includes extends InclusionTest {
     /** Gets the `A` in `A.include?(B)`. */
-    DataFlow::Node getBaseString() { result = super.getContainerNode() }
+    final DataFlow::Node getBaseString() { result = super.getContainerNode() }
 
     /** Gets the `B` in `A.include?(B)`. */
-    DataFlow::Node getSubstring() { result = super.getContainedNode() }
+    final DataFlow::Node getSubstring() { result = super.getContainedNode() }
   }
 
   /**
@@ -123,20 +123,20 @@ module StringOps {
     /**
      * Gets the `A` in `A.start_with?(B)`.
      */
-    DataFlow::Node getBaseString() { result = super.getBaseString() }
+    final DataFlow::Node getBaseString() { result = super.getBaseString() }
 
     /**
      * Gets the `B` in `A.start_with?(B)`.
      */
-    DataFlow::Node getSubstring() { result = super.getSubstring() }
+    final DataFlow::Node getSubstring() { result = super.getSubstring() }
 
     /**
      * Gets the polarity if the check.
      *
      * If the polarity is `false` the check returns `true` if the string does not end
      * with the given substring.
      */
-    boolean getPolarity() { result = super.getPolarity() }
+    final boolean getPolarity() { result = super.getPolarity() }
   }
 
   /**

ruby/ql/src/queries/security/cwe-020/IncompleteUrlSubstringSanitizationSpecific.qll

@@ -4,5 +4,5 @@ import codeql.ruby.security.performance.RegExpTreeView::RegExpPatterns as RegExp
 
 /** Holds if `node` may evaluate to `value` */
 predicate mayHaveStringValue(DataFlow::Node node, string value) {
-  node.asExpr().getExpr().getConstantValue().getString() = value
+  node.asExpr().getConstantValue().getString() = value
 }