Skip to content

Commit

Permalink
Error prone RedundantModifier check supports more interface components
Browse files Browse the repository at this point in the history
```diff
public interface Iface {
-  public static void a() {}
+  static void a() {}
}
```

```diff
public interface Iface {
-  public static final int VALUE = 1;
+  int VALUE = 1;
}
```
  • Loading branch information
carterkozak committed Nov 4, 2019
1 parent e877003 commit 07cc7a8
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.ModifiersTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;

import java.util.Locale;
import javax.lang.model.element.Modifier;

Expand All @@ -45,7 +47,7 @@
severity = BugPattern.SeverityLevel.ERROR,
summary = "Avoid using redundant modifiers")
public final class RedundantModifier extends BugChecker
implements BugChecker.ClassTreeMatcher, BugChecker.MethodTreeMatcher {
implements BugChecker.ClassTreeMatcher, BugChecker.MethodTreeMatcher, BugChecker.VariableTreeMatcher {

private static final Matcher<ClassTree> STATIC_ENUM_OR_INTERFACE = Matchers.allOf(
Matchers.anyOf(Matchers.kindIs(Tree.Kind.ENUM), Matchers.kindIs(Tree.Kind.INTERFACE)),
Expand All @@ -66,6 +68,19 @@ public final class RedundantModifier extends BugChecker
Matchers.not(Matchers.hasModifier(Modifier.DEFAULT)),
Matchers.anyOf(methodHasExplicitModifier(Modifier.PUBLIC), methodHasExplicitModifier(Modifier.ABSTRACT)));

private static final Matcher<MethodTree> INTERFACE_STATIC_METHOD_MODIFIERS = Matchers.allOf(
Matchers.enclosingClass(Matchers.kindIs(Tree.Kind.INTERFACE)),
Matchers.isStatic(),
methodHasExplicitModifier(Modifier.PUBLIC));

private static final Matcher<VariableTree> INTERFACE_FIELD_MODIFIERS = Matchers.allOf(
Matchers.enclosingClass(Matchers.kindIs(Tree.Kind.INTERFACE)),
Matchers.isStatic(),
Matchers.anyOf(
variableHasExplicitModifier(Modifier.PUBLIC),
variableHasExplicitModifier(Modifier.STATIC),
variableHasExplicitModifier(Modifier.FINAL)));

private static final Matcher<MethodTree> UNNECESSARY_FINAL_METHOD_ON_FINAL_CLASS = Matchers.allOf(
Matchers.not(Matchers.isStatic()),
Matchers.enclosingClass(Matchers.allOf(
Expand Down Expand Up @@ -108,6 +123,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.")
.addFix(SuggestedFixes.removeModifiers(tree, state, Modifier.PUBLIC))
.build();
}
if (UNNECESSARY_FINAL_METHOD_ON_FINAL_CLASS.matches(tree, state)) {
return buildDescription(tree)
.setMessage("Redundant 'final' modifier on an instance method of a final class.")
Expand All @@ -117,6 +138,19 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
return Description.NO_MATCH;
}

@Override
public Description matchVariable(VariableTree tree, VisitorState state) {
if (INTERFACE_FIELD_MODIFIERS.matches(tree, state)) {
return buildDescription(tree)
.setMessage("Interface fields are public, static, and final by default. "
+ "These modifiers are unnecessary to specify.")
.addFix(SuggestedFixes.removeModifiers(
tree, state, Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL))
.build();
}
return Description.NO_MATCH;
}

private static Matcher<MethodTree> methodHasExplicitModifier(Modifier modifier) {
return (Matcher<MethodTree>) (methodTree, state) ->
containsModifier(methodTree.getModifiers(), state, modifier);
Expand All @@ -127,6 +161,11 @@ private static Matcher<ClassTree> classHasExplicitModifier(Modifier modifier) {
containsModifier(classTree.getModifiers(), state, modifier);
}

private static Matcher<VariableTree> variableHasExplicitModifier(Modifier modifier) {
return (Matcher<VariableTree>) (variableTree, state) ->
containsModifier(variableTree.getModifiers(), state, modifier);
}

private static boolean containsModifier(ModifiersTree tree, VisitorState state, Modifier modifier) {
if (!tree.getFlags().contains(modifier)) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,50 @@ void allowStaticMethods() {
).doTest();
}

@Test
void fixInterfacePublicStaticMethod() {
fix()
.addInputLines(
"Test.java",
"public interface Test {",
" public static void a() {}",
"}"
)
.addOutputLines(
"Test.java",
"public interface Test {",
" static void a() {}",
"}"
).doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
}

@Test
void fixInterfaceStaticFieldModifiers() {
fix()
.addInputLines(
"Test.java",
"public interface Test {",
" int VALUE_0 = 0;",
" public static final int VALUE_1 = 1;",
" public final int VALUE_2 = 2;",
" public static int VALUE_3 = 3;",
" final int VALUE_4 = 4;",
" static int VALUE_5 = 5;",
"}"
)
.addOutputLines(
"Test.java",
"public interface Test {",
" int VALUE_0 = 0;",
" int VALUE_1 = 1;",
" int VALUE_2 = 2;",
" int VALUE_3 = 3;",
" int VALUE_4 = 4;",
" int VALUE_5 = 5;",
"}"
).doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
}

private RefactoringValidator fix() {
return RefactoringValidator.of(new RedundantModifier(), getClass());
}
Expand Down

0 comments on commit 07cc7a8

Please sign in to comment.