Ruby: Deprecate Pattern classes

ruby/ql/lib/codeql/ruby/ast/Control.qll

@@ -102,7 +102,7 @@ class IfExpr extends ConditionalExpr, TIfExpr {
     cond = false and result = this.getElse()
   }
 
-  override AstNode getAChild(string pred) {
+  final override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
     pred = "getThen" and result = this.getThen()
@@ -192,8 +192,8 @@ class UnlessExpr extends ConditionalExpr, TUnlessExpr {
 
   final override string toString() { result = "unless ..." }
 
-  override AstNode getAChild(string pred) {
-    result = ConditionalExpr.super.getAChild(pred)
+  final override AstNode getAChild(string pred) {
+    result = super.getAChild(pred)
     or
     pred = "getThen" and result = this.getThen()
     or
@@ -229,8 +229,8 @@ class IfModifierExpr extends ConditionalExpr, TIfModifierExpr {
 
   final override string toString() { result = "... if ..." }
 
-  override AstNode getAChild(string pred) {
-    result = ConditionalExpr.super.getAChild(pred)
+  final override AstNode getAChild(string pred) {
+    result = super.getAChild(pred)
     or
     pred = "getBody" and result = this.getBody()
   }
@@ -264,8 +264,8 @@ class UnlessModifierExpr extends ConditionalExpr, TUnlessModifierExpr {
 
   final override string toString() { result = "... unless ..." }
 
-  override AstNode getAChild(string pred) {
-    result = ConditionalExpr.super.getAChild(pred)
+  final override AstNode getAChild(string pred) {
+    result = super.getAChild(pred)
     or
     pred = "getBody" and result = this.getBody()
   }
@@ -300,8 +300,8 @@ class TernaryIfExpr extends ConditionalExpr, TTernaryIfExpr {
 
   final override string toString() { result = "... ? ... : ..." }
 
-  override AstNode getAChild(string pred) {
-    result = ConditionalExpr.super.getAChild(pred)
+  final override AstNode getAChild(string pred) {
+    result = super.getAChild(pred)
     or
     pred = "getThen" and result = this.getThen()
     or
@@ -390,7 +390,7 @@ class CaseExpr extends ControlExpr instanceof CaseExprImpl {
 
   final override string toString() { result = "case ..." }
 
-  override AstNode getAChild(string pred) {
+  final override AstNode getAChild(string pred) {
     result = ControlExpr.super.getAChild(pred)
     or
     pred = "getValue" and result = this.getValue()
@@ -444,7 +444,7 @@ class WhenExpr extends Expr, TWhenExpr {
 
   final override string toString() { result = "when ..." }
 
-  override AstNode getAChild(string pred) {
+  final override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
     pred = "getBody" and result = this.getBody()
@@ -517,7 +517,7 @@ class InClause extends Expr, TInClause {
 
   final override string toString() { result = "in ... then ..." }
 
-  override AstNode getAChild(string pred) {
+  final override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
     pred = "getBody" and result = this.getBody()
@@ -552,7 +552,7 @@ class ConditionalLoop extends Loop, TConditionalLoop {
   Expr getCondition() { none() }
 
   override AstNode getAChild(string pred) {
-    result = Loop.super.getAChild(pred)
+    result = super.getAChild(pred)
     or
     pred = "getCondition" and result = this.getCondition()
   }
@@ -692,7 +692,7 @@ class ForExpr extends Loop, TForExpr {
   final override StmtSequence getBody() { toGenerated(result) = g.getBody() }
 
   /** Gets the pattern representing the iteration argument. */
-  final Pattern getPattern() { toGenerated(result) = g.getPattern() }
+  final LhsExpr getPattern() { toGenerated(result) = g.getPattern() }
 
   /**
    * Gets the value being iterated over. In the following example, the result
@@ -707,8 +707,8 @@ class ForExpr extends Loop, TForExpr {
 
   final override string toString() { result = "for ... in ..." }
 
-  override AstNode getAChild(string pred) {
-    result = Loop.super.getAChild(pred)
+  final override AstNode getAChild(string pred) {
+    result = super.getAChild(pred)
     or
     pred = "getPattern" and result = this.getPattern()
     or

ruby/ql/lib/codeql/ruby/ast/Expr.qll

@@ -66,6 +66,87 @@ class ArgumentList extends Expr, TArgumentList {
   }
 }
 
+private class LhsExpr_ =
+  TVariableAccess or TTokenConstantAccess or TScopeResolutionConstantAccess or TMethodCall or
+      TDestructuredLhsExpr;
+
+/**
+ * A "left-hand-side" (LHS) expression. An `LhsExpr` can occur on the left-hand side of
+ * operator assignments (`AssignOperation`), on the left-hand side of assignments
+ * (`AssignExpr`), as patterns in for loops (`ForExpr`), and as exception variables
+ * in `rescue` clauses (`RescueClause`).
+ *
+ * An `LhsExpr` can be a simple variable, a constant, a call, or an element reference:
+ *
+ * ```rb
+ * var = 1
+ * var += 1
+ * E = 1
+ * foo.bar = 1
+ * foo[0] = 1
+ * rescue E => var
+ * ```
+ */
+class LhsExpr extends Expr, LhsExpr_ {
+  LhsExpr() { lhsExpr(this) }
+
+  /** Gets a variable used in (or introduced by) this LHS. */
+  Variable getAVariable() { result = this.(VariableAccess).getVariable() }
+}
+
+/**
+ * A "left-hand-side" (LHS) expression of a destructured assignment.
+ *
+ * Examples:
+ * ```rb
+ * a, self.b = value
+ * (a, b), c[3] = value
+ * a, b, *rest, c, d = value
+ * ```
+ */
+class DestructuredLhsExpr extends LhsExpr, TDestructuredLhsExpr {
+  override string getAPrimaryQlClass() { result = "DestructuredLhsExpr" }
+
+  private DestructuredLhsExprImpl getImpl() { result = toGenerated(this) }
+
+  private Ruby::AstNode getChild(int i) { result = this.getImpl().getChildNode(i) }
+
+  /** Gets the `i`th element in this destructured LHS. */
+  final Expr getElement(int i) {
+    exists(Ruby::AstNode c | c = this.getChild(i) |
+      toGenerated(result) = c.(Ruby::RestAssignment).getChild()
+      or
+      toGenerated(result) = c
+    )
+  }
+
+  /** Gets an element in this destructured LHS. */
+  final Expr getAnElement() { result = this.getElement(_) }
+
+  /**
+   * Gets the index of the element with the `*` marker on it, if it exists.
+   * In the example below the index is `2`.
+   * ```rb
+   * a, b, *rest, c, d = value
+   * ```
+   */
+  final int getRestIndex() { result = this.getImpl().getRestIndex() }
+
+  override Variable getAVariable() {
+    result = this.getElement(_).(VariableWriteAccess).getVariable()
+    or
+    result = this.getElement(_).(DestructuredLhsExpr).getAVariable()
+  }
+
+  override string toString() { result = "(..., ...)" }
+
+  final override AstNode getAChild(string pred) {
+    result = super.getAChild(pred)
+    or
+    pred = "getElement" and result = this.getElement(_)
+  }
+}
+
 /** A sequence of expressions. */
 class StmtSequence extends Expr, TStmtSequence {
   override string getAPrimaryQlClass() { result = "StmtSequence" }
@@ -136,7 +217,7 @@ class BodyStmt extends StmtSequence, TBodyStmt {
   final predicate hasEnsure() { exists(this.getEnsure()) }
 
   override AstNode getAChild(string pred) {
-    result = StmtSequence.super.getAChild(pred)
+    result = super.getAChild(pred)
     or
     pred = "getRescue" and result = this.getRescue(_)
     or
@@ -214,7 +295,7 @@ class Pair extends Expr, TPair {
 
   final override string toString() { result = "Pair" }
 
-  override AstNode getAChild(string pred) {
+  final override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
     pred = "getKey" and result = this.getKey()
@@ -283,7 +364,7 @@ class RescueClause extends Expr, TRescueClause {
 
   final override string toString() { result = "rescue ..." }
 
-  override AstNode getAChild(string pred) {
+  final override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
     pred = "getException" and result = this.getException(_)
@@ -325,7 +406,7 @@ class RescueModifierExpr extends Expr, TRescueModifierExpr {
 
   final override string toString() { result = "... rescue ..." }
 
-  override AstNode getAChild(string pred) {
+  final override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
     pred = "getBody" and result = this.getBody()
@@ -386,7 +467,7 @@ class StringConcatenation extends Expr, TStringConcatenation {
 
   final override string toString() { result = "\"...\" \"...\"" }
 
-  override AstNode getAChild(string pred) {
+  final override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
     pred = "getString" and result = this.getString(_)

ruby/ql/lib/codeql/ruby/ast/Operation.qll

@@ -443,14 +443,14 @@ class NoRegExpMatchExpr extends BinaryOperation, TNoRegExpMatchExpr {
  */
 class Assignment extends Operation instanceof AssignmentImpl {
   /** Gets the left hand side of this assignment. */
-  final Pattern getLeftOperand() { result = super.getLeftOperandImpl() }
+  final LhsExpr getLeftOperand() { result = super.getLeftOperandImpl() }
 
   /** Gets the right hand side of this assignment. */
   final Expr getRightOperand() { result = super.getRightOperandImpl() }
 
   final override string toString() { result = "... " + this.getOperator() + " ..." }
 
-  override AstNode getAChild(string pred) {
+  final override AstNode getAChild(string pred) {
     result = Operation.super.getAChild(pred)
     or
     pred = "getLeftOperand" and result = this.getLeftOperand()

ruby/ql/lib/codeql/ruby/ast/Parameter.qll

@@ -7,7 +7,14 @@ private import internal.TreeSitter
 /** A parameter. */
 class Parameter extends AstNode, TParameter {
   /** Gets the callable that this parameter belongs to. */
-  final Callable getCallable() { result.getAParameter() = this }
+  final Callable getCallable() {
+    result.getAParameter() = this
+    or
+    exists(DestructuredParameter parent |
+      this = parent.getAnElement() and
+      result = parent.getCallable()
+    )
+  }
 
   /** Gets the zero-based position of this parameter. */
   final int getPosition() { this = any(Callable c).getParameter(result) }
@@ -23,21 +30,63 @@ class Parameter extends AstNode, TParameter {
 }
 
 /**
+ * A parameter defined using destructuring. For example
+ *
+ * ```rb
+ * def tuples((a,b))
+ *   puts "#{a} #{b}"
+ * end
+ * ```
+ */
+class DestructuredParameter extends Parameter, TDestructuredParameter {
+  private DestructuredParameterImpl getImpl() { result = toGenerated(this) }
+
+  private Ruby::AstNode getChild(int i) { result = this.getImpl().getChildNode(i) }
+
+  /** Gets the `i`th element in this destructured parameter. */
+  final AstNode getElement(int i) {
+    exists(Ruby::AstNode c | c = this.getChild(i) | toGenerated(result) = c)
+  }
+
+  /** Gets an element in this destructured parameter. */
+  final AstNode getAnElement() { result = this.getElement(_) }
+
+  override LocalVariable getAVariable() {
+    result = this.getAnElement().(LocalVariableWriteAccess).getVariable()
+    or
+    result = this.getAnElement().(DestructuredParameter).getAVariable()
+  }
+
+  override string toString() { result = "(..., ...)" }
+
+  final override AstNode getAChild(string pred) {
+    result = super.getAChild(pred)
+    or
+    pred = "getElement" and result = this.getElement(_)
+  }
+
+  final override string getAPrimaryQlClass() { result = "DestructuredParameter" }
+}
+
+/**
+ * DEPRECATED
+ *
  * A parameter defined using a pattern.
  *
  * This includes both simple parameters and tuple parameters.
  */
-class PatternParameter extends Parameter, Pattern, TPatternParameter {
+deprecated class PatternParameter extends Parameter, Pattern, TPatternParameter {
   override LocalVariable getAVariable() { result = Pattern.super.getAVariable() }
 }
 
-/** A parameter defined using a tuple pattern. */
-class TuplePatternParameter extends PatternParameter, TuplePattern, TTuplePatternParameter {
+/**
+ * DEPRECATED
+ *
+ * A parameter defined using a tuple pattern.
+ */
+deprecated class TuplePatternParameter extends PatternParameter, TuplePattern,
+  TDestructuredParameter {
   final override LocalVariable getAVariable() { result = TuplePattern.super.getAVariable() }
-
-  final override string getAPrimaryQlClass() { result = "TuplePatternParameter" }
-
-  override AstNode getAChild(string pred) { result = TuplePattern.super.getAChild(pred) }
 }
 
 /** A named parameter. */
@@ -72,7 +121,7 @@ class NamedParameter extends Parameter, TNamedParameter {
 }
 
 /** A simple (normal) parameter. */
-class SimpleParameter extends NamedParameter, PatternParameter, VariablePattern, TSimpleParameter instanceof SimpleParameterImpl {
+class SimpleParameter extends NamedParameter, TSimpleParameter instanceof SimpleParameterImpl {
   final override string getName() { result = SimpleParameterImpl.super.getNameImpl() }
 
   final override LocalVariable getVariable() {