Skip to content

Commit

Permalink
Avoids pre-validating the nearest enclosing type in the InjectProcess…
Browse files Browse the repository at this point in the history
…ingStep.

Fixes #3522

The pre-validation should no longer be needed as `InjectValidator` validates everything itself. In addition, the pre-validation is overly broad, and validates types that are not necessary for Dagger to validate the class.

RELNOTES=Fixes #3522: Avoids pre-validating the nearest enclosing type in the InjectProcessingStep.
PiperOrigin-RevId: 466812816
  • Loading branch information
bcorso authored and Dagger Team committed Aug 11, 2022
1 parent ea39850 commit ccecfee
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 1 deletion.
Expand Up @@ -53,6 +53,14 @@ public ImmutableSet<ClassName> annotationClassNames() {
return ImmutableSet.of(TypeNames.INJECT, TypeNames.INJECT_JAVAX, TypeNames.ASSISTED_INJECT);
}

// Override to avoid prevalidation. The InjectProcessingStep does all of the required validation
// within InjectValidator so there's no need to prevalidate the nearest enclosing type element.
// TODO(bcorso): Once all processing steps handle their own validation we can remove this.
@Override
protected boolean requiresPreValidation() {
return false;
}

@Override
protected void process(XElement injectElement, ImmutableSet<ClassName> annotations) {
// Only process an element once to avoid getting duplicate errors when an element is annotated
Expand Down
Expand Up @@ -75,7 +75,9 @@ public ImmutableSet<XElement> process(
// TODO(b/201479062): It's inefficient to require validation of the entire enclosing
// type, we should try to remove this and handle any additional validation into the
// steps that need it.
superficialValidator.throwIfNearestEnclosingTypeNotValid(element);
if (requiresPreValidation()) {
superficialValidator.throwIfNearestEnclosingTypeNotValid(element);
}
process((E) element, annotations);
} catch (TypeNotPresentException e) {
// TODO(bcorso): We should be able to remove this once we replace all calls to
Expand All @@ -97,6 +99,15 @@ public ImmutableSet<XElement> process(
return deferredElements.build();
}

/**
* Returns {@code true} if this processing step requires pre-validation of the annotated element's
* nearest enclosing type element.
*/
// TODO(bcorso): Once all processing steps handle their own validation we can remove this.
protected boolean requiresPreValidation() {
return true;
}

@Override
public void processOver(
XProcessingEnv env, Map<String, ? extends Set<? extends XElement>> elementsByAnnotation) {
Expand Down
@@ -0,0 +1,135 @@
/*
* Copyright (C) 2022 The Dagger 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 buildtests;

import static com.google.common.truth.Truth.assertThat;

import java.io.File;
import java.io.IOException;
import org.gradle.testkit.runner.BuildResult;
import org.gradle.testkit.runner.GradleRunner;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

// This is a regression test for https://github.com/google/dagger/issues/3522
@RunWith(JUnit4.class)
public class InjectClassNonDaggerMethodTest {
private static final String JAVAC_ERROR_MESSAGE =
"Foo.java:8: error: cannot find symbol";

private static final String DAGGER_ERROR_MESSAGE =
"InjectProcessingStep was unable to process 'Foo()' because 'MissingClass' could not be "
+ "resolved";

@Rule public TemporaryFolder folder = new TemporaryFolder();

@Test
public void testInjectMethod() throws IOException {
BuildResult result = setupRunner(/* useInject= */ true).buildAndFail();

// Assert that the inject method has both a javac error and a dagger error.
assertThat(result.getOutput()).contains("Task :library1:compileJava FAILED");
assertThat(result.getOutput()).contains(JAVAC_ERROR_MESSAGE);
assertThat(result.getOutput()).contains(DAGGER_ERROR_MESSAGE);
}

@Test
public void testNonInjectMethod() throws IOException {
BuildResult result = setupRunner(/* useInject= */ false).buildAndFail();

// Assert that the non-inject method has a javac error but not a dagger error.
assertThat(result.getOutput()).contains("Task :library1:compileJava FAILED");
assertThat(result.getOutput()).contains(JAVAC_ERROR_MESSAGE);
assertThat(result.getOutput()).doesNotContain(DAGGER_ERROR_MESSAGE);
}

private GradleRunner setupRunner(boolean useInject) throws IOException {
File projectDir = folder.getRoot();
GradleModule.create(projectDir)
.addSettingsFile(
"include 'app'",
"include 'library1'")
.addBuildFile(
"buildscript {",
" ext {",
String.format("dagger_version = \"%s\"", System.getProperty("dagger_version")),
String.format("kotlin_version = \"%s\"", System.getProperty("kotlin_version")),
" }",
"}",
"",
"allprojects {",
" repositories {",
" mavenCentral()",
" mavenLocal()",
" }",
"}");

GradleModule.create(projectDir, "app")
.addBuildFile(
"plugins {",
" id 'java'",
" id 'application'",
"}",
"dependencies {",
" implementation project(':library1')",
" implementation \"com.google.dagger:dagger:$dagger_version\"",
" annotationProcessor \"com.google.dagger:dagger-compiler:$dagger_version\"",
"}")
.addSrcFile(
"MyComponent.java",
"package app;",
"",
"import dagger.Component;",
"import library1.Foo;",
"",
"@Component",
"interface MyComponent {",
" Foo foo();",
"}");

GradleModule.create(projectDir, "library1")
.addBuildFile(
"plugins {",
" id 'java'",
" id 'java-library'",
"}",
"dependencies {",
" implementation \"com.google.dagger:dagger:$dagger_version\"",
" annotationProcessor \"com.google.dagger:dagger-compiler:$dagger_version\"",
"}")
.addSrcFile(
"Foo.java",
"package library1;",
"",
"import javax.inject.Inject;",
"",
"class Foo {",
" @Inject Foo() {}",
"",
useInject
? " @Inject void method(MissingClass missingClass) {}"
: " void method(MissingClass missingClass) {}",
"}");

return GradleRunner.create()
.withArguments("--stacktrace", "build")
.withProjectDir(projectDir);
}
}

0 comments on commit ccecfee

Please sign in to comment.