Add rules to suggest usage of check(), require() and error().

detekt-cli/src/main/resources/default-detekt-config.yml

@@ -511,9 +511,13 @@ style:
   UnusedPrivateMember:
     active: false
     allowedNames: "(_|ignored|expected|serialVersionUID)"
+  UseCheckOrError:
+    active: false
   UseDataClass:
     active: false
     excludeAnnotatedClasses: ""
+  UseRequire:
+    active: false
   UtilityClassWithPublicConstructor:
     active: false
   VarCouldBeVal:

detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/providers/StyleGuideProvider.kt

@@ -39,7 +39,9 @@ import io.gitlab.arturbosch.detekt.rules.style.UntilInsteadOfRangeTo
 import io.gitlab.arturbosch.detekt.rules.style.UnusedImports
 import io.gitlab.arturbosch.detekt.rules.style.UnusedPrivateClass
 import io.gitlab.arturbosch.detekt.rules.style.UnusedPrivateMember
+import io.gitlab.arturbosch.detekt.rules.style.UseCheckOrError
 import io.gitlab.arturbosch.detekt.rules.style.UseDataClass
+import io.gitlab.arturbosch.detekt.rules.style.UseRequire
 import io.gitlab.arturbosch.detekt.rules.style.UtilityClassWithPublicConstructor
 import io.gitlab.arturbosch.detekt.rules.style.VarCouldBeVal
 import io.gitlab.arturbosch.detekt.rules.style.WildcardImport
@@ -60,7 +62,9 @@ class StyleGuideProvider : RuleSetProvider {
     override val ruleSetId: String = "style"
 
     override fun instance(config: Config): RuleSet {
-        return RuleSet(ruleSetId, listOf(
+        return RuleSet(
+            ruleSetId,
+            listOf(
                 CollapsibleIfStatements(config),
                 ReturnCount(config),
                 ThrowsCount(config),
@@ -103,7 +107,10 @@ class StyleGuideProvider : RuleSetProvider {
                 VarCouldBeVal(config),
                 ForbiddenVoid(config),
                 ExplicitItLambdaParameter(config),
-                UnderscoresInNumericLiterals(config)
-        ))
+                UnderscoresInNumericLiterals(config),
+                UseRequire(config),
+                UseCheckOrError(config)
+            )
+        )
     }
 }

detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseCheckOrError.kt

@@ -0,0 +1,58 @@
+package io.gitlab.arturbosch.detekt.rules.style
+
+import io.gitlab.arturbosch.detekt.api.CodeSmell
+import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.api.Debt
+import io.gitlab.arturbosch.detekt.api.Entity
+import io.gitlab.arturbosch.detekt.api.Issue
+import io.gitlab.arturbosch.detekt.api.Rule
+import io.gitlab.arturbosch.detekt.api.Severity
+import org.jetbrains.kotlin.psi.KtCallExpression
+import org.jetbrains.kotlin.psi.KtThrowExpression
+import org.jetbrains.kotlin.psi.psiUtil.findDescendantOfType
+
+/**
+ * Kotlin provides a much more concise way to check invariants as well as pre- and post conditions than to manually throw
+ * an IllegalStateException.
+ *
+ * <noncompliant>
+ * if (value == null) throw new IllegalStateException("value should not be null")
+ * if (value < 0) throw new IllegalStateException("value is $value but should be at least 0")
+ * when(a) {
+ *   1 -> doSomething()
+ *   else -> throw IllegalStateException("Unexpected value")
+ * }
+ * </noncompliant>
+ *
+ * <compliant>
+ * checkNotNull(value) {"value should not be null"}
+ * check(value >= 0) { "value is $value but should be at least 0" }
+ * when(a) {
+ *   1 -> doSomething()
+ *   else -> error("Unexpected value")
+ * }
+ * </compliant>
+ *
+ * @author Markus Schwarz
+ */
+class UseCheckOrError(config: Config = Config.empty) : Rule(config) {
+
+    override val issue = Issue(
+        "UseRequire", Severity.Style,
+        "Use check() or error() instead of throwing an IllegalStateException.",
+        Debt.FIVE_MINS
+    )
+
+    override fun visitThrowExpression(expression: KtThrowExpression) {
+        if (expression.isIllegalStateExceptionWithoutCause()) {
+            report(CodeSmell(issue, Entity.from(expression), issue.description))
+        }
+    }
+
+    private fun KtThrowExpression.isIllegalStateExceptionWithoutCause(): Boolean {
+        val callExpression = findDescendantOfType<KtCallExpression>()
+        val argumentCount = callExpression?.valueArgumentList?.children?.size ?: 0
+
+        return callExpression?.firstChild?.text == "IllegalStateException" && argumentCount < 2
+    }
+}

detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseRequire.kt

@@ -0,0 +1,50 @@
+package io.gitlab.arturbosch.detekt.rules.style
+
+import io.gitlab.arturbosch.detekt.api.CodeSmell
+import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.api.Debt
+import io.gitlab.arturbosch.detekt.api.Entity
+import io.gitlab.arturbosch.detekt.api.Issue
+import io.gitlab.arturbosch.detekt.api.Rule
+import io.gitlab.arturbosch.detekt.api.Severity
+import org.jetbrains.kotlin.psi.KtCallExpression
+import org.jetbrains.kotlin.psi.KtThrowExpression
+import org.jetbrains.kotlin.psi.psiUtil.findDescendantOfType
+
+/**
+ * Kotlin provides a much more concise way to check preconditions than to manually throw an
+ * IllegalArgumentException.
+ *
+ * <noncompliant>
+ * if (value == null) throw new IllegalArgumentException("value should not be null")
+ * if (value < 0) throw new IllegalArgumentException("value is $value but should be at least 0")
+ * </noncompliant>
+ *
+ * <compliant>
+ * requireNotNull(value) {"value should not be null"}
+ * require(value >= 0) { "value is $value but should be at least 0" }
+ * </compliant>
+ *
+ * @author Markus Schwarz
+ */
+class UseRequire(config: Config = Config.empty) : Rule(config) {
+
+    override val issue = Issue(
+        "UseRequire", Severity.Style,
+        "Use require() instead of throwing an IllegalArgumentException.",
+        Debt.FIVE_MINS
+    )
+
+    override fun visitThrowExpression(expression: KtThrowExpression) {
+        if (expression.isIllegalArgumentException()) {
+            report(CodeSmell(issue, Entity.from(expression), issue.description))
+        }
+    }
+
+    private fun KtThrowExpression.isIllegalArgumentException(): Boolean {
+        val callExpression = findDescendantOfType<KtCallExpression>()
+        val argumentCount = callExpression?.valueArgumentList?.children?.size ?: 0
+
+        return callExpression?.firstChild?.text == "IllegalArgumentException" && argumentCount < 2
+    }
+}

detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseCheckOrErrorSpec.kt

@@ -0,0 +1,80 @@
+package io.gitlab.arturbosch.detekt.rules.style
+
+import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.test.lint
+import org.assertj.core.api.Assertions.assertThat
+import org.spekframework.spek2.Spek
+import org.spekframework.spek2.style.specification.describe
+
+class UseCheckOrErrorSpec : Spek({
+
+    val subject by memoized { UseCheckOrError(Config.empty) }
+
+    describe("UseCheckOrError rule") {
+
+        it("reports if a an IllegalStateException is thrown") {
+            val code = """
+                fun x() {
+                    doSomething()
+                    if (a < 0) throw IllegalStateException()
+                }"""
+            assertThat(subject.lint(code)).hasSize(1)
+        }
+
+        it("reports if a an IllegalStateException is thrown with an error message") {
+            val code = """
+                fun x() {
+                    doSomething()
+                    if (a < 0) throw IllegalStateException("More details")
+                }"""
+            assertThat(subject.lint(code)).hasSize(1)
+        }
+
+        it("reports if a an IllegalStateException is thrown as default case of a when expression") {
+            val code = """
+                fun x(a: Int) =
+                    when (a) {
+                        1 -> doSomething()
+                        else -> throw IllegalStateException()
+                    }"""
+            assertThat(subject.lint(code)).hasSize(1)
+        }
+
+        it("reports if an IllegalStateException is thrown by its fully qualified name") {
+            val code = """
+                fun x() {
+                    doSomething()
+                    if (a < 0) throw java.lang.IllegalStateException()
+                }"""
+            assertThat(subject.lint(code)).hasSize(1)
+        }
+
+        it("reports if an IllegalStateException is thrown by its fully qualified name using the kotlin type alias") {
+            val code = """
+                fun x() {
+                    doSomething()
+                    if (a < 0) throw kotlin.IllegalStateException()
+                }"""
+            assertThat(subject.lint(code)).hasSize(1)
+        }
+
+        it("does not report if any other kind of exception is thrown") {
+            val code = """
+                fun x() {
+                    doSomething()
+                    if (a < 0) throw SomeBusinessException()
+                }"""
+            assertThat(subject.lint(code)).isEmpty()
+        }
+
+        it("does not report an issue if the exception thrown has a message and a cause") {
+            val code = """
+                private fun missing(): Nothing {
+                    if  (cause != null) {
+                        throw IllegalStateException("message", cause)
+                    }
+                }"""
+            assertThat(subject.lint(code)).isEmpty()
+        }
+    }
+})

detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseRequireSpec.kt

@@ -0,0 +1,68 @@
+package io.gitlab.arturbosch.detekt.rules.style
+
+import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.test.lint
+import org.assertj.core.api.Assertions.assertThat
+import org.spekframework.spek2.Spek
+import org.spekframework.spek2.style.specification.describe
+
+class UseRequireSpec : Spek({
+
+    val subject by memoized { UseRequire(Config.empty) }
+
+    describe("UseRequire rule") {
+
+        it("reports if a precondition throws an IllegalArgumentException") {
+            val code = """
+                fun x(a: Int) {
+                    if (a < 0) throw IllegalArgumentException()
+                    doSomething()
+                }"""
+            assertThat(subject.lint(code)).hasSize(1)
+        }
+
+        it("reports if a precondition throws an IllegalArgumentException with more details") {
+            val code = """
+                fun x(a: Int) {
+                    if (a < 0) throw IllegalArgumentException("More details")
+                    doSomething()
+                }"""
+            assertThat(subject.lint(code)).hasSize(1)
+        }
+
+        it("reports if a precondition throws a fully qualified IllegalArgumentException") {
+            val code = """
+                fun x(a: Int) {
+                    if (a < 0) throw java.lang.IllegalArgumentException()
+                    doSomething()
+                }"""
+            assertThat(subject.lint(code)).hasSize(1)
+        }
+
+        it("reports if a precondition throws a fully qualified IllegalArgumentException using the kotlin type alias") {
+            val code = """
+                fun x(a: Int) {
+                    if (a < 0) throw kotlin.IllegalArgumentException()
+                    doSomething()
+                }"""
+            assertThat(subject.lint(code)).hasSize(1)
+        }
+
+        it("does not report if a precondition throws a different kind of exception") {
+            val code = """
+                fun x(a: Int) {
+                    if (a < 0) throw SomeBusinessException()
+                    doSomething()
+                }"""
+            assertThat(subject.lint(code)).isEmpty()
+        }
+
+        it("does not report an issue if the exception thrown has a message and a cause") {
+            val code = """
+                private fun x(a: Int): Nothing {
+                    throw IllegalArgumentException("message", cause)
+                }"""
+            assertThat(subject.lint(code)).isEmpty()
+        }
+    }
+})

docs/pages/documentation/style.md

@@ -1118,6 +1118,37 @@ can lead to confusion and potential bugs.
 
    unused private member names matching this regex are ignored
 
+### UseCheckOrError
+
+Kotlin provides a much more concise way to check invariants as well as pre- and post conditions than to manually throw
+an IllegalStateException.
+
+**Severity**: Style
+
+**Debt**: 5min
+
+#### Noncompliant Code:
+
+```kotlin
+if (value == null) throw new IllegalStateException("value should not be null")
+if (value < 0) throw new IllegalStateException("value is $value but should be at least 0")
+when(a) {
+1 -> doSomething()
+else -> throw IllegalStateException("Unexpected value")
+}
+```
+
+#### Compliant Code:
+
+```kotlin
+checkNotNull(value) {"value should not be null"}
+check(value >= 0) { "value is $value but should be at least 0" }
+when(a) {
+1 -> doSomething()
+else -> error("Unexpected value")
+}
+```
+
 ### UseDataClass
 
 Classes that simply hold data should be refactored into a `data class`. Data classes are specialized to hold data
@@ -1150,6 +1181,29 @@ class DataClassCandidate(val i: Int) {
 data class DataClass(val i: Int, val i2: Int)
 ```
 
+### UseRequire
+
+Kotlin provides a much more concise way to check preconditions than to manually throw an
+IllegalArgumentException.
+
+**Severity**: Style
+
+**Debt**: 5min
+
+#### Noncompliant Code:
+
+```kotlin
+if (value == null) throw new IllegalArgumentException("value should not be null")
+if (value < 0) throw new IllegalArgumentException("value is $value but should be at least 0")
+```
+
+#### Compliant Code:
+
+```kotlin
+requireNotNull(value) {"value should not be null"}
+require(value >= 0) { "value is $value but should be at least 0" }
+```
+
 ### UtilityClassWithPublicConstructor
 
 A class which only contains utility variables and functions with no concrete implementation can be refactored