Skip to content

Commit

Permalink
Don't change the indent for parentheses-enclosed expressions (#1410)
Browse files Browse the repository at this point in the history 
* Don't change the indent for parentheses-enclosed expressions

### What's done:

 * From now on, parentheses (`LPAR`/`RPAR`) don't increment or decrement the
   indent unless they're a part of a function declaration or a function call.
 * Parentheses which just enclose an expression don't affect the indent at all.
 * The only exclusion to the above rule is an `LPAR` immediately followed by a newline
   (in this special case the indent is still incremented).
 * This fixes #1409.
  • Loading branch information
@0x6675636b796f75676974687562
0x6675636b796f75676974687562 authored Jun 29, 2022
1 parent e9476dc commit 1445b6e
Show file tree
Hide file tree
Showing 6 changed files with 424 additions and 100 deletions.
7 changes: 5 additions & 2 deletions ...-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/StringTemplateFormatRule.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,11 @@ class StringTemplateFormatRule(configRules: List<RulesConfig>) : DiktatRule(
(!(node.treeNext
.text
.first()
.isLetterOrDigit() // checking if first letter is valid
|| node.treeNext.text.startsWith("_")) || node.treeNext.elementType == CLOSING_QUOTE)
// checking if first letter is valid
.isLetterOrDigit() ||
node.treeNext.text.startsWith("_")) ||
node.treeNext.elementType == CLOSING_QUOTE
)
} else if (!isArrayAccessExpression) {
node.hasAnyChildOfTypes(FLOAT_CONSTANT, INTEGER_CONSTANT) // it also fixes "${1.0}asd" cases
} else {
Expand Down
119 changes: 111 additions & 8 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 @@ -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
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
Loading

0 comments on commit 1445b6e

Please sign in to comment.