Parcelable: Add declaration checker

2017-06-27 16:51:39 +03:00
parent c23bca6afe
commit 4197380621
14 changed files with 421 additions and 0 deletions
@@ -1325,6 +1325,10 @@ fun main(args: Array<String>) {
        testClass<AbstractAndroidExtractionTest> {
            model("android/extraction", recursive = false, extension = null)
        }
+
+        testClass<AbstractParcelCheckerTest> {
+            model("android/parcel/checker", excludeParentDirs = true)
+        }
    }

    testGroup("idea/idea-android/tests", "idea/testData") {
@@ -0,0 +1,116 @@
+/*
+ * Copyright 2010-2017 JetBrains s.r.o.
+ *
+ * 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 org.jetbrains.kotlin.android.parcel
+
+import org.jetbrains.kotlin.android.synthetic.diagnostic.ErrorsAndroid
+import org.jetbrains.kotlin.descriptors.ClassDescriptor
+import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
+import org.jetbrains.kotlin.descriptors.PropertyDescriptor
+import org.jetbrains.kotlin.diagnostics.DiagnosticSink
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.psi.KtClass
+import org.jetbrains.kotlin.psi.KtClassOrObject
+import org.jetbrains.kotlin.psi.KtDeclaration
+import org.jetbrains.kotlin.psi.KtProperty
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.checkers.SimpleDeclarationChecker
+import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
+import org.jetbrains.kotlin.types.TypeUtils
+
+private val ANDROID_PARCELABLE_CLASS_FQNAME = FqName("android.os.Parcelable")
+
+class ParcelableDeclarationChecker : SimpleDeclarationChecker {
+    private companion object {
+        private val TRANSIENT_FQNAME = FqName(Transient::class.java.canonicalName)
+    }
+
+    override fun check(
+            declaration: KtDeclaration,
+            descriptor: DeclarationDescriptor,
+            diagnosticHolder: DiagnosticSink,
+            bindingContext: BindingContext
+    ) {
+        when (descriptor) {
+            is ClassDescriptor -> checkParcelableClass(descriptor, declaration, diagnosticHolder)
+            is PropertyDescriptor -> {
+                val containingClass = descriptor.containingDeclaration as? ClassDescriptor
+                val ktProperty = declaration as? KtProperty
+                if (containingClass != null && ktProperty != null) {
+                    checkParcelableClassProperty(descriptor, containingClass, ktProperty, diagnosticHolder)
+                }
+            }
+        }
+    }
+
+    private fun checkParcelableClassProperty(
+            property: PropertyDescriptor,
+            containingClass: ClassDescriptor,
+            declaration: KtProperty,
+            diagnosticHolder: DiagnosticSink
+    ) {
+        if (!containingClass.isMagicParcelable) return
+
+        // Do not report on calculated properties
+        if (declaration.getter?.hasBody() == true) return
+
+        if (!property.annotations.hasAnnotation(TRANSIENT_FQNAME)) {
+            diagnosticHolder.report(ErrorsAndroid.PROPERTY_WONT_BE_SERIALIZED.on(declaration.nameIdentifier ?: declaration))
+        }
+    }
+
+    private fun checkParcelableClass(descriptor: ClassDescriptor, declaration: KtDeclaration, diagnosticHolder: DiagnosticSink) {
+        if (!descriptor.isMagicParcelable) return
+
+        if (declaration !is KtClass || (declaration.isAnnotation() || declaration.isInterface() || declaration.isEnum())) {
+            val reportElement = (declaration as? KtClassOrObject)?.nameIdentifier ?: declaration
+            diagnosticHolder.report(ErrorsAndroid.PARCELABLE_SHOULD_BE_CLASS.on(reportElement))
+            return
+        }
+
+        val sealedOrAbstract = declaration.modifierList?.let { it.getModifier(KtTokens.ABSTRACT_KEYWORD) ?: it.getModifier(KtTokens.SEALED_KEYWORD) }
+        if (sealedOrAbstract != null) {
+            diagnosticHolder.report(ErrorsAndroid.PARCELABLE_SHOULD_BE_INSTANTIABLE.on(sealedOrAbstract))
+        }
+
+        if (declaration.isInner()) {
+            val reportElement = declaration.modifierList?.getModifier(KtTokens.INNER_KEYWORD) ?: declaration.nameIdentifier ?: declaration
+            diagnosticHolder.report(ErrorsAndroid.PARCELABLE_CANT_BE_INNER_CLASS.on(reportElement))
+        }
+
+        if (declaration.isLocal) {
+            diagnosticHolder.report(ErrorsAndroid.PARCELABLE_CANT_BE_LOCAL_CLASS.on(declaration.nameIdentifier ?: declaration))
+        }
+
+        val superTypes = TypeUtils.getAllSupertypes(descriptor.defaultType)
+        if (superTypes.none { it.constructor.declarationDescriptor?.fqNameSafe == ANDROID_PARCELABLE_CLASS_FQNAME }) {
+            diagnosticHolder.report(ErrorsAndroid.NO_PARCELABLE_SUPERTYPE.on(declaration.nameIdentifier ?: declaration))
+        }
+
+        val primaryConstructor = declaration.primaryConstructor
+        if (primaryConstructor == null && declaration.secondaryConstructors.isNotEmpty()) {
+            diagnosticHolder.report(ErrorsAndroid.PARCELABLE_SHOULD_HAVE_PRIMARY_CONSTRUCTOR.on(declaration.nameIdentifier ?: declaration))
+        }
+
+        for (parameter in primaryConstructor?.valueParameters.orEmpty()) {
+            if (!parameter.hasValOrVar()) {
+                diagnosticHolder.report(ErrorsAndroid.PARCELABLE_CONSTRUCTOR_PARAMETER_SHOULD_BE_VAL_OR_VAR.on(
+                        parameter.nameIdentifier ?: parameter))
+            }
+        }
+    }
+}
@@ -20,6 +20,7 @@ import com.intellij.mock.MockProject
 import com.intellij.openapi.extensions.Extensions
 import com.intellij.openapi.project.Project
 import org.jetbrains.kotlin.android.parcel.ParcelableCodegenExtension
+import org.jetbrains.kotlin.android.parcel.ParcelableDeclarationChecker
 import org.jetbrains.kotlin.android.parcel.ParcelableResolveExtension
 import org.jetbrains.kotlin.android.synthetic.codegen.AndroidOnDestroyClassBuilderInterceptorExtension
 import org.jetbrains.kotlin.android.synthetic.codegen.CliAndroidExtensionsExpressionCodegenExtension
@@ -118,6 +119,7 @@ class AndroidExtensionPropertiesComponentContainerContributor : StorageComponent
    override fun addDeclarations(container: StorageComponentContainer, platform: TargetPlatform) {
        if (platform is JvmPlatform) {
            container.useInstance(AndroidExtensionPropertiesCallChecker())
+            container.useInstance(ParcelableDeclarationChecker())
        }
    }
 }
@@ -39,6 +39,30 @@ class DefaultErrorMessagesAndroid : DefaultErrorMessages.Extension {

            MAP.put(ErrorsAndroid.UNSAFE_CALL_ON_PARTIALLY_DEFINED_RESOURCE,
                    "Potential NullPointerException. The resource is missing in some of layout versions")
+
+            MAP.put(ErrorsAndroid.PARCELABLE_SHOULD_BE_CLASS,
+                    "'Parcelable' should be a class")
+
+            MAP.put(ErrorsAndroid.PARCELABLE_SHOULD_BE_INSTANTIABLE,
+                    "'Parcelable' should not be a 'sealed' or 'abstract' class")
+
+            MAP.put(ErrorsAndroid.PARCELABLE_CANT_BE_INNER_CLASS,
+                    "'Parcelable' can't be an inner class")
+
+            MAP.put(ErrorsAndroid.PARCELABLE_CANT_BE_LOCAL_CLASS,
+                    "'Parcelable' can't be a local class")
+
+            MAP.put(ErrorsAndroid.NO_PARCELABLE_SUPERTYPE,
+                    "No 'Parcelable' supertype")
+
+            MAP.put(ErrorsAndroid.PARCELABLE_SHOULD_HAVE_PRIMARY_CONSTRUCTOR,
+                    "'Parcelable' should have a primary constructor")
+
+            MAP.put(ErrorsAndroid.PARCELABLE_CONSTRUCTOR_PARAMETER_SHOULD_BE_VAL_OR_VAR,
+                    "'Parcelable' constructor parameter should be 'val' or 'var'")
+
+            MAP.put(ErrorsAndroid.PROPERTY_WONT_BE_SERIALIZED,
+                    "Property would not be serialized into a 'Parcel'. Add '@Transient' annotation to it")
        }
    }

@@ -16,11 +16,13 @@

 package org.jetbrains.kotlin.android.synthetic.diagnostic;

+import com.intellij.psi.PsiElement;
 import org.jetbrains.kotlin.diagnostics.DiagnosticFactory0;
 import org.jetbrains.kotlin.diagnostics.DiagnosticFactory1;
 import org.jetbrains.kotlin.diagnostics.Errors;
 import org.jetbrains.kotlin.psi.KtExpression;

+import static org.jetbrains.kotlin.diagnostics.Severity.ERROR;
 import static org.jetbrains.kotlin.diagnostics.Severity.WARNING;

 public interface ErrorsAndroid {
@@ -29,6 +31,15 @@ public interface ErrorsAndroid {
    DiagnosticFactory0<KtExpression> SYNTHETIC_DEPRECATED_PACKAGE = DiagnosticFactory0.create(WARNING);
    DiagnosticFactory0<KtExpression> UNSAFE_CALL_ON_PARTIALLY_DEFINED_RESOURCE = DiagnosticFactory0.create(WARNING);

+    DiagnosticFactory0<PsiElement> PARCELABLE_SHOULD_BE_CLASS = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory0<PsiElement> PARCELABLE_SHOULD_BE_INSTANTIABLE = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory0<PsiElement> PARCELABLE_CANT_BE_INNER_CLASS = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory0<PsiElement> PARCELABLE_CANT_BE_LOCAL_CLASS = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory0<PsiElement> NO_PARCELABLE_SUPERTYPE = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory0<PsiElement> PARCELABLE_SHOULD_HAVE_PRIMARY_CONSTRUCTOR = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory0<PsiElement> PARCELABLE_CONSTRUCTOR_PARAMETER_SHOULD_BE_VAL_OR_VAR = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory0<PsiElement> PROPERTY_WONT_BE_SERIALIZED = DiagnosticFactory0.create(WARNING);
+
    @SuppressWarnings("UnusedDeclaration")
    Object _initializer = new Object() {
        {
@@ -0,0 +1,28 @@
+package test
+
+import kotlinx.android.parcel.MagicParcel
+import android.os.Parcelable
+
+@MagicParcel
+class A : Parcelable
+
+@MagicParcel
+class B(val firstName: String, val secondName: String) : Parcelable
+
+@MagicParcel
+class C(val firstName: String, <error descr="[PARCELABLE_CONSTRUCTOR_PARAMETER_SHOULD_BE_VAL_OR_VAR] 'Parcelable' constructor parameter should be 'val' or 'var'">secondName</error>: String) : Parcelable
+
+@MagicParcel
+class D(val firstName: String, vararg val secondName: String) : Parcelable
+
+@MagicParcel
+class E(val firstName: String, val secondName: String) : Parcelable {
+    constructor() : this("", "")
+}
+
+@MagicParcel
+class <error descr="[PARCELABLE_SHOULD_HAVE_PRIMARY_CONSTRUCTOR] 'Parcelable' should have a primary constructor">F</error> : Parcelable {
+    constructor(a: String) {
+        println(a)
+    }
+}
@@ -0,0 +1,31 @@
+package test
+
+import kotlinx.android.parcel.MagicParcel
+import android.os.Parcelable
+
+@MagicParcel
+open class Open(val foo: String) : Parcelable
+
+@MagicParcel
+class Final(val foo: String) : Parcelable
+
+@MagicParcel
+<error descr="[PARCELABLE_SHOULD_BE_INSTANTIABLE] 'Parcelable' should not be a 'sealed' or 'abstract' class">abstract</error> class Abstract(val foo: String) : Parcelable
+
+@MagicParcel
+<error descr="[PARCELABLE_SHOULD_BE_INSTANTIABLE] 'Parcelable' should not be a 'sealed' or 'abstract' class">sealed</error> class Sealed(val foo: String) : Parcelable {
+    class X : Sealed("")
+}
+
+class Outer {
+    @MagicParcel
+    <error descr="[PARCELABLE_CANT_BE_INNER_CLASS] 'Parcelable' can't be an inner class">inner</error> class Inner(val foo: String) : Parcelable
+}
+
+fun foo() {
+    @MagicParcel
+    <error descr="[PARCELABLE_SHOULD_BE_CLASS] 'Parcelable' should be a class">object</error> : Parcelable {}
+
+    @MagicParcel
+    class <error descr="[NO_PARCELABLE_SUPERTYPE] No 'Parcelable' supertype"><error descr="[PARCELABLE_CANT_BE_LOCAL_CLASS] 'Parcelable' can't be a local class">Local</error></error> {}
+}
@@ -0,0 +1,11 @@
+package test
+
+import kotlinx.android.parcel.MagicParcel
+import android.os.Parcel
+import android.os.Parcelable
+
+@Suppress("UNUSED_PARAMETER")
+class User(firstName: String, secondName: String, val age: Int) : Parcelable {
+    override fun writeToParcel(p0: Parcel?, p1: Int) {}
+    override fun describeContents() = 0
+}
@@ -0,0 +1,20 @@
+package test
+
+import kotlinx.android.parcel.MagicParcel
+import android.os.Parcelable
+
+@MagicParcel
+class A(val firstName: String) : Parcelable {
+    val <warning descr="[PROPERTY_WONT_BE_SERIALIZED] Property would not be serialized into a 'Parcel'. Add '@Transient' annotation to it">secondName</warning>: String = ""
+
+    val <warning descr="[PROPERTY_WONT_BE_SERIALIZED] Property would not be serialized into a 'Parcel'. Add '@Transient' annotation to it">delegated</warning> by lazy { "" }
+
+    lateinit var <warning descr="[PROPERTY_WONT_BE_SERIALIZED] Property would not be serialized into a 'Parcel'. Add '@Transient' annotation to it">lateinit</warning>: String
+
+    val customGetter: String
+        get() = ""
+
+    var customSetter: String
+        get() = ""
+        set(v) {}
+}
@@ -0,0 +1,7 @@
+package test
+
+import kotlinx.android.parcel.MagicParcel
+import android.os.Parcelable
+
+@MagicParcel
+class User(val firstName: String, val secondName: String, val age: Int) : Parcelable
@@ -0,0 +1,20 @@
+package test
+
+import kotlinx.android.parcel.MagicParcel
+import android.os.Parcelable
+
+@MagicParcel
+class <error descr="[NO_PARCELABLE_SUPERTYPE] No 'Parcelable' supertype">Without</error>(val firstName: String, val secondName: String, val age: Int)
+
+@MagicParcel
+class With(val firstName: String, val secondName: String, val age: Int) : Parcelable
+
+interface MyParcelableIntf : Parcelable
+
+abstract class MyParcelableCl : Parcelable
+
+@MagicParcel
+class WithIntfSubtype(val firstName: String, val secondName: String, val age: Int) : MyParcelableIntf
+
+@MagicParcel
+class WithClSubtype(val firstName: String, val secondName: String, val age: Int) : MyParcelableCl()
@@ -0,0 +1,25 @@
+package test
+
+import kotlinx.android.parcel.MagicParcel
+import android.os.Parcelable
+
+@MagicParcel
+interface <error descr="[PARCELABLE_SHOULD_BE_CLASS] 'Parcelable' should be a class">Intf</error> : Parcelable
+
+@MagicParcel
+object <error descr="[PARCELABLE_SHOULD_BE_CLASS] 'Parcelable' should be a class">Obj</error>
+
+class A {
+    @MagicParcel
+    companion <error descr="[PARCELABLE_SHOULD_BE_CLASS] 'Parcelable' should be a class">object</error> {
+        fun foo() {}
+    }
+}
+
+@MagicParcel
+enum class <error descr="[PARCELABLE_SHOULD_BE_CLASS] 'Parcelable' should be a class">Enum</error> {
+    WHITE, BLACK
+}
+
+@MagicParcel
+annotation class <error descr="[PARCELABLE_SHOULD_BE_CLASS] 'Parcelable' should be a class">Anno</error>
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2010-2017 JetBrains s.r.o.
+ *
+ * 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 org.jetbrains.kotlin.android
+
+import org.jetbrains.kotlin.idea.test.ConfigLibraryUtil
+import java.io.File
+
+abstract class AbstractParcelCheckerTest : KotlinAndroidTestCase() {
+    override fun setUp() {
+        super.setUp()
+        ConfigLibraryUtil.configureKotlinRuntime(myModule)
+    }
+
+    override fun tearDown() {
+        ConfigLibraryUtil.unConfigureKotlinRuntime(myModule)
+        super.tearDown()
+    }
+
+    fun doTest(filename: String) {
+        myFixture.copyDirectoryToProject("plugins/android-extensions/android-extensions-runtime/src", "src/androidExtensionsRuntime")
+
+        val ktFile = File(filename)
+        val virtualFile = myFixture.copyFileToProject(ktFile.absolutePath, "src/" + getTestName(true) + ".kt")
+        myFixture.configureFromExistingVirtualFile(virtualFile)
+
+        myFixture.checkHighlighting(true, false, true)
+    }
+}
@@ -0,0 +1,80 @@
+/*
+ * Copyright 2010-2017 JetBrains s.r.o.
+ *
+ * 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 org.jetbrains.kotlin.android;
+
+import com.intellij.testFramework.TestDataPath;
+import org.jetbrains.kotlin.test.JUnit3RunnerWithInners;
+import org.jetbrains.kotlin.test.KotlinTestUtils;
+import org.jetbrains.kotlin.test.TargetBackend;
+import org.jetbrains.kotlin.test.TestMetadata;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.util.regex.Pattern;
+
+/** This class is generated by {@link org.jetbrains.kotlin.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */
+@SuppressWarnings("all")
+@TestMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker")
+@TestDataPath("$PROJECT_ROOT")
+@RunWith(JUnit3RunnerWithInners.class)
+public class ParcelCheckerTestGenerated extends AbstractParcelCheckerTest {
+    public void testAllFilesPresentInChecker() throws Exception {
+        KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
+    }
+
+    @TestMetadata("constructors.kt")
+    public void testConstructors() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/constructors.kt");
+        doTest(fileName);
+    }
+
+    @TestMetadata("modality.kt")
+    public void testModality() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/modality.kt");
+        doTest(fileName);
+    }
+
+    @TestMetadata("notMagicParcel.kt")
+    public void testNotMagicParcel() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/notMagicParcel.kt");
+        doTest(fileName);
+    }
+
+    @TestMetadata("properties.kt")
+    public void testProperties() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/properties.kt");
+        doTest(fileName);
+    }
+
+    @TestMetadata("simple.kt")
+    public void testSimple() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/simple.kt");
+        doTest(fileName);
+    }
+
+    @TestMetadata("withoutParcelableSupertype.kt")
+    public void testWithoutParcelableSupertype() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/withoutParcelableSupertype.kt");
+        doTest(fileName);
+    }
+
+    @TestMetadata("wrongAnnotationTarget.kt")
+    public void testWrongAnnotationTarget() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/wrongAnnotationTarget.kt");
+        doTest(fileName);
+    }
+}