Skip to content

Commit

Permalink
Loosen dependencies on value-argument-comment, `value-parameter-com…
Browse files Browse the repository at this point in the history
…ment`, `type-argument-comment`, and `type-parameter-comment` rules to be disabled (pinterest#2466)

Run `argument-list-wrapping`, `class-signature` and `function-signature` when comment rules above are disabled.

Closes pinterest#2445
  • Loading branch information
paul-dingemans committed Jan 2, 2024
1 parent 4127fce commit a6295cc
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 51 deletions.
64 changes: 34 additions & 30 deletions documentation/snapshot/docs/rules/standard.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -67,12 +67,12 @@ public class ClassSignatureRule :
setOf(
// Disallow comments at unexpected locations in the type parameter list
// class Foo<in /** some comment */ Bar>
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,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -63,15 +63,15 @@ public class FunctionSignatureRule :
setOf(
// Disallow comments at unexpected locations in the type parameter list
// fun </* some comment */ T> Foo<T>.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<out /* some comment */ Any>.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,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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 =
Expand Down

0 comments on commit a6295cc

Please sign in to comment.