Skip to content

Commit

Permalink
Add configuration setting for ignoring argument-list-wrapping above…
Browse files Browse the repository at this point in the history
… treshold of parameters (pinterest#2481)

* Add configuration setting for ignoring `argument-list-wrapping` above treshold of parameters

Breaking change: For code style `ktlint_official` the default value for this setting has default `unset` which is different from previous default value `8`. Not wrapping parameters should be a conscious decision of the developer. This can either be done on a case by case basis by suppressing the rule, or by configuring the parameter.

Closes pinterest#2461
  • Loading branch information
paul-dingemans authored Jan 23, 2024
1 parent 5d9c5bb commit 66d4854
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 44 deletions.
6 changes: 5 additions & 1 deletion documentation/snapshot/docs/rules/standard.md
Original file line number Diff line number Diff line change
Expand Up @@ -1243,7 +1243,7 @@ Rule id: `no-trailing-spaces` (`standard` rule set)

## No `Unit` as return type

The `Unit` type is not allowed as return type of a function.
The `Unit` type is not allowed as return-type of a function.

=== "[:material-heart:](#) Ktlint"

Expand Down Expand Up @@ -2341,6 +2341,10 @@ All arguments should be on the same line, or every argument should be on a separ
)
```

| Configuration setting | ktlint_official | intellij_idea | android_studio |
|:----------------------------------------------------------------------------------|:---------------:|:-------------:|:--------------:|
| `ktlint_argument_list_wrapping_ignore_when_parameter_count_greater_or_equal_than` | `unset` | 8 | 8 |

Rule-id: `argument-list-wrapping` (`standard` rule set)

### Chain wrapping
Expand Down
5 changes: 5 additions & 0 deletions ktlint-ruleset-standard/api/ktlint-ruleset-standard.api
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,16 @@ public final class com/pinterest/ktlint/ruleset/standard/rules/AnnotationSpacing
}

public final class com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule : com/pinterest/ktlint/ruleset/standard/StandardRule {
public static final field Companion Lcom/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule$Companion;
public fun <init> ()V
public fun beforeFirstNode (Lcom/pinterest/ktlint/rule/engine/core/api/editorconfig/EditorConfig;)V
public fun beforeVisitChildNodes (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;ZLkotlin/jvm/functions/Function3;)V
}

public final class com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule$Companion {
public final fun getIGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY ()Lcom/pinterest/ktlint/rule/engine/core/api/editorconfig/EditorConfigProperty;
}

public final class com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRuleKt {
public static final fun getARGUMENT_LIST_WRAPPING_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.pinterest.ktlint.rule.engine.core.api.RuleId
import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint
import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint.Status.STABLE
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfigProperty
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
Expand All @@ -30,6 +31,7 @@ import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithNewline
import com.pinterest.ktlint.rule.engine.core.api.lineLengthWithoutNewlinePrefix
import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
import com.pinterest.ktlint.ruleset.standard.StandardRule
import org.ec4j.core.model.PropertyType
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
Expand Down Expand Up @@ -70,6 +72,7 @@ public class ArgumentListWrappingRule :
),
usesEditorConfigProperties =
setOf(
IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY,
INDENT_SIZE_PROPERTY,
INDENT_STYLE_PROPERTY,
MAX_LINE_LENGTH_PROPERTY,
Expand All @@ -78,6 +81,7 @@ public class ArgumentListWrappingRule :
private var editorConfigIndent = IndentConfig.DEFAULT_INDENT_CONFIG

private var maxLineLength = MAX_LINE_LENGTH_PROPERTY.defaultValue
private var ignoreWhenParameterCountGreaterOrEqualThanProperty = UNSET_IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY

override fun beforeFirstNode(editorConfig: EditorConfig) {
editorConfigIndent =
Expand All @@ -86,6 +90,7 @@ public class ArgumentListWrappingRule :
tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
)
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
ignoreWhenParameterCountGreaterOrEqualThanProperty = editorConfig[IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY]
}

override fun beforeVisitChildNodes(
Expand All @@ -111,8 +116,8 @@ public class ArgumentListWrappingRule :
node.firstChildNode?.treeNext?.elementType != RPAR &&
// skip lambda arguments
node.treeParent?.elementType != FUNCTION_LITERAL &&
// skip if number of arguments is big (we assume it with a magic number of 8)
node.children().count { it.elementType == VALUE_ARGUMENT } <= 8
// skip if number of arguments is big
node.children().count { it.elementType == VALUE_ARGUMENT } <= ignoreWhenParameterCountGreaterOrEqualThanProperty
) {
// each argument should be on a separate line if
// - at least one of the arguments is
Expand Down Expand Up @@ -298,6 +303,38 @@ public class ArgumentListWrappingRule :
}
return true
}

public companion object {
private const val UNSET_IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY = Int.MAX_VALUE
public val IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY: EditorConfigProperty<Int> =
EditorConfigProperty(
type =
PropertyType.LowerCasingPropertyType(
"ktlint_argument_list_wrapping_ignore_when_parameter_count_greater_or_equal_than",
"Do not wrap parameters on separate lines when at least the specified number of parameters are " +
"specified. Use 'unset' to always wrap each parameter.",
PropertyType.PropertyValueParser.POSITIVE_INT_VALUE_PARSER,
setOf("1", "2", "3", "4", "5", "6", "7", "8", "9", "unset"),
),
// Historically, all code styles have used 8 as the magic value.
defaultValue = 8,
ktlintOfficialCodeStyleDefaultValue = UNSET_IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY,
propertyMapper = { property, _ ->
if (property?.isUnset == true) {
UNSET_IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY
} else {
property?.getValueAs<Int>()
}
},
propertyWriter = { property ->
if (property == UNSET_IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY) {
"unset"
} else {
property.toString()
}
},
)
}
}

public val ARGUMENT_LIST_WRAPPING_RULE_ID: RuleId = ArgumentListWrappingRule().ruleId
Original file line number Diff line number Diff line change
Expand Up @@ -38,39 +38,14 @@ import java.util.Arrays

@SinceKtlint("0.7", STABLE)
public class ModifierOrderRule : StandardRule("modifier-order") {
// subset of ElementType.MODIFIER_KEYWORDS_ARRAY (+ annotations entries)
private val order =
arrayOf(
ANNOTATION_ENTRY,
PUBLIC_KEYWORD, PROTECTED_KEYWORD, PRIVATE_KEYWORD, INTERNAL_KEYWORD,
EXPECT_KEYWORD, ACTUAL_KEYWORD,
FINAL_KEYWORD, OPEN_KEYWORD, ABSTRACT_KEYWORD, SEALED_KEYWORD, CONST_KEYWORD,
EXTERNAL_KEYWORD,
OVERRIDE_KEYWORD,
LATEINIT_KEYWORD,
TAILREC_KEYWORD,
VARARG_KEYWORD,
SUSPEND_KEYWORD,
INNER_KEYWORD,
ENUM_KEYWORD, ANNOTATION_KEYWORD,
COMPANION_KEYWORD,
INLINE_KEYWORD,
INFIX_KEYWORD,
OPERATOR_KEYWORD,
DATA_KEYWORD,
// NOINLINE_KEYWORD, CROSSINLINE_KEYWORD, OUT_KEYWORD, IN_KEYWORD, REIFIED_KEYWORD
// HEADER_KEYWORD, IMPL_KEYWORD
)
private val tokenSet = TokenSet.create(*order)

override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
if (node.psi is KtDeclarationModifierList) {
val modifierArr = node.getChildren(tokenSet)
val sorted = modifierArr.copyOf().apply { sortWith(compareBy { order.indexOf(it.elementType) }) }
val sorted = modifierArr.copyOf().apply { sortWith(compareBy { ORDERED_MODIFIERS.indexOf(it.elementType) }) }
if (!Arrays.equals(modifierArr, sorted)) {
// Since annotations can be fairly lengthy and/or span multiple lines we are
// squashing them into a single placeholder text to guarantee a single line output
Expand Down Expand Up @@ -100,6 +75,42 @@ public class ModifierOrderRule : StandardRule("modifier-order") {
nonAnnotationModifiers.map { it.text }
}
}

private companion object {
// subset of ElementType.MODIFIER_KEYWORDS_ARRAY (+ annotations entries)
private val ORDERED_MODIFIERS =
arrayOf(
ANNOTATION_ENTRY,
PUBLIC_KEYWORD,
PROTECTED_KEYWORD,
PRIVATE_KEYWORD,
INTERNAL_KEYWORD,
EXPECT_KEYWORD,
ACTUAL_KEYWORD,
FINAL_KEYWORD,
OPEN_KEYWORD,
ABSTRACT_KEYWORD,
SEALED_KEYWORD,
CONST_KEYWORD,
EXTERNAL_KEYWORD,
OVERRIDE_KEYWORD,
LATEINIT_KEYWORD,
TAILREC_KEYWORD,
VARARG_KEYWORD,
SUSPEND_KEYWORD,
INNER_KEYWORD,
ENUM_KEYWORD,
ANNOTATION_KEYWORD,
COMPANION_KEYWORD,
INLINE_KEYWORD,
INFIX_KEYWORD,
OPERATOR_KEYWORD,
DATA_KEYWORD,
// NOINLINE_KEYWORD, CROSSINLINE_KEYWORD, OUT_KEYWORD, IN_KEYWORD, REIFIED_KEYWORD
// HEADER_KEYWORD, IMPL_KEYWORD
)
private val tokenSet = TokenSet.create(*ORDERED_MODIFIERS)
}
}

public val MODIFIER_ORDER_RULE_ID: RuleId = ModifierOrderRule().ruleId
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ public class NoUnusedImportsRule : StandardRule("no-unused-imports") {
private companion object {
val COMPONENT_N_REGEX = Regex("^component\\d+$")

@Suppress("ktlint:standard:argument-list-wrapping")
val OPERATOR_SET =
setOf(
// unary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,15 @@ public class SpacingAroundKeywordRule : StandardRule("keyword-spacing") {
private val noLFBeforeSet = create(ELSE_KEYWORD, CATCH_KEYWORD, FINALLY_KEYWORD)
private val tokenSet =
create(
FOR_KEYWORD, IF_KEYWORD, ELSE_KEYWORD, WHILE_KEYWORD, DO_KEYWORD,
TRY_KEYWORD, CATCH_KEYWORD, FINALLY_KEYWORD, WHEN_KEYWORD,
CATCH_KEYWORD,
DO_KEYWORD,
ELSE_KEYWORD,
FINALLY_KEYWORD,
FOR_KEYWORD,
IF_KEYWORD,
TRY_KEYWORD,
WHEN_KEYWORD,
WHILE_KEYWORD,
)

private val keywordsWithoutSpaces = create(GET_KEYWORD, SET_KEYWORD)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.pinterest.ktlint.ruleset.standard.rules

import com.pinterest.ktlint.rule.engine.core.api.editorconfig.MAX_LINE_LENGTH_PROPERTY
import com.pinterest.ktlint.ruleset.standard.StandardRuleSetProvider
import com.pinterest.ktlint.ruleset.standard.rules.ArgumentListWrappingRule.Companion.IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY
import com.pinterest.ktlint.ruleset.standard.rules.ClassSignatureRule.Companion.FORCE_MULTILINE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY
import com.pinterest.ktlint.test.KtLintAssertThat.Companion.EOL_CHAR
import com.pinterest.ktlint.test.KtLintAssertThat.Companion.MAX_LINE_LENGTH_MARKER
Expand Down Expand Up @@ -74,23 +75,25 @@ class ArgumentListWrappingRuleTest {
fun `Given that not all parameters in a function call fit on a single line`() {
val code =
"""
val x = f(a, b, c)
// $MAX_LINE_LENGTH_MARKER $EOL_CHAR
val foobar = foobar(foo, bar, baz)
""".trimIndent()
val formattedCode =
"""
val x = f(
a,
b,
c
// $MAX_LINE_LENGTH_MARKER $EOL_CHAR
val foobar = foobar(
foo,
bar,
baz
)
""".trimIndent()
argumentListWrappingRuleAssertThat(code)
.withEditorConfigOverride(MAX_LINE_LENGTH_PROPERTY to 10)
.setMaxLineLength()
.hasLintViolations(
LintViolation(1, 11, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(1, 14, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(1, 17, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(1, 18, "Missing newline before \")\""),
LintViolation(2, 21, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(2, 26, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(2, 31, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(2, 34, "Missing newline before \")\""),
).isFormattedAs(formattedCode)
}

Expand Down Expand Up @@ -292,15 +295,17 @@ class ArgumentListWrappingRuleTest {
}

@Nested
inner class `Given a function call with too man (eg more than 8) arguments` {
inner class `Given a function call with too many (eg more than 8) arguments` {
@Test
fun `Given that arguments are on a single line but exceeding max line length`() {
val code =
"""
// $MAX_LINE_LENGTH_MARKER $EOL_CHAR
val foo = foo(1, 2, 3, 4, 5, 6, 7, 8, 9)
""".trimIndent()
argumentListWrappingRuleAssertThat(code)
.withEditorConfigOverride(MAX_LINE_LENGTH_PROPERTY to 20)
.setMaxLineLength()
.withEditorConfigOverride(IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY to 8)
.hasNoLintViolations()
}

Expand All @@ -314,7 +319,22 @@ class ArgumentListWrappingRuleTest {
9
)
""".trimIndent()
argumentListWrappingRuleAssertThat(code).hasNoLintViolations()
argumentListWrappingRuleAssertThat(code)
.withEditorConfigOverride(IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY to 8)
.hasNoLintViolations()
}

@Test
fun `Given that arguments always have to be wrapped to a separate line`() {
val code =
"""
val foo = foo(
1, 2
)
""".trimIndent()
argumentListWrappingRuleAssertThat(code)
.withEditorConfigOverride(IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY to "unset")
.hasLintViolation(2, 8, "Argument should be on a separate line (unless all arguments can fit a single line)")
}
}

Expand Down

0 comments on commit 66d4854

Please sign in to comment.