Remove dependencies on discouraged-comment-location rule (#2371)

Rule `discouraged-comment-location` intended to make implementation of rules easier. By disallowing those locations, the rules did not need to take them into accound.

Disabling the `discouraged-comment-location` rule however also forced users to disable other rules as well (for example see #2338). With the number of rules that depended on `discouraged-comment-location` and the number of locations which were forbidden, this became more and more a useability problem for certain users. Disabling the `discouraged-comment-location` rule for one specific type of comment location was not possible. So only choiche was to disable the entire rule and all rules that depend on it.

The `discouraged-comment-location`  rule still exists to avoid a breaking change. But it no longer provides any functionality anymore. The logic of this rule has been moved to normal rule classes in case a discouraged comment location was added for one specific rule only. Comment locations that are more generic (e.g. used by at least two different rules) are moved to separate rules as this provides more clarity about rule dependencies.

Rules below no longer depend on any other rule to handle disallowed comment location. If needed, the rules provide custom checks:
* `chain-method-continuation`
* `if-else-wrapping`

New rules below are added to check for disallowed comment locations in constructs that are used by multiple rules:
* `type-argument/type-argument-list`
* `type-projection/type-parameter-list`
* `value-argument/value-argument-list`
* `value-parameter/value-parameter-list`
Rules that previously relied explicitly or implicitly on the `discouraged-comment-location` rule now explicitly depend on rules above. If rules above are disabled, it is now more clear why a rule has a dependency on that rule as it is more specific.

To provide above, following functionality was added as well:
* Add utility methods `afterCodeSibling`, `beforeCodeSibling` and `betweenCodeSiblings` to ASTNodeExtensions
* KtlintAssertThat now provides a more consistent interface for building rule assertion function which are depending on another rule. For some rules, the editorconfig properties regarding setting code style to `ktlint_official` have been removed as this code style is by default.

Closes #2367

documentation/snapshot/docs/rules/standard.md

@@ -128,27 +128,6 @@ Lines in a block comment which (exclusive the indentation) start with a `*` shou
 
 Rule id: `block-comment-initial-star-alignment` (`standard` rule set)
 
-## Discouraged comment location
-
-Detect discouraged comment locations (no autocorrect).
-
-!!! note
- Kotlin allows comments to be placed almost everywhere. As this can lead to code which is hard to read, most of them will never be used in practice. Ideally each rule takes comments at all possible locations into account. Sometimes this is really hard and not worth the effort. By explicitly forbidding such comment locations, the development of those rules becomes a bit easier.
-
-=== "[:material-heart-off-outline:](#) Disallowed"
-
- ```kotlin
- fun <T> /* some comment */ foo(t: T) = "some-result"
-
- fun foo() {
-  if (true)
-  // some comment
-  bar()
- }
- ```
-
-Rule id: `discouraged-comment-location` (`standard` rule set)
-
 ## Enum entry
 
 Enum entry names should be uppercase underscore-separated or upper camel-case separated.
@@ -2022,6 +2001,89 @@ Consistent removal (default) or adding of trailing commas on declaration site.
 
 Rule id: `trailing-comma-on-declaration-site` (`standard` rule set)
 
+## Type argument comment
+
+Disallows comments to be placed at certain locations inside a type argument (list). A KDoc is not allowed.
+
+=== "[:material-heart:](#) Ktlint"
+
+ ```kotlin
+ fun Foo<
+  /* some comment */ 
+  out Any
+  >.foo() {}
+ fun Foo<
+  // some comment 
+  out Any
+  >.foo() {}
+ ```
+
+=== "[:material-heart-off-outline:](#) Disallowed"
+
+ ```kotlin
+ fun Foo<out /* some comment */ Any>.foo() {}
+ fun Foo<
+  out Any, // some comment
+  >.foo() {}
+ val fooBar: FooBar<
+  /** some comment */
+  Foo,
+  Bar
+  >
+ ```
+
+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
+ >.foo() {}
+```
+
+Rule id: `type-argument-comment` (`standard` rule set)
+
+## Type parameter comment
+
+Disallows comments to be placed at certain locations inside a type parameter (list). A KDoc is not allowed.
+
+=== "[:material-heart:](#) Ktlint"
+
+ ```kotlin
+ class Foo2<
+  /* some comment */ 
+  out Bar
+  >
+ class Foo3<
+  // some comment 
+  out Bar
+  >
+ ```
+
+=== "[:material-heart-off-outline:](#) Disallowed"
+
+ ```kotlin
+ class Foo1<
+  /** some comment */
+  in Bar
+  >
+ class Foo2<in /* some comment */ Bar>
+ class Foo3<
+  in Bar, // some comment
+  >
+ ```
+
+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
+ >
+```
+
+Rule id: `type-parameter-comment` (`standard` rule set)
+
 ## Unnecessary parenthesis before trailing lambda
 
 An empty parentheses block before a lambda is redundant.
@@ -2040,6 +2102,96 @@ An empty parentheses block before a lambda is redundant.
 
 Rule id: `unnecessary-parentheses-before-trailing-lambda` (`standard` rule set)
 
+## Value argument comment
+
+Disallows comments to be placed at certain locations inside a value argument (list). A KDoc is not allowed.
+
+=== "[:material-heart:](#) Ktlint"
+
+ ```kotlin
+ val foo2 =
+  foo(
+  /* some comment */
+  bar = "bar"
+  )
+ val foo3 =
+  foo(
+  // some comment
+  bar = "bar"
+  )
+ ```
+
+=== "[:material-heart-off-outline:](#) Disallowed"
+
+ ```kotlin
+ val foo1 = foo(bar /** some comment */ = "bar")
+ val foo2 = foo(bar /* some comment */ = "bar")
+ val foo3 = 
+  foo(
+  bar = // some comment
+  "bar"
+  )
+ ```
+
+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
+ >
+```
+
+Rule id: `value-argument-comment` (`standard` rule set)
+
+## Value parameter comment
+
+Disallows comments to be placed at certain locations inside a value argument (list). A KDoc is allowed but must start on a separate line.
+
+=== "[:material-heart:](#) Ktlint"
+
+ ```kotlin
+ class Foo1(
+  /** some comment */
+  bar = "bar"
+ )
+ class Foo2(
+  /* some comment */
+  bar = "bar"
+ )
+ class Foo3(
+  // some comment
+  bar = "bar"
+ )
+ ```
+
+=== "[:material-heart-off-outline:](#) Disallowed"
+
+ ```kotlin
+ class Foo2(
+  bar /** some comment */ = "bar"
+ )
+ class Foo2(
+  bar = /* some comment */ "bar"
+ )
+ class Foo3(
+  bar =
+  // some comment
+  "bar"
+ )
+ ```
+
+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
+)
+```
+
+Rule id: `value-parameter-comment` (`standard` rule set)
+
 ## Wrapping
 
 ### Argument list wrapping

ktlint-rule-engine-core/api/ktlint-rule-engine-core.api

@@ -1,4 +1,7 @@
 public final class com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtensionKt {
+ public static final fun afterCodeSibling (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Lorg/jetbrains/kotlin/com/intellij/psi/tree/IElementType;)Z
+ public static final fun beforeCodeSibling (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Lorg/jetbrains/kotlin/com/intellij/psi/tree/IElementType;)Z
+ public static final fun betweenCodeSiblings (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Lorg/jetbrains/kotlin/com/intellij/psi/tree/IElementType;Lorg/jetbrains/kotlin/com/intellij/psi/tree/IElementType;)Z
  public static final fun children (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;)Lkotlin/sequences/Sequence;
  public static final fun findCompositeParentElementOfType (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Lorg/jetbrains/kotlin/com/intellij/psi/tree/IElementType;)Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;
  public static final fun firstChildLeafOrSelf (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;)Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;

ktlint-rule-engine-core/src/main/kotlin/com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtension.kt

@@ -470,3 +470,12 @@ public fun Sequence<ASTNode>.lineLengthWithoutNewlinePrefix(): Int {
  .takeWhile { it != '\n' }
  .length
 }
+
+public fun ASTNode.afterCodeSibling(afterElementType: IElementType): Boolean = prevCodeSibling()?.elementType == afterElementType
+
+public fun ASTNode.beforeCodeSibling(beforeElementType: IElementType): Boolean = nextCodeSibling()?.elementType == beforeElementType
+
+public fun ASTNode.betweenCodeSiblings(
+ afterElementType: IElementType,
+ beforeElementType: IElementType,
+): Boolean = afterCodeSibling(afterElementType) && beforeCodeSibling(beforeElementType)

ktlint-rule-engine-core/src/test/kotlin/com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtensionTest.kt

@@ -7,6 +7,7 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLASS
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLASS_BODY
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.ENUM_ENTRY
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN_KEYWORD
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.IDENTIFIER
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.LPAR
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.MODIFIER_LIST
@@ -22,6 +23,8 @@ import org.jetbrains.kotlin.com.intellij.lang.FileASTNode
 import org.jetbrains.kotlin.psi.psiUtil.leaves
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.ValueSource
 import kotlin.reflect.KFunction1
 
 class ASTNodeExtensionTest {
@@ -722,6 +725,67 @@ class ASTNodeExtensionTest {
  )
  }
 
+ @ParameterizedTest(name = "Text between FUN_KEYWORD and IDENTIFIER: {0}")
+ @ValueSource(
+ strings = [
+ " ",
+ "\n",
+ "/* some comment*/",
+ "// some EOL comment\n",
+ ],
+ )
+ fun `Given a function declaration then the IDENTIFIER should be after the FUN_KEYWORD element type`(separator: String) {
+ val code =
+ """
+ fun${separator}foo() = 42
+ """.trimIndent()
+ val actual =
+ transformCodeToAST(code)
+ .findChildByType(FUN)
+ ?.findChildByType(IDENTIFIER)
+ ?.afterCodeSibling(FUN_KEYWORD)
+
+ assertThat(actual).isTrue()
+ }
+
+ @ParameterizedTest(name = "Text between FUN_KEYWORD and IDENTIFIER: {0}")
+ @ValueSource(
+ strings = [
+ " ",
+ "\n",
+ "/* some comment*/",
+ "// some EOL comment\n",
+ ],
+ )
+ fun `Given a function declaration then the FUN_KEYWORD should be before the IDENTIFIER element type`(separator: String) {
+ val code =
+ """
+ fun${separator}foo() = 42
+ """.trimIndent()
+ val actual =
+ transformCodeToAST(code)
+ .findChildByType(FUN)
+ ?.findChildByType(FUN_KEYWORD)
+ ?.beforeCodeSibling(IDENTIFIER)
+
+ assertThat(actual).isTrue()
+ }
+
+ @Test
+ fun `Given a function declaration then the IDENTIFIER should be between the FUN_KEYWORD and the VALUE_PARAMETER_LIST element type`() {
+ val code =
+ """
+ fun foo() = 42
+ """.trimIndent()
+ val actual =
+ transformCodeToAST(code)
+ .findChildByType(FUN)
+ ?.findChildByType(IDENTIFIER)
+ ?.betweenCodeSiblings(FUN_KEYWORD, VALUE_PARAMETER_LIST)
+
+ assertThat(actual).isTrue()
+ }
+
  private inline fun String.transformAst(block: FileASTNode.() -> Unit): FileASTNode =
  transformCodeToAST(this)
  .apply(block)

ktlint-rule-engine/src/test/kotlin/com/pinterest/ktlint/rule/engine/internal/rules/KtlintSuppressionRuleTest.kt

@@ -2,11 +2,11 @@ package com.pinterest.ktlint.rule.engine.internal.rules
 
 import com.pinterest.ktlint.rule.engine.core.api.Rule
 import com.pinterest.ktlint.rule.engine.core.api.RuleId
-import com.pinterest.ktlint.rule.engine.core.api.RuleProvider
+import com.pinterest.ktlint.ruleset.standard.StandardRuleSetProvider
 import com.pinterest.ktlint.ruleset.standard.rules.ArgumentListWrappingRule
 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.KtlintDocumentationTest
 import com.pinterest.ktlint.test.LintViolation
 import org.junit.jupiter.api.Nested
@@ -17,25 +17,21 @@ import org.junit.jupiter.params.provider.ValueSource
 
 class KtlintSuppressionRuleTest {
  private val ktlintSuppressionRuleAssertThat =
- assertThatRule(
- provider = { KtlintSuppressionRule(emptyList()) },
- additionalRuleProviders =
- setOf(
- // Create a dummy rule for each rule id that is used in a ktlint directive or suppression in the tests in this
- // class. If no rule provider is added for the rule id, a lint violation is thrown which will bloat the tests too
- // much.
- //
- // Ids of real rules used but for which the real implementation is unwanted as it would modify the formatted code
- RuleProvider { DummyRule("standard:no-wildcard-imports") },
- RuleProvider { DummyRule("standard:no-multi-spaces") },
- RuleProvider { DummyRule("standard:max-line-length") },
- RuleProvider { DummyRule("standard:package-name") },
- // Ids of fake rules in a custom and the standard rule set
- RuleProvider { DummyRule("custom:foo") },
- RuleProvider { DummyRule("standard:bar") },
- RuleProvider { DummyRule("standard:foo") },
- ),
- )
+ assertThatRuleBuilder { KtlintSuppressionRule(emptyList()) }
+ // Create a dummy rule for each rule id that is used in a ktlint directive or suppression in the tests in this
+ // class. If no rule provider is added for the rule id, a lint violation is thrown which will bloat the tests too
+ // much.
+ //
+ // Ids of real rules used but for which the real implementation is unwanted as it would modify the formatted code
+ .addAdditionalRuleProvider { DummyRule("standard:no-wildcard-imports") }
+ .addAdditionalRuleProvider { DummyRule("standard:no-multi-spaces") }
+ .addAdditionalRuleProvider { DummyRule("standard:max-line-length") }
+ .addAdditionalRuleProvider { DummyRule("standard:package-name") }
+ // Ids of fake rules in a custom and the standard rule set
+ .addAdditionalRuleProvider { DummyRule("custom:foo") }
+ .addAdditionalRuleProvider { DummyRule("standard:bar") }
+ .addAdditionalRuleProvider { DummyRule("standard:foo") }
+ .assertThat()
 
  @Nested
  inner class `Given a suppression annotation missing the rule set id prefix` {
@@ -872,6 +868,7 @@ class KtlintSuppressionRuleTest {
  """.trimIndent()
  ktlintSuppressionRuleAssertThat(code)
  .addAdditionalRuleProvider { ArgumentListWrappingRule() }
+ .addRequiredRuleProviderDependenciesFrom(StandardRuleSetProvider())
  .hasLintViolations(
  LintViolation(1, 4, "Directive 'ktlint-disable' is deprecated. Replace with @Suppress annotation"),
  LintViolation(4, 54, "Directive 'ktlint-disable' is deprecated. Replace with @Suppress annotation"),
@@ -1359,6 +1356,7 @@ class KtlintSuppressionRuleTest {
  ktlintSuppressionRuleAssertThat(code)
  .setMaxLineLength()
  .addAdditionalRuleProvider { ArgumentListWrappingRule() }
+ .addRequiredRuleProviderDependenciesFrom(StandardRuleSetProvider())
  .hasLintViolations(
  LintViolation(8, 12, "Directive 'ktlint-disable' is deprecated. Replace with @Suppress annotation"),
  LintViolation(10, 12, "Directive 'ktlint-enable' is obsolete after migrating to suppress annotations"),