Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ruby/ql/lib/change-notes/2026-06-15-case-else-branch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: breaking
---
* The `else` branch of a `case` expression is no longer represented as a `StmtSequence` directly. Instead, a new `CaseElseBranch` AST node wraps the body (a `StmtSequence`). `CaseExpr.getElseBranch()` now returns a `CaseElseBranch`, and the body of the else branch can be accessed via `CaseElseBranch.getBody()`.
30 changes: 27 additions & 3 deletions ruby/ql/lib/codeql/ruby/ast/Control.qll
Original file line number Diff line number Diff line change
Expand Up @@ -377,18 +377,18 @@ class CaseExpr extends ControlExpr instanceof CaseExprImpl {

/**
* Gets the `n`th branch of this case expression, either a `WhenClause`, an
* `InClause`, or a `StmtSequence`.
* `InClause`, or a `CaseElseBranch`.
*/
final AstNode getBranch(int n) { result = super.getBranch(n) }

/**
* Gets a branch of this case expression, either a `WhenClause`, an
* `InClause`, or a `StmtSequence`.
* `InClause`, or a `CaseElseBranch`.
*/
final AstNode getABranch() { result = this.getBranch(_) }

/** Gets the `else` branch of this case expression, if any. */
final StmtSequence getElseBranch() { result = this.getABranch() }
final CaseElseBranch getElseBranch() { result = this.getABranch() }

/**
* Gets the number of branches of this case expression.
Expand Down Expand Up @@ -533,6 +533,30 @@ class InClause extends AstNode instanceof InClauseImpl {
}
}

/**
* An `else` branch of a `case` expression.
* ```rb
* case foo
* when 1 then puts "one"
* else puts "other"
* end
* ```
*/
class CaseElseBranch extends AstNode instanceof CaseElseBranchImpl {
final override string getAPrimaryQlClass() { result = "CaseElseBranch" }

/** Gets the body of this else branch. */
final Stmt getBody() { result = super.getBody() }

final override string toString() { result = "else ..." }

final override AstNode getAChild(string pred) {
result = AstNode.super.getAChild(pred)
or
pred = "getBody" and result = this.getBody()
}
}

/**
* A one-line pattern match using the `=>` operator. For example:
* ```rb
Expand Down
24 changes: 16 additions & 8 deletions ruby/ql/lib/codeql/ruby/ast/internal/AST.qll
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ private module Cached {
TBraceBlockSynth(Ast::AstNode parent, int i) { mkSynthChild(BraceBlockKind(), parent, i) } or
TBraceBlockReal(Ruby::Block g) { not g.getParent() instanceof Ruby::Lambda } or
TBreakStmt(Ruby::Break g) or
TCaseElseBranchSynth(Ast::AstNode parent, int i) {
mkSynthChild(CaseElseBranchKind(), parent, i)
} or
TCaseEqExpr(Ruby::Binary g) { g instanceof @ruby_binary_equalequalequal } or
TCaseExpr(Ruby::Case g) or
TCaseMatchReal(Ruby::CaseMatch g) or
Expand Down Expand Up @@ -400,14 +403,15 @@ private module Cached {
class TAstNodeSynth =
TAddExprSynth or TAssignExprSynth or TBitwiseAndExprSynth or TBitwiseOrExprSynth or
TBitwiseXorExprSynth or TBraceBlockSynth or TBodyStmtSynth or TBooleanLiteralSynth or
TCaseMatchSynth or TClassVariableAccessSynth or TConstantReadAccessSynth or
TConstantWriteAccessSynth or TDivExprSynth or TElseSynth or TExponentExprSynth or
TGlobalVariableAccessSynth or TIfSynth or TInClauseSynth or TInstanceVariableAccessSynth or
TIntegerLiteralSynth or TLShiftExprSynth or TLocalVariableAccessSynth or
TLogicalAndExprSynth or TLogicalOrExprSynth or TMethodCallSynth or TModuloExprSynth or
TMulExprSynth or TNilLiteralSynth or TRShiftExprSynth or TRangeLiteralSynth or TSelfSynth or
TSimpleParameterSynth or TSplatExprSynth or THashSplatExprSynth or TStmtSequenceSynth or
TSubExprSynth or TPairSynth or TSimpleSymbolLiteralSynth;
TCaseElseBranchSynth or TCaseMatchSynth or TClassVariableAccessSynth or
TConstantReadAccessSynth or TConstantWriteAccessSynth or TDivExprSynth or TElseSynth or
TExponentExprSynth or TGlobalVariableAccessSynth or TIfSynth or TInClauseSynth or
TInstanceVariableAccessSynth or TIntegerLiteralSynth or TLShiftExprSynth or
TLocalVariableAccessSynth or TLogicalAndExprSynth or TLogicalOrExprSynth or
TMethodCallSynth or TModuloExprSynth or TMulExprSynth or TNilLiteralSynth or
TRShiftExprSynth or TRangeLiteralSynth or TSelfSynth or TSimpleParameterSynth or
TSplatExprSynth or THashSplatExprSynth or TStmtSequenceSynth or TSubExprSynth or
TPairSynth or TSimpleSymbolLiteralSynth;

/**
* Gets the underlying TreeSitter entity for a given AST node. This does not
Expand Down Expand Up @@ -598,6 +602,8 @@ private module Cached {
or
result = TBraceBlockSynth(parent, i)
or
result = TCaseElseBranchSynth(parent, i)
or
result = TCaseMatchSynth(parent, i)
or
result = TClassVariableAccessSynth(parent, i, _)
Expand Down Expand Up @@ -718,6 +724,8 @@ TAstNodeReal fromGenerated(Ruby::AstNode n) { n = toGenerated(result) }

class TCall = TMethodCall or TYieldCall;

class TCaseElseBranch = TCaseElseBranchSynth;

class TCaseMatch = TCaseMatchReal or TCaseMatchSynth;

class TCase = TCaseExpr or TCaseMatch;
Expand Down
16 changes: 13 additions & 3 deletions ruby/ql/lib/codeql/ruby/ast/internal/Control.qll
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ class CaseWhenClause extends CaseExprImpl, TCaseExpr {
final override Expr getValue() { toGenerated(result) = g.getValue() }

final override AstNode getBranch(int n) {
toGenerated(result) = g.getChild(n) or
toGenerated(result) = g.getChild(n)
// When branches map directly to WhenClause nodes
toGenerated(result) = g.getChild(n) and not g.getChild(n) instanceof Ruby::Else
or
// The else branch is wrapped in a synthesized CaseElseBranch node
g.getChild(n) instanceof Ruby::Else and result = getSynthChild(this, n)
}
}

Expand All @@ -34,7 +37,8 @@ class CaseMatch extends CaseExprImpl, TCaseMatchReal {
final override AstNode getBranch(int n) {
toGenerated(result) = g.getClauses(n)
or
n = count(g.getClauses(_)) and toGenerated(result) = g.getElse()
// The else branch is wrapped in a synthesized CaseElseBranch node
n = count(g.getClauses(_)) and exists(g.getElse()) and result = getSynthChild(this, n)
}
}

Expand Down Expand Up @@ -87,3 +91,9 @@ class InClauseSynth extends InClauseImpl, TInClauseSynth {

final override predicate hasUnlessCondition() { none() }
}

class CaseElseBranchImpl extends AstNode, TCaseElseBranch {
CaseElseBranchImpl() { this = TCaseElseBranchSynth(_, _) }

final Stmt getBody() { synthChild(this, 0, result) }
}
69 changes: 67 additions & 2 deletions ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ newtype TSynthKind =
BodyStmtKind() or
BooleanLiteralKind(boolean value) { value = true or value = false } or
BraceBlockKind() or
CaseElseBranchKind() or
CaseMatchKind() or
ClassVariableAccessKind(ClassVariable v) or
DefinedExprKind() or
Expand Down Expand Up @@ -80,6 +81,8 @@ class SynthKind extends TSynthKind {
or
this = BraceBlockKind() and result = "BraceBlockKind"
or
this = CaseElseBranchKind() and result = "CaseElseBranchKind"
or
this = CaseMatchKind() and result = "CaseMatchKind"
or
this = ClassVariableAccessKind(_) and result = "ClassVariableAccessKind"
Expand Down Expand Up @@ -1840,7 +1843,7 @@ private module TestPatternDesugar {
or
child = SynthChild(InClauseKind()) and i = 1
or
child = SynthChild(ElseKind()) and i = 2
child = SynthChild(CaseElseBranchKind()) and i = 2
)
or
parent = TInClauseSynth(case, 1) and
Expand All @@ -1851,7 +1854,11 @@ private module TestPatternDesugar {
child = SynthChild(BooleanLiteralKind(true)) and i = 1
)
or
parent = TElseSynth(case, 2) and
parent = TCaseElseBranchSynth(case, 2) and
child = SynthChild(ElseKind()) and
i = 0
or
parent = TElseSynth(TCaseElseBranchSynth(case, 2), 0) and
child = SynthChild(BooleanLiteralKind(false)) and
i = 0
)
Expand Down Expand Up @@ -1994,3 +2001,61 @@ private module CallableBodySynthesis {
}
}
}

private module CaseElseBranchSynthesis {
pragma[nomagic]
private predicate caseElseBranchSynthesis(AstNode parent, int i, Child child) {
// Wrap the else branch of a real `case`/`when` expression
exists(Ruby::Case g, Ruby::Else elseNode, int elseIndex |
elseNode = g.getChild(elseIndex) and
(
// Create the CaseElseBranch wrapper node at the else index
parent = TCaseExpr(g) and
child = SynthChild(CaseElseBranchKind()) and
i = elseIndex
or
// The body of the CaseElseBranch is the Else node
parent = TCaseElseBranchSynth(TCaseExpr(g), elseIndex) and
child = RealChildRef(TElseReal(elseNode)) and
i = 0
)
)
or
// Wrap the else branch of a real `case`/`in` expression
exists(Ruby::CaseMatch g, Ruby::Else elseNode, int elseIndex |
elseNode = g.getElse() and
elseIndex = count(g.getClauses(_)) and
(
// Create the CaseElseBranch wrapper node at the else index
parent = TCaseMatchReal(g) and
child = SynthChild(CaseElseBranchKind()) and
i = elseIndex
or
// The body of the CaseElseBranch is the Else node
parent = TCaseElseBranchSynth(TCaseMatchReal(g), elseIndex) and
child = RealChildRef(TElseReal(elseNode)) and
i = 0
)
)
}

private class CaseElseBranchSynthesisImpl extends Synthesis {
final override predicate child(AstNode parent, int i, Child child) {
caseElseBranchSynthesis(parent, i, child)
}

final override predicate location(AstNode n, Location l) {
// Give the CaseElseBranch the location of the underlying Else node
exists(Ruby::Case g, int elseIndex |
n = TCaseElseBranchSynth(TCaseExpr(g), elseIndex) and
l = g.getChild(elseIndex).getLocation()
)
or
exists(Ruby::CaseMatch g, int elseIndex |
elseIndex = count(g.getClauses(_)) and
n = TCaseElseBranchSynth(TCaseMatchReal(g), elseIndex) and
l = g.getElse().getLocation()
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,16 @@
}
}

private class CaseElseBranchTree extends ControlFlowTree instanceof CaseElseBranch {

Check warning

Code scanning / CodeQL

Dead code Warning

This code is never used, and it's not publicly exported.
final override predicate propagatesAbnormal(AstNode child) { child = super.getBody() }

final override predicate first(AstNode first) { first(super.getBody(), first) }

final override predicate last(AstNode last, Completion c) { last(super.getBody(), last, c) }

final override predicate succ(AstNode pred, AstNode succ, Completion c) { none() }
}

private class PatternVariableAccessTree extends LocalVariableAccessTree instanceof LocalVariableWriteAccess,
CasePattern
{
Expand Down
15 changes: 9 additions & 6 deletions ruby/ql/test/library-tests/ast/Ast.expected
Original file line number Diff line number Diff line change
Expand Up @@ -815,8 +815,9 @@ control/cases.rb:
# 11| getPattern: [LocalVariableAccess] d
# 11| getBody: [StmtSequence] then ...
# 12| getStmt: [IntegerLiteral] 200
# 13| getBranch/getElseBranch: [StmtSequence] else ...
# 14| getStmt: [IntegerLiteral] 300
# 13| getBranch/getElseBranch: [CaseElseBranch] else ...
# 13| getBody: [StmtSequence] else ...
# 14| getStmt: [IntegerLiteral] 300
# 18| getStmt: [CaseExpr] case ...
# 19| getBranch: [WhenClause] when ...
# 19| getPattern: [GTExpr] ... > ...
Expand All @@ -843,8 +844,9 @@ control/cases.rb:
# 27| getPattern: [IntegerLiteral] 5
# 27| getBody: [StmtSequence] then ...
# 27| getStmt: [BooleanLiteral] true
# 28| getBranch/getElseBranch: [StmtSequence] else ...
# 28| getStmt: [BooleanLiteral] false
# 28| getBranch/getElseBranch: [CaseElseBranch] else ...
# 28| getBody: [StmtSequence] else ...
# 28| getStmt: [BooleanLiteral] false
# 31| getStmt: [CaseExpr] case ...
# 31| getValue: [MethodCall] call to expr
# 31| getReceiver: [SelfVariableAccess] self
Expand All @@ -862,8 +864,9 @@ control/cases.rb:
# 34| getAnOperand/getArgument/getGreaterOperand/getRightOperand: [IntegerLiteral] 0
# 34| getBody: [StmtSequence] then ...
# 35| getStmt: [BooleanLiteral] true
# 36| getBranch/getElseBranch: [StmtSequence] else ...
# 36| getStmt: [BooleanLiteral] false
# 36| getBranch/getElseBranch: [CaseElseBranch] else ...
# 36| getBody: [StmtSequence] else ...
# 36| getStmt: [BooleanLiteral] false
# 39| getStmt: [CaseExpr] case ...
# 39| getValue: [MethodCall] call to expr
# 39| getReceiver: [SelfVariableAccess] self
Expand Down
5 changes: 3 additions & 2 deletions ruby/ql/test/library-tests/ast/AstDesugar.expected
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,9 @@ control/cases.rb:
# 160| getPrefixElement: [IntegerLiteral] 1
# 160| getPrefixElement: [IntegerLiteral] 2
# 160| getBody: [BooleanLiteral] true
# 160| getBranch/getElseBranch: [StmtSequence] else ...
# 160| getStmt: [BooleanLiteral] false
# 160| getBranch/getElseBranch: [CaseElseBranch] else ...
# 160| getBody: [StmtSequence] else ...
# 160| getStmt: [BooleanLiteral] false
# 162| [MatchPattern] ... => ...
# 162| getDesugared: [CaseExpr] case ...
# 162| getValue: [MethodCall] call to expr
Expand Down
2 changes: 1 addition & 1 deletion ruby/ql/test/library-tests/ast/control/CaseExpr.ql
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ query predicate caseValues(CaseExpr c, Expr value) { value = c.getValue() }

query predicate caseNoValues(CaseExpr c) { not exists(c.getValue()) }

query predicate caseElseBranches(CaseExpr c, StmtSequence elseBranch) {
query predicate caseElseBranches(CaseExpr c, CaseElseBranch elseBranch) {
elseBranch = c.getElseBranch()
}

Expand Down
Loading