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); + } + } +}