From 0af6e32e26637ea3c242967bdf2ea46c2929b3f1 Mon Sep 17 00:00:00 2001 From: Denis Kumar Date: Mon, 7 Dec 2020 16:45:00 +0300 Subject: [PATCH] function style bug in NewlinesRule (#580) * function style bug in NewlinesRule ### What's done: Fixed bugs and added tests --- .../cqfn/diktat/ruleset/constants/Warnings.kt | 2 + .../cqfn/diktat/ruleset/rules/LineLength.kt | 1 - .../rules/LongNumericalValuesSeparatedRule.kt | 1 - .../ruleset/rules/NestedFunctionBlock.kt | 1 - .../ruleset/rules/files/NewlinesRule.kt | 87 +++-- .../cqfn/diktat/ruleset/utils/AstNodeUtils.kt | 6 +- .../ruleset/utils/FunctionAstNodeUtils.kt | 1 - .../cqfn/diktat/ruleset/utils/StringUtils.kt | 1 - .../ruleset/utils/indentation/Checkers.kt | 2 +- .../ruleset/utils/search/VariablesSearch.kt | 1 - .../search/VariablesWithAssignmentSearch.kt | 1 - .../utils/search/VariablesWithUsagesSearch.kt | 1 - .../chapter3/files/NewlinesRuleFixTest.kt | 9 +- .../chapter3/files/NewlinesRuleWarnTest.kt | 297 ++++++++++++++++-- .../newlines/FunctionalStyleExpected.kt | 17 + .../newlines/FunctionalStyleTest.kt | 13 + .../smoke/src/main/kotlin/Example1Expected.kt | 4 +- 17 files changed, 370 insertions(+), 75 deletions(-) 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 d1ff316194..f1b5c13b65 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 @@ -8,6 +8,8 @@ import org.jetbrains.kotlin.com.intellij.lang.ASTNode typealias EmitType = ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) +typealias ListOfList = MutableList> + /** * This class represent individual inspections of diktat code style. * A [Warnings] entry contains rule name, warning message and is used in code check. diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LineLength.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LineLength.kt index f0851d9cbc..d3deb5d092 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LineLength.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LineLength.kt @@ -311,7 +311,6 @@ class LineLength(private val configRules: List) : Rule("line-length *@param node node in which to search *@param binList mutable list of ASTNode to store nodes */ - @Suppress("WRONG_NEWLINES") private fun searchBinaryExpression(node: ASTNode, binList: MutableList) { if (node.hasChildOfType(BINARY_EXPRESSION) || node.hasChildOfType(PARENTHESIZED) || node.hasChildOfType(POSTFIX_EXPRESSION)) { diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LongNumericalValuesSeparatedRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LongNumericalValuesSeparatedRule.kt index 16bdaaa54a..07bde3447c 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LongNumericalValuesSeparatedRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LongNumericalValuesSeparatedRule.kt @@ -136,7 +136,6 @@ class LongNumericalValuesSeparatedRule(private val configRules: List) : Rule("ne } } - @Suppress("WRONG_NEWLINES") private fun countNestedBlocks(node: ASTNode, maxNestedBlockCount: Long) { node.findAllNodesWithSpecificType(LBRACE) .reversed() 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 a52f8f5d16..fb6145bb1a 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 @@ -4,6 +4,7 @@ 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.EmitType +import org.cqfn.diktat.ruleset.constants.ListOfList 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 @@ -57,6 +58,7 @@ import com.pinterest.ktlint.core.ast.ElementType.OROR 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.POSTFIX_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.PRIMARY_CONSTRUCTOR import com.pinterest.ktlint.core.ast.ElementType.RETURN import com.pinterest.ktlint.core.ast.ElementType.RETURN_KEYWORD @@ -69,6 +71,7 @@ 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.isWhiteSpaceWithNewline import com.pinterest.ktlint.core.ast.nextCodeSibling import com.pinterest.ktlint.core.ast.parent import com.pinterest.ktlint.core.ast.prevCodeSibling @@ -76,6 +79,7 @@ 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.psi.KtParameterList import org.jetbrains.kotlin.psi.KtSuperTypeList @@ -161,7 +165,7 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" if (node.isDotFromPackageOrImport()) { return } - val isIncorrect = node.run { + val isIncorrect = (if (node.elementType == ELVIS) node.treeParent else node).run { if (isCallsChain()) { val isSingleLineIfElse = parent({ it.elementType == IF }, true)?.isSingleLineIfElse() ?: false // to follow functional style these operators should be started by newline @@ -327,7 +331,6 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" } } - @Suppress("WRONG_NEWLINES") private fun handleFirstValueParameter(node: ASTNode) = node .children() .takeWhile { !it.textContains('\n') } @@ -375,24 +378,29 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" private fun ASTNode.getOrderedCallExpressions(psi: PsiElement, result: MutableList) { // if statements here have the only right order - don't change it - if (psi.children.isNotEmpty() && (psi.children[0].node.elementType != DOT_QUALIFIED_EXPRESSION && - psi.children[0].node.elementType != SAFE_ACCESS_EXPRESSION)) { - result.add( - psi.children[0] - .node - .siblings(true) - .dropWhile { it.elementType in dropChainValues } - .first() // node treeNext is ".", "?.", "!!", "::" + if (psi.children.isNotEmpty() && (!psi.isFirstChildElementType(DOT_QUALIFIED_EXPRESSION) && + !psi.isFirstChildElementType(SAFE_ACCESS_EXPRESSION))) { + val firstChild = psi.firstChild + if (firstChild.isFirstChildElementType(POSTFIX_EXPRESSION)) { + if (firstChild.isFirstChildElementType(DOT_QUALIFIED_EXPRESSION) || + firstChild.isFirstChildElementType(SAFE_ACCESS_EXPRESSION)) { + getOrderedCallExpressions(firstChild.firstChild, result) + } + result.add(firstChild.node) + } + result.add(firstChild.node + .siblings(true) + .dropWhile { it.elementType in dropChainValues } + .first() // node treeNext is ".", "?.", "!!", "::" ) } else if (psi.children.isNotEmpty()) { - getOrderedCallExpressions(psi.children[0], result) - - result.add( - psi.children[0] - .node - .siblings(true) - .dropWhile { it.elementType in dropChainValues } - .first() // node treeNext is ".", "?.", "!!", "::" + getOrderedCallExpressions(psi.firstChild, result) + + result.add(psi.firstChild + .node + .siblings(true) + .dropWhile { it.elementType in dropChainValues } + .first() // node treeNext is ".", "?.", "!!", "::" ) } } @@ -413,6 +421,14 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" private fun ASTNode.isDotFromPackageOrImport() = elementType == DOT && parent({ it.elementType == IMPORT_DIRECTIVE || it.elementType == PACKAGE_DIRECTIVE }, true) != null + private fun PsiElement.isFirstChildElementType(elementType: IElementType) = + this.firstChild.node.elementType == elementType + + /** + * This method collects chain calls and checks it + * + * @return true - if there is error, false, if there is no error and null if there is two calls in chain + */ private fun ASTNode.isCallsChain() = getParentExpressions() .lastOrNull() ?.run { @@ -420,12 +436,31 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" getOrderedCallExpressions(psi, it) } } - ?.dropWhile { !it.treeParent.textContains('(') && !it.treeParent.textContains('{') } // fixme: we can't distinguish fully qualified names (like java.lang) from chain of property accesses (like list.size) for now - ?.filter { it.getParentExpressions().count() > 1 } - ?.count() - ?.let { it > 1 } - ?: false + ?.dropWhile { !it.treeParent.textContains('(') && !it.treeParent.textContains('{') } + ?.isNotValidCalls(this) ?: false + + private fun List.isNotValidCalls(node: ASTNode): Boolean { + if (this.size == 1) { + return false + } + val callsByNewLine: ListOfList = mutableListOf() + var callsInOneNewLine: MutableList = mutableListOf() + this.forEach { + if (it.treePrev.isFollowedByNewline() || it.treePrev.isWhiteSpaceWithNewline()) { + callsByNewLine.add(callsInOneNewLine) + callsInOneNewLine = mutableListOf() + callsInOneNewLine.add(it) + } else { + callsInOneNewLine.add(it) + } + if (it.treePrev.elementType == POSTFIX_EXPRESSION && !it.treePrev.isFollowedByNewline() && configuration.maxCallsInOneLine == 1) { + return true + } + } + callsByNewLine.add(callsInOneNewLine) + return (callsByNewLine.find { it.contains(node) } ?: return false).indexOf(node) + 1 > configuration.maxCallsInOneLine + } /** * taking all expressions inside complex expression until we reach lambda arguments @@ -433,7 +468,6 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" private fun ASTNode.getParentExpressions() = parents().takeWhile { it.elementType in chainExpressionTypes && it.elementType != LAMBDA_ARGUMENT } - @Suppress("WRONG_NEWLINES") private fun isMultilineLambda(node: ASTNode): Boolean = node.findAllNodesWithSpecificType(LAMBDA_ARGUMENT) .firstOrNull() @@ -467,14 +501,17 @@ class NewlinesRule(private val configRules: List) : Rule("newlines" * 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 + val maxCallsInOneLine = config["maxCallsInOneLine"]?.toInt() ?: MAX_CALLS_IN_ONE_LINE } companion object { + const val MAX_CALLS_IN_ONE_LINE = 3 + // fixme: these token sets can be not full, need to add new once as corresponding cases are discovered. // error is raised if these operators are prepended by newline private val lineBreakAfterOperators = TokenSet.create(ANDAND, OROR, PLUS, PLUSEQ, MINUS, MINUSEQ, MUL, MULTEQ, DIV, DIVEQ) - // error is raised if these operators are followed by newline + private val lineBreakBeforeOperators = TokenSet.create(DOT, SAFE_ACCESS, ELVIS, COLONCOLON) private val expressionTypes = TokenSet.create(DOT_QUALIFIED_EXPRESSION, SAFE_ACCESS_EXPRESSION, CALLABLE_REFERENCE_EXPRESSION, BINARY_EXPRESSION) private val chainExpressionTypes = TokenSet.create(DOT_QUALIFIED_EXPRESSION, SAFE_ACCESS_EXPRESSION) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt index bba9fbb22a..3f7fe4a1c8 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt @@ -434,7 +434,6 @@ fun ASTNode?.isAccessibleOutside(): Boolean = * @param warningName a name of the warning which is checked * @return boolean result */ -@Suppress("WRONG_NEWLINES") fun ASTNode.hasSuppress(warningName: String) = parent({ node -> val annotationNode = if (node.elementType != FILE) { node.findChildByType(MODIFIER_LIST) @@ -693,10 +692,7 @@ fun ASTNode.lastLineNumber(isFixMode: Boolean) = getLineNumber(isFixMode)?.plus( /** * copy-pasted method from ktlint to determine line and column number by offset */ -@Suppress("WRONG_NEWLINES") // to keep this function with explicit return type -fun ASTNode.calculateLineColByOffset(): (offset: Int) -> Pair { - return buildPositionInTextLocator(text) -} +fun ASTNode.calculateLineColByOffset() = buildPositionInTextLocator(text) /** * Retrieves file name from user data of this node diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/FunctionAstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/FunctionAstNodeUtils.kt index 0828f368d4..b5f2f9904d 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/FunctionAstNodeUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/FunctionAstNodeUtils.kt @@ -33,7 +33,6 @@ fun ASTNode.parameterNames(): Collection { * * @return function body text as a list of strings */ -@Suppress("WRONG_NEWLINES") fun ASTNode.getBodyLines(): List { checkNodeIsFun(this) return this.getFirstChildWithType(BLOCK)?.let { blockNode -> diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/StringUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/StringUtils.kt index b1c4c556e3..06ac3590e1 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/StringUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/StringUtils.kt @@ -69,7 +69,6 @@ fun String.countSubStringOccurrences(sub: String) = this.split(sub).size - 1 * * @return list of path parts */ -@Suppress("WRONG_NEWLINES") fun String.splitPathToDirs(): List = this.replace("\\", "/") .replace("//", "/") diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt index 9808668585..62b6dbbff4 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt @@ -89,7 +89,7 @@ internal class ValueParameterListChecker(configuration: IndentationConfig) : Cus } @ExperimentalStdlibApi // to use `scan` on sequence - @Suppress("WRONG_NEWLINES", "ANNOTATION_NEW_LINE") // https://github.com/cqfn/diKTat/issues/609 + @Suppress("ANNOTATION_NEW_LINE") // https://github.com/cqfn/diKTat/issues/609 override fun checkNode(whiteSpace: PsiWhiteSpace, indentError: IndentationError): CheckResult? { if (isCheckNeeded(whiteSpace)) { val parameterList = whiteSpace.parent.node diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt index ffa605bc20..e873f1dd83 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt @@ -81,7 +81,6 @@ abstract class VariablesSearch(val node: ASTNode, private val filterForVariables * all these scopes are on lower level of inheritance that's why if in one of these scopes we will find any * variable declaration with the same name - we will understand that it is usage of another variable */ - @Suppress("WRONG_NEWLINES") protected fun isReferenceToOtherVariableWithSameName(expression: KtNameReferenceExpression, codeBlock: KtElement, property: KtProperty) = expression.parents diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt index 129bfe3163..bc9ec08ce3 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt @@ -21,7 +21,6 @@ class VariablesWithAssignmentSearch(fileNode: ASTNode, * @param property * @return */ - @Suppress("WRONG_NEWLINES") override fun KtElement.getAllSearchResults(property: KtProperty) = this.node .findAllNodesWithSpecificType(ElementType.BINARY_EXPRESSION) // filtering out all usages that are declared in the same context but are going before the variable declaration diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt index 7bfe5fde4a..c18d7e115f 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt @@ -14,7 +14,6 @@ import org.jetbrains.kotlin.psi.KtProperty class VariablesWithUsagesSearch(fileNode: ASTNode, filterForVariables: (KtProperty) -> Boolean) : VariablesSearch(fileNode, filterForVariables) { - @Suppress("WRONG_NEWLINES") override fun KtElement.getAllSearchResults(property: KtProperty) = this.node .findAllNodesWithSpecificType(ElementType.REFERENCE_EXPRESSION) // filtering out all usages that are declared in the same context but are going before the variable declaration 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 e64df30645..7effe8c38b 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 @@ -1,5 +1,7 @@ package org.cqfn.diktat.ruleset.chapter3.files +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.ruleset.constants.Warnings import org.cqfn.diktat.ruleset.rules.files.NewlinesRule import org.cqfn.diktat.util.FixTestBase @@ -8,6 +10,11 @@ import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Test class NewlinesRuleFixTest : FixTestBase("test/paragraph3/newlines", ::NewlinesRule) { + private val rulesConfigListShort: List = listOf( + RulesConfig(Warnings.WRONG_NEWLINES.name, true, + mapOf("maxCallsInOneLine" to "1")) + ) + @Test @Tag(WarningNames.REDUNDANT_SEMICOLON) fun `should remove redundant semicolons`() { @@ -23,7 +30,7 @@ class NewlinesRuleFixTest : FixTestBase("test/paragraph3/newlines", ::NewlinesRu @Test @Tag(WarningNames.WRONG_NEWLINES) fun `should fix newlines to follow functional style`() { - fixAndCompare("FunctionalStyleExpected.kt", "FunctionalStyleTest.kt") + fixAndCompare("FunctionalStyleExpected.kt", "FunctionalStyleTest.kt", rulesConfigListShort) } @Test 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 8159ce3de2..5cdafa78e2 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 @@ -1,5 +1,6 @@ package org.cqfn.diktat.ruleset.chapter3.files +import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.ruleset.constants.Warnings.REDUNDANT_SEMICOLON import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_NEWLINES import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID @@ -14,6 +15,14 @@ import org.junit.jupiter.api.Test @Suppress("LargeClass") class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { + private val rulesConfigList: List = listOf( + RulesConfig(WRONG_NEWLINES.name, true, + mapOf("maxCallsInOneLine" to "3")) + ) + private val rulesConfigListShort: List = listOf( + RulesConfig(WRONG_NEWLINES.name, true, + mapOf("maxCallsInOneLine" to "1")) + ) private val ruleId = "$DIKTAT_RULE_SET_ID:newlines" private val shouldBreakAfter = "${WRONG_NEWLINES.warnText()} should break a line after and not before" private val shouldBreakBefore = "${WRONG_NEWLINES.warnText()} should break a line before and not after" @@ -38,7 +47,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | val a = 0; | val b = if (condition) { bar(); baz()} else qux |}; - """.trimMargin(), + """.trimMargin(), LintError(6, 17, ruleId, "${REDUNDANT_SEMICOLON.warnText()} fun foo() {};", true), LintError(7, 14, ruleId, "${REDUNDANT_SEMICOLON.warnText()} val a = 0;", true), LintError(9, 2, ruleId, "${REDUNDANT_SEMICOLON.warnText()} };", true) @@ -54,7 +63,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | |import org.cqfn.diktat.Foo |import org.cqfn.diktat.example.* - """.trimMargin() + """.trimMargin() ) } @@ -72,7 +81,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | bar().let(::baz) | ::baz |} - """.trimMargin() + """.trimMargin() ) } @@ -91,6 +100,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | y | | obj!! + | | obj | .foo() | obj @@ -100,7 +110,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | obj | ::foo |} - """.trimMargin() + """.trimMargin() ) } @@ -125,7 +135,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | obj:: | foo |} - """.trimMargin(), + """.trimMargin(), LintError(3, 9, ruleId, "$shouldBreakAfter &&", true), LintError(8, 8, ruleId, "$shouldBreakBefore .", true), LintError(10, 8, ruleId, "$shouldBreakBefore ?.", true), @@ -151,7 +161,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | | true xor false or true |} - """.trimMargin() + """.trimMargin() ) } @@ -168,7 +178,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | xor | false) |} - """.trimMargin(), + """.trimMargin(), LintError(3, 9, ruleId, "$shouldBreakAfter xor", true), LintError(6, 9, ruleId, "$shouldBreakAfter xor", true) ) @@ -196,7 +206,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | true | ) |} - """.trimMargin(), + """.trimMargin(), LintError(3, 9, ruleId, "$shouldBreakAfter or", true), LintError(7, 9, ruleId, "$shouldBreakAfter xor", true), LintError(8, 9, ruleId, "$shouldBreakAfter or", true), @@ -219,7 +229,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | } | ?.qux() |} - """.trimMargin() + """.trimMargin() ) } @@ -233,7 +243,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | bar?.baz() | bar!!.baz() |} - """.trimMargin() + """.trimMargin() ) } @@ -248,10 +258,11 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | it.condition() | }?.qux() |} - """.trimMargin(), + """.trimMargin(), LintError(2, 11, ruleId, "$functionalStyleWarn .", true), LintError(3, 26, ruleId, "$functionalStyleWarn .", true), - LintError(5, 10, ruleId, "$functionalStyleWarn ?.", true) + LintError(5, 10, ruleId, "$functionalStyleWarn ?.", true), + rulesConfigList = rulesConfigListShort ) } @@ -263,7 +274,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { |fun foo(list: List?) { | if (list.size > n) list.filterNotNull().map { it.baz() } else list.let { it.bar() }.firstOrNull()?.qux() |} - """.trimMargin() + """.trimMargin() ) } @@ -278,7 +289,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | val b | = 43 |} - """.trimMargin(), + """.trimMargin(), LintError(5, 9, ruleId, "$shouldBreakAfter =", true) ) } @@ -294,7 +305,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | bar(a | , b) |} - """.trimMargin(), + """.trimMargin(), LintError(2, 9, ruleId, commaWarn, true), LintError(5, 9, ruleId, commaWarn, true) ) @@ -313,7 +324,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | a: Int | ) { } |} - """.trimMargin() + """.trimMargin() ) } @@ -328,7 +339,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | val (a, b) = f(0) |} |fun bar(f: (x: Int) -> Unit) { } - """.trimMargin() + """.trimMargin() ) } @@ -347,7 +358,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | a: Int | ) { } |} - """.trimMargin(), + """.trimMargin(), LintError(1, 16, ruleId, lparWarn, true), LintError(3, 5, ruleId, lparWarn, true), LintError(7, 5, ruleId, lparWarn, true) @@ -363,7 +374,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | b: Int) { | bar(a, b) |} - """.trimMargin() + """.trimMargin() ) } @@ -386,7 +397,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | val e = list.map { elem: Type -> bar(elem) } | val f = list.map { bar(elem) } |} - """.trimMargin() + """.trimMargin() ) } @@ -414,7 +425,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | foo(elem) | } |} - """.trimMargin(), + """.trimMargin(), LintError(3, 14, ruleId, lambdaWithArrowWarn, true), LintError(7, 9, ruleId, lambdaWithArrowWarn, true), LintError(11, 9, ruleId, lambdaWithArrowWarn, true), @@ -447,7 +458,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { |fun quux() { return } | |fun quux2(): Unit { return } - """.trimMargin() + """.trimMargin() ) } @@ -467,7 +478,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | b: Int | ) { } |} - """.trimMargin() + """.trimMargin() ) } @@ -486,7 +497,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | a: Int, b: Int | ) { } |} - """.trimMargin(), + """.trimMargin(), LintError(3, 14, ruleId, "${WRONG_NEWLINES.warnText()} argument list should be split into several lines", true), LintError(7, 12, ruleId, "${WRONG_NEWLINES.warnText()} argument list should be split into several lines", true) ) @@ -500,7 +511,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { |fun foo(): String { | return "lorem ipsum" |} - """.trimMargin(), + """.trimMargin(), LintError(2, 5, ruleId, singleReturnWarn, true) ) } @@ -544,13 +555,13 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | } | } |} - """.trimMargin() + """.trimMargin() ) } @Test @Tag(WarningNames.WRONG_NEWLINES) - fun `should trigger on non-multiline lambdas`() { + fun `should trigger for several lambdas on same line`() { lintMethod( """ |fun foo(): String { @@ -575,11 +586,9 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | bar() | } |} - """.trimMargin(), - LintError(2, 22, ruleId, "${WRONG_NEWLINES.warnText()} should follow functional style at .", true), - LintError(6, 22, ruleId, "${WRONG_NEWLINES.warnText()} should follow functional style at ?.", true), - LintError(10, 13, ruleId, "${WRONG_NEWLINES.warnText()} should follow functional style at .", true), - LintError(19, 20, ruleId, "${WRONG_NEWLINES.warnText()} should follow functional style at .", true) + """.trimMargin(), + LintError(19, 20, ruleId, "${WRONG_NEWLINES.warnText()} should follow functional style at .", true), + rulesConfigList = rulesConfigListShort ) } @@ -653,4 +662,228 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { LintError(9, 13, ruleId, "${WRONG_NEWLINES.warnText()} supertype list entries should be placed on different lines in declaration of ", true) ) } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `should warn dot qualified with first on same line`() { + lintMethod( + """ + |fun foo() { + | x.map() + | .gre().few().dfh().qwe() + |} + | + |fun foo() { + | x.map() + | .gre() + | .few() + |} + | + |fun foo() { + | x.map().gre().few().qwe() + |} + """.trimMargin(), + LintError(3, 22, ruleId, "${WRONG_NEWLINES.warnText()} should follow functional style at .", true), + LintError(13, 23, ruleId, "${WRONG_NEWLINES.warnText()} should follow functional style at .", true) + ) + } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `should warn dot qualified with first on diff line`() { + lintMethod( + """ + |fun foo() { + | x + | .map() + | .gre().few().qwe().qwe() + |} + | + |fun foo() { + | x + | .map().gre().few().vfd() + |} + | + |fun foo() { + | x + | .map() + | .gre() + | .few() + |} + """.trimMargin(), + LintError(4, 22, ruleId, "$functionalStyleWarn .", true), + LintError(9, 22, ruleId, "$functionalStyleWarn .", true) + ) + } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `should warn dot qualified with safe access`() { + lintMethod( + """ + |fun foo() { + | x + | ?.map() + | .gre().few() + |} + | + |fun foo() { + | x + | ?.map().gre().few() + |} + |fun foo() { + | x + | ?.map() + | .gre() + | .few() + |} + | + |fun foo() { + | x?.map() + | .gre() + | .few() + |} + """.trimMargin(), + LintError(4, 10, ruleId, "$functionalStyleWarn .", true), + LintError(9, 11, ruleId, "$functionalStyleWarn .", true), + LintError(9, 17, ruleId, "$functionalStyleWarn .", true), + rulesConfigList = rulesConfigListShort + ) + } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `should warn dot qualified with exclexcl`() { + lintMethod( + """ + |fun foo() { + | x!!.map() + | .gre() + | .few() + |} + | + |fun foo() { + | x!! + | .map() + | .gre() + | .few() + |} + | + |fun foo() { + | x!! + | .map().gre() + | .few() + |} + | + |fun foo() { + | x!!.map() + | .gre().few() + |} + """.trimMargin(), + LintError(2, 7, ruleId, "$functionalStyleWarn .", true), + LintError(16, 10, ruleId, "$functionalStyleWarn .", true), + LintError(21, 7, ruleId, "$functionalStyleWarn .", true), + LintError(22, 10, ruleId, "$functionalStyleWarn .", true), + rulesConfigList = rulesConfigListShort + ) + } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `should warn elvis`() { + lintMethod( + """ + |fun foo() { + | + | z.goo() + | ?: + | goo() + | + | x.goo() + | ?:goo() + | + | y.ds()?:gh() + |} + """.trimMargin(), + LintError(4, 8, ruleId, "$shouldBreakBefore ?:", true) + ) + } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `should warn elvis with several dot qualifided`() { + lintMethod( + """ + |fun foo() { + | z.goo() + | ?: + | goo().gor().goo() + |} + """.trimMargin(), + LintError(3, 8, ruleId, "$shouldBreakBefore ?:", true) + ) + } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `test configuration for calls in one line`() { + lintMethod( + """ + |fun foo() { + | z.goo().foo().qwe() + | z!!.htr().foo() + | x.goo().foo().goo() + | x.gf().gfh() ?: true + | x.gf().fge().qwe().fd() + |} + """.trimMargin(), + LintError(6, 22, ruleId, "$functionalStyleWarn .", true), rulesConfigList = rulesConfigList + ) + } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `more test for prefix`() { + lintMethod( + """ + |fun foo() { + | foo + | .bar() + | .goo() + | .qwe()!! + | + | goo()!!.gre() + | + | bfr()!!.qwe().foo().qwe().dg() + |} + | + |fun foo() { + | foo + | .bar() + | .goo()!! + | .qwe() + |} + """.trimMargin(), + LintError(9, 29, ruleId, "$functionalStyleWarn .", true) + ) + } + + @Test + @Tag(WarningNames.WRONG_NEWLINES) + fun `more test for unsafe calls`() { + lintMethod( + """ + |fun foo() { + | foo + | .bar() + | .goo()!! + | .qwe() + | + | val x = foo + | .bar!! + | .baz + |} + """.trimMargin() + ) + } } diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/FunctionalStyleExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/FunctionalStyleExpected.kt index e7eccb61dc..9b3dfdd9e8 100644 --- a/diktat-rules/src/test/resources/test/paragraph3/newlines/FunctionalStyleExpected.kt +++ b/diktat-rules/src/test/resources/test/paragraph3/newlines/FunctionalStyleExpected.kt @@ -12,3 +12,20 @@ fun foo(list: List?) { } fun bar(x :Int,y:Int) = x+ y + +fun goo() { + x.map() +.gro() + .gh() + t.map() +.hg() +.hg() + t + .map() + .filter() + .takefirst() + x + .map() + .filter() +.hre() +} diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/FunctionalStyleTest.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/FunctionalStyleTest.kt index 4315b7dadc..7232c6e203 100644 --- a/diktat-rules/src/test/resources/test/paragraph3/newlines/FunctionalStyleTest.kt +++ b/diktat-rules/src/test/resources/test/paragraph3/newlines/FunctionalStyleTest.kt @@ -10,3 +10,16 @@ fun foo(list: List?) { fun bar(x :Int,y:Int) :Int { return x+ y } + +fun goo() { + x.map().gro() + .gh() + t.map().hg().hg() + t + .map() + .filter() + .takefirst() + x + .map() + .filter().hre() +} \ No newline at end of file diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example1Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example1Expected.kt index 536bcd6cc7..c33e8b0b16 100644 --- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example1Expected.kt +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example1Expected.kt @@ -27,9 +27,7 @@ class Example { * @return */ fun String.splitPathToDirs(): List = - this - .replace("\\", "/") - .replace("//", "/") + this.replace("\\", "/").replace("//", "/") .split("/") /**