Enable non-experimental rule sets by default (#1753)

* Enable non-experimental rule sets (including custom rule sets) by default. There is no benefit in forcing the Ktlint CLI and API Consumers to enable the rule sets for their rules explicitly as the developer already consented to using their rules by either specifying a custom rule set JAR via the CLI or by using the API Consumer. However, the benefit of being able to disable a rule set still remains. Closes #1747
pinterest · Dec 31, 2022 · bc0880d · bc0880d
1 parent ed1019c
commit bc0880d
Show file tree

Hide file tree

Showing 16 changed files with 69 additions and 60 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,7 +14,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 * Add API so that KtLint API consumer is able to process a Kotlin script snippet without having to specify a file path ([#1738](https://github.com/pinterest/ktlint/issues/1738))
 * Disable the `standard:filename` rule whenever Ktlint CLI is run with option `--stdin` ([#1742](https://github.com/pinterest/ktlint/issues/1742))
 * Fix initialization of the logger when `--log-level` is specified. Throw exception when an invalid value is passed. ([#1749](https://github.com/pinterest/ktlint/issues/1749))
-* Fix loading of custom rule set JARs
+* Fix loading of custom rule set JARs.
+* Rules provided via a custom rule set JAR (Ktlint CLI) or by an API provider are enabled by default. Only rules in the `experimental` rule set are disabled by default. ([#1747](https://github.com/pinterest/ktlint/issues/1747))
 
 ### Changed
 

diff --git a/docs/faq.md b/docs/faq.md
@@ -150,7 +150,7 @@ ktlint_your-custom-rule-set_custom-rule = enabled # Enable all rules in the `cus
 ```
 
 !!! note
-    All rules from the `standard` rule set are *enabled* by default and can optionally be disabled in the `.editorconfig`. All rules from the `experimental` and *custom* rule sets are *disabled* by default and can optionally be enabled in the `.editorconfig`.
+    All rules from the `standard` and custom rule sets are *enabled* by default and can optionally be disabled in the `.editorconfig`. All rules from the `experimental` rule set are *disabled* by default and can optionally be enabled in the `.editorconfig`.
 
 An individual property can be enabled or disabled with a rule property. The name of the rule property consists of the `ktlint_` prefix followed by the rule set id followed by a `_` and the rule id. Examples:
 ```editorconfig

diff --git a/docs/rules/configuration-ktlint.md b/docs/rules/configuration-ktlint.md
@@ -36,7 +36,7 @@ ktlint_your-custom-rule-set_custom-rule = enabled # Enable all rules in the `cus
 ```
 
 !!! note
-    All rules from the `standard` rule set are *enabled* by default and can optionally be disabled in the `.editorconfig`. All rules from the `experimental` and *custom* rule sets are *disabled* by default and can optionally be enabled in the `.editorconfig`.
+    Rules from the `experimental` rule set are *disabled* by default. Either the entire rule set or individual rules from this rule set have to be enabled explicitly. All rules from the `standard` and custom rule sets are *enabled* by default and can optionally be disabled in the `.editorconfig`.
 
 An individual property can be enabled or disabled with a rule property. The name of the rule property consists of the `ktlint_` prefix followed by the rule set id followed by a `_` and the rule id. Examples:
 ```editorconfig

diff --git a/...i-consumer/src/main/kotlin/com/example/ktlint/api/consumer/rules/CustomRuleSetProvider.kt b/...i-consumer/src/main/kotlin/com/example/ktlint/api/consumer/rules/CustomRuleSetProvider.kt
@@ -3,6 +3,8 @@ package com.example.ktlint.api.consumer.rules
 import com.pinterest.ktlint.core.RuleProvider
 import com.pinterest.ktlint.ruleset.standard.IndentationRule
 
+internal val CUSTOM_RULE_SET_ID = "custom-rule-set-id"
+
 internal val KTLINT_API_CONSUMER_RULE_PROVIDERS = setOf(
     // Can provide custom rules
     RuleProvider { NoVarRule() },

diff --git a/ktlint-api-consumer/src/main/kotlin/com/example/ktlint/api/consumer/rules/NoVarRule.kt b/ktlint-api-consumer/src/main/kotlin/com/example/ktlint/api/consumer/rules/NoVarRule.kt
@@ -4,7 +4,7 @@ import com.pinterest.ktlint.core.Rule
 import com.pinterest.ktlint.core.ast.ElementType.VAR_KEYWORD
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 
-public class NoVarRule : Rule("no-var") {
+public class NoVarRule : Rule("$CUSTOM_RULE_SET_ID:no-var") {
     override fun beforeVisitChildNodes(
         node: ASTNode,
         autoCorrect: Boolean,

diff --git a/...nt-api-consumer/src/test/kotlin/com/pinterest/ktlint/api/consumer/KtLintRuleEngineTest.kt b/...nt-api-consumer/src/test/kotlin/com/pinterest/ktlint/api/consumer/KtLintRuleEngineTest.kt
@@ -1,5 +1,6 @@
 package com.pinterest.ktlint.api.consumer
 
+import com.example.ktlint.api.consumer.rules.NoVarRule
 import com.pinterest.ktlint.core.Code
 import com.pinterest.ktlint.core.KtLintRuleEngine
 import com.pinterest.ktlint.core.LintError
@@ -13,7 +14,7 @@ import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.io.TempDir
 
 /**
- * The KtLintRuleEngine is use by the Ktlint CLI and external API Consumers. Although most functionalities of the RuleEngine are already
+ * The KtLintRuleEngine is used by the Ktlint CLI and external API Consumers. Although most functionalities of the RuleEngine are already
  * tested via the Ktlint CLI Tests and normal unit tests in KtLint Core, some functionalities need additional testing from the perspective
  * of an API Consumer to ensure that the API is usable and stable across releases.
  */
@@ -66,7 +67,7 @@ class KtLintRuleEngineTest {
         }
 
         @Test
-        fun `Givens a kotlin script code snippet that does not contain an error`() {
+        fun `Given a kotlin script code snippet that does not contain an error`() {
             val ktLintRuleEngine = KtLintRuleEngine(
                 ruleProviders = setOf(
                     RuleProvider { IndentationRule() },
@@ -89,6 +90,27 @@ class KtLintRuleEngineTest {
 
             assertThat(lintErrors).isEmpty()
         }
+
+        @Test
+        fun `Given a code snippet that violates a custom rule prefixed by a rule set id`() {
+            val ktLintRuleEngine = KtLintRuleEngine(
+                ruleProviders = setOf(
+                    RuleProvider { NoVarRule() },
+                ),
+            )
+
+            val lintErrors = mutableListOf<LintError>()
+            ktLintRuleEngine.lint(
+                code = Code.CodeSnippet(
+                    """
+                    var foo = "foo"
+                    """.trimIndent(),
+                ),
+                callback = { lintErrors.add(it) },
+            )
+
+            assertThat(lintErrors).isNotEmpty
+        }
     }
 
     @Nested

diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/VisitorProvider.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/internal/VisitorProvider.kt
@@ -114,7 +114,7 @@ internal class VisitorProvider(
                 )
 
             else ->
-                ruleSetId(qualifiedRuleId) == "standard"
+                ruleSetId(qualifiedRuleId) != "experimental"
         }
 
     private fun EditorConfigProperties.isRuleEnabled(qualifiedRuleId: String) =
@@ -125,14 +125,12 @@ internal class VisitorProvider(
     private fun EditorConfigProperties.isRuleSetEnabled(qualifiedRuleId: String) =
         ruleExecution(ktLintRuleSetExecutionPropertyName(qualifiedRuleId))
             .let { ruleSetExecution ->
-                if (ruleSetExecution.name == "ktlint_standard") {
-                    // Rules in the standard rule set are enabled by default. So those rule should run unless the rule set
-                    // is disabled explicitly.
-                    ruleSetExecution != RuleExecution.disabled
-                } else {
-                    // Rules in non-standard rule set are disabled by default. So rules may only run when the rule set is
-                    // enabled explicitly.
+                if (ruleSetExecution.name == "ktlint_experimental") {
+                    // Rules in the experimental rule set are only run when enabled explicitly.
                     ruleSetExecution == RuleExecution.enabled
+                } else {
+                    // Rules in other rule sets are enabled by default.
+                    ruleSetExecution != RuleExecution.disabled
                 }
             }
 

diff --git a/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/VisitorProviderTest.kt b/ktlint-core/src/test/kotlin/com/pinterest/ktlint/core/VisitorProviderTest.kt
@@ -293,20 +293,31 @@ class VisitorProviderTest {
         }
 
         @Test
-        fun `Given that a non-standard rule set is not disabled explicitly then only run rules that are enabled explicitly`() {
+        fun `Given that the experimental rule set is not enabled explicitly then only run experimental rules that are enabled explicitly`() {
             val actual = testVisitorProvider(
                 RuleProvider { NormalRule("$EXPERIMENTAL:$RULE_B") },
                 RuleProvider { NormalRule("$EXPERIMENTAL:$RULE_C") },
-                RuleProvider { NormalRule("$CUSTOM_RULE_SET_A:$RULE_B") },
-                RuleProvider { NormalRule("$CUSTOM_RULE_SET_A:$RULE_C") },
                 editorConfigProperties = mapOf(
                     ktLintRuleExecutionEditorConfigProperty("ktlint_$EXPERIMENTAL:$RULE_B", RuleExecution.enabled),
-                    ktLintRuleExecutionEditorConfigProperty("ktlint_$CUSTOM_RULE_SET_A:$RULE_B", RuleExecution.enabled),
                 ),
             )
 
             assertThat(actual).containsExactly(
                 Visit(EXPERIMENTAL, RULE_B),
+            )
+        }
+
+        @Test
+        fun `Given that the custom rule set is not enabled explicitly then run all rules that are not disabled explicitly`() {
+            val actual = testVisitorProvider(
+                RuleProvider { NormalRule("$CUSTOM_RULE_SET_A:$RULE_B") },
+                RuleProvider { NormalRule("$CUSTOM_RULE_SET_A:$RULE_C") },
+                editorConfigProperties = mapOf(
+                    ktLintRuleExecutionEditorConfigProperty("ktlint_$CUSTOM_RULE_SET_A:$RULE_C", RuleExecution.disabled),
+                ),
+            )
+
+            assertThat(actual).containsExactly(
                 Visit(CUSTOM_RULE_SET_A, RULE_B),
             )
         }

diff --git a/ktlint-ruleset-template/src/main/kotlin/yourpkgname/CustomRuleSetProvider.kt b/ktlint-ruleset-template/src/main/kotlin/yourpkgname/CustomRuleSetProvider.kt
@@ -3,9 +3,11 @@ package yourpkgname
 import com.pinterest.ktlint.core.RuleProvider
 import com.pinterest.ktlint.core.RuleSetProviderV2
 
+internal val CUSTOM_RULE_SET_ID = "custom-rule-set-id"
+
 public class CustomRuleSetProvider :
     RuleSetProviderV2(
-        id = "custom",
+        id = CUSTOM_RULE_SET_ID,
         about = About(
             maintainer = "KtLint",
             description = "Example of a custom rule set",

diff --git a/ktlint-ruleset-template/src/main/kotlin/yourpkgname/NoVarRule.kt b/ktlint-ruleset-template/src/main/kotlin/yourpkgname/NoVarRule.kt
@@ -4,8 +4,7 @@ import com.pinterest.ktlint.core.Rule
 import com.pinterest.ktlint.core.ast.ElementType.VAR_KEYWORD
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 
-public class NoVarRule : Rule("no-var") {
-
+public class NoVarRule : Rule("$CUSTOM_RULE_SET_ID:no-var") {
     override fun beforeVisitChildNodes(
         node: ASTNode,
         autoCorrect: Boolean,

diff --git a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/GenerateEditorConfigSubCommand.kt b/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/GenerateEditorConfigSubCommand.kt
@@ -29,15 +29,8 @@ internal class GenerateEditorConfigSubCommand : Runnable {
     override fun run() {
         commandSpec.commandLine().printCommandLineHelpOrVersionUsage()
 
-        val ruleProviders =
-            ktlintCommand
-                .ruleProvidersByRuleSetId()
-                .values
-                .flatten()
-                .toSet()
-
         val ktLintRuleEngine = KtLintRuleEngine(
-            ruleProviders = ruleProviders,
+            ruleProviders = ktlintCommand.ruleProviders(),
             editorConfigOverride = EditorConfigOverride.from(CODE_STYLE_PROPERTY to codeStyle()),
             isInvokedFromCli = true,
         )

diff --git a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/KtlintCommandLine.kt b/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/KtlintCommandLine.kt
@@ -270,14 +270,6 @@ internal class KtlintCommandLine {
             exitKtLintProcess(1)
         }
 
-        val ruleProvidersByRuleSetId = ruleProvidersByRuleSetId()
-        val customRuleSetIds =
-            ruleProvidersByRuleSetId
-                .filterKeys {
-                    // Exclude the standard and experimental rule sets from Ktlint itself
-                    it != "standard" && it != "experimental"
-                }.map { it.key }
-
         val editorConfigOverride = EditorConfigOverride
             .EMPTY_EDITOR_CONFIG_OVERRIDE
             .applyIf(experimental) {
@@ -292,13 +284,6 @@ internal class KtlintCommandLine {
             }.applyIf(stdin) {
                 logger.debug { "Add editor config override to disable 'filename' rule which can not be used in combination with reading from <stdin>" }
                 plus(createRuleExecutionEditorConfigProperty("standard:filename") to RuleExecution.disabled)
-            }.applyIf(customRuleSetIds.isNotEmpty()) {
-                logger.debug { "Add editor config override to enable rule set(s) '$customRuleSetIds' from custom rule set JAR('s): '$rulesetJarPaths'" }
-                val ruleSetExecutionEditorConfigProperties =
-                    customRuleSetIds
-                        .map { createRuleSetExecutionEditorConfigProperty("$it:all") to RuleExecution.enabled }
-                        .toTypedArray()
-                plus(*ruleSetExecutionEditorConfigProperties)
             }
 
         assertStdinAndPatternsFromStdinOptionsMutuallyExclusive()
@@ -317,14 +302,8 @@ internal class KtlintCommandLine {
 
         var reporter = loadReporter()
 
-        val ruleProviders =
-            ruleProvidersByRuleSetId
-                .values
-                .flatten()
-                .toSet()
-
         val ktLintRuleEngine = KtLintRuleEngine(
-            ruleProviders = ruleProviders,
+            ruleProviders = ruleProviders(),
             editorConfigDefaults = editorConfigDefaults,
             editorConfigOverride = editorConfigOverride,
             isInvokedFromCli = true,
@@ -372,10 +351,10 @@ internal class KtlintCommandLine {
     }
 
     // Do not convert to "val" as the function depends on PicoCli options which are not fully instantiated until the "run" method is started
-    internal fun ruleProvidersByRuleSetId(): Map<String, Set<RuleProvider>> =
+    internal fun ruleProviders(): Set<RuleProvider> =
         rulesetJarPaths
             .toFilesURIList()
-            .loadRuleProvidersByRuleSetId(debug)
+            .loadRuleProviders(debug)
 
     // Do not convert to "val" as the function depends on PicoCli options which are not fully instantiated until the "run" method is started
     private fun List<String>.toFilesURIList() =

diff --git a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/LoadRuleProviders.kt b/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/LoadRuleProviders.kt
@@ -14,11 +14,13 @@ private val LOGGER = KotlinLogging.logger {}.initKtLintKLogger()
 /**
  * Loads given list of paths to jar files. For files containing a [RuleSetProviderV2] class, get all [RuleProvider]s.
  */
-internal fun List<URL>.loadRuleProvidersByRuleSetId(debug: Boolean): Map<String, Set<RuleProvider>> =
+internal fun List<URL>.loadRuleProviders(debug: Boolean): Set<RuleProvider> =
     getKtlintRulesets()
         .plus(
             getRuleProvidersFromCustomRuleSetJars(debug),
-        )
+        ).values
+        .flatten()
+        .toSet()
 
 private fun getKtlintRulesets(): Map<String, Set<RuleProvider>> {
     return loadRulesetsFrom()

diff --git a/ktlint/src/test/kotlin/com/pinterest/ktlint/RuleSetsLoaderCLITest.kt b/ktlint/src/test/kotlin/com/pinterest/ktlint/RuleSetsLoaderCLITest.kt
@@ -36,12 +36,10 @@ class RuleSetsLoaderCLITest {
                 listOf("-R", "$tempDir/$jarWithRulesetProviderV2"),
             ) {
                 SoftAssertions().apply {
-                    assertNormalExitCode()
-                    // JAR ruleset provided with path "/var/folders/24/wtp_g21953x22nr8z86gvltc0000gp/T/junit920502858262478102/custom-ruleset/rule-set-provider-v2/ktlint-ruleset-template.jar
-                    // Add editor config override to enable rule set(s) '[indent-string-template-ruleset]' from custom rule set JAR('s): '[/var/folders/24/wtp_g21953x22nr8z86gvltc0000gp/T/junit920502858262478102/custom-ruleset/rule-set-provider-v2/ktlint-ruleset-template.jar]'
+                    assertErrorExitCode()
                     assertThat(normalOutput)
                         .containsLineMatching(Regex(".* JAR ruleset provided with path .*$jarWithRulesetProviderV2.*"))
-                        .containsLineMatching(Regex(".* Add editor config override to enable rule set\\(s\\) '\\[indent-string-template-ruleset]' from custom rule set JAR\\('s\\): .*$jarWithRulesetProviderV2.*"))
+                        .containsLineMatching(Regex(".*custom-ruleset.rule-set-provider-v2.Main.kt:1:1: Unexpected var, use val instead.*custom-rule-set-id:no-var.*"))
                 }.assertAll()
             }
     }

diff --git a/ktlint/src/test/resources/cli/custom-ruleset/rule-set-provider-v2/Main.kt b/ktlint/src/test/resources/cli/custom-ruleset/rule-set-provider-v2/Main.kt
@@ -1,3 +1,5 @@
+var helloWorld = "Hello world!"
+
 fun main() {
-    println("Hello world!")
+    println(helloWorld)
 }
diff --git a/...nt/src/test/resources/cli/custom-ruleset/rule-set-provider-v2/ktlint-ruleset-template.jar b/...nt/src/test/resources/cli/custom-ruleset/rule-set-provider-v2/ktlint-ruleset-template.jar