Skip to content

Commit

Permalink
Implement BracesRequired error-prone check with suggested fixes (#1130)
Browse files Browse the repository at this point in the history 
Implement BracesRequired error-prone check with suggested fixes
```diff
- if (condition) statement;
+ if (condition) {
+   statement;
+ }
```
  • Loading branch information
@carterkozak @bulldozer-bot
carterkozak authored and bulldozer-bot[bot] committed Dec 18, 2019
1 parent 0014d13 commit a0f9d09
Show file tree
Hide file tree
Showing 5 changed files with 355 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ 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.

### Programmatic Application

Expand Down
94 changes: 94 additions & 0 deletions baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/BracesRequired.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* (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,
providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION,
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());
}
}
}
247 changes: 247 additions & 0 deletions baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/BracesRequiredTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
/*
* (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<String> list) {",
" for (String item : list) System.out.println(item);",
" }",
"}")
.addOutputLines("Test.java",
"import java.util.List;",
"class Test {",
" void f(List<String> 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(new BracesRequired(), getClass());
}
}
12 changes: 12 additions & 0 deletions changelog/@unreleased/pr-1130.v2.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
type: improvement
improvement:
description: |-
Implement BracesRequired error-prone check with suggested fixes
```diff
- if (condition) statement;
+ if (condition) {
+ statement;
+ }
```
links:
- https://github.com/palantir/gradle-baseline/pull/1130
1 change: 1 addition & 0 deletions ...ne-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
public class BaselineErrorProneExtension {
private static final ImmutableList<String> DEFAULT_PATCH_CHECKS = ImmutableList.of(
// Baseline checks
"BracesRequired",
"CatchBlockLogException",
// TODO(ckozak): re-enable pending scala check
//"CatchSpecificity",
Expand Down

0 comments on commit a0f9d09

Please sign in to comment.