Data class rule: removing several cases from the scope of the inspect…

…ion (#1471) ### What's done: - Removed annotation, value, sealed, inline, inner classes from the scope - Added tests
saveourtool · Jul 25, 2022 · 40a51a9 · 40a51a9
1 parent 632ac74
commit 40a51a9
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 9 deletions.
diff --git a/...t-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt b/...t-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt
@@ -5,27 +5,24 @@ import org.cqfn.diktat.ruleset.constants.Warnings.USE_DATA_CLASS
 import org.cqfn.diktat.ruleset.rules.DiktatRule
 import org.cqfn.diktat.ruleset.utils.*
 
-import com.pinterest.ktlint.core.ast.ElementType.ABSTRACT_KEYWORD
 import com.pinterest.ktlint.core.ast.ElementType.BLOCK
 import com.pinterest.ktlint.core.ast.ElementType.CLASS
 import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY
 import com.pinterest.ktlint.core.ast.ElementType.CLASS_INITIALIZER
-import com.pinterest.ktlint.core.ast.ElementType.ENUM_KEYWORD
 import com.pinterest.ktlint.core.ast.ElementType.FUN
-import com.pinterest.ktlint.core.ast.ElementType.INNER_KEYWORD
 import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST
 import com.pinterest.ktlint.core.ast.ElementType.OPEN_KEYWORD
 import com.pinterest.ktlint.core.ast.ElementType.PRIMARY_CONSTRUCTOR
 import com.pinterest.ktlint.core.ast.ElementType.PROPERTY
 import com.pinterest.ktlint.core.ast.ElementType.PROPERTY_ACCESSOR
 import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
-import com.pinterest.ktlint.core.ast.ElementType.SEALED_KEYWORD
 import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_LIST
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 import org.jetbrains.kotlin.psi.KtClass
 import org.jetbrains.kotlin.psi.KtClassBody
 import org.jetbrains.kotlin.psi.KtExpression
 import org.jetbrains.kotlin.psi.KtPrimaryConstructor
+import org.jetbrains.kotlin.psi.psiUtil.isAbstract
 
 /**
  * This rule checks if class can be made as data class
@@ -42,11 +39,10 @@ class DataClassesRule(configRules: List<RulesConfig>) : DiktatRule(
     }
 
     private fun handleClass(node: ASTNode) {
-        with((node.psi as KtClass)) {
-            if (isData() || isInterface()) {
-                return
-            }
+        if ((node.psi as KtClass).isDefinitelyNotDataClass()) {
+            return
         }
+
         if (node.canBeDataClass()) {
             raiseWarn(node)
         }
@@ -128,6 +124,18 @@ class DataClassesRule(configRules: List<RulesConfig>) : DiktatRule(
         return true
     }
 
+    /**
+     * We do not exclude inner classes here as if they have no
+     * methods, then we definitely can refactor the code and make them data classes.
+     * We only exclude: value/inline classes, enums, annotations, interfaces, abstract classes,
+     * sealed classes and data classes itself. For sure there will be other corner cases,
+     * for example, simple classes in Spring marked with @Entity annotation.
+     * For these classes we expect users to Suppress warning manually for each corner case.
+     **/
+    private fun KtClass.isDefinitelyNotDataClass() =
+        isValue() || isAnnotation() || isInterface() || isData() ||
+                isSealed() || isInline() || isAbstract() || isEnum()
+
     @Suppress("UnsafeCallOnNullableType")
     private fun areGoodAccessors(accessors: List<ASTNode>): Boolean {
         accessors.forEach {
@@ -146,6 +154,6 @@ class DataClassesRule(configRules: List<RulesConfig>) : DiktatRule(
 
     companion object {
         const val NAME_ID = "data-classes"
-        private val badModifiers = listOf(OPEN_KEYWORD, ABSTRACT_KEYWORD, INNER_KEYWORD, SEALED_KEYWORD, ENUM_KEYWORD)
+        private val badModifiers = listOf(OPEN_KEYWORD)
     }
 }
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/DataClassesRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/DataClassesRuleWarnTest.kt
@@ -239,6 +239,20 @@ class DataClassesRuleWarnTest : LintTestBase(::DataClassesRule) {
         )
     }
 
+    @Test
+    @Tag(USE_DATA_CLASS)
+    fun `should not trigger on enums`() {
+        lintMethod(
+            """
+                |enum class Style(val str: String) {
+                |    PASCAL_CASE("PascalCase"),
+                |    SNAKE_CASE("UPPER_SNAKE_CASE"),
+                |    ;
+                |}
+            """.trimMargin()
+        )
+    }
+
     @Test
     @Tag(USE_DATA_CLASS)
     fun `should trigger on class with parameter in constructor`() {
@@ -288,6 +302,58 @@ class DataClassesRuleWarnTest : LintTestBase(::DataClassesRule) {
         )
     }
 
+    @Test
+    @Tag(USE_DATA_CLASS)
+    fun `annotation classes bug`() {
+        lintMethod(
+            """
+                |@Retention(AnnotationRetention.SOURCE)
+                |@Target(AnnotationTarget.CLASS)
+                |annotation class NavGraphDestination(
+                |    val name: String = Defaults.NULL,
+                |    val routePrefix: String = Defaults.NULL,
+                |    val deepLink: Boolean = false,
+                |) {
+                |    object Defaults {
+                |        const val NULL = "@null"
+                |    }
+                |}
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(USE_DATA_CLASS)
+    fun `value or inline classes bug`() {
+        lintMethod(
+            """
+                |@JvmInline
+                |value class Password(private val s: String)
+                |val securePassword = Password("Don't try this in production")
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(USE_DATA_CLASS)
+    fun `sealed classes bug`() {
+        lintMethod(
+            """
+                |sealed class Password(private val s: String)
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(USE_DATA_CLASS)
+    fun `inner classes bug`() {
+        lintMethod(
+            """
+                |inner class Password(private val s: String)
+            """.trimMargin()
+        )
+    }
+
     @Test
     @Tag(USE_DATA_CLASS)
     fun `shouldn't trigger on interface`() {