Error prone RedundantModifier check supports more interface components #1021
Conversation
Generate changelog in
|
carterkozak
force-pushed
the
ckozak/RedundantModifier_interface_components
branch
from
07cc7a8 to
0700dd3
Compare
|
I haven't had a chance to test this on a large internal project yet, will post here once I've verified it. |
|
Verified on a large internal project. |
```diff
public interface Iface {
- public static void a() {}
+ static void a() {}
}
```
```diff
public interface Iface {
- public static final int VALUE = 1;
+ int VALUE = 1;
}
```
carterkozak
force-pushed
the
ckozak/RedundantModifier_interface_components
branch
from
d0cba81 to
82042b8
Compare
|
Verified the new change on a large internal project as well. |
| @@ -45,38 +45,65 @@ | |||
| severity = BugPattern.SeverityLevel.ERROR, | |||
There was a problem hiding this comment.
i'm a tiny bit concerned that having this at ERROR level might introduce unnecessary friction when people just want to run things like ./gradlew test locally. We're just detecting redundant code, not actually 'wrong' in anywa way... given that excavator can come around and auto-fix, what do you think about putting this as suggestion?
There was a problem hiding this comment.
Agreed, I initially used error because this in some part replaces a checkstyle check which we verify premerge.
Given that robots can fix this for us, and it's not a correctness issue, I don't mind moving it to a suggestion, and allowing minor regressions until the robots sweep through.
| @@ -108,6 +135,12 @@ public Description matchMethod(MethodTree tree, VisitorState state) { | |||
| .addFix(SuggestedFixes.removeModifiers(tree, state, Modifier.PUBLIC, Modifier.ABSTRACT)) | |||
| .build(); | |||
| } | |||
| if (INTERFACE_STATIC_METHOD_MODIFIERS.matches(tree, state)) { | |||
| return buildDescription(tree) | |||
| .setMessage("Interface components are public by default. The 'public' modifier is unnecessary.") | |||
There was a problem hiding this comment.
interesting that java9 lets people define private methods in interfaces! https://bugs.openjdk.java.net/browse/JDK-8071453
This should be all fine though, because we're explicitly only detecting and fixing the public keyword.
There was a problem hiding this comment.
Yep, I designed this with JEP 213 in mind.
bulldozer-bot
bot
deleted the
ckozak/RedundantModifier_interface_components
branch
public interface Iface { - public static void a() {} + static void a() {} }public interface Iface { - public static final int VALUE = 1; + int VALUE = 1; }Before this PR
After this PR
==COMMIT_MSG==
Error prone RedundantModifier check supports interface static methods and fields.
==COMMIT_MSG==