From d722c2cace0a00284650bb2b452d543478ad6484 Mon Sep 17 00:00:00 2001 From: markbrady Date: Fri, 13 Dec 2024 12:44:07 -0800 Subject: [PATCH] [StatementSwitchToExpressionSwitch] Combine immediately-preceding uninitialized variable with assignment via assignment switch PiperOrigin-RevId: 705971103 --- .../StatementSwitchToExpressionSwitch.java | 270 +++++++++++- ...StatementSwitchToExpressionSwitchTest.java | 387 +++++++++++++++++- 2 files changed, 626 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java index 4bb86a577ac..3934ec77b70 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java @@ -42,7 +42,9 @@ import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker.SwitchTreeMatcher; import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.CompileTimeConstantExpressionMatcher; import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; import com.google.errorprone.matchers.Matchers; import com.google.errorprone.util.ASTHelpers; import com.google.errorprone.util.ErrorProneComment; @@ -56,16 +58,22 @@ import com.sun.source.tree.ExpressionStatementTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.ModifiersTree; import com.sun.source.tree.ReturnTree; import com.sun.source.tree.StatementTree; import com.sun.source.tree.SwitchTree; import com.sun.source.tree.Tree; import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; +import com.sun.source.util.TreePathScanner; import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Symbol.VarSymbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.tree.JCTree.JCAssign; import com.sun.tools.javac.tree.JCTree.JCAssignOp; +import com.sun.tools.javac.tree.JCTree.JCVariableDecl; import com.sun.tools.javac.tree.Pretty; import java.io.BufferedReader; import java.io.CharArrayReader; @@ -84,6 +92,7 @@ import javax.lang.model.element.ElementKind; /** Checks for statement switches that can be expressed as an equivalent expression switch. */ +@SuppressWarnings("PatternMatchingInstanceof") @BugPattern( severity = WARNING, summary = "This statement switch can be converted to an equivalent expression switch") @@ -101,6 +110,7 @@ public final class StatementSwitchToExpressionSwitch extends BugChecker private static final AssignmentSwitchAnalysisResult DEFAULT_ASSIGNMENT_SWITCH_ANALYSIS_RESULT = AssignmentSwitchAnalysisResult.of( /* canConvertToAssignmentSwitch= */ false, + /* canCombineWithPrecedingVariableDeclaration= */ false, /* assignmentTargetOptional= */ Optional.empty(), /* assignmentKindOptional= */ Optional.empty(), /* assignmentSourceCodeOptional= */ Optional.empty()); @@ -112,6 +122,8 @@ public final class StatementSwitchToExpressionSwitch extends BugChecker DEFAULT_ASSIGNMENT_SWITCH_ANALYSIS_RESULT, /* groupedWithNextCase= */ ImmutableList.of()); private static final String EQUALS_STRING = "="; + private static final Matcher compileTimeConstExpressionMatcher = + CompileTimeConstantExpressionMatcher.instance(); // Tri-state to represent the fall-thru control flow of a particular case of a particular // statement switch @@ -282,11 +294,46 @@ private static AnalysisResult analyzeSwitchTree(SwitchTree switchTree, VisitorSt // The switch must be exhaustive (at compile time) && exhaustive; + List precedingStatements = getPrecedingStatementsInBlock(switchTree, state); + Optional assignmentTarget = + assignmentSwitchAnalysisState.assignmentTargetOptional(); + + // If present, the variable tree that can be combined with the switch block + Optional combinableVariableTree = + precedingStatements.isEmpty() + ? Optional.empty() + : Optional.of(precedingStatements) + // Don't try to combine when multiple variables are declared together + .filter( + StatementSwitchToExpressionSwitch + ::precedingTwoStatementsNotInSameVariableDeclaratorList) + // Extract the immediately preceding statement + .map(statements -> statements.get(statements.size() - 1)) + // Preceding statement must be a variable declaration + .filter(precedingStatement -> precedingStatement instanceof VariableTree) + .map(precedingStatement -> (VariableTree) precedingStatement) + // Variable must be uninitialized, or initialized with a compile-time constant + .filter( + variableTree -> + (variableTree.getInitializer() == null) + || compileTimeConstExpressionMatcher.matches( + variableTree.getInitializer(), state)) + // If we are reading the initialized value in the switch block, we can't remove it + .filter( + variableTree -> noReadsOfVariable(ASTHelpers.getSymbol(variableTree), state)) + // Type of the variable and the switch assignment must be compatible + .filter( + variableTree -> + isCompatibleWithFirstAssignment(assignmentTarget, variableTree)); + boolean canCombineWithPrecedingVariableDeclaration = + canConvertToAssignmentSwitch && combinableVariableTree.isPresent(); + return AnalysisResult.of( /* canConvertDirectlyToExpressionSwitch= */ allCasesHaveDefiniteControlFlow, canConvertToReturnSwitch, AssignmentSwitchAnalysisResult.of( canConvertToAssignmentSwitch, + canCombineWithPrecedingVariableDeclaration, assignmentSwitchAnalysisState.assignmentTargetOptional(), assignmentSwitchAnalysisState.assignmentExpressionKindOptional(), assignmentSwitchAnalysisState @@ -295,6 +342,47 @@ private static AnalysisResult analyzeSwitchTree(SwitchTree switchTree, VisitorSt ImmutableList.copyOf(groupedWithNextCase)); } + /** + * Returns whether the variable {@code symbol} is read within the scope of the {@code + * VisitorState}. (Writes to the variable are ignored.) + */ + private static boolean noReadsOfVariable(VarSymbol symbol, VisitorState state) { + + // MockNotUsedInProduction.java + Set referencedLocalVariables = new HashSet<>(); + new TreePathScanner() { + + @Override + public Void visitAssignment(AssignmentTree tree, Void unused) { + // Only looks at the right-hand side of the assignment + return scan(tree.getExpression(), null); + } + + @Override + public Void visitMemberSelect(MemberSelectTree memberSelect, Void unused) { + handle(memberSelect); + return super.visitMemberSelect(memberSelect, null); + } + + @Override + public Void visitIdentifier(IdentifierTree identifier, Void unused) { + handle(identifier); + return super.visitIdentifier(identifier, null); + } + + private void handle(Tree tree) { + var symbol = getSymbol(tree); + if (symbol instanceof VarSymbol varSymbol) { + referencedLocalVariables.add(varSymbol); + } + } + }.scan(state.getPath(), null); + + System.out.println( + String.format("XXX referencedLocalVariables: %s", referencedLocalVariables.toString())); + return !referencedLocalVariables.contains(symbol); + } + /** * Renders the Java source code for a [compound] assignment operator. The parameter must be either * an {@code AssignmentTree} or a {@code CompoundAssignmentTree}. @@ -488,6 +576,19 @@ private static boolean isCompatibleWithFirstAssignment( return Objects.equals(assignmentTargetSymbol, caseAssignmentTargetSymbol); } + private static boolean isCompatibleWithFirstAssignment( + Optional assignmentTargetOptional, VariableTree variableDefinition) { + + if (assignmentTargetOptional.isEmpty()) { + return false; + } + + Symbol assignmentTargetSymbol = getSymbol(assignmentTargetOptional.get()); + + Symbol definedSymbol = ASTHelpers.getSymbol(variableDefinition); + return Objects.equals(assignmentTargetSymbol, definedSymbol); + } + /** * Determines whether the supplied case's {@code statements} are capable of being mapped to an * equivalent expression switch case (without repeating code), returning {@code true} if so. @@ -740,6 +841,36 @@ private static SuggestedFix convertToReturnSwitch( return suggestedFixBuilder.build(); } + private static List getPrecedingStatementsInBlock( + SwitchTree switchTree, VisitorState state) { + + List precedingStatements = new ArrayList<>(); + + if (!Matchers.previousStatement(Matchers.anything()) + .matches(switchTree, state)) { + // No lowest-ancestor block or no preceding statements + System.out.println("XXX no preceding statement"); + return precedingStatements; + } + + // Fetch the lowest ancestor statement block + TreePath pathToEnclosing = state.findPathToEnclosing(BlockTree.class); + // NOMUTANTS--should early return above + if (pathToEnclosing != null) { + Tree enclosing = pathToEnclosing.getLeaf(); + if (enclosing instanceof BlockTree blockTree) { + // Path from root -> switchTree + TreePath rootToSwitchPath = TreePath.getPath(pathToEnclosing, switchTree); + + for (int i = 0; i < findBlockStatementIndex(rootToSwitchPath, blockTree); i++) { + precedingStatements.add(blockTree.getStatements().get(i)); + } + } + } + // Should have returned above + return precedingStatements; + } + /** * Retrieves a list of all statements (if any) following the supplied {@code SwitchTree} in its * lowest-ancestor statement block (if any). @@ -790,6 +921,34 @@ private static int findBlockStatementIndex(TreePath treePath, BlockTree blockTre return -1; } + /** + * Determines whether the last two preceding statements are not variable declarations within the + * same VariableDeclaratorList, for example {@code int x, y;}. VariableDeclaratorLists are defined + * in e.g. JLS 21 § 14.4. Precondition: all preceding statements are taken from the same {@code + * BlockTree}. + */ + private static boolean precedingTwoStatementsNotInSameVariableDeclaratorList( + List precedingStatements) { + + if (precedingStatements.size() < 2) { + return true; + } + + StatementTree secondToLastStatement = precedingStatements.get(precedingStatements.size() - 2); + StatementTree lastStatement = precedingStatements.get(precedingStatements.size() - 1); + if (!(secondToLastStatement instanceof VariableTree) + || !(lastStatement instanceof VariableTree)) { + return true; + } + + VariableTree variableTree1 = (VariableTree) secondToLastStatement; + VariableTree variableTree2 = (VariableTree) lastStatement; + + // Start positions will vary if the variable declarations are in the same + // VariableDeclaratorList. + return getStartPosition(variableTree1) != getStartPosition(variableTree2); + } + /** * Transforms the supplied statement switch into an assignment switch style of expression switch. * In this conversion, each nontrivial statement block is mapped one-to-one to a new expression on @@ -800,30 +959,94 @@ private static int findBlockStatementIndex(TreePath treePath, BlockTree blockTre private static SuggestedFix convertToAssignmentSwitch( SwitchTree switchTree, VisitorState state, AnalysisResult analysisResult) { + List statementsToDelete = new ArrayList<>(); + StringBuilder replacementCodeBuilder = new StringBuilder(); + + if (analysisResult + .assignmentSwitchAnalysisResult() + .canCombineWithPrecedingVariableDeclaration()) { + + List precedingStatements = getPrecedingStatementsInBlock(switchTree, state); + VariableTree variableTree = + (VariableTree) precedingStatements.get(precedingStatements.size() - 1); + + if (variableTree instanceof JCVariableDecl) { + + JCVariableDecl variableDecl = (JCVariableDecl) variableTree; + System.out.println( + "XXX JCVariableDecl: --->" + + state.getSourceForNode(variableDecl) + + " <---" + + " name " + + variableDecl.name.toString()); + } + + System.out.println( + "XXX variableTree: " + state.getSourceForNode(variableTree) + " which is comprised of:"); + + statementsToDelete.add(variableTree); + + ImmutableList allVariableComments = + state.getTokensForNode(variableTree).stream() + .flatMap(errorProneToken -> errorProneToken.comments().stream()) + .collect(toImmutableList()); + + Symbol assignmentTargetSymbol = getSymbol(variableTree.getNameExpression()); + System.out.println("XXX assignmentTargetSymbol in def: " + assignmentTargetSymbol); + + allVariableComments.stream() + .filter(comment -> !comment.getText().isEmpty()) + .forEach( + comment -> { + // + comment.content().length(); + replacementCodeBuilder.append(comment.getText()).append("\n"); + }); + + ModifiersTree modifiersTree = variableTree.getModifiers(); + if (!modifiersTree.getAnnotations().isEmpty()) { + modifiersTree + .getAnnotations() + .forEach( + annotation -> { + System.out.println("XXX annotation: " + state.getSourceForNode(annotation)); + replacementCodeBuilder.append(state.getSourceForNode(annotation)).append("\n"); + }); + } + + if (!modifiersTree.getFlags().isEmpty()) { + + modifiersTree + .getFlags() + .forEach( + flag -> { + System.out.println("XXX flag: " + flag); + replacementCodeBuilder.append(flag.toString()).append(" "); + }); + } + + replacementCodeBuilder.append(state.getSourceForNode(variableTree.getType())).append(" "); + + // replacementCodeBuilder.append(variableTree.getName().toString()).append(" = "); + } + List cases = switchTree.getCases(); ImmutableList allSwitchComments = state.getTokensForNode(switchTree).stream() .flatMap(errorProneToken -> errorProneToken.comments().stream()) .collect(toImmutableList()); - StringBuilder replacementCodeBuilder = - new StringBuilder( - state.getSourceForNode( - analysisResult - .assignmentSwitchAnalysisResult() - .assignmentTargetOptional() - .get())) - .append(" ") - // Invariant: always present when a finding exists - .append( - analysisResult - .assignmentSwitchAnalysisResult() - .assignmentSourceCodeOptional() - .get()) - .append(" ") - .append("switch ") - .append(state.getSourceForNode(switchTree.getExpression())) - .append(" {"); + replacementCodeBuilder + .append( + state.getSourceForNode( + analysisResult.assignmentSwitchAnalysisResult().assignmentTargetOptional().get())) + .append(" ") + // Invariant: always present when a finding exists + .append( + analysisResult.assignmentSwitchAnalysisResult().assignmentSourceCodeOptional().get()) + .append(" ") + .append("switch ") + .append(state.getSourceForNode(switchTree.getExpression())) + .append(" {"); StringBuilder groupedCaseCommentsAccumulator = null; boolean firstCaseInGroup = true; @@ -903,7 +1126,10 @@ private static SuggestedFix convertToAssignmentSwitch( // Close the switch statement replacementCodeBuilder.append("\n};"); - return SuggestedFix.builder().replace(switchTree, replacementCodeBuilder.toString()).build(); + SuggestedFix.Builder suggestedFixBuilder = + SuggestedFix.builder().replace(switchTree, replacementCodeBuilder.toString()); + statementsToDelete.forEach(deleteMe -> suggestedFixBuilder.replace(deleteMe, "")); + return suggestedFixBuilder.build(); } /** @@ -1258,6 +1484,10 @@ abstract static class AssignmentSwitchAnalysisResult { // Whether the statement switch can be converted to an assignment switch abstract boolean canConvertToAssignmentSwitch(); + // Whether the assignment switch can be combined with the immediately preceding variable + // declaration + abstract boolean canCombineWithPrecedingVariableDeclaration(); + // Target of the assignment switch, if any abstract Optional assignmentTargetOptional(); @@ -1269,11 +1499,13 @@ abstract static class AssignmentSwitchAnalysisResult { static AssignmentSwitchAnalysisResult of( boolean canConvertToAssignmentSwitch, + boolean canCombineWithPrecedingVariableDeclaration, Optional assignmentTargetOptional, Optional assignmentKindOptional, Optional assignmentSourceCodeOptional) { return new AutoValue_StatementSwitchToExpressionSwitch_AssignmentSwitchAnalysisResult( canConvertToAssignmentSwitch, + canCombineWithPrecedingVariableDeclaration, assignmentTargetOptional, assignmentKindOptional, assignmentSourceCodeOptional); diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java index 60a6a12efa9..f9fbaa2da7e 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java @@ -3131,9 +3131,12 @@ public int foo(Side side) { **********************************/ @Test - public void switchByEnum_assignmentSwitchToLocalHasDefault_error() { - helper - .addSourceLines( + public void switchByEnum_assignmentSwitchToLocalHasDefault2_error() { + System.out.println("YYY test case begin"); + + // Check correct generated code + refactoringHelper + .addInputLines( "Test.java", """ class Test { @@ -3147,12 +3150,11 @@ enum Side { public Test(int foo) {} public int foo(Side side) { - int x = 0; - // BUG: Diagnostic contains: [StatementSwitchToExpressionSwitch] + int y = 0, x; switch (side) { case HEART: case DIAMOND: - x = (((x + 1) * (x * x)) << 1); + x = ((y + 1) * (y * y)) << 1; break; case SPADE: throw new RuntimeException(); @@ -3163,16 +3165,49 @@ public int foo(Side side) { } } """) + .addOutputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int foo(Side side) { + int y = 0, x; + x = + switch (side) { + case HEART, DIAMOND -> ((y + 1) * (y * y)) << 1; + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return x; + } + } + """) .setArgs( ImmutableList.of( "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .doTest(); + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + + System.out.println("YYY test case ends"); + } + + @Test + public void switchByEnum_assignmentSwitchToLocalHasDefault3_error() { + System.out.println("YYYYYYY test case begin"); // Check correct generated code refactoringHelper .addInputLines( "Test.java", """ + import java.lang.annotation.Repeatable; class Test { enum Side { HEART, @@ -3181,14 +3216,23 @@ enum Side { CLUB }; + @interface MyAnnos {Test.MyAnno[] value();} + @Repeatable(Test.MyAnnos.class) + @interface MyAnno {String v() default "";} + @interface MyOtherAnno {} + public Test(int foo) {} public int foo(Side side) { - int x = 0; + int y = 0; + @MyAnno(v = "foo") + // alpha + /* beta */ @MyOtherAnno @MyAnno int /* gamma */ x /* delta */; // epsilon + // zeta switch (side) { case HEART: case DIAMOND: - x = ((x + 1) * (x * x)) << 1; + x = ((y + 1) * (y * y)) << 1; break; case SPADE: throw new RuntimeException(); @@ -3202,6 +3246,7 @@ public int foo(Side side) { .addOutputLines( "Test.java", """ + import java.lang.annotation.Repeatable; class Test { enum Side { HEART, @@ -3210,13 +3255,170 @@ enum Side { CLUB }; + @interface MyAnnos {Test.MyAnno[] value();} + @Repeatable(Test.MyAnnos.class) + @interface MyAnno {String v() default "";} + @interface MyOtherAnno {} + public Test(int foo) {} public int foo(Side side) { - int x = 0; + int y = 0; + // epsilon + // zeta + // alpha + /* beta */ + /* gamma */ + /* delta */ + @MyAnno(v = "foo") + @MyOtherAnno + @MyAnno + int x = + switch (side) { + case HEART, DIAMOND -> ((y + 1) * (y * y)) << 1; + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + ImmutableList.of( + "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + + System.out.println("YYYYYYY test case ends"); + } + + @Test + public void switchByEnum_assignmentSwitchToLocalHasDefault5_error() { + // Dead store of a compile-time constant to local variable {@code x} can be elided. + System.out.println("YYYYYYY5 test case begin"); + + // Check correct generated code + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int foo(Side side) { + int y = 0; + int x = 999; + switch (side) { + case HEART: + case DIAMOND: + x = ((y + 1) * (y * y)) << 1; + break; + case SPADE: + throw new RuntimeException(); + default: + throw new NullPointerException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int foo(Side side) { + int y = 0; + + int x = + switch (side) { + case HEART, DIAMOND -> ((y + 1) * (y * y)) << 1; + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + ImmutableList.of( + "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + + System.out.println("YYYYYYY5 test case ends"); + } + + @Test + public void switchByEnum_assignmentSwitchToLocalHasDefault4_error() { + System.out.println("YYY4 test case begin"); + + // Check correct generated code + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int foo(Side side) { + int z = 3; + int x; + int y; + switch (side) { + case HEART: + case DIAMOND: + x = ((z + 1) * (z * z)) << 1; + break; + case SPADE: + throw new RuntimeException(); + default: + throw new NullPointerException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int foo(Side side) { + int z = 3; + int x; + int y; x = switch (side) { - case HEART, DIAMOND -> ((x + 1) * (x * x)) << 1; + case HEART, DIAMOND -> ((z + 1) * (z * z)) << 1; case SPADE -> throw new RuntimeException(); default -> throw new NullPointerException(); }; @@ -3227,7 +3429,164 @@ public int foo(Side side) { .setArgs( ImmutableList.of( "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .doTest(); + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + + System.out.println("YYY4 test case ends"); + } + + @Test + public void switchByEnum_assignmentSwitchToLocalHasDefault6_error() { + System.out.println("YYY6 test case begin"); + + // Check correct generated code + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int foo(Side side) { + int z = 3; + int x; + { + { + switch (side) { + case HEART: + case DIAMOND: + x = ((z + 1) * (z * z)) << 1; + break; + case SPADE: + throw new RuntimeException(); + default: + throw new NullPointerException(); + } + } + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int foo(Side side) { + int z = 3; + int x; + { + { + x = + switch (side) { + case HEART, DIAMOND -> ((z + 1) * (z * z)) << 1; + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + } + } + return x; + } + } + """) + .setArgs( + ImmutableList.of( + "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + + System.out.println("YYY6 test case ends"); + } + + @Test + public void switchByEnum_assignmentSwitchToLocalHasDefault7_error() { + // Local variable {@code x} is initialized by reading a {@code volatile} field, which has memory + // visibility effects (refer to e.g. JLS 21 § 17.4.2); therefore, should not be combined with + // the switch assignment. + System.out.println("YYY7 test case begin"); + + // Check correct generated code + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + volatile int v = 0; + + public int foo(Side side) { + int z = 3; + int x = v; + switch (side) { + case HEART: + case DIAMOND: + x = ((z + 1) * (z * z)) << 1; + break; + case SPADE: + throw new RuntimeException(); + default: + throw new NullPointerException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + volatile int v = 0; + + public int foo(Side side) { + int z = 3; + int x = v; + x = + switch (side) { + case HEART, DIAMOND -> ((z + 1) * (z * z)) << 1; + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + ImmutableList.of( + "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + + System.out.println("YYY7 test case ends"); } @Test @@ -3934,6 +4293,10 @@ public void switchByEnum_exhaustiveAssignmentSwitch_error() { // Transformation can change error handling. Here, if the enum is not exhaustive at runtime // (say there is a new JOKER suit), then nothing would happen. But the transformed source, // would throw. + + // Note also that the initial value of {@code x} is used in the computation inside the switch, + // thus its definition is not eligible to be combined with the switch (e.g. {@code int x = + // switch (...)}). helper .addSourceLines( "Test.java",