Repository: analysis-dev/diktat (kotlin)

Issue:
USE_DATA_CLASS: False positive for Annotations
I have this annotation:
```kotlin
@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"
    }
}
```
And Diktat complains that
```
There are 1 unfixed errors:
Error on line: 3, column: 1 cannot be fixed automatically
[USE_DATA_CLASS] this class can be converted to a data class: NavGraphDestination
java.lang.AssertionError: There are 1 unfixed errors:
```

I guess that the `DataClassesRule` should check if a class is an `annotation`, beside checking for `data class` and `interface`:

https://github.com/saveourtool/diktat/blob/54c8ccefc3009b8dd2a493c30590168318765b89/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt#L46

I'm having this with Diktat 1.2.1. Let me know if you need more info.

Pull request:
### What's done:
- Removed annotation, value, sealed, inline, inner classes from the scope
- Added tests

Patch:
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt
index eea79a7662..02cd7b3af4 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt
+++ b/diktat-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)
     }
 }


Test patch:
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
index 3b07445909..ce4dba68e7 100644
--- 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`() {