Skip to content

Commit

Permalink
Orphaned default
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 646122115
  • Loading branch information
@java-team-github-bot
java-team-github-bot authored and Error Prone Team committed Jun 24, 2024
1 parent 3369aaa commit a6e0139
Show file tree
Hide file tree
Showing 2 changed files with 174 additions and 30 deletions.
55 changes: 25 additions & 30 deletions core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
import com.sun.source.util.TreePath;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.JCTree.JCAssign;
import com.sun.tools.javac.tree.JCTree.JCAssignOp;
import com.sun.tools.javac.tree.Pretty;
Expand Down Expand Up @@ -529,8 +528,7 @@ private static SuggestedFix convertDirectlyToExpressionSwitch(
// For readability, filter out trailing unlabelled break statement because these become a
// No-Op when used inside expression switches
ImmutableList<StatementTree> filteredStatements = filterOutRedundantBreak(caseTree);
String transformedBlockSource =
transformBlock(caseTree, state, cases, caseIndex, filteredStatements);
String transformedBlockSource = transformBlock(caseTree, state, filteredStatements);

if (firstCaseInGroup) {
groupedCaseCommentsAccumulator = new StringBuilder();
Expand Down Expand Up @@ -623,7 +621,7 @@ private static SuggestedFix convertToReturnSwitch(
boolean isDefaultCase = caseTree.getExpression() == null;

String transformedBlockSource =
transformReturnOrThrowBlock(caseTree, state, cases, caseIndex, getStatements(caseTree));
transformReturnOrThrowBlock(caseTree, state, getStatements(caseTree));

if (firstCaseInGroup) {
groupedCaseCommentsAccumulator = new StringBuilder();
Expand Down Expand Up @@ -725,7 +723,7 @@ private static int findBlockStatementIndex(TreePath treePath, BlockTree blockTre
/**
* 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
* the right-hand side of the arrow. Comments are presevered where possible. Precondition: the
* the right-hand side of the arrow. Comments are preserved where possible. Precondition: the
* {@code AnalysisResult} for the {@code SwitchTree} must have deduced that this conversion is
* possible.
*/
Expand Down Expand Up @@ -760,7 +758,7 @@ private static SuggestedFix convertToAssignmentSwitch(
ImmutableList<StatementTree> filteredStatements = filterOutRedundantBreak(caseTree);

String transformedBlockSource =
transformAssignOrThrowBlock(caseTree, state, cases, caseIndex, filteredStatements);
transformAssignOrThrowBlock(caseTree, state, filteredStatements);

if (firstCaseInGroup) {
groupedCaseCommentsAccumulator = new StringBuilder();
Expand Down Expand Up @@ -870,17 +868,13 @@ private static List<? extends StatementTree> getStatements(CaseTree caseTree) {

/** Transforms code for this case into the code under an expression switch. */
private static String transformBlock(
CaseTree caseTree,
VisitorState state,
List<? extends CaseTree> cases,
int caseIndex,
ImmutableList<StatementTree> filteredStatements) {
CaseTree caseTree, VisitorState state, ImmutableList<StatementTree> filteredStatements) {

StringBuilder transformedBlockBuilder = new StringBuilder();
int codeBlockStart = extractLhsComments(caseTree, state, transformedBlockBuilder);
int codeBlockEnd =
filteredStatements.isEmpty()
? getBlockEnd(state, caseTree, cases, caseIndex)
? getBlockEnd(state, caseTree)
: state.getEndPosition(Streams.findLast(filteredStatements.stream()).get());
transformedBlockBuilder.append(state.getSourceCode(), codeBlockStart, codeBlockEnd);

Expand Down Expand Up @@ -911,21 +905,30 @@ private static int extractLhsComments(

/**
* Finds the position in source corresponding to the end of the code block of the supplied {@code
* caseIndex} within all {@code cases}.
* caseIndex} within all {@code cases}. Special case: if the case tree consists of a single
* statement that is a non-empty block, then the end of the last statement of that block is
* returned.
*/
private static int getBlockEnd(
VisitorState state, CaseTree caseTree, List<? extends CaseTree> cases, int caseIndex) {
private static int getBlockEnd(VisitorState state, CaseTree caseTree) {

if (caseIndex == cases.size() - 1) {
List<? extends StatementTree> statements = caseTree.getStatements();
if (statements == null || statements.size() != 1) {
return state.getEndPosition(caseTree);
}
StatementTree onlyStatement = getOnlyElement(statements);
if (!onlyStatement.getKind().equals(BLOCK)) {
return state.getEndPosition(caseTree);
}

return ((JCTree) cases.get(caseIndex + 1)).getStartPosition();
List<? extends StatementTree> blockStatements = ((BlockTree) onlyStatement).getStatements();
return blockStatements.isEmpty()
? state.getEndPosition(caseTree)
: state.getEndPosition(blockStatements.get(blockStatements.size() - 1));
}

/**
* Determines whether the supplied {@code case}'s logic should be expressed on the right of the
* arrow symbol without braces, incorporating both language and readabilitiy considerations.
* arrow symbol without braces, incorporating both language and readability considerations.
*/
private static boolean shouldTransformCaseWithoutBraces(
ImmutableList<StatementTree> statementTrees) {
Expand Down Expand Up @@ -995,17 +998,13 @@ private static boolean isSwitchExhaustive(
* transformed to {@code x+1;}.
*/
private static String transformReturnOrThrowBlock(
CaseTree caseTree,
VisitorState state,
List<? extends CaseTree> cases,
int caseIndex,
List<? extends StatementTree> statements) {
CaseTree caseTree, VisitorState state, List<? extends StatementTree> statements) {

StringBuilder transformedBlockBuilder = new StringBuilder();
int codeBlockStart;
int codeBlockEnd =
statements.isEmpty()
? getBlockEnd(state, caseTree, cases, caseIndex)
? getBlockEnd(state, caseTree)
: state.getEndPosition(Streams.findLast(statements.stream()).get());

if (statements.size() == 1 && statements.get(0).getKind().equals(RETURN)) {
Expand All @@ -1028,17 +1027,13 @@ private static String transformReturnOrThrowBlock(
* {@code >>=}).
*/
private static String transformAssignOrThrowBlock(
CaseTree caseTree,
VisitorState state,
List<? extends CaseTree> cases,
int caseIndex,
List<? extends StatementTree> statements) {
CaseTree caseTree, VisitorState state, List<? extends StatementTree> statements) {

StringBuilder transformedBlockBuilder = new StringBuilder();
int codeBlockStart;
int codeBlockEnd =
statements.isEmpty()
? getBlockEnd(state, caseTree, cases, caseIndex)
? getBlockEnd(state, caseTree)
: state.getEndPosition(Streams.findLast(statements.stream()).get());

if (!statements.isEmpty() && statements.get(0).getKind().equals(EXPRESSION_STATEMENT)) {
Expand Down
149 changes: 149 additions & 0 deletions ...rc/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,155 @@ public void switchByEnum_caseHasOnlyComments_error() {
.doTest();
}

@Test
public void switchByEnum_surroundingBracesCannotRemove_error() {
// Can't remove braces around OBVERSE because break statements are not a member of
// KINDS_CONVERTIBLE_WITHOUT_BRACES
assumeTrue(RuntimeVersion.isAtLeast14());
helper
.addSourceLines(
"Test.java",
"class Test {",
" enum Side {OBVERSE, REVERSE};",
" public Test(int foo) {",
" }",
" ",
" public void foo(Side side) { ",
" // BUG: Diagnostic contains: [StatementSwitchToExpressionSwitch]",
" switch(side) {",
" case OBVERSE: {",
" // The quick brown fox, jumps over the lazy dog, etc.",
" break;",
" }",
" ",
" default: { ",
" throw new RuntimeException(\"Invalid type.\");",
" }",
" }",
" }",
"}")
.setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")
.doTest();

// Check correct generated code
refactoringHelper
.addInputLines(
"Test.java",
"class Test {",
" enum Side {OBVERSE, REVERSE};",
" public Test(int foo) {",
" }",
" ",
" public void foo(Side side) { ",
" switch(side) {",
" case OBVERSE: {",
" // The quick brown fox, jumps over the lazy dog, etc.",
" break;",
" }",
" ",
" default: { ",
" throw new RuntimeException(\"Invalid type.\");",
" }",
" }",
" }",
"}")
.addOutputLines(
"Test.java",
"class Test {",
" enum Side {OBVERSE, REVERSE};",
" public Test(int foo) {",
" }",
" ",
" public void foo(Side side) { ",
" switch(side) {",
" case OBVERSE -> {",
" // The quick brown fox, jumps over the lazy dog, etc.",
" break;",
" }",
" ",
" default -> ",
" throw new RuntimeException(\"Invalid type.\");",
" ",
" }",
" }",
"}")
.setArgs(
ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion"))
.doTest();
}

@Test
public void switchByEnum_surroundingBraces_error() {
// Test handling of cases with surrounding braces.
assumeTrue(RuntimeVersion.isAtLeast14());

helper
.addSourceLines(
"Test.java",
"class Test {",
" enum Side {OBVERSE, REVERSE};",
" public Test(int foo) {",
" }",
" ",
" public void foo(Side side) { ",
" // BUG: Diagnostic contains: [StatementSwitchToExpressionSwitch]",
" switch(side) {",
" case OBVERSE: {",
" // The quick brown fox, jumps over the lazy dog, etc.",
" throw new RuntimeException(\"Invalid.\");",
" }",
" ",
" default: { ",
" }",
" }",
" }",
"}")
.setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")
.doTest();

// Check correct generated code
refactoringHelper
.addInputLines(
"Test.java",
"class Test {",
" enum Side {OBVERSE, REVERSE};",
" public Test(int foo) {",
" }",
" ",
" public void foo(Side side) { ",
" switch(side) {",
" case OBVERSE: {",
" // The quick brown fox, jumps over the lazy dog, etc.",
" throw new RuntimeException(\"Invalid.\");",
" }",
" ",
" default: { ",
" }",
" }",
" }",
"}")
.addOutputLines(
"Test.java",
"class Test {",
" enum Side {OBVERSE, REVERSE};",
" public Test(int foo) {",
" }",
" ",
" public void foo(Side side) { ",
" switch(side) {",
" case OBVERSE -> ",
" // The quick brown fox, jumps over the lazy dog, etc.",
" throw new RuntimeException(\"Invalid.\");",
" default -> {}",
" ",
" }",
" }",
"}")
.setArgs(
ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion"))
.doTest();
}

/**********************************
*
* Return switch test cases
Expand Down

0 comments on commit a6e0139

Please sign in to comment.