Skip to content

Commit

Permalink
Disallow using traditional :-style switches as switch expressions
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 653064183
  • Loading branch information
cushon authored and Error Prone Team committed Jul 17, 2024
1 parent 2656f48 commit 385e43a
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 0 deletions.
25 changes: 25 additions & 0 deletions check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
Original file line number Diff line number Diff line change
Expand Up @@ -2808,6 +2808,31 @@ private static Method getCaseTreeGetExpressionsMethod() {
}
}

/**
* Returns true if the given {@link CaseTree} is in the form: {@code case <expression> ->
* <expression>}.
*/
public static boolean isRuleKind(CaseTree caseTree) {
Enum<?> kind;
try {
kind = (Enum<?>) GET_CASE_KIND_METHOD.invoke(caseTree);
} catch (ReflectiveOperationException e) {
return false;
}
return kind.name().equals("RULE");
}

private static final Method GET_CASE_KIND_METHOD = getGetCaseKindMethod();

@Nullable
private static Method getGetCaseKindMethod() {
try {
return CaseTree.class.getMethod("getCaseKind");
} catch (NoSuchMethodException e) {
return null;
}
}

/**
* Retrieves a stream containing all case expressions, in order, for a given {@code CaseTree}.
* This method acts as a facade to the {@code CaseTree.getExpressions()} API, falling back to
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright 2024 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.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.util.ASTHelpers.isRuleKind;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.matchers.Description;
import com.sun.source.tree.CaseTree;
import com.sun.source.tree.Tree;

/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
@BugPattern(summary = "Prefer -> switches for switch expressions", severity = WARNING)
public class TraditionalSwitchExpression extends BugChecker implements BugChecker.CaseTreeMatcher {

@Override
public Description matchCase(CaseTree tree, VisitorState state) {
if (isRuleKind(tree)) {
return NO_MATCH;
}
Tree parent = state.getPath().getParentPath().getLeaf();
if (!parent.getKind().name().equals("SWITCH_EXPRESSION")) {
return NO_MATCH;
}
return describeMatch(parent);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@
import com.google.errorprone.bugpatterns.ThrowsUncheckedException;
import com.google.errorprone.bugpatterns.ToStringReturnsNull;
import com.google.errorprone.bugpatterns.TooManyParameters;
import com.google.errorprone.bugpatterns.TraditionalSwitchExpression;
import com.google.errorprone.bugpatterns.TransientMisuse;
import com.google.errorprone.bugpatterns.TreeToString;
import com.google.errorprone.bugpatterns.TruthAssertExpected;
Expand Down Expand Up @@ -1075,6 +1076,7 @@ public static ScannerSupplier warningChecks() {
ThreeLetterTimeZoneID.class,
TimeUnitConversionChecker.class,
ToStringReturnsNull.class,
TraditionalSwitchExpression.class,
TruthAssertExpected.class,
TruthConstantAsserts.class,
TruthGetOrDefault.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Copyright 2024 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 org.junit.Assume.assumeTrue;

import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.util.RuntimeVersion;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public class TraditionalSwitchExpressionTest {

private final CompilationTestHelper testHelper =
CompilationTestHelper.newInstance(TraditionalSwitchExpression.class, getClass());

@Test
public void positive() {
assumeTrue(RuntimeVersion.isAtLeast14());
testHelper
.addSourceLines(
"Test.java",
"class Test {",
" int f(int i) {",
" // BUG: Diagnostic contains: Prefer -> switches for switch expressions",
" return switch (i) {",
" default:",
" yield -1;",
" };",
" }",
"}")
.doTest();
}

@Test
public void negativeStatement() {
assumeTrue(RuntimeVersion.isAtLeast14());
testHelper
.addSourceLines(
"Test.java",
"class Test {",
" void f(int i) {",
" switch (i) {",
" default:",
" return;",
" }",
" }",
"}")
.doTest();
}

@Test
public void negativeArrowStatement() {
assumeTrue(RuntimeVersion.isAtLeast14());
testHelper
.addSourceLines(
"Test.java",
"class Test {",
" void f(int i) {",
" switch (i) {",
" default -> System.err.println();",
" }",
" }",
"}")
.doTest();
}

@Test
public void negativeArrow() {
assumeTrue(RuntimeVersion.isAtLeast14());
testHelper
.addSourceLines(
"Test.java",
"class Test {",
" int f(int i) {",
" return switch (i) {",
" default -> -1;",
" };",
" }",
"}")
.doTest();
}
}

0 comments on commit 385e43a

Please sign in to comment.