Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add comments with a list of regexes to ForbiddenComment #5981

Merged
merged 30 commits into from
May 21, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
6400988
Add valuePatterns list of regexes
atulgpt Apr 10, 2023
5eba48b
Apply suggestions from code review
atulgpt Apr 11, 2023
6ef834c
Deprecate values option
atulgpt Apr 11, 2023
06350d1
Check against complete text
atulgpt Apr 13, 2023
d9dc02f
Apply suggestions from code review
atulgpt May 6, 2023
6e22238
Update the `valuePatterns` config name to `comments`
atulgpt May 6, 2023
e2a3af0
Use static import of `io.gitlab.arturbosch.detekt.test.assertThat`
atulgpt May 6, 2023
7580f5c
Use `valuesWithReason` in `comments` config
atulgpt May 15, 2023
8e11b39
Incorporate review comments and align comments for indented code
atulgpt May 16, 2023
42a80c7
Update detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt…
atulgpt May 18, 2023
373bc23
Refactor code make `trimIndentIgnoringFirstLine` out
atulgpt May 18, 2023
a97dcab
Shorten name: if it's trimmed, it cannot be also "full"
TWiStErRob May 19, 2023
b747f77
Test name fixes
TWiStErRob May 19, 2023
007d3b4
Add tests for trailing and other mixed code-comment pairs
TWiStErRob May 19, 2023
4687325
Migrate detekt config to use new property. Fixes:
TWiStErRob May 19, 2023
ffff4f7
Format for visibility
TWiStErRob May 19, 2023
b4ca396
Document behavior, add tests for documented behaviors and fix them.
TWiStErRob May 19, 2023
5673f92
De-noise ForbiddenComment instance creations
TWiStErRob May 19, 2023
60d53f1
Fix test snippets
TWiStErRob May 19, 2023
3c31fe0
Update detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt…
atulgpt May 19, 2023
ee18e1a
Apply suggestions from code review
atulgpt May 19, 2023
b6f35e9
Refactor `report` to common `reportCodeSmell` method
atulgpt May 19, 2023
a5be5a4
Update TC to reflect the updated message
atulgpt May 19, 2023
1565fb6
Update the detekt.yml `ForbiddenComment` `reason` config to match the…
atulgpt May 19, 2023
f4c59c1
Improve marker removal docs
TWiStErRob May 19, 2023
948cb73
Clarify why it matters.
TWiStErRob May 19, 2023
0a1763f
Update the warning message to only use `reason` if available
atulgpt May 19, 2023
b03bd56
Remove not necessary ZWSP
atulgpt May 19, 2023
fe491f6
Update message to remove ` in detekt`
atulgpt May 19, 2023
6166297
Minor clarifications and formatting.
TWiStErRob May 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion detekt-core/src/main/resources/default-detekt-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ style:
value: 'java.lang.annotation.Inherited'
ForbiddenComment:
active: true
values:
comments:
- 'FIXME:'
- 'STOPSHIP:'
- 'TODO:'
Expand Down
1 change: 1 addition & 0 deletions detekt-core/src/main/resources/deprecation.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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 `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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,13 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) {
)

@Configuration("forbidden comment strings")
private val values: List<String> by config(listOf("FIXME:", "STOPSHIP:", "TODO:"))
@Deprecated("Use `comments` instead, make sure you escape your text for Regular Expressions.")
private val values: List<String> by config(emptyList())

@Configuration("forbidden comment string patterns")
private val comments: List<Regex> by config(listOf("FIXME:", "STOPSHIP:", "TODO:")) {
it.map(String::toRegex)
}
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved

@Configuration("ignores comments which match the specified regular expression. For example `Ticket|Task`.")
private val allowedPatterns: Regex by config("", String::toRegex)
Expand All @@ -63,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(
Expand All @@ -74,6 +81,18 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) {
)
}
}

comments.forEach {
if (it.containsMatchIn(text)) {
report(
CodeSmell(
issue,
Entity.from(comment),
getErrorMessage(it.pattern)
)
)
}
}
}

private fun getErrorMessage(value: String): String =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@file:Suppress("ClassName")
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved

package io.gitlab.arturbosch.detekt.rules.style

import io.gitlab.arturbosch.detekt.test.TestConfig
Expand All @@ -8,6 +10,7 @@ import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test

private const val VALUES = "values"
private const val COMMENTS = "comments"
private const val ALLOWED_PATTERNS = "allowedPatterns"
private const val MESSAGE = "customMessage"

atulgpt marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -75,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 = """
Expand All @@ -98,7 +110,7 @@ class ForbiddenCommentSpec {

@Nested
inner class `when given Banana` {
val config = TestConfig(VALUES to "Banana")
val config = TestConfig(COMMENTS to "Banana")

@Test
@DisplayName("should not report TODO: usages")
Expand Down Expand Up @@ -138,7 +150,7 @@ class ForbiddenCommentSpec {
@Nested
@DisplayName("when given listOf(\"banana\")")
inner class ListOfBanana {
val config = TestConfig(VALUES to listOf("Banana"))
val config = TestConfig(COMMENTS to listOf("Banana"))

@Test
@DisplayName("should not report TODO: usages")
Expand Down Expand Up @@ -235,4 +247,60 @@ class ForbiddenCommentSpec {
assertThat(findings.first().message).isEqualTo(expectedMessage)
}
}

@Nested
inner class `custom value pattern is configured` {
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved
private val patternStr = """^//( )?(?i)REVIEW\b"""
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved
private val messageConfig = TestConfig(
COMMENTS 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."
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved
val findings = ForbiddenComment(messageConfig).compileAndLint(comment)
assertThat(findings).hasSize(1)
io.gitlab.arturbosch.detekt.test.assertThat(findings[0])
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved
.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)
}

@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)
}
}
}