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 14 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
12 changes: 7 additions & 5 deletions detekt-core/src/main/resources/default-detekt-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -566,12 +566,14 @@ style:
value: 'java.lang.annotation.Inherited'
ForbiddenComment:
active: true
values:
- 'FIXME:'
- 'STOPSHIP:'
- 'TODO:'
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved
comments:
- 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: []
Expand Down
2 changes: 2 additions & 0 deletions detekt-core/src/main/resources/deprecation.properties
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ 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
style>LibraryCodeMustSpecifyReturnType=Rule migrated to `libraries` ruleset plugin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -38,17 +39,30 @@ 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<Comment> 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.",
atulgpt marked this conversation as resolved.
Show resolved Hide resolved
)
) { list ->
list.map { Comment(it.value.toRegex(), it.reason) }
}
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)

@Configuration("error message which overrides the default one")
@Deprecated("Use `comments` and provide `reason` against each `value`")
atulgpt marked this conversation as resolved.
Show resolved Hide resolved
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)
}

Expand All @@ -63,6 +77,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,13 +89,82 @@ class ForbiddenComment(config: Config = Config.empty) : Rule(config) {
)
}
}

comments.forEach {
if (it.value.containsMatchIn(text)) {
report(
CodeSmell(
issue,
Entity.from(comment),
getErrorMessage(it)
)
)
atulgpt marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

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)

@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)
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved

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."
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved

const val DEFAULT_ERROR_MESSAGE = "This comment contains '%s' " +
"that has been forbidden: %s."
atulgpt marked this conversation as resolved.
Show resolved Hide resolved
}
}

internal fun String.getCommentContent(): String {
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved
atulgpt marked this conversation as resolved.
Show resolved Hide resolved
return if (this.startsWith("//")) {
this.removePrefix("//").removePrefix(" ")
} else {
this
.trimIndentIgnoringFirstLine()
// Process line by line.
.lineSequence()
// Remove starting, aligning and ending markers.
.map {
it
.let { fullLine ->
val trimmedStartLine = fullLine.trimStart()
if (trimmedStartLine.startsWith("/*")) {
trimmedStartLine.removePrefix("/*").removePrefix(" ")
} else if (trimmedStartLine.startsWith("*") && trimmedStartLine.startsWith("*/").not()) {
trimmedStartLine.removePrefix("*").removePrefix(" ")
} else {
fullLine
}
}
.let { lineWithoutStartMarker ->
if (lineWithoutStartMarker.endsWith("*/")) {
lineWithoutStartMarker.removeSuffix("*/").removeSuffix(" ")
} else {
lineWithoutStartMarker
}
}
}
// 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()
}