From 09bb4fa9e5477086ac73859b85fd8a928f42074d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Wed, 19 May 2021 18:38:18 +0200 Subject: [PATCH 1/4] Revert "Noisy gradle (#3655)" (#3792) This reverts commit 2c4b6fb421e4abdf8ffa24bc0af63df856dbfc41. --- .../gitlab/arturbosch/detekt/DetektPlugin.kt | 2 ++ .../arturbosch/detekt/internal/DetektPlain.kt | 2 ++ .../arturbosch/detekt/internal/SharedTasks.kt | 5 ----- .../arturbosch/detekt/DetektAndroidTest.kt | 15 -------------- .../gitlab/arturbosch/detekt/DetektJvmTest.kt | 4 ---- .../arturbosch/detekt/DetektPlainTest.kt | 4 ---- .../arturbosch/detekt/DetektTaskDslTest.kt | 7 ++----- .../detekt/testkit/DslGradleRunner.kt | 20 ------------------- 8 files changed, 6 insertions(+), 53 deletions(-) diff --git a/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt b/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt index c03a9069b..292a27c53 100644 --- a/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt +++ b/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt @@ -106,6 +106,8 @@ class DetektPlugin : Plugin { const val BASELINE_TASK_NAME = "detektBaseline" const val DETEKT_EXTENSION = "detekt" private const val GENERATE_CONFIG = "detektGenerateConfig" + internal val defaultExcludes = listOf("build/") + internal val defaultIncludes = listOf("**/*.kt", "**/*.kts") internal const val CONFIG_DIR_NAME = "config/detekt" internal const val CONFIG_FILE = "detekt.yml" diff --git a/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/internal/DetektPlain.kt b/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/internal/DetektPlain.kt index f8c3dacd8..7277b49e2 100644 --- a/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/internal/DetektPlain.kt +++ b/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/internal/DetektPlain.kt @@ -20,6 +20,8 @@ internal class DetektPlain(private val project: Project) { baseline.set(project.layout.file(project.provider { baselineFile })) } setSource(existingInputDirectoriesProvider(project, extension)) + setIncludes(DetektPlugin.defaultIncludes) + setExcludes(DetektPlugin.defaultExcludes) reportsDir.set(project.provider { extension.customReportsDir }) reports = extension.reports } diff --git a/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/internal/SharedTasks.kt b/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/internal/SharedTasks.kt index ac43e5ba1..9d8528355 100644 --- a/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/internal/SharedTasks.kt +++ b/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/internal/SharedTasks.kt @@ -24,8 +24,6 @@ internal fun Project.registerDetektTask( it.ignoreFailuresProp.set(project.provider { extension.ignoreFailures }) it.basePathProp.set(extension.basePath) it.allRulesProp.set(provider { extension.allRules }) - it.setIncludes(DEFAULT_INCLUDES) - it.setExcludes(DEFAULT_EXCLUDES) configuration(it) } @@ -52,6 +50,3 @@ internal fun DetektReport.setDefaultIfUnset(default: File) { destination = default } } - -private val DEFAULT_EXCLUDES = listOf("build/") -private val DEFAULT_INCLUDES = listOf("**/*.kt", "**/*.kts") diff --git a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektAndroidTest.kt b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektAndroidTest.kt index aa5869cb3..f4c76edcb 100644 --- a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektAndroidTest.kt +++ b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektAndroidTest.kt @@ -2,7 +2,6 @@ package io.gitlab.arturbosch.detekt import io.gitlab.arturbosch.detekt.testkit.DslGradleRunner import io.gitlab.arturbosch.detekt.testkit.ProjectLayout -import io.gitlab.arturbosch.detekt.testkit.createJavaClass import org.assertj.core.api.Assertions.assertThat import org.spekframework.spek2.Spek import org.spekframework.spek2.dsl.Skip @@ -39,7 +38,6 @@ object DetektAndroidTest : Spek({ } val gradleRunner = createGradleRunnerAndSetupProject(projectLayout) gradleRunner.writeProjectFile("app/src/main/AndroidManifest.xml", MANIFEST_CONTENT) - gradleRunner.createJavaClassAtSubmodule("AJavaClass") it("task :app:detektMain") { gradleRunner.runTasksAndCheckResult(":app:detektMain") { buildResult -> @@ -48,7 +46,6 @@ object DetektAndroidTest : Spek({ assertThat(buildResult.output).contains("--report xml:") assertThat(buildResult.output).contains("--report sarif:") assertThat(buildResult.output).doesNotContain("--report txt:") - assertThat(buildResult.output).doesNotContain("AJavaClass.java") assertThat(buildResult.tasks.map { it.path }).containsAll( listOf( ":app:detektMain", @@ -66,7 +63,6 @@ object DetektAndroidTest : Spek({ assertThat(buildResult.output).contains("--report xml:") assertThat(buildResult.output).contains("--report sarif:") assertThat(buildResult.output).doesNotContain("--report txt:") - assertThat(buildResult.output).doesNotContain("AJavaClassTest.java") assertThat(buildResult.tasks.map { it.path }).containsAll( listOf( ":app:detektDebugUnitTest", @@ -83,8 +79,6 @@ object DetektAndroidTest : Spek({ assertThat(buildResult.output).contains("--report xml:") assertThat(buildResult.output).contains("--report sarif:") assertThat(buildResult.output).doesNotContain("--report txt:") - assertThat(buildResult.output).doesNotContain("AJavaClass.java") - assertThat(buildResult.output).doesNotContain("AJavaClassTest.java") } } } @@ -148,7 +142,6 @@ object DetektAndroidTest : Spek({ } val gradleRunner = createGradleRunnerAndSetupProject(projectLayout) gradleRunner.writeProjectFile("lib/src/main/AndroidManifest.xml", MANIFEST_CONTENT) - gradleRunner.createJavaClassAtSubmodule("AJavaClass") it("task :lib:detektMain") { gradleRunner.runTasksAndCheckResult(":lib:detektMain") { buildResult -> @@ -157,7 +150,6 @@ object DetektAndroidTest : Spek({ assertThat(buildResult.output).contains("--report xml:") assertThat(buildResult.output).contains("--report sarif:") assertThat(buildResult.output).doesNotContain("--report txt:") - assertThat(buildResult.output).doesNotContain("AJavaClass.java") assertThat(buildResult.tasks.map { it.path }).containsAll( listOf( ":lib:detektMain", @@ -175,7 +167,6 @@ object DetektAndroidTest : Spek({ assertThat(buildResult.output).contains("--report xml:") assertThat(buildResult.output).contains("--report sarif:") assertThat(buildResult.output).doesNotContain("--report txt:") - assertThat(buildResult.output).doesNotContain("AJavaClassTest.java") assertThat(buildResult.tasks.map { it.path }).containsAll( listOf( ":lib:detektDebugUnitTest", @@ -192,8 +183,6 @@ object DetektAndroidTest : Spek({ assertThat(buildResult.output).contains("--report xml:") assertThat(buildResult.output).contains("--report sarif:") assertThat(buildResult.output).doesNotContain("--report txt:") - assertThat(buildResult.output).doesNotContain("AJavaClass.java") - assertThat(buildResult.output).doesNotContain("AJavaClassTest.java") } } } @@ -467,7 +456,3 @@ private fun createGradleRunnerAndSetupProject(projectLayout: ProjectLayout) = Ds """.trimIndent(), dryRun = true ).also { it.setupProject() } - -private fun DslGradleRunner.createJavaClassAtSubmodule(name: String) { - createJavaClass(projectLayout.submodules.first(), name) -} diff --git a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektJvmTest.kt b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektJvmTest.kt index 6f72d58b7..f105d6015 100644 --- a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektJvmTest.kt +++ b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektJvmTest.kt @@ -2,7 +2,6 @@ package io.gitlab.arturbosch.detekt import io.gitlab.arturbosch.detekt.testkit.DslGradleRunner import io.gitlab.arturbosch.detekt.testkit.ProjectLayout -import io.gitlab.arturbosch.detekt.testkit.createJavaClass import org.assertj.core.api.Assertions.assertThat import org.spekframework.spek2.Spek import org.spekframework.spek2.style.specification.describe @@ -35,7 +34,6 @@ object DetektJvmTest : Spek({ dryRun = true ) gradleRunner.setupProject() - gradleRunner.createJavaClass("AJavaClass") it("configures detekt type resolution task main") { gradleRunner.runTasksAndCheckResult(":detektMain") { buildResult -> @@ -44,7 +42,6 @@ object DetektJvmTest : Spek({ assertThat(buildResult.output).contains("--report sarif:") assertThat(buildResult.output).doesNotContain("--report txt:") assertThat(buildResult.output).contains("--classpath") - assertThat(buildResult.output).doesNotContain("AJavaClass.java") } } @@ -55,7 +52,6 @@ object DetektJvmTest : Spek({ assertThat(buildResult.output).contains("--report sarif:") assertThat(buildResult.output).doesNotContain("--report txt:") assertThat(buildResult.output).contains("--classpath") - assertThat(buildResult.output).doesNotContain("AJavaClassTest.java") } } } diff --git a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektPlainTest.kt b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektPlainTest.kt index 5f8c3d10f..3cc78b67d 100644 --- a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektPlainTest.kt +++ b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektPlainTest.kt @@ -2,7 +2,6 @@ package io.gitlab.arturbosch.detekt import io.gitlab.arturbosch.detekt.testkit.DslGradleRunner import io.gitlab.arturbosch.detekt.testkit.ProjectLayout -import io.gitlab.arturbosch.detekt.testkit.createJavaClass import org.assertj.core.api.Assertions.assertThat import org.spekframework.spek2.Spek import org.spekframework.spek2.style.specification.describe @@ -65,7 +64,6 @@ object DetektPlainTest : Spek({ dryRun = true ) gradleRunner.setupProject() - gradleRunner.createJavaClass("AJavaClass") it("configures detekt plain task") { gradleRunner.runTasksAndCheckResult(":detekt") { buildResult -> @@ -74,8 +72,6 @@ object DetektPlainTest : Spek({ assertThat(buildResult.output).contains("--report sarif:") assertThat(buildResult.output).doesNotContain("--report txt:") assertThat(buildResult.output).doesNotContain("--classpath") - assertThat(buildResult.output).doesNotContain("AJavaClass.java") - assertThat(buildResult.output).doesNotContain("AJavaClassTest.java") } } } diff --git a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektTaskDslTest.kt b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektTaskDslTest.kt index ea535749b..80aec466c 100644 --- a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektTaskDslTest.kt +++ b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/DetektTaskDslTest.kt @@ -143,10 +143,7 @@ internal object DetektTaskDslTest : Spek({ |} """ - val projectLayout = ProjectLayout( - numberOfSourceFilesInRootPerSourceDir = 1, - srcDirs = listOf(customSrc1, customSrc2) - ) + val projectLayout = ProjectLayout(1, srcDirs = listOf(customSrc1, customSrc2)) gradleRunner = builder .withProjectLayout(projectLayout) .withDetektConfig(config) @@ -157,7 +154,7 @@ internal object DetektTaskDslTest : Spek({ it("sets input parameter to absolute filenames of all source files") { val file1 = gradleRunner.projectFile("$customSrc1/My0Root0Class.kt") val file2 = gradleRunner.projectFile("$customSrc2/My1Root0Class.kt") - val expectedInputParam = "--input $file1,$file2 " + val expectedInputParam = "--input $file1,$file2" assertThat(result.output).contains(expectedInputParam) } diff --git a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/testkit/DslGradleRunner.kt b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/testkit/DslGradleRunner.kt index a7ae8d5ff..afe05c376 100644 --- a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/testkit/DslGradleRunner.kt +++ b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/testkit/DslGradleRunner.kt @@ -97,8 +97,6 @@ class DslGradleRunner @Suppress("LongParameterList") constructor( writeKtFile(File(rootDir, srcDir), className) } - fun Submodule.projectFile(path: String): File = File(moduleRoot, path).canonicalFile - private fun writeKtFile(dir: File, className: String, withCodeSmell: Boolean = false) { dir.mkdirs() File(dir, "$className.kt").writeText(ktFileContent(className, withCodeSmell)) @@ -159,21 +157,3 @@ class DslGradleRunner @Suppress("LongParameterList") constructor( private const val DETEKT_TASK = "detekt" } } - -fun DslGradleRunner.createJavaClass(name: String) { - projectFile("${projectLayout.srcDirs.first { it.contains("main") }}/$name.java") - .apply { createNewFile() } - .writeText("public class $name {}") - projectFile("${projectLayout.srcDirs.first { it.contains("test") }}/${name}Test.java") - .apply { createNewFile() } - .writeText("public class ${name}Test {}") -} - -fun DslGradleRunner.createJavaClass(submodule: Submodule, name: String) { - submodule.projectFile("${submodule.srcDirs.first { it.contains("main") }}/$name.java") - .apply { createNewFile() } - .writeText("public class $name {}") - submodule.projectFile("${submodule.srcDirs.first { it.contains("test") }}/${name}Test.java") - .apply { createNewFile() } - .writeText("public class ${name}Test {}") -} From f319af9128429c0121c8798ad8843a91c50178c5 Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Wed, 19 May 2021 05:12:12 +0900 Subject: [PATCH 2/4] NoNameShadowing: don't report when implicit 'it' parameter isn't used (#3793) --- detekt-psi-utils/api/detekt-psi-utils.api | 5 +++ .../detekt/rules/KtLambdaExpression.kt | 33 +++++++++++++++++++ .../detekt/rules/naming/NoNameShadowing.kt | 15 +++++---- .../rules/naming/NoNameShadowingSpec.kt | 6 ++-- .../rules/style/MultilineLambdaItParameter.kt | 30 ++--------------- 5 files changed, 50 insertions(+), 39 deletions(-) create mode 100644 detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtLambdaExpression.kt diff --git a/detekt-psi-utils/api/detekt-psi-utils.api b/detekt-psi-utils/api/detekt-psi-utils.api index f2973caec..4b9c630c8 100644 --- a/detekt-psi-utils/api/detekt-psi-utils.api +++ b/detekt-psi-utils/api/detekt-psi-utils.api @@ -83,6 +83,11 @@ public final class io/gitlab/arturbosch/detekt/rules/KtCallExpressionKt { public static final fun isCallingWithNonNullCheckArgument (Lorg/jetbrains/kotlin/psi/KtCallExpression;Lorg/jetbrains/kotlin/name/FqName;Lorg/jetbrains/kotlin/resolve/BindingContext;)Z } +public final class io/gitlab/arturbosch/detekt/rules/KtLambdaExpressionKt { + public static final fun hasImplicitParameterReference (Lorg/jetbrains/kotlin/psi/KtLambdaExpression;Lorg/jetbrains/kotlin/descriptors/ValueParameterDescriptor;Lorg/jetbrains/kotlin/resolve/BindingContext;)Z + public static final fun implicitParameter (Lorg/jetbrains/kotlin/psi/KtLambdaExpression;Lorg/jetbrains/kotlin/resolve/BindingContext;)Lorg/jetbrains/kotlin/descriptors/ValueParameterDescriptor; +} + public final class io/gitlab/arturbosch/detekt/rules/KtModifierListKt { public static final fun isAbstract (Lorg/jetbrains/kotlin/psi/KtModifierListOwner;)Z public static final fun isActual (Lorg/jetbrains/kotlin/psi/KtModifierListOwner;)Z diff --git a/detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtLambdaExpression.kt b/detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtLambdaExpression.kt new file mode 100644 index 000000000..a79e74cc7 --- /dev/null +++ b/detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtLambdaExpression.kt @@ -0,0 +1,33 @@ +package io.gitlab.arturbosch.detekt.rules + +import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor +import org.jetbrains.kotlin.psi.KtLambdaExpression +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType +import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall + +fun KtLambdaExpression.implicitParameter(bindingContext: BindingContext): ValueParameterDescriptor? { + if (valueParameters.isNotEmpty()) return null + return bindingContext[BindingContext.FUNCTION, functionLiteral]?.valueParameters?.singleOrNull() +} + +fun KtLambdaExpression.hasImplicitParameterReference( + implicitParameter: ValueParameterDescriptor, + bindingContext: BindingContext +): Boolean { + return anyDescendantOfType { + it.isImplicitParameterReference(this, implicitParameter, bindingContext) + } +} + +private fun KtNameReferenceExpression.isImplicitParameterReference( + lambda: KtLambdaExpression, + implicitParameter: ValueParameterDescriptor, + bindingContext: BindingContext +): Boolean { + return text == "it" && + getStrictParentOfType() == lambda && + getResolvedCall(bindingContext)?.resultingDescriptor == implicitParameter +} diff --git a/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/NoNameShadowing.kt b/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/NoNameShadowing.kt index 6dad173e3..569b59e30 100644 --- a/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/NoNameShadowing.kt +++ b/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/NoNameShadowing.kt @@ -8,6 +8,8 @@ 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.internal.RequiresTypeResolution +import io.gitlab.arturbosch.detekt.rules.hasImplicitParameterReference +import io.gitlab.arturbosch.detekt.rules.implicitParameter import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.psi.KtDestructuringDeclarationEntry import org.jetbrains.kotlin.psi.KtLambdaExpression @@ -81,8 +83,9 @@ class NoNameShadowing(config: Config = Config.empty) : Rule(config) { override fun visitLambdaExpression(lambdaExpression: KtLambdaExpression) { super.visitLambdaExpression(lambdaExpression) - if (bindingContext != BindingContext.EMPTY && - lambdaExpression.hasImplicitParameter() && + if (bindingContext == BindingContext.EMPTY) return + val implicitParameter = lambdaExpression.implicitParameter(bindingContext) ?: return + if (lambdaExpression.hasImplicitParameterReference(implicitParameter, bindingContext) && lambdaExpression.hasParentImplicitParameterLambda() ) { report( @@ -95,10 +98,8 @@ class NoNameShadowing(config: Config = Config.empty) : Rule(config) { } } - private fun KtLambdaExpression.hasImplicitParameter(): Boolean = - valueParameters.isEmpty() && - bindingContext[BindingContext.FUNCTION, functionLiteral]?.valueParameters?.singleOrNull() != null - private fun KtLambdaExpression.hasParentImplicitParameterLambda(): Boolean = - getParentOfTypesAndPredicate(true, KtLambdaExpression::class.java) { it.hasImplicitParameter() } != null + getParentOfTypesAndPredicate(true, KtLambdaExpression::class.java) { + implicitParameter(bindingContext) != null + } != null } diff --git a/detekt-rules-naming/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/naming/NoNameShadowingSpec.kt b/detekt-rules-naming/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/naming/NoNameShadowingSpec.kt index 7cbf788bb..b9f3d0ab6 100644 --- a/detekt-rules-naming/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/naming/NoNameShadowingSpec.kt +++ b/detekt-rules-naming/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/naming/NoNameShadowingSpec.kt @@ -62,7 +62,7 @@ class NoNameShadowingSpec : Spek({ assertThat(findings[0]).hasMessage("Name shadowed: it") } - it("report shadowing nested lambda implicit 'it' parameter") { + it("does not report when implicit 'it' parameter isn't used") { val code = """ fun test() { listOf(1).forEach { @@ -72,9 +72,7 @@ class NoNameShadowingSpec : Spek({ } """ val findings = subject.compileAndLintWithContext(env, code) - assertThat(findings).hasSize(1) - assertThat(findings).hasSourceLocation(3, 27) - assertThat(findings[0]).hasMessage("Name shadowed: implicit lambda parameter 'it'") + assertThat(findings).isEmpty() } it("does not report not shadowing variable") { diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameter.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameter.kt index 8c459be41..13437be43 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameter.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MultilineLambdaItParameter.kt @@ -9,13 +9,10 @@ import io.gitlab.arturbosch.detekt.api.Rule import io.gitlab.arturbosch.detekt.api.Severity import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution import io.gitlab.arturbosch.detekt.rules.IT_LITERAL -import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor +import io.gitlab.arturbosch.detekt.rules.hasImplicitParameterReference +import io.gitlab.arturbosch.detekt.rules.implicitParameter import org.jetbrains.kotlin.psi.KtLambdaExpression -import org.jetbrains.kotlin.psi.KtNameReferenceExpression -import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType -import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.resolve.BindingContext -import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall /** * Lambda expressions are very useful in a lot of cases and they often include very small chunks of @@ -113,27 +110,4 @@ class MultilineLambdaItParameter(val config: Config) : Rule(config) { else -> { } } } - - private fun KtLambdaExpression.implicitParameter(bindingContext: BindingContext): ValueParameterDescriptor? { - return bindingContext[BindingContext.FUNCTION, functionLiteral]?.valueParameters?.singleOrNull() - } - - private fun KtLambdaExpression.hasImplicitParameterReference( - implicitParameter: ValueParameterDescriptor, - bindingContext: BindingContext - ): Boolean { - return anyDescendantOfType { - it.isImplicitParameterReference(this, implicitParameter, bindingContext) - } - } - - private fun KtNameReferenceExpression.isImplicitParameterReference( - lambda: KtLambdaExpression, - implicitParameter: ValueParameterDescriptor, - bindingContext: BindingContext - ): Boolean { - return text == "it" && - getStrictParentOfType() == lambda && - getResolvedCall(bindingContext)?.resultingDescriptor == implicitParameter - } } From 35b9f6331dd8fc64394a5c655561c3c43c62790d Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Wed, 19 May 2021 05:04:04 +0900 Subject: [PATCH 3/4] UnnecessaryLet: report when implicit parameter isn't used (#3794) --- .../detekt/rules/style/UnnecessaryLet.kt | 14 +++++++++----- .../detekt/rules/style/UnnecessaryLetSpec.kt | 13 +++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryLet.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryLet.kt index 56b9f59ae..f9f09aa64 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryLet.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryLet.kt @@ -17,7 +17,9 @@ import org.jetbrains.kotlin.psi.KtCallExpression import org.jetbrains.kotlin.psi.KtDotQualifiedExpression import org.jetbrains.kotlin.psi.KtLambdaExpression import org.jetbrains.kotlin.psi.KtSafeQualifiedExpression -import org.jetbrains.kotlin.psi.ValueArgument +import org.jetbrains.kotlin.psi.KtSimpleNameExpression +import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType +import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.resolve.BindingContext /** @@ -117,16 +119,18 @@ private val KtLambdaExpression.firstParameter get() = valueParameters.firstOrNul private fun KtBlockExpression.hasOnlyOneStatement() = this.children.size == 1 -private fun PsiElement.countVarRefs(varName: String): Int = - children.sumBy { it.countVarRefs(varName) + if (it.textMatches(varName) && it !is ValueArgument) 1 else 0 } +private fun PsiElement.countVarRefs(varName: String, lambda: KtLambdaExpression): Int = + collectDescendantsOfType { + it.textMatches(varName) && it.getStrictParentOfType() == lambda + }.count() private fun KtLambdaExpression.countReferences(): Int { val bodyExpression = bodyExpression ?: return 0 val destructuringDeclaration = firstParameter?.destructuringDeclaration return if (destructuringDeclaration != null) { - destructuringDeclaration.entries.sumBy { bodyExpression.countVarRefs(it.nameAsSafeName.asString()) } + destructuringDeclaration.entries.sumBy { bodyExpression.countVarRefs(it.nameAsSafeName.asString(), this) } } else { val parameterName = firstParameter?.nameAsSafeName?.asString() ?: IT_LITERAL - bodyExpression.countVarRefs(parameterName) + bodyExpression.countVarRefs(parameterName, this) } } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryLetSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryLetSpec.kt index 367ca342f..7a79701ad 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryLetSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryLetSpec.kt @@ -344,6 +344,19 @@ class UnnecessaryLetSpec : Spek({ assertThat(findings).hasSize(1) assertThat(findings).allMatch { it.message == MESSAGE_USE_IF } } + + it("reports when implicit parameter isn't used") { + val content = """ + fun test(value: Int?) { + value?.let { + listOf(1).map { it } + } + } + """ + val findings = subject.compileAndLintWithContext(env, content) + assertThat(findings).hasSize(1) + assertThat(findings).allMatch { it.message == MESSAGE_USE_IF } + } } } }) From 40c3330859d13aba6b92b88acc616ef4266c486d Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Wed, 19 May 2021 19:23:13 +0200 Subject: [PATCH 4/4] Prepare Detekt 1.17.1 --- buildSrc/src/main/kotlin/Versions.kt | 2 +- docs/_config.yml | 2 +- docs/pages/changelog 1.x.x.md | 16 ++++++++++++++++ gradle/libs.versions.toml | 2 +- 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/buildSrc/src/main/kotlin/Versions.kt b/buildSrc/src/main/kotlin/Versions.kt index 82088796e..007a1593a 100644 --- a/buildSrc/src/main/kotlin/Versions.kt +++ b/buildSrc/src/main/kotlin/Versions.kt @@ -1,6 +1,6 @@ object Versions { - const val DETEKT: String = "1.17.0" + const val DETEKT: String = "1.17.1" const val SNAPSHOT_NAME: String = "main" const val JVM_TARGET: String = "1.8" diff --git a/docs/_config.yml b/docs/_config.yml index da20d9c1c..01b040801 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -117,4 +117,4 @@ description: "Meet detekt, a static code analysis tool for Kotlin." url: https://detekt.github.io baseurl: /detekt -detekt_version: 1.17.0 +detekt_version: 1.17.1 diff --git a/docs/pages/changelog 1.x.x.md b/docs/pages/changelog 1.x.x.md index d97f79065..7c3422478 100644 --- a/docs/pages/changelog 1.x.x.md +++ b/docs/pages/changelog 1.x.x.md @@ -6,6 +6,22 @@ permalink: changelog.html toc: true --- +#### 1.17.1 - 2021-05-19 + +##### Notable Changes + +This is a patch release for Detekt `1.17.0` including fixes that we considered worth a point release. + +Specifically, we're reverting a change on our Gradle Plugin. The original change [#3655](https://github.com/detekt/detekt/pull/3655) resulted in several false positives when using rules with Type Resolution on Java/Kotlin mixed codebases. + +Moreover we included a couple of false positive fixes for `NoNameShadowing` and `UnnecessaryLet` + +##### Changelog + +- Revert "Noisy gradle (#3655)" - [#3792](https://github.com/detekt/detekt/pull/3792) +- NoNameShadowing: don't report when implicit 'it' parameter isn't used - [#3793](https://github.com/detekt/detekt/pull/3793) +- UnnecessaryLet: report when implicit parameter isn't used - [#3794](https://github.com/detekt/detekt/pull/3794) + #### 1.17.0 - 2021-05-15 ##### Notable Changes diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index cd5e6ab08..5d69314c7 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -7,7 +7,7 @@ spek = "2.0.15" [libraries] binaryCompatibilityValidator-gradlePlugin = "org.jetbrains.kotlinx:binary-compatibility-validator:0.4.0" -detekt-gradlePlugin = "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.17.0" +detekt-gradlePlugin = "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.17.1" githubRelease-gradlePlugin = "com.github.breadmoirai:github-release:2.2.12" gradleVersions-gradlePlugin = "com.github.ben-manes:gradle-versions-plugin:0.28.0" nexusStaging-gradlePlugin = "io.codearte.gradle.nexus:gradle-nexus-staging-plugin:0.22.0"