From 6400988529d4feb08a6358aa647d7876c72f2f6e Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Tue, 11 Apr 2023 03:34:11 +0530 Subject: [PATCH 01/30] Add valuePatterns list of regexes --- .../detekt/rules/style/ForbiddenComment.kt | 24 +++++++++ .../rules/style/ForbiddenCommentSpec.kt | 52 +++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 675eb09a820..71494adbd4d 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -40,6 +40,12 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { @Configuration("forbidden comment strings") private val values: List by config(listOf("FIXME:", "STOPSHIP:", "TODO:")) + @Suppress("ExplicitItLambdaParameter") + @Configuration("forbidden comment string patterns") + private val valuePatterns: List by config(emptyList()) { it: List -> + it.map(String::toRegex) + } + @Configuration("ignores comments which match the specified regular expression. For example `Ticket|Task`.") private val allowedPatterns: Regex by config("", String::toRegex) @@ -74,6 +80,23 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { ) } } + + val strippedText = if (text.startsWith("// ")) { + text.substring(COMMENT_START_INDEX) + } else { + text.substring(COMMENT_START_INDEX - 1) + } + valuePatterns.forEach { + if (it.containsMatchIn(strippedText)) { + report( + CodeSmell( + issue, + Entity.from(comment), + getErrorMessage(it.pattern) + ) + ) + } + } } private fun getErrorMessage(value: String): String = @@ -82,5 +105,6 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { companion object { const val DEFAULT_ERROR_MESSAGE = "This comment contains '%s' " + "that has been defined as forbidden in detekt." + const val COMMENT_START_INDEX = 3 } } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index 2a4fa008b24..2eece64fbb3 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -1,3 +1,5 @@ +@file:Suppress("ClassName") + package io.gitlab.arturbosch.detekt.rules.style import io.gitlab.arturbosch.detekt.test.TestConfig @@ -8,6 +10,7 @@ import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test private const val VALUES = "values" +private const val VALUES_PATTERNS = "valuePatterns" private const val ALLOWED_PATTERNS = "allowedPatterns" private const val MESSAGE = "customMessage" @@ -235,4 +238,53 @@ class ForbiddenCommentSpec { assertThat(findings.first().message).isEqualTo(expectedMessage) } } + + @Nested + inner class `custom value pattern is configured` { + private val patternStr = """^(?i)REVIEW\b""" + private val messageConfig = TestConfig( + VALUES_PATTERNS to listOf("STOPSHIP", patternStr), + ) + + @Test + fun `should not report a finding when review doesn't match the pattern`() { + val comment = "// to express in the preview that it's not a normal TextView." + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).isEmpty() + } + + @Test + fun `should report a finding when STOPSHIP is present`() { + val comment = "// STOPSHIP to express in the preview that it's not a normal TextView." + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).hasSize(1) + io.gitlab.arturbosch.detekt.test.assertThat(findings[0]) + .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, "STOPSHIP")) + } + + @Test + fun `should report a finding when review pattern is matched with comment with leading space`() { + val comment = "// REVIEW foo -> flag" + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).hasSize(1) + io.gitlab.arturbosch.detekt.test.assertThat(findings[0]) + .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, patternStr)) + } + + @Test + fun `should report a finding when review pattern is matched with comment with out leading space`() { + val comment = "//REVIEW foo -> flag" + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).hasSize(1) + io.gitlab.arturbosch.detekt.test.assertThat(findings[0]) + .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, patternStr)) + } + + @Test + fun `should report a finding matching two patterns`() { + val comment = "// REVIEW foo -> flag STOPSHIP" + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).hasSize(2) + } + } } From 5eba48bda11a5bd7b17f5bbc73b4e5f1e7210be8 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Wed, 12 Apr 2023 01:17:00 +0530 Subject: [PATCH 02/30] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Róbert Papp --- .../arturbosch/detekt/rules/style/ForbiddenComment.kt | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 71494adbd4d..cfff6e9298d 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -40,9 +40,8 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { @Configuration("forbidden comment strings") private val values: List by config(listOf("FIXME:", "STOPSHIP:", "TODO:")) - @Suppress("ExplicitItLambdaParameter") @Configuration("forbidden comment string patterns") - private val valuePatterns: List by config(emptyList()) { it: List -> + private val valuePatterns: List by config(emptyList()) { it.map(String::toRegex) } @@ -81,11 +80,7 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { } } - val strippedText = if (text.startsWith("// ")) { - text.substring(COMMENT_START_INDEX) - } else { - text.substring(COMMENT_START_INDEX - 1) - } + val strippedText = text.removePrefix("//").removePrefix(" ") valuePatterns.forEach { if (it.containsMatchIn(strippedText)) { report( @@ -105,6 +100,5 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { companion object { const val DEFAULT_ERROR_MESSAGE = "This comment contains '%s' " + "that has been defined as forbidden in detekt." - const val COMMENT_START_INDEX = 3 } } From 6ef834c08482f29af850bbc2e3a04600431e822f Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Wed, 12 Apr 2023 01:42:02 +0530 Subject: [PATCH 03/30] Deprecate values option --- .../src/main/resources/default-detekt-config.yml | 2 +- .../src/main/resources/deprecation.properties | 1 + .../detekt/rules/style/ForbiddenComment.kt | 6 ++++-- .../detekt/rules/style/ForbiddenCommentSpec.kt | 13 +++++++++++-- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 92f37616b9a..a93913a00fd 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -566,7 +566,7 @@ style: value: 'java.lang.annotation.Inherited' ForbiddenComment: active: true - values: + valuePatterns: - 'FIXME:' - 'STOPSHIP:' - 'TODO:' diff --git a/detekt-core/src/main/resources/deprecation.properties b/detekt-core/src/main/resources/deprecation.properties index a7136b5a3a3..133f9e310f8 100644 --- a/detekt-core/src/main/resources/deprecation.properties +++ b/detekt-core/src/main/resources/deprecation.properties @@ -16,6 +16,7 @@ potential-bugs>IgnoredReturnValue>restrictToAnnotatedMethods=Use `restrictToConf potential-bugs>LateinitUsage>excludeAnnotatedProperties=Use `ignoreAnnotated` instead potential-bugs>MissingWhenCase=Rule deprecated as compiler performs this check by default potential-bugs>RedundantElseInWhen=Rule deprecated as compiler performs this check by default +style>ForbiddenComment>values=Use `valuePatterns` instead style>ForbiddenPublicDataClass=Rule migrated to `libraries` ruleset plugin style>FunctionOnlyReturningConstant>excludeAnnotatedFunction=Use `ignoreAnnotated` instead style>LibraryCodeMustSpecifyReturnType=Rule migrated to `libraries` ruleset plugin diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index cfff6e9298d..88ba3eb6117 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -38,10 +38,11 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { ) @Configuration("forbidden comment strings") - private val values: List by config(listOf("FIXME:", "STOPSHIP:", "TODO:")) + @Deprecated("Use `valuePatterns` instead") + private val values: List by config(emptyList()) @Configuration("forbidden comment string patterns") - private val valuePatterns: List by config(emptyList()) { + private val valuePatterns: List by config(listOf("FIXME:", "STOPSHIP:", "TODO:")) { it.map(String::toRegex) } @@ -68,6 +69,7 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { private fun checkForbiddenComment(text: String, comment: PsiElement) { if (allowedPatterns.pattern.isNotEmpty() && allowedPatterns.containsMatchIn(text)) return + @Suppress("DEPRECATION") values.forEach { if (text.contains(it, ignoreCase = true)) { report( diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index 2eece64fbb3..3c680c8593d 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -78,6 +78,15 @@ class ForbiddenCommentSpec { assertThat(findings).hasSize(1) } + @Test + fun `should report violation in single line block comment`() { + val code = """ + /*TODO: I need to fix this.*/ + """.trimIndent() + val findings = ForbiddenComment().compileAndLint(code) + assertThat(findings).hasSize(1) + } + @Test fun `should report violation in KDoc`() { val code = """ @@ -101,7 +110,7 @@ class ForbiddenCommentSpec { @Nested inner class `when given Banana` { - val config = TestConfig(VALUES to "Banana") + val config = TestConfig(VALUES_PATTERNS to "Banana") @Test @DisplayName("should not report TODO: usages") @@ -141,7 +150,7 @@ class ForbiddenCommentSpec { @Nested @DisplayName("when given listOf(\"banana\")") inner class ListOfBanana { - val config = TestConfig(VALUES to listOf("Banana")) + val config = TestConfig(VALUES_PATTERNS to listOf("Banana")) @Test @DisplayName("should not report TODO: usages") From 06350d10175e0acfc449ccf344bb21fb20f1a5b6 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Fri, 14 Apr 2023 03:15:02 +0530 Subject: [PATCH 04/30] Check against complete text --- .../gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt | 3 +-- .../arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 88ba3eb6117..2029a52ccd9 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -82,9 +82,8 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { } } - val strippedText = text.removePrefix("//").removePrefix(" ") valuePatterns.forEach { - if (it.containsMatchIn(strippedText)) { + if (it.containsMatchIn(text)) { report( CodeSmell( issue, diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index 3c680c8593d..c3e4ac17e24 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -250,7 +250,7 @@ class ForbiddenCommentSpec { @Nested inner class `custom value pattern is configured` { - private val patternStr = """^(?i)REVIEW\b""" + private val patternStr = """^//( )?(?i)REVIEW\b""" private val messageConfig = TestConfig( VALUES_PATTERNS to listOf("STOPSHIP", patternStr), ) From d9dc02fb21d00c3aced3a4ced1f9b30e37a177d9 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Sat, 6 May 2023 23:50:44 +0530 Subject: [PATCH 05/30] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Róbert Papp --- .../io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 2029a52ccd9..fab9e4f5f6c 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -38,7 +38,7 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { ) @Configuration("forbidden comment strings") - @Deprecated("Use `valuePatterns` instead") + @Deprecated("Use `valuePatterns` instead, make sure you escape your text for Regular Expressions.") private val values: List by config(emptyList()) @Configuration("forbidden comment string patterns") From 6e22238222db64cfc5eb5471fc76515a81b1c5ba Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Sun, 7 May 2023 00:13:04 +0530 Subject: [PATCH 06/30] Update the `valuePatterns` config name to `comments` Add tc when comment is present in between --- .../src/main/resources/default-detekt-config.yml | 2 +- .../src/main/resources/deprecation.properties | 2 +- .../detekt/rules/style/ForbiddenComment.kt | 6 +++--- .../detekt/rules/style/ForbiddenCommentSpec.kt | 15 +++++++++++---- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index a93913a00fd..0cb6a08292a 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -566,7 +566,7 @@ style: value: 'java.lang.annotation.Inherited' ForbiddenComment: active: true - valuePatterns: + comments: - 'FIXME:' - 'STOPSHIP:' - 'TODO:' diff --git a/detekt-core/src/main/resources/deprecation.properties b/detekt-core/src/main/resources/deprecation.properties index 133f9e310f8..2c0b8780d86 100644 --- a/detekt-core/src/main/resources/deprecation.properties +++ b/detekt-core/src/main/resources/deprecation.properties @@ -16,7 +16,7 @@ potential-bugs>IgnoredReturnValue>restrictToAnnotatedMethods=Use `restrictToConf potential-bugs>LateinitUsage>excludeAnnotatedProperties=Use `ignoreAnnotated` instead potential-bugs>MissingWhenCase=Rule deprecated as compiler performs this check by default potential-bugs>RedundantElseInWhen=Rule deprecated as compiler performs this check by default -style>ForbiddenComment>values=Use `valuePatterns` instead +style>ForbiddenComment>values=Use `comments` instead, make sure you escape your text for Regular Expressions. style>ForbiddenPublicDataClass=Rule migrated to `libraries` ruleset plugin style>FunctionOnlyReturningConstant>excludeAnnotatedFunction=Use `ignoreAnnotated` instead style>LibraryCodeMustSpecifyReturnType=Rule migrated to `libraries` ruleset plugin diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index fab9e4f5f6c..37bc23b6569 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -38,11 +38,11 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { ) @Configuration("forbidden comment strings") - @Deprecated("Use `valuePatterns` instead, make sure you escape your text for Regular Expressions.") + @Deprecated("Use `comments` instead, make sure you escape your text for Regular Expressions.") private val values: List by config(emptyList()) @Configuration("forbidden comment string patterns") - private val valuePatterns: List by config(listOf("FIXME:", "STOPSHIP:", "TODO:")) { + private val comments: List by config(listOf("FIXME:", "STOPSHIP:", "TODO:")) { it.map(String::toRegex) } @@ -82,7 +82,7 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { } } - valuePatterns.forEach { + comments.forEach { if (it.containsMatchIn(text)) { report( CodeSmell( diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index c3e4ac17e24..be7aece0c19 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -10,7 +10,7 @@ import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test private const val VALUES = "values" -private const val VALUES_PATTERNS = "valuePatterns" +private const val COMMENTS = "comments" private const val ALLOWED_PATTERNS = "allowedPatterns" private const val MESSAGE = "customMessage" @@ -110,7 +110,7 @@ class ForbiddenCommentSpec { @Nested inner class `when given Banana` { - val config = TestConfig(VALUES_PATTERNS to "Banana") + val config = TestConfig(COMMENTS to "Banana") @Test @DisplayName("should not report TODO: usages") @@ -150,7 +150,7 @@ class ForbiddenCommentSpec { @Nested @DisplayName("when given listOf(\"banana\")") inner class ListOfBanana { - val config = TestConfig(VALUES_PATTERNS to listOf("Banana")) + val config = TestConfig(COMMENTS to listOf("Banana")) @Test @DisplayName("should not report TODO: usages") @@ -252,7 +252,7 @@ class ForbiddenCommentSpec { inner class `custom value pattern is configured` { private val patternStr = """^//( )?(?i)REVIEW\b""" private val messageConfig = TestConfig( - VALUES_PATTERNS to listOf("STOPSHIP", patternStr), + COMMENTS to listOf("STOPSHIP", patternStr), ) @Test @@ -295,5 +295,12 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(2) } + + @Test + fun `should report a finding matching a pattern contained in the comment`() { + val comment = "// foo STOPSHIP bar" + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).hasSize(1) + } } } From e2a3af064762518976391edac9de01e590f6f8b4 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Sun, 7 May 2023 03:32:06 +0530 Subject: [PATCH 07/30] Use static import of `io.gitlab.arturbosch.detekt.test.assertThat` --- .../arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index be7aece0c19..07b1242d84a 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -3,6 +3,7 @@ package io.gitlab.arturbosch.detekt.rules.style import io.gitlab.arturbosch.detekt.test.TestConfig +import io.gitlab.arturbosch.detekt.test.assertThat import io.gitlab.arturbosch.detekt.test.compileAndLint import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.DisplayName @@ -267,7 +268,7 @@ class ForbiddenCommentSpec { val comment = "// STOPSHIP to express in the preview that it's not a normal TextView." val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) - io.gitlab.arturbosch.detekt.test.assertThat(findings[0]) + assertThat(findings[0]) .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, "STOPSHIP")) } @@ -276,7 +277,7 @@ class ForbiddenCommentSpec { val comment = "// REVIEW foo -> flag" val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) - io.gitlab.arturbosch.detekt.test.assertThat(findings[0]) + assertThat(findings[0]) .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, patternStr)) } @@ -285,7 +286,7 @@ class ForbiddenCommentSpec { val comment = "//REVIEW foo -> flag" val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) - io.gitlab.arturbosch.detekt.test.assertThat(findings[0]) + assertThat(findings[0]) .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, patternStr)) } From 7580f5c7c200c89d07c9829e30905def57b4ac4c Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Mon, 15 May 2023 23:07:51 +0530 Subject: [PATCH 08/30] Use `valuesWithReason` in `comments` config Use custom `getContent` which returns comments with `/*`, `*/` and `*` --- .../main/resources/default-detekt-config.yml | 10 +- .../src/main/resources/deprecation.properties | 1 + .../detekt/rules/style/ForbiddenComment.kt | 72 +++++++- .../rules/style/ForbiddenCommentSpec.kt | 168 +++++++++++++++++- 4 files changed, 234 insertions(+), 17 deletions(-) diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 0cb6a08292a..1365510582a 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -567,11 +567,13 @@ style: ForbiddenComment: active: true comments: - - 'FIXME:' - - 'STOPSHIP:' - - 'TODO:' + - reason: 'some fixes are pending.' + value: 'FIXME:' + - reason: 'some changes are present which needs to be addressed before ship.' + value: 'STOPSHIP:' + - reason: 'some changes are pending.' + value: 'TODO:' allowedPatterns: '' - customMessage: '' ForbiddenImport: active: false imports: [] diff --git a/detekt-core/src/main/resources/deprecation.properties b/detekt-core/src/main/resources/deprecation.properties index 2c0b8780d86..e92b97340f2 100644 --- a/detekt-core/src/main/resources/deprecation.properties +++ b/detekt-core/src/main/resources/deprecation.properties @@ -16,6 +16,7 @@ potential-bugs>IgnoredReturnValue>restrictToAnnotatedMethods=Use `restrictToConf potential-bugs>LateinitUsage>excludeAnnotatedProperties=Use `ignoreAnnotated` instead potential-bugs>MissingWhenCase=Rule deprecated as compiler performs this check by default potential-bugs>RedundantElseInWhen=Rule deprecated as compiler performs this check by default +style>ForbiddenComment>customMessage=Use `comments` and provide `reason` against each `value` style>ForbiddenComment>values=Use `comments` instead, make sure you escape your text for Regular Expressions. style>ForbiddenPublicDataClass=Rule migrated to `libraries` ruleset plugin style>FunctionOnlyReturningConstant>excludeAnnotatedFunction=Use `ignoreAnnotated` instead diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 37bc23b6569..02592148591 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -10,6 +10,7 @@ import io.gitlab.arturbosch.detekt.api.Severity import io.gitlab.arturbosch.detekt.api.config import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault import io.gitlab.arturbosch.detekt.api.internal.Configuration +import io.gitlab.arturbosch.detekt.api.valuesWithReason import org.jetbrains.kotlin.com.intellij.psi.PsiComment import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.kdoc.psi.impl.KDocSection @@ -42,19 +43,26 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { private val values: List by config(emptyList()) @Configuration("forbidden comment string patterns") - private val comments: List by config(listOf("FIXME:", "STOPSHIP:", "TODO:")) { - it.map(String::toRegex) + private val comments: List by config( + valuesWithReason( + "FIXME:" to "some fixes are pending.", + "STOPSHIP:" to "some changes are present which needs to be addressed before ship.", + "TODO:" to "some changes are pending.", + ) + ) { list -> + list.map { Comment(it.value.toRegex(), it.reason) } } @Configuration("ignores comments which match the specified regular expression. For example `Ticket|Task`.") private val allowedPatterns: Regex by config("", String::toRegex) @Configuration("error message which overrides the default one") + @Deprecated("Use `comments` and provide `reason` against each `value`") private val customMessage: String by config("") override fun visitComment(comment: PsiComment) { super.visitComment(comment) - val text = comment.text + val text = comment.getContent() checkForbiddenComment(text, comment) } @@ -83,23 +91,73 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { } comments.forEach { - if (it.containsMatchIn(text)) { + if (it.value.containsMatchIn(text)) { report( CodeSmell( issue, Entity.from(comment), - getErrorMessage(it.pattern) + getErrorMessage(it) ) ) } } } + private fun PsiComment.getContent(): String = text.getCommentContent() + + private fun getErrorMessage(comment: Comment): String { + return comment.reason?.let { reason -> + String.format(DEFAULT_ERROR_MESSAGE, comment.value.pattern, reason) + } ?: run { + String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, comment.value.pattern) + } + } + + @Suppress("DEPRECATION") private fun getErrorMessage(value: String): String = - customMessage.takeUnless { it.isEmpty() } ?: String.format(DEFAULT_ERROR_MESSAGE, value) + customMessage.takeUnless { it.isEmpty() } ?: String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, value) + + private data class Comment(val value: Regex, val reason: String?) companion object { - const val DEFAULT_ERROR_MESSAGE = "This comment contains '%s' " + + const val DEFAULT_ERROR_MESSAGE_WITH_NO_REASON = "This comment contains '%s' " + "that has been defined as forbidden in detekt." + + const val DEFAULT_ERROR_MESSAGE = "This comment contains '%s' " + + "that has been forbidden: %s." + } +} + +internal fun String.getCommentContent(): String { + return if (this.startsWith("//")) { + this.removePrefix("//").removePrefix(" ") + } else { + this.split("\n").joinToString("\n") { + var hasStartCommentMarker = false + var hasEndCommentMarker = false + it.run { + if (this.startsWith("/*")) { + hasStartCommentMarker = true + this.removePrefix("/*") + .removePrefix(" ") + } else { + this + } + }.run { + if (this.endsWith("*/")) { + hasEndCommentMarker = this.startsWith("*").not() + this.removeSuffix("*/") + .removeSuffix(" ") + } else { + this + } + }.run { + if (hasStartCommentMarker.not() && hasEndCommentMarker.not()) { + this.removePrefix("*").removePrefix(" ") + } else { + this + } + } + } } } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index 07b1242d84a..2638fd2b102 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -9,6 +9,10 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.DisplayName import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.MethodSource private const val VALUES = "values" private const val COMMENTS = "comments" @@ -34,6 +38,9 @@ class ForbiddenCommentSpec { fun reportTodoColon() { val findings = ForbiddenComment().compileAndLint(todoColon) assertThat(findings).hasSize(1) + assertThat(findings[0]).hasMessage( + String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, "TODO:", "some changes are pending.") + ) } @Test @@ -60,6 +67,13 @@ class ForbiddenCommentSpec { fun reportStopShipColon() { val findings = ForbiddenComment().compileAndLint(stopShipColon) assertThat(findings).hasSize(1) + assertThat(findings[0]).hasMessage( + String.format( + ForbiddenComment.DEFAULT_ERROR_MESSAGE, + "STOPSHIP:", + "some changes are present which needs to be addressed before ship." + ) + ) } @Test @@ -103,6 +117,34 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment().compileAndLint(code) assertThat(findings).hasSize(2) } + + @Test + fun `should report violation in KDoc 1`() { + @Suppress("MultilineRawStringIndentation") + val code = """ + /** + * TODO: I need to fix this. +* left shifted asterik + * right shifted astrik + * with space astrik + *with 0 space astrik + * with 1 space astrik + * with 2 space astrik + * with 3 space astrik + * with 4 space astrik + * with 5 space astrik + * with 6 space astrik + * with 7 space astrik + */ + class A { + /** + * TODO: I need to fix this. + */ + } + """.trimIndent() + val findings = ForbiddenComment().compileAndLint(code) + assertThat(findings).hasSize(2) + } } @Nested @@ -111,7 +153,7 @@ class ForbiddenCommentSpec { @Nested inner class `when given Banana` { - val config = TestConfig(COMMENTS to "Banana") + val config = TestConfig(COMMENTS to listOf("Banana")) @Test @DisplayName("should not report TODO: usages") @@ -243,7 +285,7 @@ class ForbiddenCommentSpec { fun `should report a Finding with default Message`() { val comment = "// Comment" val findings = ForbiddenComment(messageConfig).compileAndLint(comment) - val expectedMessage = String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, "Comment") + val expectedMessage = String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, "Comment") assertThat(findings).hasSize(1) assertThat(findings.first().message).isEqualTo(expectedMessage) } @@ -251,7 +293,7 @@ class ForbiddenCommentSpec { @Nested inner class `custom value pattern is configured` { - private val patternStr = """^//( )?(?i)REVIEW\b""" + private val patternStr = """^(?i)REVIEW\b""" private val messageConfig = TestConfig( COMMENTS to listOf("STOPSHIP", patternStr), ) @@ -269,7 +311,7 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) assertThat(findings[0]) - .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, "STOPSHIP")) + .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, "STOPSHIP")) } @Test @@ -278,7 +320,7 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) assertThat(findings[0]) - .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, patternStr)) + .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, patternStr)) } @Test @@ -287,7 +329,7 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) assertThat(findings[0]) - .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, patternStr)) + .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, patternStr)) } @Test @@ -304,4 +346,118 @@ class ForbiddenCommentSpec { assertThat(findings).hasSize(1) } } + + @TestInstance(TestInstance.Lifecycle.PER_CLASS) + @Nested + inner class `comment getContent` { + + @Suppress("LongMethod", "UnusedPrivateMember") + private fun getCommentsContentArguments() = listOf( + Arguments.of("// comment", "comment"), + Arguments.of("// comment", " comment"), + Arguments.of("//comment", "comment"), + Arguments.of("/* comment */", "comment"), + Arguments.of("/* comment */", " comment "), + Arguments.of("/*comment*/", "comment"), + Arguments.of("/*** comment ***/", "** comment **"), + Arguments.of( + """ + /* + *bad + * good + */ + """.trimIndent(), + """ + + bad + good + + """.trimIndent() + ), + Arguments.of( + """ + /* + *bad + * good + */ + """.trimIndent(), + """ + + bad + good + + """.trimIndent() + ), + Arguments.of( + """ + /* + comment + * a + * b + * c*/ + """.trimIndent(), + """ + + comment + a + b + c + """.trimIndent() + ), + Arguments.of( + """ + /*comment + * a + * b + * c*/ + """.trimIndent(), + """ + comment + a + b + c + """.trimIndent() + ), + Arguments.of( + """ + /* + a + b + c + */ + """.trimIndent(), + """ + + a + b + c + + """.trimIndent() + ), + Arguments.of( + """ + /* + a + b + + c + */ + """.trimIndent(), + """ + + a + b + + c + + """.trimIndent() + ), + ) + + @ParameterizedTest(name = "Given {0} comment, getContent return {1}") + @MethodSource("getCommentsContentArguments") + fun test(comment: String, content: String) { + assertThat(comment.getCommentContent()).isEqualTo(content) + } + } } From 8e11b39d996104953562f0b8c0c35fb50f3736d8 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Wed, 17 May 2023 03:42:36 +0530 Subject: [PATCH 09/30] Incorporate review comments and align comments for indented code --- .../detekt/rules/style/ForbiddenComment.kt | 42 +-- .../rules/style/ForbiddenCommentSpec.kt | 276 ++++++++++++++++-- 2 files changed, 273 insertions(+), 45 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 02592148591..d45f54464e0 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -132,32 +132,34 @@ internal fun String.getCommentContent(): String { return if (this.startsWith("//")) { this.removePrefix("//").removePrefix(" ") } else { - this.split("\n").joinToString("\n") { - var hasStartCommentMarker = false - var hasEndCommentMarker = false - it.run { - if (this.startsWith("/*")) { - hasStartCommentMarker = true - this.removePrefix("/*") + val indentedComment = if (this.contains('\n').not()) { + this + } else { + val lines = this.lines() + lines.first() + "\n" + lines.drop(1).joinToString("\n").trimIndent() + } + indentedComment.split("\n").joinToString("\n") { + it.let { fullLine -> + if (fullLine.startsWith("/*")) { + fullLine.removePrefix("/*") + .removePrefix(" ") + } else if (fullLine.startsWith(" *") && fullLine.startsWith(" */").not()) { + fullLine.removePrefix(" *") + .removePrefix(" ") + } else if (fullLine.startsWith("*") && fullLine.startsWith("*/").not()) { + fullLine.removePrefix("*") .removePrefix(" ") } else { - this + fullLine } - }.run { - if (this.endsWith("*/")) { - hasEndCommentMarker = this.startsWith("*").not() - this.removeSuffix("*/") + }.let { lineWithoutStartMarker -> + if (lineWithoutStartMarker.endsWith("*/")) { + lineWithoutStartMarker.removeSuffix("*/") .removeSuffix(" ") } else { - this - } - }.run { - if (hasStartCommentMarker.not() && hasEndCommentMarker.not()) { - this.removePrefix("*").removePrefix(" ") - } else { - this + lineWithoutStartMarker } } - } + }.trimStart('\n') } } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index 2638fd2b102..ec6aef4a637 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -119,31 +119,15 @@ class ForbiddenCommentSpec { } @Test - fun `should report violation in KDoc 1`() { - @Suppress("MultilineRawStringIndentation") + fun `should report violation in star aligned comment`() { val code = """ - /** + /* * TODO: I need to fix this. -* left shifted asterik - * right shifted astrik - * with space astrik - *with 0 space astrik - * with 1 space astrik - * with 2 space astrik - * with 3 space astrik - * with 4 space astrik - * with 5 space astrik - * with 6 space astrik - * with 7 space astrik */ - class A { - /** - * TODO: I need to fix this. - */ - } + class A """.trimIndent() val findings = ForbiddenComment().compileAndLint(code) - assertThat(findings).hasSize(2) + assertThat(findings).hasSize(1) } } @@ -345,6 +329,120 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) } + + @Test + fun `should report a finding matching a pattern contained in multiple single line comments`() { + val comment = """ + // foo STOPSHIP bar + // foo STOPSHIP bar + // foo STOPSHIP bar + """.trimIndent() + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).hasSize(3) + } + + @Test + fun `should report a finding when whole comment is not allowed`() { + val comment = """ + class A { + // stopship + val a = 0 + } + """.trimIndent() + val findings = ForbiddenComment( + TestConfig(COMMENTS to listOf("stopship")) + ).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + } + + @Nested + inner class `leading space is not allowed` { + private val patternStr = "^ " + private val messageConfig = TestConfig( + COMMENTS to listOf(patternStr), + ) + + @Test + fun `should report a finding when leading extra space is not allowed`() { + val comment = """ + class A { + // comment + val a = 0 + } + """.trimIndent() + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should not report a finding when leading extra space is not allowed with no leading space`() { + val comment = """ + class A { + // comment with space in between + val a = 0 + } + """.trimIndent() + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).isEmpty() + } + + @Test + fun `should not report a finding when leading extra space is not allowed in multiline comment`() { + val comment = """ + class A { + /* + * comment with space in between + * comment with space in between + */ + val a = 0 + } + """.trimIndent() + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).isEmpty() + } + + @Test + fun `should report a finding when leading extra space is not allowed in multiline comment with leading space`() { + val comment = """ + class A { + /* + * comment + */ + val a = 0 + } + """.trimIndent() + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should not report a finding when leading extra space is not allowed in star aligned comment`() { + val comment = """ + class A { + /* + * comment + */ + val a = 0 + } + """.trimIndent() + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).isEmpty() + } + + @Test + fun `should report a finding when leading space is not allowed in star aligned comment with leading space`() { + val comment = """ + class A { + /* + * comment + */ + val a = 0 + } + """.trimIndent() + val findings = ForbiddenComment(messageConfig).compileAndLint(comment) + assertThat(findings).hasSize(1) + } } @TestInstance(TestInstance.Lifecycle.PER_CLASS) @@ -356,19 +454,35 @@ class ForbiddenCommentSpec { Arguments.of("// comment", "comment"), Arguments.of("// comment", " comment"), Arguments.of("//comment", "comment"), + Arguments.of("// ", ""), + Arguments.of("// ", " "), Arguments.of("/* comment */", "comment"), Arguments.of("/* comment */", " comment "), + Arguments.of("/* */", ""), + Arguments.of("/* */", ""), Arguments.of("/*comment*/", "comment"), Arguments.of("/*** comment ***/", "** comment **"), Arguments.of( """ /* - *bad + * good * good */ """.trimIndent(), """ + good + good + """.trimIndent() + ), + Arguments.of( + """ + /* + *bad + * good + */ + """.trimIndent(), + """ bad good @@ -382,7 +496,6 @@ class ForbiddenCommentSpec { */ """.trimIndent(), """ - bad good @@ -397,7 +510,6 @@ class ForbiddenCommentSpec { * c*/ """.trimIndent(), """ - comment a b @@ -418,6 +530,74 @@ class ForbiddenCommentSpec { c """.trimIndent() ), + Arguments.of( + """ + /* + * good + * good + */ + """.trimIndent(), + """ + good + good + + """.trimIndent() + ), + Arguments.of( + """ + /* + *bad + * good + */ + """.trimIndent(), + """ + bad + good + + """.trimIndent() + ), + Arguments.of( + """ + /* + *bad + * good + */ + """.trimIndent(), + """ + bad + good + + """.trimIndent() + ), + Arguments.of( + """ + /* + comment + * a + * b + * c*/ + """.trimIndent(), + """ + comment + a + b + c + """.trimIndent() + ), + Arguments.of( + """ + /*comment + * a + * b + * c*/ + """.trimIndent(), + """ + comment + a + b + c + """.trimIndent() + ), Arguments.of( """ /* @@ -427,7 +607,6 @@ class ForbiddenCommentSpec { */ """.trimIndent(), """ - a b c @@ -444,7 +623,6 @@ class ForbiddenCommentSpec { */ """.trimIndent(), """ - a b @@ -452,6 +630,54 @@ class ForbiddenCommentSpec { """.trimIndent() ), + Arguments.of( + """ + /* + + + */ + """.trimIndent(), + "".trimIndent() + ), + Arguments.of( + """ + /* + + a + + */ + """.trimIndent(), + "a\n\n" + ), + Arguments.of( + """ + /* + + + */ + """.trimIndent(), + "".trimIndent() + ), + Arguments.of( + """ + /* + + a + + */ + """.trimIndent(), + " a\n\n" + ), + Arguments.of( + """ + /* + + * a + + */ + """.trimIndent(), + "a\n\n" + ), ) @ParameterizedTest(name = "Given {0} comment, getContent return {1}") From 42a80c7feecd17dc75441db9ad8eec9253e0b861 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Thu, 18 May 2023 20:13:14 +0530 Subject: [PATCH 10/30] Update detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove `run` in `getErrorMessage` Co-authored-by: Róbert Papp --- .../arturbosch/detekt/rules/style/ForbiddenComment.kt | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index d45f54464e0..c9046f4607c 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -105,13 +105,10 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { private fun PsiComment.getContent(): String = text.getCommentContent() - private fun getErrorMessage(comment: Comment): String { - return comment.reason?.let { reason -> - String.format(DEFAULT_ERROR_MESSAGE, comment.value.pattern, reason) - } ?: run { - String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, comment.value.pattern) - } - } + private fun getErrorMessage(comment: Comment): String = + comment.reason + ?.let { reason -> String.format(DEFAULT_ERROR_MESSAGE, comment.value.pattern, reason) } + ?: String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, comment.value.pattern) @Suppress("DEPRECATION") private fun getErrorMessage(value: String): String = From 373bc23caf28875bda4a038dd195726984fb17f6 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Fri, 19 May 2023 00:28:27 +0530 Subject: [PATCH 11/30] Refactor code make `trimIndentIgnoringFirstLine` out Trim start white space as to align with starting `*` Add more TCs --- .../detekt/rules/style/ForbiddenComment.kt | 66 +++--- .../rules/style/ForbiddenCommentSpec.kt | 223 +++++++++++++++--- 2 files changed, 224 insertions(+), 65 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index c9046f4607c..7f14e4cc9ed 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -129,34 +129,44 @@ internal fun String.getCommentContent(): String { return if (this.startsWith("//")) { this.removePrefix("//").removePrefix(" ") } else { - val indentedComment = if (this.contains('\n').not()) { - this - } else { - val lines = this.lines() - lines.first() + "\n" + lines.drop(1).joinToString("\n").trimIndent() - } - indentedComment.split("\n").joinToString("\n") { - it.let { fullLine -> - if (fullLine.startsWith("/*")) { - fullLine.removePrefix("/*") - .removePrefix(" ") - } else if (fullLine.startsWith(" *") && fullLine.startsWith(" */").not()) { - fullLine.removePrefix(" *") - .removePrefix(" ") - } else if (fullLine.startsWith("*") && fullLine.startsWith("*/").not()) { - fullLine.removePrefix("*") - .removePrefix(" ") - } else { - fullLine - } - }.let { lineWithoutStartMarker -> - if (lineWithoutStartMarker.endsWith("*/")) { - lineWithoutStartMarker.removeSuffix("*/") - .removeSuffix(" ") - } else { - lineWithoutStartMarker - } + this + .trimIndentIgnoringFirstLine() + // Process line by line. + .lineSequence() + // Remove starting, aligning and ending markers. + .map { + it + .let { fullLine -> + val trimmedStartFullLine = fullLine.trimStart() + if (trimmedStartFullLine.startsWith("/*")) { + trimmedStartFullLine.removePrefix("/*").removePrefix(" ") + } else if (trimmedStartFullLine.startsWith("*") && trimmedStartFullLine.startsWith("*/") + .not() + ) { + trimmedStartFullLine.removePrefix("*").removePrefix(" ") + } else { + fullLine + } + } + .let { lineWithoutStartMarker -> + if (lineWithoutStartMarker.endsWith("*/")) { + lineWithoutStartMarker.removeSuffix("*/").removeSuffix(" ") + } else { + lineWithoutStartMarker + } + } } - }.trimStart('\n') + // Trim trailing empty lines. + .dropWhile(String::isEmpty) + // Reconstruct the comment contents. + .joinToString("\n") } } + +private fun String.trimIndentIgnoringFirstLine(): String = + if ('\n' !in this) { + this + } else { + val lines = this.lineSequence() + lines.first() + "\n" + lines.drop(1).joinToString("\n").trimIndent() + } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index ec6aef4a637..cfa9e4e490a 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -20,23 +20,13 @@ private const val ALLOWED_PATTERNS = "allowedPatterns" private const val MESSAGE = "customMessage" class ForbiddenCommentSpec { - - val todoColon = "// TODO: I need to fix this." - val todo = "// TODO I need to fix this." - - val fixmeColon = "// FIXME: I need to fix this." - val fixme = "// FIXME I need to fix this." - - val stopShipColon = "// STOPSHIP: I need to fix this." - val stopShip = "// STOPSHIP I need to fix this." - @Nested inner class `the default values are configured` { @Test @DisplayName("should report TODO: usages") fun reportTodoColon() { - val findings = ForbiddenComment().compileAndLint(todoColon) + val findings = ForbiddenComment().compileAndLint("// TODO: I need to fix this.") assertThat(findings).hasSize(1) assertThat(findings[0]).hasMessage( String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, "TODO:", "some changes are pending.") @@ -45,27 +35,27 @@ class ForbiddenCommentSpec { @Test fun `should not report TODO usages`() { - val findings = ForbiddenComment().compileAndLint(todo) + val findings = ForbiddenComment().compileAndLint("// TODO I need to fix this.") assertThat(findings).isEmpty() } @Test @DisplayName("should report FIXME: usages") fun reportFixMe() { - val findings = ForbiddenComment().compileAndLint(fixmeColon) + val findings = ForbiddenComment().compileAndLint("// FIXME: I need to fix this.") assertThat(findings).hasSize(1) } @Test fun `should not report FIXME usages`() { - val findings = ForbiddenComment().compileAndLint(fixme) + val findings = ForbiddenComment().compileAndLint("// FIXME I need to fix this.") assertThat(findings).isEmpty() } @Test @DisplayName("should report STOPSHIP: usages") fun reportStopShipColon() { - val findings = ForbiddenComment().compileAndLint(stopShipColon) + val findings = ForbiddenComment().compileAndLint("// STOPSHIP: I need to fix this.") assertThat(findings).hasSize(1) assertThat(findings[0]).hasMessage( String.format( @@ -78,7 +68,7 @@ class ForbiddenCommentSpec { @Test fun `should not report STOPSHIP usages`() { - val findings = ForbiddenComment().compileAndLint(stopShip) + val findings = ForbiddenComment().compileAndLint("// STOPSHIP I need to fix this.") assertThat(findings).isEmpty() } @@ -142,21 +132,21 @@ class ForbiddenCommentSpec { @Test @DisplayName("should not report TODO: usages") fun todoColon() { - val findings = ForbiddenComment(config).compileAndLint(todoColon) + val findings = ForbiddenComment(config).compileAndLint("// TODO: I need to fix this.") assertThat(findings).isEmpty() } @Test @DisplayName("should not report FIXME: usages") fun fixmeColon() { - val findings = ForbiddenComment(config).compileAndLint(fixmeColon) + val findings = ForbiddenComment(config).compileAndLint("// FIXME: I need to fix this.") assertThat(findings).isEmpty() } @Test @DisplayName("should not report STOPME: usages") fun stopShipColon() { - val findings = ForbiddenComment(config).compileAndLint(stopShipColon) + val findings = ForbiddenComment(config).compileAndLint("// STOPSHIP: I need to fix this.") assertThat(findings).isEmpty() } @@ -182,21 +172,21 @@ class ForbiddenCommentSpec { @Test @DisplayName("should not report TODO: usages") fun todoColon() { - val findings = ForbiddenComment(config).compileAndLint(todoColon) + val findings = ForbiddenComment(config).compileAndLint("// TODO: I need to fix this.") assertThat(findings).isEmpty() } @Test @DisplayName("should not report FIXME: usages") fun fixmeColon() { - val findings = ForbiddenComment(config).compileAndLint(fixmeColon) + val findings = ForbiddenComment(config).compileAndLint("// FIXME: I need to fix this.") assertThat(findings).isEmpty() } @Test @DisplayName("should not report STOPME: usages") fun stopShipColon() { - val findings = ForbiddenComment(config).compileAndLint(stopShipColon) + val findings = ForbiddenComment(config).compileAndLint("// STOPSHIP: I need to fix this.") assertThat(findings).isEmpty() } @@ -340,24 +330,10 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(3) } - - @Test - fun `should report a finding when whole comment is not allowed`() { - val comment = """ - class A { - // stopship - val a = 0 - } - """.trimIndent() - val findings = ForbiddenComment( - TestConfig(COMMENTS to listOf("stopship")) - ).compileAndLint(comment) - assertThat(findings).hasSize(1) - } } @Nested - inner class `leading space is not allowed` { + inner class `comment on indented code` { private val patternStr = "^ " private val messageConfig = TestConfig( COMMENTS to listOf(patternStr), @@ -443,6 +419,92 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) } + + @Test + fun `should report a finding when leading space is not allowed in multiline block comment with leading space`() { + val comment = """ + class A { + /* + comment + */ + val a = 0 + } + """.trimIndent() + val findings = ForbiddenComment( + TestConfig( + COMMENTS to listOf("^ comment"), + ) + ).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should report a finding when -ve leading space is not allowed in multiline block comment with leading space`() { + val comment = """ + class A { + /* + comment + */ + val a = 0 + } + """.trimIndent() + val findings = ForbiddenComment( + TestConfig( + COMMENTS to listOf("^comment"), + ) + ).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should report a finding start space contained in multiple single line comments`() { + val comment = """ + class a { + fun test() { + // foo + val a = 0 + } + } + """.trimIndent() + val findings = ForbiddenComment( + TestConfig( + COMMENTS to listOf("^ foo"), + ) + ).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should not report when not finding start contained in multiple single line comments`() { + val comment = """ + class a { + fun test() { + // foo + val a = 0 + } + } + """.trimIndent() + val findings = ForbiddenComment( + TestConfig( + COMMENTS to listOf("^ foo"), + ) + ).compileAndLint(comment) + assertThat(findings).isEmpty() + } + + @Test + fun `should report a finding when whole comment is not allowed`() { + val comment = """ + class A { + // stopship + val a = 0 + } + """.trimIndent() + val findings = ForbiddenComment( + TestConfig(COMMENTS to listOf("stopship")) + ).compileAndLint(comment) + assertThat(findings).hasSize(1) + } } @TestInstance(TestInstance.Lifecycle.PER_CLASS) @@ -530,6 +592,20 @@ class ForbiddenCommentSpec { c """.trimIndent() ), + Arguments.of( + """ + /* comment + * a + * b + * c */ + """.trimIndent(), + """ + comment + a + b + c + """.trimIndent() + ), Arguments.of( """ /* @@ -668,6 +744,21 @@ class ForbiddenCommentSpec { """.trimIndent(), " a\n\n" ), + Arguments.of( + """ + /* + a + b + c + */ + """.trimIndent(), + """ + a + b + c + + """.trimIndent() + ), Arguments.of( """ /* @@ -678,6 +769,64 @@ class ForbiddenCommentSpec { """.trimIndent(), "a\n\n" ), + Arguments.of( + """ + /* + * foo + * bar + * baz + */ + """.trimIndent(), + """ + foo + bar + baz + + """.trimIndent() + ), + Arguments.of( + """ + /* + * foo + * bar + * baz + */ + """.trimIndent(), + """ + foo + bar + baz + + """.trimIndent() + ), + Arguments.of( + """ + /* + + a + b + c + d + *e + * f + * g + * h + * i + */ + """.trimIndent(), + """ + a + b + c + d + e + f + g + h + i + + """.trimIndent() + ), ) @ParameterizedTest(name = "Given {0} comment, getContent return {1}") From a97dcab247be076acc0a0177e6fdf7845f778312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Fri, 19 May 2023 10:14:19 +0100 Subject: [PATCH 12/30] Shorten name: if it's trimmed, it cannot be also "full" --- .../detekt/rules/style/ForbiddenComment.kt | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 7f14e4cc9ed..c248862018e 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -137,13 +137,11 @@ internal fun String.getCommentContent(): String { .map { it .let { fullLine -> - val trimmedStartFullLine = fullLine.trimStart() - if (trimmedStartFullLine.startsWith("/*")) { - trimmedStartFullLine.removePrefix("/*").removePrefix(" ") - } else if (trimmedStartFullLine.startsWith("*") && trimmedStartFullLine.startsWith("*/") - .not() - ) { - trimmedStartFullLine.removePrefix("*").removePrefix(" ") + val trimmedStartLine = fullLine.trimStart() + if (trimmedStartLine.startsWith("/*")) { + trimmedStartLine.removePrefix("/*").removePrefix(" ") + } else if (trimmedStartLine.startsWith("*") && trimmedStartLine.startsWith("*/").not()) { + trimmedStartLine.removePrefix("*").removePrefix(" ") } else { fullLine } From b747f77ec2f218c3b8fcc76504c4c0ef337ac5e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Fri, 19 May 2023 10:24:01 +0100 Subject: [PATCH 13/30] Test name fixes --- .../arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index cfa9e4e490a..ee0eeaf569d 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -439,7 +439,7 @@ class ForbiddenCommentSpec { } @Test - fun `should report a finding when -ve leading space is not allowed in multiline block comment with leading space`() { + fun `should report a finding when negative leading space is not allowed in multiline block comment with leading space`() { val comment = """ class A { /* @@ -457,7 +457,7 @@ class ForbiddenCommentSpec { } @Test - fun `should report a finding start space contained in multiple single line comments`() { + fun `should report a finding start space contained in a single line comment`() { val comment = """ class a { fun test() { @@ -475,7 +475,7 @@ class ForbiddenCommentSpec { } @Test - fun `should not report when not finding start contained in multiple single line comments`() { + fun `should not report when not finding start contained in a single line comment`() { val comment = """ class a { fun test() { From 007d3b44cb27b9b281dde57c2b7199d0f6148d74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Fri, 19 May 2023 10:25:56 +0100 Subject: [PATCH 14/30] Add tests for trailing and other mixed code-comment pairs --- .../rules/style/ForbiddenCommentSpec.kt | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index ee0eeaf569d..f46ae887450 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -507,6 +507,87 @@ class ForbiddenCommentSpec { } } + @Nested + inner class `mixed code and comment lines` { + + @Test + fun `should report a finding in trailing single line comment`() { + val comment = """ + fun f() {} // TODO implement + """.trimIndent() + val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("TODO"))).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should report a finding when multiline comment exists before code`() { + val comment = """ + class A { + /*public*/ fun f() {} + } + """.trimIndent() + val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("public"))).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should report a finding when multiline comment exists inside code`() { + val comment = """ + class A { + fun f() /*: String*/ {} + } + """.trimIndent() + val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("^: .+$"))).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should report a finding when multiline comment exists after code`() { + val comment = """ + class A { + fun f() = /*error("foo")*/ + TODO() + } + """.trimIndent() + val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("^error"))).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should report a finding when kdoc comment exists before code`() { + val comment = """ + class A { + /**public*/ fun f() {} + } + """.trimIndent() + val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("public"))).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should report a finding when kdoc comment exists inside code`() { + val comment = """ + class A { + fun f() /**: String*/ {} + } + """.trimIndent() + val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("^: .+$"))).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + + @Test + fun `should report a finding when kdoc comment exists after code`() { + val comment = """ + class A { + fun f() = /**error("foo")*/ + TODO() + } + """.trimIndent() + val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("^error"))).compileAndLint(comment) + assertThat(findings).hasSize(1) + } + } + @TestInstance(TestInstance.Lifecycle.PER_CLASS) @Nested inner class `comment getContent` { From 4687325041ae66e22e8314aaabc6f44a784c83df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Fri, 19 May 2023 11:32:41 +0100 Subject: [PATCH 15/30] Migrate detekt config to use new property. Fixes: > Task :detekt-test:detektMain Property 'style>ForbiddenComment>values' is deprecated. Use `comments` instead, make sure you escape your text for Regular Expressions.. History: @requiresTypeResolution: #3579 + 13a83895 @author: #1776 --- config/detekt/detekt.yml | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml index 72723cd8eb8..dae6189bd8d 100644 --- a/config/detekt/detekt.yml +++ b/config/detekt/detekt.yml @@ -169,12 +169,17 @@ style: active: true ForbiddenComment: active: true - values: - - 'TODO:' - - 'FIXME:' - - 'STOPSHIP:' - - '@author' - - '@requiresTypeResolution' + comments: + - value: 'FIXME:' + reason: 'Some fixes are pending, please complete them.' + - value: 'STOPSHIP:' + reason: 'Some changes are present which need to be addressed before release.' + - value: 'TODO:' + reason: 'Some changes are pending, please complete them.' + - value: '@author' + reason: 'Authors are not recorded in KDoc.' + - value: '@requiresTypeResolution' + reason: 'Use @RequiresTypeResolution annotation on the class instead.' excludes: ['**/detekt-rules-style/**/ForbiddenComment.kt'] ForbiddenImport: active: true From ffff4f7c668010d2c3f9a66379f19ad149c5eea6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Fri, 19 May 2023 15:15:28 +0100 Subject: [PATCH 16/30] Format for visibility --- .../gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index c248862018e..7fdfbdf5b1a 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -112,7 +112,8 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { @Suppress("DEPRECATION") private fun getErrorMessage(value: String): String = - customMessage.takeUnless { it.isEmpty() } ?: String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, value) + customMessage.takeUnless { it.isEmpty() } + ?: String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, value) private data class Comment(val value: Regex, val reason: String?) From b4ca396e3c4707a645d688bf18ca3f6610a74e80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Fri, 19 May 2023 15:54:58 +0100 Subject: [PATCH 17/30] Document behavior, add tests for documented behaviors and fix them. --- .../detekt/rules/style/ForbiddenComment.kt | 66 ++++++++++- .../rules/style/ForbiddenCommentSpec.kt | 105 ++++++++++++++++++ 2 files changed, 168 insertions(+), 3 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 7fdfbdf5b1a..fe5717dd9d1 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -17,15 +17,75 @@ import org.jetbrains.kotlin.kdoc.psi.impl.KDocSection import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType +// Note: ​ (zero-width-space) is used to prevent the Kotlin parser getting confused by talking about comments in a comment. /** * This rule allows to set a list of comments which are forbidden in the codebase and should only be used during * development. Offending code comments will then be reported. * + * The regular expressions in `comments` list will have the following behaviors while matching the comments: + * * Each comment will be handled individually as a whole. + * * single line comments are always separate, conjoint lines are not merged. + * * multi line comments are not split up, the regex will be applied to the whole comment. + * * KDoc comments are not split up, the regex will be applied to the whole comment. + * * The comment markers (`//`, `/​*`, `/​**`, `*` aligners, `*​/`) are removed before applying the regex. + * One leading space is removed from each line of the comment, after starting markers and aligners. + * * The regex is applied as a multiline regex, + * see [Anchors](https://www.regular-expressions.info/anchors.html) for more info. + * To match the start and end of each line, use `^` and `$`. + * To match the start and end of the whole comment, use `\A` and `\Z`. + * To turn off multiline, use `(?-m)` at the start of your regex. + * * The regex is applied with dotall semantics, meaning `.` will match any character including newlines, + * this is to ensure that freeform line-wrapping doesn't mess with simple regexes. + * To turn off this behavior, use `(?-s)` at the start of your regex, or use `[^\r\n]*` instead of `.*`. + * * The regex will be searched using "contains" semantics not "matches", + * so partial comment matches will flag forbidden comments. + * + * The rule can be configured to add extra comments to the list of forbidden comments, here are some examples: + * ```yaml + * ForbiddenComment: + * comments: + * # Repeat the default configuration if it's still needed. + * - reason: 'some fixes are pending.' + * value: 'FIXME:' + * - reason: 'some changes are present which needs to be addressed before ship.' + * value: 'STOPSHIP:' + * - reason: 'some changes are pending.' + * value: 'TODO:' + * # Add additional patterns to the list. + * + * - reason: 'Authors are not recorded in KDoc.' + * value: '@author' + * + * - reason: 'REVIEW markers are not allowed in production code, only use before PR is merged.' + * value: '^\s*(?i)REVIEW\b' + * # Explanation: at the beginning of the line, optional leading space, + * # case insensitive (e.g. REVIEW, review, Review), and REVIEW only as a full word. + * # Non-compliant: // REVIEW this code before merging. + * # Compliant: // Preview will show up here. + * + * - reason: 'Use @androidx.annotation.VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) instead.' + * value: '^private$' + * # Non-compliant: /​*private*​/ fun f() { } + * + * - reason: 'KDoc tag should have a value.' + * value: '^\s*@(?!suppress|hide)\w+\s*$' + * # Explanation: full line with optional leading and trailing space, and an at @ character followed by a word, + * # but not @suppress or @hide because those don't need content afterwards. + * # Non-compliant: /​** ... @see *​/ + * + * - reason: 'include an issue link at the beginning preceded by a space' + * value: 'BUG:(?! https://github\.com/company/repo/issues/\d+).*' + * ``` + * + * By default the commonly used todo markers are forbidden: `TODO:`, `FIXME:` and `STOPSHIP:`. + * * * val a = "" // TODO: remove please - * // FIXME: this is a hack + * /** + * * FIXME: this is a hack + * */ * fun foo() { } - * // STOPSHIP: + * /* STOPSHIP: */ * */ @ActiveByDefault(since = "1.0.0") @@ -50,7 +110,7 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { "TODO:" to "some changes are pending.", ) ) { list -> - list.map { Comment(it.value.toRegex(), it.reason) } + list.map { Comment(it.value.toRegex(setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.MULTILINE)), it.reason) } } @Configuration("ignores comments which match the specified regular expression. For example `Ticket|Task`.") diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index f46ae887450..60858da5c9f 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -588,6 +588,111 @@ class ForbiddenCommentSpec { } } + @Nested + inner class `regex semantics for comments` { + + @Test + fun `should report a finding at the beginning of lines`() { + val comment = """ + /* + * foo bar + * baz qux + */ + """.trimIndent() + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("^baz"))).compileAndLint(comment)).hasSize(1) + } + + @Test + fun `should not report a finding at the beginning of lines when the flag is off`() { + val comment = """ + /* + * foo bar + * baz qux + */ + """.trimIndent() + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("(?-m)^baz"))).compileAndLint(comment)).isEmpty() + } + + @Test + fun `should report a finding at the end of lines`() { + val comment = """ + /* + * foo bar + * baz qux + */ + """.trimIndent() + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("bar$"))).compileAndLint(comment)).hasSize(1) + } + + @Test + fun `should not report a finding at the end of lines when flag is off`() { + val comment = """ + /* + * foo bar + * baz qux + */ + """.trimIndent() + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("(?-m)bar$"))).compileAndLint(comment)).isEmpty() + } + + @Test + fun `should report a finding at the beginning of a comment`() { + val comment = """ + /* + * foo bar + * baz qux + */ + """.trimIndent() + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("^foo"))).compileAndLint(comment)).hasSize(1) + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("\\Afoo"))).compileAndLint(comment)).hasSize(1) + } + + @Test + fun `should report a finding at the end of a comment`() { + val comment = """ + /* + * foo bar + * baz qux + */ + """.trimIndent() + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("qux$"))).compileAndLint(comment)).hasSize(1) + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("qux\\Z"))).compileAndLint(comment)).hasSize(1) + } + + @Test + fun `should report a finding across lines`() { + val comment = """ + /* + * foo bar + * baz qux + */ + """.trimIndent() + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("^foo.*qux$"))).compileAndLint(comment)).hasSize(1) + } + + @Test + fun `should not report a finding across lines when the flag is off`() { + val comment = """ + /* + * foo bar + * baz qux + */ + """.trimIndent() + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("(?-s)^foo.*qux$"))).compileAndLint(comment)).isEmpty() + } + + @Test + fun `should report all separate findings at once`() { + val comment = """ + /* + * foo baz + * bar qux + */ + """.trimIndent() + assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("^foo", "^bar"))).compileAndLint(comment)).hasSize(2) + } + } + @TestInstance(TestInstance.Lifecycle.PER_CLASS) @Nested inner class `comment getContent` { From 5673f92a028377f4039231456337605cf399d800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Fri, 19 May 2023 16:37:36 +0100 Subject: [PATCH 18/30] De-noise ForbiddenComment instance creations --- .../rules/style/ForbiddenCommentSpec.kt | 79 +++++++++---------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index 60858da5c9f..be21b96d1b1 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -430,11 +430,7 @@ class ForbiddenCommentSpec { val a = 0 } """.trimIndent() - val findings = ForbiddenComment( - TestConfig( - COMMENTS to listOf("^ comment"), - ) - ).compileAndLint(comment) + val findings = ForbiddenComment("^ comment").compileAndLint(comment) assertThat(findings).hasSize(1) } @@ -448,11 +444,7 @@ class ForbiddenCommentSpec { val a = 0 } """.trimIndent() - val findings = ForbiddenComment( - TestConfig( - COMMENTS to listOf("^comment"), - ) - ).compileAndLint(comment) + val findings = ForbiddenComment("^comment").compileAndLint(comment) assertThat(findings).hasSize(1) } @@ -466,11 +458,7 @@ class ForbiddenCommentSpec { } } """.trimIndent() - val findings = ForbiddenComment( - TestConfig( - COMMENTS to listOf("^ foo"), - ) - ).compileAndLint(comment) + val findings = ForbiddenComment("^ foo").compileAndLint(comment) assertThat(findings).hasSize(1) } @@ -484,11 +472,7 @@ class ForbiddenCommentSpec { } } """.trimIndent() - val findings = ForbiddenComment( - TestConfig( - COMMENTS to listOf("^ foo"), - ) - ).compileAndLint(comment) + val findings = ForbiddenComment("^ foo").compileAndLint(comment) assertThat(findings).isEmpty() } @@ -500,9 +484,7 @@ class ForbiddenCommentSpec { val a = 0 } """.trimIndent() - val findings = ForbiddenComment( - TestConfig(COMMENTS to listOf("stopship")) - ).compileAndLint(comment) + val findings = ForbiddenComment("stopship").compileAndLint(comment) assertThat(findings).hasSize(1) } } @@ -515,7 +497,7 @@ class ForbiddenCommentSpec { val comment = """ fun f() {} // TODO implement """.trimIndent() - val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("TODO"))).compileAndLint(comment) + val findings = ForbiddenComment("TODO").compileAndLint(comment) assertThat(findings).hasSize(1) } @@ -526,7 +508,7 @@ class ForbiddenCommentSpec { /*public*/ fun f() {} } """.trimIndent() - val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("public"))).compileAndLint(comment) + val findings = ForbiddenComment("public").compileAndLint(comment) assertThat(findings).hasSize(1) } @@ -537,7 +519,7 @@ class ForbiddenCommentSpec { fun f() /*: String*/ {} } """.trimIndent() - val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("^: .+$"))).compileAndLint(comment) + val findings = ForbiddenComment("^: .+$").compileAndLint(comment) assertThat(findings).hasSize(1) } @@ -549,7 +531,7 @@ class ForbiddenCommentSpec { TODO() } """.trimIndent() - val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("^error"))).compileAndLint(comment) + val findings = ForbiddenComment("^error").compileAndLint(comment) assertThat(findings).hasSize(1) } @@ -560,7 +542,7 @@ class ForbiddenCommentSpec { /**public*/ fun f() {} } """.trimIndent() - val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("public"))).compileAndLint(comment) + val findings = ForbiddenComment("public").compileAndLint(comment) assertThat(findings).hasSize(1) } @@ -571,7 +553,7 @@ class ForbiddenCommentSpec { fun f() /**: String*/ {} } """.trimIndent() - val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("^: .+$"))).compileAndLint(comment) + val findings = ForbiddenComment("^: .+$").compileAndLint(comment) assertThat(findings).hasSize(1) } @@ -583,7 +565,7 @@ class ForbiddenCommentSpec { TODO() } """.trimIndent() - val findings = ForbiddenComment(TestConfig(COMMENTS to listOf("^error"))).compileAndLint(comment) + val findings = ForbiddenComment("^error").compileAndLint(comment) assertThat(findings).hasSize(1) } } @@ -599,7 +581,8 @@ class ForbiddenCommentSpec { * baz qux */ """.trimIndent() - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("^baz"))).compileAndLint(comment)).hasSize(1) + val findings = ForbiddenComment("^baz").compileAndLint(comment) + assertThat(findings).hasSize(1) } @Test @@ -610,7 +593,8 @@ class ForbiddenCommentSpec { * baz qux */ """.trimIndent() - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("(?-m)^baz"))).compileAndLint(comment)).isEmpty() + val findings = ForbiddenComment("(?-m)^baz").compileAndLint(comment) + assertThat(findings).isEmpty() } @Test @@ -621,7 +605,8 @@ class ForbiddenCommentSpec { * baz qux */ """.trimIndent() - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("bar$"))).compileAndLint(comment)).hasSize(1) + val findings = ForbiddenComment("bar$").compileAndLint(comment) + assertThat(findings).hasSize(1) } @Test @@ -632,7 +617,8 @@ class ForbiddenCommentSpec { * baz qux */ """.trimIndent() - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("(?-m)bar$"))).compileAndLint(comment)).isEmpty() + val findings = ForbiddenComment("(?-m)bar$").compileAndLint(comment) + assertThat(findings).isEmpty() } @Test @@ -643,8 +629,10 @@ class ForbiddenCommentSpec { * baz qux */ """.trimIndent() - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("^foo"))).compileAndLint(comment)).hasSize(1) - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("\\Afoo"))).compileAndLint(comment)).hasSize(1) + val findings1 = ForbiddenComment("^foo").compileAndLint(comment) + assertThat(findings1).hasSize(1) + val findings2 = ForbiddenComment("\\Afoo").compileAndLint(comment) + assertThat(findings2).hasSize(1) } @Test @@ -655,8 +643,10 @@ class ForbiddenCommentSpec { * baz qux */ """.trimIndent() - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("qux$"))).compileAndLint(comment)).hasSize(1) - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("qux\\Z"))).compileAndLint(comment)).hasSize(1) + val findings1 = ForbiddenComment("qux$").compileAndLint(comment) + assertThat(findings1).hasSize(1) + val findings2 = ForbiddenComment("qux\\Z").compileAndLint(comment) + assertThat(findings2).hasSize(1) } @Test @@ -667,7 +657,8 @@ class ForbiddenCommentSpec { * baz qux */ """.trimIndent() - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("^foo.*qux$"))).compileAndLint(comment)).hasSize(1) + val findings = ForbiddenComment("^foo.*qux$").compileAndLint(comment) + assertThat(findings).hasSize(1) } @Test @@ -678,7 +669,8 @@ class ForbiddenCommentSpec { * baz qux */ """.trimIndent() - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("(?-s)^foo.*qux$"))).compileAndLint(comment)).isEmpty() + val findings = ForbiddenComment("(?-s)^foo.*qux$").compileAndLint(comment) + assertThat(findings).isEmpty() } @Test @@ -689,7 +681,8 @@ class ForbiddenCommentSpec { * bar qux */ """.trimIndent() - assertThat(ForbiddenComment(TestConfig(COMMENTS to listOf("^foo", "^bar"))).compileAndLint(comment)).hasSize(2) + val findings = ForbiddenComment("^foo", "^bar").compileAndLint(comment) + assertThat(findings).hasSize(2) } } @@ -1022,3 +1015,7 @@ class ForbiddenCommentSpec { } } } + +@Suppress("TestFunctionName") // This is a factory function for ForbiddenComment +private fun ForbiddenComment(vararg comments: String): ForbiddenComment = + ForbiddenComment(TestConfig(COMMENTS to comments.toList())) From 60d53f1d1e189deef1e3ad917643d1b93614a41d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Fri, 19 May 2023 17:04:37 +0100 Subject: [PATCH 19/30] Fix test snippets --- .../arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index be21b96d1b1..f32de28aa1e 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -527,7 +527,7 @@ class ForbiddenCommentSpec { fun `should report a finding when multiline comment exists after code`() { val comment = """ class A { - fun f() = /*error("foo")*/ + fun f(): String = /*error("foo")*/ TODO() } """.trimIndent() @@ -561,7 +561,7 @@ class ForbiddenCommentSpec { fun `should report a finding when kdoc comment exists after code`() { val comment = """ class A { - fun f() = /**error("foo")*/ + fun f(): String = /**error("foo")*/ TODO() } """.trimIndent() From 3c31fe094bc1567a0ede3ad55fe48a53031badd2 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Fri, 19 May 2023 23:01:35 +0530 Subject: [PATCH 20/30] Update detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Róbert Papp --- .../io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index fe5717dd9d1..97f6f1515aa 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -117,7 +117,7 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { private val allowedPatterns: Regex by config("", String::toRegex) @Configuration("error message which overrides the default one") - @Deprecated("Use `comments` and provide `reason` against each `value`") + @Deprecated("Use `comments` and provide `reason` against each `value`.") private val customMessage: String by config("") override fun visitComment(comment: PsiComment) { From ee18e1a71f57a851af81735f1452e33f720ac3db Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Fri, 19 May 2023 23:06:38 +0530 Subject: [PATCH 21/30] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Róbert Papp --- .../arturbosch/detekt/rules/style/ForbiddenComment.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 97f6f1515aa..e25fede0df6 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -105,9 +105,9 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { @Configuration("forbidden comment string patterns") private val comments: List by config( valuesWithReason( - "FIXME:" to "some fixes are pending.", - "STOPSHIP:" to "some changes are present which needs to be addressed before ship.", - "TODO:" to "some changes are pending.", + "FIXME:" to "Forbidden FIXME todo marker in comment, please fix the problem.", + "STOPSHIP:" to "Forbidden STOPSHIP todo marker in comment, please address the problem before shipping the code.", + "TODO:" to "Forbidden TODO todo marker in comment, please do the changes.", ) ) { list -> list.map { Comment(it.value.toRegex(setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.MULTILINE)), it.reason) } @@ -182,7 +182,7 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { "that has been defined as forbidden in detekt." const val DEFAULT_ERROR_MESSAGE = "This comment contains '%s' " + - "that has been forbidden: %s." + "that has been forbidden: %s" } } From b6f35e95baa60c4c22ff06a8af6d8c14fe9688e0 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Fri, 19 May 2023 23:28:44 +0530 Subject: [PATCH 22/30] Refactor `report` to common `reportCodeSmell` method --- .../detekt/rules/style/ForbiddenComment.kt | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index e25fede0df6..28e6dabbea3 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -140,29 +140,27 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { @Suppress("DEPRECATION") values.forEach { if (text.contains(it, ignoreCase = true)) { - report( - CodeSmell( - issue, - Entity.from(comment), - getErrorMessage(it) - ) - ) + reportIssue(comment, getErrorMessage(it)) } } comments.forEach { if (it.value.containsMatchIn(text)) { - report( - CodeSmell( - issue, - Entity.from(comment), - getErrorMessage(it) - ) - ) + reportIssue(comment, getErrorMessage(it)) } } } + private fun reportIssue(comment: PsiElement, msg: String) { + report( + CodeSmell( + issue, + Entity.from(comment), + msg + ) + ) + } + private fun PsiComment.getContent(): String = text.getCommentContent() private fun getErrorMessage(comment: Comment): String = From a5be5a40d6b2fa71b7588660520b46457ae4bbbc Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Fri, 19 May 2023 23:31:34 +0530 Subject: [PATCH 23/30] Update TC to reflect the updated message --- detekt-core/src/main/resources/default-detekt-config.yml | 6 +++--- detekt-core/src/main/resources/deprecation.properties | 2 +- .../arturbosch/detekt/rules/style/ForbiddenComment.kt | 3 ++- .../arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt | 8 ++++++-- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 1365510582a..fa8838e4ddb 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -567,11 +567,11 @@ style: ForbiddenComment: active: true comments: - - reason: 'some fixes are pending.' + - reason: 'Forbidden FIXME todo marker in comment, please fix the problem.' value: 'FIXME:' - - reason: 'some changes are present which needs to be addressed before ship.' + - reason: 'Forbidden STOPSHIP todo marker in comment, please address the problem before shipping the code.' value: 'STOPSHIP:' - - reason: 'some changes are pending.' + - reason: 'Forbidden TODO todo marker in comment, please do the changes.' value: 'TODO:' allowedPatterns: '' ForbiddenImport: diff --git a/detekt-core/src/main/resources/deprecation.properties b/detekt-core/src/main/resources/deprecation.properties index e92b97340f2..ca4ccbc487f 100644 --- a/detekt-core/src/main/resources/deprecation.properties +++ b/detekt-core/src/main/resources/deprecation.properties @@ -16,7 +16,7 @@ potential-bugs>IgnoredReturnValue>restrictToAnnotatedMethods=Use `restrictToConf potential-bugs>LateinitUsage>excludeAnnotatedProperties=Use `ignoreAnnotated` instead potential-bugs>MissingWhenCase=Rule deprecated as compiler performs this check by default potential-bugs>RedundantElseInWhen=Rule deprecated as compiler performs this check by default -style>ForbiddenComment>customMessage=Use `comments` and provide `reason` against each `value` +style>ForbiddenComment>customMessage=Use `comments` and provide `reason` against each `value`. style>ForbiddenComment>values=Use `comments` instead, make sure you escape your text for Regular Expressions. style>ForbiddenPublicDataClass=Rule migrated to `libraries` ruleset plugin style>FunctionOnlyReturningConstant>excludeAnnotatedFunction=Use `ignoreAnnotated` instead diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 28e6dabbea3..83c799356c1 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -106,7 +106,8 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { private val comments: List by config( valuesWithReason( "FIXME:" to "Forbidden FIXME todo marker in comment, please fix the problem.", - "STOPSHIP:" to "Forbidden STOPSHIP todo marker in comment, please address the problem before shipping the code.", + "STOPSHIP:" to "Forbidden STOPSHIP todo marker in comment, please address the problem before shipping " + + "the code.", "TODO:" to "Forbidden TODO todo marker in comment, please do the changes.", ) ) { list -> diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index f32de28aa1e..c559ade58a8 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -29,7 +29,11 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment().compileAndLint("// TODO: I need to fix this.") assertThat(findings).hasSize(1) assertThat(findings[0]).hasMessage( - String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, "TODO:", "some changes are pending.") + String.format( + ForbiddenComment.DEFAULT_ERROR_MESSAGE, + "TODO:", + "Forbidden TODO todo marker in comment, please do the changes." + ) ) } @@ -61,7 +65,7 @@ class ForbiddenCommentSpec { String.format( ForbiddenComment.DEFAULT_ERROR_MESSAGE, "STOPSHIP:", - "some changes are present which needs to be addressed before ship." + "Forbidden STOPSHIP todo marker in comment, please address the problem before shipping the code." ) ) } From 1565fb607416284edf5ac0ac29ff0bbd959aa56a Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Fri, 19 May 2023 23:40:57 +0530 Subject: [PATCH 24/30] Update the detekt.yml `ForbiddenComment` `reason` config to match the default values --- config/detekt/detekt.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml index dae6189bd8d..5b5b6bcec62 100644 --- a/config/detekt/detekt.yml +++ b/config/detekt/detekt.yml @@ -171,11 +171,11 @@ style: active: true comments: - value: 'FIXME:' - reason: 'Some fixes are pending, please complete them.' + reason: 'Forbidden FIXME todo marker in comment, please fix the problem.' - value: 'STOPSHIP:' - reason: 'Some changes are present which need to be addressed before release.' + reason: 'Forbidden STOPSHIP todo marker in comment, please address the problem before shipping the code.' - value: 'TODO:' - reason: 'Some changes are pending, please complete them.' + reason: 'Forbidden TODO todo marker in comment, please do the changes.' - value: '@author' reason: 'Authors are not recorded in KDoc.' - value: '@requiresTypeResolution' From f4c59c12da88d27b7e96ea250b017beaac3aaacb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Fri, 19 May 2023 21:20:56 +0100 Subject: [PATCH 25/30] Improve marker removal docs --- .../gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 83c799356c1..970d0716523 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -27,8 +27,8 @@ import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType * * single line comments are always separate, conjoint lines are not merged. * * multi line comments are not split up, the regex will be applied to the whole comment. * * KDoc comments are not split up, the regex will be applied to the whole comment. - * * The comment markers (`//`, `/​*`, `/​**`, `*` aligners, `*​/`) are removed before applying the regex. - * One leading space is removed from each line of the comment, after starting markers and aligners. + * * The following comment delimiters (and indentation before them) are removed before applying the regex: + * `//`, `// `, `/​*`, `/​* `, `/​**`, `*` aligners, `*​/`, ` *​/` * * The regex is applied as a multiline regex, * see [Anchors](https://www.regular-expressions.info/anchors.html) for more info. * To match the start and end of each line, use `^` and `$`. From 948cb73d879d97d0f8ba34311df5c21c7489b85e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Fri, 19 May 2023 21:21:07 +0100 Subject: [PATCH 26/30] Clarify why it matters. --- .../io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 970d0716523..df6dfebd32d 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -39,6 +39,7 @@ import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType * To turn off this behavior, use `(?-s)` at the start of your regex, or use `[^\r\n]*` instead of `.*`. * * The regex will be searched using "contains" semantics not "matches", * so partial comment matches will flag forbidden comments. + * In practice this means there's no need to start and end the regex with `.*`. * * The rule can be configured to add extra comments to the list of forbidden comments, here are some examples: * ```yaml From 0a1763f2ea8150a72955be2c287cfe19d93bafda Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Sat, 20 May 2023 03:33:43 +0530 Subject: [PATCH 27/30] Update the warning message to only use `reason` if available --- .../detekt/rules/style/ForbiddenComment.kt | 11 ++---- .../rules/style/ForbiddenCommentSpec.kt | 38 +++++++++++-------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index df6dfebd32d..9435d2126bc 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -166,23 +166,18 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { private fun PsiComment.getContent(): String = text.getCommentContent() private fun getErrorMessage(comment: Comment): String = - comment.reason - ?.let { reason -> String.format(DEFAULT_ERROR_MESSAGE, comment.value.pattern, reason) } - ?: String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, comment.value.pattern) + comment.reason ?: String.format(DEFAULT_ERROR_MESSAGE, comment.value.pattern) @Suppress("DEPRECATION") private fun getErrorMessage(value: String): String = customMessage.takeUnless { it.isEmpty() } - ?: String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, value) + ?: String.format(DEFAULT_ERROR_MESSAGE, value) private data class Comment(val value: Regex, val reason: String?) companion object { - const val DEFAULT_ERROR_MESSAGE_WITH_NO_REASON = "This comment contains '%s' " + - "that has been defined as forbidden in detekt." - const val DEFAULT_ERROR_MESSAGE = "This comment contains '%s' " + - "that has been forbidden: %s" + "that has been defined as forbidden in detekt." } } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt index c559ade58a8..dd20491f7d3 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenCommentSpec.kt @@ -2,6 +2,7 @@ package io.gitlab.arturbosch.detekt.rules.style +import io.gitlab.arturbosch.detekt.api.ValueWithReason import io.gitlab.arturbosch.detekt.test.TestConfig import io.gitlab.arturbosch.detekt.test.assertThat import io.gitlab.arturbosch.detekt.test.compileAndLint @@ -28,13 +29,7 @@ class ForbiddenCommentSpec { fun reportTodoColon() { val findings = ForbiddenComment().compileAndLint("// TODO: I need to fix this.") assertThat(findings).hasSize(1) - assertThat(findings[0]).hasMessage( - String.format( - ForbiddenComment.DEFAULT_ERROR_MESSAGE, - "TODO:", - "Forbidden TODO todo marker in comment, please do the changes." - ) - ) + assertThat(findings[0]).hasMessage("Forbidden TODO todo marker in comment, please do the changes.") } @Test @@ -62,11 +57,7 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment().compileAndLint("// STOPSHIP: I need to fix this.") assertThat(findings).hasSize(1) assertThat(findings[0]).hasMessage( - String.format( - ForbiddenComment.DEFAULT_ERROR_MESSAGE, - "STOPSHIP:", - "Forbidden STOPSHIP todo marker in comment, please address the problem before shipping the code." - ) + "Forbidden STOPSHIP todo marker in comment, please address the problem before shipping the code." ) } @@ -258,15 +249,26 @@ class ForbiddenCommentSpec { @Nested inner class `custom message is not configured` { private val messageConfig = TestConfig(VALUES to "Comment") + private val messageConfigWithReason = ForbiddenComment( + ValueWithReason("Comment", "Comment is disallowed") + ) @Test fun `should report a Finding with default Message`() { val comment = "// Comment" val findings = ForbiddenComment(messageConfig).compileAndLint(comment) - val expectedMessage = String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, "Comment") + val expectedMessage = String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, "Comment") assertThat(findings).hasSize(1) assertThat(findings.first().message).isEqualTo(expectedMessage) } + + @Test + fun `should report a Finding with reason`() { + val comment = "// Comment" + val findings = ForbiddenComment(messageConfigWithReason).compileAndLint(comment) + assertThat(findings).hasSize(1) + assertThat(findings.first().message).isEqualTo("Comment is disallowed") + } } @Nested @@ -289,7 +291,7 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) assertThat(findings[0]) - .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, "STOPSHIP")) + .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, "STOPSHIP")) } @Test @@ -298,7 +300,7 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) assertThat(findings[0]) - .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, patternStr)) + .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, patternStr)) } @Test @@ -307,7 +309,7 @@ class ForbiddenCommentSpec { val findings = ForbiddenComment(messageConfig).compileAndLint(comment) assertThat(findings).hasSize(1) assertThat(findings[0]) - .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, patternStr)) + .hasMessage(String.format(ForbiddenComment.DEFAULT_ERROR_MESSAGE, patternStr)) } @Test @@ -1023,3 +1025,7 @@ class ForbiddenCommentSpec { @Suppress("TestFunctionName") // This is a factory function for ForbiddenComment private fun ForbiddenComment(vararg comments: String): ForbiddenComment = ForbiddenComment(TestConfig(COMMENTS to comments.toList())) + +@Suppress("TestFunctionName") +private fun ForbiddenComment(vararg comments: ValueWithReason): ForbiddenComment = + ForbiddenComment(TestConfig(COMMENTS to comments.map { mapOf("value" to it.value, "reason" to it.reason) })) From b03bd56716a9bfd194fa5333a0181b43e0e7a315 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Sat, 20 May 2023 03:58:02 +0530 Subject: [PATCH 28/30] Remove not necessary ZWSP --- .../gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 9435d2126bc..9a17288c11e 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -66,13 +66,13 @@ import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType * * - reason: 'Use @androidx.annotation.VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) instead.' * value: '^private$' - * # Non-compliant: /​*private*​/ fun f() { } + * # Non-compliant: /*private*/ fun f() { } * * - reason: 'KDoc tag should have a value.' * value: '^\s*@(?!suppress|hide)\w+\s*$' * # Explanation: full line with optional leading and trailing space, and an at @ character followed by a word, * # but not @suppress or @hide because those don't need content afterwards. - * # Non-compliant: /​** ... @see *​/ + * # Non-compliant: /** ... @see */ * * - reason: 'include an issue link at the beginning preceded by a space' * value: 'BUG:(?! https://github\.com/company/repo/issues/\d+).*' From fe491f6ee007fee671ec31e5c1e74f09e340df87 Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Sat, 20 May 2023 03:59:09 +0530 Subject: [PATCH 29/30] Update message to remove ` in detekt` --- .../io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 9a17288c11e..0aac927753e 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -177,7 +177,7 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { companion object { const val DEFAULT_ERROR_MESSAGE = "This comment contains '%s' " + - "that has been defined as forbidden in detekt." + "that has been defined as forbidden." } } From 61662970d71764a0fcc6017c66f9c62c994d3328 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Sat, 20 May 2023 18:12:40 +0100 Subject: [PATCH 30/30] Minor clarifications and formatting. --- .../detekt/rules/style/ForbiddenComment.kt | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt index 0aac927753e..58f93769ebc 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenComment.kt @@ -23,21 +23,21 @@ import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType * development. Offending code comments will then be reported. * * The regular expressions in `comments` list will have the following behaviors while matching the comments: - * * Each comment will be handled individually as a whole. - * * single line comments are always separate, conjoint lines are not merged. + * * **Each comment will be handled individually.** + * * single line comments are always separate, consecutive lines are not merged. * * multi line comments are not split up, the regex will be applied to the whole comment. * * KDoc comments are not split up, the regex will be applied to the whole comment. - * * The following comment delimiters (and indentation before them) are removed before applying the regex: + * * **The following comment delimiters (and indentation before them) are removed** before applying the regex: * `//`, `// `, `/​*`, `/​* `, `/​**`, `*` aligners, `*​/`, ` *​/` - * * The regex is applied as a multiline regex, + * * **The regex is applied as a multiline regex**, * see [Anchors](https://www.regular-expressions.info/anchors.html) for more info. * To match the start and end of each line, use `^` and `$`. * To match the start and end of the whole comment, use `\A` and `\Z`. * To turn off multiline, use `(?-m)` at the start of your regex. - * * The regex is applied with dotall semantics, meaning `.` will match any character including newlines, + * * **The regex is applied with dotall semantics**, meaning `.` will match any character including newlines, * this is to ensure that freeform line-wrapping doesn't mess with simple regexes. * To turn off this behavior, use `(?-s)` at the start of your regex, or use `[^\r\n]*` instead of `.*`. - * * The regex will be searched using "contains" semantics not "matches", + * * **The regex will be searched using "contains" semantics** not "matches", * so partial comment matches will flag forbidden comments. * In practice this means there's no need to start and end the regex with `.*`. * @@ -46,11 +46,11 @@ import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType * ForbiddenComment: * comments: * # Repeat the default configuration if it's still needed. - * - reason: 'some fixes are pending.' + * - reason: 'Forbidden FIXME todo marker in comment, please fix the problem.' * value: 'FIXME:' - * - reason: 'some changes are present which needs to be addressed before ship.' + * - reason: 'Forbidden STOPSHIP todo marker in comment, please address the problem before shipping the code.' * value: 'STOPSHIP:' - * - reason: 'some changes are pending.' + * - reason: 'Forbidden TODO todo marker in comment, please do the changes.' * value: 'TODO:' * # Add additional patterns to the list. * @@ -59,8 +59,6 @@ import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType * * - reason: 'REVIEW markers are not allowed in production code, only use before PR is merged.' * value: '^\s*(?i)REVIEW\b' - * # Explanation: at the beginning of the line, optional leading space, - * # case insensitive (e.g. REVIEW, review, Review), and REVIEW only as a full word. * # Non-compliant: // REVIEW this code before merging. * # Compliant: // Preview will show up here. * @@ -70,9 +68,8 @@ import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType * * - reason: 'KDoc tag should have a value.' * value: '^\s*@(?!suppress|hide)\w+\s*$' - * # Explanation: full line with optional leading and trailing space, and an at @ character followed by a word, - * # but not @suppress or @hide because those don't need content afterwards. * # Non-compliant: /** ... @see */ + * # Compliant: /** ... @throws IOException when there's a network problem */ * * - reason: 'include an issue link at the beginning preceded by a space' * value: 'BUG:(?! https://github\.com/company/repo/issues/\d+).*' @@ -107,8 +104,8 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { private val comments: List by config( valuesWithReason( "FIXME:" to "Forbidden FIXME todo marker in comment, please fix the problem.", - "STOPSHIP:" to "Forbidden STOPSHIP todo marker in comment, please address the problem before shipping " + - "the code.", + "STOPSHIP:" to "Forbidden STOPSHIP todo marker in comment, " + + "please address the problem before shipping the code.", "TODO:" to "Forbidden TODO todo marker in comment, please do the changes.", ) ) { list -> @@ -176,8 +173,7 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) { private data class Comment(val value: Regex, val reason: String?) companion object { - const val DEFAULT_ERROR_MESSAGE = "This comment contains '%s' " + - "that has been defined as forbidden." + const val DEFAULT_ERROR_MESSAGE = "This comment contains '%s' that has been defined as forbidden." } }