Add missing annotations on APT overriding methods (#2779)

* Run tests for annotation compiler * @nonnull GeneratedAppGlideModuleImpl.getExcludedModuleClasses * @nonnull RequestBuilder.getDownloadOnlyRequest * @nonnull RequestManager.as * Fix Gradle deprecation warning (file(File) -> files(Object))
bumptech · Jan 2, 2018 · d7bb6f9 · d7bb6f9
1 parent fd73179
commit d7bb6f9
Show file tree

Hide file tree

Showing 25 changed files with 45 additions and 8 deletions.
diff --git a/annotation/compiler/build.gradle b/annotation/compiler/build.gradle
@@ -21,6 +21,9 @@ dependencies {
     compile files(Jvm.current().getToolsJar())
 }
 
+// Make sure running `gradlew :annotation:compiler:check` actually does full quality control.
+test.dependsOn ':annotation:compiler:test:test'
+
 def packagingFolder = file("${buildDir}/intermediates")
 def repackagedJar = file("${packagingFolder}/repackaged.jar")
 def proguardedJar = file("${packagingFolder}/proguarded.jar")
@@ -34,7 +37,7 @@ task compiledJar(type: Jar, dependsOn: classes) {
 // Repackage compileOnly dependencies to avoid namespace collisions.
 task jarjar(dependsOn: [tasks.compiledJar, configurations.compileOnly]) {
     // Set up inputs and outputs to only rebuild when necessary (code change, dependency change).
-    inputs.file compiledJar
+    inputs.files compiledJar
     inputs.files configurations.compileOnly
     outputs.file repackagedJar
 

diff --git a/...ion/compiler/src/main/java/com/bumptech/glide/annotation/compiler/AppModuleGenerator.java b/...ion/compiler/src/main/java/com/bumptech/glide/annotation/compiler/AppModuleGenerator.java
@@ -64,7 +64,8 @@
  *    }
  *
  *    {@literal @java.lang.Override}
- *    public java.util.Set<java.lang.Class<?>> getExcludedModuleClasses() {
+ *    {@literal @android.support.annotation.NonNull}
+ *    public java.util.Set&lt;java.lang.Class&lt;?>> getExcludedModuleClasses() {
  *      return appGlideModule.getExcludedModuleClasses();
  *    }
  *  }
@@ -164,6 +165,7 @@ private MethodSpec generateGetExcludedModuleClasses(Collection<String> excludedC
     MethodSpec.Builder builder = MethodSpec.methodBuilder("getExcludedModuleClasses")
         .addModifiers(Modifier.PUBLIC)
         .addAnnotation(Override.class)
+        .addAnnotation(ClassName.get("android.support.annotation", "NonNull"))
         .returns(setOfClassOfWildcardOfObject);
 
     if (excludedClassNames.isEmpty()) {

diff --git a/...ompiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestBuilderGenerator.java b/...ompiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestBuilderGenerator.java
@@ -108,6 +108,9 @@ final class RequestBuilderGenerator {
       ImmutableSet.of("clone", "apply", "autoLock", "lock", "autoClone");
   private static final ClassName CHECK_RESULT_CLASS_NAME =
       ClassName.get("android.support.annotation", "CheckResult");
+  private static final AnnotationSpec NON_NULL = AnnotationSpec
+      .builder(ClassName.get("android.support.annotation", "NonNull"))
+      .build();
 
   private final ProcessingEnvironment processingEnv;
   private final ProcessorUtil processorUtil;
@@ -429,6 +432,7 @@ private MethodSpec generateDownloadOnlyRequestMethod() {
     return MethodSpec.methodBuilder("getDownloadOnlyRequest")
         .addAnnotation(Override.class)
         .addAnnotation(AnnotationSpec.builder(CHECK_RESULT_CLASS_NAME).build())
+        .addAnnotation(NON_NULL)
         .returns(generatedRequestBuilderOfFile)
         .addModifiers(Modifier.PROTECTED)
         .addStatement("return new $T<>($T.class, $N).apply($N)",

diff --git a/...ompiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestManagerGenerator.java b/...ompiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestManagerGenerator.java
@@ -64,6 +64,9 @@ final class RequestManagerGenerator {
       ClassName.get("android.support.annotation", "NonNull");
   private static final ClassName CONTEXT_CLASS_NAME =
       ClassName.get("android.content", "Context");
+  private static final AnnotationSpec NON_NULL = AnnotationSpec
+      .builder(ClassName.get("android.support.annotation", "NonNull"))
+      .build();
 
   private static final String GENERATED_REQUEST_MANAGER_SIMPLE_NAME =
       "GlideRequests";
@@ -151,10 +154,11 @@ private MethodSpec generateAsMethod(String generatedCodePackageName, TypeSpec re
     return MethodSpec.methodBuilder("as")
         .addModifiers(Modifier.PUBLIC)
         .addAnnotation(Override.class)
-        .addTypeVariable(TypeVariableName.get("ResourceType"))
-        .addParameter(classOfResouceType, "resourceClass")
         .addAnnotation(AnnotationSpec.builder(CHECK_RESULT_CLASS_NAME).build())
+        .addAnnotation(NON_NULL)
+        .addTypeVariable(TypeVariableName.get("ResourceType"))
         .returns(requestBuilderOfResourceType)
+        .addParameter(classOfResouceType.annotated(NON_NULL), "resourceClass")
         .addStatement("return new $T<>(glide, this, resourceClass, context)",
             this.generatedRequestBuilderClassName)
         .build();
@@ -166,7 +170,6 @@ private List<MethodSpec> generateRequestManagerRequestManagerMethodOverrides(
     return FluentIterable.from(
         processorUtil.findInstanceMethodsReturning(requestManagerType, requestManagerType))
         .transform(new Function<ExecutableElement, MethodSpec>() {
-          @Nullable
           @Override
           public MethodSpec apply(@Nullable ExecutableElement input) {
             return generateRequestManagerRequestManagerMethodOverride(generatedPackageName, input);

diff --git a/...r/test/src/test/resources/AppGlideModuleWithExcludesTest/GeneratedAppGlideModuleImpl.java b/...r/test/src/test/resources/AppGlideModuleWithExcludesTest/GeneratedAppGlideModuleImpl.java
@@ -1,6 +1,7 @@
 package com.bumptech.glide;
 
 import android.content.Context;
+import android.support.annotation.NonNull;
 import android.util.Log;
 import com.bumptech.glide.test.AppModuleWithExcludes;
 import java.lang.Class;
@@ -37,6 +38,7 @@ public boolean isManifestParsingEnabled() {
   }
 
   @Override
+  @NonNull
   public Set<Class<?>> getExcludedModuleClasses() {
     Set<Class<?>> excludedClasses = new HashSet<Class<?>>();
     excludedClasses.add(com.bumptech.glide.test.EmptyLibraryModule.class);

diff --git a/...rc/test/resources/AppGlideModuleWithMultipleExcludesTest/GeneratedAppGlideModuleImpl.java b/...rc/test/resources/AppGlideModuleWithMultipleExcludesTest/GeneratedAppGlideModuleImpl.java
@@ -1,6 +1,7 @@
 package com.bumptech.glide;
 
 import android.content.Context;
+import android.support.annotation.NonNull;
 import android.util.Log;
 import com.bumptech.glide.test.AppModuleWithMultipleExcludes;
 import java.lang.Class;
@@ -38,6 +39,7 @@ public boolean isManifestParsingEnabled() {
   }
 
   @Override
+  @NonNull
   public Set<Class<?>> getExcludedModuleClasses() {
     Set<Class<?>> excludedClasses = new HashSet<Class<?>>();
     excludedClasses.add(com.bumptech.glide.test.EmptyLibraryModule1.class);

diff --git a/...st/src/test/resources/EmptyAppAndLibraryGlideModulesTest/GeneratedAppGlideModuleImpl.java b/...st/src/test/resources/EmptyAppAndLibraryGlideModulesTest/GeneratedAppGlideModuleImpl.java
@@ -1,6 +1,7 @@
 package com.bumptech.glide;
 
 import android.content.Context;
+import android.support.annotation.NonNull;
 import android.util.Log;
 import com.bumptech.glide.test.EmptyAppModule;
 import com.bumptech.glide.test.EmptyLibraryModule;
@@ -39,6 +40,7 @@ public boolean isManifestParsingEnabled() {
   }
 
   @Override
+  @NonNull
   public Set<Class<?>> getExcludedModuleClasses() {
     return Collections.emptySet();
   }

diff --git a/...compiler/test/src/test/resources/EmptyAppGlideModuleTest/GeneratedAppGlideModuleImpl.java b/...compiler/test/src/test/resources/EmptyAppGlideModuleTest/GeneratedAppGlideModuleImpl.java
@@ -1,6 +1,7 @@
 package com.bumptech.glide;
 
 import android.content.Context;
+import android.support.annotation.NonNull;
 import android.util.Log;
 import com.bumptech.glide.test.EmptyAppModule;
 import java.lang.Class;
@@ -36,6 +37,7 @@ public boolean isManifestParsingEnabled() {
   }
 
   @Override
+  @NonNull
   public Set<Class<?>> getExcludedModuleClasses() {
     return Collections.emptySet();
   }

diff --git a/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequest.java b/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequests.java b/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequests.java
@@ -38,7 +38,8 @@ public GlideRequests(Glide glide, Lifecycle lifecycle, RequestManagerTreeNode tr
 
   @Override
   @CheckResult
-  public <ResourceType> GlideRequest<ResourceType> as(Class<ResourceType> resourceClass) {
+  @NonNull
+  public <ResourceType> GlideRequest<ResourceType> as(@NonNull Class<ResourceType> resourceClass) {
     return new GlideRequest<>(glide, this, resourceClass, context);
   }
 

diff --git a/...r/test/src/test/resources/GlideExtensionOptionsTest/MemoizeStaticMethod/GlideRequest.java b/...r/test/src/test/resources/GlideExtensionOptionsTest/MemoizeStaticMethod/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/...mpiler/test/src/test/resources/GlideExtensionOptionsTest/OverrideExtend/GlideRequest.java b/...mpiler/test/src/test/resources/GlideExtensionOptionsTest/OverrideExtend/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/...est/resources/GlideExtensionOptionsTest/OverrideExtendMultipleArguments/GlideRequest.java b/...est/resources/GlideExtensionOptionsTest/OverrideExtendMultipleArguments/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/...piler/test/src/test/resources/GlideExtensionOptionsTest/OverrideReplace/GlideRequest.java b/...piler/test/src/test/resources/GlideExtensionOptionsTest/OverrideReplace/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/...iler/test/src/test/resources/GlideExtensionOptionsTest/SkipStaticMethod/GlideRequest.java b/...iler/test/src/test/resources/GlideExtensionOptionsTest/SkipStaticMethod/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/...iler/test/src/test/resources/GlideExtensionOptionsTest/StaticMethodName/GlideRequest.java b/...iler/test/src/test/resources/GlideExtensionOptionsTest/StaticMethodName/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionWithOptionTest/GlideRequest.java b/annotation/compiler/test/src/test/resources/GlideExtensionWithOptionTest/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java b/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java
@@ -39,7 +39,8 @@ public GlideRequests(Glide glide, Lifecycle lifecycle, RequestManagerTreeNode tr
 
   @Override
   @CheckResult
-  public <ResourceType> GlideRequest<ResourceType> as(Class<ResourceType> resourceClass) {
+  @NonNull
+  public <ResourceType> GlideRequest<ResourceType> as(@NonNull Class<ResourceType> resourceClass) {
     return new GlideRequest<>(glide, this, resourceClass, context);
   }
 

diff --git a/.../src/test/resources/LegacyGlideExtensionOptionsTest/MemoizeStaticMethod/GlideRequest.java b/.../src/test/resources/LegacyGlideExtensionOptionsTest/MemoizeStaticMethod/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/.../test/src/test/resources/LegacyGlideExtensionOptionsTest/OverrideExtend/GlideRequest.java b/.../test/src/test/resources/LegacyGlideExtensionOptionsTest/OverrideExtend/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/...test/src/test/resources/LegacyGlideExtensionOptionsTest/OverrideReplace/GlideRequest.java b/...test/src/test/resources/LegacyGlideExtensionOptionsTest/OverrideReplace/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/...est/src/test/resources/LegacyGlideExtensionOptionsTest/SkipStaticMethod/GlideRequest.java b/...est/src/test/resources/LegacyGlideExtensionOptionsTest/SkipStaticMethod/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/...est/src/test/resources/LegacyGlideExtensionOptionsTest/StaticMethodName/GlideRequest.java b/...est/src/test/resources/LegacyGlideExtensionOptionsTest/StaticMethodName/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/...ion/compiler/test/src/test/resources/LegacyGlideExtensionWithOptionTest/GlideRequest.java b/...ion/compiler/test/src/test/resources/LegacyGlideExtensionWithOptionTest/GlideRequest.java
@@ -64,6 +64,7 @@ public class GlideRequest<TranscodeType> extends RequestBuilder<TranscodeType> i
 
   @Override
   @CheckResult
+  @NonNull
   protected GlideRequest<File> getDownloadOnlyRequest() {
     return new GlideRequest<>(File.class, this).apply(DOWNLOAD_ONLY_OPTIONS);
   }

diff --git a/...tion/compiler/test/src/test/resources/LegacyGlideExtensionWithTypeTest/GlideRequests.java b/...tion/compiler/test/src/test/resources/LegacyGlideExtensionWithTypeTest/GlideRequests.java
@@ -39,7 +39,8 @@ public GlideRequests(Glide glide, Lifecycle lifecycle, RequestManagerTreeNode tr
 
   @Override
   @CheckResult
-  public <ResourceType> GlideRequest<ResourceType> as(Class<ResourceType> resourceClass) {
+  @NonNull
+  public <ResourceType> GlideRequest<ResourceType> as(@NonNull Class<ResourceType> resourceClass) {
     return new GlideRequest<>(glide, this, resourceClass, context);
   }