From 700890ee9d31a02f53857f7d34c9ba8cf71b2a7c Mon Sep 17 00:00:00 2001 From: Ivan Gavrilovic Date: Wed, 28 Aug 2019 11:29:05 +0200 Subject: [PATCH] Add support for finding parent bindings in classpath This commit expands search for parent bindings by examining all superclasses of types processed in the current round. Bindings found in the classpath will not be generated, but information they contain will be used when generating bindings for the current round of processing. Test: ClasspathParentBindTest --- .../java/butterknife/compiler/BindingSet.java | 58 +++- .../compiler/ButterKnifeProcessor.java | 97 ++++++- .../butterknife/ClasspathParentBindTest.java | 258 ++++++++++++++++++ 3 files changed, 391 insertions(+), 22 deletions(-) create mode 100644 butterknife-runtime/src/test/java/butterknife/ClasspathParentBindTest.java diff --git a/butterknife-compiler/src/main/java/butterknife/compiler/BindingSet.java b/butterknife-compiler/src/main/java/butterknife/compiler/BindingSet.java index e5a891d06..c1724dbee 100644 --- a/butterknife-compiler/src/main/java/butterknife/compiler/BindingSet.java +++ b/butterknife-compiler/src/main/java/butterknife/compiler/BindingSet.java @@ -39,7 +39,7 @@ import static javax.lang.model.element.Modifier.PUBLIC; /** A set of all the bindings requested by a single type. */ -final class BindingSet { +final class BindingSet implements BindingInformationProvider { static final ClassName UTILS = ClassName.get("butterknife.internal", "Utils"); private static final ClassName VIEW = ClassName.get("android.view", "View"); private static final ClassName CONTEXT = ClassName.get("android.content", "Context"); @@ -66,12 +66,13 @@ final class BindingSet { private final ImmutableList viewBindings; private final ImmutableList collectionBindings; private final ImmutableList resourceBindings; - private final @Nullable BindingSet parentBinding; + private final @Nullable BindingInformationProvider parentBinding; private BindingSet(TypeName targetTypeName, ClassName bindingClassName, boolean isFinal, boolean isView, boolean isActivity, boolean isDialog, ImmutableList viewBindings, ImmutableList collectionBindings, - ImmutableList resourceBindings, @Nullable BindingSet parentBinding) { + ImmutableList resourceBindings, + @Nullable BindingInformationProvider parentBinding) { this.isFinal = isFinal; this.targetTypeName = targetTypeName; this.bindingClassName = bindingClassName; @@ -84,6 +85,11 @@ private BindingSet(TypeName targetTypeName, ClassName bindingClassName, boolean this.parentBinding = parentBinding; } + @Override + public ClassName getBindingClassName() { + return bindingClassName; + } + JavaFile brewJava(int sdk, boolean debuggable) { TypeSpec bindingConfiguration = createType(sdk, debuggable); return JavaFile.builder(bindingClassName.packageName(), bindingConfiguration) @@ -99,7 +105,7 @@ private TypeSpec createType(int sdk, boolean debuggable) { } if (parentBinding != null) { - result.superclass(parentBinding.bindingClassName); + result.superclass(parentBinding.getBindingClassName()); } else { result.addSuperinterface(UNBINDER); } @@ -667,7 +673,8 @@ private boolean hasViewLocal() { } /** True if this binding requires a view. Otherwise only a context is needed. */ - private boolean constructorNeedsView() { + @Override + public boolean constructorNeedsView() { return hasViewBindings() // || (parentBinding != null && parentBinding.constructorNeedsView()); } @@ -692,15 +699,19 @@ static Builder newBuilder(TypeElement enclosingElement) { targetType = ((ParameterizedTypeName) targetType).rawType; } - String packageName = getPackage(enclosingElement).getQualifiedName().toString(); - String className = enclosingElement.getQualifiedName().toString().substring( - packageName.length() + 1).replace('.', '$'); - ClassName bindingClassName = ClassName.get(packageName, className + "_ViewBinding"); + ClassName bindingClassName = getBindingClassName(enclosingElement); boolean isFinal = enclosingElement.getModifiers().contains(Modifier.FINAL); return new Builder(targetType, bindingClassName, isFinal, isView, isActivity, isDialog); } + static ClassName getBindingClassName(TypeElement typeElement) { + String packageName = getPackage(typeElement).getQualifiedName().toString(); + String className = typeElement.getQualifiedName().toString().substring( + packageName.length() + 1).replace('.', '$'); + return ClassName.get(packageName, className + "_ViewBinding"); + } + static final class Builder { private final TypeName targetTypeName; private final ClassName bindingClassName; @@ -709,7 +720,7 @@ static final class Builder { private final boolean isActivity; private final boolean isDialog; - private @Nullable BindingSet parentBinding; + private @Nullable BindingInformationProvider parentBinding; private final Map viewIdMap = new LinkedHashMap<>(); private final ImmutableList.Builder collectionBindings = @@ -751,7 +762,7 @@ void addResource(ResourceBinding binding) { resourceBindings.add(binding); } - void setParent(BindingSet parent) { + void setParent(BindingInformationProvider parent) { this.parentBinding = parent; } @@ -787,3 +798,28 @@ BindingSet build() { } } } + +interface BindingInformationProvider { + boolean constructorNeedsView(); + ClassName getBindingClassName(); +} + +final class ClasspathBindingSet implements BindingInformationProvider { + private boolean constructorNeedsView; + private ClassName className; + + ClasspathBindingSet(boolean constructorNeedsView, TypeElement classElement) { + this.constructorNeedsView = constructorNeedsView; + this.className = BindingSet.getBindingClassName(classElement); + } + + @Override + public ClassName getBindingClassName() { + return className; + } + + @Override + public boolean constructorNeedsView() { + return constructorNeedsView; + } +} \ No newline at end of file diff --git a/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java b/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java index 3f30e35d9..1c88beb38 100644 --- a/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java +++ b/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java @@ -30,6 +30,7 @@ import butterknife.internal.ListenerMethod; import com.google.auto.common.SuperficialValidation; import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.squareup.javapoet.JavaFile; import com.squareup.javapoet.TypeName; @@ -48,6 +49,7 @@ import java.util.Arrays; import java.util.BitSet; import java.util.Deque; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -342,6 +344,9 @@ private Map findAndParseTargets(RoundEnvironment env) { findAndParseListener(env, listener, builderMap, erasedTargetNames); } + Map classpathBindings = + findAllSupertypeBindings(builderMap, erasedTargetNames); + // Associate superclass binders with their subclass binders. This is a queue-based tree walk // which starts at the roots (superclasses) and walks to the leafs (subclasses). Deque> entries = @@ -353,11 +358,14 @@ private Map findAndParseTargets(RoundEnvironment env) { TypeElement type = entry.getKey(); BindingSet.Builder builder = entry.getValue(); - TypeElement parentType = findParentType(type, erasedTargetNames); + TypeElement parentType = findParentType(type, erasedTargetNames, classpathBindings.keySet()); if (parentType == null) { bindingMap.put(type, builder.build()); } else { - BindingSet parentBinding = bindingMap.get(parentType); + BindingInformationProvider parentBinding = bindingMap.get(parentType); + if (parentBinding == null) { + parentBinding = classpathBindings.get(parentType); + } if (parentBinding != null) { builder.setParent(parentBinding); bindingMap.put(type, builder.build()); @@ -1264,21 +1272,88 @@ private BindingSet.Builder getOrCreateBindingBuilder( return builder; } - /** Finds the parent binder type in the supplied set, if any. */ - private @Nullable TypeElement findParentType(TypeElement typeElement, Set parents) { - TypeMirror type; + /** Finds the parent binder type in the supplied sets, if any. */ + private @Nullable TypeElement findParentType( + TypeElement typeElement, Set parents, Set classpathParents) { while (true) { - type = typeElement.getSuperclass(); - if (type.getKind() == TypeKind.NONE) { - return null; - } - typeElement = (TypeElement) ((DeclaredType) type).asElement(); - if (parents.contains(typeElement)) { + typeElement = getSuperClass(typeElement); + if (typeElement == null || parents.contains(typeElement) + || classpathParents.contains(typeElement)) { return typeElement; } } } + private Map findAllSupertypeBindings( + Map builderMap, Set processedInThisRound) { + Map classpathBindings = new HashMap<>(); + + Set> supportedAnnotations = getSupportedAnnotations(); + Set> requireViewInConstructor = + ImmutableSet.>builder() + .addAll(LISTENERS).add(BindView.class).add(BindViews.class).build(); + supportedAnnotations.removeAll(requireViewInConstructor); + + for (TypeElement typeElement : builderMap.keySet()) { + // Make sure to process superclass before subclass. This is because if there is a class that + // requires a View in the constructor, all subclasses need it as well. + Deque superClasses = new ArrayDeque<>(); + TypeElement superClass = getSuperClass(typeElement); + while (superClass != null && !processedInThisRound.contains(superClass) + && !classpathBindings.containsKey(superClass)) { + superClasses.addFirst(superClass); + superClass = getSuperClass(superClass); + } + + boolean parentHasConstructorWithView = false; + while (!superClasses.isEmpty()) { + TypeElement superclass = superClasses.removeFirst(); + ClasspathBindingSet classpathBinding = + findBindingInfoForType(superclass, requireViewInConstructor, supportedAnnotations, + parentHasConstructorWithView); + if (classpathBinding != null) { + parentHasConstructorWithView |= classpathBinding.constructorNeedsView(); + classpathBindings.put(superclass, classpathBinding); + } + } + } + return ImmutableMap.copyOf(classpathBindings); + } + + private @Nullable ClasspathBindingSet findBindingInfoForType( + TypeElement typeElement, Set> requireConstructorWithView, + Set> otherAnnotations, boolean needsConstructorWithView) { + boolean foundSupportedAnnotation = false; + for (Element enclosedElement : typeElement.getEnclosedElements()) { + for (Class bindViewAnnotation : requireConstructorWithView) { + if (enclosedElement.getAnnotation(bindViewAnnotation) != null) { + return new ClasspathBindingSet(true, typeElement); + } + } + for (Class supportedAnnotation : otherAnnotations) { + if (enclosedElement.getAnnotation(supportedAnnotation) != null) { + if (needsConstructorWithView) { + return new ClasspathBindingSet(true, typeElement); + } + foundSupportedAnnotation = true; + } + } + } + if (foundSupportedAnnotation) { + return new ClasspathBindingSet(false, typeElement); + } else { + return null; + } + } + + private @Nullable TypeElement getSuperClass(TypeElement typeElement) { + TypeMirror type = typeElement.getSuperclass(); + if (type.getKind() == TypeKind.NONE) { + return null; + } + return (TypeElement) ((DeclaredType) type).asElement(); + } + @Override public SourceVersion getSupportedSourceVersion() { return SourceVersion.latestSupported(); } diff --git a/butterknife-runtime/src/test/java/butterknife/ClasspathParentBindTest.java b/butterknife-runtime/src/test/java/butterknife/ClasspathParentBindTest.java new file mode 100644 index 000000000..5df36fbd2 --- /dev/null +++ b/butterknife-runtime/src/test/java/butterknife/ClasspathParentBindTest.java @@ -0,0 +1,258 @@ +package butterknife; + +import com.google.common.collect.ImmutableList; +import com.google.testing.compile.JavaFileObjects; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.Arrays; +import java.util.Locale; + +import javax.tools.JavaCompiler; +import javax.tools.JavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.ToolProvider; + +import butterknife.compiler.ButterKnifeProcessor; + +import static com.google.common.truth.Truth.assertAbout; +import static com.google.testing.compile.JavaSourceSubjectFactory.javaSource; +import static java.nio.charset.StandardCharsets.UTF_8; + +/** Tests binding generation when superclasses are from classpath. */ +public class ClasspathParentBindTest { + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + + @Test + public void parentBindingInClasspath() throws IOException { + JavaFileObject baseClassSource = JavaFileObjects.forSourceString("test.Test", "" + + "package test;\n" + + "import android.view.View;\n" + + "import butterknife.BindView;\n" + + "import butterknife.OnClick;\n" + + "import butterknife.OnLongClick;\n" + + "public class Test {\n" + + " @BindView(1) View view;\n" + + " @BindView(2) View view2;\n" + + " @OnClick(1) void doStuff() {}\n" + + " @OnLongClick(1) boolean doMoreStuff() { return false; }\n" + + "}" + ); + + JavaFileObject subClassSource = JavaFileObjects.forSourceString("test.SubClass", "" + + "package test;\n" + + "import android.view.View;\n" + + "import butterknife.BindView;\n" + + "public class SubClass extends Test {\n" + + " @BindView(3) View view3;\n" + + "}" + ); + + JavaFileObject bindingSource = JavaFileObjects.forSourceString("test/SubClass_ViewBinding", "" + + "// Generated code from Butter Knife. Do not modify!\n" + + "package test;\n" + + "import android.view.View;\n" + + "import androidx.annotation.UiThread;\n" + + "import butterknife.internal.Utils;\n" + + "import java.lang.IllegalStateException;\n" + + "import java.lang.Override;\n" + + "public class SubClass_ViewBinding extends Test_ViewBinding {\n" + + " private SubClass target;\n" + + " @UiThread\n" + + " public SubClass_ViewBinding(SubClass target, View source) {\n" + + " super(target, source);\n" + + " this.target = target;\n" + + " target.view3 = Utils.findRequiredView(source, 3, \"field 'view3'\");\n" + + " }\n" + + " @Override\n" + + " public void unbind() {\n" + + " SubClass target = this.target;\n" + + " if (target == null) throw new IllegalStateException(\"Bindings already cleared.\");\n" + + " this.target = null;\n" + + " target.view3 = null;\n" + + " super.unbind();\n" + + " }\n" + + "}" + ); + + File classesOut = tmp.newFolder("classes-output"); + File sourcesOut = tmp.newFolder("sources-output"); + compileSources(classesOut, sourcesOut, baseClassSource); + + try (URLClassLoader compilationClasspath = new URLClassLoader( + new URL[]{classesOut.toURI().toURL()}, this.getClass().getClassLoader())) { + assertAbout(javaSource()).that(subClassSource) + .withCompilerOptions("-Xlint:-processing") + .withClasspathFrom(compilationClasspath) + .processedWith(new ButterKnifeProcessor()) + .compilesWithoutWarnings() + .and() + .generatesSources(bindingSource); + } + } + + @Test + public void indirectViewRequiredInConstructor() throws IOException { + JavaFileObject classpathClass = JavaFileObjects.forSourceString("test.Test", "" + + "package test;\n" + + "import android.view.View;\n" + + "import butterknife.BindView;\n" + + "import butterknife.OnClick;\n" + + "import butterknife.OnLongClick;\n" + + "public class Test {\n" + + " @BindView(1) View view;\n" + + " @BindView(2) View view2;\n" + + " @OnClick(1) void doStuff() {}\n" + + " @OnLongClick(1) boolean doMoreStuff() { return false; }\n" + + "}" + ); + + JavaFileObject subclassInClasspath = JavaFileObjects.forSourceString("test.SubClassTest", "" + + "package test;\n" + + "import butterknife.BindFloat;\n" + + "public class SubClassTest extends Test{\n" + + " @BindFloat(1) float value;\n" + + "}" + ); + + JavaFileObject toProcessSource = JavaFileObjects.forSourceString("test.ToProcess", "" + + "package test;\n" + + "import android.view.View;\n" + + "import butterknife.BindView;\n" + + "public class ToProcess extends SubClassTest {\n" + + " @BindView(3) View view3;\n" + + "}" + ); + + JavaFileObject bindingSource = JavaFileObjects.forSourceString("test/ToProcess_ViewBinding", "" + + "// Generated code from Butter Knife. Do not modify!\n" + + "package test;\n" + + "import android.view.View;\n" + + "import androidx.annotation.UiThread;\n" + + "import butterknife.internal.Utils;\n" + + "import java.lang.IllegalStateException;\n" + + "import java.lang.Override;\n" + + "public class ToProcess_ViewBinding extends SubClassTest_ViewBinding {\n" + + " private ToProcess target;\n" + + " @UiThread\n" + + " public ToProcess_ViewBinding(ToProcess target, View source) {\n" + + " super(target, source);\n" + + " this.target = target;\n" + + " target.view3 = Utils.findRequiredView(source, 3, \"field 'view3'\");\n" + + " }\n" + + " @Override\n" + + " public void unbind() {\n" + + " ToProcess target = this.target;\n" + + " if (target == null) throw new IllegalStateException(\"Bindings already cleared.\");\n" + + " this.target = null;\n" + + " target.view3 = null;\n" + + " super.unbind();\n" + + " }\n" + + "}" + ); + + File classesOut = tmp.newFolder("classes-output"); + File sourcesOut = tmp.newFolder("sources-output"); + compileSources(classesOut, sourcesOut, classpathClass, subclassInClasspath); + + try (URLClassLoader compilationClasspath = new URLClassLoader( + new URL[]{classesOut.toURI().toURL()}, this.getClass().getClassLoader())) { + assertAbout(javaSource()).that(toProcessSource) + .withCompilerOptions("-Xlint:-processing") + .withClasspathFrom(compilationClasspath) + .processedWith(new ButterKnifeProcessor()) + .compilesWithoutWarnings() + .and() + .generatesSources(bindingSource); + } + } + + @Test + public void viewNotRequiredInConstructor() throws IOException { + JavaFileObject baseClass = JavaFileObjects.forSourceString("test.Test", "" + + "package test;\n" + + "import butterknife.BindFloat;\n" + + "public class Test {\n" + + " @BindFloat(1) float one;\n" + + "}" + ); + + JavaFileObject subClassSource = JavaFileObjects.forSourceString("test.SubClass", "" + + "package test;\n" + + "import butterknife.BindFloat;\n" + + "public class SubClass extends Test {\n" + + " @BindFloat(2) float two;\n" + + "}" + ); + + JavaFileObject bindingSource = JavaFileObjects.forSourceString("test/SubClass_ViewBinding", "" + + "// Generated code from Butter Knife. Do not modify!\n" + + "package test;\n" + + "import android.content.Context;\n" + + "import android.view.View;\n" + + "import androidx.annotation.UiThread;\n" + + "import butterknife.internal.Utils;\n" + + "import java.lang.Deprecated;\n" + + "import java.lang.SuppressWarnings;\n" + + "public class SubClass_ViewBinding extends Test_ViewBinding {\n" + + " /**\n" + + " * @deprecated Use {@link #SubClass_ViewBinding(SubClass, Context)} for direct creation.\n" + + " * Only present for runtime invocation through {@code ButterKnife.bind()}.\n" + + " */\n" + + " @Deprecated\n" + + " @UiThread\n" + + " public SubClass_ViewBinding(SubClass target, View source) {\n" + + " this(target, source.getContext());\n" + + " }\n" + + " @UiThread\n" + + " @SuppressWarnings(\"ResourceType\")\n" + + " public SubClass_ViewBinding(SubClass target, Context context) {\n" + + " super(target, context);\n" + + " target.two = Utils.getFloat(context, 2);\n" + + " }\n" + + "}" + ); + + File classesOut = tmp.newFolder("classes-output"); + File sourcesOut = tmp.newFolder("sources-output"); + compileSources(classesOut, sourcesOut, baseClass); + + try (URLClassLoader compilationClasspath = new URLClassLoader( + new URL[]{classesOut.toURI().toURL()}, this.getClass().getClassLoader())) { + assertAbout(javaSource()).that(subClassSource) + .withCompilerOptions("-Xlint:-processing") + .withClasspathFrom(compilationClasspath) + .processedWith(new ButterKnifeProcessor()) + .compilesWithoutWarnings() + .and() + .generatesSources(bindingSource); + } + } + + private void compileSources(File classesOut, File sourcesOut, JavaFileObject... sources) { + JavaCompiler javaCompiler = ToolProvider.getSystemJavaCompiler(); + try { + try (StandardJavaFileManager fileManager = + javaCompiler.getStandardFileManager(null, Locale.getDefault(), UTF_8)) { + JavaCompiler.CompilationTask javaCompilerTask = javaCompiler.getTask(null, + fileManager, + null, + ImmutableList.of("-d", classesOut.getCanonicalPath(), "-s", sourcesOut.getCanonicalPath()), + ImmutableList.of(), + Arrays.asList(sources)); + javaCompilerTask.setProcessors(ImmutableList.of(new ButterKnifeProcessor())); + javaCompilerTask.call(); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } +}