Skip to content

Commit

Permalink
Properly indent chained dot-qualified or safe-access expressions (#1442)
Browse files Browse the repository at this point in the history
### What's done:

 * This fixes #1336
  • Loading branch information
0x6675636b796f75676974687562 authored Jul 12, 2022
1 parent 6352879 commit 0bcbd40
Show file tree
Hide file tree
Showing 4 changed files with 256 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -213,18 +213,33 @@ internal class DotCallChecker(config: IndentationConfig) : CustomIndentationChec
} || nextNode.isCommentBeforeDot()) && whiteSpace.parents.none { it.node.elementType == LONG_STRING_TEMPLATE_ENTRY }
}
?.let { node ->
val indentIncrement = (if (configuration.extendedIndentBeforeDot) 2 else 1) * configuration.indentationSize
if (node.isFromStringTemplate()) {
return CheckResult.from(indentError.actual, indentError.expected +
(if (configuration.extendedIndentBeforeDot) 2 else 1) * configuration.indentationSize, true)
indentIncrement, true)
}

// we need to get indent before the first expression in calls chain
return CheckResult.from(indentError.actual, (whiteSpace.run {
/*-
* If the parent indent (the one before a `DOT_QUALIFIED_EXPRESSION`
* or a `SAFE_ACCESS_EXPRESSION`) is `null`, then use 0 as the
* fallback value.
*
* If `indentError.expected` is used as a fallback (pre-1.2.2
* behaviour), this breaks chained dot-qualified or safe-access
* expressions (see #1336), e.g.:
*
* ```kotlin
* val a = first()
* .second()
* .third()
* ```
*/
val parentIndent = whiteSpace.run {
parents.takeWhile { it is KtDotQualifiedExpression || it is KtSafeQualifiedExpression }.lastOrNull() ?: this
}
.parentIndent()
?: indentError.expected) +
(if (configuration.extendedIndentBeforeDot) 2 else 1) * configuration.indentationSize, true)
}.parentIndent() ?: 0
val expectedIndent = parentIndent + indentIncrement
return CheckResult.from(indentError.actual, expectedIndent, true)
}
return null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.assertNo
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.describe
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.extendedIndent
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.withCustomParameters
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.dotQualifiedExpressions
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.expressionBodyFunctions
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.expressionsWrappedAfterOperator
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.parenthesesSurroundedInfixExpressions
Expand Down Expand Up @@ -248,4 +249,38 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
rulesConfigList = customConfig.asRulesConfigList())
}
}

/**
* See [#1336](https://github.com/saveourtool/diktat/issues/1336).
*/
@Nested
@TestMethodOrder(DisplayName::class)
inner class `Dot- and safe-qualified expressions` {
@ParameterizedTest(name = "extendedIndentBeforeDot = {0}")
@ValueSource(booleans = [false, true])
@Tag(WarningNames.WRONG_INDENTATION)
fun `should be properly indented`(extendedIndentBeforeDot: Boolean, @TempDir tempDir: Path) {
val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters("extendedIndentBeforeDot" to extendedIndentBeforeDot)

lintMultipleMethods(
dotQualifiedExpressions[extendedIndentBeforeDot].assertNotNull(),
tempDir = tempDir,
rulesConfigList = customConfig.asRulesConfigList())
}

@ParameterizedTest(name = "extendedIndentBeforeDot = {0}")
@ValueSource(booleans = [false, true])
@Tag(WarningNames.WRONG_INDENTATION)
fun `should be reformatted if mis-indented`(extendedIndentBeforeDot: Boolean, @TempDir tempDir: Path) {
val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters("extendedIndentBeforeDot" to extendedIndentBeforeDot)

lintMultipleMethods(
actualContent = dotQualifiedExpressions[!extendedIndentBeforeDot].assertNotNull(),
expectedContent = dotQualifiedExpressions[extendedIndentBeforeDot].assertNotNull(),
tempDir = tempDir,
rulesConfigList = customConfig.asRulesConfigList())
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,161 @@ internal object IndentationRuleTestResources {
false to parenthesesSurroundedInfixExpressionsSingleIndent,
true to parenthesesSurroundedInfixExpressionsContinuationIndent)

/**
* Dot-qualified and safe-access expressions, single indent
* (`extendedIndentBeforeDot` is **off**).
*
* When adding new code fragments to this list, be sure to also add their
* counterparts (preserving order) to
* [dotQualifiedExpressionsContinuationIndent].
*
* See [#1336](https://github.com/saveourtool/diktat/issues/1336).
*
* @see dotQualifiedExpressionsContinuationIndent
*/
@Language("kotlin")
val dotQualifiedExpressionsSingleIndent = arrayOf(
"""
|fun LocalDateTime.updateTime(
| hour: Int? = null,
| minute: Int? = null,
| second: Int? = null,
|): LocalDateTime = withHour(hour ?: getHour())
| .withMinute(minute ?: getMinute())
| .withSecond(second ?: getSecond())
""".trimMargin(),

"""
|fun f() {
| first()
| .second()
| .third()
|}
""".trimMargin(),

"""
|val a = first()
| .second()
| .third()
""".trimMargin(),

"""
|val b = first()
| ?.second()
| ?.third()
""".trimMargin(),

"""
|fun f1() = first()
| .second()
| .third()
""".trimMargin(),

"""
|fun f2() =
| first()
| .second()
| .third()
""".trimMargin(),

"""
|fun f3() = g(first()
| .second()
| .third()
| .fourth())
""".trimMargin(),

"""
|fun f4() = g(
| first()
| .second()
| .third()
| .fourth())
""".trimMargin(),
)

/**
* Dot-qualified and safe-access expressions, continuation indent
* (`extendedIndentBeforeDot` is **on**).
*
* When adding new code fragments to this list, be sure to also add their
* counterparts (preserving order) to
* [dotQualifiedExpressionsSingleIndent].
*
* See [#1336](https://github.com/saveourtool/diktat/issues/1336).
*
* @see dotQualifiedExpressionsSingleIndent
*/
@Language("kotlin")
val dotQualifiedExpressionsContinuationIndent = arrayOf(
"""
|fun LocalDateTime.updateTime(
| hour: Int? = null,
| minute: Int? = null,
| second: Int? = null,
|): LocalDateTime = withHour(hour ?: getHour())
| .withMinute(minute ?: getMinute())
| .withSecond(second ?: getSecond())
""".trimMargin(),

"""
|fun f() {
| first()
| .second()
| .third()
|}
""".trimMargin(),

"""
|val a = first()
| .second()
| .third()
""".trimMargin(),

"""
|val b = first()
| ?.second()
| ?.third()
""".trimMargin(),

"""
|fun f1() = first()
| .second()
| .third()
""".trimMargin(),

"""
|fun f2() =
| first()
| .second()
| .third()
""".trimMargin(),

"""
|fun f3() = g(first()
| .second()
| .third()
| .fourth())
""".trimMargin(),

"""
|fun f4() = g(
| first()
| .second()
| .third()
| .fourth())
""".trimMargin(),
)

/**
* Dot-qualified and safe-access expressions.
*
* See [#1336](https://github.com/saveourtool/diktat/issues/1336).
*/
val dotQualifiedExpressions = mapOf(
false to dotQualifiedExpressionsSingleIndent,
true to dotQualifiedExpressionsContinuationIndent)

@Language("kotlin")
@Suppress("COMMENT_WHITE_SPACE")
private val ifExpressionsSingleIndent = arrayOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.asSequen
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.assertNotNull
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.describe
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.withCustomParameters
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.dotQualifiedExpressions
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.expressionBodyFunctions
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.expressionsWrappedAfterOperator
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.ifExpressions
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.parenthesesSurroundedInfixExpressions
import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_INDENTATION
import org.cqfn.diktat.ruleset.rules.chapter3.files.IndentationRule
import org.cqfn.diktat.util.LintTestBase

import com.pinterest.ktlint.core.LintError
import generated.WarningNames
import org.assertj.core.api.AbstractSoftAssertions
Expand All @@ -29,6 +31,7 @@ import org.junit.jupiter.api.TestMethodOrder
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.ValueSource
import org.opentest4j.MultipleFailuresError

import java.util.function.Consumer

@Suppress("LargeClass")
Expand Down Expand Up @@ -936,6 +939,48 @@ class IndentationRuleWarnTest : LintTestBase(::IndentationRule) {
}
}

/**
* See [#1336](https://github.com/saveourtool/diktat/issues/1336).
*/
@Nested
@TestMethodOrder(DisplayName::class)
inner class `Dot- and safe-qualified expressions` {
@ParameterizedTest(name = "extendedIndentBeforeDot = {0}")
@ValueSource(booleans = [false, true])
@Tag(WarningNames.WRONG_INDENTATION)
fun `should be properly indented`(extendedIndentBeforeDot: Boolean) {
val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters("extendedIndentBeforeDot" to extendedIndentBeforeDot)

lintMultipleMethods(
dotQualifiedExpressions[extendedIndentBeforeDot].assertNotNull(),
lintErrors = emptyArray(),
customConfig.asRulesConfigList()
)
}

@ParameterizedTest(name = "extendedIndentBeforeDot = {0}")
@ValueSource(booleans = [false, true])
@Tag(WarningNames.WRONG_INDENTATION)
fun `should be reported if mis-indented`(extendedIndentBeforeDot: Boolean) {
val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters("extendedIndentBeforeDot" to extendedIndentBeforeDot)

assertSoftly { softly ->
dotQualifiedExpressions[!extendedIndentBeforeDot].assertNotNull().forEach { code ->
softly.assertThat(lintResult(code, customConfig.asRulesConfigList()))
.describedAs("lint result for ${code.describe()}")
.isNotEmpty
.hasSizeBetween(2, 3).allSatisfy(Consumer { lintError ->
assertThat(lintError.ruleId).describedAs("ruleId").isEqualTo(ruleId)
assertThat(lintError.canBeAutoCorrected).describedAs("canBeAutoCorrected").isTrue
assertThat(lintError.detail).matches(warnTextRegex)
})
}
}
}
}

@Nested
@TestMethodOrder(DisplayName::class)
inner class `If expressions` {
Expand Down

0 comments on commit 0bcbd40

Please sign in to comment.