Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoids pre-validating the nearest enclosing type in the InjectProcessingStep. #3524

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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);
}
}