diff --git a/ql/src/codeql_ruby/ast/Statement.qll b/ql/src/codeql_ruby/ast/Statement.qll index 28ae2f933e3..8cc65fd7d8a 100644 --- a/ql/src/codeql_ruby/ast/Statement.qll +++ b/ql/src/codeql_ruby/ast/Statement.qll @@ -39,11 +39,7 @@ class Statement extends AstNode { class ReturningStatement extends Statement { override ReturningStatement::Range range; - final override string toString() { - not exists(getValue()) and result = range.getStatementName() - or - result = range.getStatementName() + " " + getValue().toString() - } + final override string toString() { result = range.toString() } /** Gets the returned value, if any. */ final Expr getValue() { diff --git a/ql/src/codeql_ruby/ast/internal/Statement.qll b/ql/src/codeql_ruby/ast/internal/Statement.qll index 72da586ebbb..d09b5027023 100644 --- a/ql/src/codeql_ruby/ast/internal/Statement.qll +++ b/ql/src/codeql_ruby/ast/internal/Statement.qll @@ -8,8 +8,6 @@ module Statement { module ReturningStatement { abstract class Range extends Statement::Range { abstract Generated::ArgumentList getArgumentList(); - - abstract string getStatementName(); } } @@ -17,7 +15,7 @@ module ReturnStmt { class Range extends ReturningStatement::Range, @return { final override Generated::Return generated; - final override string getStatementName() { result = "return" } + final override string toString() { result = "return" } final override Generated::ArgumentList getArgumentList() { result = generated.getChild() } } @@ -27,7 +25,7 @@ module BreakStmt { class Range extends ReturningStatement::Range, @break { final override Generated::Break generated; - final override string getStatementName() { result = "break" } + final override string toString() { result = "break" } final override Generated::ArgumentList getArgumentList() { result = generated.getChild() } } @@ -37,7 +35,7 @@ module NextStmt { class Range extends ReturningStatement::Range, @next { final override Generated::Next generated; - final override string getStatementName() { result = "next" } + final override string toString() { result = "next" } final override Generated::ArgumentList getArgumentList() { result = generated.getChild() } } diff --git a/ql/src/codeql_ruby/controlflow/CfgNodes.qll b/ql/src/codeql_ruby/controlflow/CfgNodes.qll index 0568291669b..c0296624b3a 100644 --- a/ql/src/codeql_ruby/controlflow/CfgNodes.qll +++ b/ql/src/codeql_ruby/controlflow/CfgNodes.qll @@ -249,7 +249,7 @@ module ExprNodes { override predicate relevantChild(Expr e) { e = this.getValue() or e = this.getBranch(_) } } - /** A control-flow node that wraps an `CaseExpr` AST expression. */ + /** A control-flow node that wraps a `CaseExpr` AST expression. */ class CaseExprCfgNode extends ExprCfgNode { override CaseExprChildMapping e; @@ -259,16 +259,16 @@ module ExprNodes { final ExprCfgNode getValue() { e.hasCfgChild(e.getValue(), this, result) } /** - * Gets the `n`th branch of this case expression, + * Gets the `n`th branch of this case expression. */ - final ExprCfgNode getBranch(int i) { e.hasCfgChild(e.getBranch(i), this, result) } + final ExprCfgNode getBranch(int n) { e.hasCfgChild(e.getBranch(n), this, result) } } private class ConditionalExprChildMapping extends ExprChildMapping, ConditionalExpr { override predicate relevantChild(Expr e) { e = this.getCondition() or e = this.getBranch(_) } } - /** A control-flow node that wraps an `ConditionalExpr` AST expression. */ + /** A control-flow node that wraps a `ConditionalExpr` AST expression. */ class ConditionalExprCfgNode extends ExprCfgNode { override ConditionalExprChildMapping e; @@ -305,7 +305,7 @@ module ExprNodes { override predicate relevantChild(Expr e) { e = this.getValue() } } - /** A control-flow node that wraps an `ForExpr` AST expression. */ + /** A control-flow node that wraps a `ForExpr` AST expression. */ class ForExprCfgNode extends ExprCfgNode { override ForExprChildMapping e; @@ -315,7 +315,7 @@ module ExprNodes { final ExprCfgNode getValue() { e.hasCfgChild(e.getValue(), this, result) } } - /** A control-flow node that wraps an `ParenthesizedExpr` AST expression. */ + /** A control-flow node that wraps a `ParenthesizedExpr` AST expression. */ class ParenthesizedExprCfgNode extends ExprSequenceCfgNode { ParenthesizedExprCfgNode() { this.getExpr() instanceof ParenthesizedExpr } } diff --git a/ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll b/ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll index 068951ab0eb..4ac583160ff 100644 --- a/ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll @@ -133,20 +133,23 @@ private module Cached { nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::CaseExprCfgNode).getBranch(_) or exists(CfgNodes::ExprCfgNode exprTo, ExprReturnNode n | - nodeFrom = n and exprTo = nodeTo.asExpr() and n.getKind() instanceof BreakReturnKind - | + nodeFrom = n and + exprTo = nodeTo.asExpr() and + n.getKind() instanceof BreakReturnKind and exprTo.getNode() instanceof Loop and nodeTo.asExpr().getAPredecessor(any(SuccessorTypes::BreakSuccessor s)) = n.getExprNode() ) or nodeFrom.asExpr() = nodeTo.(ExprReturnNode).getExprNode().getReturnedValueNode() or - exists(CfgNodes::ExprNodes::ForExprCfgNode for | for = nodeTo.asExpr() | - exists(SuccessorType s, CfgNode n | not s instanceof SuccessorTypes::BreakSuccessor | - for.getAPredecessor(s) = n - ) and - nodeFrom.asExpr() = for.getValue() - ) + nodeTo.asExpr() = + any(CfgNodes::ExprNodes::ForExprCfgNode for | + exists(SuccessorType s | + not s instanceof SuccessorTypes::BreakSuccessor and + exists(for.getAPredecessor(s)) + ) and + nodeFrom.asExpr() = for.getValue() + ) } cached diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index dde6226e833..d9bb072e5d0 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -199,7 +199,7 @@ break_ensure.rb: # 3| ... > ... #-----| false -> if ... -#-----| true -> Break +#-----| true -> break # 3| element #-----| -> 0 @@ -207,7 +207,7 @@ break_ensure.rb: # 3| 0 #-----| -> ... > ... -# 4| Break +# 4| break #-----| break -> for ... in ... # 7| Ensure @@ -262,7 +262,7 @@ break_ensure.rb: # 16| ... > ... #-----| false -> if ... -#-----| true -> Break +#-----| true -> break # 16| element #-----| -> 0 @@ -270,7 +270,7 @@ break_ensure.rb: # 16| 0 #-----| -> ... > ... -# 17| Break +# 17| break #-----| break -> [ensure: break] Ensure # 19| Ensure @@ -337,7 +337,7 @@ break_ensure.rb: # 29| call to nil? #-----| false -> if ... -#-----| true -> Return +#-----| true -> return # 29| elements #-----| -> nil? @@ -345,7 +345,7 @@ break_ensure.rb: # 29| nil? #-----| -> call to nil? -# 30| Return +# 30| return #-----| return -> [ensure: return] Ensure # 32| Ensure @@ -388,11 +388,11 @@ break_ensure.rb: # 35| ... > ... #-----| false -> if ... -#-----| true -> Break +#-----| true -> break # 35| [ensure: return] ... > ... #-----| false -> [ensure: return] if ... -#-----| true -> [ensure: return] Break +#-----| true -> [ensure: return] break # 35| call to x #-----| -> 0 @@ -406,10 +406,10 @@ break_ensure.rb: # 35| [ensure: return] 0 #-----| -> [ensure: return] ... > ... -# 36| Break +# 36| break #-----| break -> for ... in ... -# 36| [ensure: return] Break +# 36| [ensure: return] break #-----| break -> [ensure: return] for ... in ... # 41| call to puts @@ -497,17 +497,17 @@ break_ensure.rb: # 51| [ensure: raise] 0 #-----| -> [ensure: raise] ... > ... -# 52| Break +# 52| break #-----| break -> for ... in ... -# 52| [ensure: raise] Break +# 52| [ensure: raise] break #-----| break -> for ... in ... # 52| 10 -#-----| -> Break +#-----| -> break # 52| [ensure: raise] 10 -#-----| -> [ensure: raise] Break +#-----| -> [ensure: raise] break case.rb: # 1| if_in_case @@ -727,11 +727,11 @@ cfg.rb: # 31| true #-----| true -> 1 -# 32| Break +# 32| break #-----| break -> while ... # 32| 1 -#-----| -> Break +#-----| -> break # 35| if ... #-----| -> self @@ -1172,7 +1172,7 @@ cfg.rb: # 91| ... > ... #-----| false -> if ... -#-----| true -> Next +#-----| true -> next # 91| x #-----| -> 3 @@ -1180,7 +1180,7 @@ cfg.rb: # 91| 3 #-----| -> ... > ... -# 91| Next +# 91| next #-----| next -> In # 92| call to puts @@ -1295,11 +1295,11 @@ cfg.rb: # 102| value #-----| -> call to puts -# 103| Return +# 103| return #-----| return -> exit parameters (normal) # 103| ElementReference -#-----| -> Return +#-----| -> return # 103| kwargs #-----| -> key @@ -2288,17 +2288,17 @@ ifs.rb: #-----| false -> if ... #-----| true -> 0 -# 13| Return +# 13| return #-----| return -> exit m2 (normal) # 13| 0 -#-----| -> Return +#-----| -> return -# 15| Return +# 15| return #-----| return -> exit m2 (normal) # 15| 1 -#-----| -> Return +#-----| -> return # 18| m3 #-----| -> m4 @@ -2386,11 +2386,11 @@ ifs.rb: # 28| b3 #-----| -> b1 -# 29| Return +# 29| return #-----| return -> exit m4 (normal) # 29| ... ? ... : ... -#-----| -> Return +#-----| -> return # 29| [false] (... ? ... : ...) #-----| false -> !b2 || !b3 @@ -2618,7 +2618,7 @@ loops.rb: #-----| -> puts # 12| ... > ... -#-----| true -> Break +#-----| true -> break #-----| false -> x # 12| x @@ -2627,14 +2627,14 @@ loops.rb: # 12| 100 #-----| -> ... > ... -# 13| Break +# 13| break #-----| break -> while ... # 14| elsif ... #-----| -> if ... # 14| ... > ... -#-----| true -> Next +#-----| true -> next #-----| false -> x # 14| x @@ -2643,7 +2643,7 @@ loops.rb: # 14| 50 #-----| -> ... > ... -# 15| Next +# 15| next #-----| next -> x # 16| elsif ... @@ -3070,11 +3070,11 @@ raise.rb: # 71| 0 #-----| -> ... < ... -# 72| Return +# 72| return #-----| return -> [ensure: return] Ensure # 72| x < 0 -#-----| -> Return +#-----| -> return # 74| call to puts #-----| -> Ensure @@ -3174,11 +3174,11 @@ raise.rb: # 84| 0 #-----| -> ... < ... -# 85| Return +# 85| return #-----| return -> [ensure: return] Ensure # 85| x < 0 -#-----| -> Return +#-----| -> return # 87| call to puts #-----| -> Ensure @@ -3293,11 +3293,11 @@ raise.rb: # 99| 0 #-----| -> ... < ... -# 100| Return +# 100| return #-----| return -> [ensure: return] Ensure # 100| x < 0 -#-----| -> Return +#-----| -> return # 102| call to puts #-----| -> Ensure @@ -3700,17 +3700,17 @@ raise.rb: # 146| [ensure: raise] Ensure #-----| -> [ensure: raise] 3 -# 147| Return +# 147| return #-----| return -> exit m12 (normal) -# 147| [ensure: raise] Return +# 147| [ensure: raise] return #-----| return -> exit m12 (normal) # 147| 3 -#-----| -> Return +#-----| -> return # 147| [ensure: raise] 3 -#-----| -> [ensure: raise] Return +#-----| -> [ensure: raise] return # 150| m13 #-----| -> m14 diff --git a/ql/test/library-tests/dataflow/local/DataflowStep.expected b/ql/test/library-tests/dataflow/local/DataflowStep.expected index e639acbe7fe..d877f7a867b 100644 --- a/ql/test/library-tests/dataflow/local/DataflowStep.expected +++ b/ql/test/library-tests/dataflow/local/DataflowStep.expected @@ -24,10 +24,10 @@ | local_dataflow.rb:10:14:10:18 | array | local_dataflow.rb:15:10:15:14 | array | | local_dataflow.rb:15:10:15:14 | array | local_dataflow.rb:15:1:17:3 | for ... in ... | | local_dataflow.rb:15:10:15:14 | array | local_dataflow.rb:19:10:19:14 | array | -| local_dataflow.rb:16:3:16:10 | break 10 | local_dataflow.rb:15:1:17:3 | for ... in ... | -| local_dataflow.rb:16:9:16:10 | 10 | local_dataflow.rb:16:3:16:10 | break 10 | +| local_dataflow.rb:16:3:16:10 | break | local_dataflow.rb:15:1:17:3 | for ... in ... | +| local_dataflow.rb:16:9:16:10 | 10 | local_dataflow.rb:16:3:16:10 | break | | local_dataflow.rb:19:5:19:5 | x | local_dataflow.rb:20:6:20:6 | x | | local_dataflow.rb:19:10:19:14 | array | local_dataflow.rb:19:1:21:3 | for ... in ... | | local_dataflow.rb:20:17:20:21 | break | local_dataflow.rb:19:1:21:3 | for ... in ... | -| local_dataflow.rb:24:2:24:8 | break 5 | local_dataflow.rb:23:1:25:3 | while ... | -| local_dataflow.rb:24:8:24:8 | 5 | local_dataflow.rb:24:2:24:8 | break 5 | +| local_dataflow.rb:24:2:24:8 | break | local_dataflow.rb:23:1:25:3 | while ... | +| local_dataflow.rb:24:8:24:8 | 5 | local_dataflow.rb:24:2:24:8 | break |