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 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -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."
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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."
)
}

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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) }))