From 8fb4dbe09280cd0aba67b1d4b313f3409ab6b318 Mon Sep 17 00:00:00 2001 From: yh-semmle Date: Sat, 7 Sep 2019 11:51:48 -0400 Subject: [PATCH] Java 13: account for changes to switch expressions --- java/ql/src/semmle/code/java/Completion.qll | 6 +- .../src/semmle/code/java/ControlFlowGraph.qll | 26 ++++---- java/ql/src/semmle/code/java/Expr.qll | 4 +- java/ql/src/semmle/code/java/Statement.qll | 59 +++++++++---------- 4 files changed, 46 insertions(+), 49 deletions(-) diff --git a/java/ql/src/semmle/code/java/Completion.qll b/java/ql/src/semmle/code/java/Completion.qll index a6195bcf600..c8b4f966a3b 100644 --- a/java/ql/src/semmle/code/java/Completion.qll +++ b/java/ql/src/semmle/code/java/Completion.qll @@ -52,13 +52,13 @@ newtype Completion = (innerValue = true or innerValue = false) } or /** - * The expression or statement completes via a `break` statement without a value. + * The expression or statement completes via a `break` statement. */ BreakCompletion(MaybeLabel l) or /** - * The expression or statement completes via a value `break` statement. + * The expression or statement completes via a `yield` statement. */ - ValueBreakCompletion(NormalOrBooleanCompletion c) or + YieldCompletion(NormalOrBooleanCompletion c) or /** * The expression or statement completes via a `continue` statement. */ diff --git a/java/ql/src/semmle/code/java/ControlFlowGraph.qll b/java/ql/src/semmle/code/java/ControlFlowGraph.qll index 45f96a36c9d..9c7cc976d46 100644 --- a/java/ql/src/semmle/code/java/ControlFlowGraph.qll +++ b/java/ql/src/semmle/code/java/ControlFlowGraph.qll @@ -804,23 +804,23 @@ private module ControlFlowGraphImpl { or // handle `switch` expression exists(SwitchExpr switch | switch = n | - // value `break` terminates the `switch` - last(switch.getAStmt(), last, ValueBreakCompletion(completion)) + // `yield` terminates the `switch` + last(switch.getAStmt(), last, YieldCompletion(completion)) or // any other abnormal completion is propagated last(switch.getAStmt(), last, completion) and - not completion instanceof ValueBreakCompletion and + not completion instanceof YieldCompletion and completion != NormalCompletion() ) or // the last node in a case rule is the last node in the right-hand side last(n.(SwitchCase).getRuleStatement(), last, completion) or - // ...and if the rhs is an expression we wrap the completion as a value break + // ...and if the rhs is an expression we wrap the completion as a yield exists(Completion caseCompletion | last(n.(SwitchCase).getRuleExpression(), last, caseCompletion) and if caseCompletion instanceof NormalOrBooleanCompletion - then completion = ValueBreakCompletion(caseCompletion) + then completion = YieldCompletion(caseCompletion) else completion = caseCompletion ) or @@ -833,18 +833,18 @@ private module ControlFlowGraphImpl { // `throw` statements or throwing calls give rise to ` Throw` completion exists(ThrowableType tt | mayThrow(n, tt) | last = n and completion = ThrowCompletion(tt)) or - // `break` statements without value give rise to a `Break` completion - exists(BreakStmt break | break = n and last = n and not break.hasValue() | + // `break` statements give rise to a `Break` completion + exists(BreakStmt break | break = n and last = n | completion = labelledBreakCompletion(MkLabel(break.getLabel())) or not exists(break.getLabel()) and completion = anonymousBreakCompletion() ) or - // value break statements get their completion wrapped as a value break + // yield statements get their completion wrapped as a yield exists(Completion caseCompletion | - last(n.(BreakStmt).getValue(), last, caseCompletion) and + last(n.(YieldStmt).getValue(), last, caseCompletion) and if caseCompletion instanceof NormalOrBooleanCompletion - then completion = ValueBreakCompletion(caseCompletion) + then completion = YieldCompletion(caseCompletion) else completion = caseCompletion ) or @@ -1112,9 +1112,9 @@ private module ControlFlowGraphImpl { n = case and result = first(case.getRuleStatement()) ) or - // Value break - exists(BreakStmt break | completion = NormalCompletion() | - n = break and result = first(break.getValue()) + // Yield + exists(YieldStmt yield | completion = NormalCompletion() | + n = yield and result = first(yield.getValue()) ) or // Synchronized statements execute their expression _before_ synchronization, so the CFG reflects that. diff --git a/java/ql/src/semmle/code/java/Expr.qll b/java/ql/src/semmle/code/java/Expr.qll index 0ab25628e47..313a8eb6aa8 100755 --- a/java/ql/src/semmle/code/java/Expr.qll +++ b/java/ql/src/semmle/code/java/Expr.qll @@ -1084,7 +1084,7 @@ class ConditionalExpr extends Expr, @conditionalexpr { } /** - * PREVIEW FEATURE in Java 12. Subject to removal in a future release. + * PREVIEW FEATURE in Java 13. Subject to removal in a future release. * * A `switch` expression. */ @@ -1117,7 +1117,7 @@ class SwitchExpr extends Expr, @switchexpr { Expr getAResult() { result = getACase().getRuleExpression() or - exists(BreakStmt break | break.(JumpStmt).getTarget() = this and result = break.getValue()) + exists(YieldStmt yield | yield.(JumpStmt).getTarget() = this and result = yield.getValue()) } /** Gets a printable representation of this expression. */ diff --git a/java/ql/src/semmle/code/java/Statement.qll b/java/ql/src/semmle/code/java/Statement.qll index 2f98615b243..6bcaa73a5da 100755 --- a/java/ql/src/semmle/code/java/Statement.qll +++ b/java/ql/src/semmle/code/java/Statement.qll @@ -417,7 +417,7 @@ class SwitchCase extends Stmt, @case { SwitchStmt getSwitch() { result.getACase() = this } /** - * PREVIEW FEATURE in Java 12. Subject to removal in a future release. + * PREVIEW FEATURE in Java 13. Subject to removal in a future release. * * Gets the switch expression to which this case belongs, if any. */ @@ -432,7 +432,7 @@ class SwitchCase extends Stmt, @case { } /** - * PREVIEW FEATURE in Java 12. Subject to removal in a future release. + * PREVIEW FEATURE in Java 13. Subject to removal in a future release. * * Holds if this `case` is a switch labeled rule of the form `... -> ...`. */ @@ -443,14 +443,14 @@ class SwitchCase extends Stmt, @case { } /** - * PREVIEW FEATURE in Java 12. Subject to removal in a future release. + * PREVIEW FEATURE in Java 13. Subject to removal in a future release. * * Gets the expression on the right-hand side of the arrow, if any. */ Expr getRuleExpression() { result.getParent() = this and result.getIndex() = -1 } /** - * PREVIEW FEATURE in Java 12. Subject to removal in a future release. + * PREVIEW FEATURE in Java 13. Subject to removal in a future release. * * Gets the statement on the right-hand side of the arrow, if any. */ @@ -465,7 +465,7 @@ class ConstCase extends SwitchCase { Expr getValue() { result.getParent() = this and result.getIndex() = 0 } /** - * PREVIEW FEATURE in Java 12. Subject to removal in a future release. + * PREVIEW FEATURE in Java 13. Subject to removal in a future release. * * Gets the `case` constant at the specified index. */ @@ -558,10 +558,11 @@ class ThrowStmt extends Stmt, @throwstmt { } } -/** A `break` or `continue` statement. */ +/** A `break`, `yield` or `continue` statement. */ class JumpStmt extends Stmt { JumpStmt() { this instanceof BreakStmt or + this instanceof YieldStmt or this instanceof ContinueStmt } @@ -581,13 +582,12 @@ class JumpStmt extends Stmt { ( result instanceof LoopStmt or - this instanceof BreakStmt and result instanceof SwitchStmt + (this instanceof BreakStmt or this instanceof YieldStmt) and + result instanceof SwitchStmt ) } - private SwitchExpr getSwitchExprTarget() { - this.(BreakStmt).hasValue() and result = this.getParent+() - } + private SwitchExpr getSwitchExprTarget() { result = this.(YieldStmt).getParent+() } private StmtParent getEnclosingTarget() { result = getSwitchExprTarget() @@ -598,7 +598,7 @@ class JumpStmt extends Stmt { } /** - * Gets the statement or `switch` expression that this `break` or `continue` jumps to. + * Gets the statement or `switch` expression that this `break`, `yield` or `continue` jumps to. */ StmtParent getTarget() { result = getLabelTarget() @@ -615,34 +615,31 @@ class BreakStmt extends Stmt, @breakstmt { /** Holds if this `break` statement has an explicit label. */ predicate hasLabel() { exists(string s | s = this.getLabel()) } - /** - * PREVIEW FEATURE in Java 12. Subject to removal in a future release. - * - * Gets the value of this `break` statement, if any. - */ - Expr getValue() { result.getParent() = this } - - /** - * PREVIEW FEATURE in Java 12. Subject to removal in a future release. - * - * Holds if this `break` statement has a value. - */ - predicate hasValue() { exists(Expr e | e.getParent() = this) } - /** Gets a printable representation of this statement. May include more detail than `toString()`. */ override string pp() { - if this.hasLabel() - then result = "break " + this.getLabel() - else - if this.hasValue() - then result = "break ..." - else result = "break" + if this.hasLabel() then result = "break " + this.getLabel() else result = "break" } /** This statement's Halstead ID (used to compute Halstead metrics). */ override string getHalsteadID() { result = "BreakStmt" } } +/** + * PREVIEW FEATURE in Java 13. Subject to removal in a future release. + * + * A `yield` statement. + */ +class YieldStmt extends Stmt, @yieldstmt { + /** + * Gets the value of this `yield` statement. + */ + Expr getValue() { result.getParent() = this } + + override string pp() { result = "yield ..." } + + override string getHalsteadID() { result = "YieldStmt" } +} + /** A `continue` statement. */ class ContinueStmt extends Stmt, @continuestmt { /** Gets the label targeted by this `continue` statement, if any. */