diff --git a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/GlideExtensionValidator.java b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/GlideExtensionValidator.java index b304e682fd..e8816f6f52 100644 --- a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/GlideExtensionValidator.java +++ b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/GlideExtensionValidator.java @@ -91,23 +91,7 @@ private void validateNewGlideOption(ExecutableElement executableElement) { } private void validateNewGlideOptionAnnotations(ExecutableElement executableElement) { - Set annotationNames = - FluentIterable.from(executableElement.getAnnotationMirrors()) - .transform(new Function() { - @Override - public String apply(AnnotationMirror input) { - return input.getAnnotationType().asElement().toString(); - } - }) - .toSet(); - if (!annotationNames.contains("android.support.annotation.NonNull")) { - processingEnvironment.getMessager().printMessage( - Kind.WARNING, - executableElement.getEnclosingElement() + "#" + executableElement.getSimpleName() - + " is missing the @NonNull annotation," - + " please add it to ensure that your extension methods are always returning non-null" - + " values"); - } + validateAnnotatedNonNull(executableElement); } private void validateDeprecatedGlideOption(ExecutableElement executableElement) { @@ -197,6 +181,7 @@ private void validateGlideType(ExecutableElement executableElement) { private void validateNewGlideType(ExecutableElement executableElement) { TypeMirror returnType = executableElement.getReturnType(); + validateNewGlideTypeAnnotations(executableElement); if (!isRequestBuilder(returnType) || !typeMatchesExpected(returnType, executableElement)) { String expectedClassName = getGlideTypeValue(executableElement); throw new IllegalArgumentException("@GlideType methods should return a RequestBuilder<" @@ -253,6 +238,30 @@ private static void validateGlideTypeParameters(ExecutableElement executableElem } } + private void validateNewGlideTypeAnnotations(ExecutableElement executableElement) { + validateAnnotatedNonNull(executableElement); + } + + private void validateAnnotatedNonNull(ExecutableElement executableElement) { + Set annotationNames = + FluentIterable.from(executableElement.getAnnotationMirrors()) + .transform(new Function() { + @Override + public String apply(AnnotationMirror input) { + return input.getAnnotationType().asElement().toString(); + } + }) + .toSet(); + if (!annotationNames.contains("android.support.annotation.NonNull")) { + processingEnvironment.getMessager().printMessage( + Kind.WARNING, + executableElement.getEnclosingElement() + "#" + executableElement.getSimpleName() + + " is missing the @NonNull annotation," + + " please add it to ensure that your extension methods are always returning non-null" + + " values"); + } + } + private static void validateStatic(ExecutableElement executableElement, Class clazz) { if (!executableElement.getModifiers().contains(Modifier.STATIC)) { throw new IllegalArgumentException("@" + clazz.getSimpleName() + " methods must be static"); diff --git a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestManagerGenerator.java b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestManagerGenerator.java index 2f1a20b507..467a08df35 100644 --- a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestManagerGenerator.java +++ b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestManagerGenerator.java @@ -60,6 +60,8 @@ final class RequestManagerGenerator { "com.bumptech.glide.manager.RequestManagerTreeNode"; private static final ClassName CHECK_RESULT_CLASS_NAME = ClassName.get("android.support.annotation", "CheckResult"); + private static final ClassName NON_NULL_CLASS_NAME = + ClassName.get("android.support.annotation", "NonNull"); private static final ClassName CONTEXT_CLASS_NAME = ClassName.get("android.content", "Context"); @@ -270,6 +272,7 @@ private MethodSpec generateAdditionalRequestManagerMethodLegacy( .addModifiers(Modifier.PUBLIC) .returns(parameterizedTypeName) .addJavadoc(processorUtil.generateSeeMethodJavadoc(extensionMethod)) + .addAnnotation(AnnotationSpec.builder(NON_NULL_CLASS_NAME).build()) .addStatement( "$T requestBuilder = this.as($T.class)", parameterizedTypeName, returnTypeClassName) .addStatement("$T.$N(requestBuilder)", @@ -290,6 +293,7 @@ private MethodSpec generateAdditionalRequestManagerMethodNew( .addModifiers(Modifier.PUBLIC) .returns(parameterizedTypeName) .addJavadoc(processorUtil.generateSeeMethodJavadoc(extensionMethod)) + .addAnnotation(AnnotationSpec.builder(NON_NULL_CLASS_NAME).build()) .addStatement( "return ($T) $T.$N(this.as($T.class))", parameterizedTypeName, diff --git a/annotation/compiler/test/src/test/java/com/bumptech/glide/annotation/compiler/InvalidGlideTypeExtensionTest.java b/annotation/compiler/test/src/test/java/com/bumptech/glide/annotation/compiler/InvalidGlideTypeExtensionTest.java index e73b67288b..80a78e171f 100644 --- a/annotation/compiler/test/src/test/java/com/bumptech/glide/annotation/compiler/InvalidGlideTypeExtensionTest.java +++ b/annotation/compiler/test/src/test/java/com/bumptech/glide/annotation/compiler/InvalidGlideTypeExtensionTest.java @@ -170,7 +170,7 @@ public void compilation_withAnnotatedStaticMethod_overridingExistingType_fails() @Test public void compilation_withAnnotatedStaticMethod_returningRequestBuilder_succeeds() { - Compilation compilation = + Compilation compilation = javac() .withProcessors(new GlideAnnotationProcessor()) .compile( @@ -178,12 +178,14 @@ public void compilation_withAnnotatedStaticMethod_returningRequestBuilder_succee JavaFileObjects.forSourceLines( "Extension", "package com.bumptech.glide.test;", + "import android.support.annotation.NonNull;", "import com.bumptech.glide.RequestBuilder;", "import com.bumptech.glide.annotation.GlideExtension;", "import com.bumptech.glide.annotation.GlideType;", "@GlideExtension", "public class Extension {", " private Extension() {}", + " @NonNull", " @GlideType(Number.class)", " public static RequestBuilder asNumber(", " RequestBuilder builder) {", @@ -233,21 +235,21 @@ public void compilation_withAnnotatedStaticMethod_returningBuilderWithIncorrectT .compile( emptyAppModule(), JavaFileObjects.forSourceLines( - "Extension", - "package com.bumptech.glide.test;", - "import com.bumptech.glide.RequestBuilder;", - "import com.bumptech.glide.annotation.GlideExtension;", - "import com.bumptech.glide.annotation.GlideType;", - "@GlideExtension", - "public class Extension {", - " private Extension() {}", - " @GlideType(Number.class)", - " public static RequestBuilder asNumber(", - " RequestBuilder builder) {", - " return builder;", - " }", - "}")); - } + "Extension", + "package com.bumptech.glide.test;", + "import com.bumptech.glide.RequestBuilder;", + "import com.bumptech.glide.annotation.GlideExtension;", + "import com.bumptech.glide.annotation.GlideType;", + "@GlideExtension", + "public class Extension {", + " private Extension() {}", + " @GlideType(Number.class)", + " public static RequestBuilder asNumber(", + " RequestBuilder builder) {", + " return builder;", + " }", + "}")); + } @Test public void compilation_withAnnotatedStaticMethod_returningBuilder_andMultipleParams_fails() { @@ -261,20 +263,20 @@ public void compilation_withAnnotatedStaticMethod_returningBuilder_andMultiplePa .compile( emptyAppModule(), JavaFileObjects.forSourceLines( - "Extension", - "package com.bumptech.glide.test;", - "import com.bumptech.glide.RequestBuilder;", - "import com.bumptech.glide.annotation.GlideExtension;", - "import com.bumptech.glide.annotation.GlideType;", - "@GlideExtension", - "public class Extension {", - " private Extension() {}", - " @GlideType(Number.class)", - " public static RequestBuilder asNumber(", - " RequestBuilder builder, Object arg1) {", - " return builder;", - " }", - "}")); + "Extension", + "package com.bumptech.glide.test;", + "import com.bumptech.glide.RequestBuilder;", + "import com.bumptech.glide.annotation.GlideExtension;", + "import com.bumptech.glide.annotation.GlideType;", + "@GlideExtension", + "public class Extension {", + " private Extension() {}", + " @GlideType(Number.class)", + " public static RequestBuilder asNumber(", + " RequestBuilder builder, Object arg1) {", + " return builder;", + " }", + "}")); } @Test @@ -287,19 +289,47 @@ public void compilation_withAnnotatedStaticMethod_returningBuilder_nonBuilderPar .compile( emptyAppModule(), JavaFileObjects.forSourceLines( - "Extension", - "package com.bumptech.glide.test;", - "import com.bumptech.glide.RequestBuilder;", - "import com.bumptech.glide.annotation.GlideExtension;", - "import com.bumptech.glide.annotation.GlideType;", - "@GlideExtension", - "public class Extension {", - " private Extension() {}", - " @GlideType(Number.class)", - " public static RequestBuilder asNumber(", - " Object arg) {", - " return null;", - " }", - "}")); - } + "Extension", + "package com.bumptech.glide.test;", + "import com.bumptech.glide.RequestBuilder;", + "import com.bumptech.glide.annotation.GlideExtension;", + "import com.bumptech.glide.annotation.GlideType;", + "@GlideExtension", + "public class Extension {", + " private Extension() {}", + " @GlideType(Number.class)", + " public static RequestBuilder asNumber(", + " Object arg) {", + " return null;", + " }", + "}")); + } + + @Test + public void compilation_withAnnotatedStaticMethod_returningRequestBuilder_missingNonNull_warns() { + Compilation compilation = + javac() + .withProcessors(new GlideAnnotationProcessor()) + .compile( + emptyAppModule(), + JavaFileObjects.forSourceLines( + "Extension", + "package com.bumptech.glide.test;", + "import com.bumptech.glide.RequestBuilder;", + "import com.bumptech.glide.annotation.GlideExtension;", + "import com.bumptech.glide.annotation.GlideType;", + "@GlideExtension", + "public class Extension {", + " private Extension() {}", + " @GlideType(Number.class)", + " public static RequestBuilder asNumber(", + " RequestBuilder builder) {", + " return builder;", + " }", + "}")); + assertThat(compilation).succeeded(); + assertThat(compilation).hadWarningCount(1); + assertThat(compilation).hadWarningContaining("@NonNull"); + assertThat(compilation).hadWarningContaining("com.bumptech.glide.test.Extension#asNumber"); + } } diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/ExtensionWithType.java b/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/ExtensionWithType.java index 6761e6d56b..1d58e6ff0c 100644 --- a/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/ExtensionWithType.java +++ b/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/ExtensionWithType.java @@ -1,5 +1,6 @@ package com.bumptech.glide.test; +import android.support.annotation.NonNull; import com.bumptech.glide.RequestBuilder; import com.bumptech.glide.annotation.GlideExtension; import com.bumptech.glide.annotation.GlideType; @@ -11,6 +12,7 @@ private ExtensionWithType() { // Utility class. } + @NonNull @GlideType(Number.class) public static RequestBuilder asNumber(RequestBuilder builder) { return builder; diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java b/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java index ac9063d4c5..5d16fc5c56 100644 --- a/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java +++ b/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java @@ -46,6 +46,7 @@ public GlideRequest as(Class resource /** * @see ExtensionWithType#asNumber(RequestBuilder) */ + @NonNull public GlideRequest asNumber() { return (GlideRequest) ExtensionWithType.asNumber(this.as(Number.class)); } diff --git a/annotation/compiler/test/src/test/resources/LegacyGlideExtensionWithTypeTest/GlideRequests.java b/annotation/compiler/test/src/test/resources/LegacyGlideExtensionWithTypeTest/GlideRequests.java index 2bde90dd42..0df1a76bb9 100644 --- a/annotation/compiler/test/src/test/resources/LegacyGlideExtensionWithTypeTest/GlideRequests.java +++ b/annotation/compiler/test/src/test/resources/LegacyGlideExtensionWithTypeTest/GlideRequests.java @@ -46,6 +46,7 @@ public GlideRequest as(Class resource /** * @see ExtensionWithType#asInteger(RequestBuilder) */ + @NonNull public GlideRequest asInteger() { GlideRequest requestBuilder = this.as(Number.class); ExtensionWithType.asInteger(requestBuilder);