Skip to content

Commit

Permalink
Add suggested fixes to MissingCasesInEnumSwitch
Browse files Browse the repository at this point in the history
RELNOTES: N/A
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=117845252
  • Loading branch information
cushon committed Mar 23, 2016
1 parent 672a344 commit a277aef
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 32 deletions.
Expand Up @@ -17,37 +17,47 @@
package com.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.Category.JDK;
import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.BugPattern.MaturityLevel.MATURE;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;

import com.google.common.collect.Sets;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.SwitchTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Description.Builder;
import com.google.errorprone.util.ASTHelpers;

import com.sun.source.tree.CaseTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.IdentifierTree;
import com.sun.source.tree.SwitchTree;
import com.sun.tools.javac.code.Symbol.TypeSymbol;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.JCTree.JCSwitch;
import com.sun.tools.javac.tree.TreeInfo;

import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.Set;

import javax.lang.model.element.ElementKind;

/**
* @author cushon@google.com (Liam Miller-Cushon)
*/
@BugPattern(name = "MissingCasesInEnumSwitch",
summary = "Enum switch statement is missing cases",
explanation = "Enums on switches should either handle all possible values of the enum, or"
+ " have an explicit 'default' case.",
category = JDK, severity = ERROR, maturity = EXPERIMENTAL)
public class MissingCasesInEnumSwitch extends BugChecker
implements SwitchTreeMatcher {
@BugPattern(
name = "MissingCasesInEnumSwitch",
summary = "Enum switch statement is missing cases",
explanation =
"Enums on switches should either handle all possible values of the enum, or"
+ " have an explicit default case",
category = JDK,
severity = WARNING,
maturity = MATURE
)
public class MissingCasesInEnumSwitch extends BugChecker implements SwitchTreeMatcher {

@Override
public Description matchSwitch(SwitchTree tree, VisitorState state) {
Expand All @@ -61,13 +71,39 @@ public Description matchSwitch(SwitchTree tree, VisitorState state) {
return Description.NO_MATCH;
}

LinkedHashSet<String> unhandled =
setDifference(ASTHelpers.enumValues(switchType), collectEnumSwitchCases(tree));
Set<String> unhandled =
Sets.difference(ASTHelpers.enumValues(switchType), collectEnumSwitchCases(tree));
if (unhandled.isEmpty()) {
return Description.NO_MATCH;
}

return buildDescription(tree).setMessage(buildMessage(unhandled)).build();
Description.Builder description = buildDescription(tree).setMessage(buildMessage(unhandled));
buildFixes(tree, state, unhandled, description);
return description.build();
}

/** Adds suggested fixes. */
private void buildFixes(
SwitchTree tree, VisitorState state, Set<String> unhandled, Builder description) {

int idx = state.getEndPosition((JCTree) tree) - 1; // preserve closing '}'

description.addFix(
SuggestedFix.replace(
idx,
idx,
String.format(
"default: throw new AssertionError(\"unexpected case: \" + %s);\n",
state.getSourceForNode(TreeInfo.skipParens((JCTree) tree.getExpression())))));

StringBuilder sb = new StringBuilder();
for (String label : unhandled) {
sb.append(String.format("case %s: ", label));
}
sb.append("break;\n");
description.addFix(SuggestedFix.replace(idx, idx, sb.toString()));

description.addFix(SuggestedFix.replace(idx, idx, "default: break;\n"));
}

/**
Expand All @@ -76,11 +112,10 @@ public Description matchSwitch(SwitchTree tree, VisitorState state) {
* <p>Examples:
* <ul>
* <li>Non-exhaustive switch, expected cases for: FOO
* <li>Non-exhaustive switch, expected cases for: FOO, BAR, BAZ, and 42 others. Did you mean to
* include a 'default' case?
* <li>Non-exhaustive switch, expected cases for: FOO, BAR, BAZ, and 42 others.
* </ul>
*/
private String buildMessage(LinkedHashSet<String> unhandled) {
private String buildMessage(Set<String> unhandled) {
final int maxCasesToPrint = 5;

StringBuilder message = new StringBuilder("Non-exhaustive switch, expected cases for: ");
Expand All @@ -99,10 +134,7 @@ private String buildMessage(LinkedHashSet<String> unhandled) {
}

if (tooManyCasesToPrint) {
message.append(String.format(", and %d others. Did you mean to include a 'default' case?",
unhandled.size() - numberToShow));
} else {
message.append(".");
message.append(String.format(", and %d others", unhandled.size() - numberToShow));
}
return message.toString();
}
Expand All @@ -119,13 +151,6 @@ private static LinkedHashSet<String> collectEnumSwitchCases(SwitchTree tree) {
return cases;
}

/** Return the difference of sets ax and bx. */
private static <T> LinkedHashSet<T> setDifference(LinkedHashSet<T> ax, LinkedHashSet<T> bx) {
LinkedHashSet<T> result = new LinkedHashSet<>(ax);
result.removeAll(bx);
return result;
}

/** Return true if the switch has a 'default' case. */
private static boolean hasDefaultCase(SwitchTree tree) {
for (CaseTree caseTree : tree.getCases()) {
Expand Down
Expand Up @@ -30,7 +30,7 @@ public abstract class Replacement {
public static Replacement create(int startPosition, int endPosition, String replaceWith) {
checkArgument(
0 <= startPosition && startPosition <= endPosition,
"Illegal range [%s, %s]",
"Illegal range [%s, %s)",
startPosition,
endPosition);
return new AutoValue_Replacement(startPosition, endPosition, replaceWith);
Expand Down
Expand Up @@ -257,6 +257,7 @@ public static ScannerSupplier errorChecks() {
EqualsHashCode.class,
Finally.class,
IncompatibleModifiersChecker.class,
MissingCasesInEnumSwitch.class,
MultipleTopLevelClasses.class,
NonAtomicVolatileUpdate.class,
NonOverridingEquals.class,
Expand Down Expand Up @@ -313,7 +314,6 @@ public static ScannerSupplier errorChecks() {
JUnitAmbiguousTestClass.class,
LockMethodChecker.class,
MalformedFormatString.class,
MissingCasesInEnumSwitch.class,
MissingFail.class,
MissingOverride.class,
ModifyingCollectionWithItself.class,
Expand Down
Expand Up @@ -106,16 +106,14 @@ public void testNonExhaustive_manyCases() throws Exception {
" enum Case { ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT }",
" void m(Case c) {",
" // BUG: Diagnostic contains:",
" // Non-exhaustive switch, expected cases for: TWO, THREE, FOUR, and 4 others."
+ " Did you mean to include a 'default' case?",
" // Non-exhaustive switch, expected cases for: TWO, THREE, FOUR, and 4 others",
" switch (c) {",
" case ONE:",
" System.err.println(\"found it!\");",
" break;",
" }",
" }",
"}"
)
"}")
.doTest();
}

Expand All @@ -136,4 +134,22 @@ public void testNonExhaustive_nonEnum() throws Exception {
"}")
.doTest();
}

@Test
public void empty() throws Exception {
compilationHelper
.addSourceLines(
"Test.java",
"class Test {",
" enum Case { ONE, TWO }",
" void m(Case e) {",
" // BUG: Diagnostic contains:",
" // mean 'default: throw new AssertionError(\"unexpected case: \" + e);'"
+ " or 'case ONE: case TWO: break;' or 'default: break;'?",
" switch (e) {",
" }",
" }",
"}")
.doTest();
}
}

0 comments on commit a277aef

Please sign in to comment.