Skip to content

Commit

Permalink
Ignore max_line_length property unless max-line-length rule is en…
Browse files Browse the repository at this point in the history
…abled (#2783)

* Align EditorConfig settings for `max_line_length` and enable/disabling the `max-line-length` rule

IMPORTANT: Potential breaking change for API Consumers that provide the `max_line_length` property but not enabling/providing the `max-line-length` rule of the Ktlint standard rule set.

Rules should only consider the maximum line length in case property `max_line_length` is set to a value between 0 and `MAX_INTEGER - 1`, and the `max-line-length` rule is provided to the KtLintRuleEngine, and the `max-line-length` rule is enabled.

It should not be enforced that property `max_line_length` should have value `unset` or `off` whenever the `max-line-length` rule is disabled, or not provided to the KtLintRuleEngine. This property is also used by IntelliJ IDEA to display the right margin where lines should be wrapped. A user should be able to disable enforce max line length wrapping in ktlint, while still seeing the margin line in the editor.

Closes #2743

* Update documentation
  • Loading branch information
paul-dingemans authored Aug 28, 2024
1 parent 64dfcba commit 4ec5072
Show file tree
Hide file tree
Showing 32 changed files with 291 additions and 81 deletions.
61 changes: 53 additions & 8 deletions documentation/snapshot/docs/rules/standard.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ import com.pinterest.ktlint.rule.engine.api.KtLintRuleException
import com.pinterest.ktlint.rule.engine.core.api.AutocorrectDecision
import com.pinterest.ktlint.rule.engine.core.api.Rule
import com.pinterest.ktlint.rule.engine.core.api.RuleAutocorrectApproveHandler
import com.pinterest.ktlint.rule.engine.core.api.RuleId
import com.pinterest.ktlint.rule.engine.core.api.RuleProvider
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CODE_STYLE_PROPERTY
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.RuleExecution
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.createRuleExecutionEditorConfigProperty
import com.pinterest.ktlint.rule.engine.internal.rulefilter.InternalRuleProvidersFilter
import com.pinterest.ktlint.rule.engine.internal.rulefilter.RuleExecutionRuleFilter
import com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter
Expand Down Expand Up @@ -46,12 +49,28 @@ internal class RuleExecutionContext private constructor(
try {
rule.startTraversalOfAST()
rule.beforeFirstNode(
// The rule get access to an EditConfig which is filtered by the properties which are actually registered as being used by
// The rule gets access to an EditConfig which is filtered by the properties which are actually registered as being used by
// the rule. In this way it can be forced that the rule actually registers the properties that it uses and the field becomes
// reliable to be used by for example the ".editorconfig" file generator.
// Note that also the CODE_STYLE_PROPERTY is provided because that property is needed to determine the default value of an
// EditorConfigProperty that is not explicitly defined.
editorConfig.filterBy(rule.usesEditorConfigProperties.plus(CODE_STYLE_PROPERTY)),
editorConfig.filterBy(
rule.usesEditorConfigProperties
// Provide the CODE_STYLE_PROPERTY as this property is needed to determine the default value of an
// EditorConfigProperty that is not explicitly defined.
.plus(CODE_STYLE_PROPERTY)
// Provide the rule execution property for the "standard:max-line-length" property based on whether a rule provider
// for this rule exists. This property is required to determine whether the property `max_line_length` needs to be
// taken into account.
.plus(
RuleId("standard:max-line-length")
.createRuleExecutionEditorConfigProperty(
if (ruleProviders.any { it.ruleId.value == "standard:max-line-length" }) {
RuleExecution.enabled
} else {
RuleExecution.disabled
},
),
),
),
)
this.executeRuleOnNodeRecursively(rootNode, rule, autocorrectHandler, emitAndApprove)
rule.afterLastNode()
Expand Down
1 change: 1 addition & 0 deletions ktlint-ruleset-standard/api/ktlint-ruleset-standard.api
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ public final class com/pinterest/ktlint/ruleset/standard/rules/MaxLineLengthRule

public final class com/pinterest/ktlint/ruleset/standard/rules/MaxLineLengthRuleKt {
public static final fun getMAX_LINE_LENGTH_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
public static final fun maxLineLength (Lcom/pinterest/ktlint/rule/engine/core/api/editorconfig/EditorConfig;)I
}

public final class com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRule : com/pinterest/ktlint/ruleset/standard/StandardRule, com/pinterest/ktlint/rule/engine/core/api/Rule$Experimental {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public class ArgumentListWrappingRule :
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
)
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
ignoreWhenParameterCountGreaterOrEqualThanProperty = editorConfig[IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public class BinaryExpressionWrappingRule :
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
)
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
}

override fun beforeVisitChildNodes(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public class ChainMethodContinuationRule :
if (indentConfig.disabled) {
stopTraversalOfAST()
}
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
forceMultilineWhenChainOperatorCountGreaterOrEqualThanProperty =
editorConfig[FORCE_MULTILINE_WHEN_CHAIN_OPERATOR_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public class ClassSignatureRule :
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
)
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
}

override fun beforeVisitChildNodes(
Expand Down Expand Up @@ -467,7 +467,7 @@ public class ClassSignatureRule :
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> AutocorrectDecision,
wrappedPrimaryConstructor: Boolean,
): Int {
var whiteSpaceCorrection = 0
val whiteSpaceCorrection = 0

val superTypes = node.superTypes() ?: return 0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class ContextReceiverWrappingRule :
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
)
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
}

override fun beforeVisitChildNodes(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public class FunctionLiteralRule :

override fun beforeFirstNode(editorConfig: EditorConfig) {
codeStyle = editorConfig[CODE_STYLE_PROPERTY]
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
indentConfig =
IndentConfig(
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class FunctionReturnTypeSpacingRule :
private var maxLineLength = MAX_LINE_LENGTH_PROPERTY.defaultValue

override fun beforeFirstNode(editorConfig: EditorConfig) {
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
}

override fun beforeVisitChildNodes(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public class FunctionSignatureRule :
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
)
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
}

override fun beforeVisitChildNodes(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ 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.MAX_LINE_LENGTH_PROPERTY
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.MAX_LINE_LENGTH_PROPERTY_OFF
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.RULE_EXECUTION_PROPERTY_TYPE
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.RuleExecution
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.ktLintRuleExecutionPropertyName
import com.pinterest.ktlint.rule.engine.core.api.isPartOf
import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpace
import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithNewline
Expand Down Expand Up @@ -62,7 +65,7 @@ public class MaxLineLengthRule :

override fun beforeFirstNode(editorConfig: EditorConfig) {
ignoreBackTickedIdentifier = editorConfig[IGNORE_BACKTICKED_IDENTIFIER_PROPERTY]
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
if (maxLineLength == MAX_LINE_LENGTH_PROPERTY_OFF) {
stopTraversalOfAST()
}
Expand Down Expand Up @@ -146,4 +149,26 @@ public class MaxLineLengthRule :
}
}

/**
* Gets the max_line_length property if the `max-line-length` rule is enabled. Otherwise, returns [Int.MAX_VALUE].
*
* Normally, rules should not have direct dependencies on other rules. This rule is an exception to that. In case the `max-line-length`
* property in the `.editorconfig` is set, or inferred via a default value based on the `ktlint_code_style`, but the `max-line-length` rule
* is disabled, then those other rules might start wrapping lines. Conceptually, the `max-line-length` rule determines whether ktlint should
* or should not use the `max_line_length` property.
*/
public fun EditorConfig.maxLineLength(): Int =
if (maxLineLengthRuleEnabled()) {
this[MAX_LINE_LENGTH_PROPERTY]
} else {
Int.MAX_VALUE
}

private fun EditorConfig.maxLineLengthRuleEnabled(): Boolean =
RuleExecution.enabled ==
getEditorConfigValueOrNull(
RULE_EXECUTION_PROPERTY_TYPE,
MAX_LINE_LENGTH_RULE_ID.ktLintRuleExecutionPropertyName(),
)

public val MAX_LINE_LENGTH_RULE_ID: RuleId = MaxLineLengthRule().ruleId
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class ParameterListSpacingRule :
private var maxLineLength = MAX_LINE_LENGTH_PROPERTY.defaultValue

override fun beforeFirstNode(editorConfig: EditorConfig) {
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
}

override fun beforeVisitChildNodes(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public class ParameterListWrappingRule :

override fun beforeFirstNode(editorConfig: EditorConfig) {
codeStyle = editorConfig[CODE_STYLE_PROPERTY]
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
indentConfig =
IndentConfig(
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public class ParameterWrappingRule :
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
)
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
}

override fun beforeVisitChildNodes(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public class PropertyWrappingRule :
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
)
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
}

override fun beforeVisitChildNodes(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public class WrappingRule :
indentStyle = editorConfig[INDENT_STYLE_PROPERTY],
tabWidth = editorConfig[INDENT_SIZE_PROPERTY],
)
maxLineLength = editorConfig[MAX_LINE_LENGTH_PROPERTY]
maxLineLength = editorConfig.maxLineLength()
}

override fun beforeVisitChildNodes(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.pinterest.ktlint.ruleset.standard.rules.ClassSignatureRule.Companion.
class ArgumentListWrappingRuleTest {
private val argumentListWrappingRuleAssertThat =
assertThatRuleBuilder { ArgumentListWrappingRule() }
.addAdditionalRuleProvider { MaxLineLengthRule() }
.addRequiredRuleProviderDependenciesFrom(StandardRuleSetProvider())
.assertThat()

Expand Down Expand Up @@ -311,7 +312,7 @@ class ArgumentListWrappingRuleTest {
argumentListWrappingRuleAssertThat(code)
.setMaxLineLength()
.withEditorConfigOverride(IGNORE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY to 8)
.hasNoLintViolations()
.hasNoLintViolationsExceptInAdditionalRules()
}

@Test
Expand Down Expand Up @@ -942,8 +943,10 @@ class ArgumentListWrappingRuleTest {
.addAdditionalRuleProvider { ClassSignatureRule() }
.addRequiredRuleProviderDependenciesFrom(StandardRuleSetProvider())
.withEditorConfigOverride(CLASS_SIGNATURE_FORCE_MULTILINE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY to 4)
.hasLintViolationForAdditionalRule(5, 37, "Super type should start on a newline")
.hasLintViolations(
.hasLintViolationsForAdditionalRule(
LintViolation(5, 37, "Super type should start on a newline"),
LintViolation(5, 56, "Exceeded max line length (55)", canBeAutoCorrected = false),
).hasLintViolations(
LintViolation(5, 44, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(5, 47, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(5, 88, "Missing newline before \")\""),
Expand Down Expand Up @@ -977,6 +980,7 @@ class ArgumentListWrappingRuleTest {
.hasLintViolationsForAdditionalRule(
LintViolation(3, 12, "Newline expected after '{'"),
LintViolation(3, 12, "Missing newline after \"{\""),
LintViolation(3, 45, "Exceeded max line length (44)", canBeAutoCorrected = false),
LintViolation(3, 50, "Newline expected before '}'"),
// Lint violation below only occurs during linting. Resolving violations above, prevents the next violation from occurring
LintViolation(3, 59, "Line is exceeding max line length. Break line after 'returns' in binary expression"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ package com.pinterest.ktlint.ruleset.standard.rules
import com.pinterest.ktlint.ruleset.standard.StandardRuleSetProvider
import com.pinterest.ktlint.test.KtLintAssertThat.Companion.EOL_CHAR
import com.pinterest.ktlint.test.KtLintAssertThat.Companion.MAX_LINE_LENGTH_MARKER
import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRule
import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRuleBuilder
import com.pinterest.ktlint.test.LintViolation
import com.pinterest.ktlint.test.MULTILINE_STRING_QUOTE
import org.junit.jupiter.api.Test

class BinaryExpressionWrappingRuleTest {
private val binaryExpressionWrappingRuleAssertThat = assertThatRule { BinaryExpressionWrappingRule() }
private val binaryExpressionWrappingRuleAssertThat =
assertThatRuleBuilder { BinaryExpressionWrappingRule() }
.addAdditionalRuleProvider { MaxLineLengthRule() }
.assertThat()

@Test
fun `Given a property with a binary expression on same line as equals, and it exceeds the max line length then wrap before the expression`() {
Expand Down Expand Up @@ -192,7 +195,7 @@ class BinaryExpressionWrappingRuleTest {
""".trimIndent()
binaryExpressionWrappingRuleAssertThat(code)
.setMaxLineLength()
.hasNoLintViolations()
.hasNoLintViolationsExceptInAdditionalRules()
}

@Test
Expand Down Expand Up @@ -293,16 +296,20 @@ class BinaryExpressionWrappingRuleTest {
).hasLintViolationsForAdditionalRules(
LintViolation(2, 19, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(2, 32, "Missing newline before \")\""),
LintViolation(2, 32, "Exceeded max line length (31)", canBeAutoCorrected = false),
LintViolation(3, 19, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(3, 23, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(3, 32, "Missing newline before \")\""),
LintViolation(3, 32, "Exceeded max line length (31)", canBeAutoCorrected = false),
LintViolation(3, 33, "Missing newline before \")\""),
LintViolation(4, 19, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(4, 23, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(4, 32, "Exceeded max line length (31)", canBeAutoCorrected = false),
LintViolation(4, 46, "Missing newline before \")\""),
LintViolation(4, 47, "Missing newline before \")\""),
LintViolation(5, 19, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(5, 23, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(5, 32, "Exceeded max line length (31)", canBeAutoCorrected = false),
LintViolation(5, 47, "Missing newline before \")\""),
LintViolation(5, 48, "Missing newline before \")\""),
).isFormattedAs(formattedCode)
Expand Down Expand Up @@ -433,6 +440,7 @@ class BinaryExpressionWrappingRuleTest {
.hasLintViolationsForAdditionalRule(
LintViolation(3, 12, "Missing newline after \"{\""),
LintViolation(3, 21, "Argument should be on a separate line (unless all arguments can fit a single line)"),
LintViolation(3, 45, "Exceeded max line length (44)", canBeAutoCorrected = false),
LintViolation(3, 48, "Missing newline before \")\""),
).hasLintViolations(
LintViolation(3, 12, "Newline expected after '{'"),
Expand All @@ -455,7 +463,7 @@ class BinaryExpressionWrappingRuleTest {
""".trimIndent()
binaryExpressionWrappingRuleAssertThat(code)
.setMaxLineLength()
.hasNoLintViolations()
.hasNoLintViolationsExceptInAdditionalRules()
}

@Test
Expand Down Expand Up @@ -489,6 +497,6 @@ class BinaryExpressionWrappingRuleTest {
""".trimIndent()
binaryExpressionWrappingRuleAssertThat(code)
.setMaxLineLength()
.hasNoLintViolations()
.hasNoLintViolationsExceptInAdditionalRules()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import org.junit.jupiter.params.provider.ValueSource
class ChainMethodContinuationRuleTest {
private val chainMethodContinuationRuleAssertThat =
assertThatRuleBuilder { ChainMethodContinuationRule() }
.addAdditionalRuleProvider { MaxLineLengthRule() }
.addRequiredRuleProviderDependenciesFrom(StandardRuleSetProvider())
.assertThat()

Expand Down Expand Up @@ -613,6 +614,7 @@ class ChainMethodContinuationRuleTest {
.setMaxLineLength()
.addAdditionalRuleProvider { ArgumentListWrappingRule() }
.addAdditionalRuleProvider { FunctionLiteralRule() }
.addAdditionalRuleProvider { MaxLineLengthRule() }
.hasLintViolations(
// Lint violation below will not be triggered during format as argument-list-wrapping rule prevents this error from occurring
LintViolation(2, 30, "Expected newline before '.'"),
Expand Down Expand Up @@ -650,6 +652,7 @@ class ChainMethodContinuationRuleTest {
.addAdditionalRuleProvider { FunctionLiteralRule() }
.addAdditionalRuleProvider { MultilineExpressionWrappingRule() }
.addAdditionalRuleProvider { IndentationRule() }
.addAdditionalRuleProvider { MaxLineLengthRule() }
.isFormattedAs(formattedCode)
.hasLintViolations(
// Lint violation below will not be triggered during format as argument-list-wrapping rule prevents this error from occurring
Expand Down
Loading

0 comments on commit 4ec5072

Please sign in to comment.