From 20b3e2c5047c45e84430ad37786cbbfba88193b4 Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Fri, 22 Jul 2022 14:43:06 +0300 Subject: [PATCH 1/6] Data class rule: removing several cases from the scope of the inspection ### What's done: - Removed annotation, value, sealed, inline, inner classes from the scope - Added tests --- .../rules/chapter6/classes/DataClassesRule.kt | 13 +++-- .../chapter6/DataClassesRuleWarnTest.kt | 52 +++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) 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..b5851ba92b 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 @@ -42,11 +42,10 @@ class DataClassesRule(configRules: List) : DiktatRule( } private fun handleClass(node: ASTNode) { - with((node.psi as KtClass)) { - if (isData() || isInterface()) { - return - } + if ((node.psi as KtClass).isDefinitelyNotADataClass()) { + return } + if (node.canBeDataClass()) { raiseWarn(node) } @@ -128,6 +127,12 @@ class DataClassesRule(configRules: List) : DiktatRule( return true } + /** we do not exclude inner classes and enums here as if they have no + * methods, then we definitely can refactor the code and make them data classes + **/ + private fun KtClass.isDefinitelyNotDataClass() = + isValue() || isAnnotation() || isInterface() || isData() || isSealed() || isInline() || isInner() + @Suppress("UnsafeCallOnNullableType") private fun areGoodAccessors(accessors: List): Boolean { accessors.forEach { 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..2f24bac51d 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 @@ -288,6 +288,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`() { From 70f46ca6509d20ecf524b199bf8845809325a51f Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Sat, 23 Jul 2022 23:13:20 +0300 Subject: [PATCH 2/6] Data class rule: removing several cases from the scope of the inspection ### What's done: - Removed annotation, value, sealed, inline, inner classes from the scope - Added tests --- .../diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b5851ba92b..aa64d12e40 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 @@ -42,7 +42,7 @@ class DataClassesRule(configRules: List) : DiktatRule( } private fun handleClass(node: ASTNode) { - if ((node.psi as KtClass).isDefinitelyNotADataClass()) { + if ((node.psi as KtClass).isDefinitelyNotDataClass()) { return } From 7b49db23fffe14646d06558d911edcb88b28987c Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Mon, 25 Jul 2022 12:39:51 +0300 Subject: [PATCH 3/6] Data class rule: removing several cases from the scope of the inspection ### What's done: - review notes --- .../rules/chapter6/classes/DataClassesRule.kt | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) 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 aa64d12e40..3db21c24ec 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 @@ -127,11 +124,17 @@ class DataClassesRule(configRules: List) : DiktatRule( return true } - /** we do not exclude inner classes and enums here as if they have no - * methods, then we definitely can refactor the code and make them data classes + /** + * We do not exclude inner classes and enums 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, 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() || isInner() + isValue() || isAnnotation() || isInterface() || isData() || + isSealed() || isInline() || isAbstract() @Suppress("UnsafeCallOnNullableType") private fun areGoodAccessors(accessors: List): Boolean { @@ -151,6 +154,6 @@ class DataClassesRule(configRules: List) : 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) } } From 11f546770b011aeb88a487b9bbb56625fee92a97 Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Mon, 25 Jul 2022 12:40:59 +0300 Subject: [PATCH 4/6] Data class rule: removing several cases from the scope of the inspection ### What's done: - review notes --- .../diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 3db21c24ec..7ae185f97b 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 @@ -97,14 +97,14 @@ class DataClassesRule(configRules: List) : DiktatRule( .none { it.elementType in badModifiers } && classBody?.getAllChildrenWithType(FUN) ?.isEmpty() - ?: false && + ?: false && getFirstChildWithType(SUPER_TYPE_LIST) == null } return classBody?.getFirstChildWithType(FUN) == null && getFirstChildWithType(SUPER_TYPE_LIST) == null && // if there is any prop with logic in accessor then don't recommend to convert class to data class classBody?.let(::areGoodProps) - ?: true + ?: true } /** From a90cf2d31d5523091ed0d66140cd47c3b6e1acd9 Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Mon, 25 Jul 2022 12:50:45 +0300 Subject: [PATCH 5/6] Data class rule: removing several cases from the scope of the inspection ### What's done: - review notes --- .../diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 7ae185f97b..3db21c24ec 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 @@ -97,14 +97,14 @@ class DataClassesRule(configRules: List) : DiktatRule( .none { it.elementType in badModifiers } && classBody?.getAllChildrenWithType(FUN) ?.isEmpty() - ?: false && + ?: false && getFirstChildWithType(SUPER_TYPE_LIST) == null } return classBody?.getFirstChildWithType(FUN) == null && getFirstChildWithType(SUPER_TYPE_LIST) == null && // if there is any prop with logic in accessor then don't recommend to convert class to data class classBody?.let(::areGoodProps) - ?: true + ?: true } /** From 74039e2d9b1470661c02e731dacfb8de7b16cda9 Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Mon, 25 Jul 2022 13:57:24 +0300 Subject: [PATCH 6/6] Data class rule: removing several cases from the scope of the inspection ### What's done: - review notes --- .../rules/chapter6/classes/DataClassesRule.kt | 6 +++--- .../ruleset/chapter6/DataClassesRuleWarnTest.kt | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) 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 3db21c24ec..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 @@ -125,16 +125,16 @@ class DataClassesRule(configRules: List) : DiktatRule( } /** - * We do not exclude inner classes and enums here as if they have no + * 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, annotations, interfaces, abstract 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() + isSealed() || isInline() || isAbstract() || isEnum() @Suppress("UnsafeCallOnNullableType") private fun areGoodAccessors(accessors: List): Boolean { 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 2f24bac51d..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`() {