Check accepted states in overridden method and call it just once.
Bug: 32342385
Test: Unit tests in the CL.
Change-Id: I8f08fbb24a9e270575d1bb6a13409b89895555da
diff --git a/lifecycle/compiler/src/main/kotlin/com/android/support/lifecycle/LifecycleProcessor.kt b/lifecycle/compiler/src/main/kotlin/com/android/support/lifecycle/LifecycleProcessor.kt
index a88cfca..8497dc0 100644
--- a/lifecycle/compiler/src/main/kotlin/com/android/support/lifecycle/LifecycleProcessor.kt
+++ b/lifecycle/compiler/src/main/kotlin/com/android/support/lifecycle/LifecycleProcessor.kt
@@ -39,6 +39,8 @@
const val INVALID_METHOD_MODIFIER = "method marked with OnState annotation can not be " +
"private"
const val INVALID_CLASS_MODIFIER = "class containing OnState methods can not be private"
+ const val INVALID_STATE_OVERRIDE_METHOD = "overridden method must handle the same " +
+ "onState changes as original method"
}
private val LIFECYCLE_PROVIDER = ClassName.get(LifecycleProvider::class.java)
@@ -132,10 +134,19 @@
return null
}
- private fun mergeAndVerifyMethods(l1: List<StateMethod>,
- l2: List<StateMethod>): List<StateMethod> {
- //TODO: remove duplicates etc
- return l1 + l2
+ private fun mergeAndVerifyMethods(classMethods: List<StateMethod>,
+ parentMethods: List<StateMethod>): List<StateMethod> {
+ return parentMethods + classMethods.filter { currentMethod ->
+ val baseMethod = parentMethods.find { m ->
+ currentMethod.method.simpleName == m.method.simpleName
+ && currentMethod.method.parameters.size == m.method.parameters.size
+ }
+ if (baseMethod != null && baseMethod.onState != currentMethod.onState) {
+ printErrorMessage(INVALID_STATE_OVERRIDE_METHOD, currentMethod.method)
+ }
+ baseMethod == null
+ }
+
}
private fun flattenObserverInfos(
@@ -157,7 +168,7 @@
val flat = flattened[sObserver]
flattened[observer] = LifecycleObserverInfo(observer.type,
- mergeAndVerifyMethods(flat!!.methods, observer.methods))
+ mergeAndVerifyMethods(observer.methods, flat!!.methods))
}
world.values.forEach(::traverse)
diff --git a/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/InvalidCasesTest.kt b/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/InvalidCasesTest.kt
new file mode 100644
index 0000000..455e721
--- /dev/null
+++ b/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/InvalidCasesTest.kt
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.support.lifecycle
+
+import com.android.support.lifecycle.utils.processClass
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+@RunWith(Parameterized::class)
+class InvalidCasesTest(val name: String, val errorMsg: String) {
+ companion object {
+ @JvmStatic
+ @Parameterized.Parameters(name = "failingCase({0})")
+ fun data() : Collection<Array<Any>> = listOf(
+ arrayOf<Any>("InvalidFirstArg", LifecycleProcessor.INVALID_FIRST_ARGUMENT),
+ arrayOf<Any>("InvalidSecondArg", LifecycleProcessor.INVALID_SECOND_ARGUMENT),
+ arrayOf<Any>("TooManyArgs", LifecycleProcessor.TOO_MANY_ARGS_ERROR_MSG),
+ arrayOf<Any>("TooManyArgs", LifecycleProcessor.TOO_MANY_ARGS_ERROR_MSG),
+ arrayOf<Any>("InvalidMethodModifier", LifecycleProcessor.INVALID_METHOD_MODIFIER),
+ arrayOf<Any>("InvalidClassModifier", LifecycleProcessor.INVALID_CLASS_MODIFIER),
+ arrayOf<Any>("InvalidInheritance", LifecycleProcessor.INVALID_STATE_OVERRIDE_METHOD)
+ )
+ }
+
+ @Test
+ fun shouldFailWithError() {
+ processClass(name).failsToCompile().withErrorContaining(errorMsg)
+ }
+}
\ No newline at end of file
diff --git a/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/ProcessorTest.kt b/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/ProcessorTest.kt
deleted file mode 100644
index fb9856c..0000000
--- a/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/ProcessorTest.kt
+++ /dev/null
@@ -1,94 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-
-package com.android.support.lifecycle
-
-import org.junit.Test
-import org.junit.runner.RunWith
-import org.junit.runners.JUnit4
-import com.google.common.truth.Truth.assertAbout
-import com.google.testing.compile.CompileTester
-import com.google.testing.compile.JavaFileObjects
-import com.google.testing.compile.JavaSourceSubjectFactory.javaSource
-import java.io.File
-import java.nio.charset.Charset
-import javax.tools.JavaFileObject
-
-@RunWith(JUnit4::class)
-class ProcessorTest {
-
- fun load(className: String, folder: String = ""): JavaFileObject {
- val folderPath = "src/tests/test-data/${if (folder.isEmpty()) "" else folder + "/" }"
- val code = File("$folderPath/$className.java").readText(Charset.defaultCharset())
- return JavaFileObjects.forSourceString("foo.$className", code);
- }
-
- fun processClass(className: String): CompileTester {
- val processedWith = assertAbout(javaSource())
- .that(load(className))
- .processedWith(LifecycleProcessor())
- return checkNotNull(processedWith)
- }
-
- @Test
- fun testTest() {
- processClass("Bar").compilesWithoutError()
- }
-
- @Test
- fun testInvalidMethodModifier() {
- processClass("InvalidMethodModifier").failsToCompile()?.withErrorContaining(
- LifecycleProcessor.INVALID_METHOD_MODIFIER)
- }
-
- @Test
- fun testInvalidClassModifier() {
- processClass("InvalidClassModifier").failsToCompile()?.withErrorContaining(
- LifecycleProcessor.INVALID_CLASS_MODIFIER)
- }
-
- @Test
- fun testTooManyArguments() {
- processClass("TooManyArgs").failsToCompile()?.withErrorContaining(
- LifecycleProcessor.TOO_MANY_ARGS_ERROR_MSG)
- }
-
- @Test
- fun testInvalidFirstArg() {
- processClass("InvalidFirstArg").failsToCompile()?.withErrorContaining(
- LifecycleProcessor.INVALID_FIRST_ARGUMENT)
- }
-
- @Test
- fun testInvalidSecondArg() {
- processClass("InvalidSecondArg").failsToCompile()?.withErrorContaining(
- LifecycleProcessor.INVALID_SECOND_ARGUMENT)
- }
-
- @Test
- fun testInheritance() {
- processClass("InheritanceOk").compilesWithoutError()
- }
-
- @Test
- fun testInheritance2() {
- processClass("InheritanceOk2").compilesWithoutError().and().generatesSources(
- load("InheritanceOk2Base_LifecycleAdapter", "expected"),
- load("InheritanceOk2Derived_LifecycleAdapter", "expected")
- )
- }
-}
\ No newline at end of file
diff --git a/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/ValidCasesTest.kt b/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/ValidCasesTest.kt
new file mode 100644
index 0000000..2a1ee5d
--- /dev/null
+++ b/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/ValidCasesTest.kt
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.support.lifecycle
+
+import com.android.support.lifecycle.utils.load
+import com.android.support.lifecycle.utils.processClass
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+@RunWith(JUnit4::class)
+class ValidCasesTest {
+ @Test
+ fun testTest() {
+ processClass("Bar").compilesWithoutError()
+ }
+
+ @Test
+ fun testInheritance() {
+ processClass("InheritanceOk1").compilesWithoutError()
+ }
+
+ @Test
+ fun testInheritance2() {
+ processClass("InheritanceOk2").compilesWithoutError().and().generatesSources(
+ load("InheritanceOk2Base_LifecycleAdapter", "expected"),
+ load("InheritanceOk2Derived_LifecycleAdapter", "expected")
+ )
+ }
+
+ @Test
+ fun testInheritance3() {
+ processClass("InheritanceOk3").compilesWithoutError().and().generatesSources(
+ load("InheritanceOk3Base_LifecycleAdapter", "expected"),
+ load("InheritanceOk3Derived_LifecycleAdapter", "expected")
+ )
+ }
+}
\ No newline at end of file
diff --git a/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/utils/TestUtils.kt b/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/utils/TestUtils.kt
new file mode 100644
index 0000000..b3f78d4
--- /dev/null
+++ b/lifecycle/compiler/src/tests/kotlin/com/android/support/lifecycle/utils/TestUtils.kt
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.support.lifecycle.utils
+
+import com.android.support.lifecycle.LifecycleProcessor
+import com.google.common.truth.Truth
+import com.google.testing.compile.CompileTester
+import com.google.testing.compile.JavaFileObjects
+import com.google.testing.compile.JavaSourceSubjectFactory
+import java.io.File
+import java.nio.charset.Charset
+import javax.tools.JavaFileObject
+
+fun load(className: String, folder: String = ""): JavaFileObject {
+ val folderPath = "src/tests/test-data/${if (folder.isEmpty()) "" else folder + "/" }"
+ val code = File("$folderPath/$className.java").readText(Charset.defaultCharset())
+ return JavaFileObjects.forSourceString("foo.$className", code);
+}
+
+fun processClass(className: String): CompileTester {
+ val processedWith = Truth.assertAbout(JavaSourceSubjectFactory.javaSource())
+ .that(load(className))
+ .processedWith(LifecycleProcessor())
+ return checkNotNull(processedWith)
+}
diff --git a/lifecycle/compiler/src/tests/test-data/InheritanceOk.java b/lifecycle/compiler/src/tests/test-data/InheritanceOk.java
deleted file mode 100644
index b10c198..0000000
--- a/lifecycle/compiler/src/tests/test-data/InheritanceOk.java
+++ /dev/null
@@ -1,33 +0,0 @@
-package foo;
-import com.android.support.lifecycle.OnState;
-import static com.android.support.lifecycle.Lifecycle.STARTED;
-import static com.android.support.lifecycle.Lifecycle.STOPPED;
-import com.android.support.lifecycle.LifecycleProvider;
-import java.util.HashMap;
-
-class Base1 {
- @OnState(STOPPED)
- public void onStop(LifecycleProvider provider, int prevstate){}
-}
-
-class Proxy extends Base1 { }
-
-class Derived1 extends Proxy {
- @OnState(STOPPED)
- public void onStop2(LifecycleProvider provider, int prevstate){}
-}
-
-class Derived2 extends Proxy {
- @OnState(STOPPED)
- public void onStop2(LifecycleProvider provider, int prevstate){}
-}
-
-class Base2 {
- @OnState(STOPPED)
- public void onStop(LifecycleProvider provider, int prevstate){}
-}
-
-class Derived3 extends Base2 {
- @OnState(STOPPED)
- public void onStop2(LifecycleProvider provider, int prevstate){}
-}
diff --git a/lifecycle/compiler/src/tests/test-data/InheritanceOk1.java b/lifecycle/compiler/src/tests/test-data/InheritanceOk1.java
new file mode 100644
index 0000000..abef84c
--- /dev/null
+++ b/lifecycle/compiler/src/tests/test-data/InheritanceOk1.java
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package foo;
+
+import static com.android.support.lifecycle.Lifecycle.STOPPED;
+
+import com.android.support.lifecycle.LifecycleProvider;
+import com.android.support.lifecycle.OnState;
+
+class Base1 {
+ @OnState(STOPPED)
+ public void onStop(LifecycleProvider provider, int prevstate){}
+}
+
+class Proxy extends Base1 { }
+
+class Derived1 extends Proxy {
+ @OnState(STOPPED)
+ public void onStop2(LifecycleProvider provider, int prevstate){}
+}
+
+class Derived2 extends Proxy {
+ @OnState(STOPPED)
+ public void onStop2(LifecycleProvider provider, int prevstate){}
+}
+
+class Base2 {
+ @OnState(STOPPED)
+ public void onStop(LifecycleProvider provider, int prevstate){}
+}
+
+class Derived3 extends Base2 {
+ @OnState(STOPPED)
+ public void onStop2(LifecycleProvider provider, int prevstate){}
+}
diff --git a/lifecycle/compiler/src/tests/test-data/InheritanceOk3.java b/lifecycle/compiler/src/tests/test-data/InheritanceOk3.java
new file mode 100644
index 0000000..a2abe98
--- /dev/null
+++ b/lifecycle/compiler/src/tests/test-data/InheritanceOk3.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package foo;
+
+import com.android.support.lifecycle.LifecycleProvider;
+import com.android.support.lifecycle.OnState;
+
+import static com.android.support.lifecycle.Lifecycle.STOPPED;
+
+class InheritanceOk3Base {
+ @OnState(STOPPED)
+ public void onStop(LifecycleProvider provider, int prevstate){}
+}
+
+class InheritanceOk3Derived extends InheritanceOk3Base {
+ @OnState(STOPPED)
+ public void onStop(LifecycleProvider provider, int prevstate){}
+}
diff --git a/lifecycle/compiler/src/tests/test-data/InvalidInheritance.java b/lifecycle/compiler/src/tests/test-data/InvalidInheritance.java
new file mode 100644
index 0000000..f3ef168
--- /dev/null
+++ b/lifecycle/compiler/src/tests/test-data/InvalidInheritance.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package foo;
+
+import com.android.support.lifecycle.OnState;
+
+import static com.android.support.lifecycle.Lifecycle.STARTED;
+import static com.android.support.lifecycle.Lifecycle.STOPPED;
+
+class Base {
+ @OnState(STOPPED)
+ void onStop(){}
+}
+
+class Derived extends Base {
+ @OnState(STARTED | STOPPED)
+ void onStop(){}
+}
diff --git a/lifecycle/compiler/src/tests/test-data/expected/InheritanceOk3Base_LifecycleAdapter.java b/lifecycle/compiler/src/tests/test-data/expected/InheritanceOk3Base_LifecycleAdapter.java
new file mode 100644
index 0000000..95f083e
--- /dev/null
+++ b/lifecycle/compiler/src/tests/test-data/expected/InheritanceOk3Base_LifecycleAdapter.java
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package foo;
+
+import com.android.support.lifecycle.GenericLifecycleObserver;
+import com.android.support.lifecycle.LifecycleProvider;
+
+import java.lang.Object;
+import java.lang.Override;
+
+class InheritanceOk3Base_LifecycleAdapter implements GenericLifecycleObserver {
+ final InheritanceOk3Base mReceiver;
+
+ InheritanceOk3Base_LifecycleAdapter(InheritanceOk3Base receiver) {
+ this.mReceiver = receiver;
+ }
+
+ @Override
+ public void onStateChanged(LifecycleProvider provider, int previousState) {
+ final int curState = provider.getLifecycle().getCurrentState();
+ if ((curState & 1024) != 0) {
+ mReceiver.onStop(provider, previousState);
+ }
+ }
+
+ public Object getReceiver() {
+ return mReceiver;
+ }
+}
diff --git a/lifecycle/compiler/src/tests/test-data/expected/InheritanceOk3Derived_LifecycleAdapter.java b/lifecycle/compiler/src/tests/test-data/expected/InheritanceOk3Derived_LifecycleAdapter.java
new file mode 100644
index 0000000..a3d3340
--- /dev/null
+++ b/lifecycle/compiler/src/tests/test-data/expected/InheritanceOk3Derived_LifecycleAdapter.java
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package foo;
+
+import com.android.support.lifecycle.GenericLifecycleObserver;
+import com.android.support.lifecycle.LifecycleProvider;
+
+import java.lang.Object;
+import java.lang.Override;
+
+class InheritanceOk3Derived_LifecycleAdapter implements GenericLifecycleObserver {
+ final InheritanceOk3Derived mReceiver;
+
+ InheritanceOk3Derived_LifecycleAdapter(InheritanceOk3Derived receiver) {
+ this.mReceiver = receiver;
+ }
+
+ @Override
+ public void onStateChanged(LifecycleProvider provider, int previousState) {
+ final int curState = provider.getLifecycle().getCurrentState();
+ if ((curState & 1024) != 0) {
+ mReceiver.onStop(provider, previousState);
+ }
+ }
+
+ public Object getReceiver() {
+ return mReceiver;
+ }
+}