From 8eae200f20d715da5afe72d25e1f0ccdb5a98a5f Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Sun, 1 Jan 2023 07:55:07 -0800 Subject: [PATCH] Discourage using labels on non-loop statements PiperOrigin-RevId: 498879570 --- .../bugpatterns/LabelledBreakTarget.java | 44 +++++++++++ .../scanner/BuiltInCheckerSuppliers.java | 2 + .../bugpatterns/LabelledBreakTargetTest.java | 74 +++++++++++++++++++ docs/bugpattern/LabelledBreakTarget.md | 2 + 4 files changed, 122 insertions(+) create mode 100644 core/src/main/java/com/google/errorprone/bugpatterns/LabelledBreakTarget.java create mode 100644 core/src/test/java/com/google/errorprone/bugpatterns/LabelledBreakTargetTest.java create mode 100644 docs/bugpattern/LabelledBreakTarget.md diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/LabelledBreakTarget.java b/core/src/main/java/com/google/errorprone/bugpatterns/LabelledBreakTarget.java new file mode 100644 index 00000000000..0d4b02288ce --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/LabelledBreakTarget.java @@ -0,0 +1,44 @@ +/* + * Copyright 2023 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 com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.LabeledStatementTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.sun.source.tree.LabeledStatementTree; + +/** A BugPattern; see the summary. */ +@BugPattern(summary = "Labels should only be used on loops.", severity = WARNING) +public class LabelledBreakTarget extends BugChecker implements LabeledStatementTreeMatcher { + @Override + public Description matchLabeledStatement(LabeledStatementTree tree, VisitorState state) { + switch (tree.getStatement().getKind()) { + case DO_WHILE_LOOP: + case ENHANCED_FOR_LOOP: + case FOR_LOOP: + case WHILE_LOOP: + return NO_MATCH; + default: + break; + } + return describeMatch(tree); + } +} 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 29bdc3b9771..2cbbad725cf 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -197,6 +197,7 @@ import com.google.errorprone.bugpatterns.JavaLangClash; import com.google.errorprone.bugpatterns.JavaUtilDateChecker; import com.google.errorprone.bugpatterns.JdkObsolete; +import com.google.errorprone.bugpatterns.LabelledBreakTarget; import com.google.errorprone.bugpatterns.LambdaFunctionalInterface; import com.google.errorprone.bugpatterns.LenientFormatStringValidation; import com.google.errorprone.bugpatterns.LiteByteStringUtf8; @@ -909,6 +910,7 @@ public static ScannerSupplier errorChecks() { JodaPlusMinusLong.class, JodaTimeConverterManager.class, JodaWithDurationAddedLong.class, + LabelledBreakTarget.class, LiteEnumValueOf.class, LiteProtoToString.class, LockNotBeforeTry.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/LabelledBreakTargetTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/LabelledBreakTargetTest.java new file mode 100644 index 00000000000..ea88e66fe30 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/LabelledBreakTargetTest.java @@ -0,0 +1,74 @@ +/* + * Copyright 2023 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 com.google.errorprone.CompilationTestHelper; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** {@link LabelledBreakTarget}Test */ +@RunWith(JUnit4.class) +public class LabelledBreakTargetTest { + private final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance(LabelledBreakTarget.class, getClass()); + + @Test + public void positive() { + compilationHelper + .addSourceLines( + "Test.java", + "class Test {", + " void f() {", + " // BUG: Diagnostic contains:", + " FOO:", + " if (true) {", + " break FOO;", + " }", + " }", + "}") + .doTest(); + } + + @Test + public void negative() { + compilationHelper + .addSourceLines( + "Test.java", + "class Test {", + " void f(Iterable xs) {", + " ONE:", + " while (true) {", + " break ONE;", + " }", + " TWO:", + " do {", + " break TWO;", + " } while (true);", + " THREE:", + " for (var x : xs) {", + " break THREE;", + " }", + " FOUR:", + " for (int i = 0; i < 10; i++) {", + " break FOUR;", + " }", + " }", + "}") + .doTest(); + } +} diff --git a/docs/bugpattern/LabelledBreakTarget.md b/docs/bugpattern/LabelledBreakTarget.md new file mode 100644 index 00000000000..cc695fac7ec --- /dev/null +++ b/docs/bugpattern/LabelledBreakTarget.md @@ -0,0 +1,2 @@ +Labels should only be used on loops. For other statements, consider refactoring +to express the control flow without labelled breaks.