-
Notifications
You must be signed in to change notification settings - Fork 39
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rule 6.1.2. Prefer data classes instead of classes without any functi…
…onal logic * feature/rule-6.1.2-data-classes(#430) ### What's done: * Rule logic made * Added tests
- Loading branch information
Showing
9 changed files
with
237 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
119 changes: 119 additions & 0 deletions
119
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/classes/DataClassesRule.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<RulesConfig>) : 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<ASTNode>): 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 | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
98 changes: 98 additions & 0 deletions
98
diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/DataClassesRuleWarnTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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() | ||
) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3ea3348
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't able to retrieve PDD puzzles from the code base and submit them to GitHub. If you think that it's a bug on our side, please submit it to yegor256/0pdd:
Please, copy and paste this stack trace to GitHub: