From 5ad37b7a2efd231dff6e6acde406a6846dfe150b Mon Sep 17 00:00:00 2001 From: ronshapiro Date: Tue, 22 Jan 2019 12:53:49 -0800 Subject: [PATCH] Prevent user-written code from referring to Dagger-generated implementation code RELNOTES: New check that prevents user-written code from referring to Dagger-generated implementation code ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=230392954 --- .../inject/dagger/RefersToDaggerCodegen.java | 125 +++++++++++++++ .../scanner/BuiltInCheckerSuppliers.java | 2 + .../dagger/RefersToDaggerCodegenTest.java | 149 ++++++++++++++++++ 3 files changed, 276 insertions(+) create mode 100644 core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/RefersToDaggerCodegen.java create mode 100644 core/src/test/java/com/google/errorprone/bugpatterns/inject/dagger/RefersToDaggerCodegenTest.java diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/RefersToDaggerCodegen.java b/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/RefersToDaggerCodegen.java new file mode 100644 index 00000000000..f456ef1f4b1 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/RefersToDaggerCodegen.java @@ -0,0 +1,125 @@ +/* + * Copyright 2019 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.bugpatterns.inject.dagger; + +import static com.google.auto.common.AnnotationMirrors.getAnnotationValue; +import static com.google.common.collect.Iterables.getOnlyElement; +import static com.google.errorprone.util.ASTHelpers.getSymbol; + +import com.google.common.collect.ImmutableSet; +import com.google.errorprone.BugPattern; +import com.google.errorprone.BugPattern.SeverityLevel; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.tools.javac.code.Attribute; +import com.sun.tools.javac.code.Attribute.Compound; +import com.sun.tools.javac.code.Symbol.ClassSymbol; +import com.sun.tools.javac.code.Symbol.MethodSymbol; +import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.util.Name; +import java.util.List; +import javax.lang.model.element.AnnotationValue; + +/** + * Checks that the only code that refers to Dagger generated code is other Dagger generated code. + */ +@BugPattern( + name = "RefersToDaggerCodegen", + summary = "Don't refer to Dagger's internal or generated code", + severity = SeverityLevel.ERROR) +public final class RefersToDaggerCodegen extends BugChecker implements MethodInvocationTreeMatcher { + private static final ImmutableSet DAGGER_INTERNAL_PACKAGES = + ImmutableSet.of( + "dagger.internal", + "dagger.producers.internal", + "dagger.producers.monitoring.internal", + "dagger.android.internal"); + private static final ImmutableSet GENERATED_BASE_TYPES = + ImmutableSet.of("dagger.internal.Factory", "dagger.producers.internal.AbstractProducer"); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + MethodSymbol method = getSymbol(tree); + ClassSymbol rootClassOfMethod = method.outermostClass(); + + if (!isGeneratedFactoryType(rootClassOfMethod, state) + && !isMembersInjectionInvocation(method, state) + && !isDaggerInternalClass(rootClassOfMethod)) { + return Description.NO_MATCH; + } + + if (isAllowedToReferenceDaggerInternals(state)) { + return Description.NO_MATCH; + } + + return describeMatch(tree); + } + + private boolean isMembersInjectionInvocation(MethodSymbol method, VisitorState state) { + if (method.getSimpleName().contentEquals("injectMembers")) { + return false; + } + return isGeneratedBaseType(method.outermostClass(), state, "dagger.MembersInjector"); + } + + // TODO(ronshapiro): if we ever start emitting an annotation that has class retention, use that + // instead of checking for subtypes of generated code + private static boolean isGeneratedFactoryType(ClassSymbol symbol, VisitorState state) { + // TODO(ronshapiro): check annotation creators, inaccessible map key proxies, or inaccessible + // module constructor proxies? + return GENERATED_BASE_TYPES.stream() + .anyMatch(baseType -> isGeneratedBaseType(symbol, state, baseType)); + } + + private static boolean isGeneratedBaseType( + ClassSymbol symbol, VisitorState state, String baseTypeName) { + Type baseType = state.getTypeFromString(baseTypeName); + return ASTHelpers.isSubtype(symbol.asType(), baseType, state); + } + + private static boolean isDaggerInternalClass(ClassSymbol symbol) { + return DAGGER_INTERNAL_PACKAGES.contains(symbol.packge().getQualifiedName().toString()); + } + + private static boolean isAllowedToReferenceDaggerInternals(VisitorState state) { + ClassSymbol rootCallingClass = getSymbol(state.findEnclosing(ClassTree.class)).outermostClass(); + if (rootCallingClass.getQualifiedName().toString().startsWith("dagger.")) { + return true; + } + + for (Compound annotation : rootCallingClass.getAnnotationMirrors()) { + Name annotationName = + ((ClassSymbol) annotation.getAnnotationType().asElement()).getQualifiedName(); + if (annotationName.contentEquals("javax.annotation.Generated") + || annotationName.contentEquals("javax.annotation.processing.Generated")) { + AnnotationValue valueAttribute = getAnnotationValue(annotation, "value"); + @SuppressWarnings("unchecked") + List valuesList = (List) valueAttribute.getValue(); + return valuesList.size() == 1 + && getOnlyElement(valuesList) + .getValue() + .equals("dagger.internal.codegen.ComponentProcessor"); + } + } + return false; + } +} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index 4bf749f91ca..17982c0e750 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -336,6 +336,7 @@ import com.google.errorprone.bugpatterns.inject.dagger.EmptySetMultibindingContributions; import com.google.errorprone.bugpatterns.inject.dagger.PrivateConstructorForNoninstantiableModule; import com.google.errorprone.bugpatterns.inject.dagger.ProvidesNull; +import com.google.errorprone.bugpatterns.inject.dagger.RefersToDaggerCodegen; import com.google.errorprone.bugpatterns.inject.dagger.ScopeOnModule; import com.google.errorprone.bugpatterns.inject.dagger.UseBinds; import com.google.errorprone.bugpatterns.inject.guice.AssistedInjectScoping; @@ -547,6 +548,7 @@ public static ScannerSupplier errorChecks() { RandomModInteger.class, RandomCast.class, RectIntersectReturnValueIgnored.class, + RefersToDaggerCodegen.class, RestrictedApiChecker.class, ReturnValueIgnored.class, SelfAssignment.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/inject/dagger/RefersToDaggerCodegenTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/inject/dagger/RefersToDaggerCodegenTest.java new file mode 100644 index 00000000000..fd4840548ac --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/inject/dagger/RefersToDaggerCodegenTest.java @@ -0,0 +1,149 @@ +/* + * Copyright 2019 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.bugpatterns.inject.dagger; + +import com.google.errorprone.CompilationTestHelper; +import javax.inject.Inject; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link RefersToDaggerCodegen}. */ +@RunWith(JUnit4.class) +public class RefersToDaggerCodegenTest { + private static final String PACKAGE_NAME = InjectedClass.class.getPackage().getName(); + private static final String FULLY_QUALIFIED_FACTORY_NAME = + PACKAGE_NAME + ".RefersToDaggerCodegenTest_InjectedClass_Factory"; + + private CompilationTestHelper compilationTestHelper = + CompilationTestHelper.newInstance(RefersToDaggerCodegen.class, getClass()); + + static class InjectedClass { + @Inject + InjectedClass() {} + } + + public static class LooksLike_Factory { + public static LooksLike_Factory create() { + return new LooksLike_Factory(); + } + } + + public static class MembersInjectedClass { + @Inject Object object; + } + + @Test + public void testPositiveCase() { + compilationTestHelper + .addSourceLines( + "in/TestClass.java", + "import static " + FULLY_QUALIFIED_FACTORY_NAME + ".create;", + "import " + FULLY_QUALIFIED_FACTORY_NAME + ";", + "", + "class TestClass {", + " void TestClass() {", + " // BUG: Diagnostic contains: Dagger's internal or generated code", + " RefersToDaggerCodegenTest_InjectedClass_Factory.create();", + "", + " // BUG: Diagnostic contains: Dagger's internal or generated code", + " " + FULLY_QUALIFIED_FACTORY_NAME + ".create();", + "", + " // BUG: Diagnostic contains: Dagger's internal or generated code", + " create();", + "", + " // BUG: Diagnostic contains: Dagger's internal or generated code", + " RefersToDaggerCodegenTest_InjectedClass_Factory.newInstance();", + " }", + "}") + .doTest(); + } + + @Test + public void okToReferenceInDaggerCodegen() { + String generatedAnnotationName; + try { + Class.forName("java.lang.Module"); + generatedAnnotationName = "javax.annotation.processing.Generated"; + } catch (ClassNotFoundException e) { + generatedAnnotationName = "javax.annotation.Generated"; + } + compilationTestHelper + .addSourceLines( + "in/TestClass.java", + "import static " + FULLY_QUALIFIED_FACTORY_NAME + ".create;", + "import " + FULLY_QUALIFIED_FACTORY_NAME + ";", + "", + "@" + generatedAnnotationName + "(\"dagger.internal.codegen.ComponentProcessor\")", + "class TestClass {", + " void TestClass() {", + " RefersToDaggerCodegenTest_InjectedClass_Factory.create();", + "", + " " + FULLY_QUALIFIED_FACTORY_NAME + ".create();", + "", + " create();", + " }", + "}") + .doTest(); + } + + @Test + public void dontErrorOnCodeThatLooksLikeDaggerGeneration() { + String looksLikeFactoryFullyQualifiedName = + PACKAGE_NAME + ".RefersToDaggerCodegenTest.LooksLike_Factory"; + compilationTestHelper + .addSourceLines( + "in/TestClass.java", + "import static " + looksLikeFactoryFullyQualifiedName + ".create;", + "import " + looksLikeFactoryFullyQualifiedName + ";", + "", + "class TestClass {", + " void TestClass() {", + " LooksLike_Factory.create();", + "", + " " + looksLikeFactoryFullyQualifiedName + ".create();", + "", + " create();", + " }", + "}") + .doTest(); + } + + @Test + public void membersInjector() { + compilationTestHelper + .addSourceLines( + "in/TestClass.java", + "import dagger.MembersInjector;", + "import " + PACKAGE_NAME + ".RefersToDaggerCodegenTest;", + "import " + + PACKAGE_NAME + + ".RefersToDaggerCodegenTest_MembersInjectedClass_MembersInjector;", + "", + "class TestClass {", + " void TestClass() {", + " // BUG: Diagnostic contains: Dagger's internal or generated code", + " RefersToDaggerCodegenTest_MembersInjectedClass_MembersInjector.create(null);", + "", + " MembersInjector membersInjector", + " = null;", + " membersInjector.injectMembers(null);", // allowed + " }", + "}") + .doTest(); + } +}