Skip to content

Commit

Permalink
WRONG_INDENTATION: fix the regression introduced with 998d0e9 (#1503)
Browse files Browse the repository at this point in the history 
### What's done:

 * Fixes the false-positive warnings reported for single-line string templates.
 * Closes #1490.
 * See #811 (the original feature request).
 * See #1364 (the pull request which introduced the regression).
 * See 998d0e9 (the commit which introduced the regression).
  • Loading branch information
@0x6675636b796f75676974687562
0x6675636b796f75676974687562 authored Aug 15, 2022
1 parent ce65178 commit b8bd8a2
Show file tree
Hide file tree
Showing 7 changed files with 351 additions and 149 deletions.
121 changes: 94 additions & 27 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 @@ -2,6 +2,8 @@
* Main logic of indentation including Rule and utility classes and methods.
*/

@file:Suppress("FILE_UNORDERED_IMPORTS")// False positives, see #1494.

package org.cqfn.diktat.ruleset.rules.chapter3.files

import org.cqfn.diktat.common.config.rules.RulesConfig
Expand Down Expand Up @@ -79,11 +81,13 @@ import org.jetbrains.kotlin.psi.psiUtil.parents
import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf
import org.jetbrains.kotlin.psi.psiUtil.startOffset

import java.util.ArrayDeque as Stack

import kotlin.contracts.ExperimentalContracts
import kotlin.contracts.contract
import kotlin.math.abs
import kotlin.reflect.KCallable

import java.util.ArrayDeque as Stack

/**
* Rule that checks indentation. The following general rules are checked:
* 1. Only spaces should be used each indentation is equal to 4 spaces
Expand Down Expand Up @@ -198,21 +202,6 @@ 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 IndentContext.visitWhiteSpace(astNode: ASTNode) {
require(astNode.isMultilineWhitespace()) {
Expand Down Expand Up @@ -242,10 +231,10 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
addException(astNode.treeParent, abs(indentError.expected - indentError.actual), false)
}

val difOffsetCloseAndOpenQuote = isCloseAndOpenQuoterOffset(astNode, indentError.actual)
val alignedOpeningAndClosingQuotes = hasAlignedOpeningAndClosingQuotes(astNode, indentError.actual)

if ((checkResult?.isCorrect != true && expectedIndent != indentError.actual) || !difOffsetCloseAndOpenQuote) {
val warnText = if (!difOffsetCloseAndOpenQuote) {
if ((checkResult?.isCorrect != true && expectedIndent != indentError.actual) || !alignedOpeningAndClosingQuotes) {
val warnText = if (!alignedOpeningAndClosingQuotes) {
"the same number of indents to the opening and closing quotes was expected"
} else {
"expected $expectedIndent but was ${indentError.actual}"
Expand All @@ -268,7 +257,7 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
expectedIndent: Int,
actualIndent: Int
) {
val nextNodeDot = getNextDotExpression(whiteSpace.node.treeNext)
val nextNodeDot = whiteSpace.node.treeNext.getNextDotExpression()
if (nextNodeDot != null &&
nextNodeDot.elementType == DOT_QUALIFIED_EXPRESSION &&
nextNodeDot.firstChildNode.elementType == STRING_TEMPLATE &&
Expand Down Expand Up @@ -361,12 +350,6 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
}
}

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

/**
* Modifies [templateEntry] by correcting its indentation level.
*
Expand Down Expand Up @@ -734,11 +717,30 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
private fun ASTNode.isMultilineWhitespace(): Boolean =
elementType == WHITE_SPACE && textContains(NEWLINE)

@OptIn(ExperimentalContracts::class)
private fun ASTNode?.isMultilineStringTemplate(): Boolean {
contract {
returns(true) implies (this@isMultilineStringTemplate != null)
}

this ?: return false

return elementType == STRING_TEMPLATE &&
getAllChildrenWithType(LITERAL_STRING_TEMPLATE_ENTRY).any { entry ->
entry.textContains(NEWLINE)
}
}

/**
* @return `true` if this is a [String.trimIndent] or [String.trimMargin]
* call, `false` otherwise.
*/
@OptIn(ExperimentalContracts::class)
private fun ASTNode?.isTrimIndentOrMarginCall(): Boolean {
contract {
returns(true) implies (this@isTrimIndentOrMarginCall != null)
}

this ?: return false

require(elementType == CALL_EXPRESSION) {
Expand All @@ -758,6 +760,12 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
return identifier.text in knownTrimFunctionPatterns
}

private fun ASTNode.getNextDotExpression(): ASTNode? =
when (elementType) {
DOT_QUALIFIED_EXPRESSION -> this
else -> getFirstChildWithType(DOT_QUALIFIED_EXPRESSION)
}

/**
* @return the matching closing brace type for this opening brace type,
* or vice versa.
Expand Down Expand Up @@ -791,5 +799,64 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
this > 0 -> this
else -> 0
}

/**
* Processes fragments like:
*
* ```kotlin
* f(
* """
* |foobar
* """.trimMargin()
* )
* ```
*
* @param whitespace the whitespace node between an [LPAR] and the
* `trimIndent()`- or `trimMargin()`- terminated string template, which is
* an effective argument of a function call. The string template is
* expected to begin on a separate line (otherwise, there'll be no
* whitespace in-between).
* @return `true` if the opening and the closing quotes of the string
* template are aligned, `false` otherwise.
*/
private fun hasAlignedOpeningAndClosingQuotes(whitespace: ASTNode, expectedIndent: Int): Boolean {
require(whitespace.isMultilineWhitespace()) {
"The node is $whitespace while a multi-line $WHITE_SPACE expected"
}

/*
* Here, we expect that `nextNode` is a VALUE_ARGUMENT which contains
* the dot-qualified expression (`STRING_TEMPLATE.trimIndent()` or
* `STRING_TEMPLATE.trimMargin()`).
*/
val nextFunctionArgument = whitespace.treeNext
if (nextFunctionArgument.elementType == VALUE_ARGUMENT) {
val memberOrExtensionCall = nextFunctionArgument.getNextDotExpression()

/*
* Limit allowed member or extension calls to `trimIndent()` and
* `trimMargin()`.
*/
if (memberOrExtensionCall != null &&
memberOrExtensionCall.getFirstChildWithType(CALL_EXPRESSION).isTrimIndentOrMarginCall()) {
val stringTemplate = memberOrExtensionCall.getFirstChildWithType(STRING_TEMPLATE)

/*
* Limit the logic to multi-line string templates only (the
* opening and closing quotes of a single-line template are,
* obviously, always mis-aligned).
*/
if (stringTemplate != null && stringTemplate.isMultilineStringTemplate()) {
val closingQuoteIndent = stringTemplate.getFirstChildWithType(CLOSING_QUOTE)
?.treePrev
?.text
?.length ?: -1
return expectedIndent == closingQuoteIndent
}
}
}

return true
}
}
}
176 changes: 172 additions & 4 deletions ...t-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleFixTest.kt
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@file:Suppress("FILE_UNORDERED_IMPORTS")// False positives, see #1494.

package org.cqfn.diktat.ruleset.chapter3.spaces

import org.cqfn.diktat.common.config.rules.RulesConfig
Expand All @@ -10,12 +12,20 @@ import org.cqfn.diktat.ruleset.utils.indentation.IndentationConfig.Companion.EXT
import org.cqfn.diktat.ruleset.utils.indentation.IndentationConfig.Companion.EXTENDED_INDENT_FOR_EXPRESSION_BODIES
import org.cqfn.diktat.ruleset.utils.indentation.IndentationConfig.Companion.EXTENDED_INDENT_OF_PARAMETERS
import org.cqfn.diktat.ruleset.utils.indentation.IndentationConfig.Companion.NEWLINE_AT_END
import org.cqfn.diktat.test.framework.processing.FileComparisonResult
import org.cqfn.diktat.util.FixTestBase

import generated.WarningNames
import org.assertj.core.api.Assertions.assertThat
import org.intellij.lang.annotations.Language
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestMethodOrder
import org.junit.jupiter.api.io.TempDir
import java.nio.file.Path

import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationConfigFactory as IndentationConfig

/**
* Legacy indentation tests.
Expand Down Expand Up @@ -64,9 +74,167 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
fixAndCompare("ConstructorExpected.kt", "ConstructorTest.kt")
}

@Test
@Tag(WarningNames.WRONG_INDENTATION)
fun `multiline string`() {
fixAndCompare("MultilionStringExpected.kt", "MultilionStringTest.kt")
@Nested
@TestMethodOrder(NaturalDisplayName::class)
inner class `Multi-line string literals` {
/**
* Correctly-indented opening quotation mark, incorrectly-indented
* closing quotation mark.
*/
@Test
@Tag(WarningNames.WRONG_INDENTATION)
@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION") // False positives
fun `case 1 - mis-aligned opening and closing quotes`(@TempDir tempDir: Path) {
val actualCode = """
|fun f() {
| g(
| ""${'"'}
| |val q = 1
| |
| ""${'"'}.trimMargin(),
| arg1 = "arg1"
| )
|}
""".trimMargin()

val expectedCode = """
|fun f() {
| g(
| ""${'"'}
| |val q = 1
| |
| ""${'"'}.trimMargin(),
| arg1 = "arg1"
| )
|}
""".trimMargin()

val lintResult = fixAndCompareContent(actualCode, expectedCode, tempDir)
assertThat(lintResult.actualContent)
.describedAs("lint result for ${actualCode.describe()}")
.isEqualTo(lintResult.expectedContent)
}

/**
* Both the opening and the closing quotation marks are incorrectly
* indented (indentation level is less than needed).
*/
@Test
@Tag(WarningNames.WRONG_INDENTATION)
@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION") // False positives
fun `case 2`(@TempDir tempDir: Path) {
val actualCode = """
|fun f() {
| g(
| ""${'"'}
| |val q = 1
| |
| ""${'"'}.trimMargin(),
| arg1 = "arg1"
| )
|}
""".trimMargin()

val expectedCode = """
|fun f() {
| g(
| ""${'"'}
| |val q = 1
| |
| ""${'"'}.trimMargin(),
| arg1 = "arg1"
| )
|}
""".trimMargin()

val lintResult = fixAndCompareContent(actualCode, expectedCode, tempDir)
assertThat(lintResult.actualContent)
.describedAs("lint result for ${actualCode.describe()}")
.isEqualTo(lintResult.expectedContent)
}

/**
* Both the opening and the closing quotation marks are incorrectly
* indented (indentation level is greater than needed).
*/
@Test
@Tag(WarningNames.WRONG_INDENTATION)
@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION") // False positives
fun `case 3`(@TempDir tempDir: Path) {
val actualCode = """
|fun f() {
| g(
| ""${'"'}
| |val q = 1
| |
| ""${'"'}.trimMargin(),
| arg1 = "arg1"
| )
|}
""".trimMargin()

val expectedCode = """
|fun f() {
| g(
| ""${'"'}
| |val q = 1
| |
| ""${'"'}.trimMargin(),
| arg1 = "arg1"
| )
|}
""".trimMargin()

val lintResult = fixAndCompareContent(actualCode, expectedCode, tempDir)
assertThat(lintResult.actualContent)
.describedAs("lint result for ${actualCode.describe()}")
.isEqualTo(lintResult.expectedContent)
}

/**
* Both the opening and the closing quotation marks are incorrectly
* indented and misaligned.
*/
@Test
@Tag(WarningNames.WRONG_INDENTATION)
@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION") // False positives
fun `case 4 - mis-aligned opening and closing quotes`(@TempDir tempDir: Path) {
val actualCode = """
|fun f() {
| g(
| ""${'"'}
| |val q = 1
| |
| ""${'"'}.trimMargin(),
| arg1 = "arg1"
| )
|}
""".trimMargin()

val expectedCode = """
|fun f() {
| g(
| ""${'"'}
| |val q = 1
| |
| ""${'"'}.trimMargin(),
| arg1 = "arg1"
| )
|}
""".trimMargin()

val lintResult = fixAndCompareContent(actualCode, expectedCode, tempDir)
assertThat(lintResult.actualContent)
.describedAs("lint result for ${actualCode.describe()}")
.isEqualTo(lintResult.expectedContent)
}

private fun fixAndCompareContent(@Language("kotlin") actualCode: String,
@Language("kotlin") expectedCode: String,
tempDir: Path
): FileComparisonResult {
val config = IndentationConfig(NEWLINE_AT_END to false).withCustomParameters().asRulesConfigList()
return fixAndCompareContent(actualCode, expectedCode, tempDir, config)
}
}
}
Loading

0 comments on commit b8bd8a2

Please sign in to comment.