diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 122c7b98c2b..bd03283fb56 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -10,9 +10,8 @@ - We use [JUnit 5](https://junit.org/junit5/docs/current/user-guide/) for testing. Please use the `Spec.kt` suffix on new test classes. If your new rule requires type resolution (i.e. it utilises `BindingContext`) then annotate your test class with `@KotlinCoreEnvironmentTest` and have the test class accept `KotlinCoreEnvironment` as a parameter. - See `SpekTestDiscoverySpec.kt` in the `custom-checks` module for a complete example. -- We are in the process of migrating existing [Spek](https://github.com/spekframework/spek) tests to JUnit 5. For easier - testing of these tests you might want to use the [Spek IntelliJ Plugin](https://plugins.jetbrains.com/plugin/10915-spek-framework). + See "Testing a rule that uses type resolution" section of the [Using Type Resolution](../docs/pages/gettingstarted/type-resolution.md) + guide for details. - Feel free to add your name to the contributors list at the end of the readme file when opening a pull request. - The code in `detekt-api` and any rule in `detekt-rules` must be documented. We generate documentation for our website based on these modules. - If some Kotlin code in `resources` folder (like `detekt-formatting`) shows a compilation error, right click on it and use `Mark as plain text`. diff --git a/build-logic/src/main/kotlin/module.gradle.kts b/build-logic/src/main/kotlin/module.gradle.kts index 3fc43b78557..64c00360cd4 100644 --- a/build-logic/src/main/kotlin/module.gradle.kts +++ b/build-logic/src/main/kotlin/module.gradle.kts @@ -26,9 +26,6 @@ tasks.withType().configureEach { tasks.withType().configureEach { useJUnitPlatform() - systemProperty("spek2.jvm.cg.scan.concurrency", 1) // use one thread for classpath scanning - systemProperty("spek2.execution.test.timeout", 0) // disable test timeout - systemProperty("spek2.discovery.parallel.enabled", 0) // disable parallel test discovery systemProperty("junit.jupiter.testinstance.lifecycle.default", "per_class") val compileSnippetText: Boolean = if (project.hasProperty("compile-test-snippets")) { (project.property("compile-test-snippets") as String).toBoolean() diff --git a/build.gradle.kts b/build.gradle.kts index 2e40159784b..3f14edd2b5d 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -27,7 +27,6 @@ allprojects { dependencies { detekt(project(":detekt-cli")) - detektPlugins(project(":custom-checks")) detektPlugins(project(":detekt-formatting")) } diff --git a/code-coverage-report/build.gradle.kts b/code-coverage-report/build.gradle.kts index 795fd19aff6..046a8cf777d 100644 --- a/code-coverage-report/build.gradle.kts +++ b/code-coverage-report/build.gradle.kts @@ -14,7 +14,6 @@ reporting { jacoco.toolVersion = libs.versions.jacoco.get() dependencies { - jacocoAggregation(projects.customChecks) jacocoAggregation(projects.detektApi) jacocoAggregation(projects.detektCli) jacocoAggregation(projects.detektCore) diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml index d7cd09e74a0..35a100c6eb7 100644 --- a/config/detekt/detekt.yml +++ b/config/detekt/detekt.yml @@ -1,15 +1,3 @@ -config: - # is automatically ignored when custom-checks.jar is on the classpath - # however other CI checks use the argsfile where our plugin is not applied - # we need to care take of this by explicitly allowing this properties - excludes: 'custom-checks.*' - -custom-checks: - active: true - SpekTestDiscovery: - active: true - includes: ['**/test/**/*Spec.kt'] - comments: CommentOverPrivateProperty: active: true diff --git a/custom-checks/build.gradle.kts b/custom-checks/build.gradle.kts deleted file mode 100644 index 9e4622e960b..00000000000 --- a/custom-checks/build.gradle.kts +++ /dev/null @@ -1,10 +0,0 @@ -plugins { - id("module") -} - -dependencies { - implementation(projects.detektApi) - testImplementation(projects.detektTest) - testImplementation(libs.assertj) - testRuntimeOnly(libs.spek.dsl) -} diff --git a/custom-checks/src/main/kotlin/io/github/detekt/custom/CustomChecksConfigValidator.kt b/custom-checks/src/main/kotlin/io/github/detekt/custom/CustomChecksConfigValidator.kt deleted file mode 100644 index 8bf25695ce9..00000000000 --- a/custom-checks/src/main/kotlin/io/github/detekt/custom/CustomChecksConfigValidator.kt +++ /dev/null @@ -1,37 +0,0 @@ -package io.github.detekt.custom - -import io.github.detekt.custom.CustomChecksProvider.Companion.RULE_SET_NAME -import io.gitlab.arturbosch.detekt.api.Config -import io.gitlab.arturbosch.detekt.api.ConfigValidator -import io.gitlab.arturbosch.detekt.api.Notification -import io.gitlab.arturbosch.detekt.api.internal.SimpleNotification - -class CustomChecksConfigValidator : ConfigValidator { - - override fun validate(config: Config): Collection { - val result = mutableListOf() - val customRules = config.subConfig(RULE_SET_NAME) - - fun validateSpekTestDiscoveryConfig() { - val rule = SpekTestDiscovery::class.java.simpleName - val spekRule = customRules.subConfig(rule) - spekRule.valueOrNull>(SpekTestDiscovery.SCOPING_FUNCTIONS) - ?.filter { it.contains(".") } - ?.forEach { - result.add( - SimpleNotification("$rule>${SpekTestDiscovery.SCOPING_FUNCTIONS}: $it must not be qualified.") - ) - } - spekRule.valueOrNull>(SpekTestDiscovery.ALLOWED_TYPES) - ?.filterNot { it.contains(".") } - ?.forEach { - result.add( - SimpleNotification("$rule>${SpekTestDiscovery.ALLOWED_TYPES}: $it must be qualified.") - ) - } - } - - validateSpekTestDiscoveryConfig() - return result - } -} diff --git a/custom-checks/src/main/kotlin/io/github/detekt/custom/CustomChecksProvider.kt b/custom-checks/src/main/kotlin/io/github/detekt/custom/CustomChecksProvider.kt deleted file mode 100644 index 86759b85a33..00000000000 --- a/custom-checks/src/main/kotlin/io/github/detekt/custom/CustomChecksProvider.kt +++ /dev/null @@ -1,19 +0,0 @@ -package io.github.detekt.custom - -import io.gitlab.arturbosch.detekt.api.Config -import io.gitlab.arturbosch.detekt.api.RuleSet -import io.gitlab.arturbosch.detekt.api.RuleSetProvider - -class CustomChecksProvider : RuleSetProvider { - - override val ruleSetId = RULE_SET_NAME - - override fun instance(config: Config) = RuleSet( - ruleSetId, - listOf(SpekTestDiscovery(config)) - ) - - companion object { - const val RULE_SET_NAME = "custom-checks" - } -} diff --git a/custom-checks/src/main/kotlin/io/github/detekt/custom/SpekTestDiscovery.kt b/custom-checks/src/main/kotlin/io/github/detekt/custom/SpekTestDiscovery.kt deleted file mode 100644 index cba649a8e31..00000000000 --- a/custom-checks/src/main/kotlin/io/github/detekt/custom/SpekTestDiscovery.kt +++ /dev/null @@ -1,148 +0,0 @@ -package io.github.detekt.custom - -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 io.gitlab.arturbosch.detekt.api.config -import io.gitlab.arturbosch.detekt.api.internal.Configuration -import io.gitlab.arturbosch.detekt.rules.fqNameOrNull -import org.jetbrains.kotlin.psi.KtCallExpression -import org.jetbrains.kotlin.psi.KtClass -import org.jetbrains.kotlin.psi.KtLambdaExpression -import org.jetbrains.kotlin.psi.KtProperty -import org.jetbrains.kotlin.psi.KtSuperTypeCallEntry -import org.jetbrains.kotlin.resolve.BindingContext -import org.jetbrains.kotlin.resolve.calls.util.getType - -/** - * Expensive setup code can slow down test discovery. - * Make sure to use memoization when declaring non-trivial types. - * - * - * class MyTest : Spek({ - * describe("...") { - * val ast = expensiveParse("code") - * - * test("...") { - * assertThat(ast)... - * } - * } - * }) - * - * - * - * class MyTest : Spek({ - * val ast by memoized { expensiveParse("code") } - * - * test("...") { - * assertThat(ast)... - * } - * }) - * - */ -class SpekTestDiscovery(config: Config = Config.empty) : Rule(config) { - - @Configuration("full qualified type") - private val allowedTypes: Set by config( - listOf( - "kotlin.Nothing", - "kotlin.String", - "kotlin.Int", - "kotlin.Double", - "java.nio.file.Path", - "java.io.File" - ), - List::toSet, - ) - - @Configuration("names of functions used to declare a test group") - private val scopingFunctions: Set by config( - listOf("describe", "context"), - List::toSet, - ) - - override val issue = Issue( - javaClass.simpleName, - Severity.Performance, - """Spek tests can be quite expensive during test discovery. - |Compared to Junit5, Spek does not only use reflection to discover the test classes but also needs - |to instantiate and run the constructor closure to find tests. - |Try using only simple setup code to not slow down the startup of single tests or test suites. - """.trimMargin(), - Debt.TEN_MINS - ) - - override fun visitClass(klass: KtClass) { - bindingContext != BindingContext.EMPTY ?: return - if (extendsSpek(klass)) { - val lambda = getInitLambda(klass) ?: return - inspectSpekGroup(lambda) - } - } - - private fun inspectSpekGroup(lambda: KtLambdaExpression) { - lambda.bodyExpression?.statements?.forEach { - when (it) { - is KtProperty -> handleProperties(it) - is KtCallExpression -> handleScopingFunctions(it) - } - } - } - - private fun handleProperties(property: KtProperty) { - if (!property.hasDelegate()) { - val initExpr = property.initializer - val fqType = initExpr?.getType(bindingContext) - ?.fqNameOrNull() - ?.asString() - if (fqType != null && fqType !in allowedTypes) { - report( - CodeSmell( - issue, - Entity.atName(property), - "Variable declarations which do not meet the allowed types should be memoized." - ) - ) - } - } - } - - private fun handleScopingFunctions(call: KtCallExpression) { - val calledName = call.calleeExpression?.text - if (calledName in scopingFunctions && call.valueArguments.isNotEmpty()) { - val scopingLambda = call.valueArguments.last() - .getArgumentExpression() - as? KtLambdaExpression - ?: return - inspectSpekGroup(scopingLambda) - } - } - - private fun extendsSpek(klass: KtClass): Boolean { - val entries = klass.superTypeListEntries - if (entries.size == 1) { - val entry = entries.first() - val superType = entry.typeReference?.text - return superType == "Spek" - } - return false - } - - private fun getInitLambda(klass: KtClass): KtLambdaExpression? { - val superType = klass.superTypeListEntries.first() as? KtSuperTypeCallEntry - if (superType?.valueArguments?.size == 1) { - val expr = superType.valueArguments.first().getArgumentExpression() - return expr as? KtLambdaExpression - } - return null - } - - companion object { - const val ALLOWED_TYPES = "allowedTypes" - const val SCOPING_FUNCTIONS = "scopingFunctions" - } -} diff --git a/custom-checks/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.ConfigValidator b/custom-checks/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.ConfigValidator deleted file mode 100644 index ff62c201bb3..00000000000 --- a/custom-checks/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.ConfigValidator +++ /dev/null @@ -1 +0,0 @@ -io.github.detekt.custom.CustomChecksConfigValidator diff --git a/custom-checks/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.RuleSetProvider b/custom-checks/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.RuleSetProvider deleted file mode 100644 index bde5c0144ee..00000000000 --- a/custom-checks/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.RuleSetProvider +++ /dev/null @@ -1 +0,0 @@ -io.github.detekt.custom.CustomChecksProvider diff --git a/custom-checks/src/test/kotlin/io/github/detekt/custom/SpekTestDiscoverySpec.kt b/custom-checks/src/test/kotlin/io/github/detekt/custom/SpekTestDiscoverySpec.kt deleted file mode 100644 index 4967713d725..00000000000 --- a/custom-checks/src/test/kotlin/io/github/detekt/custom/SpekTestDiscoverySpec.kt +++ /dev/null @@ -1,98 +0,0 @@ -package io.github.detekt.custom - -import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest -import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext -import org.assertj.core.api.Assertions.assertThat -import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment -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 - -@KotlinCoreEnvironmentTest -class SpekTestDiscoverySpec(private val env: KotlinCoreEnvironment) { - val subject = SpekTestDiscovery() - - @Nested - inner class VariableDeclarationsInSpekGroupsShouldOnlyBeSimple { - - @Nested - inner class TopLevelScope { - - @Test - fun `allows strings, paths and files by default`() { - val code = createSpekCode( - """ - val s = "simple" - val p = Paths.get("") - val f = File("") - """ - ) - - assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() - } - - @Test - fun `detects disallowed types on top level scope`() { - val code = createSpekCode("val s = Any()") - - assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) - } - - @Test - fun `allows memoized blocks`() { - val code = createSpekCode("val s by memoized { Any() }") - - assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() - } - } - - @Nested - inner class DescribeAndContextBlocks { - - @ParameterizedTest - @ValueSource(strings = ["describe", "context"]) - fun `allows strings, files and paths by default`(name: String) { - val code = createSpekCode( - """ - $name("group") { - val s = "simple" - val p = Paths.get("") - val f = File("") - val m by memoized { Any() } - } - """ - ) - - assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() - } - - @ParameterizedTest - @ValueSource(strings = ["describe", "context"]) - fun `disallows non memoized declarations`(name: String) { - val code = createSpekCode( - """ - $name("group") { - val complex = Any() - } - """ - ) - - assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) - } - } - } -} - -private fun createSpekCode(content: String) = """ -import org.spekframework.spek2.Spek -import org.spekframework.spek2.style.specification.describe -import java.io.File -import java.nio.file.Paths - -class Test : Spek({ - describe("top") { - $content - } -}) -""" diff --git a/detekt-test-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KotlinEnvironmentTestSetup.kt b/detekt-test-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KotlinEnvironmentTestSetup.kt index 856cc04622c..7daf4b41b84 100644 --- a/detekt-test-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KotlinEnvironmentTestSetup.kt +++ b/detekt-test-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KotlinEnvironmentTestSetup.kt @@ -11,6 +11,10 @@ import org.spekframework.spek2.dsl.Root import org.spekframework.spek2.lifecycle.CachingMode import java.nio.file.Path +@Deprecated( + "This is specific to Spek and will be removed in a future release. Documentation has been updated to " + + "show alternative approaches: https://detekt.dev/type-resolution.html#testing-a-rule-that-uses-type-resolution" +) fun Root.setupKotlinEnvironment(additionalJavaSourceRootPath: Path? = null) { val wrapper by memoized( CachingMode.SCOPE, diff --git a/docs/pages/extensions.md b/docs/pages/extensions.md index fe2cfeda190..0aa6d86be06 100644 --- a/docs/pages/extensions.md +++ b/docs/pages/extensions.md @@ -161,29 +161,29 @@ To let detekt know about the new processor, we specify a `resources/META-INF/ser with the full qualify name of our processor as the content: `io.gitlab.arturbosch.detekt.sample.extensions.processors.NumberOfLoopsProcessor`. -To test the code we use the `detekt-test` module and write a `Spek` testcase. +To test the code we use the `detekt-test` module and write a JUnit 5 testcase. ```kotlin -class NumberOfLoopsProcessorSpec : Spek({ - - it("should expect two loops") { - val code = """ - fun main() { - for (i in 0..10) { - while (i < 5) { - println(i) - } - } - } - """ - - val ktFile = compileContentForTest(code) - NumberOfLoopsProcessor().onProcess(ktFile) - - assertThat(ktFile.getUserData(NumberOfLoopsProcessor.numberOfLoopsKey)).isEqualTo(2) - } -}) - +class NumberOfLoopsProcessorTest { + + @Test + fun `should expect two loops`() { + val code = """ + fun main() { + for (i in 0..10) { + while (i < 5) { + println(i) + } + } + } + """ + + val ktFile = compileContentForTest(code) + NumberOfLoopsProcessor().onProcess(ktFile) + + assertThat(ktFile.getUserData(NumberOfLoopsProcessor.numberOfLoopsKey)).isEqualTo(2) + } +} ``` #### Custom Reports diff --git a/docs/pages/gettingstarted/type-resolution.md b/docs/pages/gettingstarted/type-resolution.md index 46e39924ed5..40d4a5283f1 100644 --- a/docs/pages/gettingstarted/type-resolution.md +++ b/docs/pages/gettingstarted/type-resolution.md @@ -111,10 +111,38 @@ If the `bindingContext` is not `EMPTY`, you are free to use it to resolve types To test a rule that uses type resolution, you can use the [`lintWithContext`](https://github.com/detekt/detekt/blob/d3546ff0d539d57e7a502dacbf66e91587fff098/detekt-test/src/main/kotlin/io/gitlab/arturbosch/detekt/test/RuleExtensions.kt#L40-L44) and [`compileAndLintWithContext`](https://github.com/detekt/detekt/blob/cd659ce8737fb177caf140f46f73a1a86b22be56/detekt-test/src/main/kotlin/io/gitlab/arturbosch/detekt/test/RuleExtensions.kt#L63-L72) extension functions. -If you're using Spek for testing, you can use the `setupKotlinEnvironment()` util function, and get access to the `KotlinCoreEnvironment` by simply calling `val env: KotlinCoreEnvironment by memoized()`: +If you're using JUnit 5 for testing, you can use the `@KotlinCoreEnvironmentTest` annotation on your test class, and +accept a parameter of type `KotlinCoreEnvironment` in the class constructor. You can then access the environment by +referencing the parameter specified in the constructor: ```kotlin -class MyRuleSpec : Spek({ +@KotlinCoreEnvironmentTest +class MyRuleSpec(private val env: KotlinCoreEnvironment) { + @Test + fun `reports cast that cannot succeed`() { + val code = """/* The code you want to test */""" + assertThat(MyRuleSpec().compileAndLintWithContext(env, code)).hasSize(1) + } +} +``` + +If you're using Spek for testing, you can create a `setupKotlinEnvironment()` util function, and get access to the +`KotlinCoreEnvironment` by simply calling `val env: KotlinCoreEnvironment by memoized()`: + +```kotlin +fun org.spekframework.spek2.dsl.Root.setupKotlinEnvironment(additionalJavaSourceRootPath: Path? = null) { + val wrapper by memoized( + CachingMode.SCOPE, + { createEnvironment(additionalJavaSourceRootPaths = listOfNotNull(additionalJavaSourceRootPath?.toFile())) }, + { it.dispose() } + ) + + // `env` name is used for delegation + @Suppress("UNUSED_VARIABLE") + val env: KotlinCoreEnvironment by memoized(CachingMode.EACH_GROUP) { wrapper.env } +} + +class MyRuleTest : Spek({ setupKotlinEnvironment() val env: KotlinCoreEnvironment by memoized() @@ -126,4 +154,4 @@ class MyRuleSpec : Spek({ }) ``` -If you're using another testing framework (e.g. JUnit), you can use the [`createEnvironment()`](https://github.com/detekt/detekt/blob/cd659ce8737fb177caf140f46f73a1a86b22be56/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/KotlinCoreEnvironmentWrapper.kt#L26-L31) method from `detekt-test-utils`. +If you're using another testing framework (e.g. JUnit 4), you can use the [`createEnvironment()`](https://github.com/detekt/detekt/blob/cd659ce8737fb177caf140f46f73a1a86b22be56/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/KotlinCoreEnvironmentWrapper.kt#L26-L31) method from `detekt-test-utils`. diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 858a7f38517..520b8220839 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -3,7 +3,6 @@ dokka = "1.6.10" jacoco = "0.8.8" kotlin = "1.6.20" ktlint = "0.45.2" -spek = "2.0.18" junit = "5.8.2" [libraries] @@ -29,8 +28,7 @@ ktlint-rulesetExperimental = { module = "com.pinterest.ktlint:ktlint-ruleset-exp dokka-jekyll = { module = "org.jetbrains.dokka:jekyll-plugin", version.ref = "dokka"} -spek-dsl = { module = "org.spekframework.spek2:spek-dsl-jvm", version.ref = "spek" } -spek-runner = { module = "org.spekframework.spek2:spek-runner-junit5", version.ref = "spek" } +spek-dsl = { module = "org.spekframework.spek2:spek-dsl-jvm", version = "2.0.18" } junit-api = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "junit" } @@ -49,6 +47,3 @@ gradleVersions = { id = "com.github.ben-manes.versions", version = "0.42.0" } pluginPublishing = { id = "com.gradle.plugin-publish", version = "0.21.0" } shadow = { id = "com.github.johnrengelman.shadow", version = "7.1.2" } sonarqube = { id = "org.sonarqube", version = "3.3" } - -[bundles] -testImplementation = ["assertj", "spek-dsl"] diff --git a/settings.gradle.kts b/settings.gradle.kts index a6d39a3f8a7..4d8e46b4e23 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -7,7 +7,6 @@ pluginManagement { } include("code-coverage-report") -include("custom-checks") include("detekt-api") include("detekt-cli") include("detekt-core")