diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java index 74fba50045d8..6a5b103f6dda 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java @@ -707,32 +707,6 @@ public class CFGBuilder { } } - /** - * Return true if given expression contains return statement (which could be inside switch expression) - * @param expression expression to analyze - * @return true if it has nested return - */ - private static boolean hasNestedReturn(@NotNull PsiExpression expression) { - class Visitor extends JavaRecursiveElementWalkingVisitor { - boolean hasReturn; - - @Override - public void visitLambdaExpression(PsiLambdaExpression expression) {} - - @Override - public void visitClass(PsiClass aClass) {} - - @Override - public void visitReturnStatement(PsiReturnStatement statement) { - hasReturn = true; - stopWalking(); - } - } - Visitor visitor = new Visitor(); - expression.accept(visitor); - return visitor.hasReturn; - } - /** * Inlines given lambda. Lambda parameters are assumed to be assigned already (if necessary). *

@@ -748,20 +722,14 @@ public class CFGBuilder { PsiElement body = lambda.getBody(); PsiExpression expression = LambdaUtil.extractSingleExpressionFromBody(body); if (expression != null) { - if (hasNestedReturn(expression)) { - DfaVariableValue variable = createTempVariable(LambdaUtil.getFunctionalInterfaceReturnType(lambda)); - myAnalyzer.inlineExpression(lambda, expression, resultNullability, variable); - push(variable); - } else { - pushExpression(expression); - boxUnbox(expression, LambdaUtil.getFunctionalInterfaceReturnType(lambda)); - if (resultNullability == Nullability.NOT_NULL) { - checkNotNull(expression, NullabilityProblemKind.nullableFunctionReturn); - } + pushExpression(expression); + boxUnbox(expression, LambdaUtil.getFunctionalInterfaceReturnType(lambda)); + if (resultNullability == Nullability.NOT_NULL) { + checkNotNull(expression, NullabilityProblemKind.nullableFunctionReturn); } } else if(body instanceof PsiCodeBlock) { DfaVariableValue variable = createTempVariable(LambdaUtil.getFunctionalInterfaceReturnType(lambda)); - myAnalyzer.inlineBlock(lambda, (PsiCodeBlock)body, resultNullability, variable); + myAnalyzer.inlineBlock((PsiCodeBlock)body, resultNullability, variable); push(variable); } else { pushUnknown(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index c66dddc8c453..b97b6fd73f9e 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -801,24 +801,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { PsiExpression returnValue = statement.getReturnValue(); InlinedBlockContext context = myInlinedBlockContext; - while (context != null && context.isSwitch()) { - context = context.myPreviousBlock; - } if (context != null) { - if (returnValue != null) { - DfaVariableValue var = context.myTarget; - addInstruction(new PushInstruction(var, null, true)); - returnValue.accept(this); - generateBoxingUnboxingInstructionFor(returnValue, var.getType()); - if (context.myForceNonNullBlockResult) { - addInstruction(new CheckNotNullInstruction(NullabilityProblemKind.nullableFunctionReturn.problem(returnValue))); - } - addInstruction(new AssignInstruction(returnValue, null)); - addInstruction(new PopInstruction()); - } - - controlTransfer(new InstructionTransfer(getEndOffset(context.myCodeBlock), getVariablesInside(context.myCodeBlock)), - getTrapsInsideElement(context.myCodeBlock)); + // We treat return inside switch expression (which is disallowed syntax) as break-with-value + context.generateReturn(returnValue, this); } else { if (returnValue != null) { @@ -850,52 +835,48 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { @Override public void visitSwitchLabeledRuleStatement(PsiSwitchLabeledRuleStatement statement) { + PsiSwitchBlock switchBlock = statement.getEnclosingSwitchBlock(); + if (switchBlock == null) return; startElement(statement); PsiStatement body = statement.getBody(); + PsiCodeBlock switchBody = switchBlock.getBody(); + boolean expressionSwitch = myInlinedBlockContext != null && myInlinedBlockContext.myCodeBlock == switchBody; + if (expressionSwitch && body instanceof PsiExpressionStatement) { + myInlinedBlockContext.generateReturn(((PsiExpressionStatement)body).getExpression(), this); + } if (body != null) { - if (body instanceof PsiExpressionStatement && myInlinedBlockContext != null && - myInlinedBlockContext.myCodeBlock == statement.getEnclosingSwitchBlock()) { - addInstruction(new PushInstruction(myInlinedBlockContext.myTarget, null, true)); - PsiExpression expression = ((PsiExpressionStatement)body).getExpression(); - expression.accept(this); - generateBoxingUnboxingInstructionFor(expression, myInlinedBlockContext.myTarget.getType()); - addInstruction(new AssignInstruction(null, myInlinedBlockContext.myTarget)); - addInstruction(new PopInstruction()); - } else { - body.accept(this); - } - if (!(body instanceof PsiThrowStatement)) { - jumpOut(statement.getEnclosingSwitchBlock()); - } + body.accept(this); + } + if (!(body instanceof PsiThrowStatement)) { + jumpOut(expressionSwitch ? switchBody : switchBlock); } finishElement(statement); } @Override public void visitSwitchStatement(PsiSwitchStatement switchStmt) { - processSwitch(switchStmt, null); + startElement(switchStmt); + processSwitch(switchStmt); + finishElement(switchStmt); } @Override public void visitSwitchExpression(PsiSwitchExpression expression) { PsiCodeBlock body = expression.getBody(); if (body == null) { - PsiExpression selector = expression.getExpression(); - if (selector != null) { - selector.accept(this); - } - addInstruction(new PopInstruction()); + processSwitch(expression); pushUnknown(); } else { + startElement(expression); DfaVariableValue resultVariable = createTempVariable(expression.getType()); - enterInlinedBlock(expression, Nullability.UNKNOWN, resultVariable); - processSwitch(expression, resultVariable); + enterInlinedBlock(body, Nullability.UNKNOWN, resultVariable); + processSwitch(expression); exitInlinedBlock(); addInstruction(new PushInstruction(resultVariable, expression)); + finishElement(expression); } } - private void processSwitch(@NotNull PsiSwitchBlock switchBlock, @Nullable DfaVariableValue resultVariable) { - startElement(switchBlock); + private void processSwitch(@NotNull PsiSwitchBlock switchBlock) { PsiExpression caseExpression = switchBlock.getExpression(); Set enumValues = null; DfaVariableValue expressionValue = null; @@ -990,7 +971,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (expressionValue != null) { addInstruction(new FlushVariableInstruction(expressionValue)); } - finishElement(switchBlock); } @Override @@ -2058,35 +2038,24 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { * @param resultNullability desired nullability returned by block return statement * @param target a variable to store the block result (returned via {@code return} statement) */ - void inlineBlock(@NotNull PsiElement anchor, @NotNull PsiCodeBlock block, @NotNull Nullability resultNullability, @NotNull DfaVariableValue target) { - enterInlinedBlock(anchor, resultNullability, target); - startElement(anchor); + void inlineBlock(@NotNull PsiCodeBlock block, @NotNull Nullability resultNullability, @NotNull DfaVariableValue target) { + enterInlinedBlock(block, resultNullability, target); block.accept(this); - finishElement(anchor); exitInlinedBlock(); } - void inlineExpression(@NotNull PsiElement anchor, @NotNull PsiExpression expr, @NotNull Nullability resultNullability, @NotNull DfaVariableValue target) { - enterInlinedBlock(anchor, resultNullability, target); - startElement(anchor); - addInstruction(new PushInstruction(target, null, true)); - expr.accept(this); - addInstruction(new AssignInstruction(null, target)); - addInstruction(new PopInstruction()); - finishElement(anchor); - exitInlinedBlock(); - } - - private void enterInlinedBlock(@NotNull PsiElement block, + private void enterInlinedBlock(@NotNull PsiCodeBlock block, @NotNull Nullability resultNullability, @NotNull DfaVariableValue target) { // Transfer value is pushed to avoid emptying stack beyond this point pushTrap(new Trap.InsideInlinedBlock(block)); addInstruction(new PushInstruction(myFactory.controlTransfer(ReturnTransfer.INSTANCE, FList.emptyList()), null)); myInlinedBlockContext = new InlinedBlockContext(myInlinedBlockContext, block, resultNullability == Nullability.NOT_NULL, target); + startElement(block); } private void exitInlinedBlock() { + finishElement(myInlinedBlockContext.myCodeBlock); myInlinedBlockContext = myInlinedBlockContext.myPreviousBlock; popTrap(Trap.InsideInlinedBlock.class); // Pop transfer value @@ -2144,24 +2113,39 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } } - public static class InlinedBlockContext { + static class InlinedBlockContext { final InlinedBlockContext myPreviousBlock; - final @NotNull PsiElement myCodeBlock; // either PsiLambdaExpression or PsiSwitchExpression currently + final @NotNull PsiCodeBlock myCodeBlock; final boolean myForceNonNullBlockResult; final @NotNull DfaVariableValue myTarget; - public InlinedBlockContext(InlinedBlockContext previousBlock, - @NotNull PsiElement codeBlock, - boolean forceNonNullBlockResult, - @NotNull DfaVariableValue target) { + InlinedBlockContext(InlinedBlockContext previousBlock, + @NotNull PsiCodeBlock codeBlock, + boolean forceNonNullBlockResult, + @NotNull DfaVariableValue target) { myPreviousBlock = previousBlock; myCodeBlock = codeBlock; myForceNonNullBlockResult = forceNonNullBlockResult; myTarget = target; } - - public boolean isSwitch() { - return myCodeBlock instanceof PsiSwitchExpression; + + boolean isSwitch() { + return myCodeBlock.getParent() instanceof PsiSwitchExpression; + } + + void generateReturn(PsiExpression returnValue, ControlFlowAnalyzer analyzer) { + if (returnValue != null) { + analyzer.addInstruction(new PushInstruction(myTarget, null, true)); + returnValue.accept(analyzer); + analyzer.generateBoxingUnboxingInstructionFor(returnValue, myTarget.getType()); + if (myForceNonNullBlockResult) { + analyzer.addInstruction(new CheckNotNullInstruction(NullabilityProblemKind.nullableFunctionReturn.problem(returnValue))); + } + analyzer.addInstruction(new AssignInstruction(returnValue, null)); + analyzer.addInstruction(new PopInstruction()); + } + + analyzer.jumpOut(myCodeBlock); } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/controlTransfer.kt b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/controlTransfer.kt index 604224ed4791..62d6caeb6d99 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/controlTransfer.kt +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/controlTransfer.kt @@ -124,7 +124,7 @@ sealed class Trap(val anchor: PsiElement) { return handler.dispatch() } } - class InsideInlinedBlock(block: PsiElement): Trap(block) { + class InsideInlinedBlock(block: PsiCodeBlock): Trap(block) { override fun dispatch(handler: ControlTransferHandler): List { (handler.state.pop() as DfaControlTransferValue).target as ReturnTransfer return handler.dispatch() diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/SwitchExpressionsJava12.java b/java/java-tests/testData/inspection/dataFlow/fixture/SwitchExpressionsJava12.java index fa35af7e334f..84e11ada1e01 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/SwitchExpressionsJava12.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/SwitchExpressionsJava12.java @@ -55,103 +55,22 @@ public class SwitchExpressionsJava12 { if (i < 1) {} } - static void testSwitchInInlinedLambda(int i) { - int x = ((IntSupplier)() -> { - int j = switch (i) { - case 1 -> 2; - case 2 -> 3; - case 3 -> { - System.out.println("hello"); - return 1; // exit lambda, not switch! - } - default -> 4; + static void testIncomplete(Integer i) { + if (i == null) { + System.out.println(switch (i)); + } else { + int x = switch(i) { + case 1 -> + case 2 -> { + if (i == 1) {} + throw new IllegalArgumentException(); + } + default -> 1; }; - if (j < 2) {} - if (j == 4 && i == 2) {} - if (i == 3) {} - return 0; - }).getAsInt(); - if (x == 1 && i == 3) {} - } - - static void testSwitchInInlinedLambdaWithInnerFinally(int i) { - int x = ((IntSupplier)() -> { - int j = switch (i) { - case 1 -> 2; - case 2 -> 3; - case 3 -> { - try { - System.out.println("hello"); - return 1; - } - finally { - return 2; - } - } - default -> 4; - }; - if (j < 2) {} - if (j == 4 && i == 2) {} - if (i == 3) {} - return 0; - }).getAsInt(); - if (x == 1 && i == 3) {} - if (x == 2 && i == 3) {} - } - - static void testSwitchInInlinedLambdaWithOuterFinally(int i) { - int x = ((IntSupplier)() -> { - int j = 0; - try { - j = switch (i) { - case 1 -> 2; - case 2 -> 3; - case 3 -> { - System.out.println("hello"); - return 1; - } - default -> 4; - }; + if (x != 1) { + // the only case where we don't know the returned value is i == 1, so the warning is logical here + if (i == 1) {} } - finally { - if (j == 0) { - return 2; - } - } - if (j < 2) {} - if (j == 4 && i == 2) {} - if (i == 3) {} - return 0; - }).getAsInt(); - if (x == 1 && i == 3) {} - if (x == 2 && i == 3) {} - } - - static int get(int x) { - return x; - } - - void testStatementInsideExpressionInsideBlockLambda(int x, int y) { - int i = ((IntSupplier)() -> { - System.out.println(); - return switch(x) { - case 1 -> { - switch (y) { - default -> get(x); - } - return 5; - } - default -> 10; - }; - }).getAsInt(); - if (i != 10) {} - } - - void testSwitchWithReturnInsideExpressionLambda(int x) { - int i = ((IntSupplier)() -> x < 0 ? 0 : switch(x) { - case 1 -> { return 2; } - default -> 3; - }).getAsInt(); - if (i == 2 && x == 1) {} + } } }