From c1440aaa95c49d8472d8187f0d6db9b69ea9c305 Mon Sep 17 00:00:00 2001 From: Paul Dingemans Date: Wed, 27 Dec 2023 11:25:16 +0100 Subject: [PATCH] Loosen dependencies on `value-argument-comment`, `value-parameter-comment`, `type-argument-comment`, and `type-parameter-comment` rules to be disabled (#2466) Run `argument-list-wrapping`, `class-signature` and `function-signature` when comment rules above are disabled. Closes #2445 --- documentation/snapshot/docs/rules/standard.md | 64 ++++++++++--------- .../rules/ArgumentListWrappingRule.kt | 3 +- .../standard/rules/ClassSignatureRule.kt | 6 +- .../standard/rules/FunctionSignatureRule.kt | 8 +-- .../rules/ArgumentListWrappingRuleTest.kt | 22 +++---- .../rules/FunctionSignatureRuleTest.kt | 26 ++++++++ 6 files changed, 78 insertions(+), 51 deletions(-) diff --git a/documentation/snapshot/docs/rules/standard.md b/documentation/snapshot/docs/rules/standard.md index 6049bf39ff..1025073d08 100644 --- a/documentation/snapshot/docs/rules/standard.md +++ b/documentation/snapshot/docs/rules/standard.md @@ -2041,14 +2041,15 @@ Disallows comments to be placed at certain locations inside a type argument (lis > ``` -Note: Although code sample below might look ok, it is semantically and programmatically unclear to which element `some comment 1` refers. As of that comments are only allowed when starting on a separate line. -```kotlin -fun Foo< - out Bar1, // some comment 1 - // some comment 2 - out Bar2, // some comment +!!! note + In some projects it is an accepted practice to use EOL comments to document the parameter *before* the comma as is shown below: + ```kotlin + fun Foo< + out Bar1, // some comment + out Bar2, // some other comment >.foo() {} -``` + ``` + Although this code sample might look ok, it is semantically and programmatically unclear to which type `some comment` refers. From the developer perspective it might be clear that it belongs to type `Bar1`. From the parsers perspective, it does belong to type `Bar2`. Rule id: `type-argument-comment` (`standard` rule set) @@ -2082,14 +2083,15 @@ Disallows comments to be placed at certain locations inside a type parameter (li > ``` -Note: Although code sample below might look ok, it is semantically and programmatically unclear to which element `some comment 1` refers. As of that comments are only allowed when starting on a separate line. -```kotlin -class Foo< - out Bar1, // some comment 1 - // some comment 2 - out Bar2, // some comment +!!! note + In some projects it is an accepted practice to use EOL comments to document the parameter *before* the comma as is shown below: + ```kotlin + class Foo< + out Bar1, // some comment + out Bar2, // some other comment > -``` + ``` + Although this code sample might look ok, it is semantically and programmatically unclear on which parameter `some comment` refers. From the developer perspective it might be clear that it belongs to type `Bar1`. From the parsers perspective, it does belong to type `Bar2`. Rule id: `type-parameter-comment` (`standard` rule set) @@ -2142,14 +2144,15 @@ Disallows comments to be placed at certain locations inside a value argument (li ) ``` -Note: Although code sample below might look ok, it is semantically and programmatically unclear to which element `some comment 1` refers. As of that comments are only allowed when starting on a separate line. -```kotlin -class Foo< - out Bar1, // some comment 1 - // some comment 2 - out Bar2, // some comment - > -``` +!!! note + In a lot of projects it is an accepted practice to use EOL comments to document the parameter *before* the comma as is shown below: + ```kotlin + fun foo( + bar1: Bar1, // some comment + bar2: Bar2, // some other comment + ) + ``` + Although this code sample might look ok, it is semantically and programmatically unclear on which parameter `some comment` refers. From the developer perspective it might be clear that it belongs to parameter `bar1`. From the parsers perspective, it does belong to parameter `bar2`. This might lead to [unexpected behavior in Intellij IDEA](https://github.com/pinterest/ktlint/issues/2445#issuecomment-1863432022). Rule id: `value-argument-comment` (`standard` rule set) @@ -2190,14 +2193,15 @@ Disallows comments to be placed at certain locations inside a value argument (li ) ``` -Note: Although code sample below might look ok, it is semantically and programmatically unclear to which element `some comment 1` refers. As of that comments are only allowed when starting on a separate line. -```kotlin -class Foo( - bar: Bar1, // some comment 1 - // some comment 2 - bar2: Bar2, // some comment -) -``` +!!! note + In a lot of projects it is an accepted practice to use EOL comments to document the parameter *before* the comma as is shown below: + ```kotlin + class Foo( + bar1: Bar1, // some comment + bar2: Bar2, // some other comment + ) + ``` + Although this code sample might look ok, it is semantically and programmatically unclear on which parameter `some comment` refers. From the developer perspective it might be clear that it belongs to parameter `bar1`. From the parsers perspective, it does belong to parameter `bar2`. This might lead to [unexpected behavior in Intellij IDEA](https://github.com/pinterest/ktlint/issues/2445#issuecomment-1863432022). Rule id: `value-parameter-comment` (`standard` rule set) diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt index 087df09abb..fec961d5da 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt @@ -13,7 +13,6 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_ARGUMENT_LIST import com.pinterest.ktlint.rule.engine.core.api.ElementType.WHITE_SPACE import com.pinterest.ktlint.rule.engine.core.api.IndentConfig import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule -import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule.Mode.ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule.Mode.REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED import com.pinterest.ktlint.rule.engine.core.api.RuleId import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint @@ -64,7 +63,7 @@ public class ArgumentListWrappingRule : // class Foo( // bar /* some comment */: Bar // ) - RunAfterRule(VALUE_ARGUMENT_COMMENT_RULE_ID, ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED), + RunAfterRule(VALUE_ARGUMENT_COMMENT_RULE_ID, REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED), RunAfterRule(WRAPPING_RULE_ID, REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED), RunAfterRule(CLASS_SIGNATURE_RULE_ID, REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED), ), diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ClassSignatureRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ClassSignatureRule.kt index 526a31c1ea..96a27a96b4 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ClassSignatureRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ClassSignatureRule.kt @@ -21,7 +21,7 @@ import com.pinterest.ktlint.rule.engine.core.api.IndentConfig import com.pinterest.ktlint.rule.engine.core.api.IndentConfig.Companion.DEFAULT_INDENT_CONFIG import com.pinterest.ktlint.rule.engine.core.api.Rule import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule -import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule.Mode.ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED +import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule.Mode.REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAsLateAsPossible import com.pinterest.ktlint.rule.engine.core.api.RuleId import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint @@ -67,12 +67,12 @@ public class ClassSignatureRule : setOf( // Disallow comments at unexpected locations in the type parameter list // class Foo - RunAfterRule(TYPE_PARAMETER_COMMENT_RULE_ID, ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED), + RunAfterRule(TYPE_PARAMETER_COMMENT_RULE_ID, REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED), // Disallow comments at unexpected locations in the value parameter list // class Foo( // bar /* some comment */: Bar // ) - RunAfterRule(VALUE_PARAMETER_COMMENT_RULE_ID, ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED), + RunAfterRule(VALUE_PARAMETER_COMMENT_RULE_ID, REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED), // Run after wrapping and spacing rules RunAsLateAsPossible, ), diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionSignatureRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionSignatureRule.kt index b4a496476b..83099a838c 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionSignatureRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionSignatureRule.kt @@ -18,7 +18,7 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_PARAMETER_LIS import com.pinterest.ktlint.rule.engine.core.api.ElementType.WHITE_SPACE import com.pinterest.ktlint.rule.engine.core.api.IndentConfig import com.pinterest.ktlint.rule.engine.core.api.IndentConfig.Companion.DEFAULT_INDENT_CONFIG -import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule.Mode.ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED +import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule.Mode.REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED 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.EXPERIMENTAL @@ -63,15 +63,15 @@ public class FunctionSignatureRule : setOf( // Disallow comments at unexpected locations in the type parameter list // fun Foo.foo() {} - VisitorModifier.RunAfterRule(TYPE_PARAMETER_COMMENT_RULE_ID, ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED), + VisitorModifier.RunAfterRule(TYPE_PARAMETER_COMMENT_RULE_ID, REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED), // Disallow comments at unexpected locations in the type argument list // fun Foo.foo() {} - VisitorModifier.RunAfterRule(TYPE_ARGUMENT_COMMENT_RULE_ID, ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED), + VisitorModifier.RunAfterRule(TYPE_ARGUMENT_COMMENT_RULE_ID, REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED), // Disallow comments at unexpected locations in the value parameter list // fun foo( // bar /* some comment */: Bar // ) - VisitorModifier.RunAfterRule(VALUE_PARAMETER_COMMENT_RULE_ID, ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED), + VisitorModifier.RunAfterRule(VALUE_PARAMETER_COMMENT_RULE_ID, REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED), // Run after wrapping and spacing rules VisitorModifier.RunAsLateAsPossible, ), diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRuleTest.kt index 557423ef24..52744a7e6c 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRuleTest.kt @@ -901,24 +901,22 @@ class ArgumentListWrappingRuleTest { } @Test - fun `Given a value argument with a disallowed comment`() { + fun `Issue 2445 - Given a value argument followed by EOL comment after comma`() { val code = """ - // $MAX_LINE_LENGTH_MARKER $EOL_CHAR - val foo = foo(bar = /* some disallowed comment location */ "bar") - """.trimIndent() - val formattedCode = - """ - // $MAX_LINE_LENGTH_MARKER $EOL_CHAR val foo = foo( - bar = /* some disallowed comment location */ "bar" + bar1 = "bar1", // some comment 1 + bar2 = "bar2", // some comment 2 ) """.trimIndent() @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length") argumentListWrappingRuleAssertThat(code) - .setMaxLineLength() - .withEditorConfigOverride(FORCE_MULTILINE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY to 1) - .hasLintViolationForAdditionalRule(2, 21, "A (block or EOL) comment inside or on same line after a 'value_argument' is not allowed. It may be placed on a separate line above.", false) - .isFormattedAs(formattedCode) + .addAdditionalRuleProvider { ValueArgumentCommentRule() } + .hasLintViolationsForAdditionalRule( + LintViolation(2, 20, "A comment in a 'value_argument_list' is only allowed when placed on a separate line", false), + LintViolation(3, 20, "A comment in a 'value_argument_list' is only allowed when placed on a separate line", false), + ) + // When ValueArgumentCommentRule is not loaded or enabled + argumentListWrappingRuleAssertThat(code).hasNoLintViolations() } } diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionSignatureRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionSignatureRuleTest.kt index ff3c50ad45..f83693346b 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionSignatureRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionSignatureRuleTest.kt @@ -226,6 +226,7 @@ class FunctionSignatureRuleTest { @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length") functionSignatureWrappingRuleAssertThat(code) .setMaxLineLength() + .addAdditionalRuleProvider { ValueParameterCommentRule() } .hasLintViolationsForAdditionalRule( LintViolation(5, 17, "A comment in a 'value_parameter_list' is only allowed when placed on a separate line", false), LintViolation(6, 18, "A (block or EOL) comment inside or on same line after a 'value_parameter' is not allowed. It may be placed on a separate line above.", false), @@ -235,6 +236,31 @@ class FunctionSignatureRuleTest { ).hasNoLintViolationsExceptInAdditionalRules() } + @Test + fun `Issue 2445 - Given value-parameter-comment rule is disabled or not loaded`() { + val code = + """ + private fun f5(a /* some comment */: Any, b: Any): String = "some-result" + private fun f6(a: /* some comment */ Any, b: Any): String = "some-result" + private fun f7(a: Any /* some comment */, b: Any): String = "some-result" + private fun f11( + a: Any, // some-comment + b: Any + ): String = "f10" + """.trimIndent() + @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length") + functionSignatureWrappingRuleAssertThat(code) + .addAdditionalRuleProvider { ValueParameterCommentRule() } + .hasLintViolationsForAdditionalRule( + LintViolation(1, 18, "A (block or EOL) comment inside or on same line after a 'value_parameter' is not allowed. It may be placed on a separate line above.", false), + LintViolation(2, 19, "A (block or EOL) comment inside or on same line after a 'value_parameter' is not allowed. It may be placed on a separate line above.", false), + LintViolation(3, 23, "A (block or EOL) comment inside or on same line after a 'value_parameter' is not allowed. It may be placed on a separate line above.", false), + LintViolation(5, 13, "A comment in a 'value_parameter_list' is only allowed when placed on a separate line", false), + ).hasNoLintViolationsExceptInAdditionalRules() + // When ValueParameterCommentRule is not loaded or disabled: + functionSignatureWrappingRuleAssertThat(code).hasNoLintViolations() + } + @Test fun `Given a function signature with a newline between the last parameter in the parameter list and the closing parenthesis, but which does not fit on a single line then reformat to a proper multiline signature`() { val code =