diff --git a/README.md b/README.md index 6ce7b20f8..4716659be 100644 --- a/README.md +++ b/README.md @@ -182,7 +182,6 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c - `ExceptionSpecificity`: Prefer more specific catch types than Exception and Throwable. - `ThrowSpecificity`: Prefer to declare more specific `throws` types than Exception and Throwable. - `UnsafeGaugeRegistration`: Use TaggedMetricRegistry.registerWithReplacement over TaggedMetricRegistry.gauge. -- `BracesRequired`: Require braces for loops and if expressions. - `CollectionStreamForEach`: Collection.forEach is more efficient than Collection.stream().forEach. - `LoggerEnclosingClass`: Loggers created using getLogger(Class) must reference their enclosing class. - `UnnecessaryLambdaArgumentParentheses`: Lambdas with a single parameter do not require argument parentheses. diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/BracesRequired.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/BracesRequired.java deleted file mode 100644 index 1ec8850b9..000000000 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/BracesRequired.java +++ /dev/null @@ -1,91 +0,0 @@ -/* - * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. - * - * 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.palantir.baseline.errorprone; - -import com.google.auto.service.AutoService; -import com.google.errorprone.BugPattern; -import com.google.errorprone.VisitorState; -import com.google.errorprone.bugpatterns.BugChecker; -import com.google.errorprone.fixes.SuggestedFix; -import com.google.errorprone.matchers.Description; -import com.sun.source.tree.DoWhileLoopTree; -import com.sun.source.tree.EnhancedForLoopTree; -import com.sun.source.tree.ForLoopTree; -import com.sun.source.tree.IfTree; -import com.sun.source.tree.StatementTree; -import com.sun.source.tree.Tree; -import com.sun.source.tree.WhileLoopTree; -import javax.annotation.Nullable; - -@AutoService(BugChecker.class) -@BugPattern( - name = "BracesRequired", - link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", - linkType = BugPattern.LinkType.CUSTOM, - severity = BugPattern.SeverityLevel.WARNING, - summary = "Braces are required for readability") -public final class BracesRequired extends BugChecker - implements BugChecker.DoWhileLoopTreeMatcher, - BugChecker.ForLoopTreeMatcher, - BugChecker.EnhancedForLoopTreeMatcher, - BugChecker.IfTreeMatcher, - BugChecker.WhileLoopTreeMatcher { - - @Override - public Description matchIf(IfTree tree, VisitorState state) { - check(tree.getThenStatement(), state); - StatementTree elseStatement = tree.getElseStatement(); - if (elseStatement != null - // Additional check for else if - && elseStatement.getKind() != Tree.Kind.IF) { - check(elseStatement, state); - } - return Description.NO_MATCH; - } - - @Override - public Description matchWhileLoop(WhileLoopTree tree, VisitorState state) { - check(tree.getStatement(), state); - return Description.NO_MATCH; - } - - @Override - public Description matchDoWhileLoop(DoWhileLoopTree tree, VisitorState state) { - check(tree.getStatement(), state); - return Description.NO_MATCH; - } - - @Override - public Description matchEnhancedForLoop(EnhancedForLoopTree tree, VisitorState state) { - check(tree.getStatement(), state); - return Description.NO_MATCH; - } - - @Override - public Description matchForLoop(ForLoopTree tree, VisitorState state) { - check(tree.getStatement(), state); - return Description.NO_MATCH; - } - - private void check(@Nullable StatementTree tree, VisitorState state) { - if (tree != null && tree.getKind() != Tree.Kind.BLOCK && tree.getKind() != Tree.Kind.EMPTY_STATEMENT) { - state.reportMatch(buildDescription(tree) - .addFix(SuggestedFix.replace(tree, "{" + state.getSourceForNode(tree) + "}")) - .build()); - } - } -} diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/BracesRequiredTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/BracesRequiredTest.java deleted file mode 100644 index 76d36ed90..000000000 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/BracesRequiredTest.java +++ /dev/null @@ -1,257 +0,0 @@ -/* - * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. - * - * 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.palantir.baseline.errorprone; - -import com.google.errorprone.BugCheckerRefactoringTestHelper; -import org.junit.jupiter.api.Test; - -public class BracesRequiredTest { - - @Test - void testFix_if_then() { - fix().addInputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " if (param) System.out.println();", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " if (param) {", - " System.out.println();", - " }", - " }", - "}") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - @Test - void testFix_if_else() { - fix().addInputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " if (param) {", - " System.out.println(\"if\");", - " } else", - " System.out.println(\"else\");", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " if (param) {", - " System.out.println(\"if\");", - " } else {", - " System.out.println(\"else\");", - " }", - " }", - "}") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - @Test - void testFix_if_both() { - fix().addInputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " if (param)", - " System.out.println(\"if\");", - " else", - " System.out.println(\"else\");", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " if (param) {", - " System.out.println(\"if\");", - " } else {", - " System.out.println(\"else\");", - " }", - " }", - "}") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - @Test - void testFix_if_emptyThen() { - fix().addInputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " if (param); else", - " System.out.println(\"else\");", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " if (param); else {", - " System.out.println(\"else\");", - " }", - " }", - "}") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - @Test - void testFix_while() { - fix().addInputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " while (param) System.out.println();", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " while (param) {", - " System.out.println();", - " }", - " }", - "}") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - @Test - void testFix_doWhile() { - fix().addInputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " do System.out.println(); while (param);", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " do {", - " System.out.println();", - " } while (param);", - " }", - "}") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - @Test - void testFix_for() { - fix().addInputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " for (int i = 0; i < 5; i++) System.out.println();", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " for (int i = 0; i < 5; i++) {", - " System.out.println();", - " }", - " }", - "}") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - @Test - void testFix_enhancedFor() { - fix().addInputLines( - "Test.java", - "import java.util.List;", - "class Test {", - " void f(List list) {", - " for (String item : list) System.out.println(item);", - " }", - "}") - .addOutputLines( - "Test.java", - "import java.util.List;", - "class Test {", - " void f(List list) {", - " for (String item : list) {", - " System.out.println(item);", - " }", - " }", - "}") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - @Test - void testFix_nested() { - fix().addInputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " if (param) if (param) {", - " System.out.println();", - " }", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test {", - " void f(boolean param) {", - " if (param) {", - " if (param) {", - " System.out.println();", - " }", - " }", - " }", - "}") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - @Test - void testFix_elseIf() { - fix().addInputLines( - "Test.java", - "class Test {", - " void f(boolean p0, boolean p1) {", - " if (p0) System.out.println();", - " else if (p1) System.out.println();", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test {", - " void f(boolean p0, boolean p1) {", - " if (p0) {", - " System.out.println();", - " } else if (p1) {", - " System.out.println();", - " }", - " }", - "}") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - private RefactoringValidator fix() { - return RefactoringValidator.of(BracesRequired.class, getClass()); - } -} diff --git a/changelog/@unreleased/pr-1766.v2.yml b/changelog/@unreleased/pr-1766.v2.yml new file mode 100644 index 000000000..b26bbd319 --- /dev/null +++ b/changelog/@unreleased/pr-1766.v2.yml @@ -0,0 +1,6 @@ +type: improvement +improvement: + description: Replace our `BracesRequired` check+fix with upstream `MissingBraces` + added in [v2.7.0](https://github.com/google/error-prone/releases/tag/v2.7.0) + links: + - https://github.com/palantir/gradle-baseline/pull/1766 diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java index 17959a41d..5cfcae1c0 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java @@ -28,7 +28,6 @@ public class BaselineErrorProneExtension { */ private static final ImmutableList DEFAULT_PATCH_CHECKS = ImmutableList.of( // Baseline checks - "BracesRequired", "CatchBlockLogException", // TODO(ckozak): re-enable pending scala check // "CatchSpecificity", @@ -75,6 +74,7 @@ public class BaselineErrorProneExtension { // Built-in checks "ArrayEquals", "BadImport", + "MissingBraces", "MissingOverride", "ObjectsHashCodePrimitive", "PreferJavaTimeOverload",