Skip to content

Commit

Permalink
Add nullability annotations to more generated classes (#2749)
Browse files Browse the repository at this point in the history 
Signed-off-by: Alex Saveau <[email protected]>
  • Loading branch information
@SUPERCILEX @sjudd
SUPERCILEX authored and sjudd committed Dec 23, 2017
1 parent bb96b63 commit 276d4ff
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 80 deletions.
65 changes: 34 additions & 31 deletions annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/GlideGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<VariableElement, ParameterSpec>() {
@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) {
Expand All @@ -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<ExecutableElement> discoverGlideMethodsToOverride() {
return processorUtil.findStaticMethods(glideType);
}
Expand All @@ -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();
}
}
6 changes: 3 additions & 3 deletions annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/ProcessorUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>This method exists because by Javadoc doesn't handle type parameters({@literal <T>}
Expand Down Expand Up @@ -439,12 +439,12 @@ private final class FilterPublicMethods implements Predicate<Element> {
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);
}

Expand Down
1 change: 0 additions & 1 deletion ...ompiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestManagerGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ final class RequestManagerGenerator {
glideType = elementUtils.getTypeElement(GLIDE_QUALIFIED_NAME);
}

@Nullable
TypeSpec generate(
String generatedCodePackageName, @Nullable TypeSpec requestOptions, TypeSpec requestBuilder,
Set<String> glideExtensions) {
Expand Down
65 changes: 30 additions & 35 deletions ...ompiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestOptionsGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,7 @@ private List<MethodAndStaticVar> 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 {
Expand Down Expand Up @@ -354,7 +353,6 @@ private List<MethodAndStaticVar> generateMethodsForRequestOptionsExtensionDeprec
.addAnnotation(Override.class);
}


// Adds: <AnnotatedClass>.<thisMethodName>(RequestOptions<?>, <arg1>, <arg2>, <argN>);
List<Object> args = new ArrayList<>();
StringBuilder code = new StringBuilder("$T.$L($L, ");
Expand Down Expand Up @@ -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<? extends VariableElement> 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) {
Expand Down Expand Up @@ -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.<methodName> == null) {
// GlideOptions.<methodName> = new GlideOptions().<methodName>().autoClone()
// }
Expand All @@ -577,6 +550,7 @@ private MethodAndStaticVar generateStaticMethodEquivalentForExtensionMethod(
.endControlFlow()
.addStatement("return $T.$N", glideOptionsName, staticVariableName);
} else {
// Generates code that looks like:
// return new GlideOptions().<methodName>()
methodSpecBuilder.addStatement(
"return " + createNewOptionAndCall, glideOptionsName, instanceMethodName);
Expand All @@ -593,6 +567,27 @@ private MethodAndStaticVar generateStaticMethodEquivalentForExtensionMethod(
return new MethodAndStaticVar(methodSpecBuilder.build(), requiredStaticField);
}

private StringBuilder createNewOptionAndCall(boolean memoize,
MethodSpec.Builder methodSpecBuilder,
List<? extends VariableElement> parameters, String start, List<ParameterSpec> 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");
Expand Down
26 changes: 16 additions & 10 deletions annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,23 @@ private GlideApp() {
* @see Glide#getPhotoCacheDir(Context)
*/
@Nullable
public static File getPhotoCacheDir(Context arg0) {
public static File getPhotoCacheDir(@NonNull Context arg0) {
return Glide.getPhotoCacheDir(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);
}

/**
* @see Glide#get(Context)
*/
@NonNull
public static Glide get(Context arg0) {
public static Glide get(@NonNull Context arg0) {
return Glide.get(arg0);
}

Expand All @@ -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);
}

Expand All @@ -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);
}
}

0 comments on commit 276d4ff

Please sign in to comment.