-
Notifications
You must be signed in to change notification settings - Fork 745
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Prevent user-written code from referring to Dagger-generated implemen…
…tation 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
- Loading branch information
1 parent
f4cded3
commit 5ad37b7
Showing
3 changed files
with
276 additions
and
0 deletions.
There are no files selected for viewing
125 changes: 125 additions & 0 deletions
125
.../src/main/java/com/google/errorprone/bugpatterns/inject/dagger/RefersToDaggerCodegen.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<String> DAGGER_INTERNAL_PACKAGES = | ||
ImmutableSet.of( | ||
"dagger.internal", | ||
"dagger.producers.internal", | ||
"dagger.producers.monitoring.internal", | ||
"dagger.android.internal"); | ||
private static final ImmutableSet<String> 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<Attribute> valuesList = (List<Attribute>) valueAttribute.getValue(); | ||
return valuesList.size() == 1 | ||
&& getOnlyElement(valuesList) | ||
.getValue() | ||
.equals("dagger.internal.codegen.ComponentProcessor"); | ||
} | ||
} | ||
return false; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
149 changes: 149 additions & 0 deletions
149
.../test/java/com/google/errorprone/bugpatterns/inject/dagger/RefersToDaggerCodegenTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<RefersToDaggerCodegenTest.MembersInjectedClass> membersInjector", | ||
" = null;", | ||
" membersInjector.injectMembers(null);", // allowed | ||
" }", | ||
"}") | ||
.doTest(); | ||
} | ||
} |