Skip to content

Commit 65b4a43

Browse files
Copilotaschackmull
authored andcommitted
Add ExceptionList AST node for rescue clauses with 2+ exceptions
1 parent 0834e64 commit 65b4a43

7 files changed

Lines changed: 145 additions & 53 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: breaking
3+
---
4+
* When a `rescue` clause has two or more exception types, the exceptions are no longer direct children of the `RescueClause` node. Instead, a new `ExceptionList` AST node wraps the exceptions. Use `RescueClause.getExceptions()` to get the `ExceptionList` node, and `ExceptionList.getException(int n)` to access the individual exceptions. For `rescue` clauses with zero or one exception, the behavior is unchanged and `RescueClause.getException(int n)` continues to work as before.

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

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,37 @@ class Pair extends Expr instanceof PairImpl {
280280
final override string getAPrimaryQlClass() { result = "Pair" }
281281
}
282282

283+
/**
284+
* A list of exception types in a rescue clause. For example, the exception list
285+
* `FirstError, SecondError` in:
286+
* ```rb
287+
* begin
288+
* do_something
289+
* rescue FirstError, SecondError => e
290+
* handle_error(e)
291+
* end
292+
* ```
293+
* This node is only present when there are two or more exceptions in the list.
294+
*/
295+
class ExceptionList extends Expr, TExceptionList {
296+
private Ruby::Exceptions g;
297+
298+
ExceptionList() { this = TExceptionList(g) }
299+
300+
final override string getAPrimaryQlClass() { result = "ExceptionList" }
301+
302+
/** Gets the `n`th exception in this list. */
303+
final Expr getException(int n) { toGenerated(result) = g.getChild(n) }
304+
305+
final override string toString() { result = "..., ..." }
306+
307+
final override AstNode getAChild(string pred) {
308+
result = super.getAChild(pred)
309+
or
310+
pred = "getException" and result = this.getException(_)
311+
}
312+
}
313+
283314
/**
284315
* A rescue clause. For example:
285316
* ```rb
@@ -305,8 +336,16 @@ class RescueClause extends Expr, TRescueClause {
305336
* handle_error(e)
306337
* end
307338
* ```
339+
* When there are two or more exceptions, use `getExceptions()` to get the `ExceptionList` node.
308340
*/
309-
final Expr getException(int n) { toGenerated(result) = g.getExceptions().getChild(n) }
341+
final Expr getException(int n) {
342+
// 0 or 1 exception: no ExceptionList node, access directly
343+
not exists(this.getExceptions()) and
344+
toGenerated(result) = g.getExceptions().getChild(n)
345+
or
346+
// 2+ exceptions: delegate through ExceptionList
347+
result = this.getExceptions().getException(n)
348+
}
310349

311350
/**
312351
* Gets an exception to match, if any. For example `FirstError` or `SecondError` in:
@@ -320,6 +359,19 @@ class RescueClause extends Expr, TRescueClause {
320359
*/
321360
final Expr getAnException() { result = this.getException(_) }
322361

362+
/**
363+
* Gets the exception list node when there are two or more exceptions to match. For example,
364+
* the exception list `FirstError, SecondError` in:
365+
* ```rb
366+
* begin
367+
* do_something
368+
* rescue FirstError, SecondError => e
369+
* handle_error(e)
370+
* end
371+
* ```
372+
*/
373+
final ExceptionList getExceptions() { result = TExceptionList(g.getExceptions()) }
374+
323375
/**
324376
* Gets the variable to which to assign the matched exception, if any.
325377
* For example `err` in:
@@ -343,8 +395,13 @@ class RescueClause extends Expr, TRescueClause {
343395
final override AstNode getAChild(string pred) {
344396
result = super.getAChild(pred)
345397
or
398+
// For 0 or 1 exceptions, exceptions are direct children
399+
not exists(this.getExceptions()) and
346400
pred = "getException" and result = this.getException(_)
347401
or
402+
// For 2+ exceptions, the ExceptionList node is the direct child
403+
pred = "getExceptions" and result = this.getExceptions()
404+
or
348405
pred = "getVariableExpr" and result = this.getVariableExpr()
349406
or
350407
pred = "getBody" and result = this.getBody()

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ private module Cached {
155155
TEndBlock(Ruby::EndBlock g) or
156156
TEnsure(Ruby::Ensure g) or
157157
TEqExpr(Ruby::Binary g) { g instanceof @ruby_binary_equalequal } or
158+
TExceptionList(Ruby::Exceptions g) { strictcount(g.getChild(_)) > 1 } or
158159
TExponentExprReal(Ruby::Binary g) { g instanceof @ruby_binary_starstar } or
159160
TExponentExprSynth(Ast::AstNode parent, int i) { mkSynthChild(ExponentExprKind(), parent, i) } or
160161
TFalseLiteral(Ruby::False g) or
@@ -375,7 +376,8 @@ private module Cached {
375376
TClassVariableAccessReal or TComplementExpr or TComplexLiteral or TDefinedExprReal or
376377
TDelimitedSymbolLiteral or TDestructuredLeftAssignment or TDestructuredParameter or
377378
TDivExprReal or TDo or TDoBlock or TElementReference or TElseReal or TElsif or TEmptyStmt or
378-
TEncoding or TEndBlock or TEnsure or TEqExpr or TExponentExprReal or TFalseLiteral or
379+
TEncoding or TEndBlock or TEnsure or TEqExpr or TExceptionList or TExponentExprReal or
380+
TFalseLiteral or
379381
TFile or TFindPattern or TFloatLiteral or TForExpr or TForwardParameter or
380382
TForwardArgument or TGEExpr or TGTExpr or TGlobalVariableAccessReal or
381383
THashKeySymbolLiteral or THashLiteral or THashPattern or THashSplatExprReal or
@@ -475,6 +477,7 @@ private module Cached {
475477
n = TEndBlock(result) or
476478
n = TEnsure(result) or
477479
n = TEqExpr(result) or
480+
n = TExceptionList(result) or
478481
n = TExponentExprReal(result) or
479482
n = TFalseLiteral(result) or
480483
n = TFile(result) or
@@ -765,7 +768,7 @@ class TExpr =
765768
TSelf or TArgumentList or TRescueClause or TRescueModifierExpr or TPair or TStringConcatenation or
766769
TCall or TBlockArgument or TConstantAccess or TControlExpr or TLiteral or TCallable or
767770
TVariableAccess or TStmtSequence or TOperation or TForwardArgument or TDestructuredLhsExpr or
768-
TMatchPattern or TTestPattern;
771+
TMatchPattern or TTestPattern or TExceptionList;
769772

770773
class TSplatExpr = TSplatExprReal or TSplatExprSynth;
771774

ruby/ql/test/library-tests/ast/Ast.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,16 @@ calls/calls.rb:
445445
# 255| getEnsure: [StmtSequence] ensure ...
446446
# 255| getStmt: [MethodCall] call to bar
447447
# 255| getReceiver: [ConstantReadAccess] X
448+
# 257| getStmt: [BeginExpr] begin ...
449+
# 258| getRescue: [RescueClause] rescue ...
450+
# 258| getExceptions: [ExceptionList] ..., ...
451+
# 258| getException: [MethodCall] call to foo
452+
# 258| getReceiver: [SelfVariableAccess] self
453+
# 258| getException: [MethodCall] call to bar
454+
# 258| getReceiver: [ConstantReadAccess] X
455+
# 259| getEnsure: [StmtSequence] ensure ...
456+
# 259| getStmt: [MethodCall] call to baz
457+
# 259| getReceiver: [SelfVariableAccess] self
448458
# 263| getStmt: [RescueModifierExpr] ... rescue ...
449459
# 263| getBody: [MethodCall] call to foo
450460
# 263| getReceiver: [SelfVariableAccess] self

0 commit comments

Comments
 (0)