From 08b2deb6a98dcd5e99c5d75738d5a8133873dae8 Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Mon, 16 Nov 2020 12:03:29 +0300 Subject: [PATCH 1/2] Updating Readme.md ### What's done: Removed extra tags as the code for maven plugin is small right now --- README.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 821fca0897..e60794c6ed 100644 --- a/README.md +++ b/README.md @@ -74,9 +74,6 @@ This plugin is available since version 0.1.3. You can see how it is configured i If you use it and encounter any problems, feel free to open issues on [github](https://github.com/cqfn/diktat/issues). Add this plugin to your pom.xml: -
- Maven plugin snippet
- ```xml org.cqfn.diktat @@ -102,10 +99,8 @@ Add this plugin to your pom.xml: ``` -
- -To run diktat check use command `$ mvn diktat:check@diktat`. -To run diktat in autocorrect mode use command `$ mvn diktat:fix@diktat`. +To run diktat in **only-check** mode use command `$ mvn diktat:check@diktat`. +To run diktat in **autocorrect** mode use command `$ mvn diktat:fix@diktat`. ## Run with Gradle using diktat-gradle-plugin This plugin is available since version 0.1.4. You can see how the plugin is configured in our project for self-checks: [build.gradle.kts](build.gradle.kts). From bb1227ae27c6f79d9a3d975ce7437dd0ff00beb4 Mon Sep 17 00:00:00 2001 From: Alexander Tsay <48321920+aktsay6@users.noreply.github.com> Date: Mon, 16 Nov 2020 12:36:18 +0300 Subject: [PATCH 2/2] Rule 6.2.2(#447) (#501) * feature/rule-6.2.2(#447) ### What's done: * Added rule logic * Added warn tests --- diktat-analysis.yml | 3 + .../src/main/kotlin/generated/WarningNames.kt | 2 + .../cqfn/diktat/ruleset/constants/Warnings.kt | 3 +- .../ruleset/rules/DiktatRuleSetProvider.kt | 1 + .../rules/ExtensionFunctionsSameNameRule.kt | 137 ++++++++++++++++ .../main/resources/diktat-analysis-huawei.yml | 3 + .../src/main/resources/diktat-analysis.yml | 3 + .../ExtensionFunctionsSameNameWarnTest.kt | 151 ++++++++++++++++++ info/available-rules.md | 3 +- 9 files changed, 304 insertions(+), 2 deletions(-) create mode 100644 diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ExtensionFunctionsSameNameRule.kt create mode 100644 diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ExtensionFunctionsSameNameWarnTest.kt diff --git a/diktat-analysis.yml b/diktat-analysis.yml index 4f0097c735..21cdc8c574 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -287,4 +287,7 @@ enabled: true # Checks explicit supertype qualification - name: USELESS_SUPERTYPE + enabled: true +# Checks if extension function with the same signature don't have related classes +- name: EXTENSION_FUNCTION_SAME_SIGNATURE enabled: true \ 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 17ae0f745e..66ebdba606 100644 --- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt +++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt @@ -215,4 +215,6 @@ public object WarningNames { public const val COMPACT_OBJECT_INITIALIZATION: String = "COMPACT_OBJECT_INITIALIZATION" public const val USELESS_SUPERTYPE: String = "USELESS_SUPERTYPE" + + public const val EXTENSION_FUNCTION_SAME_SIGNATURE: String = "EXTENSION_FUNCTION_SAME_SIGNATURE" } 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 77e5eaf80d..8641368aa7 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 @@ -125,10 +125,11 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR(false, "Use `field` keyword instead of property name inside property accessors"), MULTIPLE_INIT_BLOCKS(true, "Avoid using multiple `init` blocks, this logic can be moved to constructors or properties declarations"), CLASS_SHOULD_NOT_BE_ABSTRACT(true, "class should not be abstract, because it has no abstract functions"), - 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"), + TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED(true, "trivial property accessors are not recommended"), + EXTENSION_FUNCTION_SAME_SIGNATURE(false, "extension functions should not have same signature if their receiver classes are related"), ; /** 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 d10a61237f..df7262601a 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 @@ -103,6 +103,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy ::NullableTypeRule, ::ImmutableValNoVarRule, ::AvoidNestedFunctionsRule, + ::ExtensionFunctionsSameNameRule, // formatting: moving blocks, adding line breaks, indentations etc. ::ConsecutiveSpacesRule, ::WhiteSpaceRule, // this rule should be after other rules that can cause wrong spacing diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ExtensionFunctionsSameNameRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ExtensionFunctionsSameNameRule.kt new file mode 100644 index 0000000000..5afb08cb9b --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ExtensionFunctionsSameNameRule.kt @@ -0,0 +1,137 @@ +package org.cqfn.diktat.ruleset.rules + +import com.pinterest.ktlint.core.Rule +import com.pinterest.ktlint.core.ast.ElementType.CLASS +import com.pinterest.ktlint.core.ast.ElementType.COLON +import com.pinterest.ktlint.core.ast.ElementType.DOT +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.SUPER_TYPE_CALL_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.ElementType.VALUE_ARGUMENT_LIST +import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER_LIST +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.ruleset.constants.Warnings.EXTENSION_FUNCTION_SAME_SIGNATURE +import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType +import org.cqfn.diktat.ruleset.utils.findChildAfter +import org.cqfn.diktat.ruleset.utils.findChildBefore +import org.cqfn.diktat.ruleset.utils.findLeafWithSpecificType +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.KtNamedFunction +import org.jetbrains.kotlin.psi.KtParameter +import org.jetbrains.kotlin.psi.KtParameterList + +typealias RelatedClasses = List> +typealias SimilarSignatures = List> + +/** + * This rule checks if extension functions with the same signature don't have related classes + */ +class ExtensionFunctionsSameNameRule(private val configRules: List) : Rule("extension-functions-same-name") { + private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) + private var isFixMode: Boolean = false + + override fun visit(node: ASTNode, + autoCorrect: Boolean, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { + emitWarn = emit + isFixMode = autoCorrect + + /** + * 1) Collect all classes that extend other classes (collect related classes) + * 2) Collect all extension functions with same signature + * 3) Check if classes of functions are related + */ + if (node.elementType == FILE) { + val relatedClasses = collectAllRelatedClasses(node) + val extFunctionsWithSameName = collectAllExtensionFunctions(node) + handleFunctions(relatedClasses, extFunctionsWithSameName) + } + + } + + // Fixme: should find all related classes in project, not only in file + @Suppress("UnsafeCallOnNullableType") + private fun collectAllRelatedClasses(node: ASTNode) : List> { + val classListWithInheritance = node.findAllNodesWithSpecificType(CLASS) + .filterNot { (it.psi as KtClass).isInterface() } + .filter { it.hasChildOfType(SUPER_TYPE_LIST) } + + val pairs = mutableListOf>() + classListWithInheritance.forEach { classNode -> + val callEntries = classNode.findChildByType(SUPER_TYPE_LIST)!!.getAllChildrenWithType(SUPER_TYPE_CALL_ENTRY) + + callEntries.forEach { entry -> + val className = (classNode.psi as KtClass).name!! + val entryName = entry.findLeafWithSpecificType(IDENTIFIER)!! + pairs.add(Pair(className, entryName.text)) + } + } + return pairs + } + + @Suppress("UnsafeCallOnNullableType") + private fun collectAllExtensionFunctions(node: ASTNode) : SimilarSignatures { + val extensionFunctionList = node.findAllNodesWithSpecificType(FUN).filter { it.hasChildOfType(TYPE_REFERENCE) && it.hasChildOfType(DOT) } + val distinctFunctionSignatures = mutableMapOf() // maps function signatures on node it is used by + val extensionFunctionsPairs = mutableListOf>() // pairs extension functions with same signature + + extensionFunctionList.forEach { func -> + val functionName = (func.psi as KtNamedFunction).name!! + // List is used to show param names in warning + val params = (func.getFirstChildWithType(VALUE_PARAMETER_LIST)!!.psi as KtParameterList).parameters.map { it.name!! } + val returnType = func.findChildAfter(COLON, TYPE_REFERENCE)?.text + val className = func.findChildBefore(DOT, TYPE_REFERENCE)!!.text + val signature = FunctionSignature(functionName, params, returnType) + + if (distinctFunctionSignatures.contains(signature)) { + val secondFuncClassName = distinctFunctionSignatures[signature]!!.findChildBefore(DOT, TYPE_REFERENCE)!!.text + extensionFunctionsPairs.add(Pair( + ExtensionFunction(secondFuncClassName, signature, distinctFunctionSignatures[signature]!!), + ExtensionFunction(className, signature, func))) + } else { + distinctFunctionSignatures[signature] = func + } + } + + return extensionFunctionsPairs + } + + private fun handleFunctions(relatedClasses: RelatedClasses, functions: SimilarSignatures) { + functions.forEach { + val firstClassName = it.first.className + val secondClassName = it.second.className + + if (relatedClasses.hasRelatedClasses(Pair(firstClassName, secondClassName))) { + raiseWarning(it.first.node, it.first, it.second) + raiseWarning(it.second.node, it.first, it.second) + } + } + } + + private fun RelatedClasses.hasRelatedClasses(pair: Pair): Boolean { + return any { it.first == pair.first && it.second == pair.second + || it.first == pair.second && it.second == pair.first } + } + + private fun raiseWarning(node: ASTNode, firstFunc: ExtensionFunction, secondFunc: ExtensionFunction) { + EXTENSION_FUNCTION_SAME_SIGNATURE.warn(configRules, emitWarn, isFixMode, "$firstFunc and $secondFunc", node.startOffset, node) + } + + data class FunctionSignature(val name: String, val parameters: List, val returnType: String?) { + override fun toString(): String { + return "$name$parameters${if(returnType != null) ": $returnType" else ""}" + } + } + data class ExtensionFunction(val className: String, val signature: FunctionSignature, val node: ASTNode) { + override fun toString(): String { + return "fun $className.$signature" + } + } +} diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index dbd6fc74f5..0e56e932c6 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -285,4 +285,7 @@ enabled: true # Checks explicit supertype qualification - name: USELESS_SUPERTYPE + enabled: true +# Checks if extension function with the same signature don't have related classes +- name: EXTENSION_FUNCTION_SAME_SIGNATURE enabled: true \ 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 99cb7048bf..d05bedba56 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -287,4 +287,7 @@ enabled: true # Checks explicit supertype qualification - name: USELESS_SUPERTYPE + enabled: true +# Checks if extension function with the same signature don't have related classes +- name: EXTENSION_FUNCTION_SAME_SIGNATURE enabled: true \ No newline at end of file diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ExtensionFunctionsSameNameWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ExtensionFunctionsSameNameWarnTest.kt new file mode 100644 index 0000000000..1acf19119e --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ExtensionFunctionsSameNameWarnTest.kt @@ -0,0 +1,151 @@ +package org.cqfn.diktat.ruleset.chapter6 + +import com.pinterest.ktlint.core.LintError +import generated.WarningNames.EXTENSION_FUNCTION_SAME_SIGNATURE +import org.cqfn.diktat.ruleset.constants.Warnings +import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID +import org.cqfn.diktat.ruleset.rules.ExtensionFunctionsSameNameRule +import org.cqfn.diktat.util.LintTestBase +import org.junit.jupiter.api.Disabled +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +class ExtensionFunctionsSameNameWarnTest : LintTestBase(::ExtensionFunctionsSameNameRule) { + private val ruleId = "$DIKTAT_RULE_SET_ID:extension-functions-same-name" + + @Test + @Tag(EXTENSION_FUNCTION_SAME_SIGNATURE) + fun `should trigger on functions with same signatures`() { + lintMethod( + """ + |open class A + |class B: A(), C + |class D: A() + | + |fun A.foo() = "A" + |fun B.foo() = "B" + |fun D.foo() = "D" + | + |fun printClassName(s: A) { print(s.foo()) } + | + |fun main() { printClassName(B()) } + """.trimMargin(), + LintError(5, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_SAME_SIGNATURE.warnText()} fun A.foo[] and fun B.foo[]"), + LintError(5, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_SAME_SIGNATURE.warnText()} fun A.foo[] and fun D.foo[]"), + LintError(6, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_SAME_SIGNATURE.warnText()} fun A.foo[] and fun B.foo[]"), + LintError(7, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_SAME_SIGNATURE.warnText()} fun A.foo[] and fun D.foo[]"), + ) + } + + @Test + @Tag(EXTENSION_FUNCTION_SAME_SIGNATURE) + fun `should trigger on functions with same signatures 2`() { + lintMethod( + """ + |open class A + |class B: A(), C + | + |fun A.foo(some: Int) = "A" + |fun B.foo(some: Int) = "B" + | + |fun printClassName(s: A) { print(s.foo()) } + | + |fun main() { printClassName(B()) } + """.trimMargin(), + LintError(4, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_SAME_SIGNATURE.warnText()} fun A.foo[some] and fun B.foo[some]"), + LintError(5, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_SAME_SIGNATURE.warnText()} fun A.foo[some] and fun B.foo[some]") + ) + } + + @Test + @Tag(EXTENSION_FUNCTION_SAME_SIGNATURE) + fun `should not trigger on functions with different signatures`() { + lintMethod( + """ + |open class A + |class B: A(), C + | + |fun A.foo(): Boolean = return true + |fun B.foo() = "B" + | + |fun printClassName(s: A) { print(s.foo()) } + | + |fun main() { printClassName(B()) } + """.trimMargin() + ) + } + + @Test + @Tag(EXTENSION_FUNCTION_SAME_SIGNATURE) + fun `should not trigger on functions with different signatures 2`() { + lintMethod( + """ + |open class A + |class B: A(), C + | + |fun A.foo() = return true + |fun B.foo(some: Int) = "B" + | + |fun printClassName(s: A) { print(s.foo()) } + | + |fun main() { printClassName(B()) } + """.trimMargin() + ) + } + + @Test + @Tag(EXTENSION_FUNCTION_SAME_SIGNATURE) + fun `should not trigger on functions with unrelated classes`() { + lintMethod( + """ + |interface A + |class B: A + |class C + | + |fun C.foo() = "C" + |fun B.foo() = "B" + | + |fun printClassName(s: A) { print(s.foo()) } + | + |fun main() { printClassName(B()) } + """.trimMargin() + ) + } + + @Test + @Tag(EXTENSION_FUNCTION_SAME_SIGNATURE) + fun `should not trigger on regular func`() { + lintMethod( + """ + |interface A + |class B: A + |class C + | + |fun foo() = "C" + |fun bar() = "B" + | + |fun printClassName(s: A) { print(s.foo()) } + | + |fun main() { printClassName(B()) } + """.trimMargin() + ) + } + + @Test + @Disabled + @Tag(EXTENSION_FUNCTION_SAME_SIGNATURE) + fun `should trigger on classes in other files`() { + lintMethod( + """ + |fun A.foo() = "A" + |fun B.foo() = "B" + | + |fun printClassName(s: A) { print(s.foo()) } + | + |fun main() { printClassName(B()) } + """.trimMargin(), + LintError(1, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_SAME_SIGNATURE.warnText()} fun A.foo() and fun B.foo()"), + LintError(2, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_SAME_SIGNATURE.warnText()} fun A.foo() and fun B.foo()") + ) + } +} diff --git a/info/available-rules.md b/info/available-rules.md index f2ddcdbbe8..632c4ab378 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -100,4 +100,5 @@ | 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 | - | - | | 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 | | | | \ No newline at end of file +| 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 | - | + | \ No newline at end of file