Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't change the indent for parentheses-enclosed expressions #1410

Merged
merged 5 commits into from
Jun 29, 2022
Merged

Don't change the indent for parentheses-enclosed expressions #1410

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
023496c
Don't change the indent for parentheses-enclosed expressions
0x6675636b796f75676974687562 Jun 28, 2022
a88d8ea
Add unit tests
0x6675636b796f75676974687562 Jun 28, 2022
a848a3e
Add support for the LPAR immediately followed by a newline special case
0x6675636b796f75676974687562 Jun 29, 2022
6cf4d5a
Merge branch 'master' into bugfix/andrey.shcheglov/lpar-rpar-in-binar…
0x6675636b796f75676974687562 Jun 29, 2022
ba086e8
Correct file formatting
0x6675636b796f75676974687562 Jun 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 111 additions & 8 deletions diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import org.cqfn.diktat.ruleset.utils.isSpaceCharacter
import org.cqfn.diktat.ruleset.utils.lastIndent
import org.cqfn.diktat.ruleset.utils.leaveOnlyOneNewLine

import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION
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
Expand All @@ -45,6 +46,7 @@ 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.PARENTHESIZED
import com.pinterest.ktlint.core.ast.ElementType.RBRACE
import com.pinterest.ktlint.core.ast.ElementType.RBRACKET
import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
Expand All @@ -55,7 +57,10 @@ 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.VALUE_ARGUMENT_LIST
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.visit
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
Expand All @@ -73,6 +78,7 @@ import org.jetbrains.kotlin.psi.psiUtil.startOffset
import org.slf4j.LoggerFactory

import kotlin.math.abs
import kotlin.reflect.KCallable

/**
* Rule that checks indentation. The following general rules are checked:
Expand Down Expand Up @@ -176,9 +182,9 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
val context = IndentContext(configuration)
node.visit { astNode ->
context.checkAndReset(astNode)
if (astNode.elementType in increasingTokens) {
if (astNode.isIndentIncrementing()) {
context.storeIncrementingToken(astNode.elementType)
} else if (astNode.elementType in decreasingTokens && !astNode.treePrev.let { it.elementType == WHITE_SPACE && it.textContains(NEWLINE) }) {
} else if (astNode.isIndentDecrementing() && !astNode.treePrev.let { it.elementType == WHITE_SPACE && it.textContains(NEWLINE) }) {
// if decreasing token is after WHITE_SPACE with \n, indents are corrected in visitWhiteSpace method
context.dec(astNode.elementType)
} else if (astNode.elementType == WHITE_SPACE && astNode.textContains(NEWLINE) && astNode.treeNext != null) {
Expand Down Expand Up @@ -208,7 +214,7 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
context.maybeIncrement()
positionByOffset = astNode.treeParent.calculateLineColByOffset()
val whiteSpace = astNode.psi as PsiWhiteSpace
if (astNode.treeNext.elementType in decreasingTokens) {
if (astNode.treeNext.isIndentDecrementing()) {
// if newline is followed by closing token, it should already be indented less
context.dec(astNode.treeNext.elementType)
}
Expand Down Expand Up @@ -493,7 +499,7 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
}
regularIndent -= config.indentationSize
}
if (activeTokens.isNotEmpty() && activeTokens.peek() == matchingTokens.find { it.second == token }?.first) {
if (activeTokens.isNotEmpty() && activeTokens.peek() == token.braceMatchOrNull()) {
activeTokens.pop()
}
}
Expand Down Expand Up @@ -546,11 +552,20 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
companion object {
private val log = LoggerFactory.getLogger(IndentationRule::class.java)
const val NAME_ID = "indentation"
private val increasingTokens = listOf(LPAR, LBRACE, LBRACKET, LONG_TEMPLATE_ENTRY_START)
private val decreasingTokens = listOf(RPAR, RBRACE, RBRACKET, LONG_TEMPLATE_ENTRY_END)
private val matchingTokens = increasingTokens.zip(decreasingTokens)
private val increasingTokens: Set<IElementType> = linkedSetOf(LPAR, LBRACE, LBRACKET, LONG_TEMPLATE_ENTRY_START)
private val decreasingTokens: Set<IElementType> = linkedSetOf(RPAR, RBRACE, RBRACKET, LONG_TEMPLATE_ENTRY_END)

/**
* This is essentially a bi-map, which allows to look up a closing brace
* type by an opening brace type, or vice versa.
*/
private val matchingTokens = (increasingTokens.asSequence() zip decreasingTokens.asSequence()).flatMap { (opening, closing) ->
sequenceOf(opening to closing, closing to opening)
}.toMap()
private val stringLiteralTokens = listOf(SHORT_STRING_TEMPLATE_ENTRY, LONG_STRING_TEMPLATE_ENTRY)
private val knownTrimFunctionPatterns = setOf("trimIndent", "trimMargin")
private val knownTrimFunctionPatterns = sequenceOf(String::trimIndent, String::trimMargin)
.map(KCallable<String>::name)
.toSet()

/**
* @return a string which consists of `N` [space][SPACE] characters.
Expand All @@ -560,6 +575,87 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
get() =
SPACE.toString().repeat(n = this)

/**
* @return `true` if the indent should be incremented once this node is
* encountered, `false` otherwise.
* @see isIndentDecrementing
*/
private fun ASTNode.isIndentIncrementing(): Boolean =
when (elementType) {
/*
* This is a special case of an opening parenthesis which *may* or
* *may not* be indent-incrementing.
*/
LPAR -> isParenthesisAffectingIndent()

else -> elementType in increasingTokens
}

/**
* @return `true` if the indent should be decremented once this node is
* encountered, `false` otherwise.
* @see isIndentIncrementing
*/
private fun ASTNode.isIndentDecrementing(): Boolean =
when (elementType) {
/*
* This is a special case of a closing parenthesis which *may* or
* *may not* be indent-decrementing.
*/
RPAR -> isParenthesisAffectingIndent()

else -> elementType in decreasingTokens
}

/**
* Parentheses always affect indent when they're a part of a
* [VALUE_PARAMETER_LIST] (formal arguments) or a [VALUE_ARGUMENT_LIST]
* (effective function call arguments).
*
* When they're children of a [PARENTHESIZED] (often inside a
* [BINARY_EXPRESSION]), contribute to the indent depending on whether
* there's a newline after the opening parenthesis.
*
* @return whether this [LPAR] or [RPAR] node affects indent.
* @see BINARY_EXPRESSION
* @see PARENTHESIZED
* @see VALUE_ARGUMENT_LIST
* @see VALUE_PARAMETER_LIST
*/
private fun ASTNode.isParenthesisAffectingIndent(): Boolean {
require(elementType in arrayOf(LPAR, RPAR)) {
elementType.toString()
}

return when (treeParent.elementType) {
PARENTHESIZED -> when (elementType) {
/*
* `LPAR` inside a binary expression only contributes to the
* indent if it's immediately followed by a newline.
*/
LPAR -> treeNext.isWhiteSpaceWithNewline()

/*
* `RPAR` inside a binary expression affects the indent only
* if its matching `LPAR` node does so.
*/
else -> {
val openingParenthesis = elementType.braceMatchOrNull()?.let { braceMatch ->
treeParent.findChildByType(braceMatch)
}
openingParenthesis?.isParenthesisAffectingIndent() ?: false
}
}

/*
* Either a control-flow statement (one of IF, WHEN, FOR or
* DO_WHILE), a function declaration (VALUE_PARAMETER_LIST or
* PROPERTY_ACCESSOR), or a function call (VALUE_ARGUMENT_LIST).
*/
else -> true
}
}

/**
* @return `true` if this is a [String.trimIndent] or [String.trimMargin]
* call, `false` otherwise.
Expand All @@ -584,6 +680,13 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
return identifier.text in knownTrimFunctionPatterns
}

/**
* @return the matching closing brace type for this opening brace type,
* or vice versa.
*/
private fun IElementType.braceMatchOrNull(): IElementType? =
matchingTokens[this]

/**
* Checks this [REGULAR_STRING_PART] child of a [LITERAL_STRING_TEMPLATE_ENTRY].
*
Expand Down
103 changes: 47 additions & 56 deletions ...t-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleFixTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@ import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.IndentationConfig
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.asRulesConfigList
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.asSequenceWithConcatenation
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.extendedIndent
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestMixin.withCustomParameters
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.expressionBodyFunctionsContinuationIndent
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.expressionBodyFunctionsSingleIndent
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.expressionsWrappedAfterOperatorContinuationIndent
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.expressionsWrappedAfterOperatorSingleIndent
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.whitespaceInStringLiteralsContinuationIndent
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.whitespaceInStringLiteralsSingleIndent
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
import org.cqfn.diktat.ruleset.chapter3.spaces.IndentationRuleTestResources.whitespaceInStringLiterals
import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_INDENTATION
import org.cqfn.diktat.ruleset.rules.chapter3.files.IndentationRule
import org.cqfn.diktat.util.FixTestBase
Expand Down Expand Up @@ -127,13 +126,8 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters("extendedIndentAfterOperators" to extendedIndentAfterOperators)

val expressionBodyFunctions = when {
extendedIndentAfterOperators -> expressionBodyFunctionsContinuationIndent
else -> expressionBodyFunctionsSingleIndent
}

lintMultipleMethods(
expressionBodyFunctions,
expressionBodyFunctions[extendedIndentAfterOperators].assertNotNull(),
tempDir = tempDir,
rulesConfigList = customConfig.asRulesConfigList())
}
Expand All @@ -145,18 +139,9 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters("extendedIndentAfterOperators" to extendedIndentAfterOperators)

val expressionBodyFunctionsActual = when {
extendedIndentAfterOperators -> expressionBodyFunctionsSingleIndent
else -> expressionBodyFunctionsContinuationIndent
}
val expressionBodyFunctionsExpected = when {
extendedIndentAfterOperators -> expressionBodyFunctionsContinuationIndent
else -> expressionBodyFunctionsSingleIndent
}

lintMultipleMethods(
actualContent = expressionBodyFunctionsActual,
expectedContent = expressionBodyFunctionsExpected,
actualContent = expressionBodyFunctions[!extendedIndentAfterOperators].assertNotNull(),
expectedContent = expressionBodyFunctions[extendedIndentAfterOperators].assertNotNull(),
tempDir = tempDir,
rulesConfigList = customConfig.asRulesConfigList())
}
Expand All @@ -175,13 +160,8 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters(*extendedIndent(enabled = extendedIndent))

val whitespaceInStringLiterals = when {
extendedIndent -> whitespaceInStringLiteralsContinuationIndent
else -> whitespaceInStringLiteralsSingleIndent
}

lintMultipleMethods(
whitespaceInStringLiterals,
whitespaceInStringLiterals[extendedIndent].assertNotNull(),
tempDir = tempDir,
rulesConfigList = customConfig.asRulesConfigList())
}
Expand All @@ -193,18 +173,9 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters(*extendedIndent(enabled = extendedIndent))

val whitespaceInStringLiteralsActual = when {
extendedIndent -> whitespaceInStringLiteralsSingleIndent
else -> whitespaceInStringLiteralsContinuationIndent
}
val whitespaceInStringLiteralsExpected = when {
extendedIndent -> whitespaceInStringLiteralsContinuationIndent
else -> whitespaceInStringLiteralsSingleIndent
}

lintMultipleMethods(
actualContent = whitespaceInStringLiteralsActual,
expectedContent = whitespaceInStringLiteralsExpected,
actualContent = whitespaceInStringLiterals[!extendedIndent].assertNotNull(),
expectedContent = whitespaceInStringLiterals[extendedIndent].assertNotNull(),
tempDir = tempDir,
rulesConfigList = customConfig.asRulesConfigList())
}
Expand All @@ -223,13 +194,8 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters("extendedIndentAfterOperators" to extendedIndentAfterOperators)

val expressionsWrappedAfterOperator = when {
extendedIndentAfterOperators -> expressionsWrappedAfterOperatorContinuationIndent
else -> expressionsWrappedAfterOperatorSingleIndent
}

lintMultipleMethods(
expressionsWrappedAfterOperator,
expressionsWrappedAfterOperator[extendedIndentAfterOperators].assertNotNull(),
tempDir = tempDir,
rulesConfigList = customConfig.asRulesConfigList())
}
Expand All @@ -241,18 +207,43 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters("extendedIndentAfterOperators" to extendedIndentAfterOperators)

val expressionsWrappedAfterOperatorActual = when {
extendedIndentAfterOperators -> expressionsWrappedAfterOperatorSingleIndent
else -> expressionsWrappedAfterOperatorContinuationIndent
}
val expressionsWrappedAfterOperatorExpected = when {
extendedIndentAfterOperators -> expressionsWrappedAfterOperatorContinuationIndent
else -> expressionsWrappedAfterOperatorSingleIndent
}
lintMultipleMethods(
actualContent = expressionsWrappedAfterOperator[!extendedIndentAfterOperators].assertNotNull(),
expectedContent = expressionsWrappedAfterOperator[extendedIndentAfterOperators].assertNotNull(),
tempDir = tempDir,
rulesConfigList = customConfig.asRulesConfigList())
}
}

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

lintMultipleMethods(
parenthesesSurroundedInfixExpressions[extendedIndentAfterOperators].assertNotNull(),
tempDir = tempDir,
rulesConfigList = customConfig.asRulesConfigList())
}

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

lintMultipleMethods(
actualContent = expressionsWrappedAfterOperatorActual,
expectedContent = expressionsWrappedAfterOperatorExpected,
actualContent = parenthesesSurroundedInfixExpressions[!extendedIndentAfterOperators].assertNotNull(),
expectedContent = parenthesesSurroundedInfixExpressions[extendedIndentAfterOperators].assertNotNull(),
tempDir = tempDir,
rulesConfigList = customConfig.asRulesConfigList())
}
Expand Down
14 changes: 14 additions & 0 deletions ...rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTestMixin.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,18 @@ internal object IndentationRuleTestMixin {
else -> "\"$first\u2026\" ($count line(s))"
}
}

/**
* Casts a nullable value to a non-`null` one, similarly to the `!!`
* operator.
*
* @return a non-`null` value.
*/
fun <T> T?.assertNotNull(): T {
check(this != null) {
"Expecting actual not to be null"
}

return this
}
}
Loading