diff --git a/diktat-analysis.yml b/diktat-analysis.yml index df272c0d04..b3d835e50c 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -192,7 +192,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 3c511eaa1c..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 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..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 @@ -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 @@ -21,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 @@ -37,46 +40,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 +92,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 +109,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 +129,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 +222,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 +308,69 @@ 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) { + 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" + else -> { + log.warn("Unexpected node element type ${node.elementType}") + return + } + } + if (numEntries > configuration.maxParametersInOneLine) { + when (node.elementType) { + VALUE_PARAMETER_LIST -> handleFirstValueParameter(node) + } + + handleValueParameterList(node, entryType) + } + } + + 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()) + } + } + + 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) { + 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 +448,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 a13b8adf21..eb9b406278 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -191,7 +191,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 03ddabfa5b..827ca90ccd 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -193,7 +193,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..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 @@ -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" @@ -579,4 +580,75 @@ 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) { } + | + |// 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) { } + """.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 14d6ad051f..2e3e6821d8 100644 --- a/info/diktat-kotlin-coding-style-guide-en.md +++ b/info/diktat-kotlin-coding-style-guide-en.md @@ -1013,13 +1013,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, @@ -1030,6 +1029,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