Array in data class inspection: warning on array properties in data classes #KT-10299 Fixed

2016-03-25 18:09:25 +03:00
parent 047828bd6d
commit 20bac6178c
7 changed files with 123 additions and 0 deletions
@@ -0,0 +1,6 @@
+<html>
+<body>
+This inspection reports array properties in a data class without overridden <code>equals()</code> or <code>hashCode()</code> inside.
+Due to default <code>equals()</code> behaviour for arrays in JVM, it's strongly recommended to override <code>equals()</code> and <code>hashCode()</code> in such cases.
+</body>
+</html>
@@ -1418,6 +1418,13 @@
                     level="WARNING"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.ArrayInDataClassInspection"
+                     displayName="Array property in data class"
+                     groupName="Kotlin"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2010-2016 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.idea.inspections
+
+import com.intellij.codeInspection.LocalInspectionToolSession
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.kotlin.builtins.KotlinBuiltIns
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.KtClass
+import org.jetbrains.kotlin.psi.KtFunction
+import org.jetbrains.kotlin.psi.KtVisitorVoid
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
+import org.jetbrains.kotlin.util.OperatorNameConventions
+
+class ArrayInDataClassInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
+        return object : KtVisitorVoid() {
+            override fun visitClass(klass: KtClass) {
+                if (!klass.isData()) return
+                val constructor = klass.getPrimaryConstructor() ?: return
+                if (hasOverriddenEqualsAndHashCode(klass)) return
+                val context = constructor.analyze(BodyResolveMode.PARTIAL)
+                for (parameter in constructor.valueParameters) {
+                    if (!parameter.hasValOrVar()) continue
+                    val type = context.get(BindingContext.TYPE, parameter.typeReference) ?: continue
+                    if (KotlinBuiltIns.isArray(type) || KotlinBuiltIns.isPrimitiveArray(type)) {
+                        holder.registerProblem(parameter,
+                                               "Array property in data class: it's recommended to override equals() / hashCode()",
+                                               ProblemHighlightType.WEAK_WARNING)
+                    }
+                }
+            }
+
+            private fun hasOverriddenEqualsAndHashCode(klass: KtClass): Boolean {
+                var overriddenEquals = false
+                var overriddenHashCode = false
+                for (declaration in klass.declarations) {
+                    if (declaration !is KtFunction) continue
+                    if (!declaration.hasModifier(KtTokens.OVERRIDE_KEYWORD)) continue
+                    if (declaration.nameAsName == OperatorNameConventions.EQUALS && declaration.valueParameters.size == 1) {
+                        val parameter = declaration.valueParameters.single()
+                        val context = declaration.analyze(BodyResolveMode.PARTIAL)
+                        val type = context.get(BindingContext.TYPE, parameter.typeReference)
+                        if (type != null && KotlinBuiltIns.isNullableAny(type)) {
+                            overriddenEquals = true
+                        }
+                    }
+                    if (declaration.name == "hashCode" && declaration.valueParameters.size == 0) {
+                        overriddenHashCode = true
+                    }
+                }
+                return overriddenEquals && overriddenHashCode
+            }
+        }
+    }
+}
@@ -0,0 +1,18 @@
+<problems>
+  <problem>
+    <file>test.kt</file>
+    <line>1</line>
+    <module>light_idea_test_case</module>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Array property in data class</problem_class>
+    <description>Array property in data class: it's recommended to override equals() / hashCode()</description>
+  </problem>
+  <problem>
+    <file>test.kt</file>
+    <line>5</line>
+    <module>light_idea_test_case</module>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Array property in data class</problem_class>
+    <description>Array property in data class: it's recommended to override equals() / hashCode()</description>
+  </problem>
+</problems>
@@ -0,0 +1 @@
+// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.ArrayInDataClassInspection
@@ -0,0 +1,11 @@
+data class First(val x: Array<String>, val y: Int)
+
+class Second(val x: Array<String>)
+
+data class Third(val x: String, val y: IntArray)
+
+data class Correct(val x: IntArray) {
+    override fun equals(other: Any?) = other is Correct
+
+    override fun hashCode() = 0
+}
@@ -112,6 +112,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest {
            doTest(fileName);
        }

+        @TestMetadata("arrayInDataClass/inspectionData/inspections.test")
+        public void testArrayInDataClass_inspectionData_Inspections_test() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/arrayInDataClass/inspectionData/inspections.test");
+            doTest(fileName);
+        }
+
        @TestMetadata("canBeVal/inspectionData/inspections.test")
        public void testCanBeVal_inspectionData_Inspections_test() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/canBeVal/inspectionData/inspections.test");
				`@@ -0,0 +1 @@`
				`// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.ArrayInDataClassInspection`