From 208c1edba48dc4d93d0c7f49b5cefae5bab34ea2 Mon Sep 17 00:00:00 2001 From: Alexander Tsay <48321920+aktsay6@users.noreply.github.com> Date: Tue, 9 Feb 2021 16:30:14 +0300 Subject: [PATCH] Bugfix. Inspect the logic of the data class rule (#761) * bugfix/data-classes-secondary-constructor(#750) ### What's done: * Logic extended * Added tests * Fixed bugs --- .../rules/chapter6/classes/DataClassesRule.kt | 27 +++- .../chapter6/DataClassesRuleWarnTest.kt | 129 +++++++++++++++++- 2 files changed, 152 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 f89eb1239b..de0f2b6d91 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 @@ -3,14 +3,13 @@ package org.cqfn.diktat.ruleset.rules.chapter6.classes import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.ruleset.constants.Warnings.USE_DATA_CLASS import org.cqfn.diktat.ruleset.rules.DiktatRule -import org.cqfn.diktat.ruleset.utils.getAllChildrenWithType -import org.cqfn.diktat.ruleset.utils.getFirstChildWithType -import org.cqfn.diktat.ruleset.utils.hasChildOfType +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.DATA_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.ENUM_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.FUN @@ -20,6 +19,7 @@ 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 @@ -56,15 +56,36 @@ class DataClassesRule(configRules: List) : DiktatRule("data-classes @Suppress("UnsafeCallOnNullableType", "FUNCTION_BOOLEAN_PREFIX", "ComplexMethod") private fun ASTNode.canBeDataClass(): Boolean { val isNotPropertyInClassBody = findChildByType(CLASS_BODY)?.let { (it.psi as KtClassBody).properties.isEmpty() } ?: true + val constructorParametersNames: MutableList = mutableListOf() val hasPropertyInConstructor = findChildByType(PRIMARY_CONSTRUCTOR) ?.let { constructor -> (constructor.psi as KtPrimaryConstructor) .valueParameters + .onEach { + if (!it.hasValOrVar()) { + constructorParametersNames.add(it.name!!) + } + } .run { isNotEmpty() && all { it.hasValOrVar() } } } ?: false if (isNotPropertyInClassBody && !hasPropertyInConstructor) { return false } + // if parameter of the primary constructor is used in init block then it is hard to refactor this class to data class + if (constructorParametersNames.isNotEmpty()) { + val initBlocks = findChildByType(CLASS_BODY)?.getAllChildrenWithType(CLASS_INITIALIZER) + initBlocks?.forEach { init -> + val refExpressions = init.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) + if (refExpressions.any { it.text in constructorParametersNames }) { + return false + } + } + } + return hasAppropriateClassBody() + } + + @Suppress("UnsafeCallOnNullableType") + private fun ASTNode.hasAppropriateClassBody(): Boolean { val classBody = getFirstChildWithType(CLASS_BODY) if (hasChildOfType(MODIFIER_LIST)) { val list = getFirstChildWithType(MODIFIER_LIST)!! 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 f25d4b2f99..e9638b1656 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 @@ -143,9 +143,136 @@ class DataClassesRuleWarnTest : LintTestBase(::DataClassesRule) { | val q = 10 | fun foo() = 10 |} - """.trimMargin(), + """.trimMargin(), LintError(1, 1, ruleId, "${Warnings.USE_DATA_CLASS.warnText()} B"), LintError(5, 1, ruleId, "${Warnings.USE_DATA_CLASS.warnText()} Ab") ) } + + @Test + @Tag(USE_DATA_CLASS) + fun `shouldn't trigger on class with init block`() { + lintMethod( + """ + |class Credentials(auth: String) { + | val gitHubUserName: String + | val gitHubAuthToken: String + | + | init { + | auth.let { + | + | } + | } + |} + """.trimMargin() + ) + } + + @Test + @Tag(USE_DATA_CLASS) + fun `should not trigger on init block with if`() { + lintMethod( + """ + |class Credentials(auth: String, second: Int?, third: Double) { + | val gitHubUserName: String + | val gitHubAuthToken: String + | + | init { + | if (second != null) { + | } + | } + |} + """.trimMargin() + ) + } + + @Test + @Tag(USE_DATA_CLASS) + fun `should not trigger on init block with function call`() { + lintMethod( + """ + |class Credentials(auth: String, second: Int?, third: Double) { + | val gitHubUserName: String + | val gitHubAuthToken: String + | + | init { + | foo(third) + | } + |} + """.trimMargin() + ) + } + + @Test + @Tag(USE_DATA_CLASS) + fun `should not trigger on class with several parameters`() { + lintMethod( + """ + |class Credentials(auth: String, second: Int?, third: Double) { + | val gitHubUserName: String + | val gitHubAuthToken: String + | + | init { + | auth.let { + | + | } + | + | if (second != null) { + | } + | + | foo(third) + | } + |} + """.trimMargin() + ) + } + + @Test + @Tag(USE_DATA_CLASS) + fun `should trigger on class with parameter in constructor`() { + lintMethod( + """ + |class Credentials(auth: String) { + | val gitHubUserName: String = auth.toUpperCase() + | val gitHubAuthToken: String = auth.toLowerCase() + |} + """.trimMargin(), + LintError(1, 1, ruleId, "${Warnings.USE_DATA_CLASS.warnText()} Credentials") + ) + } + + @Test + @Tag(USE_DATA_CLASS) + fun `should trigger on class with parameter in constructor and init block`() { + lintMethod( + """ + |class Credentials(auth: String) { + | val gitHubUserName: String = auth.toUpperCase() + | val gitHubAuthToken: String = auth.toLowerCase() + | + | init { + | // some logic + | } + |} + """.trimMargin(), + LintError(1, 1, ruleId, "${Warnings.USE_DATA_CLASS.warnText()} Credentials") + ) + } + + @Test + @Tag(USE_DATA_CLASS) + fun `should not trigger on init block with one ref expression`() { + lintMethod( + """ + |class Credentials(auth: String, some: Int?) { + | val gitHubUserName: String = auth.toUpperCase() + | val gitHubAuthToken: String = auth.toLowerCase() + | + | init { + | val a = auth + | } + |} + """.trimMargin() + ) + } }