Ruby: fix QL warnings

Author: aibaars

ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll

@@ -414,10 +414,10 @@ class TVariableReal =
 class TLocalVariable = TLocalVariableReal or TLocalVariableSynth or TSelfVariable;
 
 /**
- * This class only exists to avoid negative recursion warnings. Ideally,
- * we would use `VariableImpl` directly, but that results in incorrect
- * negative recursion warnings. Adding new root-defs for the predicates
- * below works around this.
+ * A "real" (ie. non-synthesized) variable. This class only exists to
+ * avoid negative recursion warnings. Ideally, we would use `VariableImpl`
+ * directly, but that results in incorrect negative recursion warnings.
+ * Adding new root-defs for the predicates below works around this.
  */
 abstract class VariableReal extends TVariableReal {
   abstract string getNameImpl();

ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImplShared.qll

@@ -214,7 +214,7 @@ abstract class SplitKind extends SplitKindBase {
   abstract string toString();
 }
 
-/** Provides the interface for implementing an entity to split on. */
+/** An interface for implementing an entity to split on. */
 abstract class SplitImpl extends Split {
   /** Gets the kind of this split. */
   abstract SplitKind getKind();

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplCommon.qll

@@ -1290,7 +1290,7 @@ class DataFlowCallOption extends TDataFlowCallOption {
   }
 }
 
-/** Content tagged with the type of a containing object. */
+/** A `Content` tagged with the type of a containing object. */
 class TypedContent extends MkTypedContent {
   private Content c;
   private DataFlowType t;

ruby/ql/lib/codeql/ruby/frameworks/StandardLibrary.qll

@@ -347,7 +347,7 @@ class ModuleConstGetCallCodeExecution extends CodeExecution::Range, DataFlow::Ca
   override DataFlow::Node getCode() { result = this.getArgument(0) }
 }
 
-/** Flow summary for `Regexp.escape` and its alias, `Regexp.quote`. */
+/** A flow summary for `Regexp.escape` and its alias, `Regexp.quote`. */
 class RegexpEscapeSummary extends SummarizedCallable {
   RegexpEscapeSummary() { this = "Regexp.escape" }
 

ruby/ql/lib/codeql/ruby/security/ServerSideRequestForgeryCustomizations.qll

@@ -46,6 +46,6 @@ module ServerSideRequestForgery {
     HttpRequestAsSink() { exists(HTTP::Client::Request req | req.getURL() = this) }
   }
 
-  /** String interpolation with a fixed prefix, considered as a flow sanitizer. */
+  /** A string interpolation with a fixed prefix, considered as a flow sanitizer. */
   class StringInterpolationAsSanitizer extends PrefixedStringInterpolation, Sanitizer { }
 }

ruby/ql/lib/codeql/ruby/security/performance/ReDoSUtil.qll

@@ -1052,13 +1052,13 @@ private module SuffixConstruction {
    */
   pragma[noinline]
   private string relevant(RegExpRoot root) {
-    exists(ascii(result))
+    exists(ascii(result)) and exists(root)
     or
     exists(InputSymbol s | belongsTo(s, root) | result = intersect(s, _))
     or
     // The characters from `hasSimpleRejectEdge`. Only `\n` is really needed (as `\n` is not in the `ascii` relation).
     // The three chars must be kept in sync with `hasSimpleRejectEdge`.
-    result = ["|", "\n", "Z"]
+    result = ["|", "\n", "Z"] and exists(root)
   }
 
   /**

ruby/ql/lib/codeql/ruby/typetracking/TypeTracker.qll

@@ -3,7 +3,7 @@
 private import TypeTrackerSpecific
 
 /**
- * Any string that may appear as the name of a piece of content. This will usually include things like:
+ * A string that may appear as the name of a piece of content. This will usually include things like:
  * - Attribute names (in Python)
  * - Property names (in JavaScript)
  *
@@ -18,7 +18,7 @@ class ContentName extends string {
   ContentName() { this = getPossibleContentName() }
 }
 
-/** Either a content name, or the empty string (representing no content). */
+/** A content name, or the empty string (representing no content). */
 class OptionalContentName extends string {
   OptionalContentName() { this instanceof ContentName or this = "" }
 }
@@ -200,7 +200,7 @@ module StepSummary {
 private newtype TTypeTracker = MkTypeTracker(Boolean hasCall, OptionalContentName content)
 
 /**
- * Summary of the steps needed to track a value to a given dataflow node.
+ * A summary of the steps needed to track a value to a given dataflow node.
  *
  * This can be used to track objects that implement a certain API in order to
  * recognize calls to that API. Note that type-tracking does not by itself provide a
@@ -347,7 +347,7 @@ module TypeTracker {
 private newtype TTypeBackTracker = MkTypeBackTracker(Boolean hasReturn, OptionalContentName content)
 
 /**
- * Summary of the steps needed to back-track a use of a value to a given dataflow node.
+ * A summary of the steps needed to back-track a use of a value to a given dataflow node.
  *
  * This can for example be used to track callbacks that are passed to a certain API,
  * so we can model specific parameters of that callback as having a certain type.

ruby/ql/lib/tutorial.qll

@@ -6,122 +6,22 @@
  */
 class Person extends string {
   Person() {
-    this = "Ronil" or
-    this = "Dina" or
-    this = "Ravi" or
-    this = "Bruce" or
-    this = "Jo" or
-    this = "Aida" or
-    this = "Esme" or
-    this = "Charlie" or
-    this = "Fred" or
-    this = "Meera" or
-    this = "Maya" or
-    this = "Chad" or
-    this = "Tiana" or
-    this = "Laura" or
-    this = "George" or
-    this = "Will" or
-    this = "Mary" or
-    this = "Almira" or
-    this = "Susannah" or
-    this = "Rhoda" or
-    this = "Cynthia" or
-    this = "Eunice" or
-    this = "Olive" or
-    this = "Virginia" or
-    this = "Angeline" or
-    this = "Helen" or
-    this = "Cornelia" or
-    this = "Harriet" or
-    this = "Mahala" or
-    this = "Abby" or
-    this = "Margaret" or
-    this = "Deb" or
-    this = "Minerva" or
-    this = "Severus" or
-    this = "Lavina" or
-    this = "Adeline" or
-    this = "Cath" or
-    this = "Elisa" or
-    this = "Lucretia" or
-    this = "Anne" or
-    this = "Eleanor" or
-    this = "Joanna" or
-    this = "Adam" or
-    this = "Agnes" or
-    this = "Rosanna" or
-    this = "Clara" or
-    this = "Melissa" or
-    this = "Amy" or
-    this = "Isabel" or
-    this = "Jemima" or
-    this = "Cordelia" or
-    this = "Melinda" or
-    this = "Delila" or
-    this = "Jeremiah" or
-    this = "Elijah" or
-    this = "Hester" or
-    this = "Walter" or
-    this = "Oliver" or
-    this = "Hugh" or
-    this = "Aaron" or
-    this = "Reuben" or
-    this = "Eli" or
-    this = "Amos" or
-    this = "Augustus" or
-    this = "Theodore" or
-    this = "Ira" or
-    this = "Timothy" or
-    this = "Cyrus" or
-    this = "Horace" or
-    this = "Simon" or
-    this = "Asa" or
-    this = "Frank" or
-    this = "Nelson" or
-    this = "Leonard" or
-    this = "Harrison" or
-    this = "Anthony" or
-    this = "Louis" or
-    this = "Milton" or
-    this = "Noah" or
-    this = "Cornelius" or
-    this = "Abdul" or
-    this = "Warren" or
-    this = "Harvey" or
-    this = "Dennis" or
-    this = "Wesley" or
-    this = "Sylvester" or
-    this = "Gilbert" or
-    this = "Sullivan" or
-    this = "Edmund" or
-    this = "Wilson" or
-    this = "Perry" or
-    this = "Matthew" or
-    this = "Simba" or
-    this = "Nala" or
-    this = "Rafiki" or
-    this = "Shenzi" or
-    this = "Ernest" or
-    this = "Gertrude" or
-    this = "Oscar" or
-    this = "Lilian" or
-    this = "Raymond" or
-    this = "Elgar" or
-    this = "Elmer" or
-    this = "Herbert" or
-    this = "Maude" or
-    this = "Mae" or
-    this = "Otto" or
-    this = "Edwin" or
-    this = "Ophelia" or
-    this = "Parsley" or
-    this = "Sage" or
-    this = "Rosemary" or
-    this = "Thyme" or
-    this = "Garfunkel" or
-    this = "King Basil" or
-    this = "Stephen"
+    this =
+      [
+        "Ronil", "Dina", "Ravi", "Bruce", "Jo", "Aida", "Esme", "Charlie", "Fred", "Meera", "Maya",
+        "Chad", "Tiana", "Laura", "George", "Will", "Mary", "Almira", "Susannah", "Rhoda",
+        "Cynthia", "Eunice", "Olive", "Virginia", "Angeline", "Helen", "Cornelia", "Harriet",
+        "Mahala", "Abby", "Margaret", "Deb", "Minerva", "Severus", "Lavina", "Adeline", "Cath",
+        "Elisa", "Lucretia", "Anne", "Eleanor", "Joanna", "Adam", "Agnes", "Rosanna", "Clara",
+        "Melissa", "Amy", "Isabel", "Jemima", "Cordelia", "Melinda", "Delila", "Jeremiah", "Elijah",
+        "Hester", "Walter", "Oliver", "Hugh", "Aaron", "Reuben", "Eli", "Amos", "Augustus",
+        "Theodore", "Ira", "Timothy", "Cyrus", "Horace", "Simon", "Asa", "Frank", "Nelson",
+        "Leonard", "Harrison", "Anthony", "Louis", "Milton", "Noah", "Cornelius", "Abdul", "Warren",
+        "Harvey", "Dennis", "Wesley", "Sylvester", "Gilbert", "Sullivan", "Edmund", "Wilson",
+        "Perry", "Matthew", "Simba", "Nala", "Rafiki", "Shenzi", "Ernest", "Gertrude", "Oscar",
+        "Lilian", "Raymond", "Elgar", "Elmer", "Herbert", "Maude", "Mae", "Otto", "Edwin",
+        "Ophelia", "Parsley", "Sage", "Rosemary", "Thyme", "Garfunkel", "King Basil", "Stephen"
+      ]
   }
 
   /** Gets the hair color of the person. If the person is bald, there is no result. */
@@ -936,25 +836,12 @@ class Person extends string {
 
   /** Holds if the person is deceased. */
   predicate isDeceased() {
-    this = "Ernest" or
-    this = "Gertrude" or
-    this = "Oscar" or
-    this = "Lilian" or
-    this = "Edwin" or
-    this = "Raymond" or
-    this = "Elgar" or
-    this = "Elmer" or
-    this = "Herbert" or
-    this = "Maude" or
-    this = "Mae" or
-    this = "Otto" or
-    this = "Ophelia" or
-    this = "Parsley" or
-    this = "Sage" or
-    this = "Rosemary" or
-    this = "Thyme" or
-    this = "Garfunkel" or
-    this = "King Basil"
+    this =
+      [
+        "Ernest", "Gertrude", "Oscar", "Lilian", "Edwin", "Raymond", "Elgar", "Elmer", "Herbert",
+        "Maude", "Mae", "Otto", "Ophelia", "Parsley", "Sage", "Rosemary", "Thyme", "Garfunkel",
+        "King Basil"
+      ]
   }
 
   /** Gets a parent of the person (alive or deceased). */
@@ -1195,12 +1082,7 @@ class Person extends string {
   }
 
   /** Holds if the person is allowed in the region. Initially, all villagers are allowed in every region. */
-  predicate isAllowedIn(string region) {
-    region = "north" or
-    region = "south" or
-    region = "east" or
-    region = "west"
-  }
+  predicate isAllowedIn(string region) { region = ["north", "south", "east", "west"] }
 }
 
 /** Returns a parent of the person. */

ruby/ql/test/TestUtilities/InlineExpectationsTest.qll

@@ -93,7 +93,7 @@
 private import InlineExpectationsTestPrivate
 
 /**
- * Base class for tests with inline expectations. The test extends this class to provide the actual
+ * The base class for tests with inline expectations. The test extends this class to provide the actual
  * results of the query, which are then compared with the expected results in comments to produce a
  * list of failure messages that point out where the actual results differ from the expected
  * results.