Add new @Scopemetadata to generated factories to store fqn of scope.

This is useful so that we can skip superficial validation of annotations that are not scopes when validating inject types in subsequent compilation units.

RELNOTES=Fixes missing scopes described in #3136. However, this change only fixes one case of missing scopes on @Inject types -- there are a number of other cases that still need to be fixed for this bug to actually be closed.
PiperOrigin-RevId: 421042966

java/dagger/internal/ScopeMetadata.java

@@ -0,0 +1,31 @@
+/*
+ * 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 dagger.internal;
+
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.CLASS;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/** Stores the scope information about a type after it's been processed. */
+@Retention(CLASS)
+@Target(TYPE)
+public @interface ScopeMetadata {
+  /** The qualified name of the scope, if one exists, otherwise an empty string. */
+  String value() default "";
+}

java/dagger/internal/codegen/binding/BindingFactory.java

@@ -86,17 +86,20 @@ public final class BindingFactory {
   private final KeyFactory keyFactory;
   private final DependencyRequestFactory dependencyRequestFactory;
   private final InjectionSiteFactory injectionSiteFactory;
+  private final InjectionAnnotations injectionAnnotations;
 
   @Inject
   BindingFactory(
       DaggerTypes types,
       KeyFactory keyFactory,
       DependencyRequestFactory dependencyRequestFactory,
-      InjectionSiteFactory injectionSiteFactory) {
+      InjectionSiteFactory injectionSiteFactory,
+      InjectionAnnotations injectionAnnotations) {
     this.types = types;
     this.keyFactory = keyFactory;
     this.dependencyRequestFactory = dependencyRequestFactory;
     this.injectionSiteFactory = injectionSiteFactory;
+    this.injectionAnnotations = injectionAnnotations;
   }
 
   /**
@@ -145,7 +148,7 @@ public ProvisionBinding injectionBinding(
                 constructorElement.hasAnnotation(TypeNames.ASSISTED_INJECT)
                     ? ASSISTED_INJECTION
                     : INJECTION)
-            .scope(uniqueScopeOf(constructorElement.getEnclosingElement()));
+            .scope(injectionAnnotations.getScope(constructorElement));
 
     if (hasNonDefaultTypeParameters(enclosingType)) {
       builder.unresolved(injectionBinding(constructorElement, Optional.empty()));

java/dagger/internal/codegen/validation/DaggerSuperficialValidation.java → java/dagger/internal/codegen/binding/DaggerSuperficialValidation.java

@@ -14,12 +14,13 @@
  * limitations under the License.
  */
 
-package dagger.internal.codegen.validation;
+package dagger.internal.codegen.binding;
 
 import static androidx.room.compiler.processing.compat.XConverters.toJavac;
 import static com.google.common.collect.Lists.reverse;
 import static java.util.stream.Collectors.joining;
 
+import androidx.room.compiler.processing.XAnnotation;
 import androidx.room.compiler.processing.XElement;
 import androidx.room.compiler.processing.XExecutableElement;
 import androidx.room.compiler.processing.XTypeElement;
@@ -128,6 +129,42 @@ private static void validateAnnotationsOf(Element element) {
     }
   }
 
+  /**
+   * Strictly validates the given annotation belonging to the given element.
+   *
+   * <p>This validation is considered "strict" because it will fail if an annotation's type is not
+   * valid. This fixes the bug described in b/213880825, but cannot be applied indiscriminately or
+   * it would break in many cases where we don't care about an annotation.
+   *
+   * <p>Note: This method does not actually check that the given annotation belongs to the given
+   * element. The element is only used to given better context to the error message in the case that
+   * validation fails.
+   */
+  public static void strictValidateAnnotationsOf(XElement element) {
+    element.getAllAnnotations()
+        .forEach(annotation -> strictValidateAnnotationOf(element, annotation));
+  }
+
+  /**
+   * Strictly validates the given annotation belonging to the given element.
+   *
+   * <p>This validation is considered "strict" because it will fail if an annotation's type is not
+   * valid. This fixes the bug described in b/213880825, but cannot be applied indiscriminately or
+   * it would break in many cases where we don't care about an annotation.
+   *
+   * <p>Note: This method does not actually check that the given annotation belongs to the given
+   * element. The element is only used to given better context to the error message in the case that
+   * validation fails.
+   */
+  public static void strictValidateAnnotationOf(XElement element, XAnnotation annotation) {
+    try {
+      strictValidateAnnotation(toJavac(annotation));
+    } catch (RuntimeException exception) {
+      throw ValidationException.from(exception)
+          .append(String.format("%s element: %s", toJavac(element).getKind(), element));
+    }
+  }
+
   /**
    * Returns true if all of the given elements return true from {@link #validateElement(Element)}.
    */
@@ -305,6 +342,29 @@ public static void validateAnnotations(Iterable<? extends AnnotationMirror> anno
     }
   }
 
+  /**
+   * This validation is the same as {@link #validateAnnotation(AnnotationMirror)} but also
+   * validates the annotation's kind directly to look for {@link TypeKind.ERROR} types.
+   *
+   * See b/213880825.
+   */
+  // TODO(bcorso): Merge this into the normal validateAnnotation() method. For now, this method is
+  // separated to avoid breaking existing usages that aren't setup to handle this extra validation.
+  private static void strictValidateAnnotation(AnnotationMirror annotationMirror) {
+    try {
+      validateType("annotation type", annotationMirror.getAnnotationType());
+      // There's a bug in TypeVisitor specifically when validating annotation types which will
+      // visit the visitDeclared() method rather than visitError() even when it's an ERROR kind.
+      // Thus, we check the kind directly here and fail validation if it's an ERROR kind.
+      if (annotationMirror.getAnnotationType().getKind() == TypeKind.ERROR) {
+        throw ValidationException.create();
+      }
+      validateAnnotationValues(annotationMirror.getElementValues());
+    } catch (RuntimeException exception) {
+      throw ValidationException.from(exception).append("annotation: " + annotationMirror);
+    }
+  }
+
   public static void validateAnnotation(AnnotationMirror annotationMirror) {
     try {
       validateType("annotation type", annotationMirror.getAnnotationType());

java/dagger/internal/codegen/binding/InjectionAnnotations.java

@@ -20,10 +20,15 @@
 import static androidx.room.compiler.processing.compat.XConverters.toXProcessing;
 import static com.google.auto.common.MoreElements.asType;
 import static com.google.auto.common.MoreElements.asVariable;
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.Iterables.getOnlyElement;
 import static dagger.internal.codegen.base.MoreAnnotationValues.getStringValue;
+import static dagger.internal.codegen.binding.SourceFiles.factoryNameForElement;
 import static dagger.internal.codegen.binding.SourceFiles.memberInjectedFieldSignatureForVariable;
 import static dagger.internal.codegen.binding.SourceFiles.membersInjectorNameForType;
+import static dagger.internal.codegen.extension.DaggerCollectors.toOptional;
 import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet;
 import static dagger.internal.codegen.langmodel.DaggerElements.getAnnotationMirror;
 import static dagger.internal.codegen.langmodel.DaggerElements.isAnnotationPresent;
@@ -42,11 +47,14 @@
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableSet;
+import dagger.internal.codegen.base.Scopes;
 import dagger.internal.codegen.extension.DaggerCollectors;
 import dagger.internal.codegen.extension.DaggerStreams;
 import dagger.internal.codegen.javapoet.TypeNames;
 import dagger.internal.codegen.kotlin.KotlinMetadataUtil;
 import dagger.internal.codegen.langmodel.DaggerElements;
+import dagger.spi.model.DaggerAnnotation;
+import dagger.spi.model.Scope;
 import java.util.Optional;
 import java.util.stream.Stream;
 import javax.inject.Inject;
@@ -77,6 +85,61 @@ public final class InjectionAnnotations {
     this.kotlinMetadataUtil = kotlinMetadataUtil;
   }
 
+  /**
+   * Returns the scope on the inject constructor's type if it exists.
+   *
+   * @throws IllegalArgumentException if the given constructor is not an inject constructor or there
+   *   are more than one scope annotations present.
+   */
+  public Optional<Scope> getScope(XConstructorElement injectConstructor) {
+    return getScopes(injectConstructor).stream().collect(toOptional());
+  }
+
+  /**
+   * Returns the scopes on the inject constructor's type, or an empty set if none exist.
+   *
+   * @throws IllegalArgumentException if the given constructor is not an inject constructor.
+   */
+  public ImmutableSet<Scope> getScopes(XConstructorElement injectConstructor) {
+    checkArgument(injectConstructor.hasAnyAnnotation(TypeNames.INJECT, TypeNames.ASSISTED_INJECT));
+    XTypeElement factory = processingEnv.findTypeElement(factoryNameForElement(injectConstructor));
+    if (factory != null && factory.hasAnnotation(TypeNames.SCOPE_METADATA)) {
+      String scopeName = factory.getAnnotation(TypeNames.SCOPE_METADATA).getAsString("value");
+      if (scopeName.isEmpty()) {
+        return ImmutableSet.of();
+      }
+      ImmutableSet<XAnnotation> scopeAnnotations =
+          injectConstructor.getEnclosingElement().getAllAnnotations().stream()
+              .filter(
+                  annotation ->
+                      scopeName.contentEquals(
+                          annotation.getType().getTypeElement().getQualifiedName()))
+              .collect(toImmutableSet());
+      checkState(
+          !scopeAnnotations.isEmpty(),
+          "Expected scope, %s, on inject type, %s.",
+          scopeName,
+          injectConstructor.getEnclosingElement());
+      checkState(
+          scopeAnnotations.size() == 1,
+          "Expected a single scope, %s, on inject type, %s, but found multiple: %s",
+          scopeName,
+          injectConstructor.getEnclosingElement(),
+          scopeAnnotations);
+      XAnnotation scopeAnnotation = getOnlyElement(scopeAnnotations);
+      // Do superficial validation before we convert to a Scope, otherwise the @Scope annotation may
+      // appear to be missing from the annotation if it's no longer on the classpath.
+      DaggerSuperficialValidation.strictValidateAnnotationOf(
+          injectConstructor.getEnclosingElement(), scopeAnnotation);
+      return ImmutableSet.of(Scope.scope(DaggerAnnotation.from(scopeAnnotation)));
+    }
+
+    // Fall back to validating all annotations if the ScopeMetadata isn't available.
+    DaggerSuperficialValidation
+        .strictValidateAnnotationsOf(injectConstructor.getEnclosingElement());
+    return Scopes.scopesOf(injectConstructor.getEnclosingElement());
+  }
+
   public Optional<XAnnotation> getQualifier(XElement element) {
     return getQualifier(toJavac(element)).map(qualifier -> toXProcessing(qualifier, processingEnv));
   }

java/dagger/internal/codegen/javapoet/TypeNames.java

@@ -70,6 +70,7 @@ public final class TypeNames {
   public static final ClassName PROVIDER = ClassName.get("javax.inject", "Provider");
   public static final ClassName PROVIDER_OF_LAZY =
       ClassName.get("dagger.internal", "ProviderOfLazy");
+  public static final ClassName SCOPE_METADATA = ClassName.get("dagger.internal", "ScopeMetadata");
   public static final ClassName SET_FACTORY = ClassName.get("dagger.internal", "SetFactory");
   public static final ClassName SINGLE_CHECK = ClassName.get("dagger.internal", "SingleCheck");
   public static final ClassName LAZY = ClassName.get("dagger", "Lazy");

java/dagger/internal/codegen/validation/InjectValidator.java

@@ -40,6 +40,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.squareup.javapoet.ClassName;
 import dagger.internal.codegen.base.ClearableCache;
+import dagger.internal.codegen.binding.DaggerSuperficialValidation;
 import dagger.internal.codegen.binding.InjectionAnnotations;
 import dagger.internal.codegen.compileroption.CompilerOptions;
 import dagger.internal.codegen.javapoet.TypeNames;
@@ -251,8 +252,8 @@ private ValidationReport validateConstructor(XConstructorElement constructorElem
           constructorElement);
     }
 
-    DaggerSuperficialValidation.validateAnnotationsOf(enclosingElement);
-    ImmutableSet<Scope> scopes = scopesOf(enclosingElement);
+    // Note: superficial validation of the annotations is done as part of getting the scopes.
+    ImmutableSet<Scope> scopes = injectionAnnotations.getScopes(constructorElement);
     if (injectAnnotation.equals(TypeNames.ASSISTED_INJECT)) {
       for (Scope scope : scopes) {
         builder.addError(

java/dagger/internal/codegen/validation/TypeCheckingProcessingStep.java

@@ -29,7 +29,7 @@
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.Maps;
 import com.squareup.javapoet.ClassName;
-import dagger.internal.codegen.validation.DaggerSuperficialValidation.ValidationException;
+import dagger.internal.codegen.binding.DaggerSuperficialValidation.ValidationException;
 import java.util.Map;
 import java.util.Set;
 import javax.inject.Inject;

java/dagger/internal/codegen/writing/FactoryGenerator.java

@@ -50,6 +50,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
+import com.squareup.javapoet.AnnotationSpec;
 import com.squareup.javapoet.ClassName;
 import com.squareup.javapoet.CodeBlock;
 import com.squareup.javapoet.FieldSpec;
@@ -63,13 +64,16 @@
 import dagger.internal.codegen.binding.Binding;
 import dagger.internal.codegen.binding.ProvisionBinding;
 import dagger.internal.codegen.compileroption.CompilerOptions;
+import dagger.internal.codegen.javapoet.TypeNames;
 import dagger.internal.codegen.kotlin.KotlinMetadataUtil;
 import dagger.internal.codegen.langmodel.DaggerElements;
 import dagger.internal.codegen.langmodel.DaggerTypes;
 import dagger.internal.codegen.writing.InjectionMethods.InjectionSiteMethod;
 import dagger.internal.codegen.writing.InjectionMethods.ProvisionMethod;
 import dagger.spi.model.BindingKind;
+import dagger.spi.model.DaggerAnnotation;
 import dagger.spi.model.DependencyRequest;
+import dagger.spi.model.Scope;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -125,6 +129,11 @@ private TypeSpec.Builder factoryBuilder(ProvisionBinding binding) {
             .addModifiers(PUBLIC, FINAL)
             .addTypeVariables(bindingTypeElementTypeVariableNames(binding));
 
+    if (binding.kind() == BindingKind.INJECTION
+        || binding.kind() == BindingKind.ASSISTED_INJECTION) {
+      factoryBuilder.addAnnotation(scopeMetadataAnnotation(binding));
+    }
+
     factoryTypeName(binding).ifPresent(factoryBuilder::addSuperinterface);
     addConstructorAndFields(binding, factoryBuilder);
     factoryBuilder.addMethod(getMethod(binding));
@@ -292,6 +301,16 @@ private MethodSpec getMethod(ProvisionBinding binding) {
     return getMethod.build();
   }
 
+  private AnnotationSpec scopeMetadataAnnotation(ProvisionBinding binding) {
+    AnnotationSpec.Builder builder = AnnotationSpec.builder(TypeNames.SCOPE_METADATA);
+    binding.scope()
+        .map(Scope::scopeAnnotation)
+        .map(DaggerAnnotation::className)
+        .map(ClassName::canonicalName)
+        .ifPresent(scopeCanonicalName -> builder.addMember("value", "$S", scopeCanonicalName));
+    return builder.build();
+  }
+
   private static TypeName providedTypeName(ProvisionBinding binding) {
     return binding.contributedType().getTypeName();
   }

javatests/artifacts/dagger/build-tests/src/test/java/buildtests/TransitiveScopeTest.java

@@ -36,22 +36,24 @@ public class TransitiveScopeTest {
 
   @Test
   public void testTransitiveScope_WithImplementation() throws IOException {
-    BuildResult result = setupBuildWith("implementation");
-
-    // TODO(bcorso): This is a repro of https://github.com/google/dagger/issues/3136.
-    // Once the issue is fixed, this test case should fail to build.
-    assertThat(result.task(":app:assemble").getOutcome()).isEqualTo(SUCCESS);
-    assertThat(result.getOutput()).contains("@Inject library1.Foo(): UNSCOPED");
+    BuildResult result = setupRunnerWith("implementation").buildAndFail();
+    assertThat(result.getOutput()).contains("Task :app:compileJava FAILED");
+    // TODO(bcorso): Give more context about what couldn't be resolved once we've fixed the issue
+    // described in https://github.com/google/dagger/issues/2208.
+    assertThat(result.getOutput())
+        .contains(
+            "error: dagger.internal.codegen.ComponentProcessor was unable to process "
+                + "'app.MyComponent' because not all of its dependencies could be resolved.");
   }
 
   @Test
   public void testTransitiveScope_WithApi() throws IOException {
-    BuildResult result = setupBuildWith("api");
+    BuildResult result = setupRunnerWith("api").build();
     assertThat(result.task(":app:assemble").getOutcome()).isEqualTo(SUCCESS);
     assertThat(result.getOutput()).contains("@Inject library1.Foo(): SCOPED");
   }
 
-  private BuildResult setupBuildWith(String dependencyType) throws IOException {
+  private GradleRunner setupRunnerWith(String dependencyType) throws IOException {
     File projectDir = folder.getRoot();
     GradleModule.create(projectDir)
         .addSettingsFile(
@@ -190,7 +192,6 @@ private BuildResult setupBuildWith(String dependencyType) throws IOException {
 
     return GradleRunner.create()
         .withArguments("--stacktrace", "build")
-        .withProjectDir(projectDir)
-        .build();
+        .withProjectDir(projectDir);
   }
 }

javatests/artifacts/dagger/transitive-annotation-app/library1/src/main/java/library1/Foo.java

@@ -30,7 +30,7 @@
  * the classpath. In most cases, Dagger shouldn't care that the annotation isn't on the classpath
  */
 @Singleton
-// @MyTransitiveAnnotation(VALUE): Not supported on inject-types yet.
+@MyTransitiveAnnotation(VALUE)
 public final class Foo extends FooBase {
   @MyTransitiveAnnotation(VALUE)
   int nonDaggerField;

javatests/dagger/internal/codegen/AssistedFactoryTest.java

@@ -450,7 +450,11 @@ public void testFactoryGeneratorDuplicatedParamNames() {
     JavaFileObject generatedSrc =
         compilerMode
             .javaFileBuilder("test.DaggerTestComponent")
-            .addLines("package test;", "", GeneratedLines.generatedAnnotations())
+            .addLines(
+                "package test;",
+                "",
+                "@ScopeMetadata",
+                GeneratedLines.generatedAnnotations())
             .addLinesIn(
                 FAST_INIT_MODE,
                 "public final class Foo_Factory {",