Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run argument-list-wrapping, class-signature and function-signature when comment rules are disabled #2466

Merged
merged 1 commit into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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