Skip to content

Commit

Permalink
Merge pull request #1846 from pinterest/1775-wrapping-parameter-type
Browse files Browse the repository at this point in the history
Wrapping value parameter and property
  • Loading branch information
paul-dingemans authored Mar 8, 2023
2 parents 5ebafa9 + e64291f commit 12b6eda
Show file tree
Hide file tree
Showing 12 changed files with 766 additions and 31 deletions.
8 changes: 5 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ Previously the default value for `.editorconfig` property `max_line_length` was
* Add new experimental rule for `ktlint_official` code style that disallows a class to start with a blank line `no-empty-first-line-in-class-body`.
* Add new experimental rule for `ktlint_official` code style that enforces consistent usage of braces in all branches of a singe if or if-else-if statement `if-else-bracing`.
* Add new experimental rule for `ktlint_official` code style that disallows consecutive comments except EOL comments `no-consecutive-comments`
* Wrap the type or value of a function or class parameter in case the maximum line length is exceeded `parameter-wrapping` ([#1846](https://github.com/pinterest/ktlint/pull/1846))
* Wrap the type or value of a property in case the maximum line length is exceeded `property-wrapping` ([#1846](https://github.com/pinterest/ktlint/pull/1846))

### Removed

Expand All @@ -238,10 +240,10 @@ Previously the default value for `.editorconfig` property `max_line_length` was
* Do not add the (first line of the) body expression on the same line as the function signature in case the max line length would be exceeded. `function-signature`.
* Do not add the first line of a multiline body expression on the same line as the function signature in case function body expression wrapping property is set to `multiline`. `function-signature`.
* Disable the `standard:filename` rule whenever Ktlint CLI is run with option `--stdin` ([#1742](https://github.com/pinterest/ktlint/issues/1742))
* The parameters of a function literal containing a multiline parameter list are aligned with first parameter whenever the first parameter is on the same line as the start of that function literal (not allowed in `ktlint_official` code style) `indent` ([#1756](https://github.com/pinterest/ktlint/issues/1756)).
* The parameters of a function literal containing a multiline parameter list are aligned with first parameter whenever the first parameter is on the same line as the start of that function literal (not allowed in `ktlint_official` code style) `indent` ([#1756](https://github.com/pinterest/ktlint/issues/1756))
* Do not throw exception when enum class does not contain entries `trailing-comma-on-declaration-site` ([#1711](https://github.com/pinterest/ktlint/issues/1711))
* Fix continuation indent for a dot qualified array access expression in `ktlint_official` code style only `indent` ([#1540](https://github.com/pinterest/ktlint/issues/1540)).
* When generating the `.editorconfig` use value `off` for the `max_line_length` property instead of value `-1` to denote that lines are not restricted to a maximum length ([#1824](https://github.com/pinterest/ktlint/issues/1824)).
* Fix continuation indent for a dot qualified array access expression in `ktlint_official` code style only `indent` ([#1540](https://github.com/pinterest/ktlint/issues/1540))
* When generating the `.editorconfig` use value `off` for the `max_line_length` property instead of value `-1` to denote that lines are not restricted to a maximum length ([#1824](https://github.com/pinterest/ktlint/issues/1824))
* Do not report an "unnecessary semicolon" after adding a trailing comma to an enum class containing a code element after the last enum entry `trailing-comma-on-declaration-site` ([#1786](https://github.com/pinterest/ktlint/issues/1786))
* A newline before a function return type should not be removed in case that leads to exceeding the maximum line length `function-return-type-spacing` ([#1764](https://github.com/pinterest/ktlint/issues/1764))

Expand Down
12 changes: 12 additions & 0 deletions docs/rules/standard.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,18 @@ When class/function signature doesn't fit on a single line, each parameter must

Rule id: `parameter-list-wrapping`

## Parameter wrapping

When a function or class parameter doesn't fit on a single line, wrap the type or value to a separate line

Rule id: `parameter-wrapping`

## Property wrapping

When a property doesn't fit on a single line, wrap the type or value to a separate line

Rule id: `property-wrapping`

## String template

Consistent string templates (`$v` instead of `${v}`, `${p.v}` instead of `${p.v.toString()}`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,16 @@ class FormatReporterTest {

companion object {
@Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length")
val SOME_LINT_ERROR_CAN_BE_AUTOCORRECTED = KtlintCliError(1, 1, "some-rule", "This error can be autocorrected", LINT_CAN_BE_AUTOCORRECTED)
val SOME_LINT_ERROR_CAN_BE_AUTOCORRECTED =
KtlintCliError(1, 1, "some-rule", "This error can be autocorrected", LINT_CAN_BE_AUTOCORRECTED)

@Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length")
val SOME_LINT_ERROR_CAN_NOT_BE_AUTOCORRECTED = KtlintCliError(1, 1, "rule-1", "This error can *not* be autocorrected", LINT_CAN_NOT_BE_AUTOCORRECTED)
val SOME_LINT_ERROR_CAN_NOT_BE_AUTOCORRECTED =
KtlintCliError(1, 1, "rule-1", "This error can *not* be autocorrected", LINT_CAN_NOT_BE_AUTOCORRECTED)

@Suppress("ktlint:argument-list-wrapping", "ktlint:max-line-length")
val SOME_FORMAT_ERROR_IS_AUTOCORRECTED = KtlintCliError(1, 1, "rule-1", "This error can *not* be autocorrected", FORMAT_IS_AUTOCORRECTED)
val SOME_FORMAT_ERROR_IS_AUTOCORRECTED =
KtlintCliError(1, 1, "rule-1", "This error can *not* be autocorrected", FORMAT_IS_AUTOCORRECTED)

const val SOME_FILE_PATH_1 = "/path/to/some-file-1.kt"
const val SOME_FILE_PATH_2 = "/path/to/some-file-2.kt"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package com.pinterest.ktlint.rule.engine.core.api

import com.pinterest.ktlint.rule.engine.core.api.ElementType.REGULAR_STRING_PART
import com.pinterest.ktlint.rule.engine.core.api.ElementType.STRING_TEMPLATE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.VAL_KEYWORD
import com.pinterest.ktlint.rule.engine.core.api.ElementType.VARARG_KEYWORD
import com.pinterest.ktlint.rule.engine.core.api.ElementType.VAR_KEYWORD
import com.pinterest.ktlint.rule.engine.core.api.ElementType.WHITE_SPACE
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiComment
Expand Down Expand Up @@ -358,3 +361,19 @@ public fun leavesInOpenRange(
from
.leaves()
.takeWhile { it != to && it != to.firstChildNode }

public fun ASTNode.isValOrVarKeyword(): Boolean = elementType == VAL_KEYWORD || elementType == VAR_KEYWORD || elementType == VARARG_KEYWORD

/**
* Creates a sequences of leaves including the [ASTNode] in case it is a [LeafElement] itself. By default, the leaves are traversed in
* forward order. Setting [forward] to `false` changes this to traversal in backward direction.
*/
public fun ASTNode.leavesIncludingSelf(forward: Boolean = true): Sequence<ASTNode> {
val sequence =
if (isLeaf()) {
sequenceOf(this)
} else {
emptySequence()
}
return sequence.plus(leaves(forward))
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ import com.pinterest.ktlint.ruleset.standard.rules.NullableTypeSpacingRule
import com.pinterest.ktlint.ruleset.standard.rules.PackageNameRule
import com.pinterest.ktlint.ruleset.standard.rules.ParameterListSpacingRule
import com.pinterest.ktlint.ruleset.standard.rules.ParameterListWrappingRule
import com.pinterest.ktlint.ruleset.standard.rules.ParameterWrappingRule
import com.pinterest.ktlint.ruleset.standard.rules.PropertyNamingRule
import com.pinterest.ktlint.ruleset.standard.rules.PropertyWrappingRule
import com.pinterest.ktlint.ruleset.standard.rules.SpacingAroundAngleBracketsRule
import com.pinterest.ktlint.ruleset.standard.rules.SpacingAroundColonRule
import com.pinterest.ktlint.ruleset.standard.rules.SpacingAroundCommaRule
Expand Down Expand Up @@ -124,7 +126,9 @@ public class StandardRuleSetProvider :
RuleProvider { PackageNameRule() },
RuleProvider { ParameterListSpacingRule() },
RuleProvider { ParameterListWrappingRule() },
RuleProvider { ParameterWrappingRule() },
RuleProvider { PropertyNamingRule() },
RuleProvider { PropertyWrappingRule() },
RuleProvider { SpacingAroundAngleBracketsRule() },
RuleProvider { SpacingAroundColonRule() },
RuleProvider { SpacingAroundCommaRule() },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.CALL_EXPRESSION
import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLASS
import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLASS_BODY
import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLOSING_QUOTE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.COLON
import com.pinterest.ktlint.rule.engine.core.api.ElementType.CONDITION
import com.pinterest.ktlint.rule.engine.core.api.ElementType.CONSTRUCTOR_DELEGATION_CALL
import com.pinterest.ktlint.rule.engine.core.api.ElementType.CONTEXT_RECEIVER_LIST
Expand Down Expand Up @@ -459,6 +460,22 @@ public class IndentationRule :
).prevCodeLeaf()
}

if (codeStyle == ktlint_official) {
// Deviate from standard IntelliJ IDEA formatting to allow formatting below:
// fun process(
// aVariableWithAVeryLongName:
// TypeWithAVeryLongNameThatDoesNotFitOnSameLineAsTheVariableName
// ): List<Output>
node
.findChildByType(COLON)
?.let { fromAstNode ->
nextToAstNode = startIndentContext(
fromAstNode = fromAstNode,
toAstNode = nextToAstNode,
).prevCodeLeaf()
}
}

// Leading annotations and comments should be indented at same level as constructor itself
val fromAstNode = node.skipLeadingWhitespaceCommentsAndAnnotations()
if (fromAstNode != node.firstChildNode &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public class ParameterListWrappingRule :
val newIndentLevel = getNewIndentLevel(node)
node
.children()
.forEach { child -> wrapParemeterInList(newIndentLevel, child, emit, autoCorrect) }
.forEach { child -> wrapParameterInList(newIndentLevel, child, emit, autoCorrect) }
}

private fun getNewIndentLevel(node: ASTNode): Int {
Expand All @@ -177,7 +177,7 @@ public class ParameterListWrappingRule :
}
}

private fun wrapParemeterInList(
private fun wrapParameterInList(
newIndentLevel: Int,
child: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
package com.pinterest.ktlint.ruleset.standard.rules

import com.pinterest.ktlint.logger.api.initKtLintKLogger
import com.pinterest.ktlint.rule.engine.core.api.ElementType
import com.pinterest.ktlint.rule.engine.core.api.ElementType.CALL_EXPRESSION
import com.pinterest.ktlint.rule.engine.core.api.ElementType.COLON
import com.pinterest.ktlint.rule.engine.core.api.ElementType.COMMA
import com.pinterest.ktlint.rule.engine.core.api.ElementType.EQ
import com.pinterest.ktlint.rule.engine.core.api.ElementType.TYPE_REFERENCE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_PARAMETER
import com.pinterest.ktlint.rule.engine.core.api.IndentConfig
import com.pinterest.ktlint.rule.engine.core.api.RuleId
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_SIZE_PROPERTY
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_STYLE_PROPERTY
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.MAX_LINE_LENGTH_PROPERTY
import com.pinterest.ktlint.rule.engine.core.api.firstChildLeafOrSelf
import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithNewline
import com.pinterest.ktlint.rule.engine.core.api.lastChildLeafOrSelf
import com.pinterest.ktlint.rule.engine.core.api.leavesIncludingSelf
import com.pinterest.ktlint.rule.engine.core.api.lineIndent
import com.pinterest.ktlint.rule.engine.core.api.nextCodeLeaf
import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceAfterMe
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceBeforeMe
import com.pinterest.ktlint.ruleset.standard.StandardRule
import mu.KotlinLogging
import org.jetbrains.kotlin.com.intellij.lang.ASTNode

private val LOGGER = KotlinLogging.logger {}.initKtLintKLogger()

/**
* This rule inserts missing newlines inside a value parameter.
*
* Whenever a linebreak is inserted, this rules assumes that the parent node it indented correctly. So the indentation
* is fixed with respect to indentation of the parent. This is just a simple best effort for the case that the
* indentation rule is not run.
*
* This rule has many similarities with the [PropertyWrappingRule] but some subtle differences.
*/
public class ParameterWrappingRule :
StandardRule(
id = "parameter-wrapping",
usesEditorConfigProperties = setOf(
INDENT_SIZE_PROPERTY,
INDENT_STYLE_PROPERTY,
MAX_LINE_LENGTH_PROPERTY,
),
) {
private var line = 1
private lateinit var indentConfig: IndentConfig
private var maxLineLength: Int = MAX_LINE_LENGTH_PROPERTY.defaultValue

override fun beforeFirstNode(editorConfig: EditorConfig) {
line = 1
indentConfig = IndentConfig(
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
)
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
}

override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
if (node.elementType == VALUE_PARAMETER) {
rearrangeValueParameter(node, autoCorrect, emit)
}
}

private fun rearrangeValueParameter(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
require(node.elementType == VALUE_PARAMETER)

val baseIndent = node.lineIndent()

// Find the first node after the indenting whitespace on the same line as the identifier
val nodeFirstChildLeafOrSelf = node.firstChildLeafOrSelf()
val fromNode =
node
.findChildByType(ElementType.IDENTIFIER)
?.leavesIncludingSelf(forward = false)
?.firstOrNull { it.prevLeaf().isWhiteSpaceWithNewline() || it == nodeFirstChildLeafOrSelf }
?: node

node
.findChildByType(COLON)
?.let { colon ->
if (baseIndent.length + fromNode.sumOfTextLengthUntil(colon) > maxLineLength) {
fromNode.sumOfTextLengthUntil(colon)
requireNewlineAfterLeaf(colon, autoCorrect, emit, baseIndent)
return
}
}

node
.findChildByType(TYPE_REFERENCE)
?.let { typeReference ->
if (baseIndent.length + fromNode.sumOfTextLengthUntil(typeReference.orTrailingComma()) > maxLineLength) {
requireNewlineBeforeLeaf(typeReference, autoCorrect, emit, baseIndent)
return
}
}

node
.findChildByType(EQ)
?.let { equal ->
if (baseIndent.length + fromNode.sumOfTextLengthUntil(equal.orTrailingComma()) > maxLineLength) {
requireNewlineAfterLeaf(equal, autoCorrect, emit, baseIndent)
return
}
}

node
.findChildByType(CALL_EXPRESSION)
?.let { callExpression ->
if (baseIndent.length + fromNode.sumOfTextLengthUntil(callExpression.orTrailingComma()) > maxLineLength) {
requireNewlineBeforeLeaf(callExpression, autoCorrect, emit, baseIndent)
return
}
}
}

private fun ASTNode.orTrailingComma() =
lastChildLeafOrSelf()
.nextCodeLeaf()
?.takeIf { it.elementType == COMMA }
?: this

private fun ASTNode.sumOfTextLengthUntil(astNode: ASTNode): Int {
val stopAtLeaf = astNode.lastChildLeafOrSelf()
return leavesIncludingSelf()
.takeWhile { !it.isWhiteSpaceWithNewline() && it.prevLeaf() != stopAtLeaf }
.sumOf { it.textLength }
}

private fun requireNewlineBeforeLeaf(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
indent: String,
) {
emit(
node.startOffset - 1,
"""Missing newline before "${node.text}"""",
true,
)
LOGGER.trace { "$line: " + ((if (!autoCorrect) "would have " else "") + "inserted newline before ${node.text}") }
if (autoCorrect) {
node.upsertWhitespaceBeforeMe("\n" + indent)
}
}

private fun requireNewlineAfterLeaf(
nodeAfterWhichNewlineIsRequired: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
indent: String? = null,
nodeToFix: ASTNode = nodeAfterWhichNewlineIsRequired,
) {
emit(
nodeAfterWhichNewlineIsRequired.startOffset + 1,
"""Missing newline after "${nodeAfterWhichNewlineIsRequired.text}"""",
true,
)
LOGGER.trace {
"$line: " + (if (!autoCorrect) "would have " else "") + "inserted newline after ${nodeAfterWhichNewlineIsRequired.text}"
}
if (autoCorrect) {
val tempIndent = indent ?: (nodeToFix.lineIndent() + indentConfig.indent)
nodeToFix.upsertWhitespaceAfterMe("\n" + tempIndent)
}
}
}

public val PARAMETER_WRAPPING_RULE_ID: RuleId = ParameterWrappingRule().ruleId
Loading

0 comments on commit 12b6eda

Please sign in to comment.