diff --git a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/GlideGenerator.java b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/GlideGenerator.java index 1e3949a2f1..512fc42a73 100644 --- a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/GlideGenerator.java +++ b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/GlideGenerator.java @@ -7,7 +7,7 @@ import com.squareup.javapoet.AnnotationSpec; import com.squareup.javapoet.ClassName; import com.squareup.javapoet.MethodSpec; -import com.squareup.javapoet.ParameterSpec; +import com.squareup.javapoet.MethodSpec.Builder; import com.squareup.javapoet.TypeSpec; import java.util.ArrayList; import java.util.List; @@ -147,33 +147,9 @@ private MethodSpec overrideGlideStaticMethod(ExecutableElement methodToOverride) MethodSpec.methodBuilder(methodToOverride.getSimpleName().toString()) .addModifiers(Modifier.PUBLIC, Modifier.STATIC) .addJavadoc(processorUtil.generateSeeMethodJavadoc(methodToOverride)) - .addParameters(Lists.transform(parameters, - new Function() { - @Override - public ParameterSpec apply(VariableElement input) { - return ParameterSpec.get(input); - } - })); + .addParameters(ProcessorUtil.getParameters(methodToOverride)); - String visibleForTestingTypeQualifiedName = - processingEnv - .getElementUtils() - .getTypeElement(VISIBLE_FOR_TESTING_QUALIFIED_NAME) - .toString(); - for (AnnotationMirror mirror : methodToOverride.getAnnotationMirrors()) { - builder.addAnnotation(AnnotationSpec.get(mirror)); - - // Suppress a lint warning if we're overriding a VisibleForTesting method. - // See #1977. - String annotationQualfiedName = mirror.getAnnotationType().toString(); - if (annotationQualfiedName.equals(visibleForTestingTypeQualifiedName)) { - builder.addAnnotation( - AnnotationSpec.builder( - ClassName.get(SUPPRESS_LINT_PACKAGE_NAME, SUPPRESS_LINT_CLASS_NAME)) - .addMember("value", "$S", "VisibleForTests") - .build()); - } - } + addReturnAnnotations(builder, methodToOverride); boolean returnsValue = element != null; if (returnsValue) { @@ -197,6 +173,31 @@ public ParameterSpec apply(VariableElement input) { return builder.build(); } + private Builder addReturnAnnotations(Builder builder, ExecutableElement methodToOverride) { + String visibleForTestingTypeQualifiedName = + processingEnv + .getElementUtils() + .getTypeElement(VISIBLE_FOR_TESTING_QUALIFIED_NAME) + .toString(); + + for (AnnotationMirror mirror : methodToOverride.getAnnotationMirrors()) { + builder.addAnnotation(AnnotationSpec.get(mirror)); + + // Suppress a lint warning if we're overriding a VisibleForTesting method. + // See #1977. + String annotationQualifiedName = mirror.getAnnotationType().toString(); + if (annotationQualifiedName.equals(visibleForTestingTypeQualifiedName)) { + builder.addAnnotation( + AnnotationSpec.builder( + ClassName.get(SUPPRESS_LINT_PACKAGE_NAME, SUPPRESS_LINT_CLASS_NAME)) + .addMember("value", "$S", "VisibleForTests") + .build()); + } + } + + return builder; + } + private List discoverGlideMethodsToOverride() { return processorUtil.findStaticMethods(glideType); } @@ -213,15 +214,17 @@ private MethodSpec overrideGlideWithMethod( Preconditions.checkArgument( parameters.size() == 1, "Expected size of 1, but got %s", methodToOverride); VariableElement parameter = parameters.iterator().next(); - return MethodSpec.methodBuilder(methodToOverride.getSimpleName().toString()) + + Builder builder = MethodSpec.methodBuilder(methodToOverride.getSimpleName().toString()) .addModifiers(Modifier.PUBLIC, Modifier.STATIC) .addJavadoc(processorUtil.generateSeeMethodJavadoc(methodToOverride)) + .addParameters(ProcessorUtil.getParameters(methodToOverride)) .returns(generatedRequestManagerClassName) - .addParameter(ClassName.get(parameter.asType()), parameter.getSimpleName().toString()) .addStatement("return ($T) $T.$N($L)", generatedRequestManagerClassName, glideType, methodToOverride.getSimpleName().toString(), - parameter.getSimpleName()) - .build(); + parameter.getSimpleName()); + + return addReturnAnnotations(builder, methodToOverride).build(); } } diff --git a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/ProcessorUtil.java b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/ProcessorUtil.java index ac49364b2b..3daf58dad8 100644 --- a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/ProcessorUtil.java +++ b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/ProcessorUtil.java @@ -220,7 +220,7 @@ private CodeBlock generateSeeMethodJavadocInternal( return CodeBlock.of(javadocString.toString(), javadocArgs.toArray(new Object[0])); } - /** + /** * Returns a safe String to use in a Javadoc that will function in a link. * *

This method exists because by Javadoc doesn't handle type parameters({@literal } @@ -439,12 +439,12 @@ private final class FilterPublicMethods implements Predicate { private final TypeMirror returnType; private final MethodType methodType; - FilterPublicMethods(@Nullable TypeMirror returnType, MethodType methodType) { + FilterPublicMethods(@Nullable TypeMirror returnType, MethodType methodType) { this.returnType = returnType; this.methodType = methodType; } - FilterPublicMethods(@Nullable TypeElement returnType, MethodType methodType) { + FilterPublicMethods(@Nullable TypeElement returnType, MethodType methodType) { this(returnType != null ? returnType.asType() : null, methodType); } 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 6ed2e79b60..2f1a20b507 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 @@ -95,7 +95,6 @@ final class RequestManagerGenerator { glideType = elementUtils.getTypeElement(GLIDE_QUALIFIED_NAME); } - @Nullable TypeSpec generate( String generatedCodePackageName, @Nullable TypeSpec requestOptions, TypeSpec requestBuilder, Set glideExtensions) { diff --git a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestOptionsGenerator.java b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestOptionsGenerator.java index 0cffa7c9a2..6fcb003f09 100644 --- a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestOptionsGenerator.java +++ b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestOptionsGenerator.java @@ -276,8 +276,7 @@ private List generateMethodsForRequestOptionsExtensionNew( methodLiterals.substring(0, methodLiterals.length() - 2)); } extensionRequestOptionsArgument = CodeBlock.builder() - .add( - "super.$N(" + methodLiterals + ")", methodArgs.toArray(new Object[0])) + .add("super.$N(" + methodLiterals + ")", methodArgs.toArray(new Object[0])) .build() .toString(); } else { @@ -354,7 +353,6 @@ private List generateMethodsForRequestOptionsExtensionDeprec .addAnnotation(Override.class); } - // Adds: .(RequestOptions, , , ); List args = new ArrayList<>(); StringBuilder code = new StringBuilder("$T.$L($L, "); @@ -440,25 +438,12 @@ private MethodAndStaticVar generateStaticMethodEquivalentForRequestOptionsStatic MethodSpec.Builder methodSpecBuilder = MethodSpec.methodBuilder(staticMethodName) .addModifiers(Modifier.PUBLIC, Modifier.STATIC) - .addJavadoc(processorUtil.generateSeeMethodJavadoc(staticMethod)) - .returns(glideOptionsName); + .addJavadoc(processorUtil.generateSeeMethodJavadoc(staticMethod)) + .returns(glideOptionsName); List parameters = staticMethod.getParameters(); - StringBuilder createNewOptionAndCall = new StringBuilder("new $T().$N("); - if (!parameters.isEmpty()) { - methodSpecBuilder.addParameters(ProcessorUtil.getParameters(staticMethod)); - for (VariableElement parameter : parameters) { - createNewOptionAndCall.append(parameter.getSimpleName().toString()); - // use the Application Context to avoid memory leaks. - if (memoize && isAndroidContext(parameter)) { - createNewOptionAndCall.append(".getApplicationContext()"); - } - createNewOptionAndCall.append(", "); - } - createNewOptionAndCall = new StringBuilder( - createNewOptionAndCall.substring(0, createNewOptionAndCall.length() - 2)); - } - createNewOptionAndCall.append(")"); + StringBuilder createNewOptionAndCall = createNewOptionAndCall(memoize, methodSpecBuilder, + parameters, "new $T().$N(", ProcessorUtil.getParameters(staticMethod)); FieldSpec requiredStaticField = null; if (memoize) { @@ -542,24 +527,12 @@ private MethodAndStaticVar generateStaticMethodEquivalentForExtensionMethod( // Remove is not supported. parameters = parameters.subList(1, parameters.size()); - StringBuilder createNewOptionAndCall = new StringBuilder("new $T().$L("); - if (!parameters.isEmpty()) { - methodSpecBuilder.addParameters(ProcessorUtil.getParameters(parameters)); - for (VariableElement parameter : parameters) { - createNewOptionAndCall.append(parameter.getSimpleName().toString()); - // use the Application Context to avoid memory leaks. - if (memoize && isAndroidContext(parameter)) { - createNewOptionAndCall.append(".getApplicationContext()"); - } - createNewOptionAndCall.append(", "); - } - createNewOptionAndCall = new StringBuilder( - createNewOptionAndCall.substring(0, createNewOptionAndCall.length() - 2)); - } - createNewOptionAndCall.append(")"); + StringBuilder createNewOptionAndCall = createNewOptionAndCall(memoize, methodSpecBuilder, + parameters, "new $T().$L(", ProcessorUtil.getParameters(parameters)); FieldSpec requiredStaticField = null; if (memoize) { + // Generates code that looks like: // if (GlideOptions. == null) { // GlideOptions. = new GlideOptions().().autoClone() // } @@ -577,6 +550,7 @@ private MethodAndStaticVar generateStaticMethodEquivalentForExtensionMethod( .endControlFlow() .addStatement("return $T.$N", glideOptionsName, staticVariableName); } else { + // Generates code that looks like: // return new GlideOptions().() methodSpecBuilder.addStatement( "return " + createNewOptionAndCall, glideOptionsName, instanceMethodName); @@ -593,6 +567,27 @@ private MethodAndStaticVar generateStaticMethodEquivalentForExtensionMethod( return new MethodAndStaticVar(methodSpecBuilder.build(), requiredStaticField); } + private StringBuilder createNewOptionAndCall(boolean memoize, + MethodSpec.Builder methodSpecBuilder, + List parameters, String start, List specs) { + StringBuilder createNewOptionAndCall = new StringBuilder(start); + if (!parameters.isEmpty()) { + methodSpecBuilder.addParameters(specs); + for (VariableElement parameter : parameters) { + createNewOptionAndCall.append(parameter.getSimpleName().toString()); + // use the Application Context to avoid memory leaks. + if (memoize && isAndroidContext(parameter)) { + createNewOptionAndCall.append(".getApplicationContext()"); + } + createNewOptionAndCall.append(", "); + } + createNewOptionAndCall = new StringBuilder( + createNewOptionAndCall.substring(0, createNewOptionAndCall.length() - 2)); + } + createNewOptionAndCall.append(")"); + return createNewOptionAndCall; + } + private boolean isAndroidContext(VariableElement variableElement) { Element element = processingEnvironment.getTypeUtils().asElement(variableElement.asType()); return element.toString().equals("android.content.Context"); diff --git a/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java b/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java index 685506a887..4a1e8e1087 100644 --- a/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java +++ b/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java @@ -32,7 +32,7 @@ private GlideApp() { * @see Glide#getPhotoCacheDir(Context) */ @Nullable - public static File getPhotoCacheDir(Context arg0) { + public static File getPhotoCacheDir(@NonNull Context arg0) { return Glide.getPhotoCacheDir(arg0); } @@ -40,7 +40,7 @@ public static File getPhotoCacheDir(Context arg0) { * @see Glide#getPhotoCacheDir(Context, String) */ @Nullable - public static File getPhotoCacheDir(Context arg0, String arg1) { + public static File getPhotoCacheDir(@NonNull Context arg0, @NonNull String arg1) { return Glide.getPhotoCacheDir(arg0, arg1); } @@ -48,7 +48,7 @@ public static File getPhotoCacheDir(Context arg0, String arg1) { * @see Glide#get(Context) */ @NonNull - public static Glide get(Context arg0) { + public static Glide get(@NonNull Context arg0) { return Glide.get(arg0); } @@ -67,7 +67,7 @@ public static void init(Glide glide) { */ @VisibleForTesting @SuppressLint("VisibleForTests") - public static void init(Context arg0, GlideBuilder arg1) { + public static void init(@NonNull Context arg0, @NonNull GlideBuilder arg1) { Glide.init(arg0, arg1); } @@ -83,42 +83,48 @@ public static void tearDown() { /** * @see Glide#with(Context) */ - public static GlideRequests with(Context arg0) { + @NonNull + public static GlideRequests with(@NonNull Context arg0) { return (GlideRequests) Glide.with(arg0); } /** * @see Glide#with(Activity) */ - public static GlideRequests with(Activity arg0) { + @NonNull + public static GlideRequests with(@NonNull Activity arg0) { return (GlideRequests) Glide.with(arg0); } /** * @see Glide#with(FragmentActivity) */ - public static GlideRequests with(FragmentActivity arg0) { + @NonNull + public static GlideRequests with(@NonNull FragmentActivity arg0) { return (GlideRequests) Glide.with(arg0); } /** * @see Glide#with(Fragment) */ - public static GlideRequests with(Fragment arg0) { + @NonNull + public static GlideRequests with(@NonNull Fragment arg0) { return (GlideRequests) Glide.with(arg0); } /** * @see Glide#with(Fragment) */ - public static GlideRequests with(android.support.v4.app.Fragment arg0) { + @NonNull + public static GlideRequests with(@NonNull android.support.v4.app.Fragment arg0) { return (GlideRequests) Glide.with(arg0); } /** * @see Glide#with(View) */ - public static GlideRequests with(View arg0) { + @NonNull + public static GlideRequests with(@NonNull View arg0) { return (GlideRequests) Glide.with(arg0); } }