Implement BracesRequired error-prone check with suggested fixes #1130
Conversation
```diff
- if (condition) statement;
+ if (condition) {
+ statement;
+ }
```
Generate changelog in
|
|
Can we support the 'statement' being on the immediate line beneath the if? |
Yep, the auto-formatter takes care of that, we only need to build the right AST here :-) |
|
Fantastic. |
| public Description matchIf(IfTree tree, VisitorState state) { | ||
| check(tree.getThenStatement(), state); | ||
| check(tree.getElseStatement(), state); | ||
| return Description.NO_MATCH; |
There was a problem hiding this comment.
seems a little odd to always return NO_MATCH and rely on a side effect of check to actually do the fix
There was a problem hiding this comment.
Agreed, it's a bit odd. It allows us to keep the code cleaner and reusable without creating potentially unnecessary SuggestedFix.Builder objects for every conditional and loop. Several of the upstream checks are structured similarly.
baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/BracesRequiredTest.java
Show resolved
Hide resolved
|
👍 programming at scale |
|
Released 2.45.0 |
Before this PR
No automatic fix for missing braces.
After this PR
==COMMIT_MSG==
Implement BracesRequired error-prone check with suggested fixes
==COMMIT_MSG==
Possible downsides?
None, we already validate this using the default checkstyle configuration. The new check allows our automation to fix projects that don't yet comply.