From 7c299efed5c896c6b20ee8cc496cb2a854ac7594 Mon Sep 17 00:00:00 2001 From: ghm Date: Tue, 15 Feb 2022 06:06:27 -0800 Subject: [PATCH] UseBinds: don't add `abstract` to interface methods. And drive-by use the AST interfaces a bit more. PiperOrigin-RevId: 428760994 --- .../bugpatterns/inject/dagger/UseBinds.java | 48 +++++++++---------- .../inject/dagger/UseBindsTest.java | 26 ++++++++++ 2 files changed, 50 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/UseBinds.java b/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/UseBinds.java index 75fdc62009d..a48f58ad7d1 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/UseBinds.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/UseBinds.java @@ -42,23 +42,21 @@ import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.AnnotationTree; +import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.BlockTree; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodTree; +import com.sun.source.tree.ModifiersTree; import com.sun.source.tree.ReturnTree; import com.sun.source.tree.StatementTree; import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; import com.sun.source.tree.VariableTree; -import com.sun.tools.javac.code.Flags; -import com.sun.tools.javac.code.Flags.Flag; import com.sun.tools.javac.code.Symbol; -import com.sun.tools.javac.tree.JCTree.JCAnnotation; -import com.sun.tools.javac.tree.JCTree.JCAssign; -import com.sun.tools.javac.tree.JCTree.JCClassDecl; -import com.sun.tools.javac.tree.JCTree.JCExpression; -import com.sun.tools.javac.tree.JCTree.JCMethodDecl; -import com.sun.tools.javac.tree.JCTree.JCModifiers; import com.sun.tools.javac.util.Name; -import java.util.EnumSet; +import java.util.HashSet; import java.util.List; import java.util.Set; import javax.lang.model.element.Modifier; @@ -108,7 +106,7 @@ public Description matchMethod(MethodTree method, VisitorState state) { return NO_MATCH; } - JCClassDecl enclosingClass = ASTHelpers.findEnclosingNode(state.getPath(), JCClassDecl.class); + ClassTree enclosingClass = ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class); // Dagger 1 modules don't support @Binds. if (!IS_DAGGER_2_MODULE.matches(enclosingClass, state)) { @@ -134,32 +132,33 @@ public Description matchMethod(MethodTree method, VisitorState state) { } private Description fixByModifyingMethod( - VisitorState state, JCClassDecl enclosingClass, MethodTree method) { + VisitorState state, ClassTree enclosingClass, MethodTree method) { return describeMatch( method, SuggestedFix.builder() .addImport("dagger.Binds") - .merge(convertMethodToBinds(method, state)) + .merge(convertMethodToBinds(method, enclosingClass, state)) .merge(makeConcreteClassAbstract(enclosingClass, state)) .build()); } - private static SuggestedFix.Builder convertMethodToBinds(MethodTree method, VisitorState state) { + private static SuggestedFix.Builder convertMethodToBinds( + MethodTree method, ClassTree enclosingClass, VisitorState state) { SuggestedFix.Builder fix = SuggestedFix.builder(); - JCModifiers modifiers = ((JCMethodDecl) method).getModifiers(); + ModifiersTree modifiers = method.getModifiers(); ImmutableList.Builder modifierStringsBuilder = ImmutableList.builder().add("@Binds"); - for (JCAnnotation annotation : modifiers.annotations) { + for (AnnotationTree annotation : modifiers.getAnnotations()) { Name annotationQualifiedName = getSymbol(annotation).getQualifiedName(); if (annotationQualifiedName.contentEquals(PROVIDES_CLASS_NAME) || annotationQualifiedName.contentEquals(PRODUCES_CLASS_NAME)) { - List arguments = annotation.getArguments(); + List arguments = annotation.getArguments(); if (!arguments.isEmpty()) { - JCExpression argument = Iterables.getOnlyElement(arguments); + ExpressionTree argument = Iterables.getOnlyElement(arguments); checkState(argument.getKind().equals(ASSIGNMENT)); - JCAssign assignment = (JCAssign) argument; + AssignmentTree assignment = (AssignmentTree) argument; checkState(getSymbol(assignment.getVariable()).getSimpleName().contentEquals("type")); String typeName = getSymbol(assignment.getExpression()).getSimpleName().toString(); switch (typeName) { @@ -184,12 +183,13 @@ private static SuggestedFix.Builder convertMethodToBinds(MethodTree method, Visi } } - EnumSet methodFlags = ASTHelpers.asFlagSet(modifiers.flags); - methodFlags.remove(Flags.Flag.STATIC); - methodFlags.remove(Flags.Flag.FINAL); - methodFlags.add(Flags.Flag.ABSTRACT); - - for (Flag flag : methodFlags) { + Set methodFlags = new HashSet<>(modifiers.getFlags()); + methodFlags.remove(Modifier.STATIC); + methodFlags.remove(Modifier.FINAL); + if (!enclosingClass.getKind().equals(Kind.INTERFACE)) { + methodFlags.add(Modifier.ABSTRACT); + } + for (Modifier flag : methodFlags) { modifierStringsBuilder.add(flag.toString()); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/inject/dagger/UseBindsTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/inject/dagger/UseBindsTest.java index b40eff1f773..e7351415e2c 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/inject/dagger/UseBindsTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/inject/dagger/UseBindsTest.java @@ -82,6 +82,32 @@ public void staticProvidesMethod() { .doTest(); } + @Test + public void staticProvidesMethod_inInterface() { + testHelper + .addInputLines( + "in/Test.java", + "import java.security.SecureRandom;", + "import java.util.Random;", + "@" + moduleAnnotation, + "interface Test {", + " @" + bindingMethodAnnotation, + " static Random provideRandom(SecureRandom impl) {", + " return impl;", + " }", + "}") + .addOutputLines( + "out/Test.java", + "import dagger.Binds;", + "import java.security.SecureRandom;", + "import java.util.Random;", + "@" + moduleAnnotation, + "interface Test {", + " @Binds Random provideRandom(SecureRandom impl);", + "}") + .doTest(); + } + @Test public void intoSetMethod() { testHelper