Skip to content

Commit

Permalink
Bugfix/indentation of multiline string (#1364)
Browse files Browse the repository at this point in the history 
### Whats added:
 * corrected the logic of checking MultilionString
 * added fix test in IndentationRuleFixTest.kt - MultilionStringExpected.kt, MultilionStringTest.kt
 * corrected all incorrect multiline string
 * added new wrong message - "the same number of indents to the opening and closing quotes was expected"
  • Loading branch information
@Arrgentum
Arrgentum authored Jun 16, 2022
1 parent 12a39ab commit 998d0e9
Show file tree
Hide file tree
Showing 60 changed files with 1,034 additions and 922 deletions.
4 changes: 2 additions & 2 deletions diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRule.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ abstract class DiktatRule(
logic(node)
} catch (internalError: Throwable) {
log.error(
"""Internal error has occurred in rule [$id]. Please make an issue on this bug at https://github.com/saveourtool/diKTat/.
"""Internal error has occurred in rule [$id]. Please make an issue on this bug at https://github.com/saveourtool/diKTat/.
As a workaround you can disable these inspections in yml config: <$inspections>.
Root cause of the problem is in [${node.getFilePath()}] file.
""".trimIndent(), internalError
""".trimIndent(), internalError
)
// we are very sorry for throwing common Error here, but unfortunately we are not able to throw
// any existing Exception, as they will be caught in ktlint framework and the logging will be confusing:
Expand Down
59 changes: 44 additions & 15 deletions diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.cqfn.diktat.ruleset.utils.indentation.SuperTypeListChecker
import org.cqfn.diktat.ruleset.utils.indentation.ValueParameterListChecker

import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.CLOSING_QUOTE
import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.ELSE
import com.pinterest.ktlint.core.ast.ElementType.FILE
Expand All @@ -32,13 +33,15 @@ import com.pinterest.ktlint.core.ast.ElementType.LONG_STRING_TEMPLATE_ENTRY
import com.pinterest.ktlint.core.ast.ElementType.LONG_TEMPLATE_ENTRY_END
import com.pinterest.ktlint.core.ast.ElementType.LONG_TEMPLATE_ENTRY_START
import com.pinterest.ktlint.core.ast.ElementType.LPAR
import com.pinterest.ktlint.core.ast.ElementType.OPEN_QUOTE
import com.pinterest.ktlint.core.ast.ElementType.RBRACE
import com.pinterest.ktlint.core.ast.ElementType.RBRACKET
import com.pinterest.ktlint.core.ast.ElementType.RPAR
import com.pinterest.ktlint.core.ast.ElementType.SAFE_ACCESS_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.SHORT_STRING_TEMPLATE_ENTRY
import com.pinterest.ktlint.core.ast.ElementType.STRING_TEMPLATE
import com.pinterest.ktlint.core.ast.ElementType.THEN
import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
import com.pinterest.ktlint.core.ast.visit
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
Expand Down Expand Up @@ -78,6 +81,7 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
}
private lateinit var filePath: String
private lateinit var customIndentationCheckers: List<CustomIndentationChecker>
private lateinit var positionByOffset: (Int) -> Pair<Int, Int>

override fun logic(node: ASTNode) {
if (node.elementType == FILE) {
Expand Down Expand Up @@ -173,10 +177,25 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
}
}

private fun isCloseAndOpenQuoterOffset(nodeWhiteSpace: ASTNode, expectedIndent: Int): Boolean {
val nextNode = nodeWhiteSpace.treeNext
if (nextNode.elementType == VALUE_ARGUMENT) {
val nextNodeDot = getNextDotExpression(nextNode)
nextNodeDot?.getFirstChildWithType(STRING_TEMPLATE)?.let {
if (it.getAllChildrenWithType(LITERAL_STRING_TEMPLATE_ENTRY).size > 1) {
val closingQuote = it.getFirstChildWithType(CLOSING_QUOTE)?.treePrev?.text
?.length ?: -1
return expectedIndent == closingQuote
}
}
}
return true
}

@Suppress("ForbiddenComment")
private fun visitWhiteSpace(astNode: ASTNode, context: IndentContext) {
context.maybeIncrement()

positionByOffset = astNode.treeParent.calculateLineColByOffset()
val whiteSpace = astNode.psi as PsiWhiteSpace
if (astNode.treeNext.elementType in decreasingTokens) {
// if newline is followed by closing token, it should already be indented less
Expand All @@ -199,8 +218,15 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
context.addException(astNode.treeParent, abs(indentError.expected - indentError.actual), false)
}

if (checkResult?.isCorrect != true && expectedIndent != indentError.actual) {
WRONG_INDENTATION.warnAndFix(configRules, emitWarn, isFixMode, "expected $expectedIndent but was ${indentError.actual}",
val difOffsetCloseAndOpenQuote = isCloseAndOpenQuoterOffset(astNode, indentError.actual)

if ((checkResult?.isCorrect != true && expectedIndent != indentError.actual) || !difOffsetCloseAndOpenQuote) {
val warnText = if (!difOffsetCloseAndOpenQuote) {
"the same number of indents to the opening and closing quotes was expected"
} else {
"expected $expectedIndent but was ${indentError.actual}"
}
WRONG_INDENTATION.warnAndFix(configRules, emitWarn, isFixMode, warnText,
whiteSpace.startOffset + whiteSpace.text.lastIndexOf('\n') + 1, whiteSpace.node) {
checkStringLiteral(whiteSpace, expectedIndent, indentError.actual)
whiteSpace.node.indentBy(expectedIndent)
Expand All @@ -216,16 +242,16 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
expectedIndent: Int,
actualIndent: Int
) {
val nextNode = whiteSpace.node.treeNext
if (nextNode != null &&
nextNode.elementType == DOT_QUALIFIED_EXPRESSION &&
nextNode.firstChildNode.elementType == STRING_TEMPLATE &&
nextNode.firstChildNode.text.startsWith("\"\"\"") &&
nextNode.findChildByType(CALL_EXPRESSION)?.text?.let {
val nextNodeDot = getNextDotExpression(whiteSpace.node.treeNext)
if (nextNodeDot != null &&
nextNodeDot.elementType == DOT_QUALIFIED_EXPRESSION &&
nextNodeDot.firstChildNode.elementType == STRING_TEMPLATE &&
nextNodeDot.firstChildNode.text.startsWith("\"\"\"") &&
nextNodeDot.findChildByType(CALL_EXPRESSION)?.text?.let {
it == "trimIndent()" ||
it == "trimMargin()"
} == true) {
fixStringLiteral(whiteSpace, expectedIndent, actualIndent)
fixStringLiteral(nextNodeDot.firstChildNode, expectedIndent, actualIndent)
}
}

Expand All @@ -234,15 +260,12 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
*/
@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION")
private fun fixStringLiteral(
whiteSpace: PsiWhiteSpace,
stringTemplate: ASTNode,
expectedIndent: Int,
actualIndent: Int
) {
val textIndent = " ".repeat(expectedIndent + INDENT_SIZE)
val templateEntries = whiteSpace.node
.treeNext
.firstChildNode
.getAllChildrenWithType(LITERAL_STRING_TEMPLATE_ENTRY)
val templateEntries = stringTemplate.getAllChildrenWithType(LITERAL_STRING_TEMPLATE_ENTRY)
templateEntries.forEach { node ->
if (!node.text.contains("\n")) {
fixFirstTemplateEntries(node, textIndent, actualIndent)
Expand All @@ -256,6 +279,12 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
.trim())
}

private fun getNextDotExpression(node: ASTNode) = if (node.elementType == DOT_QUALIFIED_EXPRESSION) {
node
} else {
node.getFirstChildWithType(DOT_QUALIFIED_EXPRESSION)
}

/**
* This method fixes all lines of string template except the last one
* Also it considers $foo insertions in string
Expand Down
52 changes: 19 additions & 33 deletions diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter1/IdentifierNamingWarnTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
const val M_GLOB = ""
val aPrefix = ""
""".trimIndent()

lintMethod(code,
LintError(1, 11, ruleId, "${VARIABLE_HAS_PREFIX.warnText()} M_GLOB", true),
LintError(2, 5, ruleId, "${VARIABLE_HAS_PREFIX.warnText()} aPrefix", true)
Expand All @@ -336,7 +335,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
val I_AM_CONSTANT1 = ""
const val I_AM_CONSTANT2 = ""
""".trimIndent()

lintMethod(code,
LintError(1, 5, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} I_AM_CONSTANT1", true),
LintError(1, 5, ruleId, "${VARIABLE_HAS_PREFIX.warnText()} I_AM_CONSTANT1", true),
Expand All @@ -358,7 +356,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
}
}
""".trimIndent()

lintMethod(code,
LintError(5, 13, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} STRANGECASE", true)
)
Expand All @@ -376,7 +373,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
}
}
""".trimIndent()

lintMethod(code,
LintError(3, 35, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} STRANGECASE", true)
)
Expand All @@ -387,13 +383,13 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
fun `FUNCTION_BOOLEAN_PREFIX - positive example`() {
lintMethod(
"""
fun ASTNode.hasEmptyLineAfter(): Boolean { }
fun hasEmptyLineAfter(): Boolean { }
fun ASTNode.isEmpty(): Boolean { }
fun isEmpty(): Boolean { }
fun ASTNode.hasEmptyLineAfter(): Boolean { }
fun hasEmptyLineAfter(): Boolean { }
fun ASTNode.isEmpty(): Boolean { }
fun isEmpty(): Boolean { }
override fun empty(): Boolean { }
override fun ASTNode.empty(): Boolean { }
""".trimIndent()
""".trimIndent()
)
}

Expand All @@ -402,11 +398,11 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
fun `FUNCTION_BOOLEAN_PREFIX - negative example`() {
lintMethod(
"""
fun ASTNode.emptyLineAfter(): Boolean { }
fun emptyLineAfter(): Boolean { }
fun ASTNode.empty(): Boolean { }
fun empty(): Boolean { }
""".trimIndent(),
fun ASTNode.emptyLineAfter(): Boolean { }
fun emptyLineAfter(): Boolean { }
fun ASTNode.empty(): Boolean { }
fun empty(): Boolean { }
""".trimIndent(),
LintError(1, 13, ruleId, "${FUNCTION_BOOLEAN_PREFIX.warnText()} emptyLineAfter", true),
LintError(2, 5, ruleId, "${FUNCTION_BOOLEAN_PREFIX.warnText()} emptyLineAfter", true),
LintError(3, 13, ruleId, "${FUNCTION_BOOLEAN_PREFIX.warnText()} empty", true),
Expand All @@ -419,12 +415,12 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
fun `all prefixes for boolean methods`() {
lintMethod(
"""
fun hasEmptyLineAfter(): Boolean { }
fun haveEmptyLineAfter(): Boolean { }
fun isEmpty(): Boolean { }
fun hasEmptyLineAfter(): Boolean { }
fun haveEmptyLineAfter(): Boolean { }
fun isEmpty(): Boolean { }
fun shouldBeEmpty(): Boolean { }
fun areEmpty(): Boolean { }
""".trimIndent()
""".trimIndent()
)
}

Expand All @@ -436,7 +432,7 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
fun equalsSome(): Boolean { }
fun fooBar(): Boolean { }
fun equivalentToAnother(): Boolean { }
""".trimIndent(),
""".trimIndent(),
rulesConfigList = rulesConfigBooleanFunctions
)
}
Expand All @@ -447,15 +443,15 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
// valid example, should not cause exceptions
lintMethod(
"""
fun foo(predicate: (Int) -> Boolean) = Unit
""".trimIndent()
fun foo(predicate: (Int) -> Boolean) = Unit
""".trimIndent()
)

// identifier names in function types are still checked if present
lintMethod(
"""
fun foo(predicate: (a: Int) -> Boolean) = Unit
""".trimIndent(),
fun foo(predicate: (a: Int) -> Boolean) = Unit
""".trimIndent(),
LintError(1, 21, ruleId, "${IDENTIFIER_LENGTH.warnText()} a", false)
)
}
Expand All @@ -469,7 +465,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
override fun visit(node: ASTNode) {}
})
""".trimIndent()

lintMethod(code)
}

Expand All @@ -484,7 +479,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
}
}
""".trimIndent()

lintMethod(code)
}

Expand All @@ -501,7 +495,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
}
}
""".trimIndent()

lintMethod(code, LintError(4, 17, ruleId, "${IDENTIFIER_LENGTH.warnText()} e", false))
}

Expand All @@ -516,7 +509,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
}
}
""".trimIndent()

lintMethod(code)
}

Expand All @@ -529,7 +521,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
val `foo variable` = ""
}
""".trimIndent()

lintMethod(code,
LintError(1, 5, ruleId, "${BACKTICKS_PROHIBITED.warnText()} `foo function`"),
LintError(1, 20, ruleId, "${BACKTICKS_PROHIBITED.warnText()} `argument with backstick`"),
Expand All @@ -546,7 +537,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
fun `test function with backstick`() {
}
""".trimIndent()

lintMethod(code)
}

Expand All @@ -561,7 +551,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
}
""".trimIndent()

lintMethod(code, LintError(3, 9, ruleId, "${BACKTICKS_PROHIBITED.warnText()} `should not be used`"))
}

Expand All @@ -572,7 +561,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
"""
class `my class name` {}
""".trimIndent()

lintMethod(code, LintError(1, 7, ruleId, "${BACKTICKS_PROHIBITED.warnText()} `my class name`"))
}

Expand Down Expand Up @@ -614,7 +602,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
}
}
""".trimIndent()

lintMethod(code)
}

Expand All @@ -632,7 +619,6 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
B
}
""".trimIndent()

lintMethod(code,
LintError(2, 9, ruleId, "${CONFUSING_IDENTIFIER_NAMING.warnText()} better name is: obj, dgt", false),
LintError(2, 9, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} D", true),
Expand Down
2 changes: 1 addition & 1 deletion diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter1/MethodNamingWarnTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class MethodNamingWarnTest : LintTestBase(::IdentifierNaming) {
class TestPackageName {
typealias RelatedClasses = List<Pair<String, String>>
}
""".trimIndent()
""".trimIndent()
)
}

Expand Down
8 changes: 4 additions & 4 deletions diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter1/PackageNamingWarnTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,15 @@ class PackageNamingWarnTest : LintTestBase(::PackageNaming) {
lintMethod(
"""
package org.cqfn.diktat.ruleset.chapter1
""".trimIndent(),
""".trimIndent(),
fileName = "~/diktat/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter1/EnumValueCaseTest.kt",
rulesConfigList = rulesConfigList
)

lintMethod(
"""
package org.cqfn.diktat.chapter1
""".trimIndent(),
""".trimIndent(),
LintError(1, 9, ruleId, "${PACKAGE_NAME_INCORRECT_PATH.warnText()} org.cqfn.diktat.ruleset.chapter1", true),
fileName = "~/diktat/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter1/EnumValueCaseTest.kt",
rulesConfigList = rulesConfigList
Expand All @@ -298,15 +298,15 @@ class PackageNamingWarnTest : LintTestBase(::PackageNaming) {
lintMethod(
"""
|package org.cqfn.diktat.test.processing
""".trimMargin(),
""".trimMargin(),
fileName = "/home/testu/project/module/src/test/kotlin/org/cqfn/diktat/test/processing/SpecialPackageNaming.kt",
rulesConfigList = rulesConfigList
)

lintMethod(
"""
|package kotlin.collections
""".trimMargin(),
""".trimMargin(),
fileName = "/home/testu/project/module/src/main/kotlin/kotlin/collections/Collections.kt",
rulesConfigList = listOf(
RulesConfig("DIKTAT_COMMON", true, mapOf("domainName" to "kotlin"))
Expand Down
Loading

0 comments on commit 998d0e9

Please sign in to comment.