From 325411c5d254deb38f590f4782fe6162d87e356b Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Mon, 19 Oct 2020 18:45:01 +0300 Subject: [PATCH 1/5] Added logic for parameters and supertypes lists to NewlinesRule ### What's done: * Changed logic * Added tests * Updated code style guide --- diktat-analysis.yml | 3 +- diktat-rules/pom.xml | 4 +- .../ruleset/rules/files/NewlinesRule.kt | 99 ++++++++++++++++--- .../main/resources/diktat-analysis-huawei.yml | 3 +- .../src/main/resources/diktat-analysis.yml | 3 +- .../chapter3/files/NewlinesRuleFixTest.kt | 6 ++ .../chapter3/files/NewlinesRuleWarnTest.kt | 62 ++++++++++++ .../newlines/ParameterListExpected.kt | 26 +++++ .../paragraph3/newlines/ParameterListTest.kt | 15 +++ info/diktat-kotlin-coding-style-guide-en.md | 25 +++-- 10 files changed, 224 insertions(+), 22 deletions(-) create mode 100644 diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListExpected.kt create mode 100644 diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListTest.kt diff --git a/diktat-analysis.yml b/diktat-analysis.yml index e3f311d5c0..3e354c139e 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -189,7 +189,8 @@ configuration: {} - name: WRONG_NEWLINES enabled: true - configuration: {} + configuration: + maxParametersInOneLine: 2 - name: TOO_MANY_CONSECUTIVE_SPACES enabled: true configuration: diff --git a/diktat-rules/pom.xml b/diktat-rules/pom.xml index 31e11a7f20..75c21dc8e5 100644 --- a/diktat-rules/pom.xml +++ b/diktat-rules/pom.xml @@ -10,8 +10,8 @@ org.cqfn.diktat diktat-parent 0.1.3-SNAPSHOT - + 1.8 1.8 @@ -73,6 +73,8 @@ + ${project.basedir}/src/main/kotlin + ${project.basedir}/src/test/kotlin org.jetbrains.kotlin diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt index b02bbc8e0a..4fc3bf301d 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt @@ -8,6 +8,7 @@ import com.pinterest.ktlint.core.ast.ElementType.BLOCK import com.pinterest.ktlint.core.ast.ElementType.BLOCK_COMMENT import com.pinterest.ktlint.core.ast.ElementType.CALLABLE_REFERENCE_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.CLASS import com.pinterest.ktlint.core.ast.ElementType.COLON import com.pinterest.ktlint.core.ast.ElementType.COLONCOLON import com.pinterest.ktlint.core.ast.ElementType.COMMA @@ -37,46 +38,44 @@ import com.pinterest.ktlint.core.ast.ElementType.PACKAGE_DIRECTIVE import com.pinterest.ktlint.core.ast.ElementType.PLUS import com.pinterest.ktlint.core.ast.ElementType.PLUSEQ import com.pinterest.ktlint.core.ast.ElementType.PRIMARY_CONSTRUCTOR -import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.RETURN import com.pinterest.ktlint.core.ast.ElementType.RETURN_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.SAFE_ACCESS import com.pinterest.ktlint.core.ast.ElementType.SAFE_ACCESS_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.SECONDARY_CONSTRUCTOR import com.pinterest.ktlint.core.ast.ElementType.SEMICOLON -import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT +import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_LIST import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT_LIST +import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER_LIST import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE -import com.pinterest.ktlint.core.ast.isLeaf import com.pinterest.ktlint.core.ast.nextCodeSibling import com.pinterest.ktlint.core.ast.parent import com.pinterest.ktlint.core.ast.prevCodeSibling +import org.cqfn.diktat.common.config.rules.RuleConfiguration import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.common.config.rules.getRuleConfig import org.cqfn.diktat.ruleset.constants.Warnings.REDUNDANT_SEMICOLON import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_NEWLINES import org.cqfn.diktat.ruleset.utils.appendNewlineMergingWhiteSpace import org.cqfn.diktat.ruleset.utils.emptyBlockList import org.cqfn.diktat.ruleset.utils.extractLineOfText import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType +import org.cqfn.diktat.ruleset.utils.getIdentifierName import org.cqfn.diktat.ruleset.utils.isBeginByNewline import org.cqfn.diktat.ruleset.utils.isEol import org.cqfn.diktat.ruleset.utils.isFollowedByNewline import org.cqfn.diktat.ruleset.utils.isSingleLineIfElse import org.cqfn.diktat.ruleset.utils.leaveOnlyOneNewLine +import org.cqfn.diktat.ruleset.utils.log +import org.jetbrains.kotlin.backend.common.onlyIf import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl -import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet -import org.jetbrains.kotlin.ir.expressions.IrStatementOrigin -import org.jetbrains.kotlin.psi.KtDotQualifiedExpression -import org.jetbrains.kotlin.psi.KtExpression -import org.jetbrains.kotlin.psi.KtExpressionImpl -import org.jetbrains.kotlin.psi.KtPostfixExpression -import org.jetbrains.kotlin.psi.KtQualifiedExpression -import org.jetbrains.kotlin.psi.KtSafeQualifiedExpression +import org.jetbrains.kotlin.psi.KtParameterList +import org.jetbrains.kotlin.psi.KtSuperTypeList import org.jetbrains.kotlin.psi.psiUtil.children import org.jetbrains.kotlin.psi.psiUtil.parents import org.jetbrains.kotlin.psi.psiUtil.siblings @@ -91,6 +90,7 @@ import org.jetbrains.kotlin.psi.psiUtil.siblings * 6. Ensures that function or constructor name isn't separated from `(` by space or newline * 7. Ensures that in multiline lambda newline follows arrow or, in case of lambda without explicit parameters, opening brace * 8. Checks that functions with single `return` are simplified to functions with expression body + * 9. parameter or argument lists and supertype lists that have more than 2 elements should be separated by newlines */ @Suppress("ForbiddenComment") class NewlinesRule(private val configRules: List) : Rule("newlines") { @@ -107,8 +107,11 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" private val dropChainValues = TokenSet.create(EOL_COMMENT, WHITE_SPACE, BLOCK_COMMENT, KDOC) } - private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) + private val configuration by lazy { + NewlinesRuleConfiguration(configRules.getRuleConfig(WRONG_NEWLINES)?.configuration ?: emptyMap()) + } private var isFixMode: Boolean = false + private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) override fun visit(node: ASTNode, autoCorrect: Boolean, @@ -124,9 +127,13 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" COMMA -> handleComma(node) BLOCK -> handleLambdaBody(node) RETURN -> handleReturnStatement(node) + SUPER_TYPE_LIST, VALUE_PARAMETER_LIST -> handleList(node) } } + /** + * Check that EOL semicolon is used only in enums + */ private fun handleSemicolon(node: ASTNode) { if (node.isEol() && node.treeParent.elementType != ENUM_ENTRY) { // semicolon at the end of line which is not part of enum members declarations @@ -213,6 +220,9 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" } } + /** + * Check that newline is not placed before a comma + */ private fun handleComma(node: ASTNode) { val prevNewLine = node .parent({ it.treePrev != null }, strict = false) @@ -296,6 +306,64 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" } } + /** + * Checks that members of [VALUE_PARAMETER_LIST] (list of function parameters at declaration site) are separated with newlines. + * Also checks that entries of [SUPER_TYPE_LIST] are separated by newlines. + */ + private fun handleList(node: ASTNode) { + val (numEntries, entryType) = when (node.elementType) { + VALUE_PARAMETER_LIST -> (node.psi as KtParameterList).parameters.size to "value parameters" + SUPER_TYPE_LIST -> (node.psi as KtSuperTypeList).entries.size to "supertype list entries" + else -> { + log.warn("Unexpected node element type ${node.elementType}") + return + } + } + if (numEntries > configuration.maxParametersInOneLine) { + when (node.elementType) { + VALUE_PARAMETER_LIST -> handleFirstValueParameter(node) + } + + node + .children() + .filter { + it.elementType == COMMA && + !it.treeNext.run { elementType == WHITE_SPACE && textContains('\n') } + } + .toList() + .onlyIf({ isNotEmpty() }) { invalidCommas -> + WRONG_NEWLINES.warnAndFix(configRules, emitWarn, isFixMode, + "$entryType should be placed on different lines in declaration of <${node.getParentIdentifier()}>", node.startOffset, node) { + invalidCommas.forEach { comma -> + val nextWhiteSpace = comma.treeNext.takeIf { it.elementType == WHITE_SPACE } + comma.appendNewlineMergingWhiteSpace(nextWhiteSpace, nextWhiteSpace?.treeNext ?: comma.treeNext) + } + } + } + } + } + + private fun handleFirstValueParameter(node: ASTNode) { + node + .children() + .takeWhile { !it.textContains('\n') } + .filter { it.elementType == VALUE_PARAMETER } + .toList() + .onlyIf({ size > 1 }) { + WRONG_NEWLINES.warnAndFix(configRules, emitWarn, isFixMode, "first parameter should be placed on a separate line " + + "or all other parameters should be aligned with it in declaration of <${node.getParentIdentifier()}>", node.startOffset, node) { + node.appendNewlineMergingWhiteSpace(it.first().treePrev.takeIf { it.elementType == WHITE_SPACE }, it.first()) + } + } + } + + @Suppress("UnsafeCallOnNullableType") + private fun ASTNode.getParentIdentifier() = when (treeParent.elementType) { + PRIMARY_CONSTRUCTOR -> treeParent.treeParent + SECONDARY_CONSTRUCTOR -> parent(CLASS)!! + else -> treeParent + } + .getIdentifierName()?.text private fun ASTNode.getOrderedCallExpressions(psi: PsiElement, result: MutableList) { // if statements here have the only right order - don't change it @@ -373,3 +441,10 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" firstChildNode.elementType == IDENTIFIER && treeParent.elementType == BINARY_EXPRESSION } + +private class NewlinesRuleConfiguration(config: Map) : RuleConfiguration(config) { + /** + * If the number of parameters on one line is more than this threshold, all parameters should be placed on separate lines. + */ + val maxParametersInOneLine = config["maxParametersInOneLine"]?.toInt() ?: 2 +} diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index 5c7e89d7fe..fd29037e25 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -186,7 +186,8 @@ configuration: {} - name: WRONG_NEWLINES enabled: true - configuration: {} + configuration: + maxParametersInOneLine: 2 - name: TOO_MANY_CONSECUTIVE_SPACES enabled: true configuration: diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml index 98a236b47c..74bfce979d 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -189,7 +189,8 @@ configuration: {} - name: WRONG_NEWLINES enabled: true - configuration: {} + configuration: + maxParametersInOneLine: 2 - name: TOO_MANY_CONSECUTIVE_SPACES enabled: true configuration: diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt index 4e15e27d19..cf2062db62 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt @@ -48,4 +48,10 @@ class NewlinesRuleFixTest : FixTestBase("test/paragraph3/newlines", ::NewlinesRu fun `should replace functions with only return with expression body`() { fixAndCompare("ExpressionBodyExpected.kt", "ExpressionBodyTest.kt") } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `should insert newlines in a long parameter or supertype list`() { + fixAndCompare("ParameterListExpected.kt", "ParameterListTest.kt") + } } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt index 0cbd4c0fbb..2ad8d7956f 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt @@ -579,4 +579,66 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { LintError(19, 20, ruleId,"${WRONG_NEWLINES.warnText()} should follow functional style at .", true) ) } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `should suggest newlines in a long argument list of a constructor`() { + lintMethod( + """ + |class Foo(val arg1: Int, arg2: Int) { } + | + |class Foo(val arg1: Int, arg2: Int, arg3: Int) { + | constructor(arg1: Int, arg2: String, arg3: String) : this(arg1, 0, 0) { } + |} + | + |class Foo(val arg1: Int, + | var arg2: Int, + | arg3: Int) { } + """.trimMargin(), + LintError(3, 10, ruleId, "${WRONG_NEWLINES.warnText()} first parameter should be placed on a separate line or all other parameters should be aligned with it in declaration of ", true), + LintError(3, 10, ruleId, "${WRONG_NEWLINES.warnText()} value parameters should be placed on different lines in declaration of ", true), + LintError(4, 16, ruleId, "${WRONG_NEWLINES.warnText()} first parameter should be placed on a separate line or all other parameters should be aligned with it in declaration of ", true), + LintError(4, 16, ruleId, "${WRONG_NEWLINES.warnText()} value parameters should be placed on different lines in declaration of ", true) + ) + } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `should suggest newlines in a long argument list`() { + lintMethod( + """ + |fun foo(arg1: Int, arg2: Int) { } + | + |fun bar(arg1: Int, arg2: Int, arg3: Int) { } + | + |fun baz(arg1: Int, + | arg2: Int, + | arg3: Int) { } + """.trimMargin(), + LintError(3, 8, ruleId, "${WRONG_NEWLINES.warnText()} first parameter should be placed on a separate line or all other parameters should be aligned with it in declaration of ", true), + LintError(3, 8, ruleId, "${WRONG_NEWLINES.warnText()} value parameters should be placed on different lines in declaration of ", true) + ) + } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `should suggest newlines in a long supertype list`() { + lintMethod( + """ + |class Foo : + | FooBase(), + | BazInterface, + | BazSuperclass { } + | + |class Foo : FooBase(), BazInterface, + | BazSuperclass { } + | + |class Foo : FooBase(), BazInterface, BazSuperclass { } + | + |class Foo : FooBase() { } + """.trimMargin(), + LintError(6, 13, ruleId, "${WRONG_NEWLINES.warnText()} supertype list entries should be placed on different lines in declaration of ", true), + LintError(9, 13, ruleId, "${WRONG_NEWLINES.warnText()} supertype list entries should be placed on different lines in declaration of ", true) + ) + } } diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListExpected.kt new file mode 100644 index 0000000000..c18ac4bf33 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListExpected.kt @@ -0,0 +1,26 @@ +package test.paragraph3.newlines + +fun bar( +arg1: Int, + arg2: Int, + arg3: Int) { } + +class Foo : FooBase(), + BazInterface, + BazSuperclass { } + +class Foo(val arg1: Int, arg2: Int) { } + +class Foo( +val arg1: Int, + arg2: Int, + arg3: Int) { + constructor( +arg1: Int, + arg2: String, + arg3: String) : this(arg1, 0, 0) { } +} + +class Foo(val arg1: Int, + var arg2: Int, + arg3: Int) { } \ No newline at end of file diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListTest.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListTest.kt new file mode 100644 index 0000000000..a6e18116a4 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListTest.kt @@ -0,0 +1,15 @@ +package test.paragraph3.newlines + +fun bar(arg1: Int, arg2: Int, arg3: Int) { } + +class Foo : FooBase(), BazInterface, BazSuperclass { } + +class Foo(val arg1: Int, arg2: Int) { } + +class Foo(val arg1: Int, arg2: Int, arg3: Int) { + constructor(arg1: Int, arg2: String, arg3: String) : this(arg1, 0, 0) { } +} + +class Foo(val arg1: Int, + var arg2: Int, + arg3: Int) { } \ No newline at end of file diff --git a/info/diktat-kotlin-coding-style-guide-en.md b/info/diktat-kotlin-coding-style-guide-en.md index 54d06279dd..a94827cf43 100644 --- a/info/diktat-kotlin-coding-style-guide-en.md +++ b/info/diktat-kotlin-coding-style-guide-en.md @@ -1034,13 +1034,12 @@ use: override fun toString() = "hi" ``` -7. If argument list in function declaration (including constructors)/function call contains more than 2 arguments - these arguments should be split by newlines in the following style: +7. If argument list in function declaration (including constructors) contains more than 2 arguments, these arguments should be split by newlines: ```kotlin -val a = checkMissingPackageName( - node, - realPackageName, - params.fileName!! -) +class Foo(val a: String, + b: String, + val c: String) { +} fun foo( a: String, @@ -1051,6 +1050,20 @@ fun foo( } ``` +If and only if the first parameter is on the same line as an opening parenthesis, all parameters can horizontally aligned by the first parameter. +Otherwise, there should be a line break after an opening parenthesis. + +Kotlin 1.4 introduced a trailing comma as an optional feature, so it is generally recommended to place all parameters on a separate line +and append trailing comma. It makes resolving of merge conflicts easier. + +8. If supertype list has more than 2 elements, they should be separated by newlines +```kotlin +class MyFavouriteVeryLongClassHolder : + MyLongHolder(), + SomeOtherInterface, + AndAnotherOne { } +``` + ### Blank lines ### Recommendation 3.6: Reduce unnecessary blank lines and keep the code compact From 9567686b7303dc35ec97edf4b03208074f92d977 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Mon, 19 Oct 2020 18:54:45 +0300 Subject: [PATCH 2/5] Added logic for parameters and supertypes lists to NewlinesRule ### What's done: * Code style --- .../org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt | 2 +- .../ruleset/chapter3/files/NewlinesRuleWarnTest.kt | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt index 4fc3bf301d..ea90bc2b6f 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt @@ -92,7 +92,7 @@ import org.jetbrains.kotlin.psi.psiUtil.siblings * 8. Checks that functions with single `return` are simplified to functions with expression body * 9. parameter or argument lists and supertype lists that have more than 2 elements should be separated by newlines */ -@Suppress("ForbiddenComment") +@Suppress("ForbiddenComment", "LargeClass") class NewlinesRule(private val configRules: List) : Rule("newlines") { companion object { // fixme: these token sets can be not full, need to add new once as corresponding cases are discovered. diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt index 2ad8d7956f..da4aa4b985 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt @@ -595,9 +595,11 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | var arg2: Int, | arg3: Int) { } """.trimMargin(), - LintError(3, 10, ruleId, "${WRONG_NEWLINES.warnText()} first parameter should be placed on a separate line or all other parameters should be aligned with it in declaration of ", true), + LintError(3, 10, ruleId, "${WRONG_NEWLINES.warnText()} first parameter should be placed on a separate line or all other parameters " + + "should be aligned with it in declaration of ", true), LintError(3, 10, ruleId, "${WRONG_NEWLINES.warnText()} value parameters should be placed on different lines in declaration of ", true), - LintError(4, 16, ruleId, "${WRONG_NEWLINES.warnText()} first parameter should be placed on a separate line or all other parameters should be aligned with it in declaration of ", true), + LintError(4, 16, ruleId, "${WRONG_NEWLINES.warnText()} first parameter should be placed on a separate line or all other parameters " + + "should be aligned with it in declaration of ", true), LintError(4, 16, ruleId, "${WRONG_NEWLINES.warnText()} value parameters should be placed on different lines in declaration of ", true) ) } @@ -615,7 +617,8 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | arg2: Int, | arg3: Int) { } """.trimMargin(), - LintError(3, 8, ruleId, "${WRONG_NEWLINES.warnText()} first parameter should be placed on a separate line or all other parameters should be aligned with it in declaration of ", true), + LintError(3, 8, ruleId, "${WRONG_NEWLINES.warnText()} first parameter should be placed on a separate line or all other parameters " + + "should be aligned with it in declaration of ", true), LintError(3, 8, ruleId, "${WRONG_NEWLINES.warnText()} value parameters should be placed on different lines in declaration of ", true) ) } From 54f108f7d8082fda211b809cf45d4b51bc50ab8e Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Tue, 20 Oct 2020 10:25:16 +0300 Subject: [PATCH 3/5] Added logic for parameters and supertypes lists to NewlinesRule ### What's done: * Code style --- .../kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt | 2 +- .../cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt index ea90bc2b6f..4fc3bf301d 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt @@ -92,7 +92,7 @@ import org.jetbrains.kotlin.psi.psiUtil.siblings * 8. Checks that functions with single `return` are simplified to functions with expression body * 9. parameter or argument lists and supertype lists that have more than 2 elements should be separated by newlines */ -@Suppress("ForbiddenComment", "LargeClass") +@Suppress("ForbiddenComment") class NewlinesRule(private val configRules: List) : Rule("newlines") { companion object { // fixme: these token sets can be not full, need to add new once as corresponding cases are discovered. diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt index da4aa4b985..8fbf46cc47 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt @@ -11,6 +11,7 @@ import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Test +@Suppress("LargeClass") class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { private val ruleId = "$DIKTAT_RULE_SET_ID:newlines" private val shouldBreakAfter = "${WRONG_NEWLINES.warnText()} should break a line after and not before" From fe97df98298204a43d47e179539c248b8f8bdece Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Wed, 21 Oct 2020 18:40:00 +0300 Subject: [PATCH 4/5] Issues #420 ### What's done: * Fixes --- .../org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt | 7 +++++++ .../diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt index 4fc3bf301d..e2bfff7352 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt @@ -22,6 +22,8 @@ import com.pinterest.ktlint.core.ast.ElementType.EOL_COMMENT import com.pinterest.ktlint.core.ast.ElementType.EQ import com.pinterest.ktlint.core.ast.ElementType.FUN import com.pinterest.ktlint.core.ast.ElementType.FUNCTION_LITERAL +import com.pinterest.ktlint.core.ast.ElementType.FUNCTION_TYPE +import com.pinterest.ktlint.core.ast.ElementType.FUNCTION_TYPE_RECEIVER import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER import com.pinterest.ktlint.core.ast.ElementType.IF import com.pinterest.ktlint.core.ast.ElementType.IMPORT_DIRECTIVE @@ -311,6 +313,11 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" * Also checks that entries of [SUPER_TYPE_LIST] are separated by newlines. */ private fun handleList(node: ASTNode) { + if (node.elementType == VALUE_PARAMETER_LIST && node.treeParent.elementType.let { it == FUNCTION_TYPE || it == FUNCTION_TYPE_RECEIVER }) { + // do not check other value lists + return + } + val (numEntries, entryType) = when (node.elementType) { VALUE_PARAMETER_LIST -> (node.psi as KtParameterList).parameters.size to "value parameters" SUPER_TYPE_LIST -> (node.psi as KtSuperTypeList).entries.size to "supertype list entries" diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt index 8fbf46cc47..75d22e9799 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt @@ -614,6 +614,12 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | |fun bar(arg1: Int, arg2: Int, arg3: Int) { } | + |// should not trigger on functional types + |fun bar(arg1: (_arg1: Int, _arg2: Int, _arg3: Int) -> Int) { } + | + |// should not trigger on functional type receivers + |fun bar(arg1: Foo.(_arg1: Int, _arg2: Int, _arg3: Int) -> Int) { } + | |fun baz(arg1: Int, | arg2: Int, | arg3: Int) { } From 75dad24ebb6e936f390ba9ee0c66fc96afcd682f Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Mon, 26 Oct 2020 18:45:28 +0300 Subject: [PATCH 5/5] Issue #420 ### What's done: * code style --- .../ruleset/rules/files/NewlinesRule.kt | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt index e2bfff7352..70b34eed57 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt @@ -331,27 +331,11 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" VALUE_PARAMETER_LIST -> handleFirstValueParameter(node) } - node - .children() - .filter { - it.elementType == COMMA && - !it.treeNext.run { elementType == WHITE_SPACE && textContains('\n') } - } - .toList() - .onlyIf({ isNotEmpty() }) { invalidCommas -> - WRONG_NEWLINES.warnAndFix(configRules, emitWarn, isFixMode, - "$entryType should be placed on different lines in declaration of <${node.getParentIdentifier()}>", node.startOffset, node) { - invalidCommas.forEach { comma -> - val nextWhiteSpace = comma.treeNext.takeIf { it.elementType == WHITE_SPACE } - comma.appendNewlineMergingWhiteSpace(nextWhiteSpace, nextWhiteSpace?.treeNext ?: comma.treeNext) - } - } - } + handleValueParameterList(node, entryType) } } - private fun handleFirstValueParameter(node: ASTNode) { - node + private fun handleFirstValueParameter(node: ASTNode) = node .children() .takeWhile { !it.textContains('\n') } .filter { it.elementType == VALUE_PARAMETER } @@ -362,7 +346,23 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" node.appendNewlineMergingWhiteSpace(it.first().treePrev.takeIf { it.elementType == WHITE_SPACE }, it.first()) } } - } + + private fun handleValueParameterList(node: ASTNode, entryType: String) = node + .children() + .filter { + it.elementType == COMMA && + !it.treeNext.run { elementType == WHITE_SPACE && textContains('\n') } + } + .toList() + .onlyIf({ isNotEmpty() }) { invalidCommas -> + WRONG_NEWLINES.warnAndFix(configRules, emitWarn, isFixMode, + "$entryType should be placed on different lines in declaration of <${node.getParentIdentifier()}>", node.startOffset, node) { + invalidCommas.forEach { comma -> + val nextWhiteSpace = comma.treeNext.takeIf { it.elementType == WHITE_SPACE } + comma.appendNewlineMergingWhiteSpace(nextWhiteSpace, nextWhiteSpace?.treeNext ?: comma.treeNext) + } + } + } @Suppress("UnsafeCallOnNullableType") private fun ASTNode.getParentIdentifier() = when (treeParent.elementType) {