diff --git a/diktat-analysis.yml b/diktat-analysis.yml index 5f2acffa7f..c5a57987fd 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -366,3 +366,7 @@ - name: COMPACT_OBJECT_INITIALIZATION enabled: true configuration: {} +# Checks explicit supertype qualification +- name: USELESS_SUPERTYPE + 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 2f2f6d9005..17ae0f745e 100644 --- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt +++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt @@ -213,4 +213,6 @@ public object WarningNames { public const val CUSTOM_GETTERS_SETTERS: String = "CUSTOM_GETTERS_SETTERS" public const val COMPACT_OBJECT_INITIALIZATION: String = "COMPACT_OBJECT_INITIALIZATION" + + public const val USELESS_SUPERTYPE: String = "USELESS_SUPERTYPE" } 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 101fb2b249..77e5eaf80d 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 @@ -128,6 +128,7 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED(true, "trivial property accessors are not recommended"), CUSTOM_GETTERS_SETTERS(false, "Custom getters and setters are not recommended, use class methods instead"), COMPACT_OBJECT_INITIALIZATION(true, "class instance can be initialized in `apply` block"), + USELESS_SUPERTYPE(true,"unnecessary supertype specification"), ; /** 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 031af66f22..d10a61237f 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 @@ -65,6 +65,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy ::PackageNaming, ::IdentifierNaming, // code structure + ::UselessSupertype, ::ClassLikeStructuresOrderRule, ::WhenMustHaveElseRule, ::BracesInConditionalsAndLoopsRule, diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/UselessSupertype.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/UselessSupertype.kt new file mode 100644 index 0000000000..cb5a5e47aa --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/UselessSupertype.kt @@ -0,0 +1,138 @@ +package org.cqfn.diktat.ruleset.rules + +import com.pinterest.ktlint.core.Rule +import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION +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_KEYWORD +import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION +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.MODIFIER_LIST +import com.pinterest.ktlint.core.ast.ElementType.OPEN_KEYWORD +import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.SUPER_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_CALL_ENTRY +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.ElementType.TYPE_REFERENCE +import com.pinterest.ktlint.core.ast.parent +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.ruleset.constants.EmitType +import org.cqfn.diktat.ruleset.constants.Warnings.USELESS_SUPERTYPE +import org.cqfn.diktat.ruleset.utils.* +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.psi.psiUtil.siblings + +/** + * rule 6.1.5 + * Explicit supertype qualification should not be used if there is not clash between called methods + * fixme can't fix supertypes that are defined in other files. + */ +class UselessSupertype(private val configRules: List) : Rule("useless-override") { + + companion object { + private val SUPER_TYPE = listOf(SUPER_TYPE_CALL_ENTRY, SUPER_TYPE_ENTRY) + } + + private lateinit var emitWarn: EmitType + private var isFixMode: Boolean = false + + override fun visit(node: ASTNode, + autoCorrect: Boolean, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { + emitWarn = emit + isFixMode = autoCorrect + + if (node.elementType == CLASS) + checkClass(node) + } + + private fun checkClass(node: ASTNode) { + val superNodes = node.findChildByType(SUPER_TYPE_LIST)?.findAllNodesWithCondition({ it.elementType in SUPER_TYPE })?.takeIf { it.isNotEmpty() } ?: return + val qualifiedSuperCalls = node.findAllNodesWithSpecificType(DOT_QUALIFIED_EXPRESSION) + .mapNotNull { findFunWithSuper(it) }.ifEmpty { return } + if (superNodes.size == 1) { + qualifiedSuperCalls.map { removeSupertype(it.first) } + } else { + handleManyImpl(superNodes, qualifiedSuperCalls) + } + } + + private fun handleManyImpl(superNodes: List, overrideNodes: List>) { + val uselessSuperType = findAllSupers(superNodes, overrideNodes.map { it.second.text }) + ?.filter { it.value == 1 } // filtering methods whose names occur only once + ?.map { it.key } // take their names + ?: return + overrideNodes + .filter { + it.second.text in uselessSuperType + }.map { + removeSupertype(it.first) + } + } + + @Suppress("UnsafeCallOnNullableType") + private fun removeSupertype(node: ASTNode) { + USELESS_SUPERTYPE.warnAndFix(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) { + val startNode = node.parent({ it.elementType == SUPER_EXPRESSION })!!.findChildByType(REFERENCE_EXPRESSION)!! + val lastNode = startNode.siblings(true).last() + startNode.treeParent.removeRange(startNode.treeNext, lastNode) + startNode.treeParent.removeChild(lastNode) + } + } + + /** + * Method finds pair of identifier supertype and method name else return null + * example: super.foo() -> return Pair(A, foo) + * super.foo() -> null + * @param node - node of type DOT_QUALIFIED_EXPRESSION + * + * @return pair of identifier + */ + @Suppress("UnsafeCallOnNullableType") + private fun findFunWithSuper(node: ASTNode): Pair? { + return Pair( + node.findChildByType(SUPER_EXPRESSION) + ?.findChildByType(TYPE_REFERENCE) + ?.findAllNodesWithSpecificType(IDENTIFIER) + ?.firstOrNull() ?: return null, + node.findChildByType(CALL_EXPRESSION) + ?.findAllNodesWithSpecificType(IDENTIFIER) + ?.firstOrNull() ?: return null) + } + + /** + * The method looks in the same file for all super interfaces or a class, in each it looks for methods + * that can be overridden and creates a map with a key - the name of the method and value - the number of times it meets + * + * @param superTypeList - list of identifiers super classes + * @param methodsName - name of overrides methods + * + * @return map name of method and the number of times it meets + */ + @Suppress("UnsafeCallOnNullableType") + private fun findAllSupers(superTypeList: List, methodsName: List): Map? { + val fileNode = superTypeList.first().parent({ it.elementType == FILE })!! + val superNodesIdentifier = superTypeList.map { it.findAllNodesWithSpecificType(IDENTIFIER).first().text } + val superNodes = fileNode.findAllNodesWithCondition({ superClass -> + superClass.elementType == CLASS && + superClass.getIdentifierName()!!.text in superNodesIdentifier + }).mapNotNull { it.findChildByType(CLASS_BODY) } + if (superNodes.size != superTypeList.size) + return null + val functionNameMap = hashMapOf() + superNodes.forEach { classBody -> + val overrideFunctions = classBody.findAllNodesWithSpecificType(FUN) + .filter { + (if (classBody.treeParent.hasChildOfType(CLASS_KEYWORD)) it.findChildByType(MODIFIER_LIST)!!.hasChildOfType(OPEN_KEYWORD) else true) && + it.getIdentifierName()!!.text in methodsName + } + overrideFunctions.forEach { + functionNameMap.compute(it.getIdentifierName()!!.text) { _, oldValue -> (oldValue ?: 0) + 1} + } + } + return functionNameMap.toMap() + } +} diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index 0d7bf44df2..942953b4c4 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -363,5 +363,9 @@ configuration: { } # Checks if class instantiation can be wrapped in `apply` for better readability - name: COMPACT_OBJECT_INITIALIZATION + enabled: true + configuration: {} +# Checks explicit supertype qualification +- name: USELESS_SUPERTYPE 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 02bf764bcf..d813db5b43 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -365,5 +365,9 @@ configuration: { } # Checks if class instantiation can be wrapped in `apply` for better readability - name: COMPACT_OBJECT_INITIALIZATION + enabled: true + configuration: {} +# Checks explicit supertype qualification +- name: USELESS_SUPERTYPE enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/UselessSupertypeFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/UselessSupertypeFixTest.kt new file mode 100644 index 0000000000..e150924323 --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/UselessSupertypeFixTest.kt @@ -0,0 +1,22 @@ +package org.cqfn.diktat.ruleset.chapter6 + +import generated.WarningNames +import org.cqfn.diktat.util.FixTestBase +import org.cqfn.diktat.ruleset.rules.UselessSupertype +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +class UselessSupertypeFixTest : FixTestBase("test/paragraph6/useless-override", ::UselessSupertype) { + + @Test + @Tag(WarningNames.USELESS_SUPERTYPE) + fun `fix example with one super`() { + fixAndCompare("UselessOverrideExpected.kt", "UselessOverrideTest.kt") + } + + @Test + @Tag(WarningNames.USELESS_SUPERTYPE) + fun `fix several super`() { + fixAndCompare("SeveralSuperTypesExpected.kt", "SeveralSuperTypesTest.kt") + } +} diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/UselessSupertypeWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/UselessSupertypeWarnTest.kt new file mode 100644 index 0000000000..4af8fda272 --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/UselessSupertypeWarnTest.kt @@ -0,0 +1,91 @@ +package org.cqfn.diktat.ruleset.chapter6 + +import com.pinterest.ktlint.core.LintError +import generated.WarningNames +import org.cqfn.diktat.ruleset.constants.Warnings.USELESS_SUPERTYPE +import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID +import org.cqfn.diktat.ruleset.rules.UselessSupertype +import org.cqfn.diktat.util.LintTestBase +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +class UselessSupertypeWarnTest: LintTestBase(::UselessSupertype) { + + private val ruleId = "$DIKTAT_RULE_SET_ID:useless-override" + + @Test + @Tag(WarningNames.USELESS_SUPERTYPE) + fun `check simple wrong example`() { + lintMethod( + """ + open class Rectangle { + open fun draw() { /* ... */ } + } + + class Square() : Rectangle() { + override fun draw() { + /** + * + * hehe + */ + super.draw() + } + } + + class Square2() : Rectangle() { + override fun draw() { + //hehe + /* + hehe + */ + super.draw() + } + } + + class Square2() : Rectangle() { + override fun draw() { + val q = super.draw() + } + } + + class A: Runnable { + override fun run() { + + } + } + """.trimMargin(), + LintError(11,35, ruleId, "${USELESS_SUPERTYPE.warnText()} Rectangle", true), + LintError(21,35, ruleId, "${USELESS_SUPERTYPE.warnText()} Rectangle", true) + ) + } + + @Test + @Tag(WarningNames.USELESS_SUPERTYPE) + fun `check example with two super`() { + lintMethod( + """ + open class Rectangle { + open fun draw() { /* ... */ } + } + + interface KK { + fun draw() {} + fun kk() {} + } + + class Square2() : Rectangle(), KK { + override fun draw() { + super.draw() + super.draw() + } + + private fun goo() { + super.kk() + } + + } + """.trimMargin(), + LintError(17,35, ruleId, "${USELESS_SUPERTYPE.warnText()} KK", true) + ) + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph6/useless-override/SeveralSuperTypesExpected.kt b/diktat-rules/src/test/resources/test/paragraph6/useless-override/SeveralSuperTypesExpected.kt new file mode 100644 index 0000000000..d6a17c35b0 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph6/useless-override/SeveralSuperTypesExpected.kt @@ -0,0 +1,28 @@ +package test.paragraph6.`useless-override` + +open class A { + open fun foo(){} +} + +interface C { + fun foo(){} + fun goo(){} +} + +class B: A(){ + override fun foo() { + super.foo() + } +} + +class D: A(), C { + override fun foo() { + super.foo() + super.foo() + super.goo() + } + + private fun qwe(){ + val q = super.goo() + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph6/useless-override/SeveralSuperTypesTest.kt b/diktat-rules/src/test/resources/test/paragraph6/useless-override/SeveralSuperTypesTest.kt new file mode 100644 index 0000000000..da8d5501a4 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph6/useless-override/SeveralSuperTypesTest.kt @@ -0,0 +1,28 @@ +package test.paragraph6.`useless-override` + +open class A { + open fun foo(){} +} + +interface C { + fun foo(){} + fun goo(){} +} + +class B: A(){ + override fun foo() { + super.foo() + } +} + +class D: A(), C { + override fun foo() { + super.foo() + super.foo() + super.goo() + } + + private fun qwe(){ + val q = super.goo() + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph6/useless-override/UselessOverrideExpected.kt b/diktat-rules/src/test/resources/test/paragraph6/useless-override/UselessOverrideExpected.kt new file mode 100644 index 0000000000..5d50b2da29 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph6/useless-override/UselessOverrideExpected.kt @@ -0,0 +1,37 @@ +package test.paragraph6 + +open class Rectangle { + open fun draw() { /* ... */ } +} + +class Square() : Rectangle() { + override fun draw() { + /** + * + * hehe + */ + super.draw() + } +} + +class Square2() : Rectangle() { + override fun draw() { + //hehe + /* + hehe + */ + super.draw() + } +} + +class Square2() : Rectangle() { + override fun draw() { + val q = super.draw() + } +} + +class A: Runnable { + override fun run() { + + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph6/useless-override/UselessOverrideTest.kt b/diktat-rules/src/test/resources/test/paragraph6/useless-override/UselessOverrideTest.kt new file mode 100644 index 0000000000..679a641295 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph6/useless-override/UselessOverrideTest.kt @@ -0,0 +1,37 @@ +package test.paragraph6 + +open class Rectangle { + open fun draw() { /* ... */ } +} + +class Square() : Rectangle() { + override fun draw() { + /** + * + * hehe + */ + super.draw() + } +} + +class Square2() : Rectangle() { + override fun draw() { + //hehe + /* + hehe + */ + super.draw() + } +} + +class Square2() : Rectangle() { + override fun draw() { + val q = super.draw() + } +} + +class A: Runnable { + override fun run() { + + } +} diff --git a/info/available-rules.md b/info/available-rules.md index d0f08298e6..f2ddcdbbe8 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -95,6 +95,7 @@ | 6 | 6.1.1 | SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY | Check: warns if there is only one secondary constructor in a class
Fix: converts it to a primary constructor | yes | no | Support more complicated logic of constructor conversion | | 6 | 6.1.2 | USE_DATA_CLASS | Check: if class can be made as data class | no | - | yes | | 6 | 6.1.4 | MULTIPLE_INIT_BLOCKS | Checks that classes have only one init block | yes | no | - | +| 6 | 6.1.5 | USELESS_SUPERTYPE | Check: if override function can be removed | yes| - | | | 6 | 6.1.6 | CLASS_SHOULD_NOT_BE_ABSTRACT | Checks: if abstract class has any abstract method. If not, warns that class should not be abstract
Fix: deletes abstract modifier | yes | - | - | | 6 | 6.1.9 | WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR | Check: used the name of a variable in the custom getter or setter | no | - | | 6 | 6.1.10 | TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED | Check: if there are any trivial getters or setters
Fix: Delete trivial getter or setter | yes | - | - |