Skip to content

Commit

Permalink
Feature. Recommendation 6.4.2: Objects should be used for Stateless I…
Browse files Browse the repository at this point in the history
…nterfaces

* feature/recommendation-6.4.2(#449)

### What's done:
  * Added rule logic
  * Added warn tests
  * Added fix tests
  • Loading branch information
aktsay6 authored Dec 11, 2020
1 parent 791413b commit 6ea31f7
Show file tree
Hide file tree
Showing 13 changed files with 246 additions and 2 deletions.
3 changes: 3 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -311,4 +311,7 @@
enabled: true
# Checks if there is class/object that can be replace with extension function
- name: AVOID_USING_UTILITY_CLASS
enabled: true
# If there is stateless class it is preferred to use object
- name: OBJECT_IS_PREFERRED
enabled: true
Empty file modified diktat-gradle-plugin/gradlew.bat
100644 → 100755
Empty file.
2 changes: 2 additions & 0 deletions diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -227,4 +227,6 @@ public object WarningNames {
public const val NO_CORRESPONDING_PROPERTY: String = "NO_CORRESPONDING_PROPERTY"

public const val AVOID_USING_UTILITY_CLASS: String = "AVOID_USING_UTILITY_CLASS"

public const val OBJECT_IS_PREFERRED: String = "OBJECT_IS_PREFERRED"
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ enum class Warnings(
EMPTY_PRIMARY_CONSTRUCTOR(true, "6.1.3", "avoid empty primary constructor"),
NO_CORRESPONDING_PROPERTY(false, "6.1.7", "backing property should have the same name, but there is no corresponding property"),
AVOID_USING_UTILITY_CLASS(false, "6.4.1", "avoid using utility classes/objects, use extensions functions"),
OBJECT_IS_PREFERRED(true, "6.4.2", "it is better to use object for stateless classes"),
;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import org.cqfn.diktat.ruleset.rules.classes.CompactInitialization
import org.cqfn.diktat.ruleset.rules.classes.DataClassesRule
import org.cqfn.diktat.ruleset.rules.classes.SingleConstructorRule
import org.cqfn.diktat.ruleset.rules.classes.SingleInitRule
import org.cqfn.diktat.ruleset.rules.classes.StatelessClassesRule
import org.cqfn.diktat.ruleset.rules.comments.CommentsRule
import org.cqfn.diktat.ruleset.rules.comments.HeaderCommentRule
import org.cqfn.diktat.ruleset.rules.files.BlankLinesRule
Expand Down Expand Up @@ -52,7 +53,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
val diktatExecutionPath = File(diktatConfigFile)
if (!diktatExecutionPath.exists()) {
// for some aggregators of static analyzers we need to provide configuration for cli
// in this case diktat would take the configuration from the direcory where jar file is stored
// in this case diktat would take the configuration from the directory where jar file is stored
val ruleSetProviderPath =
DiktatRuleSetProvider::class
.java
Expand Down Expand Up @@ -112,6 +113,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
::CustomGetterSetterRule,
::CompactInitialization,
// other rules
::StatelessClassesRule,
::ImplicitBackingPropertyRule,
::StringTemplateFormatRule,
::DataClassesRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package org.cqfn.diktat.ruleset.rules.classes

import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.ruleset.constants.EmitType
import org.cqfn.diktat.ruleset.constants.Warnings
import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType
import org.cqfn.diktat.ruleset.utils.getAllChildrenWithType
import org.cqfn.diktat.ruleset.utils.getFirstChildWithType
import org.cqfn.diktat.ruleset.utils.hasChildOfType

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType.CLASS
import com.pinterest.ktlint.core.ast.ElementType.CLASS_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.FILE
import com.pinterest.ktlint.core.ast.ElementType.FUN
import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER
import com.pinterest.ktlint.core.ast.ElementType.INTERFACE_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.OBJECT_DECLARATION
import com.pinterest.ktlint.core.ast.ElementType.OBJECT_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_ENTRY
import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_LIST
import com.pinterest.ktlint.core.ast.children
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.psi.KtClass

/**
* This rule checks if class is stateless and if so changes it to object.
*/
class StatelessClassesRule(private val configRule: List<RulesConfig>) : Rule("stateless-class") {
private var isFixMode: Boolean = false
private lateinit var emitWarn: EmitType

override fun visit(node: ASTNode,
autoCorrect: Boolean,
emit: EmitType) {
emitWarn = emit
isFixMode = autoCorrect

// Fixme: We should find interfaces in all project and then check them
if (node.elementType == FILE) {
val interfacesNodes = node
.findAllNodesWithSpecificType(CLASS)
.filter { it.hasChildOfType(INTERFACE_KEYWORD) }
node
.findAllNodesWithSpecificType(CLASS)
.filterNot { it.hasChildOfType(INTERFACE_KEYWORD) }
.forEach { handleClass(it, interfacesNodes) }
}
}

@Suppress("UnsafeCallOnNullableType")
private fun handleClass(node: ASTNode, interfaces: List<ASTNode>) {
if (isClassExtendsValidInterface(node, interfaces) && isStatelessClass(node)) {
Warnings.OBJECT_IS_PREFERRED.warnAndFix(configRule, emitWarn, isFixMode,
"class ${(node.psi as KtClass).name!!}", node.startOffset, node) {
val newObjectNode = CompositeElement(OBJECT_DECLARATION)
node.treeParent.addChild(newObjectNode, node)
node.children().forEach {
newObjectNode.addChild(it.copyElement(), null)
}
newObjectNode.addChild(LeafPsiElement(OBJECT_KEYWORD, "object"),
newObjectNode.getFirstChildWithType(CLASS_KEYWORD))
newObjectNode.removeChild(newObjectNode.getFirstChildWithType(CLASS_KEYWORD)!!)
node.treeParent.removeChild(node)
}
}
}

private fun isStatelessClass(node: ASTNode): Boolean {
val properties = (node.psi as KtClass).getProperties()
val functions = node.findAllNodesWithSpecificType(FUN)
return properties.isNullOrEmpty() &&
functions.isNotEmpty() &&
!(node.psi as KtClass).hasExplicitPrimaryConstructor()
}

private fun isClassExtendsValidInterface(node: ASTNode, interfaces: List<ASTNode>): Boolean =
node.findChildByType(SUPER_TYPE_LIST)
?.getAllChildrenWithType(SUPER_TYPE_ENTRY)
?.isNotEmpty()
?.and(isClassInheritsStatelessInterface(node, interfaces))
?: false

@Suppress("UnsafeCallOnNullableType")
private fun isClassInheritsStatelessInterface(node: ASTNode, interfaces: List<ASTNode>): Boolean {
val classInterfaces = node
.findChildByType(SUPER_TYPE_LIST)
?.getAllChildrenWithType(SUPER_TYPE_ENTRY)

val foundInterfaces = interfaces.filter { inter ->
classInterfaces!!.any { it.text == inter.getFirstChildWithType(IDENTIFIER)!!.text }
}

return foundInterfaces.any { (it.psi as KtClass).getProperties().isEmpty() }
}
}
3 changes: 3 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -309,4 +309,7 @@
enabled: true
# Checks if there is class/object that can be replace with extension function
- name: AVOID_USING_UTILITY_CLASS
enabled: true
# If there is stateless class it is preferred to use object
- name: OBJECT_IS_PREFERRED
enabled: true
3 changes: 3 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -311,4 +311,7 @@
enabled: true
# Checks if there is class/object that can be replace with extension function
- name: AVOID_USING_UTILITY_CLASS
enabled: true
# If there is stateless class it is preferred to use object
- name: OBJECT_IS_PREFERRED
enabled: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.cqfn.diktat.ruleset.chapter6

import org.cqfn.diktat.ruleset.rules.classes.StatelessClassesRule
import org.cqfn.diktat.util.FixTestBase

import generated.WarningNames.OBJECT_IS_PREFERRED
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

class StatelessClassesRuleFixTest : FixTestBase("test/chapter6/stateless_classes", ::StatelessClassesRule) {
@Test
@Tag(OBJECT_IS_PREFERRED)
fun `fix class to object keyword`() {
fixAndCompare("StatelessClassExpected.kt", "StatelessClassTest.kt")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package org.cqfn.diktat.ruleset.chapter6

import org.cqfn.diktat.ruleset.constants.Warnings
import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.rules.classes.StatelessClassesRule
import org.cqfn.diktat.util.LintTestBase

import com.pinterest.ktlint.core.LintError
import generated.WarningNames.OBJECT_IS_PREFERRED
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

class StatelessClassesRuleWarnTest : LintTestBase(::StatelessClassesRule) {
private val ruleId = "$DIKTAT_RULE_SET_ID:stateless-class"

@Test
@Tag(OBJECT_IS_PREFERRED)
fun `should not trigger on class not extending any interface`() {
lintMethod(
"""
|class Some : I() {
| override fun some()
|}
""".trimMargin()
)
}

@Test
@Tag(OBJECT_IS_PREFERRED)
fun `should trigger on class extending interface`() {
lintMethod(
"""
|class Some : I {
| override fun some()
|}
|
|interface I {
| fun some()
|}
""".trimMargin(),
LintError(1, 1, ruleId, "${Warnings.OBJECT_IS_PREFERRED.warnText()} class Some", true)
)
}

@Test
@Tag(OBJECT_IS_PREFERRED)
fun `should not trigger on class with constructor`() {
lintMethod(
"""
|class Some(b: Int) : I {
|
| override fun some()
|}
""".trimMargin()
)
}

@Test
@Tag(OBJECT_IS_PREFERRED)
fun `should not trigger on class with no interface in this file`() {
lintMethod(
"""
|class Some : I {
|
| override fun some()
|}
""".trimMargin()
)
}

@Test
@Tag(OBJECT_IS_PREFERRED)
fun `should not trigger on class with state`() {
lintMethod(
"""
|class Some : I {
| val a = 5
| override fun some()
|}
""".trimMargin()
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package test.chapter6.stateless_classes

interface I {
fun foo()
}

object O: I {
override fun foo() {}
}

/**
* Some KDOC
*/
object A: I {
override fun foo() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package test.chapter6.stateless_classes

interface I {
fun foo()
}

class O: I {
override fun foo() {}
}

/**
* Some KDOC
*/
class A: I {
override fun foo() {}
}
3 changes: 2 additions & 1 deletion info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,5 @@
| 6 | 6.1.8 | CUSTOM_GETTERS_SETTERS | Check: Inspection that checks that no custom getters and setters are used for properties | no | - | - |
| 6 | 6.1.11 | COMPACT_OBJECT_INITIALIZATION | Checks if class instantiation can be wrapped in `apply` for better readability | | | |
| 6 | 6.2.2 | EXTENSION_FUNCTION_SAME_SIGNATURE | Checks if extension function has the same signature as another extension function and their classes are related | no | - | + |
| 6 | 6.4.1 | AVOID_USING_UTILITY_CLASS | Checks if there is class/object that can be replace with extension function | no | - | - |
| 6 | 6.4.1 | AVOID_USING_UTILITY_CLASS | Checks if there is class/object that can be replace with extension function | no | - | - |
| 6 | 6.4.2 | OBJECT_IS_PREFERRED | Checks: if class is stateless it is preferred to use `object` | yes | - | + |

0 comments on commit 6ea31f7

Please sign in to comment.