Skip to content

Commit

Permalink
match nested classes in interfaces
Browse files Browse the repository at this point in the history
  • Loading branch information
@carterkozak
carterkozak committed Nov 5, 2019
1 parent 03a3f7e commit 82042b8
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 41 deletions.
60 changes: 60 additions & 0 deletions baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/MoreMatchers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,17 @@

package com.palantir.baseline.errorprone;

import com.google.errorprone.VisitorState;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.sun.source.tree.ClassTree;
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 com.sun.source.util.TreePath;
import java.util.Locale;
import javax.lang.model.element.Modifier;

/** Additional {@link Matcher} factory methods shared by baseline checks. */
final class MoreMatchers {
Expand Down Expand Up @@ -49,5 +57,57 @@ static <T extends Tree> Matcher<T> isSubtypeOf(String baseTypeString) {
Matchers.not(Matchers.kindIs(Tree.Kind.NULL_LITERAL)));
}

/**
* Matches enclosing classes on {@link ClassTree} blocks. This differs from
* {@link Matchers#enclosingClass(Matcher)} which matches the input {@link ClassTree},
* not the enclosing class.
*/
static <T extends ClassTree> Matcher<T> classEnclosingClass(Matcher<ClassTree> matcher) {
return (Matcher<T>) (classTree, state) -> {
TreePath currentPath = state.getPath().getParentPath();
while (currentPath != null) {
Tree leaf = currentPath.getLeaf();
if (leaf instanceof ClassTree) {
return matcher.matches((ClassTree) leaf, state);
}
currentPath = currentPath.getParentPath();
}
return false;
};
}

/**
* Works similarly to {@link Matchers#hasModifier(Modifier)}, but only matches elements
* which explicitly list the modifier. For example, all components nested in an interface
* are public by default, but they don't necessarily use the public keyword.
*/
static <T extends Tree> Matcher<T> hasExplicitModifier(Modifier modifier) {
return (Matcher<T>) (tree, state) -> {
if (tree instanceof ClassTree) {
return containsModifier(((ClassTree) tree).getModifiers(), state, modifier);
}
if (tree instanceof MethodTree) {
return containsModifier(((MethodTree) tree).getModifiers(), state, modifier);
}
if (tree instanceof VariableTree) {
return containsModifier(((VariableTree) tree).getModifiers(), state, modifier);
}
return false;
};
}

private static boolean containsModifier(ModifiersTree tree, VisitorState state, Modifier modifier) {
if (!tree.getFlags().contains(modifier)) {
return false;
}
String source = state.getSourceForNode(tree);
// getSourceForNode returns null when there are no modifiers specified
if (source == null) {
return false;
}
// nested interfaces report a static modifier despite not being present
return source.contains(modifier.name().toLowerCase(Locale.ENGLISH));
}

private MoreMatchers() {}
}
65 changes: 25 additions & 40 deletions baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/RedundantModifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.google.errorprone.matchers.Matchers;
import com.sun.source.tree.ClassTree;
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;
Expand All @@ -50,47 +49,61 @@ public final class RedundantModifier extends BugChecker

private static final Matcher<ClassTree> STATIC_ENUM_OR_INTERFACE = Matchers.allOf(
Matchers.anyOf(Matchers.kindIs(Tree.Kind.ENUM), Matchers.kindIs(Tree.Kind.INTERFACE)),
classHasExplicitModifier(Modifier.STATIC));
MoreMatchers.hasExplicitModifier(Modifier.STATIC));

private static final Matcher<MethodTree> PRIVATE_ENUM_CONSTRUCTOR = Matchers.allOf(
Matchers.methodIsConstructor(),
Matchers.enclosingClass(Matchers.kindIs(Tree.Kind.ENUM)),
methodHasExplicitModifier(Modifier.PRIVATE));
MoreMatchers.hasExplicitModifier(Modifier.PRIVATE));

private static final Matcher<MethodTree> STATIC_FINAL_METHOD = Matchers.allOf(
methodHasExplicitModifier(Modifier.STATIC),
methodHasExplicitModifier(Modifier.FINAL));
MoreMatchers.hasExplicitModifier(Modifier.STATIC),
MoreMatchers.hasExplicitModifier(Modifier.FINAL));

private static final Matcher<MethodTree> UNNECESSARY_INTERFACE_METHOD_MODIFIERS = Matchers.allOf(
Matchers.enclosingClass(Matchers.kindIs(Tree.Kind.INTERFACE)),
Matchers.not(Matchers.isStatic()),
Matchers.not(Matchers.hasModifier(Modifier.DEFAULT)),
Matchers.anyOf(methodHasExplicitModifier(Modifier.PUBLIC), methodHasExplicitModifier(Modifier.ABSTRACT)));
Matchers.anyOf(
MoreMatchers.hasExplicitModifier(Modifier.PUBLIC),
MoreMatchers.hasExplicitModifier(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));
MoreMatchers.hasExplicitModifier(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)));
MoreMatchers.hasExplicitModifier(Modifier.PUBLIC),
MoreMatchers.hasExplicitModifier(Modifier.STATIC),
MoreMatchers.hasExplicitModifier(Modifier.FINAL)));

private static final Matcher<ClassTree> INTERFACE_NESTED_CLASS_MODIFIERS = Matchers.allOf(
MoreMatchers.classEnclosingClass(Matchers.kindIs(Tree.Kind.INTERFACE)),
Matchers.anyOf(
MoreMatchers.hasExplicitModifier(Modifier.PUBLIC),
MoreMatchers.hasExplicitModifier(Modifier.STATIC)));

private static final Matcher<MethodTree> UNNECESSARY_FINAL_METHOD_ON_FINAL_CLASS = Matchers.allOf(
Matchers.not(Matchers.isStatic()),
Matchers.enclosingClass(Matchers.allOf(
Matchers.kindIs(Tree.Kind.CLASS),
classHasExplicitModifier(Modifier.FINAL))),
MoreMatchers.hasExplicitModifier(Modifier.FINAL))),
Matchers.allOf(
methodHasExplicitModifier(Modifier.FINAL),
MoreMatchers.hasExplicitModifier(Modifier.FINAL),
Matchers.not(Matchers.hasAnnotation(SafeVarargs.class))));

@Override
public Description matchClass(ClassTree tree, VisitorState state) {
if (INTERFACE_NESTED_CLASS_MODIFIERS.matches(tree, state)) {
return buildDescription(tree)
.setMessage("Types nested in interfaces are public and static by default.")
.addFix(SuggestedFixes.removeModifiers(tree, state, Modifier.PUBLIC, Modifier.STATIC))
.build();
}
if (STATIC_ENUM_OR_INTERFACE.matches(tree, state)) {
return buildDescription(tree)
.setMessage(tree.getKind().name().toLowerCase(Locale.ENGLISH)
Expand Down Expand Up @@ -149,32 +162,4 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
}
return Description.NO_MATCH;
}

private static Matcher<MethodTree> methodHasExplicitModifier(Modifier modifier) {
return (Matcher<MethodTree>) (methodTree, state) ->
containsModifier(methodTree.getModifiers(), state, modifier);
}

private static Matcher<ClassTree> classHasExplicitModifier(Modifier modifier) {
return (Matcher<ClassTree>) (classTree, state) ->
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;
}
String source = state.getSourceForNode(tree);
// getSourceForNode returns null when there are no modifiers specified
if (source == null) {
return false;
}
// nested interfaces report a static modifier despite not being present
return source.contains(modifier.name().toLowerCase(Locale.ENGLISH));
}
}
27 changes: 26 additions & 1 deletion ...ine-error-prone/src/test/java/com/palantir/baseline/errorprone/RedundantModifierTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ void allowNestedInterface() {
helper().addSourceLines(
"Enclosing.java",
"interface Enclosing {",
" public interface Test {",
" interface Test {",
" }",
"}"
).doTest();
Expand Down Expand Up @@ -326,6 +326,31 @@ void fixInterfaceStaticFieldModifiers() {
).doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
}

@Test
void fixInterfaceNestedPublicClass() {
fix()
.addInputLines(
"Test.java",
"public interface Test {",
" public static final class Class0 {}",
" public class Class1 {}",
" static class Class2 {}",
" final class Class3 {}",
" public interface Interface0 {}",
"}"
)
.addOutputLines(
"Test.java",
"public interface Test {",
" final class Class0 {}",
" class Class1 {}",
" class Class2 {}",
" final class Class3 {}",
" interface Interface0 {}",
"}"
).doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
}

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

0 comments on commit 82042b8

Please sign in to comment.