Disable autoCorrect property for all rules if global flag is set to f…

…alse (#2413)

* Fix max line length issues for readability

* Disable autoCorrect property for all rules if global flag is set to false - #2341

This way we make sure no KtFile is modified and our reporting calculates the right issue locations.

detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/DisabledAutoCorrectConfig.kt

@@ -0,0 +1,23 @@
+package io.gitlab.arturbosch.detekt.api.internal
+
+import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.api.Notification
+
+@Suppress("UNCHECKED_CAST")
+class DisabledAutoCorrectConfig(val wrapped: Config) : Config, ValidatableConfiguration {
+
+    override fun subConfig(key: String): Config = DisabledAutoCorrectConfig(wrapped.subConfig(key))
+
+    override fun <T : Any> valueOrDefault(key: String, default: T): T = when (key) {
+        "autoCorrect" -> false as T
+        else -> wrapped.valueOrDefault(key, default)
+    }
+
+    override fun <T : Any> valueOrNull(key: String): T? = when (key) {
+        "autoCorrect" -> false as T
+        else -> wrapped.valueOrNull(key)
+    }
+
+    override fun validate(baseline: Config, excludePatterns: Set<Regex>): List<Notification> =
+        validateConfig(wrapped, baseline, excludePatterns)
+}

detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/Configurations.kt

@@ -3,6 +3,7 @@ package io.gitlab.arturbosch.detekt.cli
 import io.gitlab.arturbosch.detekt.api.CompositeConfig
 import io.gitlab.arturbosch.detekt.api.Config
 import io.gitlab.arturbosch.detekt.api.YamlConfig
+import io.gitlab.arturbosch.detekt.api.internal.DisabledAutoCorrectConfig
 import io.gitlab.arturbosch.detekt.api.internal.FailFastConfig
 import io.gitlab.arturbosch.detekt.api.internal.PathFilters
 import java.nio.file.Path
@@ -38,6 +39,10 @@ fun CliArgs.loadConfiguration(): Config {
             ?: initializedDefaultConfig, initializedDefaultConfig)
     }
 
+    if (!autoCorrect) {
+        declaredConfig = DisabledAutoCorrectConfig(declaredConfig ?: loadDefaultConfig())
+    }
+
     return declaredConfig ?: loadDefaultConfig()
 }
 

detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/ConfigurationsSpec.kt

@@ -47,9 +47,12 @@ internal class ConfigurationsSpec : Spek({
         }
 
         it("should fail on invalid config value") {
-            assertThatIllegalArgumentException().isThrownBy { CliArgs { config = "," }.loadConfiguration() }
-            assertThatExceptionOfType(ParameterException::class.java).isThrownBy { CliArgs { config = "sfsjfsdkfsd" }.loadConfiguration() }
-            assertThatExceptionOfType(ParameterException::class.java).isThrownBy { CliArgs { config = "./i.do.not.exist.yml" }.loadConfiguration() }
+            assertThatIllegalArgumentException()
+                .isThrownBy { CliArgs { config = "," }.loadConfiguration() }
+            assertThatExceptionOfType(ParameterException::class.java)
+                .isThrownBy { CliArgs { config = "sfsjfsdkfsd" }.loadConfiguration() }
+            assertThatExceptionOfType(ParameterException::class.java)
+                .isThrownBy { CliArgs { config = "./i.do.not.exist.yml" }.loadConfiguration() }
         }
     }
 
@@ -76,9 +79,12 @@ internal class ConfigurationsSpec : Spek({
         }
 
         it("should fail on invalid config value") {
-            assertThatExceptionOfType(Config.InvalidConfigurationError::class.java).isThrownBy { CliArgs { configResource = "," }.loadConfiguration() }
-            assertThatExceptionOfType(ParameterException::class.java).isThrownBy { CliArgs { configResource = "sfsjfsdkfsd" }.loadConfiguration() }
-            assertThatExceptionOfType(ParameterException::class.java).isThrownBy { CliArgs { configResource = "./i.do.not.exist.yml" }.loadConfiguration() }
+            assertThatExceptionOfType(Config.InvalidConfigurationError::class.java)
+                .isThrownBy { CliArgs { configResource = "," }.loadConfiguration() }
+            assertThatExceptionOfType(ParameterException::class.java)
+                .isThrownBy { CliArgs { configResource = "sfsjfsdkfsd" }.loadConfiguration() }
+            assertThatExceptionOfType(ParameterException::class.java)
+                .isThrownBy { CliArgs { configResource = "./i.do.not.exist.yml" }.loadConfiguration() }
         }
     }
 
@@ -143,36 +149,50 @@ internal class ConfigurationsSpec : Spek({
     }
 
     describe("fail fast only") {
+
         val config = CliArgs {
             configResource = "/configs/empty.yml"
             failFast = true
         }.loadConfiguration()
 
         it("should override active to true by default") {
-            assertThat(config.subConfig("comments").subConfig("UndocumentedPublicClass").valueOrDefault("active", false)).isEqualTo(true)
+            val actual = config.subConfig("comments")
+                .subConfig("UndocumentedPublicClass")
+                .valueOrDefault("active", false)
+            assertThat(actual).isEqualTo(true)
         }
 
         it("should override maxIssues to 0 by default") {
             assertThat(config.subConfig("build").valueOrDefault("maxIssues", -1)).isEqualTo(0)
         }
 
         it("should keep config from default") {
-            assertThat(config.subConfig("style").subConfig("MaxLineLength").valueOrDefault("maxLineLength", -1)).isEqualTo(120)
+            val actual = config.subConfig("style")
+                .subConfig("MaxLineLength")
+                .valueOrDefault("maxLineLength", -1)
+            assertThat(actual).isEqualTo(120)
         }
     }
 
     describe("fail fast override") {
+
         val config = CliArgs {
             configResource = "/configs/fail-fast-will-override-here.yml"
             failFast = true
         }.loadConfiguration()
 
         it("should override config when specified") {
-            assertThat(config.subConfig("style").subConfig("MaxLineLength").valueOrDefault("maxLineLength", -1)).isEqualTo(100)
+            val actual = config.subConfig("style")
+                .subConfig("MaxLineLength")
+                .valueOrDefault("maxLineLength", -1)
+            assertThat(actual).isEqualTo(100)
         }
 
         it("should override active when specified") {
-            assertThat(config.subConfig("comments").subConfig("CommentOverPrivateMethod").valueOrDefault("active", true)).isEqualTo(false)
+            val actual = config.subConfig("comments")
+                .subConfig("CommentOverPrivateMethod")
+                .valueOrDefault("active", true)
+            assertThat(actual).isEqualTo(false)
         }
 
         it("should override maxIssues when specified") {
@@ -189,16 +209,82 @@ internal class ConfigurationsSpec : Spek({
         }.loadConfiguration()
 
         it("should override config when specified") {
-            assertThat(config.subConfig("style").subConfig("MaxLineLength").valueOrDefault("maxLineLength", -1)).isEqualTo(100)
-            assertThat(config.subConfig("style").subConfig("MaxLineLength").valueOrDefault("excludeCommentStatements", false)).isTrue()
+            val ruleConfig = config.subConfig("style").subConfig("MaxLineLength")
+            val lineLength = ruleConfig.valueOrDefault("maxLineLength", -1)
+            val excludeComments = ruleConfig.valueOrDefault("excludeCommentStatements", false)
+
+            assertThat(lineLength).isEqualTo(100)
+            assertThat(excludeComments).isTrue()
         }
 
         it("should be active=false by default") {
-            assertThat(config.subConfig("comments").subConfig("CommentOverPrivateFunction").valueOrDefault("active", true)).isFalse()
+            val actual = config.subConfig("comments")
+                .subConfig("CommentOverPrivateFunction")
+                .valueOrDefault("active", true)
+            assertThat(actual).isFalse()
         }
 
         it("should be maxIssues=0 by default") {
-            assertThat(config.subConfig("build").valueOrDefault("maxIssues", -1)).isEqualTo(0)
+            val actual = config.subConfig("build").valueOrDefault("maxIssues", -1)
+            assertThat(actual).isEqualTo(0)
+        }
+    }
+
+    describe("auto correct config") {
+
+        context("when specified it respects all autoCorrect values of rules and rule sets") {
+            val config = CliArgs {
+                autoCorrect = true
+                configResource = "/configs/config-with-auto-correct.yml"
+            }.loadConfiguration()
+
+            val style = config.subConfig("style")
+            val comments = config.subConfig("comments")
+
+            it("is disabled for rule sets") {
+                assertThat(style.valueOrNull<Boolean>("autoCorrect")).isTrue()
+                assertThat(comments.valueOrNull<Boolean>("autoCorrect")).isFalse()
+            }
+
+            it("is disabled for rules") {
+                assertThat(style.subConfig("MagicNumber").valueOrNull<Boolean>("autoCorrect")).isTrue()
+                assertThat(style.subConfig("MagicString").valueOrNull<Boolean>("autoCorrect")).isFalse()
+                assertThat(comments.subConfig("ClassDoc").valueOrNull<Boolean>("autoCorrect")).isTrue()
+                assertThat(comments.subConfig("FunctionDoc").valueOrNull<Boolean>("autoCorrect")).isFalse()
+            }
+        }
+
+        mapOf(
+            CliArgs {
+                configResource = "/configs/config-with-auto-correct.yml"
+            }.loadConfiguration() to "when not specified all autoCorrect values are overridden to false",
+            CliArgs {
+                autoCorrect = false
+                configResource = "/configs/config-with-auto-correct.yml"
+            }.loadConfiguration() to "when specified as false, all autoCorrect values are overridden to false",
+            CliArgs {
+                autoCorrect = false
+                failFast = true
+                buildUponDefaultConfig = true
+                configResource = "/configs/config-with-auto-correct.yml"
+            }.loadConfiguration() to "regardless of other cli options, autoCorrect values are overridden to false"
+        ).forEach { (config, testContext) ->
+            context(testContext) {
+                val style = config.subConfig("style")
+                val comments = config.subConfig("comments")
+
+                it("is disabled for rule sets") {
+                    assertThat(style.valueOrNull<Boolean>("autoCorrect")).isFalse()
+                    assertThat(comments.valueOrNull<Boolean>("autoCorrect")).isFalse()
+                }
+
+                it("is disabled for rules") {
+                    assertThat(style.subConfig("MagicNumber").valueOrNull<Boolean>("autoCorrect")).isFalse()
+                    assertThat(style.subConfig("MagicString").valueOrNull<Boolean>("autoCorrect")).isFalse()
+                    assertThat(comments.subConfig("ClassDoc").valueOrNull<Boolean>("autoCorrect")).isFalse()
+                    assertThat(comments.subConfig("FunctionDoc").valueOrNull<Boolean>("autoCorrect")).isFalse()
+                }
+            }
         }
     }
 })

detekt-cli/src/test/resources/configs/config-with-auto-correct.yml

@@ -0,0 +1,13 @@
+style:
+  autoCorrect: true
+  MagicNumber:
+    autoCorrect: true
+  MagicString:
+    autoCorrect: false
+
+comments:
+  autoCorrect: false
+  ClassDoc:
+    autoCorrect: true
+  FunctionDoc:
+    autoCorrect: false

docs/pages/kdoc/detekt-api/alltypes/index.md

@@ -151,6 +151,11 @@ Basic visitor which is used inside detekt.
 Guarantees a better looking name as the extended base class :).
 
 
+|
+
+##### [io.gitlab.arturbosch.detekt.api.internal.DisabledAutoCorrectConfig](../io.gitlab.arturbosch.detekt.api.internal/-disabled-auto-correct-config/index.html)
+
+
 |
 
 ##### [io.gitlab.arturbosch.detekt.api.Entity](../io.gitlab.arturbosch.detekt.api/-entity/index.html)

docs/pages/kdoc/detekt-api/io.gitlab.arturbosch.detekt.api.internal/-disabled-auto-correct-config/-init-.md

@@ -0,0 +1,9 @@
+---
+title: DisabledAutoCorrectConfig.<init> - detekt-api
+---
+
+[detekt-api](../../index.html) / [io.gitlab.arturbosch.detekt.api.internal](../index.html) / [DisabledAutoCorrectConfig](index.html) / [&lt;init&gt;](./-init-.html)
+
+# &lt;init&gt;
+
+`DisabledAutoCorrectConfig(wrapped: `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html)`)`

docs/pages/kdoc/detekt-api/io.gitlab.arturbosch.detekt.api.internal/-disabled-auto-correct-config/index.md

@@ -0,0 +1,25 @@
+---
+title: DisabledAutoCorrectConfig - detekt-api
+---
+
+[detekt-api](../../index.html) / [io.gitlab.arturbosch.detekt.api.internal](../index.html) / [DisabledAutoCorrectConfig](./index.html)
+
+# DisabledAutoCorrectConfig
+
+`class DisabledAutoCorrectConfig : `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html)`, `[`ValidatableConfiguration`](../-validatable-configuration/index.html)
+
+### Constructors
+
+| [<init>](-init-.html) | `DisabledAutoCorrectConfig(wrapped: `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html)`)` |
+
+### Properties
+
+| [wrapped](wrapped.html) | `val wrapped: `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html) |
+
+### Functions
+
+| [subConfig](sub-config.html) | Tries to retrieve part of the configuration based on given key.`fun subConfig(key: `[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`): `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html) |
+| [validate](validate.html) | `fun validate(baseline: `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html)`, excludePatterns: `[`Set`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-set/index.html)`<`[`Regex`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.text/-regex/index.html)`>): `[`List`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-list/index.html)`<`[`Notification`](../../io.gitlab.arturbosch.detekt.api/-notification/index.html)`>` |
+| [valueOrDefault](value-or-default.html) | Retrieves a sub configuration or value based on given key. If configuration property cannot be found the specified default value is returned.`fun <T : `[`Any`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-any/index.html)`> valueOrDefault(key: `[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`, default: T): T` |
+| [valueOrNull](value-or-null.html) | Retrieves a sub configuration or value based on given key. If the configuration property cannot be found, null is returned.`fun <T : `[`Any`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-any/index.html)`> valueOrNull(key: `[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`): T?` |
+

docs/pages/kdoc/detekt-api/io.gitlab.arturbosch.detekt.api.internal/-disabled-auto-correct-config/sub-config.md

@@ -0,0 +1,12 @@
+---
+title: DisabledAutoCorrectConfig.subConfig - detekt-api
+---
+
+[detekt-api](../../index.html) / [io.gitlab.arturbosch.detekt.api.internal](../index.html) / [DisabledAutoCorrectConfig](index.html) / [subConfig](./sub-config.html)
+
+# subConfig
+
+`fun subConfig(key: `[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`): `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html)
+
+Tries to retrieve part of the configuration based on given key.
+

docs/pages/kdoc/detekt-api/io.gitlab.arturbosch.detekt.api.internal/-disabled-auto-correct-config/validate.md

@@ -0,0 +1,9 @@
+---
+title: DisabledAutoCorrectConfig.validate - detekt-api
+---
+
+[detekt-api](../../index.html) / [io.gitlab.arturbosch.detekt.api.internal](../index.html) / [DisabledAutoCorrectConfig](index.html) / [validate](./validate.html)
+
+# validate
+
+`fun validate(baseline: `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html)`, excludePatterns: `[`Set`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-set/index.html)`<`[`Regex`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.text/-regex/index.html)`>): `[`List`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-list/index.html)`<`[`Notification`](../../io.gitlab.arturbosch.detekt.api/-notification/index.html)`>`

docs/pages/kdoc/detekt-api/io.gitlab.arturbosch.detekt.api.internal/-disabled-auto-correct-config/value-or-default.md

@@ -0,0 +1,13 @@
+---
+title: DisabledAutoCorrectConfig.valueOrDefault - detekt-api
+---
+
+[detekt-api](../../index.html) / [io.gitlab.arturbosch.detekt.api.internal](../index.html) / [DisabledAutoCorrectConfig](index.html) / [valueOrDefault](./value-or-default.html)
+
+# valueOrDefault
+
+`fun <T : `[`Any`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-any/index.html)`> valueOrDefault(key: `[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`, default: T): T`
+
+Retrieves a sub configuration or value based on given key. If configuration property cannot be found
+the specified default value is returned.
+

docs/pages/kdoc/detekt-api/io.gitlab.arturbosch.detekt.api.internal/-disabled-auto-correct-config/value-or-null.md

@@ -0,0 +1,13 @@
+---
+title: DisabledAutoCorrectConfig.valueOrNull - detekt-api
+---
+
+[detekt-api](../../index.html) / [io.gitlab.arturbosch.detekt.api.internal](../index.html) / [DisabledAutoCorrectConfig](index.html) / [valueOrNull](./value-or-null.html)
+
+# valueOrNull
+
+`fun <T : `[`Any`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-any/index.html)`> valueOrNull(key: `[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`): T?`
+
+Retrieves a sub configuration or value based on given key.
+If the configuration property cannot be found, null is returned.
+

docs/pages/kdoc/detekt-api/io.gitlab.arturbosch.detekt.api.internal/-disabled-auto-correct-config/wrapped.md

@@ -0,0 +1,9 @@
+---
+title: DisabledAutoCorrectConfig.wrapped - detekt-api
+---
+
+[detekt-api](../../index.html) / [io.gitlab.arturbosch.detekt.api.internal](../index.html) / [DisabledAutoCorrectConfig](index.html) / [wrapped](./wrapped.html)
+
+# wrapped
+
+`val wrapped: `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html)

docs/pages/kdoc/detekt-api/io.gitlab.arturbosch.detekt.api.internal/-validatable-configuration/index.md

@@ -15,6 +15,7 @@ title: ValidatableConfiguration - detekt-api
 ### Inheritors
 
 | [CompositeConfig](../../io.gitlab.arturbosch.detekt.api/-composite-config/index.html) | Wraps two different configuration which should be considered when retrieving properties.`class CompositeConfig : `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html)`, `[`ValidatableConfiguration`](./index.html) |
+| [DisabledAutoCorrectConfig](../-disabled-auto-correct-config/index.html) | `class DisabledAutoCorrectConfig : `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html)`, `[`ValidatableConfiguration`](./index.html) |
 | [FailFastConfig](../-fail-fast-config/index.html) | `data class FailFastConfig : `[`Config`](../../io.gitlab.arturbosch.detekt.api/-config/index.html)`, `[`ValidatableConfiguration`](./index.html) |
 | [YamlConfig](../../io.gitlab.arturbosch.detekt.api/-yaml-config/index.html) | Config implementation using the yaml format. SubConfigurations can return sub maps according to the yaml specification.`class YamlConfig : `[`BaseConfig`](../../io.gitlab.arturbosch.detekt.api/-base-config/index.html)`, `[`ValidatableConfiguration`](./index.html) |
 

docs/pages/kdoc/detekt-api/io.gitlab.arturbosch.detekt.api.internal/index.md

@@ -12,6 +12,7 @@ title: io.gitlab.arturbosch.detekt.api.internal - detekt-api
 | [CyclomaticComplexity](-cyclomatic-complexity/index.html) | Counts the cyclomatic complexity of nodes.`class CyclomaticComplexity : `[`DetektVisitor`](../io.gitlab.arturbosch.detekt.api/-detekt-visitor/index.html) |
 | [DefaultRuleSetProvider](-default-rule-set-provider.html) | Interface which marks sub-classes as provided by detekt via the rules sub-module.`interface DefaultRuleSetProvider : `[`RuleSetProvider`](../io.gitlab.arturbosch.detekt.api/-rule-set-provider/index.html) |
 | [DetektPomModel](-detekt-pom-model/index.html) | Adapted from https://github.com/pinterest/ktlint/blob/master/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLint.kt Licenced under the MIT licence - https://github.com/pinterest/ktlint/blob/master/LICENSE`class DetektPomModel : UserDataHolderBase, PomModel` |
+| [DisabledAutoCorrectConfig](-disabled-auto-correct-config/index.html) | `class DisabledAutoCorrectConfig : `[`Config`](../io.gitlab.arturbosch.detekt.api/-config/index.html)`, `[`ValidatableConfiguration`](-validatable-configuration/index.html) |
 | [FailFastConfig](-fail-fast-config/index.html) | `data class FailFastConfig : `[`Config`](../io.gitlab.arturbosch.detekt.api/-config/index.html)`, `[`ValidatableConfiguration`](-validatable-configuration/index.html) |
 | [PathFilters](-path-filters/index.html) | `class PathFilters` |
 | [SimpleNotification](-simple-notification/index.html) | `data class SimpleNotification : `[`Notification`](../io.gitlab.arturbosch.detekt.api/-notification/index.html) |