From 2fd3f9d6abc8711991a10a3bff10aea010f86a90 Mon Sep 17 00:00:00 2001 From: markbrady Date: Tue, 20 Jan 2026 13:07:32 -0800 Subject: [PATCH] [RefactorSwitch] add new checker to refactor arrow switches for improved readability and conciseness PiperOrigin-RevId: 858719719 --- .../bugpatterns/RefactorSwitch.java | 1192 ++++++ .../errorprone/bugpatterns/SwitchUtils.java | 428 ++ .../scanner/BuiltInCheckerSuppliers.java | 2 + .../bugpatterns/RefactorSwitchTest.java | 3452 +++++++++++++++++ docs/bugpattern/RefactorSwitch.md | 89 + 5 files changed, 5163 insertions(+) create mode 100644 core/src/main/java/com/google/errorprone/bugpatterns/RefactorSwitch.java create mode 100644 core/src/main/java/com/google/errorprone/bugpatterns/SwitchUtils.java create mode 100644 core/src/test/java/com/google/errorprone/bugpatterns/RefactorSwitchTest.java create mode 100644 docs/bugpattern/RefactorSwitch.md diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/RefactorSwitch.java b/core/src/main/java/com/google/errorprone/bugpatterns/RefactorSwitch.java new file mode 100644 index 00000000000..975bb0c2061 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/RefactorSwitch.java @@ -0,0 +1,1192 @@ +/* + * Copyright 2026 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.bugpatterns; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.common.collect.Iterables.getLast; +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.bugpatterns.SwitchUtils.KINDS_RETURN_OR_THROW; +import static com.google.errorprone.bugpatterns.SwitchUtils.REMOVE_DEFAULT_CASE_SHORT_DESCRIPTION; +import static com.google.errorprone.bugpatterns.SwitchUtils.analyzeCaseForNullAndDefault; +import static com.google.errorprone.bugpatterns.SwitchUtils.findCombinableVariableTree; +import static com.google.errorprone.bugpatterns.SwitchUtils.getPrecedingStatementsInBlock; +import static com.google.errorprone.bugpatterns.SwitchUtils.getStatements; +import static com.google.errorprone.bugpatterns.SwitchUtils.hasContinueInTree; +import static com.google.errorprone.bugpatterns.SwitchUtils.isCompatibleWithFirstAssignment; +import static com.google.errorprone.bugpatterns.SwitchUtils.isSwitchCoveringAllEnumValues; +import static com.google.errorprone.bugpatterns.SwitchUtils.printCaseExpressionsOrPatternAndGuard; +import static com.google.errorprone.bugpatterns.SwitchUtils.renderComments; +import static com.google.errorprone.bugpatterns.SwitchUtils.renderNullDefaultKindPrefix; +import static com.google.errorprone.bugpatterns.SwitchUtils.renderVariableTreeAnnotations; +import static com.google.errorprone.bugpatterns.SwitchUtils.renderVariableTreeComments; +import static com.google.errorprone.bugpatterns.SwitchUtils.renderVariableTreeFlags; +import static com.google.errorprone.matchers.Description.NO_MATCH; +import static com.google.errorprone.util.ASTHelpers.getStartPosition; +import static com.google.errorprone.util.ASTHelpers.hasImplicitType; +import static java.util.stream.Collectors.joining; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Range; +import com.google.common.collect.Streams; +import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.SwitchExpressionTreeMatcher; +import com.google.errorprone.bugpatterns.BugChecker.SwitchTreeMatcher; +import com.google.errorprone.bugpatterns.SwitchUtils.CaseQualifications; +import com.google.errorprone.bugpatterns.SwitchUtils.NullDefaultKind; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; +import com.google.errorprone.util.ErrorProneComment; +import com.google.errorprone.util.Reachability; +import com.google.errorprone.util.SourceVersion; +import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.BlockTree; +import com.sun.source.tree.CaseTree; +import com.sun.source.tree.CompoundAssignmentTree; +import com.sun.source.tree.ExpressionStatementTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.LambdaExpressionTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.PatternCaseLabelTree; +import com.sun.source.tree.ReturnTree; +import com.sun.source.tree.StatementTree; +import com.sun.source.tree.SwitchExpressionTree; +import com.sun.source.tree.SwitchTree; +import com.sun.source.tree.ThrowTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.VariableTree; +import com.sun.source.tree.YieldTree; +import com.sun.source.util.TreePath; +import com.sun.source.util.TreePathScanner; +import com.sun.tools.javac.code.Type; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Stream; +import javax.inject.Inject; + +/** Checks for refactorings that can be applied to simplify arrow switches. */ +@BugPattern(severity = WARNING, summary = "This switch can be refactored to be more readable") +public final class RefactorSwitch extends BugChecker + implements SwitchTreeMatcher, SwitchExpressionTreeMatcher { + private static final AssignmentSwitchAnalysisResult DEFAULT_ASSIGNMENT_SWITCH_ANALYSIS_RESULT = + new AssignmentSwitchAnalysisResult( + /* canConvertToAssignmentSwitch= */ false, + /* precedingVariableDeclaration= */ Optional.empty(), + /* assignmentTargetOptional= */ Optional.empty(), + /* assignmentKindOptional= */ Optional.empty(), + /* assignmentSourceCodeOptional= */ Optional.empty()); + + private static final ReturnSwitchAnalysisResult DEFAULT_RETURN_SWITCH_ANALYSIS_RESULT = + new ReturnSwitchAnalysisResult(/* canConvertToReturnSwitch= */ false, ImmutableList.of()); + + private static final SimplifyAnalysisResult DEFAULT_SIMPLIFY_ANALYSIS_RESULT = + new SimplifyAnalysisResult(false, ImmutableList.of()); + + /** Default (negative) result for overall analysis. Note that the value is immutable. */ + private static final AnalysisResult DEFAULT_ANALYSIS_RESULT = + new AnalysisResult( + DEFAULT_RETURN_SWITCH_ANALYSIS_RESULT, + DEFAULT_ASSIGNMENT_SWITCH_ANALYSIS_RESULT, + DEFAULT_SIMPLIFY_ANALYSIS_RESULT, + /* canRemoveDefault= */ false); + + private final boolean enableAssignmentSwitch; + private final boolean enableReturnSwitch; + private final boolean enableSimplifySwitch; + + @Inject + RefactorSwitch(ErrorProneFlags flags) { + enableAssignmentSwitch = + flags.getBoolean("RefactorSwitch:EnableAssignmentSwitch").orElse(false); + enableReturnSwitch = flags.getBoolean("RefactorSwitch:EnableReturnSwitch").orElse(false); + enableSimplifySwitch = flags.getBoolean("RefactorSwitch:EnableSimplifySwitch").orElse(false); + } + + @Override + public Description matchSwitch(SwitchTree switchTree, VisitorState visitorState) { + // NOMUTANTS -- This is a performance optimization + if (!enableReturnSwitch && !enableAssignmentSwitch && !enableSimplifySwitch) { + return NO_MATCH; + } + + if (!SourceVersion.supportsPatternMatchingSwitch(visitorState.context)) { + return NO_MATCH; + } + + AnalysisResult analysisResult = analyzeSwitchTree(switchTree, visitorState); + + List suggestedFixes = new ArrayList<>(); + if (enableReturnSwitch + && analysisResult.returnSwitchAnalysisResult().canConvertToReturnSwitch()) { + suggestedFixes.add( + convertToReturnSwitch( + switchTree, visitorState, analysisResult, /* removeDefault= */ false)); + + if (analysisResult.canRemoveDefault()) { + suggestedFixes.add( + convertToReturnSwitch( + switchTree, visitorState, analysisResult, /* removeDefault= */ true)); + } + } + + if (enableAssignmentSwitch + && analysisResult.assignmentSwitchAnalysisResult().canConvertToAssignmentSwitch()) { + suggestedFixes.add( + convertToAssignmentSwitch( + switchTree, visitorState, analysisResult, /* removeDefault= */ false)); + + if (analysisResult.canRemoveDefault()) { + suggestedFixes.add( + convertToAssignmentSwitch( + switchTree, visitorState, analysisResult, /* removeDefault= */ true)); + } + } + + if (enableSimplifySwitch && analysisResult.simplifyCaseAnalysisResult().canSimplify) { + suggestedFixes.add( + convertToSimplifiedSwitch(switchTree, visitorState, /* removeDefault= */ false)); + + if (analysisResult.canRemoveDefault()) { + suggestedFixes.add( + convertToSimplifiedSwitch(switchTree, visitorState, /* removeDefault= */ true)); + } + } + + return suggestedFixes.isEmpty() + ? NO_MATCH + : buildDescription(switchTree).addAllFixes(suggestedFixes).build(); + } + + @Override + public Description matchSwitchExpression( + SwitchExpressionTree switchExpressionTree, VisitorState visitorState) { + if (!enableSimplifySwitch) { + return NO_MATCH; + } + + AnalysisResult analysisResult = analyzeSwitchExpressionTree(switchExpressionTree, visitorState); + + List suggestedFixes = new ArrayList<>(); + if (analysisResult.simplifyCaseAnalysisResult().canSimplify) { + suggestedFixes.add( + convertToSimplifiedSwitchExpression( + switchExpressionTree, visitorState, /* removeDefault= */ false)); + + if (analysisResult.canRemoveDefault()) { + suggestedFixes.add( + convertToSimplifiedSwitchExpression( + switchExpressionTree, visitorState, /* removeDefault= */ true)); + } + } + + return suggestedFixes.isEmpty() + ? NO_MATCH + : buildDescription(switchExpressionTree).addAllFixes(suggestedFixes).build(); + } + + /** + * Analyzes a {@code SwitchTree}, and determines any possible findings and suggested fixes related + * to expression switches that can be made. Does not report any findings or suggested fixes up to + * the Error Prone framework. + */ + private static AnalysisResult analyzeSwitchTree(SwitchTree switchTree, VisitorState state) { + + // Don't convert switch within switch because findings may overlap + if (ASTHelpers.findEnclosingNode(state.getPath(), SwitchTree.class) != null) { + return DEFAULT_ANALYSIS_RESULT; + } + + if (ASTHelpers.findEnclosingNode(state.getPath(), SwitchExpressionTree.class) != null) { + return DEFAULT_ANALYSIS_RESULT; + } + + List cases = switchTree.getCases(); + + // Does each case consist solely of returning a (non-void) expression? + CaseQualifications returnSwitchCaseQualifications = CaseQualifications.NO_CASES_ASSESSED; + // Does each case consist solely of a throw or the same symbol assigned in the same way? + Optional assignmentTargetOptional = Optional.empty(); + Optional assignmentKindOptional = Optional.empty(); + + AssignmentSwitchAnalysisState assignmentSwitchAnalysisState = + new AssignmentSwitchAnalysisState( + CaseQualifications.NO_CASES_ASSESSED, + assignmentTargetOptional, + assignmentKindOptional, + Optional.empty()); + + boolean canSimplify = false; + + // Set of all enum values (names) explicitly listed in a case tree + Set handledEnumValues = new HashSet<>(); + + // One-pass scan through each case in switch + for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) { + CaseTree caseTree = cases.get(caseIndex); + ImmutableList statements = getStatements(caseTree); + + // All cases must be of kind CaseTree.CaseKind.RULE + if (caseTree.getCaseKind() != CaseTree.CaseKind.RULE) { + return DEFAULT_ANALYSIS_RESULT; + } + + // Accumulate enum values included in this case + handledEnumValues.addAll( + caseTree.getExpressions().stream() + .map(ASTHelpers::getSymbol) + .filter(x -> x != null) + .map(symbol -> symbol.getSimpleName().toString()) + .collect(toImmutableSet())); + + returnSwitchCaseQualifications = + analyzeCaseForReturnSwitch(returnSwitchCaseQualifications, statements); + assignmentSwitchAnalysisState = + analyzeCaseForAssignmentSwitch(assignmentSwitchAnalysisState, statements); + canSimplify = canSimplify || analyzeCaseForSimplify(statements, caseTree); + } + + ImmutableList> caseRhsSourceCodeRanges = + cases.stream() + .map( + caseTree -> + Range.closedOpen( + getStartPosition(caseTree.getBody()), + state.getEndPosition(caseTree.getBody()))) + .collect(toImmutableList()); + // Has at least once case with a pattern + boolean hasPattern = + cases.stream() + .flatMap(caseTree -> caseTree.getLabels().stream()) + .anyMatch(caseLabelTree -> caseLabelTree instanceof PatternCaseLabelTree); + boolean hasDefault = cases.stream().anyMatch(ASTHelpers::isSwitchDefault); + boolean coversAllEnumValues = + isSwitchCoveringAllEnumValues( + handledEnumValues, ASTHelpers.getType(switchTree.getExpression())); + boolean anyCaseCanCompleteNormally = + cases.stream() + .anyMatch( + caseTree -> Reachability.canCompleteNormally((StatementTree) caseTree.getBody())); + + boolean canRemoveDefault = hasDefault && coversAllEnumValues; + + // `continue` is possible from switch statements, but not from switch expressions. A more + // sophisticated analysis would check whether the `continue` is exiting from the `switch` + // (disallowed) or merely internal to some subtree of the `switch` (possibly ok) + boolean hasContinue = hasContinueInTree(switchTree); + + boolean canConvertToAssignmentSwitch = + // The switch must be known to be exhaustive at compile time + (hasDefault || hasPattern || coversAllEnumValues) + && assignmentSwitchAnalysisState + .assignmentSwitchCaseQualifications() + .equals(CaseQualifications.ALL_CASES_QUALIFY) + && !hasContinue; + + // If the switch cannot complete normally, this is sufficient to ensure every case cannot. + // Alternatively, if the switch *can* complete normally and covers all enum values, with each + // case unable to complete normally, then we will also propose conversion to a return switch + // (safe unless the runtime enum values were to differ) + boolean canConvertToReturnSwitch = + (!Reachability.canCompleteNormally(switchTree) + || (!anyCaseCanCompleteNormally && coversAllEnumValues)) + && returnSwitchCaseQualifications.equals(CaseQualifications.ALL_CASES_QUALIFY) + && !hasContinue; + + ImmutableList precedingStatements = getPrecedingStatementsInBlock(state); + Optional assignmentTarget = + assignmentSwitchAnalysisState.assignmentTargetOptional(); + + // If present, the variable tree that can be combined with the switch block + Optional combinableVariableTree = + canConvertToAssignmentSwitch + ? assignmentTarget.flatMap( + target -> findCombinableVariableTree(target, precedingStatements, state)) + : Optional.empty(); + + return new AnalysisResult( + new ReturnSwitchAnalysisResult(canConvertToReturnSwitch, caseRhsSourceCodeRanges), + new AssignmentSwitchAnalysisResult( + canConvertToAssignmentSwitch, + combinableVariableTree, + assignmentSwitchAnalysisState.assignmentTargetOptional(), + assignmentSwitchAnalysisState.assignmentExpressionKindOptional(), + assignmentSwitchAnalysisState + .assignmentTreeOptional() + .map(SwitchUtils::renderJavaSourceOfAssignment)), + new SimplifyAnalysisResult(canSimplify, caseRhsSourceCodeRanges), + canRemoveDefault); + } + + /** + * Analyzes a {@code SwitchExpressionTree}, and determines any possible findings and suggested + * fixes related to expression switches that can be made. Does not report any findings or + * suggested fixes up to the Error Prone framework. + */ + private static AnalysisResult analyzeSwitchExpressionTree( + SwitchExpressionTree switchTree, VisitorState state) { + // Nothing to be done for an empty switch + if (switchTree.getCases().isEmpty()) { + return DEFAULT_ANALYSIS_RESULT; + } + + // Don't convert switch within switch because findings may overlap + if (ASTHelpers.findEnclosingNode(state.getPath(), SwitchTree.class) != null) { + return DEFAULT_ANALYSIS_RESULT; + } + + if (ASTHelpers.findEnclosingNode(state.getPath(), SwitchExpressionTree.class) != null) { + return DEFAULT_ANALYSIS_RESULT; + } + List cases = switchTree.getCases(); + + boolean canSimplify = false; + + // Set of all enum values (names) explicitly listed in a case tree + Set handledEnumValues = new HashSet<>(); + + // One-pass scan through each case in switch + for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) { + CaseTree caseTree = cases.get(caseIndex); + + // All cases must be of kind CaseTree.CaseKind.RULE + if (caseTree.getCaseKind() != CaseTree.CaseKind.RULE) { + return DEFAULT_ANALYSIS_RESULT; + } + ImmutableList statements = getStatements(caseTree); + + // Accumulate enum values included in this case + handledEnumValues.addAll( + caseTree.getExpressions().stream() + .map(ASTHelpers::getSymbol) + .filter(x -> x != null) + .map(symbol -> symbol.getSimpleName().toString()) + .collect(toImmutableSet())); + + canSimplify = canSimplify || analyzeCaseForSimplify(statements, caseTree); + } + + ImmutableList> caseRhsSourceCodeRanges = + cases.stream() + .map( + caseTree -> + Range.closedOpen( + getStartPosition(caseTree.getBody()), + state.getEndPosition(caseTree.getBody()))) + .collect(toImmutableList()); + + boolean hasDefault = cases.stream().anyMatch(ASTHelpers::isSwitchDefault); + boolean coversAllEnumValues = + isSwitchCoveringAllEnumValues( + handledEnumValues, ASTHelpers.getType(switchTree.getExpression())); + boolean canRemoveDefault = hasDefault && coversAllEnumValues; + + return new AnalysisResult( + DEFAULT_RETURN_SWITCH_ANALYSIS_RESULT, + DEFAULT_ASSIGNMENT_SWITCH_ANALYSIS_RESULT, + new SimplifyAnalysisResult(canSimplify, caseRhsSourceCodeRanges), + canRemoveDefault); + } + + /** Analyzes a single case for simplifications. */ + private static boolean analyzeCaseForSimplify( + ImmutableList statements, CaseTree caseTree) { + // Remove redundant braces + if (caseTree.getBody() instanceof BlockTree blockTree + && blockTree.getStatements().size() == 1 + && blockTree.getStatements().get(0) instanceof BlockTree) { + return true; + } + + // Convert `-> {yield x;}` to `-> x` + if (statements.size() == 1 && statements.get(0) instanceof YieldTree) { + return true; + } + + return false; + } + + /** + * Returns a map from return trees to the scope that they are returning from. Special case: when + * returning from the scope of the switch statement itself, the {@code switchTree} itself is used + * as a sentinel value (no attempt is made to identify the specific enclosing scope). + */ + private static Map analyzeReturnControlFlow( + SwitchTree switchTree, VisitorState state) { + ArrayDeque returnScope = new ArrayDeque<>(); + returnScope.push(switchTree); + HashMap result = new HashMap<>(); + // One can only return from a method or a lambda expression + new TreePathScanner() { + @Override + public Void visitMethod(MethodTree methodTree, Void unused) { + returnScope.push(methodTree); + super.visitMethod(methodTree, null); + returnScope.pop(); + return null; + } + + @Override + public Void visitLambdaExpression(LambdaExpressionTree lambdaExpressionTree, Void unused) { + returnScope.push(lambdaExpressionTree); + super.visitLambdaExpression(lambdaExpressionTree, null); + returnScope.pop(); + return null; + } + + @Override + public Void visitReturn(ReturnTree node, Void unused) { + result.put(node, returnScope.peek()); + return null; + } + }.scan(state.getPath(), null); + return result; + } + + /** + * Analyze a single {@code case} of a single {@code switch} statement to determine whether it is + * convertible to a return switch. The supplied {@code previousCaseQualifications} is updated and + * returned based on this analysis. + */ + private static CaseQualifications analyzeCaseForReturnSwitch( + CaseQualifications previousCaseQualifications, ImmutableList statements) { + // An empty RHS can't yield a value + if (statements.isEmpty()) { + return CaseQualifications.SOME_OR_ALL_CASES_DONT_QUALIFY; + } + StatementTree lastStatement = getLast(statements); + if (!KINDS_RETURN_OR_THROW.contains(lastStatement.getKind())) { + return CaseQualifications.SOME_OR_ALL_CASES_DONT_QUALIFY; + } + + // For this analysis, cases that don't return something can be disregarded + if (!(lastStatement instanceof ReturnTree returnTree)) { + return previousCaseQualifications; + } + + // NOMUTANTS -- This is a performance optimization, not a correctness check + if (!previousCaseQualifications.equals(CaseQualifications.NO_CASES_ASSESSED)) { + // There is no need to inspect the type compatibility of the return values, because if they + // were incompatible then the compilation would have failed before reaching this point + return previousCaseQualifications; + } + + // This is the first value-returning case that we are examining + Type returnType = ASTHelpers.getType(returnTree.getExpression()); + return returnType == null + // Return of void does not qualify + ? CaseQualifications.SOME_OR_ALL_CASES_DONT_QUALIFY + : CaseQualifications.ALL_CASES_QUALIFY; + } + + /** + * Analyze a single {@code case} of a single {@code switch} statement to determine whether it is + * convertible to an assignment switch. The supplied {@code previousAssignmentSwitchAnalysisState} + * is updated and returned based on this analysis. + */ + private static AssignmentSwitchAnalysisState analyzeCaseForAssignmentSwitch( + AssignmentSwitchAnalysisState previousAssignmentSwitchAnalysisState, + List statements) { + + CaseQualifications caseQualifications = + previousAssignmentSwitchAnalysisState.assignmentSwitchCaseQualifications(); + Optional assignmentExpressionKindOptional = + previousAssignmentSwitchAnalysisState.assignmentExpressionKindOptional(); + Optional assignmentTargetOptional = + previousAssignmentSwitchAnalysisState.assignmentTargetOptional(); + Optional assignmentTreeOptional = + previousAssignmentSwitchAnalysisState.assignmentTreeOptional(); + + // An empty RHS can't assign anything; we don't support >1 statement in the RHS + if (statements.isEmpty() || statements.size() > 1) { + return new AssignmentSwitchAnalysisState( + CaseQualifications.SOME_OR_ALL_CASES_DONT_QUALIFY, + assignmentTargetOptional, + assignmentExpressionKindOptional, + assignmentTreeOptional); + } + + // Invariant: exactly one statement on RHS + StatementTree firstStatement = statements.get(0); + // Must be an assignment or a throw + if (firstStatement instanceof ThrowTree) { + return previousAssignmentSwitchAnalysisState; + } + if (!(firstStatement instanceof ExpressionStatementTree expressionStatementTree)) { + // Definitely not an assignment + return new AssignmentSwitchAnalysisState( + CaseQualifications.SOME_OR_ALL_CASES_DONT_QUALIFY, + assignmentTargetOptional, + assignmentExpressionKindOptional, + assignmentTreeOptional); + } + + ExpressionTree expression = expressionStatementTree.getExpression(); + Optional caseAssignmentTargetOptional = Optional.empty(); + Optional caseAssignmentKindOptional = Optional.empty(); + Optional caseAssignmentTreeOptional = Optional.empty(); + + // The assignment could be a normal assignment ("=") or a compound assignment (e.g. "+=") + switch (expression) { + case CompoundAssignmentTree compoundAssignmentTree -> { + caseAssignmentTargetOptional = Optional.of(compoundAssignmentTree.getVariable()); + caseAssignmentKindOptional = Optional.of(compoundAssignmentTree.getKind()); + caseAssignmentTreeOptional = Optional.of(expression); + } + case AssignmentTree assignmentTree -> { + caseAssignmentTargetOptional = Optional.of(assignmentTree.getVariable()); + caseAssignmentKindOptional = Optional.of(Tree.Kind.ASSIGNMENT); + caseAssignmentTreeOptional = Optional.of(expression); + } + default -> { + // Something else that is not an assignment + return new AssignmentSwitchAnalysisState( + CaseQualifications.SOME_OR_ALL_CASES_DONT_QUALIFY, + assignmentTargetOptional, + assignmentExpressionKindOptional, + assignmentTreeOptional); + } + } + + boolean compatibleOperator = + // First assignment seen? + (assignmentExpressionKindOptional.isEmpty() && caseAssignmentKindOptional.isPresent()) + // Not first assignment, but compatible with the first? + || (assignmentExpressionKindOptional.isPresent() + && caseAssignmentKindOptional.isPresent() + && assignmentExpressionKindOptional.get().equals(caseAssignmentKindOptional.get())); + boolean compatibleReference = + // First assignment seen? + (assignmentTargetOptional.isEmpty() && caseAssignmentTargetOptional.isPresent()) + // Not first assignment, but assigning to same symbol as the first assignment? + || isCompatibleWithFirstAssignment( + assignmentTargetOptional, caseAssignmentTargetOptional); + + if (compatibleOperator && compatibleReference) { + caseQualifications = + caseQualifications.equals(CaseQualifications.NO_CASES_ASSESSED) + ? CaseQualifications.ALL_CASES_QUALIFY + : caseQualifications; + } else { + caseQualifications = CaseQualifications.SOME_OR_ALL_CASES_DONT_QUALIFY; + } + + // Save the assignment target/kind in the state, but never overwrite existing target/kind + return new AssignmentSwitchAnalysisState( + caseQualifications, + assignmentTargetOptional.isEmpty() + ? caseAssignmentTargetOptional + : assignmentTargetOptional, + assignmentExpressionKindOptional.isEmpty() + ? caseAssignmentKindOptional + : assignmentExpressionKindOptional, + assignmentTreeOptional.isEmpty() ? caseAssignmentTreeOptional : assignmentTreeOptional); + } + + /** + * Converts a switch that {@code return}s on each case, and is exhaustive, into a {@code return + * switch}. + */ + private static SuggestedFix convertToReturnSwitch( + SwitchTree switchTree, + VisitorState state, + AnalysisResult analysisResult, + boolean removeDefault) { + + Map returnToScope = analyzeReturnControlFlow(switchTree, state); + + SuggestedFix.Builder suggestedFixBuilder = SuggestedFix.builder(); + List cases = switchTree.getCases(); + + suggestedFixBuilder.prefixWith(switchTree, "return "); + // We need to add a semicolon as in `return switch ... ;` + suggestedFixBuilder.postfixWith(switchTree, ";"); + + for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) { + CaseTree caseTree = cases.get(caseIndex); + + NullDefaultKind nullDefaultKind = analyzeCaseForNullAndDefault(caseTree); + if (removeDefault && nullDefaultKind.equals(NullDefaultKind.KIND_DEFAULT)) { + // Delete removed default (and its code) entirely + suggestedFixBuilder.delete(caseTree); + continue; + } + + transformCaseForReturnSwitch( + switchTree, + caseTree, + state, + returnToScope, + analysisResult.returnSwitchAnalysisResult().caseRhsSourceCodeRanges().get(caseIndex), + nullDefaultKind, + removeDefault, + suggestedFixBuilder); + } + + // The transformed code can cause other existing code to become dead code. So, we must analyze + // and delete such dead code, otherwise the suggested autofix could fail to compile. + + // The `return switch ...` will always return or throw + Tree cannotCompleteNormallyTree = switchTree; + // Search up the AST for enclosing statement blocks, marking any newly-dead code for deletion + // along the way + Tree prev = state.getPath().getLeaf(); + for (Tree tree : state.getPath().getParentPath()) { + if (tree instanceof BlockTree blockTree) { + var statements = blockTree.getStatements(); + int indexInBlock = statements.indexOf(prev); + // A single mock of the immediate child statement block (or switch) is sufficient to + // analyze reachability here; deeper-nested statements are not relevant. + boolean nextStatementReachable = + Reachability.canCompleteNormally( + statements.get(indexInBlock), ImmutableMap.of(cannotCompleteNormallyTree, false)); + // If we continue to the ancestor statement block, it will be because the end of this + // statement block is not reachable + cannotCompleteNormallyTree = blockTree; + if (nextStatementReachable) { + break; + } + + // If a next statement in this block exists, then it is not reachable. + if (indexInBlock < statements.size() - 1) { + String deletedRegion = + state + .getSourceCode() + .subSequence( + state.getEndPosition(statements.get(indexInBlock)), + state.getEndPosition(blockTree)) + .toString(); + // If the region we would delete looks interesting, bail out and just delete the orphaned + // statements. + if (deletedRegion.contains("LINT.")) { + statements + .subList(indexInBlock + 1, statements.size()) + .forEach(suggestedFixBuilder::delete); + } else { + // If the region doesn't seem to contain interesting comments, delete it along with + // comments: those comments are often just of the form "Unreachable code". + suggestedFixBuilder.replace( + state.getEndPosition(statements.get(indexInBlock)), + state.getEndPosition(blockTree), + "}"); + } + } + } + prev = tree; + } + + if (removeDefault) { + suggestedFixBuilder.setShortDescription(REMOVE_DEFAULT_CASE_SHORT_DESCRIPTION); + } + return suggestedFixBuilder.build(); + } + + /** + * Converts a single `yield` statement to a single expression within the {@code + * suggestedFixBuilder}, for example `case null -> {yield 0;}` would become `case null -> 0;`. + * Precondition: it must have been validated that this transformation is syntactically valid + * before invoking this method. + */ + private static void simplifySingleYieldStatement( + YieldTree yieldStatement, + VisitorState state, + CaseTree caseTree, + SuggestedFix.Builder suggestedFixBuilder, + ImmutableList allComments, + boolean renderWholeCase, + NullDefaultKind nullDefaultKind, + boolean removeDefault) { + Tree value = yieldStatement.getValue(); + StringBuilder suffixBuilder = new StringBuilder(); + suffixBuilder.append(";"); + StringBuilder replacementBuilder = new StringBuilder(); + // Extract any comments that would be orphaned by the simplification + StringBuilder commentBuilder = new StringBuilder(); + Range caseRhsSourceCodeRange = + Range.closedOpen(getStartPosition(value), state.getEndPosition(value)); + ImmutableList orphanedComments = + computeOrphanedComments( + allComments, + renderWholeCase + ? caseRhsSourceCodeRange + : Range.closedOpen(getStartPosition(value), state.getEndPosition(value)), + caseRhsSourceCodeRange); + String renderedOrphans = renderComments(orphanedComments); + if (!renderedOrphans.isEmpty()) { + commentBuilder.append("\n").append(renderedOrphans).append("\n"); + } + + Tree replacementTarget; + if (renderWholeCase) { + // Render both LHS and RHS + replacementBuilder.append(renderNullDefaultKindPrefix(nullDefaultKind, removeDefault)); + if (nullDefaultKind.equals(NullDefaultKind.KIND_NEITHER)) { + replacementBuilder.append(printCaseExpressionsOrPatternAndGuard(caseTree, state)); + } + replacementBuilder.append(" -> "); + replacementTarget = caseTree; + } else { + // Render only the RHS + replacementTarget = caseTree.getBody(); + } + suggestedFixBuilder.replace( + replacementTarget, + replacementBuilder.toString() + + commentBuilder + + state.getSourceForNode(value) + + suffixBuilder); + } + + /** + * Transforms the RHS block of a case for a return switch. This involves replacing relevant + * `return`s with `yield`s, removing any redundant braces, and adjusting comments if needed. + */ + private static void transformCaseForReturnSwitch( + SwitchTree switchTree, + CaseTree caseTree, + VisitorState state, + Map returnToScope, + Range caseRhsSourceCodeRange, + NullDefaultKind nullDefaultKind, + boolean removeDefault, + SuggestedFix.Builder suggestedFixBuilder) { + ImmutableList statements = getStatements(caseTree); + + Optional singleStatement = Optional.empty(); + if (statements.size() == 1) { + StatementTree at = statements.get(0); + singleStatement = Optional.of(at); + } + if (singleStatement.isPresent()) { + ImmutableList allComments = + state.getTokensForNode(caseTree.getBody()).stream() + .flatMap(errorProneToken -> errorProneToken.comments().stream()) + .collect(toImmutableList()); + // Can the RHS be made into an expression? e.g. `case null -> {return 0;}` becomes + // `case null -> 0;` + if (singleStatement.get() instanceof ReturnTree rt) { + Range printedRange = + Range.closedOpen( + getStartPosition(rt.getExpression()), state.getEndPosition(rt.getExpression())); + + StringBuilder replacementBuilder = new StringBuilder(); + String renderedOrphans = + renderComments( + computeOrphanedComments(allComments, caseRhsSourceCodeRange, printedRange)); + if (!renderedOrphans.isEmpty()) { + replacementBuilder.append("\n").append(renderedOrphans).append("\n"); + } + replacementBuilder.append(state.getSourceForNode(rt.getExpression())).append(";"); + if (removeDefault && nullDefaultKind.equals(NullDefaultKind.KIND_NULL_AND_DEFAULT)) { + // LHS must be rendered to remove the "default" + replacementBuilder.insert( + 0, renderNullDefaultKindPrefix(nullDefaultKind, removeDefault) + " -> "); + suggestedFixBuilder.replace(caseTree, replacementBuilder.toString()); + } else { + suggestedFixBuilder.replace(caseTree.getBody(), replacementBuilder.toString()); + } + } else if (singleStatement.get() instanceof ThrowTree tt) { + // RHS is just a single throw + Range printedRange = + Range.closedOpen(getStartPosition(tt), state.getEndPosition(tt)); + + StringBuilder replacementBuilder = new StringBuilder(); + String renderedOrphans = + renderComments( + computeOrphanedComments(allComments, caseRhsSourceCodeRange, printedRange)); + if (!renderedOrphans.isEmpty()) { + replacementBuilder.append("\n").append(renderedOrphans).append("\n"); + } + replacementBuilder.append(state.getSourceForNode(tt)); + if (removeDefault && nullDefaultKind.equals(NullDefaultKind.KIND_NULL_AND_DEFAULT)) { + // LHS must be rendered to remove the "default" + replacementBuilder.insert( + 0, renderNullDefaultKindPrefix(nullDefaultKind, removeDefault) + " -> "); + suggestedFixBuilder.replace(caseTree, replacementBuilder.toString()); + } else { + suggestedFixBuilder.replace(caseTree.getBody(), replacementBuilder.toString()); + } + } + return; + } + + // Invariant: singleStatement is empty; zero or multiple statements on RHS + // Can redundant braces be removed from the RHS? + Tree at = caseTree.getBody(); + while (at instanceof BlockTree bt + && bt.getStatements().size() == 1 + && bt.getStatements().get(0) instanceof BlockTree) { + // Strip out this brace and descend into the inner block (which must exist) + suggestedFixBuilder.replace( + ASTHelpers.getStartPosition(bt), ASTHelpers.getStartPosition(bt) + 1, ""); + suggestedFixBuilder.replace(state.getEndPosition(bt) - 1, state.getEndPosition(bt), ""); + at = bt.getStatements().get(0); + } + + // Transform relevant `return`s to `yield`s + new TreePathScanner() { + @Override + public Void visitReturn(ReturnTree returnTree, Void unused) { + if (returnToScope.get(returnTree) == switchTree) { + StringBuilder yieldStatementBuilder = new StringBuilder(); + yieldStatementBuilder + .append("yield ") + .append(state.getSourceForNode(returnTree.getExpression())) + .append(";"); + suggestedFixBuilder.replace(returnTree, yieldStatementBuilder.toString()); + } + return super.visitReturn(returnTree, null); + } + }.scan(TreePath.getPath(state.getPath(), caseTree), null); + // LHS must be re-rendered to remove the "default" + if (removeDefault && nullDefaultKind.equals(NullDefaultKind.KIND_NULL_AND_DEFAULT)) { + suggestedFixBuilder.replace( + getStartPosition(caseTree), + getStartPosition(caseTree.getBody()), + renderNullDefaultKindPrefix(nullDefaultKind, removeDefault) + " -> "); + } + } + + /** + * Transforms the supplied switch into an assignment switch (e.g. {@code x = switch ... ;}). + * Comments are preserved where possible. Precondition: the {@code AnalysisResult} must have + * deduced that this conversion is possible. + */ + private static SuggestedFix convertToAssignmentSwitch( + SwitchTree switchTree, + VisitorState state, + AnalysisResult analysisResult, + boolean removeDefault) { + SuggestedFix.Builder suggestedFixBuilder = SuggestedFix.builder(); + StringBuilder lhsBuilder = new StringBuilder(); + + analysisResult + .assignmentSwitchAnalysisResult() + .precedingVariableDeclaration() + .ifPresent( + variableTree -> { + suggestedFixBuilder.delete(variableTree); + + lhsBuilder.append( + Streams.concat( + renderVariableTreeComments(variableTree, state).stream(), + renderVariableTreeAnnotations(variableTree, state).stream(), + Stream.of(renderVariableTreeFlags(variableTree))) + .collect(joining("\n"))); + + // Local variables declared with "var" must unfortunately be handled as a special case + // because getSourceForNode() returns null for the source code of a "var" declaration. + String sourceForType = + hasImplicitType(variableTree, state) + ? "var" + : state.getSourceForNode(variableTree.getType()); + + lhsBuilder.append(sourceForType).append(" "); + }); + + lhsBuilder + .append( + state.getSourceForNode( + analysisResult.assignmentSwitchAnalysisResult().assignmentTargetOptional().get())) + .append(" ") + // Invariant: always present when a finding exists + .append( + analysisResult.assignmentSwitchAnalysisResult().assignmentSourceCodeOptional().get()) + .append(" "); + suggestedFixBuilder.prefixWith(switchTree, lhsBuilder.toString()); + + List cases = switchTree.getCases(); + for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) { + CaseTree caseTree = cases.get(caseIndex); + + NullDefaultKind nullDefaultKind = analyzeCaseForNullAndDefault(caseTree); + if (removeDefault && nullDefaultKind.equals(NullDefaultKind.KIND_DEFAULT)) { + // Delete removed default (and its code) entirely + suggestedFixBuilder.replace(caseTree, ""); + continue; + } + + // Invariant: exactly one statement on RHS + ImmutableList statements = getStatements(caseTree); + StatementTree statement = statements.get(0); + ImmutableList allComments = + state.getTokensForNode(caseTree.getBody()).stream() + .flatMap(errorProneToken -> errorProneToken.comments().stream()) + .collect(toImmutableList()); + Range caseRhsSourceCodeRange = + Range.closedOpen(getStartPosition(statement), state.getEndPosition(statement)); + + Optional treeOptional = Optional.empty(); + boolean addSemicolonSuffix = false; + // Always throw or valid assignment/compound assignment because checked in analysis, thus + // treeOptional is always re-assigned + if (statement instanceof ThrowTree throwTree) { + treeOptional = Optional.of(throwTree); + } else if (statement instanceof ExpressionStatementTree expressionStatementTree) { + if (expressionStatementTree.getExpression() + instanceof CompoundAssignmentTree compoundAssignmentTree) { + treeOptional = Optional.of(compoundAssignmentTree.getExpression()); + addSemicolonSuffix = true; + } else if (expressionStatementTree.getExpression() + instanceof AssignmentTree assignmentTree) { + treeOptional = Optional.of(assignmentTree.getExpression()); + addSemicolonSuffix = true; + } + } + StringBuilder replacementBuilder = new StringBuilder(); + + // Invariant: treeOptional always present + treeOptional.ifPresent( + tree -> { + ImmutableList orphanedComments = + computeOrphanedComments( + allComments, + Range.closedOpen(getStartPosition(tree), state.getEndPosition(tree)), + caseRhsSourceCodeRange); + String renderedOrphans = renderComments(orphanedComments); + if (!renderedOrphans.isEmpty()) { + replacementBuilder.append("\n").append(renderedOrphans).append("\n"); + } + replacementBuilder.append(state.getSourceForNode(tree)); + }); + + if (addSemicolonSuffix) { + replacementBuilder.append(";"); + } + + if (removeDefault && nullDefaultKind.equals(NullDefaultKind.KIND_NULL_AND_DEFAULT)) { + // RHS & LHS must be re-rendered to remove the "default" + replacementBuilder.insert( + 0, renderNullDefaultKindPrefix(nullDefaultKind, removeDefault) + " -> "); + suggestedFixBuilder.replace(caseTree, replacementBuilder.toString()); + } else { + // Just re-render the RHS + suggestedFixBuilder.replace(caseTree.getBody(), replacementBuilder.toString()); + } + } // case loop + + // Add a semicolon after the switch, since it's now a switch expression + suggestedFixBuilder.postfixWith(switchTree, ";"); + + if (removeDefault) { + suggestedFixBuilder.setShortDescription(REMOVE_DEFAULT_CASE_SHORT_DESCRIPTION); + } + return suggestedFixBuilder.build(); + } + + /** Converts a {@code SwitchExpressionTree} to a simplified form, and suggests a fix. */ + private static SuggestedFix convertToSimplifiedSwitchExpression( + SwitchExpressionTree switchTree, VisitorState state, boolean removeDefault) { + + return convertToSimplifiedCommon(switchTree.getCases(), state, removeDefault); + } + + /** Converts a {@code SwitchTree} to a simplified form, and suggests a fix. */ + private static SuggestedFix convertToSimplifiedSwitch( + SwitchTree switchTree, VisitorState state, boolean removeDefault) { + + return convertToSimplifiedCommon(switchTree.getCases(), state, removeDefault); + } + + /** + * Converts a switch statement to a simplified form. Where possible, a single suggested fix will + * contain multiple simplifications. + */ + private static SuggestedFix convertToSimplifiedCommon( + List cases, VisitorState state, boolean removeDefault) { + + SuggestedFix.Builder suggestedFixBuilder = SuggestedFix.builder(); + for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) { + CaseTree caseTree = cases.get(caseIndex); + + NullDefaultKind nullDefaultKind = analyzeCaseForNullAndDefault(caseTree); + if (removeDefault && nullDefaultKind.equals(NullDefaultKind.KIND_DEFAULT)) { + // Delete removed default (and its code) entirely + suggestedFixBuilder.delete(caseTree); + continue; + } + // The entire case must be re-rendered to remove a "default" + boolean renderWholeCase = + removeDefault && nullDefaultKind.equals(NullDefaultKind.KIND_NULL_AND_DEFAULT); + + ImmutableList statements = getStatements(caseTree); + Optional singleYieldStatement = Optional.empty(); + if (statements.size() == 1 && statements.get(0) instanceof YieldTree yieldTree) { + singleYieldStatement = Optional.of(yieldTree); + } + + ImmutableList allComments = + state.getTokensForNode(caseTree.getBody()).stream() + .flatMap(errorProneToken -> errorProneToken.comments().stream()) + .collect(toImmutableList()); + + if (singleYieldStatement.isPresent()) { + simplifySingleYieldStatement( + singleYieldStatement.get(), + state, + caseTree, + suggestedFixBuilder, + allComments, + renderWholeCase, + nullDefaultKind, + removeDefault); + } else if (renderWholeCase) { + StringBuilder commentBuilder = new StringBuilder(); + Range caseRhsSourceCodeRange = + Range.closedOpen( + getStartPosition(caseTree.getBody()), state.getEndPosition(caseTree.getBody())); + ImmutableList orphanedComments = + computeOrphanedComments(allComments, caseRhsSourceCodeRange, caseRhsSourceCodeRange); + String renderedOrphans = renderComments(orphanedComments); + if (!renderedOrphans.isEmpty()) { + commentBuilder.append("\n").append(renderedOrphans).append("\n"); + } + + // Look for brace simplification on the RHS + Tree at = caseTree.getBody(); + while (at instanceof BlockTree bt + && bt.getStatements().size() == 1 + && bt.getStatements().get(0) instanceof BlockTree) { + // Strip out this brace and descend into the inner block (which must exist) + at = bt.getStatements().get(0); + } + + // Render both LHS and RHS + StringBuilder lhs = new StringBuilder(); + lhs.append(renderNullDefaultKindPrefix(nullDefaultKind, removeDefault)); + if (nullDefaultKind.equals(NullDefaultKind.KIND_NEITHER)) { + lhs.append(printCaseExpressionsOrPatternAndGuard(caseTree, state)); + } + suggestedFixBuilder.replace( + caseTree, lhs + " -> " + commentBuilder + state.getSourceForNode(at)); + } else { + // Not a single yield statement, and we don't need to re-render the whole case + // Can redundant braces be removed from the RHS? + Tree at = caseTree.getBody(); + while (at instanceof BlockTree bt + && bt.getStatements().size() == 1 + && bt.getStatements().get(0) instanceof BlockTree) { + // Strip out this brace and descend into the inner block (which must exist) + suggestedFixBuilder.replace( + ASTHelpers.getStartPosition(bt), ASTHelpers.getStartPosition(bt) + 1, ""); + suggestedFixBuilder.replace(state.getEndPosition(bt) - 1, state.getEndPosition(bt), ""); + at = bt.getStatements().get(0); + } + + // Braces are not required if there is exactly one statement on the right hand of the arrow, + // and it's either an ExpressionStatement or a Throw. Refer to JLS 14 §14.11.1 + if (at instanceof BlockTree blockTree && blockTree.getStatements().size() == 1) { + StatementTree statement = blockTree.getStatements().get(0); + if (statement instanceof ThrowTree || statement instanceof ExpressionStatementTree) { + suggestedFixBuilder.replace( + ASTHelpers.getStartPosition(blockTree), + ASTHelpers.getStartPosition(blockTree) + 1, + ""); + suggestedFixBuilder.replace( + state.getEndPosition(blockTree) - 1, state.getEndPosition(blockTree), ""); + } + } + } + } + + if (removeDefault) { + suggestedFixBuilder.setShortDescription(REMOVE_DEFAULT_CASE_SHORT_DESCRIPTION); + } + return suggestedFixBuilder.build(); + } + + /** Returns a range that encloses the given comment, offset by the given start position. */ + private static Range buildCommentRange(ErrorProneComment comment, int startPosition) { + return Range.closedOpen(comment.getPos() + startPosition, comment.getEndPos() + startPosition); + } + + /** + * Returns the comments that are within the case's RHS source code range but outside the printed + * range for the RHS. + */ + private static ImmutableList computeOrphanedComments( + ImmutableList allComments, + Range caseRhsSourceCodeRange, + Range printedRange) { + return allComments.stream() + // Within this case's source code + .filter( + comment -> + caseRhsSourceCodeRange.encloses( + buildCommentRange(comment, caseRhsSourceCodeRange.lowerEndpoint()))) + // But outside the printed range for the RHS + .filter( + comment -> + !buildCommentRange(comment, caseRhsSourceCodeRange.lowerEndpoint()) + .isConnected(printedRange)) + .collect(toImmutableList()); + } + + record AnalysisResult( + // Result of analyzing whether the switch can be converted to a return switch + ReturnSwitchAnalysisResult returnSwitchAnalysisResult, + // Result of analyzing whether the switch can be converted to an assignment switch + AssignmentSwitchAnalysisResult assignmentSwitchAnalysisResult, + // Result of analyzing whether the switch can be simplified + SimplifyAnalysisResult simplifyCaseAnalysisResult, + // Whether the default case can be removed + boolean canRemoveDefault) {} + + record AssignmentSwitchAnalysisState( + // Current qualification for conversion based on cases examined so far + CaseQualifications assignmentSwitchCaseQualifications, + // What is being assigned to (if any) + Optional assignmentTargetOptional, + // The kind of assignment being performed (if any) + Optional assignmentExpressionKindOptional, + // The tree of the assignment being performed (if any) + Optional assignmentTreeOptional) {} + + record AssignmentSwitchAnalysisResult( + // The switch can be converted to an assignment switch + boolean canConvertToAssignmentSwitch, + // The variable declaration that preceded the switch (if any) + Optional precedingVariableDeclaration, + // What is being assigned to (if any) + Optional assignmentTargetOptional, + // The kind of assignment being performed (if any) + Optional assignmentKindOptional, + // The range of source code covered by each case + Optional assignmentSourceCodeOptional) {} + + record ReturnSwitchAnalysisResult( + // The switch can be converted to a return switch + boolean canConvertToReturnSwitch, + // Range of source code covered by each case + ImmutableList> caseRhsSourceCodeRanges) {} + + record SimplifyAnalysisResult( + // At least one case can be simplified + boolean canSimplify, + // Range of source code covered by each case + ImmutableList> caseRhsSourceCodeRanges) {} +} diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/SwitchUtils.java b/core/src/main/java/com/google/errorprone/bugpatterns/SwitchUtils.java new file mode 100644 index 00000000000..b44989d8e16 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/SwitchUtils.java @@ -0,0 +1,428 @@ +/* + * Copyright 2026 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.bugpatterns; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.Iterables.getLast; +import static com.google.errorprone.util.ASTHelpers.getStartPosition; +import static com.google.errorprone.util.ASTHelpers.getSymbol; +import static com.google.errorprone.util.ASTHelpers.isSwitchDefault; +import static com.sun.source.tree.Tree.Kind.RETURN; +import static com.sun.source.tree.Tree.Kind.THROW; +import static java.util.stream.Collectors.joining; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.errorprone.VisitorState; +import com.google.errorprone.matchers.CompileTimeConstantExpressionMatcher; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.util.ASTHelpers; +import com.google.errorprone.util.ErrorProneComment; +import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.BlockTree; +import com.sun.source.tree.CaseTree; +import com.sun.source.tree.ContinueTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.PatternCaseLabelTree; +import com.sun.source.tree.StatementTree; +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.source.util.TreeScanner; +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.Pretty; +import java.io.StringWriter; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import javax.lang.model.element.ElementKind; +import org.jspecify.annotations.Nullable; + +/** Utility methods for refactoring switches. */ +public final class SwitchUtils { + static final Matcher COMPILE_TIME_CONSTANT_MATCHER = + CompileTimeConstantExpressionMatcher.instance(); + static final ImmutableSet KINDS_RETURN_OR_THROW = ImmutableSet.of(THROW, RETURN); + + static final String EQUALS_STRING = "="; + static final String REMOVE_DEFAULT_CASE_SHORT_DESCRIPTION = + "Remove default case because all enum values handled"; + + /** + * Tri-state of whether the if-chain is valid, invalid, or possibly valid for conversion to a + * switch. + */ + enum Validity { + MAYBE_VALID, + INVALID, + VALID + } + + /** + * Tri-state to represent whether cases within a single switch statement meet an (unspecified) + * qualification predicate + */ + enum CaseQualifications { + NO_CASES_ASSESSED, + ALL_CASES_QUALIFY, + SOME_OR_ALL_CASES_DONT_QUALIFY + } + + /** + * The kind of null/default cases included within a single CaseTree. + * + *

This enum is used to classify whether a CaseTree includes a null and/or default. Referencing + * JLS 21 §14.11.1, the `SwitchLabel:` production has specific rules applicable to null/default + * cases: `case null, [default]` and `default`. All other scenarios are lumped into KIND_NEITHER. + */ + enum NullDefaultKind { + KIND_NULL_AND_DEFAULT, + KIND_DEFAULT, + KIND_NULL, + KIND_NEITHER + } + + /** + * Renders the Java source code for a [compound] assignment operator. The parameter must be either + * an {@code AssignmentTree} or a {@code CompoundAssignmentTree}. + */ + static String renderJavaSourceOfAssignment(ExpressionTree tree) { + // Simple assignment tree? + if (tree instanceof JCAssign) { + return EQUALS_STRING; + } + + // Invariant: must be a compound assignment tree + JCAssignOp jcAssignOp = (JCAssignOp) tree; + Pretty pretty = new Pretty(new StringWriter(), /* sourceOutput= */ true); + return pretty.operatorName(jcAssignOp.getTag().noAssignOp()) + EQUALS_STRING; + } + + /** Render the supplied comments, separated by newlines. */ + static String renderComments(ImmutableList comments) { + return comments.stream() + .map(ErrorProneComment::getText) + .filter(commentText -> !commentText.isEmpty()) + .collect(joining("\n")); + } + + /** Retrieves a list of all statements (if any) preceding the current path, if any. */ + static ImmutableList getPrecedingStatementsInBlock(VisitorState state) { + TreePath path = state.getPath(); + if (!(path.getParentPath().getLeaf() instanceof BlockTree blockTree)) { + return ImmutableList.of(); + } + var statements = blockTree.getStatements(); + return ImmutableList.copyOf(statements.subList(0, statements.indexOf(path.getLeaf()))); + } + + static Optional findCombinableVariableTree( + ExpressionTree assignmentTarget, + ImmutableList precedingStatements, + VisitorState state) { + // Don't try to combine when multiple variables are declared together + if (precedingStatements.isEmpty() + || !precedingTwoStatementsNotInSameVariableDeclaratorList(precedingStatements)) { + return Optional.empty(); + } + if (!(getLast(precedingStatements) instanceof VariableTree variableTree)) { + return Optional.empty(); + } + if (variableTree.getInitializer() != null + && !(COMPILE_TIME_CONSTANT_MATCHER.matches(variableTree.getInitializer(), state) + // Safe to elide dead store of an enum value + || isEnumValue(variableTree.getInitializer(), state))) { + return Optional.empty(); + } + // If we are reading the initialized value in the switch block, we can't remove it + if (!noReadsOfVariable(ASTHelpers.getSymbol(variableTree), state)) { + return Optional.empty(); + } + // The variable and the switch's assignment must be compatible + if (!isVariableCompatibleWithAssignment(assignmentTarget, variableTree)) { + return Optional.empty(); + } + return Optional.of(variableTree); + } + + static boolean isEnumValue(ExpressionTree expression, VisitorState state) { + Type type = ASTHelpers.getType(expression); + if (type == null) { + return false; + } + return type.asElement().getKind() == ElementKind.ENUM; + } + + /** + * Determines whether local variable {@code symbol} has no reads within the scope of the {@code + * VisitorState}. (Writes to the variable are ignored.) + */ + static boolean noReadsOfVariable(VarSymbol symbol, VisitorState state) { + 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); + } + + void handle(Tree tree) { + var symbol = getSymbol(tree); + if (symbol instanceof VarSymbol varSymbol) { + referencedLocalVariables.add(varSymbol); + } + } + }.scan(state.getPath(), null); + + return !referencedLocalVariables.contains(symbol); + } + + /** + * 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}. + */ + static boolean precedingTwoStatementsNotInSameVariableDeclaratorList( + List precedingStatements) { + + if (precedingStatements.size() < 2) { + return true; + } + + StatementTree secondToLastStatement = precedingStatements.get(precedingStatements.size() - 2); + StatementTree lastStatement = Iterables.getLast(precedingStatements); + if (!(secondToLastStatement instanceof VariableTree variableTree1) + || !(lastStatement instanceof VariableTree variableTree2)) { + return true; + } + + // Start positions will vary if the variable declarations are in the same + // VariableDeclaratorList. + return getStartPosition(variableTree1) != getStartPosition(variableTree2); + } + + /** + * Determines whether a variable definition is compatible with an assignment target (e.g. of a + * switch statement). Compatibility means that the assignment is being made to to the same + * variable that is being defined. + */ + static boolean isVariableCompatibleWithAssignment( + ExpressionTree assignmentTarget, VariableTree variableDefinition) { + Symbol assignmentTargetSymbol = getSymbol(assignmentTarget); + Symbol definedSymbol = ASTHelpers.getSymbol(variableDefinition); + + return Objects.equals(assignmentTargetSymbol, definedSymbol); + } + + /** + * Returns the statements on the RHS of a {@link CaseTree}, if any exist, otherwise an empty list. + * If the only statement is a block statement, return the block's inner statements instead, + * unwrapping multiple blocks if needed. + */ + static ImmutableList getStatements(CaseTree caseTree) { + Tree at = caseTree.getBody(); + + while (at instanceof BlockTree bt) { + if (bt.getStatements().size() == 1) { + at = bt.getStatements().get(0); + } else { + break; + } + } + + return switch (at) { + case BlockTree blockTree -> ImmutableList.copyOf(blockTree.getStatements()); + case StatementTree statementTree -> ImmutableList.of(statementTree); + case null, default -> ImmutableList.of(); + }; + } + + /** Returns whether the switch statement covers all possible values of the enum. */ + static boolean isSwitchCoveringAllEnumValues(Set handledEnumValues, Type switchType) { + // Handles switching on enum only (map is bijective) + if (switchType.asElement().getKind() != ElementKind.ENUM) { + // Give up on search + return false; + } + return handledEnumValues.containsAll(ASTHelpers.enumValues(switchType.asElement())); + } + + /** Determines whether any continue statements are present in the tree. */ + static boolean hasContinueInTree(Tree tree) { + Boolean result = + new TreeScanner() { + @Override + public Boolean visitContinue(ContinueTree continueTree, Void unused) { + return true; + } + + @Override + public Boolean reduce(@Nullable Boolean left, @Nullable Boolean right) { + return Objects.equals(left, true) || Objects.equals(right, true); + } + }.scan(tree, null); + + return result != null && result; + } + + /** + * In a switch with multiple assignments, determine whether a subsequent assignment target is + * compatible with the first assignment target. + */ + static boolean isCompatibleWithFirstAssignment( + Optional assignmentTargetOptional, + Optional caseAssignmentTargetOptional) { + + if (assignmentTargetOptional.isEmpty() || caseAssignmentTargetOptional.isEmpty()) { + return false; + } + + Symbol assignmentTargetSymbol = getSymbol(assignmentTargetOptional.get()); + // For non-symbol assignment targets, multiple assignments are not currently supported + if (assignmentTargetSymbol == null) { + return false; + } + + Symbol caseAssignmentTargetSymbol = getSymbol(caseAssignmentTargetOptional.get()); + return Objects.equals(assignmentTargetSymbol, caseAssignmentTargetSymbol); + } + + /** + * Renders all comments of the supplied {@code variableTree} into a list of Strings, in code + * order. + */ + static ImmutableList renderVariableTreeComments( + VariableTree variableTree, VisitorState state) { + return state.getTokensForNode(variableTree).stream() + .flatMap(errorProneToken -> errorProneToken.comments().stream()) + .filter(comment -> !comment.getText().isEmpty()) + .map(ErrorProneComment::getText) + .collect(toImmutableList()); + } + + /** + * Renders all annotations of the supplied {@code variableTree} into a list of Strings, in code + * order. + */ + static ImmutableList renderVariableTreeAnnotations( + VariableTree variableTree, VisitorState state) { + return variableTree.getModifiers().getAnnotations().stream() + .map(state::getSourceForNode) + .collect(toImmutableList()); + } + + /** + * Renders the flags of the supplied variable declaration, such as "final", into a single + * space-separated String. + */ + static String renderVariableTreeFlags(VariableTree variableTree) { + StringBuilder flagsBuilder = new StringBuilder(); + if (!variableTree.getModifiers().getFlags().isEmpty()) { + flagsBuilder.append( + variableTree.getModifiers().getFlags().stream() + .map(flag -> flag + " ") + .collect(joining(""))); + } + return flagsBuilder.toString(); + } + + /** + * Prints source for all expressions in a given {@code case}, separated by commas, or the pattern + * and guard (if present). + */ + static String printCaseExpressionsOrPatternAndGuard(CaseTree caseTree, VisitorState state) { + if (!hasCasePattern(caseTree)) { + return caseTree.getExpressions().stream().map(state::getSourceForNode).collect(joining(", ")); + } + // Currently, `case`s can only have a single pattern, however the compiler's class structure + // does not reflect this restriction. + StringBuilder sb = + new StringBuilder( + caseTree.getLabels().stream().map(state::getSourceForNode).collect(joining(", "))); + if (caseTree.getGuard() != null) { + sb.append(" when ").append(state.getSourceForNode(caseTree.getGuard())).append(" "); + } + return sb.toString(); + } + + static boolean hasCasePattern(CaseTree caseTree) { + return caseTree.getLabels().stream() + .anyMatch(caseLabelTree -> caseLabelTree instanceof PatternCaseLabelTree); + } + + /** + * Determines whether the supplied {@code caseTree} case contains `case null` and/or `default`. + */ + static NullDefaultKind analyzeCaseForNullAndDefault(CaseTree caseTree) { + boolean hasDefault = isSwitchDefault(caseTree); + boolean hasNull = + caseTree.getExpressions().stream() + .anyMatch(expression -> expression.getKind().equals(Tree.Kind.NULL_LITERAL)); + + if (hasNull && hasDefault) { + return NullDefaultKind.KIND_NULL_AND_DEFAULT; + } else if (hasNull) { + return NullDefaultKind.KIND_NULL; + } else if (hasDefault) { + return NullDefaultKind.KIND_DEFAULT; + } + + return NullDefaultKind.KIND_NEITHER; + } + + /** + * Renders the Java source prefix needed for the supplied {@code nullDefaultKind}, incorporating + * whether the `default` case should be removed. + */ + static String renderNullDefaultKindPrefix( + NullDefaultKind nullDefaultKind, boolean removeDefault) { + + return switch (nullDefaultKind) { + case KIND_NULL_AND_DEFAULT -> removeDefault ? "case null" : "case null, default"; + case KIND_NULL -> "case null"; + case KIND_DEFAULT -> removeDefault ? "" : "default"; + case KIND_NEITHER -> "case "; + }; + } + + private SwitchUtils() {} +} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index de0546e5b2b..c670fce6bae 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -339,6 +339,7 @@ import com.google.errorprone.bugpatterns.RedundantOverride; import com.google.errorprone.bugpatterns.RedundantSetterCall; import com.google.errorprone.bugpatterns.RedundantThrows; +import com.google.errorprone.bugpatterns.RefactorSwitch; import com.google.errorprone.bugpatterns.ReferenceEquality; import com.google.errorprone.bugpatterns.RemoveUnusedImports; import com.google.errorprone.bugpatterns.RequiredModifiersChecker; @@ -1112,6 +1113,7 @@ public static ScannerSupplier warningChecks() { QualifierOrScopeOnInjectMethod.class, ReachabilityFenceUsage.class, RedundantControlFlow.class, + RefactorSwitch.class, ReferenceEquality.class, RethrowReflectiveOperationExceptionAsLinkageError.class, ReturnAtTheEndOfVoidFunction.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/RefactorSwitchTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/RefactorSwitchTest.java new file mode 100644 index 00000000000..7a17f839058 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/RefactorSwitchTest.java @@ -0,0 +1,3452 @@ +/* + * Copyright 2026 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.bugpatterns; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.fixes.Fix; +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link RefactorSwitch}. */ +@RunWith(JUnit4.class) +public final class RefactorSwitchTest { + private static final String SUIT = + """ + enum Suit { + HEART, + SPADE, + DIAMOND, + CLUB + }; + """; + private final CompilationTestHelper helper = + CompilationTestHelper.newInstance(RefactorSwitch.class, getClass()) + .addSourceLines("Suit.java", SUIT); + private final BugCheckerRefactoringTestHelper refactoringHelper = + BugCheckerRefactoringTestHelper.newInstance(RefactorSwitch.class, getClass()) + .addInputLines("Suit.java", SUIT) + .expectUnchanged(); + private final BugCheckerRefactoringTestHelper refactoringHelper2 = + BugCheckerRefactoringTestHelper.newInstance(RefactorSwitch.class, getClass()) + .addInputLines("Suit.java", SUIT) + .expectUnchanged(); + + @Test + public void refactorChain_returnSwitchExhaustive_error() { + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + import java.util.HashSet; + import java.util.Set; + + class Test { + public String foo(Suit s) { + Integer i = s == null ? 1 : 2; + Set set = new HashSet<>(); + switch (i) { + case 1 -> throw new NullPointerException("aaa"); + case 2 -> { + System.out.println("hello"); + if (true) { + return "bbb"; + } + for (int j = 0; j < 10; j++) { + if (i == 0) { + return "for"; + } + for (String setString : set) { + return "enhancedfor"; + } + while (j > 10) { + return "while"; + } + do { + if (i == 0) { + return "dowhile"; + } + } while (false); + } + Supplier supplier = + () -> { + return "lam"; + }; + return "aaa"; + } + case 3 -> { + { + var newClass = + new Comparable() { + @Override + public int compareTo(Integer other) { + return 0; + } + }; + return newClass == null ? "ccc" : "ddd"; + } + } + case 4 -> /* a */ { // b + /* c */ return i == 0 ? "zero" : "nonzero"; // d + } + case 5 -> { + throw new NullPointerException("unnecessary braces"); + } + case 6 -> { // don't remove comment + { + throw new NullPointerException("multiple unnecessary braces"); + } + } + default -> { + { + return "goodbye"; + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + import java.util.HashSet; + import java.util.Set; + + class Test { + public String foo(Suit s) { + Integer i = s == null ? 1 : 2; + Set set = new HashSet<>(); + return switch (i) { + case 1 -> throw new NullPointerException("aaa"); + case 2 -> { + System.out.println("hello"); + if (true) { + yield "bbb"; + } + for (int j = 0; j < 10; j++) { + if (i == 0) { + yield "for"; + } + for (String setString : set) { + yield "enhancedfor"; + } + while (j > 10) { + yield "while"; + } + do { + if (i == 0) { + yield "dowhile"; + } + } while (false); + } + Supplier supplier = + () -> { + return "lam"; + }; + yield "aaa"; + } + case 3 -> { + var newClass = + new Comparable() { + @Override + public int compareTo(Integer other) { + return 0; + } + }; + yield newClass == null ? "ccc" : "ddd"; + } + + case 4 -> /* a */ + // b + /* c */ + // d + i == 0 ? "zero" : "nonzero"; + case 5 -> throw new NullPointerException("unnecessary braces"); + case 6 -> + // don't remove comment + throw new NullPointerException("multiple unnecessary braces"); + default -> "goodbye"; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_hasContinue_noError() { + // Continuing out of a switch statement is allowed, but continuing out of a switch expression is + // not. + helper + .addSourceLines( + "Test.java", + """ + class Test { + public boolean foo(Suit suit) { + for (int i = 0; i < 10; i++) { + switch (suit) { + case HEART, CLUB, SPADE -> { + return true; + } + case DIAMOND -> { + if (i > 99) { + continue; + } + return false; + } + default -> { + return false; + } + } + // Here is not reachable + } + return false; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void refactorChain_returnSwitchEnumRemoveDefault_error() { + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> { + { + return "black"; + } + } + default -> { + { + return "should suggest to remove"; + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> "black"; + default -> "should suggest to remove"; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(FixChoosers.FIRST) + .doTest(TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> { + { + return "black"; + } + } + default -> { + { + return "should suggest to remove"; + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> "black"; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_javaDoc_error() { + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + enum SideOfCoin { + OBVERSE, + REVERSE + }; + + private String renderName(SideOfCoin sideOfCoin) { + switch (sideOfCoin) { + case OBVERSE -> { + return "Heads"; + } + case REVERSE -> { + return "Tails"; + } + } + // This should never happen, but removing this will cause a compile-time error + throw new RuntimeException("Unknown side of coin"); + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + enum SideOfCoin { + OBVERSE, + REVERSE + }; + + private String renderName(SideOfCoin sideOfCoin) { + return switch (sideOfCoin) { + case OBVERSE -> "Heads"; + case REVERSE -> "Tails"; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_javaDocPreservesComments_error() { + // Preserves comments that should not be removed + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + enum SideOfCoin { + OBVERSE, + REVERSE + }; + + private String renderName(SideOfCoin sideOfCoin) { + switch (sideOfCoin) { + case OBVERSE -> { + return "Heads"; + } + case REVERSE -> { + return "Tails"; + } + } + // This should never happen, but removing this will cause a compile-time error + throw new RuntimeException("Unknown side of coin"); + // LINT. Don't remove this + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + enum SideOfCoin { + OBVERSE, + REVERSE + }; + + private String renderName(SideOfCoin sideOfCoin) { + return switch (sideOfCoin) { + case OBVERSE -> "Heads"; + case REVERSE -> "Tails"; + }; + // This should never happen, but removing this will cause a compile-time error + + // LINT. Don't remove this + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_returnSwitchNullDefaultRemoveDefault_error() { + + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> { + { + return "black"; + } + } + case null, default -> { + { + return "should suggest to remove"; + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> "black"; + case null, default -> "should suggest to remove"; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(FixChoosers.FIRST) + .doTest(TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> { + { + return "black"; + } + } + case null, default -> { + { + return "should suggest to remove"; + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> "black"; + case null -> "should suggest to remove"; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_returnSwitchNullDefaultThrowRemoveDefault_error() { + + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> { + { + return "black"; + } + } + case null, default -> { + { + throw new NullPointerException("should suggest to remove"); + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> "black"; + case null, default -> throw new NullPointerException("should suggest to remove"); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(FixChoosers.FIRST) + .doTest(TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> { + { + return "black"; + } + } + case null, default -> { + { + throw new NullPointerException("should suggest to remove"); + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> "black"; + case null -> throw new NullPointerException("should suggest to remove"); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_returnSwitchNullDefaultGenericBlockRemoveDefault_error() { + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> { + { + return "black"; + } + } + case null, default -> { + { + System.out.println("should suggest to remove default"); + return "should suggest to remove default"; + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> "black"; + case null, default -> { + System.out.println("should suggest to remove default"); + yield "should suggest to remove default"; + } + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(FixChoosers.FIRST) + .doTest(TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> { + { + return "black"; + } + } + case null, default -> { + { + System.out.println("should suggest to remove default"); + return "should suggest to remove default"; + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART, DIAMOND -> throw new NullPointerException("red"); + case SPADE, CLUB -> "black"; + case null -> { + System.out.println("should suggest to remove default"); + yield "should suggest to remove default"; + } + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_returnSwitchNonExhaustive_noError() { + helper + .addSourceLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + Integer i = s == null ? 1 : 2; + switch (i) { + case 1 -> throw new NullPointerException("one"); + case 2 -> { + return "two"; + } + } + return "default"; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void refactorChain_returnSwitchEmpty_noError() { + helper + .addSourceLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + Integer i = s == null ? 1 : 2; + switch (i) { + } + return "default"; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_returnSwitch_error() { + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int invoke() { + return 123; + } + + public int foo(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + return invoke(); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int invoke() { + return 123; + } + + public int foo(Suit suit) { + return switch (suit) { + case HEART, DIAMOND -> invoke(); + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(); + } + + @Test + public void switchByEnum_multipleStatementsAndTheLastNotReturn_error() { + + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + return 1; + } + case SPADE -> { + System.out.println("hello"); + throw new RuntimeException(); + } + default -> throw new NullPointerException(); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case HEART, DIAMOND -> 1; + case SPADE -> { + System.out.println("hello"); + throw new RuntimeException(); + } + default -> throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(); + } + + @Test + public void switchByEnum_casePatternAndGuard_error() { + + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + return 1; + } + case SPADE -> { + System.out.println("spade"); + throw new RuntimeException(); + } + case CLUB -> { + throw new NullPointerException(); + } + case Suit s when s == Suit.HEART -> throw new NullPointerException(); + default -> throw new NullPointerException(); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case HEART, DIAMOND -> 1; + case SPADE -> { + System.out.println("spade"); + throw new RuntimeException(); + } + case CLUB -> throw new NullPointerException(); + case Suit s when s == Suit.HEART -> throw new NullPointerException(); + default -> throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(FixChoosers.FIRST) + .doTest(); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + return 1; + } + case SPADE -> { + System.out.println("spade"); + throw new RuntimeException(); + } + case CLUB -> { + throw new NullPointerException(); + } + case Suit s when s == Suit.HEART -> throw new NullPointerException(); + default -> throw new NullPointerException(); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case HEART, DIAMOND -> 1; + case SPADE -> { + System.out.println("spade"); + throw new RuntimeException(); + } + case CLUB -> throw new NullPointerException(); + case Suit s when s == Suit.HEART -> throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(); + } + + @Test + public void switchByEnum_nullGroupedWithDefault_error() { + // Null can be grouped with default + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + return 1; + } + case SPADE -> { + System.out.println("hello"); + throw new RuntimeException(); + } + case null, default -> throw new NullPointerException(); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case HEART, DIAMOND -> 1; + case SPADE -> { + System.out.println("hello"); + throw new RuntimeException(); + } + case null, default -> throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(); + } + + @Test + public void switchByEnumReturnSwitch_nullDefaultSameProduction_error() { + // Null can be grouped together with default in a single SwitchLabelProduction in Java 21+ + // as `case null [, default]` + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + return 1; + } + case SPADE, CLUB -> { + System.out.println("hello"); + throw new RuntimeException(); + } + case null, default -> { + throw new NullPointerException(); + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case HEART, DIAMOND -> 1; + case SPADE, CLUB -> { + System.out.println("hello"); + throw new RuntimeException(); + } + case null, default -> throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + return 1; + } + case SPADE, CLUB -> { + System.out.println("hello"); + throw new RuntimeException(); + } + case null, default -> { + throw new NullPointerException(); + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case HEART, DIAMOND -> 1; + case SPADE, CLUB -> { + System.out.println("hello"); + throw new RuntimeException(); + } + case null -> throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(); + } + + @Test + public void switchByEnum_middleNullCase3_error() { + // null case is converted without being grouped with default + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + return 1; + } + case SPADE -> { + System.out.println("hello"); + throw new RuntimeException(); + } + case null -> throw new RuntimeException("single null case"); + default -> throw new NullPointerException(); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case HEART, DIAMOND -> 1; + case SPADE -> { + System.out.println("hello"); + throw new RuntimeException(); + } + case null -> throw new RuntimeException("single null case"); + default -> throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(); + } + + @Test + public void switchByEnum_exhaustiveWithDefault_error() { + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int invoke() { + return 123; + } + + public int foo(Suit suit) { + String z = "dkfj"; + switch (z) { + case "", "DIAMOND", "SPADE" -> { + return invoke(); + } + default -> { + return 2; + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int invoke() { + return 123; + } + + public int foo(Suit suit) { + String z = "dkfj"; + return switch (z) { + case "", "DIAMOND", "SPADE" -> invoke(); + default -> 2; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_defaultFallThru_noError() { + // No error because default doesn't return anything within its block + helper + .addSourceLines( + "Test.java", + """ + class Test { + public int invoke() { + return 123; + } + + public int foo(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + return invoke(); + } + case SPADE -> throw new RuntimeException(); + default -> {} + } + return -2; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_returnSwitchVoid_noError() { + // A void cannot be converted to a return switch + helper + .addSourceLines( + "Test.java", + """ + class Test { + public void foo(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + return; + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_returnYield_noError() { + // Does not attempt to convert "yield" expressions in colon-style switches + helper + .addSourceLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case HEART: + yield 2; + case DIAMOND: + yield 3; + case SPADE: + // Fall through + default: + throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnumExhaustive_qualifiedCaseLabels() { + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + switch (suit) { + case Suit.HEART -> { + return 1; + } + case Suit.CLUB -> { + return 2; + } + case Suit.DIAMOND -> { + return 3; + } + case Suit.SPADE -> { + return 4; + } + } + throw new AssertionError(); + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case Suit.HEART -> 1; + case Suit.CLUB -> 2; + case Suit.DIAMOND -> 3; + case Suit.SPADE -> 4; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + /*********************************************************************************************** + * Assignment switch tests + **********************************************************************************************/ + + @Test + public void switchByEnum_assignmentSwitchWithComments_error() { + + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.lang.annotation.Repeatable; + import java.util.HashMap; + import java.util.Map; + import java.util.Set; + + class Test { + @interface MyAnnos { + Test.MyAnno[] value(); + } + + @Repeatable(Test.MyAnnos.class) + @interface MyAnno { + String v() default ""; + } + + @interface MyOtherAnno {} + + public int y = 0; + + public int foo(Suit suit) { + @MyAnno(v = "foo") + // alpha + /* beta */ @MyOtherAnno + @MyAnno + final /* chi */ int /* gamma */ x /* delta */; // epsilon + // zeta + switch (suit) { + case HEART, DIAMOND -> { + x = ((y + 1) * (y * y)) << 1; + } + case SPADE -> // Spade reason + throw new RuntimeException(); + default -> { + { + throw new NullPointerException(); + } + } + } + Map map = null; + switch (suit) { + case HEART, DIAMOND -> { + map = new HashMap<>(); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.lang.annotation.Repeatable; + import java.util.HashMap; + import java.util.Map; + import java.util.Set; + + class Test { + @interface MyAnnos { + Test.MyAnno[] value(); + } + + @Repeatable(Test.MyAnnos.class) + @interface MyAnno { + String v() default ""; + } + + @interface MyOtherAnno {} + + public int y = 0; + + public int foo(Suit suit) { + // epsilon + // zeta + // alpha + /* beta */ + /* chi */ + /* gamma */ + /* delta */ + @MyAnno(v = "foo") + @MyOtherAnno + @MyAnno + final int x = + switch (suit) { + case HEART, DIAMOND -> ((y + 1) * (y * y)) << 1; + case SPADE -> // Spade reason + throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + + Map map = + switch (suit) { + case HEART, DIAMOND -> new HashMap<>(); + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void variableInTransitiveEnclosingBlock_shouldNotBeMoved() { + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + public Map foo(Suit suit) { + Map map = null; + if (toString().length() == 2) + switch (suit) { + case HEART, DIAMOND -> { + map = new HashMap<>(); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return map; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.HashMap; + import java.util.Map; + + class Test { + public Map foo(Suit suit) { + Map map = null; + if (toString().length() == 2) + map = + switch (suit) { + case HEART, DIAMOND -> new HashMap<>(); + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return map; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_assignmentSwitchToNearbyDefined_error() { + // The switch block cannot be combined with the variable declaration for {@code x} because the + // variable declaration is nearby, but not immediately preceding the switch block. + + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int z = 3; + int x; + int y; + switch (suit) { + case HEART, DIAMOND -> { + x = ((z + 1) * (z * z)) << 1; + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int z = 3; + int x; + int y; + x = + switch (suit) { + case HEART, DIAMOND -> ((z + 1) * (z * z)) << 1; + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_assignmentSwitchDifferentBlockScope_error() { + // Local variable {@code x} is defined before the switch block, but at a different (parent) + // block scope than the switch block. Therefore, it should not be combined with the switch + // assignment. + + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int z = 3; + int x; + { + { + switch (suit) { + case HEART, DIAMOND -> { + x = ((z + 1) * (z * z)) << 1; + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + } + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int z = 3; + int x; + { + { + x = + switch (suit) { + case HEART, DIAMOND -> ((z + 1) * (z * z)) << 1; + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + } + } + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_assignmentSwitchToLocalHasDefaultVolatile_error() { + // Local variable {@code x} is initialized by reading a {@code volatile} field, which includes + // inter-thread action effects (refer to e.g. JLS 21 § 17.4.2); therefore, should not be + // combined with the switch assignment because those effects could be different from the + // original source code. See also e.g. Shuyang Liu, John Bender, and Jens Palsberg. Compiling + // Volatile Correctly in Java. In 36th European Conference on Object-Oriented Programming (ECOOP + // 2022). Leibniz International Proceedings in Informatics (LIPIcs), Volume 222, pp. 6:1-6:26, + // Schloss Dagstuhl – Leibniz-Zentrum für Informatik (2022) + + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + volatile int v = 0; + + public int foo(Suit suit) { + int z = 3; + int x = v; + switch (suit) { + case HEART, DIAMOND -> { + x = ((z + 1) * (z * z)) << 1; + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + volatile int v = 0; + + public int foo(Suit suit) { + int z = 3; + int x = v; + x = + switch (suit) { + case HEART, DIAMOND -> ((z + 1) * (z * z)) << 1; + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_assignmentSwitchVarEnum_error() { + // var type + enum value merging with definition + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + var suit2 = Suit.SPADE; + switch (suit) { + case HEART, DIAMOND -> { + suit2 = Suit.SPADE; + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return suit2 == null ? 0 : 1; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + + var suit2 = + switch (suit) { + case HEART, DIAMOND -> Suit.SPADE; + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return suit2 == null ? 0 : 1; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_assignmentSwitchVarConstant_error() { + // var type + compile time constant merging with definition + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + var rv = 0; + switch (suit) { + case HEART, DIAMOND -> { + rv = 1; + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return rv; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + + var rv = + switch (suit) { + case HEART, DIAMOND -> 1; + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return rv; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_assignmentSwitchMixedReferences_error() { + // Must deduce that "x" and "this.x" refer to same thing + // Note that suggested fix uses the style of the first case (in source order). + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + int x; + + public Test(int foo) { + x = -1; + } + + public int foo(Suit suit) { + switch (suit) { + /* Comment before first case */ + case /* LHS comment */ HEART -> { + this.x <<= 2; + } + case DIAMOND -> { + x <<= (((x + 1) * (x * x)) << 1); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + int x; + + public Test(int foo) { + x = -1; + } + + public int foo(Suit suit) { + this.x <<= + switch (suit) { + /* Comment before first case */ + case /* LHS comment */ HEART -> 2; + case DIAMOND -> (((x + 1) * (x * x)) << 1); + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_assignmentSwitchMixedReferences_noError() { + // Must deduce that "x" and "this.y" refer to different things + helper + .addSourceLines( + "Test.java", + """ + class Test { + int x, y; + + public Test(int foo) { + x = -1; + y = -1; + } + + public int foo(Suit suit) { + switch (suit) { + case HEART -> { + x = 2; + } + case DIAMOND -> { + this.y = (((x + 1) * (x * x)) << 1); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_sameVariableDeclaratorBlock_error() { + // x and y are declared in the same VariableDeclaratorList, therefore cannot be combined, + // however assignment switch conversion can still be applied + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int y = 0, x; + switch (suit) { + case HEART -> { + x = 2; + } + case DIAMOND -> { + x = (((y + 1) * (y * y)) << 1); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return 0; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int y = 0, x; + x = + switch (suit) { + case HEART -> 2; + case DIAMOND -> (((y + 1) * (y * y)) << 1); + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return 0; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_declaredTwoLinesAbove_error() { + // Variable `x` is declared two lines above the switch statement, therefore cannot be combined, + // but the assignment switch conversion can still be applied + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x; + int y; + switch (suit) { + case HEART -> { + x = 2; + } + case DIAMOND -> { + x = "hello".length(); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return 0; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x; + int y; + x = + switch (suit) { + case HEART -> 2; + case DIAMOND -> "hello".length(); + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return 0; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_assignmentSwitchTwoAssignments_noError() { + // Can't convert multiple assignments, even if redundant + helper + .addSourceLines( + "Test.java", + """ + class Test { + int x; + + public Test(int foo) { + x = -1; + } + + public int foo(Suit suit) { + switch (suit) { + case HEART -> { + x = 2; + x = 3; + } + case DIAMOND -> { + this.x = (((x + 1) * (x * x)) << 1); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_assignmentSwitchToSingleArray_error() { + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + int[] x; + + public Test(int foo) { + x = null; + } + + public int[] foo(Suit suit) { + switch (suit) { + case HEART -> throw new RuntimeException(); + case DIAMOND -> { + x[6] <<= (((x[6] + 1) * (x[6] * x[5]) << 1)); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + int[] x; + + public Test(int foo) { + x = null; + } + + public int[] foo(Suit suit) { + x[6] <<= + switch (suit) { + case HEART -> throw new RuntimeException(); + case DIAMOND -> (((x[6] + 1) * (x[6] * x[5]) << 1)); + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(); + } + + @Test + public void switchByEnum_assignmentSwitchToMultipleArray_noError() { + // Multiple array dereferences or other non-variable left-hand-suit expressions may (in + // principle) be convertible to assignment switches, but this feature is not supported at this + // time + helper + .addSourceLines( + "Test.java", + """ + class Test { + int[] x; + + public Test(int foo) { + x = null; + } + + public int[] foo(Suit suit) { + switch (suit) { + case HEART -> { + // Inline comment + x[6] <<= 2; + } + case DIAMOND -> { + x[6] <<= (((x[6] + 1) * (x[6] * x[5]) << 1)); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_assignmentSwitchToMultipleDistinct_noError() { + // x[5] and x[6] are distinct assignment targets + helper + .addSourceLines( + "Test.java", + """ + class Test { + int[] x; + + public Test(int foo) { + x = null; + } + + public int[] foo(Suit suit) { + switch (suit) { + case HEART -> { + // Inline comment + x[6] <<= 2; + } + case DIAMOND -> { + x[5] <<= (((x[6] + 1) * (x[6] * x[5]) << 1)); + } + case SPADE -> { + throw new RuntimeException(); + } + default -> { + throw new NullPointerException(); + } + } + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_assignmentSwitchMixedKinds_noError() { + // Different assignment types ("=" versus "+="). The check does not attempt to alter the + // assignments to make the assignment types match (e.g. does not change to "x = x + 2") + helper + .addSourceLines( + "Test.java", + """ + class Test { + int x; + + public Test(int foo) { + x = -1; + } + + public int foo(Suit suit) { + switch (suit) { + case HEART -> { + x += 2; + } + case DIAMOND -> { + x = (((x + 1) * (x * x)) << 1); + } + case SPADE -> throw new RuntimeException(); + default -> throw new NullPointerException(); + } + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_assignmentLabelledContinue_noError() { + helper + .addSourceLines( + "Test.java", + """ + class Test { + int x; + + public Test(int foo) { + x = -1; + } + + public int foo(Suit suit) { + before: + for (; ; ) { + switch (suit) { + case HEART -> { + x = 2; + } + case DIAMOND -> { + x = (((x + 1) * (x * x)) << 1); + } + case SPADE -> { + continue before; + } + default -> throw new NullPointerException(); + } + break; + } + after: + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + 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 (...)}). + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + switch (suit) { + case HEART, DIAMOND -> { + x = (((x + 1) * (x * x)) << 2); + } + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + x = + switch (suit) { + case HEART, DIAMOND -> (((x + 1) * (x * x)) << 2); + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + // There should be no second fix that attempts to remove the default case because there is + // no default case. + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnumAssignment_nullDefaultSameProduction_error() { + // Null can be grouped together with default in a single SwitchLabel production in Java 21+ + // as `case null [, default]` + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + switch (suit) { + case HEART, DIAMOND -> { + x = x + 1; + } + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + case null, default -> throw new IllegalArgumentException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + x = + switch (suit) { + case HEART, DIAMOND -> x + 1; + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + case null, default -> throw new IllegalArgumentException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + switch (suit) { + case HEART, DIAMOND -> { + x = x + 1; + } + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + case null, default -> throw new IllegalArgumentException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + x = + switch (suit) { + case HEART, DIAMOND -> x + 1; + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + case null -> throw new IllegalArgumentException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnumAssignment_fullyRemovesDefaultCase_error() { + // Removes default case and its code entirely + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + switch (suit) { + case HEART, DIAMOND -> { + x = x + 1; + } + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + default -> throw new IllegalArgumentException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + x = + switch (suit) { + case HEART, DIAMOND -> x + 1; + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + default -> throw new IllegalArgumentException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + switch (suit) { + case HEART, DIAMOND -> { + x = x + 1; + } + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + default -> throw new IllegalArgumentException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + x = + switch (suit) { + case HEART, DIAMOND -> x + 1; + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_exhaustiveCompoundAssignmentSwitch_error() { + // Verify compound assignments (here, +=) + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + switch (suit) { + case HEART, DIAMOND -> { + x += (((x + 1) * (x * x)) << 1); + } + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + x += + switch (suit) { + case HEART, DIAMOND -> (((x + 1) * (x * x)) << 1); + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(); + } + + @Test + public void switchByEnum_compoundAssignmentExampleInDocumentation_error() { + // This code appears as an example in the documentation (added surrounding class) + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + int score = 0; + + private void updateScore(Suit suit) { + switch (suit) { + case HEART, DIAMOND -> { + score += -1; + } + case SPADE -> { + score += 2; + } + case CLUB -> { + score += 3; + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + int score = 0; + + private void updateScore(Suit suit) { + score += + switch (suit) { + case HEART, DIAMOND -> -1; + case SPADE -> 2; + case CLUB -> 3; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_exhaustiveAssignmentSwitchCaseList_error() { + // Statement switch has cases with multiple values + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + switch (suit) { + case HEART, DIAMOND -> { + x = (((x + 1) * (x * x)) << 1); + } + case SPADE, CLUB -> { + throw new NullPointerException(); + } + } + return x; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + x = + switch (suit) { + case HEART, DIAMOND -> (((x + 1) * (x * x)) << 1); + case SPADE, CLUB -> throw new NullPointerException(); + }; + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(); + } + + @Test + public void switchByEnum_nonExhaustiveAssignmentSwitch_noError() { + // No HEART case + helper + .addSourceLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + switch (suit) { + case DIAMOND -> { + x = (((x + 1) * (x * x)) << 1); + } + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + } + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_switchInSwitch_noError() { + helper + .addSourceLines( + "Test.java", + """ + class Test { + public boolean foo(Suit suit) { + Integer i = suit == null ? 0 : 1; + switch (i) { + case 0: + break; + default: + switch (suit) { + case HEART -> { + return true; + } + default -> { + return true; + } + } + } + return false; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=true", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + /*********************************************************************************************** + * Simplify switch tests + **********************************************************************************************/ + + @Test + public void switchByEnum_simplify_noError() { + // No HEART case + helper + .addSourceLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + switch (suit) { + case DIAMOND -> { + x = (((x + 1) * (x * x)) << 1); + } + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + } + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .doTest(); + } + + @Test + public void switchByEnum_switchStatementWithinSwitchStatement_noError() { + // Arrow-style switch within an arrow-style switch, both statement switches + helper + .addSourceLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + int x = 0; + switch (suit) { + case DIAMOND -> { + switch (x) { + case 1 -> { + x = 1; + } + case 2 -> { + x = 2; + } + case 3 -> { + x = 3; + } + } + } + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + } + return x; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .doTest(); + } + + @Test + public void refactorChain_simplify_error() { + + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART -> throw new NullPointerException("heart"); + case DIAMOND -> { + System.out.println("diamond"); + yield "diamond"; + } + case SPADE -> { + { + Supplier supplier = + () -> { + { + return "black"; + } + }; + yield "black"; + } + } + case CLUB -> { + { + yield "also black"; + } + } + case null, default -> { + { + System.out.println("should suggest to remove default"); + yield "should suggest to remove default"; + } + } + }; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART -> throw new NullPointerException("heart"); + case DIAMOND -> { + System.out.println("diamond"); + yield "diamond"; + } + case SPADE -> { + Supplier supplier = + () -> { + { + return "black"; + } + }; + yield "black"; + } + + case CLUB -> "also black"; + case null, default -> { + System.out.println("should suggest to remove default"); + yield "should suggest to remove default"; + } + }; + } + } + + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .setFixChooser(FixChoosers.FIRST) + .doTest(TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART -> throw new NullPointerException("heart"); + case DIAMOND -> { + System.out.println("diamond"); + yield "diamond"; + } + case SPADE -> { + { + Supplier supplier = + () -> { + { + return "black"; + } + }; + yield "black"; + } + } + case CLUB -> { + { + yield "also black"; + } + } + case null, default -> { + { + System.out.println("should suggest to remove default"); + yield "should suggest to remove default"; + } + } + }; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART -> throw new NullPointerException("heart"); + case DIAMOND -> { + System.out.println("diamond"); + yield "diamond"; + } + case SPADE -> { + Supplier supplier = + () -> { + { + return "black"; + } + }; + yield "black"; + } + + case CLUB -> "also black"; + case null -> { + System.out.println("should suggest to remove default"); + yield "should suggest to remove default"; + } + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_cannotSimplifyIf_error() { + // The "if" block cannot be simplified into an expression + + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + + public String makeString() { + return "foo"; + } + + public String foo(Suit s) { + return switch (s) { + case HEART -> makeString(); + case DIAMOND, SPADE -> { + yield makeString(); + } + case CLUB -> { + if (true) { + yield makeString(); + } else { + System.out.println("Desktop MinuteMaid URLs not supported for flow: " + s); + yield makeString(); + } + } + default -> { + yield makeString(); + } + }; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + + public String makeString() { + return "foo"; + } + + public String foo(Suit s) { + return switch (s) { + case HEART -> makeString(); + case DIAMOND, SPADE -> makeString(); + case CLUB -> { + if (true) { + yield makeString(); + } else { + System.out.println("Desktop MinuteMaid URLs not supported for flow: " + s); + yield makeString(); + } + } + default -> makeString(); + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .setFixChooser(FixChoosers.FIRST) + .doTest(TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + + public String makeString() { + return "foo"; + } + + public String foo(Suit s) { + return switch (s) { + case HEART -> makeString(); + case DIAMOND, SPADE -> { + yield makeString(); + } + case CLUB -> { + if (true) { + yield makeString(); + } else { + System.out.println("Desktop MinuteMaid URLs not supported for flow: " + s); + yield makeString(); + } + } + default -> { + yield makeString(); + } + }; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + + public String makeString() { + return "foo"; + } + + public String foo(Suit s) { + return switch (s) { + case HEART -> makeString(); + case DIAMOND, SPADE -> makeString(); + case CLUB -> { + if (true) { + yield makeString(); + } else { + System.out.println("Desktop MinuteMaid URLs not supported for flow: " + s); + yield makeString(); + } + } + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_simplifyBraces_error() { + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + switch (s) { + case HEART -> throw new NullPointerException("heart"); + case DIAMOND -> { + System.out.println("diamond"); + } + case null, default -> { + { + System.out.println("others"); + } + } + } + return ""; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + switch (s) { + case HEART -> throw new NullPointerException("heart"); + case DIAMOND -> System.out.println("diamond"); + + case null, default -> System.out.println("others"); + } + return ""; + } + } + + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_simplifyBracesPartial_error() { + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART -> { + { + throw new NullPointerException("heart"); + } + } + case DIAMOND -> { + { + yield "diamond"; + } + } + case SPADE, CLUB -> "black"; + case null, default -> { + { + yield "others"; + } + } + }; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART -> throw new NullPointerException("heart"); + + case DIAMOND -> "diamond"; + case SPADE, CLUB -> "black"; + case null, default -> "others"; + }; + } + } + + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .setFixChooser(FixChoosers.FIRST) + .doTest(TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART -> { + { + throw new NullPointerException("heart"); + } + } + case DIAMOND -> { + { + yield "diamond"; + } + } + case SPADE, CLUB -> "black"; + case null, default -> { + { + yield "others"; + } + } + }; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + return switch (s) { + case HEART -> throw new NullPointerException("heart"); + + case DIAMOND -> "diamond"; + case SPADE, CLUB -> "black"; + case null -> "others"; + }; + } + } + + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_simplifyBracesPartialSwitchStatement_error() { + // We are not testing the return switch conversion here, just simplifying a switch statement + // (includes removing the default case) + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + switch (s) { + case HEART -> { + { + throw new NullPointerException("heart"); + } + } + case DIAMOND -> { + { + return "diamond"; + } + } + case SPADE, CLUB -> { + return "black"; + } + case null, default -> { + { + return "others"; + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + switch (s) { + case HEART -> throw new NullPointerException("heart"); + + case DIAMOND -> { + return "diamond"; + } + + case SPADE, CLUB -> { + return "black"; + } + case null, default -> { + return "others"; + } + } + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .setFixChooser(FixChoosers.FIRST) + .doTest(TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + switch (s) { + case HEART -> { + { + throw new NullPointerException("heart"); + } + } + case DIAMOND -> { + { + return "diamond"; + } + } + case SPADE, CLUB -> {return "black";} + case null, default -> { + { + return "others"; + } + } + } + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + switch (s) { + case HEART -> throw new NullPointerException("heart"); + + case DIAMOND -> { + return "diamond"; + } + + case SPADE, CLUB -> { + return "black"; + } + case null -> { + return "others"; + } + } + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .setFixChooser(RefactorSwitchTest::assertSecondAndLastFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void refactorChain_simplifyBracesCannotRemoveDefault_error() { + // HEART case is missing, so default cannot be removed in secondary fix + refactoringHelper + .addInputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + return switch (s) { + case DIAMOND -> { + { + yield "diamond"; + } + } + case SPADE, CLUB -> "black"; + case null, default -> { + { + yield "others"; + } + } + }; + } + } + """) + .addOutputLines( + "Test.java", + """ + import java.util.function.Supplier; + + class Test { + public String foo(Suit s) { + return switch (s) { + case DIAMOND -> "diamond"; + case SPADE, CLUB -> "black"; + case null, default -> "others"; + }; + } + } + + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .setFixChooser(RefactorSwitchTest::assertOneFixAndChoose) + .doTest(TEXT_MATCH); + } + + @Test + public void switchByEnum_cannotSimplify_noError() { + // HEART case has a size-one block, but cannot be simplified because it has an if statement (we + // don't attempt to convert simple if statements into ternaries) + helper + .addSourceLines( + "Test.java", + """ + class Test { + public boolean foo(Suit suit) { + return switch (suit) { + case HEART -> { + if (true) { + yield true; + } else { + yield false; + } + } + case DIAMOND -> true; + case SPADE -> true; + default -> true; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=true") + .doTest(); + } + + /*********************************************************************************************** + * Disabled tests + **********************************************************************************************/ + + @Test + public void switchByEnum_allFlagsDisabledSwitch_noError() { + // The switch could be refactored if not disabled by flags + helper + .addSourceLines( + "Test.java", + """ + class Test { + public boolean foo(Suit suit) { + switch (suit) { + case HEART -> { + return true; + } + case DIAMOND -> { + return true; + } + case SPADE -> { + return true; + } + default -> { + return true; + } + } + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + @Test + public void switchByEnum_allFlagsDisabledSwitchExpr_noError() { + // The switch could be refactored if not disabled by flags + helper + .addSourceLines( + "Test.java", + """ + class Test { + public boolean foo(Suit suit) { + return switch (suit) { + case HEART -> { + yield true; + } + case DIAMOND -> true; + case SPADE -> true; + default -> true; + }; + } + } + """) + .setArgs( + "-XepOpt:RefactorSwitch:EnableAssignmentSwitch=false", + "-XepOpt:RefactorSwitch:EnableReturnSwitch=false", + "-XepOpt:RefactorSwitch:EnableSimplifySwitch=false") + .doTest(); + } + + /** + * Asserts that there is exactly one suggested fix and returns it. + * + *

Similar to {@code FixChoosers.FIRST}, but also asserts that there is exactly one fix. + */ + public static Fix assertOneFixAndChoose(List fixes) { + assertThat(fixes).hasSize(1); + return fixes.get(0); + } + + /** Asserts that there are exactly two suggested fixes and returns the second one. */ + public static Fix assertSecondAndLastFixAndChoose(List fixes) { + assertThat(fixes).hasSize(2); + return fixes.get(1); + } +} diff --git a/docs/bugpattern/RefactorSwitch.md b/docs/bugpattern/RefactorSwitch.md new file mode 100644 index 00000000000..1d34da74b69 --- /dev/null +++ b/docs/bugpattern/RefactorSwitch.md @@ -0,0 +1,89 @@ +This checker aims to improve the readability of new-style arrow `switch`es by +simplifying and refactoring them. + +### Simplifications: + +* When a `case` consists only of a `yield`ing some value, it can be re-written + with just the value. For example, `case FOO -> { yield "bar"; }` can be + shortened to `case FOO -> "bar";` +* When a case has redundant braces around the right-hand side of the arrow, + they can be removed. For example, `case FOO -> { System.out.println("bar"); + }` can shortened to `case FOO -> System.out.println("bar");` + +### Factoring out `return switch`: + +* When every value of an `enum` is covered by a `case` which `return`s, the + `return` can be factored out + +``` {.bad} +enum SideOfCoin {OBVERSE, REVERSE}; + +private String renderName(SideOfCoin sideOfCoin) { + switch(sideOfCoin) { + case OBVERSE -> { + return "Heads"; + } + case REVERSE -> { + return "Tails"; + } + } + // This should never happen, but removing this will cause a compile-time error + throw new RuntimeException("Unknown side of coin"); +} +``` + +The transformed code is simpler and elides the "should never happen" handler. + +``` {.good} +enum SideOfCoin {OBVERSE, REVERSE}; + +private String renderName(SideOfCoin sideOfCoin) { + return switch(sideOfCoin) { + case OBVERSE -> "Heads"; + case REVERSE -> "Tails"; + }; +} +``` + +* When switching on something other than an `enum`, if the `case`s are + exhaustive, then a similar refactoring can be performed. + +### Factoring out assignment `switch`: + +* When every `case` just assigns a value to the same variable, the assignment + can potentially be factored out +* If a local variable is defined and then immediately overwritten by a + `switch`, the definition and assignment can potentially be combined + +``` {.bad} +enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS}; + +private void updateScore(Suit suit) { + int score = 0; + switch(suit) { + case HEARTS, DIAMONDS -> { + score = -1; + } + case SPADES -> { + score = 2; + } + case CLUBS -> { + score = 3; + } + } +} +``` + +Which can be consolidated: + +``` {.good} +enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS}; + +private void updateScore(Suit suit) { + int score = switch(suit) { + case HEARTS, DIAMONDS -> -1; + case SPADES -> 2; + case CLUBS -> 3; + }; +} +```