diff --git a/java/ql/consistency-queries/children.ql b/java/ql/consistency-queries/children.ql index 7386ee79c00..4771110ecdb 100644 --- a/java/ql/consistency-queries/children.ql +++ b/java/ql/consistency-queries/children.ql @@ -46,7 +46,9 @@ predicate gapInChildren(Element e, int i) { // value should be, because kotlinc doesn't load annotation defaults and we // want to leave a space for another extractor to fill in the default if it // is able. - not e instanceof Annotation + not e instanceof Annotation and + // Pattern case statements legitimately have a TypeAccess (-2) and a pattern (0) but not a rule (-1) + not (i = -1 and e instanceof PatternCase and not e.(PatternCase).isRule()) } predicate lateFirstChild(Element e, int i) { diff --git a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll index 572c8629626..e28b9f606b6 100644 --- a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll +++ b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll @@ -434,6 +434,15 @@ private module ControlFlowGraphImpl { ) } + /** + * Gets a SwitchCase's successor SwitchCase, if any. + */ + private predicate nextSwitchCase(SwitchCase pred, SwitchCase succ) { + exists(SwitchExpr se, int idx | se.getCase(idx) = pred and se.getCase(idx + 1) = succ) + or + exists(SwitchStmt ss, int idx | ss.getCase(idx) = pred and ss.getCase(idx + 1) = succ) + } + /** * Expressions and statements with CFG edges in post-order AST traversal. * @@ -467,7 +476,8 @@ private module ControlFlowGraphImpl { this instanceof NotInstanceOfExpr or this instanceof LocalVariableDeclExpr and - not this = any(InstanceOfExpr ioe).getLocalVariableDeclExpr() + not this = any(InstanceOfExpr ioe).getLocalVariableDeclExpr() and + not this = any(PatternCase pc).getDecl() or this instanceof StringTemplateExpr or @@ -493,7 +503,9 @@ private module ControlFlowGraphImpl { or this.(BlockStmt).getNumStmt() = 0 or - this instanceof SwitchCase and not this.(SwitchCase).isRule() + this instanceof SwitchCase and + not this.(SwitchCase).isRule() and + not this instanceof PatternCase or this instanceof EmptyStmt or @@ -887,6 +899,14 @@ private module ControlFlowGraphImpl { else completion = caseCompletion ) or + // The last node in a case could always be a failing pattern check. + last = n.(PatternCase) and + completion = basicBooleanCompletion(false) + or + // The last node in a non-rule case is its variable declaration. + last = n.(PatternCase).getDecl() and + completion = NormalCompletion() + or // the last statement of a synchronized statement is the last statement of its body last(n.(SynchronizedStmt).getBlock(), last, completion) or @@ -1201,8 +1221,14 @@ private module ControlFlowGraphImpl { // From the entry point control is transferred first to the expression... n = switch and result = first(switch.getExpr()) or - // ...and then to one of the cases. - last(switch.getExpr(), n, completion) and result = first(switch.getACase()) + // ...and then for a vanilla switch to any case, or for a pattern switch to the first one. + exists(SwitchCase firstExecutedCase | + if switch.getACase() instanceof PatternCase + then firstExecutedCase = switch.getCase(0) + else firstExecutedCase = switch.getACase() + | + last(switch.getExpr(), n, completion) and result = first(firstExecutedCase) + ) or // Statements within a switch body execute sequentially. exists(int i | @@ -1216,7 +1242,13 @@ private module ControlFlowGraphImpl { n = switch and result = first(switch.getExpr()) or // ...and then to one of the cases. - last(switch.getExpr(), n, completion) and result = first(switch.getACase()) + exists(SwitchCase firstExecutedCase | + if switch.getACase() instanceof PatternCase + then firstExecutedCase = switch.getCase(0) + else firstExecutedCase = switch.getACase() + | + last(switch.getExpr(), n, completion) and result = first(firstExecutedCase) + ) or // Statements within a switch body execute sequentially. exists(int i | @@ -1224,11 +1256,29 @@ private module ControlFlowGraphImpl { ) ) or + // Edge from rule SwitchCases to their body, after any variable assignment if applicable. // No edges in a non-rule SwitchCase - the constant expression in a ConstCase isn't included in the CFG. - exists(SwitchCase case | completion = NormalCompletion() | - n = case and result = first(case.getRuleExpression()) + exists(SwitchCase case, ControlFlowNode preBodyNode | + completion = NormalCompletion() and + if case instanceof PatternCase + then preBodyNode = case.(PatternCase).getDecl() + else preBodyNode = case + | + n = preBodyNode and result = first(case.getRuleExpression()) or - n = case and result = first(case.getRuleStatement()) + n = preBodyNode and result = first(case.getRuleStatement()) + ) + or + // A pattern case conducts a type test, then branches to either the next case or the assignment. + exists(PatternCase case | + n = case and + ( + completion = basicBooleanCompletion(false) and + nextSwitchCase(case, result) + or + completion = basicBooleanCompletion(true) and + result = case.getDecl() + ) ) or // Yield diff --git a/java/ql/lib/semmle/code/java/Expr.qll b/java/ql/lib/semmle/code/java/Expr.qll index 89e79b3ad0a..4d0936272f7 100644 --- a/java/ql/lib/semmle/code/java/Expr.qll +++ b/java/ql/lib/semmle/code/java/Expr.qll @@ -1509,17 +1509,28 @@ class SwitchExpr extends Expr, StmtParent, @switchexpr { */ Stmt getStmt(int index) { result = this.getAStmt() and result.getIndex() = index } + /** + * Gets the `i`th case of this `switch` expression, + * which may be either a normal `case` or a `default`. + */ + SwitchCase getCase(int i) { + result = rank[i](SwitchCase case, int idx | case.isNthChildOf(this, idx) | case order by idx) + } + /** * Gets a case of this `switch` expression, * which may be either a normal `case` or a `default`. */ - SwitchCase getACase() { result = this.getAConstCase() or result = this.getDefaultCase() } + SwitchCase getACase() { result.getParent() = this } /** Gets a (non-default) `case` of this `switch` expression. */ - ConstCase getAConstCase() { result.getParent() = this } + ConstCase getAConstCase() { result = this.getACase() } + + /** Gets a (non-default) pattern `case` of this `switch` expression. */ + PatternCase getAPatternCase() { result = this.getACase() } /** Gets the `default` case of this switch expression, if any. */ - DefaultCase getDefaultCase() { result.getParent() = this } + DefaultCase getDefaultCase() { result = this.getACase() } /** Gets the expression of this `switch` expression. */ Expr getExpr() { result.getParent() = this } @@ -1592,7 +1603,9 @@ class NotInstanceOfExpr extends Expr, @notinstanceofexpr { * A local variable declaration expression. * * Contexts in which such expressions may occur include - * local variable declaration statements and `for` loops. + * local variable declaration statements, `for` loops, + * and binding patterns such as `if (x instanceof T t)` and + * `case String s:`. */ class LocalVariableDeclExpr extends Expr, @localvariabledeclexpr { /** Gets an access to the variable declared by this local variable declaration expression. */ @@ -1612,18 +1625,33 @@ class LocalVariableDeclExpr extends Expr, @localvariabledeclexpr { exists(EnhancedForStmt efs | efs.getVariable() = this | result.isNthChildOf(efs, -1)) or exists(InstanceOfExpr ioe | this.getParent() = ioe | result.isNthChildOf(ioe, 1)) + or + exists(PatternCase pc | this.getParent() = pc | result.isNthChildOf(pc, -2)) } /** Gets the name of the variable declared by this local variable declaration expression. */ string getName() { result = this.getVariable().getName() } + /** Gets the switch statement or expression whose pattern declares this identifier, if any. */ + StmtParent getAssociatedSwitch() { + result = this.getParent().(PatternCase).getParent() + } + + /** Holds if this is a declaration stemming from a pattern switch case. */ + predicate hasAssociatedSwitch() { + exists(this.getAssociatedSwitch()) + } + /** Gets the initializer expression of this local variable declaration expression, if any. */ - Expr getInit() { result.isNthChildOf(this, 0) } + Expr getInit() { + result.isNthChildOf(this, 0) + } /** Holds if this variable declaration implicitly initializes the variable. */ predicate hasImplicitInit() { exists(CatchClause cc | cc.getVariable() = this) or - exists(EnhancedForStmt efs | efs.getVariable() = this) + exists(EnhancedForStmt efs | efs.getVariable() = this) or + this.hasAssociatedSwitch() } /** Gets a printable representation of this expression. */ diff --git a/java/ql/lib/semmle/code/java/Statement.qll b/java/ql/lib/semmle/code/java/Statement.qll index fe0ba23093a..40f3c4c6f68 100644 --- a/java/ql/lib/semmle/code/java/Statement.qll +++ b/java/ql/lib/semmle/code/java/Statement.qll @@ -382,17 +382,28 @@ class SwitchStmt extends Stmt, @switchstmt { */ Stmt getStmt(int index) { result = this.getAStmt() and result.getIndex() = index } + /** + * Gets the `i`th case of this `switch` statement, + * which may be either a normal `case` or a `default`. + */ + SwitchCase getCase(int i) { + result = rank[i](SwitchCase case, int idx | case.isNthChildOf(this, idx) | case order by idx) + } + /** * Gets a case of this `switch` statement, * which may be either a normal `case` or a `default`. */ - SwitchCase getACase() { result = this.getAConstCase() or result = this.getDefaultCase() } + SwitchCase getACase() { result.getParent() = this } - /** Gets a (non-default) `case` of this `switch` statement. */ - ConstCase getAConstCase() { result.getParent() = this } + /** Gets a (non-default) constant `case` of this `switch` statement. */ + ConstCase getAConstCase() { result = this.getACase() } + + /** Gets a (non-default) pattern `case` of this `switch` statement. */ + PatternCase getAPatternCase() { result = this.getACase() } /** Gets the `default` case of this switch statement, if any. */ - DefaultCase getDefaultCase() { result.getParent() = this } + DefaultCase getDefaultCase() { result = this.getACase() } /** Gets the expression of this `switch` statement. */ Expr getExpr() { result.getParent() = this } @@ -464,15 +475,15 @@ class SwitchCase extends Stmt, @case { /** A constant `case` of a switch statement. */ class ConstCase extends SwitchCase { - ConstCase() { exists(Expr e | e.getParent() = this | e.getIndex() >= 0) } + ConstCase() { exists(Literal e | e.getParent() = this and e.getIndex() >= 0) } /** Gets the `case` constant at index 0. */ - Expr getValue() { result.getParent() = this and result.getIndex() = 0 } + Expr getValue() { result.isNthChildOf(this, 0) } /** * Gets the `case` constant at the specified index. */ - Expr getValue(int i) { result.getParent() = this and result.getIndex() = i and i >= 0 } + Expr getValue(int i) { result.isNthChildOf(this, i) and i >= 0 } override string pp() { result = "case ..." } @@ -483,6 +494,24 @@ class ConstCase extends SwitchCase { override string getAPrimaryQlClass() { result = "ConstCase" } } +/** A pattern case of a `switch` statement */ +class PatternCase extends SwitchCase { + LocalVariableDeclExpr patternVar; + + PatternCase() { patternVar.isNthChildOf(this, 0) } + + /** Gets the variable declared by this pattern case. */ + LocalVariableDeclExpr getDecl() { result.isNthChildOf(this, 0) } + + override string pp() { result = "case T t ..." } + + override string toString() { result = "case T t ..." } + + override string getHalsteadID() { result = "PatternCase" } + + override string getAPrimaryQlClass() { result = "PatternCase" } +} + /** A `default` case of a `switch` statement */ class DefaultCase extends SwitchCase { DefaultCase() { not exists(Expr e | e.getParent() = this | e.getIndex() >= 0) } diff --git a/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll b/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll index 293ba894fdf..c9ae5353127 100644 --- a/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll +++ b/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll @@ -80,6 +80,15 @@ private predicate step(Node n1, Node n2) { for.getVariable() = def.(BaseSsaUpdate).getDefiningExpr() and for.getExpr() = n1.asExpr() ) + or + exists(PatternCase pc | + pc.getDecl() = def.(BaseSsaUpdate).getDefiningExpr() and + ( + pc.getSwitch().getExpr() = n1.asExpr() + or + pc.getSwitchExpr().getExpr() = n1.asExpr() + ) + ) | v.getAnUltimateDefinition() = def and v.getAUse() = n2.asExpr() diff --git a/java/ql/test/library-tests/printAst/A.java b/java/ql/test/library-tests/printAst/A.java index 4cf4e8699da..cd47860f2ce 100644 --- a/java/ql/test/library-tests/printAst/A.java +++ b/java/ql/test/library-tests/printAst/A.java @@ -50,6 +50,34 @@ class A { if (thing instanceof String s) { throw new RuntimeException(s); } + switch (thing) { + case String s -> System.out.println(s); + case Integer i -> System.out.println("An integer: " + i); + default -> { } + } + switch (thing) { + case String s: + System.out.println(s); + break; + case Integer i: + System.out.println("An integer:" + i); + break; + default: + break; + } + var thingAsString = switch(thing) { + case String s -> s; + case Integer i -> "An integer: " + i; + default -> "Something else"; + }; + var thingAsString2 = switch(thing) { + case String s: + yield s; + case Integer i: + yield "An integer: " + i; + default: + yield "Something else"; + }; } } catch (RuntimeException rte) { @@ -70,4 +98,4 @@ class A { * Javadoc for fields */ int i, j, k; -} \ No newline at end of file +} diff --git a/java/ql/test/library-tests/printAst/PrintAst.expected b/java/ql/test/library-tests/printAst/PrintAst.expected index 219889b35f6..28dfa0ed5c1 100644 --- a/java/ql/test/library-tests/printAst/PrintAst.expected +++ b/java/ql/test/library-tests/printAst/PrintAst.expected @@ -119,36 +119,117 @@ A.java: # 51| 0: [ClassInstanceExpr] new RuntimeException(...) # 51| -3: [TypeAccess] RuntimeException # 51| 0: [VarAccess] s -# 55| 0: [CatchClause] catch (...) +# 53| 2: [SwitchStmt] switch (...) +# 53| -1: [VarAccess] thing +# 54| 0: [PatternCase] case T t ... +# 54| -2: [TypeAccess] String +# 54| -1: [ExprStmt] ; +# 54| 0: [MethodAccess] println(...) +# 54| -1: [VarAccess] System.out +# 54| -1: [TypeAccess] System +# 54| 0: [VarAccess] s +# 54| 0: [LocalVariableDeclExpr] s +# 55| 1: [PatternCase] case T t ... +# 55| -2: [TypeAccess] Integer +# 55| -1: [ExprStmt] ; +# 55| 0: [MethodAccess] println(...) +# 55| -1: [VarAccess] System.out +# 55| -1: [TypeAccess] System +# 55| 0: [AddExpr] ... + ... +# 55| 0: [StringLiteral] "An integer: " +# 55| 1: [VarAccess] i +# 55| 0: [LocalVariableDeclExpr] i +# 56| 2: [DefaultCase] default +# 56| -1: [BlockStmt] { ... } +# 58| 3: [SwitchStmt] switch (...) +# 58| -1: [VarAccess] thing +# 59| 0: [PatternCase] case T t ... +# 59| -2: [TypeAccess] String +# 59| 0: [LocalVariableDeclExpr] s +# 60| 1: [ExprStmt] ; +# 60| 0: [MethodAccess] println(...) +# 60| -1: [VarAccess] System.out +# 60| -1: [TypeAccess] System +# 60| 0: [VarAccess] s +# 61| 2: [BreakStmt] break +# 62| 3: [PatternCase] case T t ... +# 62| -2: [TypeAccess] Integer +# 62| 0: [LocalVariableDeclExpr] i +# 63| 4: [ExprStmt] ; +# 63| 0: [MethodAccess] println(...) +# 63| -1: [VarAccess] System.out +# 63| -1: [TypeAccess] System +# 63| 0: [AddExpr] ... + ... +# 63| 0: [StringLiteral] "An integer:" +# 63| 1: [VarAccess] i +# 64| 5: [BreakStmt] break +# 65| 6: [DefaultCase] default +# 66| 7: [BreakStmt] break +# 68| 4: [LocalVariableDeclStmt] var ...; +# 68| 1: [LocalVariableDeclExpr] thingAsString +# 68| 0: [SwitchExpr] switch (...) +# 68| -1: [VarAccess] thing +# 69| 0: [PatternCase] case T t ... +# 69| -2: [TypeAccess] String +# 69| -1: [VarAccess] s +# 69| 0: [LocalVariableDeclExpr] s +# 70| 1: [PatternCase] case T t ... +# 70| -2: [TypeAccess] Integer +# 70| -1: [AddExpr] ... + ... +# 70| 0: [StringLiteral] "An integer: " +# 70| 1: [VarAccess] i +# 70| 0: [LocalVariableDeclExpr] i +# 71| 2: [DefaultCase] default +# 71| -1: [StringLiteral] "Something else" +# 73| 5: [LocalVariableDeclStmt] var ...; +# 73| 1: [LocalVariableDeclExpr] thingAsString2 +# 73| 0: [SwitchExpr] switch (...) +# 73| -1: [VarAccess] thing +# 74| 0: [PatternCase] case T t ... +# 74| -2: [TypeAccess] String +# 74| 0: [LocalVariableDeclExpr] s +# 75| 1: [YieldStmt] yield ... +# 75| 0: [VarAccess] s +# 76| 2: [PatternCase] case T t ... +# 76| -2: [TypeAccess] Integer +# 76| 0: [LocalVariableDeclExpr] i +# 77| 3: [YieldStmt] yield ... +# 77| 0: [AddExpr] ... + ... +# 77| 0: [StringLiteral] "An integer: " +# 77| 1: [VarAccess] i +# 78| 4: [DefaultCase] default +# 79| 5: [YieldStmt] yield ... +# 79| 0: [StringLiteral] "Something else" +# 83| 0: [CatchClause] catch (...) #-----| 0: (Single Local Variable Declaration) -# 55| 0: [TypeAccess] RuntimeException -# 55| 1: [LocalVariableDeclExpr] rte -# 55| 1: [BlockStmt] { ... } -# 56| 0: [ReturnStmt] return ... -# 60| 10: [Class] E -# 64| 3: [FieldDeclaration] E A; +# 83| 0: [TypeAccess] RuntimeException +# 83| 1: [LocalVariableDeclExpr] rte +# 83| 1: [BlockStmt] { ... } +# 84| 0: [ReturnStmt] return ... +# 88| 10: [Class] E +# 92| 3: [FieldDeclaration] E A; #-----| -3: (Javadoc) -# 61| 1: [Javadoc] /** Javadoc for enum constant */ -# 62| 0: [JavadocText] Javadoc for enum constant -# 64| -1: [TypeAccess] E -# 64| 0: [ClassInstanceExpr] new E(...) -# 64| -3: [TypeAccess] E -# 65| 4: [FieldDeclaration] E B; +# 89| 1: [Javadoc] /** Javadoc for enum constant */ +# 90| 0: [JavadocText] Javadoc for enum constant +# 92| -1: [TypeAccess] E +# 92| 0: [ClassInstanceExpr] new E(...) +# 92| -3: [TypeAccess] E +# 93| 4: [FieldDeclaration] E B; #-----| -3: (Javadoc) -# 61| 1: [Javadoc] /** Javadoc for enum constant */ -# 62| 0: [JavadocText] Javadoc for enum constant -# 65| -1: [TypeAccess] E -# 65| 0: [ClassInstanceExpr] new E(...) -# 65| -3: [TypeAccess] E -# 66| 5: [FieldDeclaration] E C; +# 89| 1: [Javadoc] /** Javadoc for enum constant */ +# 90| 0: [JavadocText] Javadoc for enum constant +# 93| -1: [TypeAccess] E +# 93| 0: [ClassInstanceExpr] new E(...) +# 93| -3: [TypeAccess] E +# 94| 5: [FieldDeclaration] E C; #-----| -3: (Javadoc) -# 61| 1: [Javadoc] /** Javadoc for enum constant */ -# 62| 0: [JavadocText] Javadoc for enum constant -# 66| -1: [TypeAccess] E -# 66| 0: [ClassInstanceExpr] new E(...) -# 66| -3: [TypeAccess] E -# 72| 11: [FieldDeclaration] int i, ...; +# 89| 1: [Javadoc] /** Javadoc for enum constant */ +# 90| 0: [JavadocText] Javadoc for enum constant +# 94| -1: [TypeAccess] E +# 94| 0: [ClassInstanceExpr] new E(...) +# 94| -3: [TypeAccess] E +# 100| 11: [FieldDeclaration] int i, ...; #-----| -3: (Javadoc) -# 69| 1: [Javadoc] /** Javadoc for fields */ -# 70| 0: [JavadocText] Javadoc for fields -# 72| -1: [TypeAccess] int +# 97| 1: [Javadoc] /** Javadoc for fields */ +# 98| 0: [JavadocText] Javadoc for fields +# 100| -1: [TypeAccess] int diff --git a/java/ql/test/library-tests/printAst/options b/java/ql/test/library-tests/printAst/options index a2f4d45311b..a0d1b7e7002 100644 --- a/java/ql/test/library-tests/printAst/options +++ b/java/ql/test/library-tests/printAst/options @@ -1 +1 @@ -//semmle-extractor-options: --javac-args -source 17 -target 17 +//semmle-extractor-options: --javac-args --release 21