Merge pull request #22014 from github/copilot/update-rescue-clause-exception-handling

Ruby AST: preserve ExceptionList node in RescueClause for 2+ exceptions

Author: aschackmull

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

@@ -280,6 +280,36 @@ class Pair extends Expr instanceof PairImpl {
   final override string getAPrimaryQlClass() { result = "Pair" }
 }
 
+/**
+ * A disjunctive exception pattern match in a rescue clause. For example, the exception list
+ * `FirstError, SecondError` in:
+ * ```rb
+ * begin
+ *   do_something
+ * rescue FirstError, SecondError => e
+ *   handle_error(e)
+ * end
+ * ```
+ */
+class ExceptionList extends Expr, TExceptionList {
+  private Ruby::Exceptions g;
+
+  ExceptionList() { this = TExceptionList(g) }
+
+  final override string getAPrimaryQlClass() { result = "ExceptionList" }
+
+  /** Gets the `n`th exception in this list. */
+  final Expr getException(int n) { toGenerated(result) = g.getChild(n) }
+
+  final override string toString() { result = "..., ..." }
+
+  final override AstNode getAChild(string pred) {
+    result = super.getAChild(pred)
+    or
+    pred = "getException" and result = this.getException(_)
+  }
+}
+
 /**
  * A rescue clause. For example:
  * ```rb
@@ -296,36 +326,69 @@ class RescueClause extends Expr, TRescueClause {
 
   final override string getAPrimaryQlClass() { result = "RescueClause" }
 
+  /** Gets the exception list pattern when there are multiple exception expressions. */
+  private ExceptionList getExceptions() { result = TExceptionList(g.getExceptions()) }
+
   /**
    * Gets the `n`th exception to match, if any. For example `FirstError` or `SecondError` in:
    * ```rb
    * begin
-   *  do_something
+   *   do_something
    * rescue FirstError, SecondError => e
    *   handle_error(e)
    * end
    * ```
    */
-  final Expr getException(int n) { toGenerated(result) = g.getExceptions().getChild(n) }
+  final Expr getException(int n) {
+    // 0 or 1 exception: no ExceptionList node, access directly
+    not exists(this.getExceptions()) and
+    toGenerated(result) = g.getExceptions().getChild(n)
+    or
+    // 2+ exceptions: delegate through ExceptionList
+    result = this.getExceptions().getException(n)
+  }
 
   /**
    * Gets an exception to match, if any. For example `FirstError` or `SecondError` in:
    * ```rb
    * begin
-   *  do_something
+   *   do_something
    * rescue FirstError, SecondError => e
    *   handle_error(e)
    * end
    * ```
    */
   final Expr getAnException() { result = this.getException(_) }
 
+  /**
+   * Gets the exception pattern to match, if any.
+   *
+   * This is either a single exception expression, or an `ExceptionList`
+   * representing a disjunctive match of multiple exceptions when there are two
+   * or more exceptions expressions.
+   *
+   * For example, in the following code, the exception pattern is the
+   * exception list `FirstError, SecondError`:
+   * ```rb
+   * begin
+   *   do_something
+   * rescue FirstError, SecondError => e
+   *   handle_error(e)
+   * end
+   * ```
+   */
+  final Expr getPattern() {
+    result = this.getExceptions()
+    or
+    not exists(this.getExceptions()) and result = this.getAnException()
+  }
+
   /**
    * Gets the variable to which to assign the matched exception, if any.
    * For example `err` in:
    * ```rb
    * begin
-   *  do_something
+   *   do_something
    * rescue StandardError => err
    *   handle_error(err)
    * end
@@ -343,7 +406,7 @@ class RescueClause extends Expr, TRescueClause {
   final override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
-    pred = "getException" and result = this.getException(_)
+    pred = "getPattern" and result = this.getPattern()
     or
     pred = "getVariableExpr" and result = this.getVariableExpr()
     or

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

@@ -155,6 +155,7 @@ private module Cached {
     TEndBlock(Ruby::EndBlock g) or
     TEnsure(Ruby::Ensure g) or
     TEqExpr(Ruby::Binary g) { g instanceof @ruby_binary_equalequal } or
+    TExceptionList(Ruby::Exceptions g) { strictcount(g.getChild(_)) > 1 } or
     TExponentExprReal(Ruby::Binary g) { g instanceof @ruby_binary_starstar } or
     TExponentExprSynth(Ast::AstNode parent, int i) { mkSynthChild(ExponentExprKind(), parent, i) } or
     TFalseLiteral(Ruby::False g) or
@@ -375,8 +376,8 @@ private module Cached {
         TClassVariableAccessReal or TComplementExpr or TComplexLiteral or TDefinedExprReal or
         TDelimitedSymbolLiteral or TDestructuredLeftAssignment or TDestructuredParameter or
         TDivExprReal or TDo or TDoBlock or TElementReference or TElseReal or TElsif or TEmptyStmt or
-        TEncoding or TEndBlock or TEnsure or TEqExpr or TExponentExprReal or TFalseLiteral or
-        TFile or TFindPattern or TFloatLiteral or TForExpr or TForwardParameter or
+        TEncoding or TEndBlock or TEnsure or TEqExpr or TExceptionList or TExponentExprReal or
+        TFalseLiteral or TFile or TFindPattern or TFloatLiteral or TForExpr or TForwardParameter or
         TForwardArgument or TGEExpr or TGTExpr or TGlobalVariableAccessReal or
         THashKeySymbolLiteral or THashLiteral or THashPattern or THashSplatExprReal or
         THashSplatNilParameter or THashSplatParameter or THereDoc or TIdentifierMethodCall or
@@ -475,6 +476,7 @@ private module Cached {
     n = TEndBlock(result) or
     n = TEnsure(result) or
     n = TEqExpr(result) or
+    n = TExceptionList(result) or
     n = TExponentExprReal(result) or
     n = TFalseLiteral(result) or
     n = TFile(result) or
@@ -765,7 +767,7 @@ class TExpr =
   TSelf or TArgumentList or TRescueClause or TRescueModifierExpr or TPair or TStringConcatenation or
       TCall or TBlockArgument or TConstantAccess or TControlExpr or TLiteral or TCallable or
       TVariableAccess or TStmtSequence or TOperation or TForwardArgument or TDestructuredLhsExpr or
-      TMatchPattern or TTestPattern;
+      TMatchPattern or TTestPattern or TExceptionList;
 
 class TSplatExpr = TSplatExprReal or TSplatExprSynth;