Suppress unchecked+rawtypes warnings on all Dagger generated code.

An internal change recently enable these warnings (which seem to have been turned off by default). This is causing spam for Dagger builds because we haven't traditionally been aware of the issues where this arises. This is a temporary stopgap measure to disable those warnings until we build infrastructure to determine how to locally-suppress this code. Also fix two of these warnings in our own test code. Fixes #1039 Fixes #1172 RELNOTES=n/a ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=251730395
google · Jun 6, 2019 · ce23333 · ce23333
1 parent adfa71d
commit ce23333
Show file tree

Hide file tree

Showing 30 changed files with 355 additions and 325 deletions.
diff --git a/java/dagger/internal/codegen/ProducerFactoryGenerator.java b/java/dagger/internal/codegen/ProducerFactoryGenerator.java
@@ -28,6 +28,7 @@
 import static dagger.internal.codegen.SourceFiles.generateBindingFieldsForDependencies;
 import static dagger.internal.codegen.SourceFiles.generatedClassNameForBinding;
 import static dagger.internal.codegen.SourceFiles.parameterizedGeneratedTypeNameForBinding;
+import static dagger.internal.codegen.javapoet.AnnotationSpecs.Suppression.FUTURE_RETURN_VALUE_IGNORED;
 import static dagger.internal.codegen.javapoet.AnnotationSpecs.Suppression.UNCHECKED;
 import static dagger.internal.codegen.javapoet.CodeBlocks.makeParametersCodeBlock;
 import static dagger.internal.codegen.javapoet.CodeBlocks.toParametersCodeBlock;
@@ -48,8 +49,8 @@
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
-import com.squareup.javapoet.AnnotationSpec;
 import com.squareup.javapoet.ClassName;
 import com.squareup.javapoet.CodeBlock;
 import com.squareup.javapoet.FieldSpec;
@@ -58,6 +59,7 @@
 import com.squareup.javapoet.TypeName;
 import com.squareup.javapoet.TypeSpec;
 import dagger.internal.codegen.javapoet.AnnotationSpecs;
+import dagger.internal.codegen.javapoet.AnnotationSpecs.Suppression;
 import dagger.internal.codegen.javapoet.TypeNames;
 import dagger.internal.codegen.langmodel.DaggerElements;
 import dagger.model.DependencyRequest;
@@ -115,12 +117,6 @@ Optional<TypeSpec.Builder> write(ClassName generatedTypeName, ProductionBinding
 
     TypeSpec.Builder factoryBuilder =
         classBuilder(generatedTypeName)
-            .addAnnotation(
-                // TODO(beder): examine if we can remove this or prevent subtypes of Future from
-                // being produced
-                AnnotationSpec.builder(SuppressWarnings.class)
-                    .addMember("value", "$S", "FutureReturnValueIgnored")
-                    .build())
             .addModifiers(PUBLIC, FINAL)
             .addTypeVariables(bindingTypeElementTypeVariableNames(binding));
 
@@ -550,4 +546,10 @@ private FluentIterable<? extends TypeName> getThrownTypeNames(
       Iterable<? extends TypeMirror> thrownTypes) {
     return FluentIterable.from(thrownTypes).transform(TypeName::get);
   }
+
+  @Override
+  protected ImmutableSet<Suppression> warningSuppressions() {
+    // TODO(beder): examine if we can remove this or prevent subtypes of Future from being produced
+    return ImmutableSet.of(FUTURE_RETURN_VALUE_IGNORED);
+  }
 }
diff --git a/java/dagger/internal/codegen/SourceFileGenerator.java b/java/dagger/internal/codegen/SourceFileGenerator.java
@@ -18,12 +18,17 @@
 
 import static com.google.auto.common.GeneratedAnnotations.generatedAnnotation;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static dagger.internal.codegen.javapoet.AnnotationSpecs.Suppression.RAWTYPES;
+import static dagger.internal.codegen.javapoet.AnnotationSpecs.Suppression.UNCHECKED;
 
 import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableSet;
 import com.squareup.javapoet.AnnotationSpec;
 import com.squareup.javapoet.ClassName;
 import com.squareup.javapoet.JavaFile;
 import com.squareup.javapoet.TypeSpec;
+import dagger.internal.codegen.javapoet.AnnotationSpecs;
+import dagger.internal.codegen.javapoet.AnnotationSpecs.Suppression;
 import dagger.internal.codegen.langmodel.DaggerElements;
 import java.util.Optional;
 import javax.annotation.processing.Filer;
@@ -33,8 +38,8 @@
 
 /**
  * A template class that provides a framework for properly handling IO while generating source files
- * from an annotation processor.  Particularly, it makes a best effort to ensure that files that
- * fail to write successfully are deleted.
+ * from an annotation processor. Particularly, it makes a best effort to ensure that files that fail
+ * to write successfully are deleted.
  *
  * @param <T> The input type from which source is to be generated.
  */
@@ -80,8 +85,7 @@ void generate(T input) throws SourceFileGenerationException {
       // if the code above threw a SFGE, use that
       Throwables.propagateIfPossible(e, SourceFileGenerationException.class);
       // otherwise, throw a new one
-      throw new SourceFileGenerationException(
-          Optional.empty(), e, originatingElement(input));
+      throw new SourceFileGenerationException(Optional.empty(), e, originatingElement(input));
     }
   }
 
@@ -97,6 +101,15 @@ private JavaFile buildJavaFile(
                         .addMember("comments", "$S", GENERATED_COMMENTS)
                         .build());
     generatedAnnotation.ifPresent(typeSpecBuilder::addAnnotation);
+
+    // TODO(b/134590785): remove this and only suppress annotations locally, if necessary
+    typeSpecBuilder.addAnnotation(
+        AnnotationSpecs.suppressWarnings(
+            ImmutableSet.<Suppression>builder()
+                .addAll(warningSuppressions())
+                .add(UNCHECKED, RAWTYPES)
+                .build()));
+
     JavaFile.Builder javaFileBuilder =
         JavaFile.builder(generatedTypeName.packageName(), typeSpecBuilder.build())
             .skipJavaLangImports(true);
@@ -106,9 +119,7 @@ private JavaFile buildJavaFile(
     return javaFileBuilder.build();
   }
 
-  /**
-   * Implementations should return the {@link ClassName} for the top-level type to be generated.
-   */
+  /** Implementations should return the {@link ClassName} for the top-level type to be generated. */
   abstract ClassName nameGeneratedType(T input);
 
   /** Returns the originating element of the generating type. */
@@ -121,4 +132,10 @@ private JavaFile buildJavaFile(
   // TODO(ronshapiro): write() makes more sense in JavaWriter where all writers are mutable.
   // consider renaming to something like typeBuilder() which conveys the mutability of the result
   abstract Optional<TypeSpec.Builder> write(ClassName generatedTypeName, T input);
+
+  /** Returns {@link Suppression}s that are applied to files generated by this generator. */
+  // TODO(b/134590785): When suppressions are removed locally, remove this and inline the usages
+  protected ImmutableSet<Suppression> warningSuppressions() {
+    return ImmutableSet.of();
+  }
 }
diff --git a/java/dagger/internal/codegen/javapoet/AnnotationSpecs.java b/java/dagger/internal/codegen/javapoet/AnnotationSpecs.java
@@ -16,34 +16,38 @@
 
 package dagger.internal.codegen.javapoet;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkArgument;
 
-import com.google.common.base.Ascii;
-import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.squareup.javapoet.AnnotationSpec;
-import java.util.Arrays;
 
 /** Static factories to create {@link AnnotationSpec}s. */
 public final class AnnotationSpecs {
   /** Values for an {@link SuppressWarnings} annotation. */
   public enum Suppression {
-    RAWTYPES,
-    UNCHECKED,
+    RAWTYPES("rawtypes"),
+    UNCHECKED("unchecked"),
+    FUTURE_RETURN_VALUE_IGNORED("FutureReturnValueIgnored"),
     ;
 
-    @Override
-    public String toString() {
-      return Ascii.toLowerCase(name());
+    private final String value;
+
+    Suppression(String value) {
+      this.value = value;
     }
   }
 
   /** Creates an {@link AnnotationSpec} for {@link SuppressWarnings}. */
   public static AnnotationSpec suppressWarnings(Suppression first, Suppression... rest) {
-    checkNotNull(first);
-    Arrays.stream(rest).forEach(Preconditions::checkNotNull);
+    return suppressWarnings(ImmutableSet.copyOf(Lists.asList(first, rest)));
+  }
+
+  /** Creates an {@link AnnotationSpec} for {@link SuppressWarnings}. */
+  public static AnnotationSpec suppressWarnings(ImmutableSet<Suppression> suppressions) {
+    checkArgument(!suppressions.isEmpty());
     AnnotationSpec.Builder builder = AnnotationSpec.builder(SuppressWarnings.class);
-    Lists.asList(first, rest).forEach(suppression -> builder.addMember("value", "$S", suppression));
+    suppressions.forEach(suppression -> builder.addMember("value", "$S", suppression.value));
     return builder.build();
   }
 

diff --git a/javatests/dagger/functional/MultibindingTest.java b/javatests/dagger/functional/MultibindingTest.java
@@ -80,7 +80,7 @@ public void unwrappedAnnotationKeyMap() {
 
   @Test
   public void wrappedAnnotationKeyMap() {
-    @SuppressWarnings("unchecked")
+    @SuppressWarnings({"unchecked", "rawtypes"})
     Class<? extends Number>[] classes = new Class[] {Long.class, Integer.class};
     assertThat(multibindingComponent.wrappedAnnotationKeyMap())
         .containsExactly(

diff --git a/javatests/dagger/functional/membersinject/MembersInjectModule.java b/javatests/dagger/functional/membersinject/MembersInjectModule.java
@@ -26,7 +26,7 @@ class MembersInjectModule {
 
   @Provides int[] provideIntArray() { return new int[10]; }
 
-  @SuppressWarnings("unchecked")
+  @SuppressWarnings({"unchecked", "rawtypes"})
   @Provides MembersInjectGenericParent<String[]>[] provideFooArrayOfStringArray() { return new MembersInjectGenericParent[10]; }
 
 }