From f1df8516b057c77d4f35a2866709f6601dd895b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ste=CC=81phane=20Nicolas?= Date: Fri, 4 Aug 2017 09:26:30 -0700 Subject: [PATCH 1/5] Add support for inheritance in modules --- .../compiler/ButterKnifeProcessor.java | 40 +++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java b/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java index e67a68470..32ccede23 100644 --- a/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java +++ b/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java @@ -1,5 +1,6 @@ package butterknife.compiler; +import android.support.annotation.NonNull; import butterknife.BindAnim; import butterknife.BindArray; import butterknife.BindBitmap; @@ -50,10 +51,10 @@ import java.util.Arrays; import java.util.BitSet; import java.util.Deque; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; -import java.util.HashSet; import java.util.Map; import java.util.Set; import javax.annotation.processing.AbstractProcessor; @@ -75,6 +76,7 @@ import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import javax.lang.model.type.TypeVariable; +import javax.lang.model.util.ElementFilter; import javax.lang.model.util.Elements; import javax.lang.model.util.Types; import javax.tools.Diagnostic.Kind; @@ -367,6 +369,12 @@ private Map findAndParseTargets(RoundEnvironment env) { bindingMap.put(type, builder.build()); } else { BindingSet parentBinding = bindingMap.get(parentType); + + // parent binding is null, let's try to find a previouly generated binding + if(parentBinding == null && hasViewBinder(parentType)) { + parentBinding = createStubBindingSet(parentType); + } + if (parentBinding != null) { builder.setParent(parentBinding); bindingMap.put(type, builder.build()); @@ -380,7 +388,33 @@ private Map findAndParseTargets(RoundEnvironment env) { return bindingMap; } - private void logParsingError(Element element, Class annotation, + @NonNull private BindingSet createStubBindingSet(TypeElement parentType) { + BindingSet parentBinding; + BindingSet.Builder parentBuilder = BindingSet.newBuilder(parentType); + if(hasViewBindings(parentType)) { + //add a fake field to the parent class so that it will indicate it has a view bindings. + //this is required for the subclass to generate a proper view binder + parentBuilder.addField(new Id(-1), new FieldViewBinding("", null, false)); + } + parentBinding = parentBuilder.build(); + return parentBinding; + } + + private boolean hasViewBindings(TypeElement parentType) { + for (VariableElement fieldElement : ElementFilter.fieldsIn(parentType.getEnclosedElements())) { + if (fieldElement.getAnnotation(BindView.class) != null || fieldElement.getAnnotation(BindViews.class) != null) { + return true; + } + } + return false; + } + + private boolean hasViewBinder(TypeElement typeElement) { + final String viewBindingClassName = typeElement.getQualifiedName().toString() + "_ViewBinding"; + return elementUtils.getTypeElement(viewBindingClassName) != null; + } + + private void logParsingError(Element element, Class annotation, Exception e) { StringWriter stackTrace = new StringWriter(); e.printStackTrace(new PrintWriter(stackTrace)); @@ -1284,7 +1318,7 @@ private TypeElement findParentType(TypeElement typeElement, Set par return null; } typeElement = (TypeElement) ((DeclaredType) type).asElement(); - if (parents.contains(typeElement)) { + if (parents.contains(typeElement) || hasViewBinder(typeElement)) { return typeElement; } } From cbc7c47e662dde0ebf10d549b45aeb4ce532a5a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ste=CC=81phane=20Nicolas?= Date: Fri, 4 Aug 2017 09:50:11 -0700 Subject: [PATCH 2/5] make BK support inheritance between modules --- .../compiler/ButterKnifeProcessor.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java b/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java index 32ccede23..390407de8 100644 --- a/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java +++ b/butterknife-compiler/src/main/java/butterknife/compiler/ButterKnifeProcessor.java @@ -371,7 +371,7 @@ private Map findAndParseTargets(RoundEnvironment env) { BindingSet parentBinding = bindingMap.get(parentType); // parent binding is null, let's try to find a previouly generated binding - if(parentBinding == null && hasViewBinder(parentType)) { + if (parentBinding == null && hasViewBinder(parentType)) { parentBinding = createStubBindingSet(parentType); } @@ -391,7 +391,7 @@ private Map findAndParseTargets(RoundEnvironment env) { @NonNull private BindingSet createStubBindingSet(TypeElement parentType) { BindingSet parentBinding; BindingSet.Builder parentBuilder = BindingSet.newBuilder(parentType); - if(hasViewBindings(parentType)) { + if (hasViewBindings(parentType)) { //add a fake field to the parent class so that it will indicate it has a view bindings. //this is required for the subclass to generate a proper view binder parentBuilder.addField(new Id(-1), new FieldViewBinding("", null, false)); @@ -402,7 +402,8 @@ private Map findAndParseTargets(RoundEnvironment env) { private boolean hasViewBindings(TypeElement parentType) { for (VariableElement fieldElement : ElementFilter.fieldsIn(parentType.getEnclosedElements())) { - if (fieldElement.getAnnotation(BindView.class) != null || fieldElement.getAnnotation(BindViews.class) != null) { + if (fieldElement.getAnnotation(BindView.class) != null + || fieldElement.getAnnotation(BindViews.class) != null) { return true; } } @@ -410,11 +411,11 @@ private boolean hasViewBindings(TypeElement parentType) { } private boolean hasViewBinder(TypeElement typeElement) { - final String viewBindingClassName = typeElement.getQualifiedName().toString() + "_ViewBinding"; - return elementUtils.getTypeElement(viewBindingClassName) != null; - } + final String viewBindingClassName = typeElement.getQualifiedName().toString() + "_ViewBinding"; + return elementUtils.getTypeElement(viewBindingClassName) != null; + } - private void logParsingError(Element element, Class annotation, + private void logParsingError(Element element, Class annotation, Exception e) { StringWriter stackTrace = new StringWriter(); e.printStackTrace(new PrintWriter(stackTrace)); From f68da6bfe37a3751d749f0d15cb5db09c5d63a54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ste=CC=81phane=20Nicolas?= Date: Fri, 4 Aug 2017 10:06:34 -0700 Subject: [PATCH 3/5] Add module to sample to demonstrate that it works --- sample/base-library/build.gradle | 27 +++++++++++++++++++ .../base-library/src/main/AndroidManifest.xml | 1 + .../butterknife/baselibrary/BaseActivity.java | 11 ++++++++ .../src/main/res/values/strings.xml | 8 ++++++ sample/library/build.gradle | 7 ++++- .../butterknife/library/SimpleActivity.java | 12 ++++----- .../library/src/main/res/values/strings.xml | 5 ---- settings.gradle | 1 + 8 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 sample/base-library/build.gradle create mode 100644 sample/base-library/src/main/AndroidManifest.xml create mode 100644 sample/base-library/src/main/java/com/example/butterknife/baselibrary/BaseActivity.java create mode 100644 sample/base-library/src/main/res/values/strings.xml delete mode 100644 sample/library/src/main/res/values/strings.xml diff --git a/sample/base-library/build.gradle b/sample/base-library/build.gradle new file mode 100644 index 000000000..df6bacd6f --- /dev/null +++ b/sample/base-library/build.gradle @@ -0,0 +1,27 @@ +buildscript { + repositories { + mavenCentral() + jcenter() + } + + dependencies { + classpath "com.jakewharton:butterknife-gradle-plugin:${versions.release}" + } +} + +apply plugin: 'com.android.library' +apply plugin: 'com.jakewharton.butterknife' + +android { + compileSdkVersion versions.compileSdk + buildToolsVersion versions.buildTools + + defaultConfig { + minSdkVersion versions.minSdk + } +} + +dependencies { + compile deps.release.runtime + annotationProcessor deps.release.compiler +} diff --git a/sample/base-library/src/main/AndroidManifest.xml b/sample/base-library/src/main/AndroidManifest.xml new file mode 100644 index 000000000..72d0525b8 --- /dev/null +++ b/sample/base-library/src/main/AndroidManifest.xml @@ -0,0 +1 @@ + diff --git a/sample/base-library/src/main/java/com/example/butterknife/baselibrary/BaseActivity.java b/sample/base-library/src/main/java/com/example/butterknife/baselibrary/BaseActivity.java new file mode 100644 index 000000000..f4a3f8f71 --- /dev/null +++ b/sample/base-library/src/main/java/com/example/butterknife/baselibrary/BaseActivity.java @@ -0,0 +1,11 @@ +package com.example.butterknife.baselibrary; + +import android.app.Activity; +import butterknife.BindString; + +public class BaseActivity extends Activity { + @BindString(R2.string.app_name) protected String butterKnife; + @BindString(R2.string.field_method) protected String fieldMethod; + @BindString(R2.string.by_jake_wharton) protected String byJakeWharton; + @BindString(R2.string.say_hello) protected String sayHello; +} diff --git a/sample/base-library/src/main/res/values/strings.xml b/sample/base-library/src/main/res/values/strings.xml new file mode 100644 index 000000000..11580e252 --- /dev/null +++ b/sample/base-library/src/main/res/values/strings.xml @@ -0,0 +1,8 @@ + + + + Butter Knife + Field and method binding for Android views. + by Jake Wharton + Say Hello + diff --git a/sample/library/build.gradle b/sample/library/build.gradle index 27111f3ee..2f5d21442 100644 --- a/sample/library/build.gradle +++ b/sample/library/build.gradle @@ -24,7 +24,12 @@ android { dependencies { implementation deps.release.runtime - annotationProcessor deps.release.compiler + implementation project(':sample:base-library') + + //TODO this change is just to demonstrate that the new compiler works well + //but should be reversed when the new version is released + //annotationProcessor deps.release.compiler + annotationProcessor project(':butterknife-compiler') testImplementation deps.junit testImplementation deps.truth diff --git a/sample/library/src/main/java/com/example/butterknife/library/SimpleActivity.java b/sample/library/src/main/java/com/example/butterknife/library/SimpleActivity.java index 5f40f5af0..c8e1768ef 100644 --- a/sample/library/src/main/java/com/example/butterknife/library/SimpleActivity.java +++ b/sample/library/src/main/java/com/example/butterknife/library/SimpleActivity.java @@ -1,7 +1,6 @@ package com.example.butterknife.library; import android.annotation.SuppressLint; -import android.app.Activity; import android.os.Bundle; import android.support.annotation.NonNull; import android.view.View; @@ -16,11 +15,12 @@ import butterknife.OnClick; import butterknife.OnItemClick; import butterknife.OnLongClick; +import com.example.butterknife.baselibrary.BaseActivity; import java.util.List; import static android.widget.Toast.LENGTH_SHORT; -public class SimpleActivity extends Activity { +public class SimpleActivity extends BaseActivity { private static final ButterKnife.Action ALPHA_FADE = new ButterKnife.Action() { @Override public void apply(@NonNull View view, int index) { AlphaAnimation alphaAnimation = new AlphaAnimation(0, 1); @@ -62,10 +62,10 @@ public class SimpleActivity extends Activity { ButterKnife.bind(this); // Contrived code to use the bound fields. - title.setText("Butter Knife"); - subtitle.setText("Field and method binding for Android views."); - footer.setText("by Jake Wharton"); - hello.setText("Say Hello"); + title.setText(butterKnife); + subtitle.setText(fieldMethod); + footer.setText(byJakeWharton); + hello.setText(sayHello); adapter = new SimpleAdapter(this); listOfThings.setAdapter(adapter); diff --git a/sample/library/src/main/res/values/strings.xml b/sample/library/src/main/res/values/strings.xml deleted file mode 100644 index b73bb3432..000000000 --- a/sample/library/src/main/res/values/strings.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - - Butter Knife - \ No newline at end of file diff --git a/settings.gradle b/settings.gradle index cd8829a5a..71c34c60f 100644 --- a/settings.gradle +++ b/settings.gradle @@ -6,6 +6,7 @@ include ':butterknife-lint' include ':butterknife-integration-test' //include ':sample:app' +//include ':sample:base-library' //include ':sample:library' rootProject.name = 'butterknife-parent' From 3f2f26b45ba71ccc608834ac59b0bdd7958934eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ste=CC=81phane=20Nicolas?= Date: Fri, 4 Aug 2017 10:33:48 -0700 Subject: [PATCH 4/5] Add a unit test that simulates a base class already compiled in a module --- .../java/butterknife/InheritanceTest.java | 62 +++++++++++++++++++ .../java/butterknife/precompiled/Base.java | 8 +++ 2 files changed, 70 insertions(+) create mode 100644 butterknife/src/test/java/butterknife/InheritanceTest.java create mode 100644 butterknife/src/test/java/butterknife/precompiled/Base.java diff --git a/butterknife/src/test/java/butterknife/InheritanceTest.java b/butterknife/src/test/java/butterknife/InheritanceTest.java new file mode 100644 index 000000000..7903a996c --- /dev/null +++ b/butterknife/src/test/java/butterknife/InheritanceTest.java @@ -0,0 +1,62 @@ +package butterknife; + +import butterknife.compiler.ButterKnifeProcessor; +import com.google.common.collect.ImmutableList; +import com.google.testing.compile.JavaFileObjects; +import javax.tools.JavaFileObject; +import javax.tools.StandardLocation; +import org.junit.Test; + +import static com.google.common.truth.Truth.assertAbout; +import static com.google.testing.compile.JavaSourceSubjectFactory.javaSource; +import static com.google.testing.compile.JavaSourcesSubjectFactory.javaSources; +import static java.util.Arrays.asList; + +public class InheritanceTest { + + @Test public void bindingViewFinalClassWithBaseClassAlreadyCompiledInDifferentModule() { + JavaFileObject testSource = JavaFileObjects.forSourceString("test.Test", "" + + "package test;\n" + + "import android.view.View;\n" + + "import butterknife.BindView;\n" + + "import butterknife.precompiled.Base;\n" + + "public final class Test extends Base {\n" + + " @BindView(1) View thing;\n" + + "}" + ); + + JavaFileObject bindingTestSource = JavaFileObjects.forSourceString("test/Test_ViewBinding", "" + + "package test;\n" + + "import android.support.annotation.UiThread;\n" + + "import android.view.View;\n" + + "import butterknife.internal.Utils;\n" + + "import butterknife.precompiled.Base_ViewBinding;\n" + + "import java.lang.IllegalStateException;\n" + + "import java.lang.Override;\n" + + "public final class Test_ViewBinding extends Base_ViewBinding {\n" + + " private Test target;\n" + + " @UiThread\n" + + " public Test_ViewBinding(Test target, View source) {\n" + + " super(target, source);\n" + + " this.target = target;\n" + + " target.thing = Utils.findRequiredView(source, 1, \"field 'thing'\");\n" + + " }\n" + + " @Override\n" + + " public void unbind() {\n" + + " Test target = this.target;\n" + + " if (target == null) throw new IllegalStateException(\"Bindings already cleared.\");\n" + + " this.target = null\n" + + " target.thing = null;\n" + + " super.unbind();\n" + + " }\n" + + "}" + ); + + assertAbout(javaSources()).that(asList(testSource)) + .withCompilerOptions("-Xlint:-processing") + .processedWith(new ButterKnifeProcessor()) + .compilesWithoutWarnings() + .and() + .generatesSources(bindingTestSource); + } +} diff --git a/butterknife/src/test/java/butterknife/precompiled/Base.java b/butterknife/src/test/java/butterknife/precompiled/Base.java new file mode 100644 index 000000000..7d809d607 --- /dev/null +++ b/butterknife/src/test/java/butterknife/precompiled/Base.java @@ -0,0 +1,8 @@ +package butterknife.precompiled; + +import android.view.View; +import butterknife.BindView; + +public class Base { + @BindView(1) View thing; +} From 664f760798c18b9f17f34a46cbfb2f747fa90ef4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ste=CC=81phane=20Nicolas?= Date: Fri, 4 Aug 2017 12:02:55 -0700 Subject: [PATCH 5/5] fix build, we need to explicitly pass auto dep with the new model --- butterknife/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/butterknife/build.gradle b/butterknife/build.gradle index 8577bb417..ec6b8e18d 100644 --- a/butterknife/build.gradle +++ b/butterknife/build.gradle @@ -41,6 +41,7 @@ dependencies { testImplementation deps.junit testImplementation deps.truth testImplementation deps.compiletesting + testImplementation deps.auto.common testImplementation files(getRuntimeJar()) testImplementation files(org.gradle.internal.jvm.Jvm.current().getToolsJar()) testImplementation project(':butterknife-compiler')