From 3ea334895266bd408a447337eecf67474d4ad884 Mon Sep 17 00:00:00 2001 From: Alexander Tsay <48321920+aktsay6@users.noreply.github.com> Date: Thu, 22 Oct 2020 15:53:20 +0300 Subject: [PATCH] Rule 6.1.2. Prefer data classes instead of classes without any functional logic * feature/rule-6.1.2-data-classes(#430) ### What's done: * Rule logic made * Added tests --- diktat-analysis.yml | 4 + .../src/main/kotlin/generated/WarningNames.kt | 2 + .../cqfn/diktat/ruleset/constants/Warnings.kt | 3 + .../ruleset/rules/DiktatRuleSetProvider.kt | 2 + .../ruleset/rules/classes/DataClassesRule.kt | 119 ++++++++++++++++++ .../main/resources/diktat-analysis-huawei.yml | 4 + .../src/main/resources/diktat-analysis.yml | 4 + .../chapter6/DataClassesRuleWarnTest.kt | 98 +++++++++++++++ info/available-rules.md | 1 + 9 files changed, 237 insertions(+) create mode 100644 diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/classes/DataClassesRule.kt create mode 100644 diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/DataClassesRuleWarnTest.kt diff --git a/diktat-analysis.yml b/diktat-analysis.yml index 7b28f66c83..97f70f1472 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -320,5 +320,9 @@ configuration: {} # Checks that property in constructor doesn't contains comment - name: KDOC_NO_CONSTRUCTOR_PROPERTY + enabled: true + configuration: {} +# Checks if class can be made as data class +- name: USE_DATA_CLASS enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt index 00e0d51ce7..0063a7c753 100644 --- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt +++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt @@ -189,4 +189,6 @@ public object WarningNames { public const val WRONG_OVERLOADING_FUNCTION_ARGUMENTS: String = "WRONG_OVERLOADING_FUNCTION_ARGUMENTS" + + public const val USE_DATA_CLASS: String = "USE_DATA_CLASS" } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt index e5d19f6069..886bad30bd 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt @@ -114,6 +114,9 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S TOO_MANY_PARAMETERS(false, "function has too many parameters"), NESTED_BLOCK(false, "function has too many nested blocks and should be simplified"), WRONG_OVERLOADING_FUNCTION_ARGUMENTS(false, "use default argument instead of function overloading"), + + // ======== chapter 6 ======== + USE_DATA_CLASS(false, "this class can be converted to a data class"), ; /** diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt index 7602a99da4..d26d92d4b7 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt @@ -7,6 +7,7 @@ import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.common.config.rules.RulesConfigReader import org.cqfn.diktat.ruleset.constants.Warnings import org.cqfn.diktat.ruleset.rules.calculations.AccurateCalculationsRule +import org.cqfn.diktat.ruleset.rules.classes.DataClassesRule import org.cqfn.diktat.ruleset.rules.comments.CommentsRule import org.cqfn.diktat.ruleset.rules.comments.HeaderCommentRule import org.cqfn.diktat.ruleset.rules.files.BlankLinesRule @@ -52,6 +53,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy ::PackageNaming, ::StringTemplateFormatRule, ::FileSize, + ::DataClassesRule, ::IdentifierNaming, ::LocalVariablesRule, ::ClassLikeStructuresOrderRule, diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/classes/DataClassesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/classes/DataClassesRule.kt new file mode 100644 index 0000000000..7071a27c0e --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/classes/DataClassesRule.kt @@ -0,0 +1,119 @@ +package org.cqfn.diktat.ruleset.rules.classes + +import com.pinterest.ktlint.core.Rule +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.DATA_KEYWORD +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.PROPERTY +import com.pinterest.ktlint.core.ast.ElementType.PROPERTY_ACCESSOR +import com.pinterest.ktlint.core.ast.ElementType.SEALED_KEYWORD +import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_LIST +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.ruleset.constants.Warnings.USE_DATA_CLASS +import org.cqfn.diktat.ruleset.utils.getAllChildrenWithType +import org.cqfn.diktat.ruleset.utils.getFirstChildWithType +import org.cqfn.diktat.ruleset.utils.hasChildOfType +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.psi.KtClass +import org.jetbrains.kotlin.psi.KtExpression + +/** + * This rule checks if class can be made as data class + */ +class DataClassesRule(private val configRule: List) : Rule("data-classes") { + private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) + private var isFixMode: Boolean = false + + companion object { + private val BAD_MODIFIERS = listOf(OPEN_KEYWORD, ABSTRACT_KEYWORD, INNER_KEYWORD, SEALED_KEYWORD, ENUM_KEYWORD) + } + override fun visit(node: ASTNode, + autoCorrect: Boolean, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { + emitWarn = emit + isFixMode = autoCorrect + + if (node.elementType == CLASS) { + handleClass(node) + } + } + + private fun handleClass(node: ASTNode) { + if (node.isDataClass()) + return + + if (node.canBeDataClass()) { + raiseWarn(node) + } + } + + // fixme: Need to know types of vars and props to create data class + private fun raiseWarn(node: ASTNode) { + USE_DATA_CLASS.warn(configRule, emitWarn, isFixMode, "${(node.psi as KtClass).name}", node.startOffset, node) + } + + @Suppress("UnsafeCallOnNullableType") + private fun ASTNode.canBeDataClass() : Boolean { + val classBody = getFirstChildWithType(CLASS_BODY) + if (hasChildOfType(MODIFIER_LIST)) { + val list = getFirstChildWithType(MODIFIER_LIST)!! + return list.getChildren(null).none { it.elementType in BAD_MODIFIERS } + && classBody?.getAllChildrenWithType(FUN)?.isEmpty() ?: false + && getFirstChildWithType(SUPER_TYPE_LIST) == null + } + return classBody?.getAllChildrenWithType(FUN)?.isEmpty() ?: false + && getFirstChildWithType(SUPER_TYPE_LIST) == null + // if there is any prop with logic in accessor then don't recommend to convert class to data class + && if (classBody != null) areGoodProps(classBody) else true + } + + /** + * Checks if any property with accessor contains logic in accessor + */ + private fun areGoodProps(node: ASTNode): Boolean { + val propertiesWithAccessors = node.getAllChildrenWithType(PROPERTY).filter { it.hasChildOfType(PROPERTY_ACCESSOR) } + + if (propertiesWithAccessors.isNotEmpty()) { + return propertiesWithAccessors.any { + val accessors = it.getAllChildrenWithType(PROPERTY_ACCESSOR) + + areGoodAccessors(accessors) + } + } + + return true + } + + @Suppress("UnsafeCallOnNullableType") + private fun areGoodAccessors(accessors: List): Boolean { + accessors.forEach { + if (it.hasChildOfType(BLOCK)) { + val block = it.getFirstChildWithType(BLOCK)!! + + return block + .getChildren(null) + .filter { expr -> expr.psi is KtExpression } + .count() <= 1 + } + } + + return true + } + + + @Suppress("UnsafeCallOnNullableType") + private fun ASTNode.isDataClass(): Boolean { + if (hasChildOfType(MODIFIER_LIST)) { + val list = getFirstChildWithType(MODIFIER_LIST)!! + return list.getChildren(null).any { it.elementType == DATA_KEYWORD } + } + return false + } +} diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index ae60e46a70..30139672c1 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -319,5 +319,9 @@ configuration: {} # Checks that property in constructor doesn't contains comment - name: KDOC_NO_CONSTRUCTOR_PROPERTY + enabled: true + configuration: {} +# Checks if class can be made as data class +- name: USE_DATA_CLASS enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml index 66559c1800..43c98b1106 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -321,5 +321,9 @@ configuration: {} # Checks that property in constructor doesn't contains comment - name: KDOC_NO_CONSTRUCTOR_PROPERTY + enabled: true + configuration: {} +# Checks if class can be made as data class +- name: USE_DATA_CLASS enabled: true configuration: {} \ No newline at end of file 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 new file mode 100644 index 0000000000..f1c325cefc --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/DataClassesRuleWarnTest.kt @@ -0,0 +1,98 @@ +package org.cqfn.diktat.ruleset.chapter6 + +import com.pinterest.ktlint.core.LintError +import generated.WarningNames.USE_DATA_CLASS +import org.cqfn.diktat.ruleset.constants.Warnings +import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID +import org.cqfn.diktat.ruleset.rules.classes.DataClassesRule +import org.cqfn.diktat.util.LintTestBase +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +class DataClassesRuleWarnTest : LintTestBase(::DataClassesRule) { + private val ruleId = "$DIKTAT_RULE_SET_ID:data-classes" + + @Test + @Tag(USE_DATA_CLASS) + fun `trigger on default class`() { + lintMethod( + """ + |class Some(val a: Int = 5) { + | + |} + """.trimMargin(), + LintError(1, 1, ruleId, "${Warnings.USE_DATA_CLASS.warnText()} Some") + ) + } + + @Test + @Tag(USE_DATA_CLASS) + fun `should trigger - dont forget to consider this class in fix`() { + lintMethod( + """ + |class Test { + | var a: Int = 0 + | get() = field + | set(value: Int) { field = value} + |} + """.trimMargin(), + LintError(1, 1, ruleId, "${Warnings.USE_DATA_CLASS.warnText()} Test") + ) + } + + @Test + @Tag(USE_DATA_CLASS) + fun `should not trigger if there is some logic in accessor`() { + lintMethod( + """ + |class Test { + | var a: Int = 0 + | get() = field + | set(value: Int) { + | field = value + | someFun(value) + | } + |} + """.trimMargin() + ) + } + + @Test + @Tag(USE_DATA_CLASS) + fun `should not trigger on class with bad modifiers`() { + lintMethod( + """ + |data class Some(val a: Int = 5) { + | + |} + | + |abstract class Another() {} + | + |open class Open(){} + | + |sealed class Clazz{} + | + |data class CheckInner { + | inner class Inner {} + |} + | + |enum class Num { + | + |} + """.trimMargin() + ) + } + + @Test + @Tag(USE_DATA_CLASS) + fun `should not trigger on classes with functions`() { + lintMethod( + """ + |class Some { + | val prop = 5 + | private fun someFunc() {} + |} + """.trimMargin() + ) + } +} diff --git a/info/available-rules.md b/info/available-rules.md index 4847da44d3..506f508be5 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -90,3 +90,4 @@ | 5 | 5.2.1 | LAMBDA_IS_NOT_LAST_PARAMETER | Checks that lambda inside function parameters isn't in the end | no | - | | 5 | 5.2.2 | TOO_MANY_PARAMETERS | Check: if function contains more parameters than allowed | no | maxParameterListSize | | 5 | 5.2.3 | WRONG_OVERLOADING_FUNCTION_ARGUMENTS | Check: function has overloading instead use default arguments | no | -| +| 6 | 6.1.2 | USE_DATA_CLASS | Check: if class can be made as data class | no | - | yes |