From c3c5f5c0a8d1533888a49cd697746f1c08585d46 Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Mon, 12 Sep 2022 20:53:53 +0300 Subject: [PATCH 01/10] WRONG_OVERLOADING_FUNCTION_ARGUMENTS: adding exceptions to the rule ### What's done: - rule will be triggered if and only if it has same modifiers --- .../chapter5/OverloadingArgumentsFunction.kt | 48 ++++++++++++++++++- .../OverloadingArgumentsFunctionWarnTest.kt | 43 +++++++++++++++-- 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt index ae705213dc..4710c06239 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt @@ -6,13 +6,20 @@ import org.cqfn.diktat.ruleset.rules.DiktatRule import org.cqfn.diktat.ruleset.utils.allSiblings import org.cqfn.diktat.ruleset.utils.findChildAfter import org.cqfn.diktat.ruleset.utils.findChildBefore +import org.cqfn.diktat.ruleset.utils.prettyPrint import com.pinterest.ktlint.core.ast.ElementType.FUN import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER import com.pinterest.ktlint.core.ast.ElementType.TYPE_REFERENCE +import org.jetbrains.kotlin.com.google.common.primitives.Ints.min import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.fir.analysis.checkers.getVisibility +import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtFunction +import org.jetbrains.kotlin.psi.KtParameter +import org.jetbrains.kotlin.psi.psiUtil.children import org.jetbrains.kotlin.psi.psiUtil.startOffset +import org.jetbrains.kotlin.psi.psiUtil.visibilityModifier /** * Rule that suggests to use functions with default parameters instead of multiple overloads @@ -36,15 +43,54 @@ class OverloadingArgumentsFunction(configRules: List) : DiktatRule( .asSequence() .filter { it.elementType == FUN } .map { it.psi as KtFunction } - .filter { it.nameIdentifier!!.text == funPsi.nameIdentifier!!.text && it.valueParameters.containsAll(funPsi.valueParameters) } + .filter { it.isOverloadedBy(funPsi) } + .filter { it.hasSameModifiers(funPsi) } .filter { funPsi.node.findChildBefore(IDENTIFIER, TYPE_REFERENCE)?.text == it.node.findChildBefore(IDENTIFIER, TYPE_REFERENCE)?.text } .filter { funPsi.node.findChildAfter(IDENTIFIER, TYPE_REFERENCE)?.text == it.node.findChildAfter(IDENTIFIER, TYPE_REFERENCE)?.text } .toList() + if (allOverloadFunction.isNotEmpty()) { WRONG_OVERLOADING_FUNCTION_ARGUMENTS.warn(configRules, emitWarn, isFixMode, funPsi.node.findChildByType(IDENTIFIER)!!.text, funPsi.startOffset, funPsi.node) } } + /** + * We can raise errors only on those methods that have same modifiers (inline/public/etc.) + */ + private fun KtFunction.hasSameModifiers(other: KtFunction): Boolean { + return this.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS)?.sortedBy { it.text } == + other.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS)?.sortedBy { it.text } + } + + /** + * we need to compare following things for two functions: + * 1) that function arguments go in the same order in both method + * 2) that arguments have SAME names (you can think that it is not necessary, + * but usually if developer really wants to overload method - he will have same names of arguments) + * 3) arguments have same types (obviously) + * + * So we need to find methods with following arguments: foo(a: Int, b: Int) and foo(a: Int). foo(b: Int) is NOT suitable + */ + private fun KtFunction.isOverloadedBy(other: KtFunction): Boolean { + // no need to process methods with different names + if (this.nameIdentifier?.text != other.nameIdentifier?.text) return false + // if this function has more arguments, than other, then we will compare it on the next iteration cycle (at logic() level) + // this hack will help us to point only to one function with smaller number of arguments + if (this.valueParameters.size < other.valueParameters.size) return false + + for (i in 0 until other.valueParameters.size) { + // all arguments on the same position should match by name and type + if (!( + this.valueParameters[i].getFunctionName() == other.valueParameters[i].getFunctionName() && + this.valueParameters[i].getFunctionType() == other.valueParameters[i].getFunctionType()) + ) return false + } + return true + } + + private fun KtParameter.getFunctionName() = this.nameIdentifier?.text + private fun KtParameter.getFunctionType() = this.typeReference?.text + companion object { const val NAME_ID = "overloading-default-values" } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/OverloadingArgumentsFunctionWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/OverloadingArgumentsFunctionWarnTest.kt index 2cc1e3b593..1291395313 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/OverloadingArgumentsFunctionWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/OverloadingArgumentsFunctionWarnTest.kt @@ -33,27 +33,60 @@ class OverloadingArgumentsFunctionWarnTest : LintTestBase(::OverloadingArguments | |fun goo(a: Double? = 0.0) {} | - |override fun goo() {} + |override fun goo() {} // this definitely is not an overload case... why we were treating it as an overload? New diktat rule! | |class A { | fun foo() {} |} | |abstract class B { - | abstract fun foo(a: Int) + | abstract fun foo(a: Int) // modifiers are different. This is not related to default arguments. New diktat rule! | | fun foo(){} |} """.trimMargin(), LintError(1, 1, ruleId, "${WRONG_OVERLOADING_FUNCTION_ARGUMENTS.warnText()} foo", false), - LintError(16, 1, ruleId, "${WRONG_OVERLOADING_FUNCTION_ARGUMENTS.warnText()} goo", false), - LintError(25, 4, ruleId, "${WRONG_OVERLOADING_FUNCTION_ARGUMENTS.warnText()} foo", false) ) } @Test @Tag(WarningNames.WRONG_OVERLOADING_FUNCTION_ARGUMENTS) - fun `check simple`() { + fun `functions with modifiers`() { + lintMethod( + """ + |public fun foo() {} + |private fun foo(a: Int) {} + |inline fun foo(a: Int, b: Int) {} + """.trimMargin(), + ) + } + + @Test + @Tag(WarningNames.WRONG_OVERLOADING_FUNCTION_ARGUMENTS) + fun `functions with unordered, but same modifiers`() { + lintMethod( + """ + |fun foo(a: Double) {} + |fun foo(a: Double, b: Int) {} + """.trimMargin(), + LintError(1, 1, ruleId, "${WRONG_OVERLOADING_FUNCTION_ARGUMENTS.warnText()} foo", false) + ) + } + + @Test + @Tag(WarningNames.WRONG_OVERLOADING_FUNCTION_ARGUMENTS) + fun `functions with unordered, but same modifiers and different names`() { + lintMethod( + """ + |fun foo(a: Double) {} + |fun foo(b: Double, b: Int) {} + """.trimMargin(), + ) + } + + @Test + @Tag(WarningNames.WRONG_OVERLOADING_FUNCTION_ARGUMENTS) + fun `check for extensions`() { lintMethod( """ private fun isComparisonWithAbs(psiElement: PsiElement) = From 7fe9d8f9242f50a0d95fdd80faa86f0d186c29c4 Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Tue, 13 Sep 2022 13:31:48 +0300 Subject: [PATCH 02/10] WRONG_OVERLOADING_FUNCTION_ARGUMENTS: adding exceptions to the rule ### What's done: - rule will be triggered if and only if it has same modifiers --- .../chapter5/OverloadingArgumentsFunction.kt | 30 +++++++++---------- .../OverloadingArgumentsFunctionWarnTest.kt | 6 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt index 4710c06239..263d22c52a 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt @@ -6,20 +6,15 @@ import org.cqfn.diktat.ruleset.rules.DiktatRule import org.cqfn.diktat.ruleset.utils.allSiblings import org.cqfn.diktat.ruleset.utils.findChildAfter import org.cqfn.diktat.ruleset.utils.findChildBefore -import org.cqfn.diktat.ruleset.utils.prettyPrint import com.pinterest.ktlint.core.ast.ElementType.FUN import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER import com.pinterest.ktlint.core.ast.ElementType.TYPE_REFERENCE -import org.jetbrains.kotlin.com.google.common.primitives.Ints.min import org.jetbrains.kotlin.com.intellij.lang.ASTNode -import org.jetbrains.kotlin.fir.analysis.checkers.getVisibility import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtFunction import org.jetbrains.kotlin.psi.KtParameter -import org.jetbrains.kotlin.psi.psiUtil.children import org.jetbrains.kotlin.psi.psiUtil.startOffset -import org.jetbrains.kotlin.psi.psiUtil.visibilityModifier /** * Rule that suggests to use functions with default parameters instead of multiple overloads @@ -57,10 +52,10 @@ class OverloadingArgumentsFunction(configRules: List) : DiktatRule( /** * We can raise errors only on those methods that have same modifiers (inline/public/etc.) */ - private fun KtFunction.hasSameModifiers(other: KtFunction): Boolean { - return this.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS)?.sortedBy { it.text } == - other.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS)?.sortedBy { it.text } - } + private fun KtFunction.hasSameModifiers(other: KtFunction): Boolean = this.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS) + ?.sortedBy { it.text } == + other.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS) + ?.sortedBy { it.text } /** * we need to compare following things for two functions: @@ -73,17 +68,22 @@ class OverloadingArgumentsFunction(configRules: List) : DiktatRule( */ private fun KtFunction.isOverloadedBy(other: KtFunction): Boolean { // no need to process methods with different names - if (this.nameIdentifier?.text != other.nameIdentifier?.text) return false + if (this.nameIdentifier?.text != other.nameIdentifier?.text) { + return false + } // if this function has more arguments, than other, then we will compare it on the next iteration cycle (at logic() level) // this hack will help us to point only to one function with smaller number of arguments - if (this.valueParameters.size < other.valueParameters.size) return false + if (this.valueParameters.size < other.valueParameters.size) { + return false + } for (i in 0 until other.valueParameters.size) { // all arguments on the same position should match by name and type - if (!( - this.valueParameters[i].getFunctionName() == other.valueParameters[i].getFunctionName() && - this.valueParameters[i].getFunctionType() == other.valueParameters[i].getFunctionType()) - ) return false + if (this.valueParameters[i].getFunctionName() != other.valueParameters[i].getFunctionName() || + this.valueParameters[i].getFunctionType() != other.valueParameters[i].getFunctionType() + ) { + return false + } } return true } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/OverloadingArgumentsFunctionWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/OverloadingArgumentsFunctionWarnTest.kt index 1291395313..eac4209fa8 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/OverloadingArgumentsFunctionWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/OverloadingArgumentsFunctionWarnTest.kt @@ -57,7 +57,7 @@ class OverloadingArgumentsFunctionWarnTest : LintTestBase(::OverloadingArguments |public fun foo() {} |private fun foo(a: Int) {} |inline fun foo(a: Int, b: Int) {} - """.trimMargin(), + """.trimMargin(), ) } @@ -68,7 +68,7 @@ class OverloadingArgumentsFunctionWarnTest : LintTestBase(::OverloadingArguments """ |fun foo(a: Double) {} |fun foo(a: Double, b: Int) {} - """.trimMargin(), + """.trimMargin(), LintError(1, 1, ruleId, "${WRONG_OVERLOADING_FUNCTION_ARGUMENTS.warnText()} foo", false) ) } @@ -80,7 +80,7 @@ class OverloadingArgumentsFunctionWarnTest : LintTestBase(::OverloadingArguments """ |fun foo(a: Double) {} |fun foo(b: Double, b: Int) {} - """.trimMargin(), + """.trimMargin(), ) } From 4515dc39b5c9e93358f41be314472be196bbcf0d Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Wed, 14 Sep 2022 17:02:58 +0300 Subject: [PATCH 03/10] WRONG_OVERLOADING_FUNCTION_ARGUMENTS: adding exceptions to the rule ### What's done: - rule will be triggered if and only if it has same modifiers --- .../chapter5/OverloadingArgumentsFunction.kt | 11 ++-- info/guide/guide-chapter-5.md | 57 ++++++++++--------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt index 263d22c52a..1cc6fcae17 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt @@ -52,10 +52,13 @@ class OverloadingArgumentsFunction(configRules: List) : DiktatRule( /** * We can raise errors only on those methods that have same modifiers (inline/public/etc.) */ - private fun KtFunction.hasSameModifiers(other: KtFunction): Boolean = this.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS) - ?.sortedBy { it.text } == - other.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS) - ?.sortedBy { it.text } + private fun KtFunction.hasSameModifiers(other: KtFunction) = + this.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS) + ?.map { it.text } + ?.sortedBy { it } == + other.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS) + ?.map { it.text } + ?.sortedBy { it } /** * we need to compare following things for two functions: diff --git a/info/guide/guide-chapter-5.md b/info/guide/guide-chapter-5.md index 687ac000b2..f23dbe73b8 100644 --- a/info/guide/guide-chapter-5.md +++ b/info/guide/guide-chapter-5.md @@ -3,8 +3,8 @@ This section describes the rules of using functions in your code. ### 5.1 Function design Developers can write clean code by gaining knowledge of how to build design patterns and avoid code smells. -You should utilize this approach, along with functional style, when writing Kotlin code. -The concepts behind functional style are as follows: +You should utilize this approach, along with functional style, when writing Kotlin code. +The concepts behind functional style are as follows: Functions are the smallest unit of combinable and reusable code. They should have clean logic, **high cohesion**, and **low coupling** to organize the code effectively. The code in functions should be simple and not conceal the author's original intentions. @@ -15,7 +15,7 @@ The only exceptions to this are state machines. Kotlin is [designed](https://www.slideshare.net/abreslav/whos-more-functional-kotlin-groovy-scala-or-java) to support and encourage functional programming, featuring the corresponding built-in mechanisms. Also, it supports standard collections and sequences feature methods that enable functional programming (for example, `apply`, `with`, `let`, and `run`), Kotlin Higher-Order functions, function types, lambdas, and default function arguments. -As [previously discussed](#r4.1.3), Kotlin supports and encourages the use of immutable types, which in turn motivates programmers to write pure functions that avoid side effects and have a corresponding output for specific input. +As [previously discussed](#r4.1.3), Kotlin supports and encourages the use of immutable types, which in turn motivates programmers to write pure functions that avoid side effects and have a corresponding output for specific input. The pipeline data flow for the pure function comprises a functional paradigm. It is easy to implement concurrent programming when you have chains of function calls, where each step features the following characteristics: 1. Simplicity 2. Verifiability @@ -27,13 +27,13 @@ The pipeline data flow for the pure function comprises a functional paradigm. It There can be only one side effect in this data stream, which can be placed only at the end of the execution queue. -#### 5.1.1 Avoid functions that are too long +#### 5.1.1 Avoid functions that are too long The function should be displayable on one screen and only implement one certain logic. If a function is too long, it often means complex and could be split or simplified. Functions should consist of 30 lines (non-empty and non-comment) in total. **Exception:** Some functions that implement complex algorithms may exceed 30 lines due to aggregation and comprehensiveness. -Linter warnings for such functions **can be suppressed**. +Linter warnings for such functions **can be suppressed**. Even if a long function works well, new problems or bugs may appear due to the function's complex logic once it is modified by someone else. Therefore, it is recommended to split such functions into several separate and shorter functions that are easier to manage. @@ -41,7 +41,7 @@ This approach will enable other programmers to read and modify the code properly #### 5.1.2 Avoid deep nesting of function code blocks, limiting to four levels The nesting depth of a function's code block is the depth of mutual inclusion between the code control blocks in the function (for example: if, for, while, and when). -Each nesting level will increase the amount of effort needed to read the code because you need to remember the current "stack" (for example, entering conditional statements and loops). +Each nesting level will increase the amount of effort needed to read the code because you need to remember the current "stack" (for example, entering conditional statements and loops). **Exception:** The nesting levels of the lambda expressions, local classes, and anonymous classes in functions are calculated based on the innermost function. The nesting levels of enclosing methods are not accumulated. Functional decomposition should be implemented to avoid confusion for the developer who reads the code. This will help the reader switch between contexts. @@ -52,37 +52,37 @@ With nested functions, the visibility context may not be evident to the code rea **Invalid example**: ```kotlin -fun foo() { - fun nested():String { - return "String from nested function" - } - println("Nested Output: ${nested()}") -} -``` +fun foo() { + fun nested():String { + return "String from nested function" + } + println("Nested Output: ${nested()}") +} +``` #### 5.1.4 Negated function calls Don't use negated function calls if it can be replaced with negated version of this function **Invalid example**: ```kotlin -fun foo() { +fun foo() { val list = listOf(1, 2, 3) - + if (!list.isEmpty()) { // Some cool logic } -} -``` +} +``` **Valid example**: ```kotlin -fun foo() { +fun foo() { val list = listOf(1, 2, 3) - + if (list.isNotEmpty()) { // Some cool logic } -} -``` +} +``` ### 5.2 Function arguments @@ -99,7 +99,7 @@ fun myFoo(someArg: Int, myLambda: () -> Unit) { } // usage -myFoo(1) { +myFoo(1) { println("hey") } ``` @@ -113,7 +113,8 @@ It is recommended that you use Data Classes and Maps to unify these function arg #### 5.2.3 Use default values for function arguments instead of overloading them In Java, default values for function arguments are prohibited. That is why the function should be overloaded when you need to create a function with fewer arguments. -In Kotlin, you can use default arguments instead. +In Kotlin, you can use default arguments instead. This is useful if methods have same modifiers (private/inline/etc.). +If you would like to have some different logic and code in these methods - then name them differently accordingly. **Invalid example**: ```kotlin @@ -124,14 +125,14 @@ private fun foo(arg: Int) { private fun foo() { // ... } -``` +``` **Valid example**: ```kotlin private fun foo(arg: Int = 0) { // ... } -``` +``` #### 5.2.4 Synchronizing code inside asynchronous code Try to avoid using `runBlocking` in asynchronous code @@ -140,14 +141,14 @@ Try to avoid using `runBlocking` in asynchronous code GlobalScope.async { runBlocking { count++ - } + } } ``` #### 5.2.5 Long lambdas should have explicit parameters The lambda without parameters shouldn't be too long. If a lambda is too long, it can confuse the user. Lambda without parameters should consist of 10 lines (non-empty and non-comment) in total. -#### 5.2.6 Avoid using unnecessary, custom label +#### 5.2.6 Avoid using unnecessary, custom label Expressions with unnecessary, custom labels generally increase complexity and worsen the maintainability of the code. **Invalid example**: @@ -191,4 +192,4 @@ arrays.map { arrays.map { array -> array.map { it.foo() } } -``` \ No newline at end of file +``` From 130b827421cac21973dc2adb93165404283ce53b Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Wed, 14 Sep 2022 17:44:55 +0300 Subject: [PATCH 04/10] WRONG_OVERLOADING_FUNCTION_ARGUMENTS: adding exceptions to the rule ### What's done: - rule will be triggered if and only if it has same modifiers --- .../org/cqfn/diktat/util/FixTestBase.kt | 35 ++++++++----------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/util/FixTestBase.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/util/FixTestBase.kt index 82a7234cde..c26808cc7d 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/util/FixTestBase.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/util/FixTestBase.kt @@ -43,8 +43,22 @@ open class FixTestBase( /** * @param expectedPath path to file with expected result, relative to [resourceFilePath] * @param testPath path to file with code that will be transformed by formatter, relative to [resourceFilePath] + * @param overrideRulesConfigList optional override to [rulesConfigList] + * @see fixAndCompareContent */ - protected fun fixAndCompare(expectedPath: String, testPath: String) { + protected fun fixAndCompare( + expectedPath: String, + testPath: String, + overrideRulesConfigList: List = emptyList() + ) { + val testComparatorUnit = if (overrideRulesConfigList.isNotEmpty()) { + TestComparatorUnit(resourceFilePath) { text, fileName -> + format(ruleSetProviderRef, text, fileName, overrideRulesConfigList) + } + } else { + testComparatorUnit + } + Assertions.assertTrue( testComparatorUnit .compareFilesFromResources(expectedPath, testPath) @@ -75,25 +89,6 @@ open class FixTestBase( return result } - /** - * @param expectedPath path to file with expected result, relative to [resourceFilePath] - * @param testPath path to file with code that will be transformed by formatter, relative to [resourceFilePath] - * @param overrideRulesConfigList optional override to [rulesConfigList] - * @see fixAndCompareContent - */ - protected fun fixAndCompare(expectedPath: String, - testPath: String, - overrideRulesConfigList: List - ) { - val testComparatorUnit = TestComparatorUnit(resourceFilePath) { text, fileName -> - format(ruleSetProviderRef, text, fileName, overrideRulesConfigList) - } - Assertions.assertTrue( - testComparatorUnit - .compareFilesFromResources(expectedPath, testPath) - ) - } - /** * Unlike [fixAndCompare], this method doesn't perform any assertions. * From 8fd9f9631a5108cdfff57b1d0f1ac23b905ea42b Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Wed, 14 Sep 2022 18:21:46 +0300 Subject: [PATCH 05/10] WRONG_OVERLOADING_FUNCTION_ARGUMENTS: adding exceptions to the rule ### What's done: - rule will be triggered if and only if it has same modifiers --- .../test/smoke/src/main/kotlin/Example4Expected.kt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt index 4d992d7d7d..8678da19d4 100644 --- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt @@ -40,7 +40,6 @@ fun `method name incorrect, part 4`() { // ;warn:41:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: foo (cannot be auto-corrected){{.*}} // ;warn:41:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: foo (cannot be auto-corrected){{.*}} -// ;warn:41:1: [WRONG_OVERLOADING_FUNCTION_ARGUMENTS] use default argument instead of function overloading: foo (cannot be auto-corrected){{.*}} fun foo() { foo( 0, @@ -63,11 +62,10 @@ fun bar() { } } -// ;warn:66:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: foo (cannot be auto-corrected){{.*}} -// ;warn:66:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: foo (cannot be auto-corrected){{.*}} -// ;warn:66:1: [WRONG_OVERLOADING_FUNCTION_ARGUMENTS] use default argument instead of function overloading: foo (cannot be auto-corrected){{.*}} +// ;warn:66:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: boo (cannot be auto-corrected){{.*}} +// ;warn:66:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: boo (cannot be auto-corrected){{.*}} @Suppress("") -fun foo() { +fun boo() { val y = "akgjsaujtmaksdkfasakgjsaujtmaksdkfasakgjsaujtmaksdkfasakgjsaujtm" + " aksdkfasfasakgjsaujtmaksdfasafasakgjsaujtmaksdfasakgjsaujtmaksdfasakgjsaujtmaksdfasakgjsaujtmaksdfasakgjsaujtmaksdkgjsaujtmaksdfasakgjsaujtmaksd" } From 820484ab5de498afb18dd423f4ebedfeecfb5096 Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Wed, 14 Sep 2022 18:40:58 +0300 Subject: [PATCH 06/10] WRONG_OVERLOADING_FUNCTION_ARGUMENTS: adding exceptions to the rule ### What's done: - rule will be triggered if and only if it has same modifiers --- .../kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt | 2 +- .../test/resources/test/smoke/src/main/kotlin/Example4Test.kt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt index 2813274f35..6276dc6556 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt @@ -83,7 +83,7 @@ class DiktatSmokeTest : DiktatSmokeTestBase() { @Test @Tag("DiktatRuleSetProvider") - fun `disable charters`() { + fun `disable chapters`() { overrideRulesConfig( emptyList(), mapOf( diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Test.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Test.kt index 831284655e..552a34989c 100644 --- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Test.kt +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Test.kt @@ -51,6 +51,6 @@ fun bar() { } @Suppress("") -fun foo() { +fun boo() { val y = "akgjsaujtmaksdkfasakgjsaujtmaksdkfasakgjsaujtmaksdkfasakgjsaujtm aksdkfasfasakgjsaujtmaksdfasafasakgjsaujtmaksdfasakgjsaujtmaksdfasakgjsaujtmaksdfasakgjsaujtmaksdfasakgjsaujtmaksdkgjsaujtmaksdfasakgjsaujtmaksd" -} \ No newline at end of file +} From b1de8150abb9889dfb3c197e9e6f2b62895052f6 Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Wed, 14 Sep 2022 18:47:05 +0300 Subject: [PATCH 07/10] WRONG_OVERLOADING_FUNCTION_ARGUMENTS: adding exceptions to the rule ### What's done: - rule will be triggered if and only if it has same modifiers --- .../rules/chapter5/OverloadingArgumentsFunction.kt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt index 1cc6fcae17..795ffafe1d 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter5/OverloadingArgumentsFunction.kt @@ -53,12 +53,14 @@ class OverloadingArgumentsFunction(configRules: List) : DiktatRule( * We can raise errors only on those methods that have same modifiers (inline/public/etc.) */ private fun KtFunction.hasSameModifiers(other: KtFunction) = - this.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS) - ?.map { it.text } - ?.sortedBy { it } == - other.modifierList?.node?.getChildren(KtTokens.MODIFIER_KEYWORDS) - ?.map { it.text } - ?.sortedBy { it } + this.getSortedModifiers() == + other.getSortedModifiers() + + private fun KtFunction.getSortedModifiers() = this.modifierList + ?.node + ?.getChildren(KtTokens.MODIFIER_KEYWORDS) + ?.map { it.text } + ?.sortedBy { it } /** * we need to compare following things for two functions: From fba450a8a7a6b23df1c6c6ea0e01f62b8fe511cc Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Wed, 14 Sep 2022 18:58:55 +0300 Subject: [PATCH 08/10] WRONG_OVERLOADING_FUNCTION_ARGUMENTS: adding exceptions to the rule ### What's done: - rule will be triggered if and only if it has same modifiers --- .../smoke/src/main/kotlin/Example4Expected.kt | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt index 8678da19d4..31a967ed77 100644 --- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt @@ -1,8 +1,8 @@ // ;warn:1:1: [FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared: Example4Expected.kt vs SpecialTagsInKdoc{{.*}} package org.cqfn.diktat -// ;warn:4:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: SpecialTagsInKdoc (cannot be auto-corrected){{.*}} -// ;warn:15:8: [KDOC_NO_EMPTY_TAGS] no empty descriptions in tag blocks are allowed: @return (cannot be auto-corrected){{.*}} +// ;warn:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: SpecialTagsInKdoc (cannot be auto-corrected){{.*}} +// ;warn:8: [KDOC_NO_EMPTY_TAGS] no empty descriptions in tag blocks are allowed: @return (cannot be auto-corrected){{.*}} class SpecialTagsInKdoc { /** * Empty function to test KDocs @@ -17,9 +17,9 @@ class SpecialTagsInKdoc { fun test() = Unit } -// ;warn:20:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} -// ;warn:20:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} -// ;warn:23:5: [BACKTICKS_PROHIBITED] backticks should not be used in identifier's naming. The only exception test methods marked with @Test annotation: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} +// ;warn:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} +// ;warn:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} +// ;warn:5: [BACKTICKS_PROHIBITED] backticks should not be used in identifier's naming. The only exception test methods marked with @Test annotation: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} fun `method name incorrect, part 4`() { val code = """ class TestPackageName { @@ -38,8 +38,8 @@ fun `method name incorrect, part 4`() { .foo() } -// ;warn:41:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: foo (cannot be auto-corrected){{.*}} -// ;warn:41:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: foo (cannot be auto-corrected){{.*}} +// ;warn:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: foo (cannot be auto-corrected){{.*}} +// ;warn:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: foo (cannot be auto-corrected){{.*}} fun foo() { foo( 0, @@ -51,8 +51,8 @@ fun foo() { ) } -// ;warn:55:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: bar (cannot be auto-corrected){{.*}} -// ;warn:55:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: bar (cannot be auto-corrected){{.*}} +// ;warn:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: bar (cannot be auto-corrected){{.*}} +// ;warn:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: bar (cannot be auto-corrected){{.*}} fun bar() { val diktatExtension = project.extensions.create(DIKTAT_EXTENSION, DiktatExtension::class.java).apply { inputs = project.fileTree("src").apply { @@ -62,8 +62,8 @@ fun bar() { } } -// ;warn:66:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: boo (cannot be auto-corrected){{.*}} -// ;warn:66:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: boo (cannot be auto-corrected){{.*}} +// ;warn:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: boo (cannot be auto-corrected){{.*}} +// ;warn:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: boo (cannot be auto-corrected){{.*}} @Suppress("") fun boo() { val y = "akgjsaujtmaksdkfasakgjsaujtmaksdkfasakgjsaujtmaksdkfasakgjsaujtm" + From dc0e02e0b68324ffac325b8017f604932b9843e3 Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Wed, 14 Sep 2022 20:18:30 +0300 Subject: [PATCH 09/10] WRONG_OVERLOADING_FUNCTION_ARGUMENTS: adding exceptions to the rule ### What's done: - rule will be triggered if and only if it has same modifiers --- .../smoke/src/main/kotlin/Example4Expected.kt | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt index 31a967ed77..0e9a51d174 100644 --- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt @@ -1,8 +1,8 @@ // ;warn:1:1: [FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared: Example4Expected.kt vs SpecialTagsInKdoc{{.*}} package org.cqfn.diktat -// ;warn:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: SpecialTagsInKdoc (cannot be auto-corrected){{.*}} -// ;warn:8: [KDOC_NO_EMPTY_TAGS] no empty descriptions in tag blocks are allowed: @return (cannot be auto-corrected){{.*}} +// ;warn:4:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: SpecialTagsInKdoc (cannot be auto-corrected){{.*}} +// ;warn:15:8: [KDOC_NO_EMPTY_TAGS] no empty descriptions in tag blocks are allowed: @return (cannot be auto-corrected){{.*}} class SpecialTagsInKdoc { /** * Empty function to test KDocs @@ -17,9 +17,9 @@ class SpecialTagsInKdoc { fun test() = Unit } -// ;warn:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} -// ;warn:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} -// ;warn:5: [BACKTICKS_PROHIBITED] backticks should not be used in identifier's naming. The only exception test methods marked with @Test annotation: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} +// ;warn:20:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} +// ;warn:20:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} +// ;warn:23:5: [BACKTICKS_PROHIBITED] backticks should not be used in identifier's naming. The only exception test methods marked with @Test annotation: `method name incorrect, part 4` (cannot be auto-corrected){{.*}} fun `method name incorrect, part 4`() { val code = """ class TestPackageName { @@ -38,8 +38,8 @@ fun `method name incorrect, part 4`() { .foo() } -// ;warn:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: foo (cannot be auto-corrected){{.*}} -// ;warn:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: foo (cannot be auto-corrected){{.*}} +// ;warn:42:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: foo (cannot be auto-corrected){{.*}} +// ;warn:42:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: foo (cannot be auto-corrected){{.*}} fun foo() { foo( 0, @@ -51,8 +51,8 @@ fun foo() { ) } -// ;warn:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: bar (cannot be auto-corrected){{.*}} -// ;warn:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: bar (cannot be auto-corrected){{.*}} +// ;warn:55:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: bar (cannot be auto-corrected){{.*}} +// ;warn:55:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: bar (cannot be auto-corrected){{.*}} fun bar() { val diktatExtension = project.extensions.create(DIKTAT_EXTENSION, DiktatExtension::class.java).apply { inputs = project.fileTree("src").apply { @@ -62,8 +62,8 @@ fun bar() { } } -// ;warn:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: boo (cannot be auto-corrected){{.*}} -// ;warn:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: boo (cannot be auto-corrected){{.*}} +// ;warn:66:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: boo (cannot be auto-corrected){{.*}} +// ;warn:66:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: boo (cannot be auto-corrected){{.*}} @Suppress("") fun boo() { val y = "akgjsaujtmaksdkfasakgjsaujtmaksdkfasakgjsaujtmaksdkfasakgjsaujtm" + From 8ac1b7a91a434f3b4ebf04f20d2481f86fd8e60c Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Wed, 14 Sep 2022 20:59:04 +0300 Subject: [PATCH 10/10] WRONG_OVERLOADING_FUNCTION_ARGUMENTS: adding exceptions to the rule ### What's done: - rule will be triggered if and only if it has same modifiers --- .../test/smoke/src/main/kotlin/Example4Expected.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt index 0e9a51d174..c62b955e9c 100644 --- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example4Expected.kt @@ -38,8 +38,8 @@ fun `method name incorrect, part 4`() { .foo() } -// ;warn:42:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: foo (cannot be auto-corrected){{.*}} -// ;warn:42:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: foo (cannot be auto-corrected){{.*}} +// ;warn:41:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: foo (cannot be auto-corrected){{.*}} +// ;warn:41:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: foo (cannot be auto-corrected){{.*}} fun foo() { foo( 0, @@ -51,8 +51,8 @@ fun foo() { ) } -// ;warn:55:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: bar (cannot be auto-corrected){{.*}} -// ;warn:55:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: bar (cannot be auto-corrected){{.*}} +// ;warn:54:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: bar (cannot be auto-corrected){{.*}} +// ;warn:54:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: bar (cannot be auto-corrected){{.*}} fun bar() { val diktatExtension = project.extensions.create(DIKTAT_EXTENSION, DiktatExtension::class.java).apply { inputs = project.fileTree("src").apply { @@ -62,8 +62,8 @@ fun bar() { } } -// ;warn:66:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: boo (cannot be auto-corrected){{.*}} -// ;warn:66:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: boo (cannot be auto-corrected){{.*}} +// ;warn:65:1: [MISSING_KDOC_TOP_LEVEL] all public and internal top-level classes and functions should have Kdoc: boo (cannot be auto-corrected){{.*}} +// ;warn:65:1: [MISSING_KDOC_ON_FUNCTION] all public, internal and protected functions should have Kdoc with proper tags: boo (cannot be auto-corrected){{.*}} @Suppress("") fun boo() { val y = "akgjsaujtmaksdkfasakgjsaujtmaksdkfasakgjsaujtmaksdkfasakgjsaujtm" +