Skip to content

Commit

Permalink
[StatementSwitchToExpressionSwitch] for direct conversion pattern, if…
Browse files Browse the repository at this point in the history
… the switch has a `default:` clause and is otherwise exhaustive (even without said `default:` clause), then propose an alternative fix which removes the `default:` clause and its code.

The first fix always retains the `default:` clause.

PiperOrigin-RevId: 716336071
  • Loading branch information
markhbrady authored and Error Prone Team committed Jan 16, 2025
1 parent 1bb8831 commit 4f61501
Show file tree
Hide file tree
Showing 2 changed files with 286 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,15 @@ public Description matchSwitch(SwitchTree switchTree, VisitorState state) {
}
}
if (enableDirectConversion && analysisResult.canConvertDirectlyToExpressionSwitch()) {
suggestedFixes.add(convertDirectlyToExpressionSwitch(switchTree, state, analysisResult));
suggestedFixes.add(
convertDirectlyToExpressionSwitch(
switchTree, state, analysisResult, /* removeDefault= */ false));

if (analysisResult.canRemoveDefault()) {
suggestedFixes.add(
convertDirectlyToExpressionSwitch(
switchTree, state, analysisResult, /* removeDefault= */ true));
}
}

return suggestedFixes.isEmpty()
Expand Down Expand Up @@ -639,10 +647,14 @@ private static boolean areStatementsConvertibleToExpressionSwitch(
/**
* Transforms the supplied statement switch into an expression switch directly. In this
* conversion, each nontrivial statement block is mapped one-to-one to a new {@code Expression} or
* {@code StatementBlock} on the right-hand side. Comments are preserved where possible.
* {@code StatementBlock} on the right-hand side (the `default:` case is removed if {@code
* removeDefault} is true). Comments are preserved where possible.
*/
private static SuggestedFix convertDirectlyToExpressionSwitch(
SwitchTree switchTree, VisitorState state, AnalysisResult analysisResult) {
SwitchTree switchTree,
VisitorState state,
AnalysisResult analysisResult,
boolean removeDefault) {

List<? extends CaseTree> cases = switchTree.getCases();
ImmutableList<ErrorProneComment> allSwitchComments =
Expand All @@ -662,6 +674,11 @@ private static SuggestedFix convertDirectlyToExpressionSwitch(
CaseTree caseTree = cases.get(caseIndex);
boolean isDefaultCase = isSwitchDefault(caseTree);

if (removeDefault && isDefaultCase) {
// Skip default case
continue;
}

// For readability, filter out trailing unlabelled break statement because these become a
// No-Op when used inside expression switches
ImmutableList<StatementTree> filteredStatements = filterOutRedundantBreak(caseTree);
Expand Down Expand Up @@ -756,7 +773,12 @@ private static SuggestedFix convertDirectlyToExpressionSwitch(
// Close the switch statement
replacementCodeBuilder.append("\n}");

return SuggestedFix.builder().replace(switchTree, replacementCodeBuilder.toString()).build();
SuggestedFix.Builder suggestedFixBuilder = SuggestedFix.builder();
if (removeDefault) {
suggestedFixBuilder.setShortDescription(REMOVE_DEFAULT_CASE_SHORT_DESCRIPTION);
}
suggestedFixBuilder.replace(switchTree, replacementCodeBuilder.toString());
return suggestedFixBuilder.build();
}

/**
Expand Down
Loading

0 comments on commit 4f61501

Please sign in to comment.