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.

Sign up for GitHub

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

Jump to bottom

Adds issue details to findings on FindingsReport and FileBasedFindingsReporter #4464

Merged
merged 5 commits into from Jan 31, 2022
Merged

Adds issue details to findings on FindingsReport and FileBasedFindingsReporter #4464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8973109
Changes FindingsReporter detail level
gfreivasc Jan 7, 2022
d9382ed
Updates docs with reporting changes
gfreivasc Jan 9, 2022
41d7f0b
Adapts tests to findings reporter changes
gfreivasc Jan 11, 2022
b5b96c4
Creates extension method for console report
gfreivasc Jan 18, 2022
b6bee2b
Truncates long messages for console reports
gfreivasc Jan 18, 2022
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
4 changes: 2 additions & 2 deletions detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/runners/RunnerSpec.kt
View file
Open in desktop
Expand Up @@ -129,7 +129,7 @@ class RunnerSpec {

@Test
fun `writes no build related output to output printer`() {
assertThat(outPrintStream.toString()).doesNotContain("test - [Poko]")
assertThat(outPrintStream.toString()).doesNotContain("test - [A failure]")
}

@Test
Expand Down Expand Up @@ -158,7 +158,7 @@ class RunnerSpec {

@Test
fun `writes output to output printer`() {
assertThat(outPrintStream.toString()).contains("test - [Poko]")
assertThat(outPrintStream.toString()).contains("test - [A failure]")
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/runners/TestRules.kt
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ class TestProvider : RuleSetProvider {
}

class TestRule : Rule() {
override val issue = Issue("test", Severity.Minor, "", Debt.FIVE_MINS)
override val issue = Issue("test", Severity.Minor, "A failure", Debt.FIVE_MINS)
override fun visitClass(klass: KtClass) {
if (klass.name == "Poko") {
report(CodeSmell(issue, Entity.from(klass), issue.description))
Expand Down
21 changes: 20 additions & 1 deletion detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/reporting/Reporting.kt
View file
Open in desktop
Expand Up @@ -10,6 +10,8 @@ import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.Detektion
import io.gitlab.arturbosch.detekt.api.Finding
import io.gitlab.arturbosch.detekt.api.RuleSetId
import io.gitlab.arturbosch.detekt.api.ThresholdedCodeSmell
import kotlin.text.Typography.ellipsis

internal fun defaultReportMapping(reportId: String) = when (reportId) {
TxtOutputReport::class.java.simpleName -> "txt"
Expand All @@ -30,7 +32,7 @@ internal fun printFindings(findings: Map<String, List<Finding>>): String {
append("$key - $debt debt\n")
issues.forEach {
append("\t")
append(it.compact().yellow())
append(it.detailed().yellow())
append("\n")
}
}
Expand All @@ -46,6 +48,8 @@ const val EXCLUDE_CORRECTABLE = "excludeCorrectable"
const val DETEKT_OUTPUT_REPORT_PATHS_KEY = "detekt.output.report.paths.key"
const val DETEKT_OUTPUT_REPORT_BASE_PATH_KEY = "detekt.output.report.base.path"

private const val REPORT_MESSAGE_SIZE_LIMIT = 80

fun Config.excludeCorrectable(): Boolean = subConfig(BUILD).valueOrDefault(EXCLUDE_CORRECTABLE, false)

fun Detektion.filterEmptyIssues(config: Config): Map<RuleSetId, List<Finding>> {
Expand All @@ -68,3 +72,18 @@ fun Detektion.filterAutoCorrectedIssues(config: Config): Map<RuleSetId, List<Fin
}
return filteredFindings
}

private fun Finding.truncatedMessage(): String {
val message = messageOrDescription()
.replace(Regex("\\s+"), " ")
.trim()
return when {
message.length > REPORT_MESSAGE_SIZE_LIMIT -> "${message.take(REPORT_MESSAGE_SIZE_LIMIT)}($ellipsis)"
else -> message
}
}
Comment on lines +76 to +84
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cortinico I applied this treatment to handle the scenarios you've raised. As seen above, I've loosely set REPORT_MESSAGE_SIZE_LIMIT to be 80. There is a test that validates both truncation and space characters (\n, \r, \t, ...).


private fun Finding.detailed(): String = when (this) {
is ThresholdedCodeSmell -> "$id - $metric - [${truncatedMessage()}] at ${location.compact()}"
else -> "$id - [${truncatedMessage()}] at ${location.compact()}"
}
19 changes: 19 additions & 0 deletions .../src/test/kotlin/io/gitlab/arturbosch/detekt/core/reporting/console/FindingsReportSpec.kt
View file
Open in desktop
Expand Up @@ -6,7 +6,9 @@ import io.gitlab.arturbosch.detekt.api.Finding
import io.gitlab.arturbosch.detekt.core.reporting.AutoCorrectableIssueAssert
import io.gitlab.arturbosch.detekt.core.reporting.decolorized
import io.gitlab.arturbosch.detekt.test.TestDetektion
import io.gitlab.arturbosch.detekt.test.createEntity
import io.gitlab.arturbosch.detekt.test.createFinding
import io.gitlab.arturbosch.detekt.test.createIssue
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Nested
Expand Down Expand Up @@ -69,6 +71,23 @@ class FindingsReportSpec {
val report = FindingsReport()
AutoCorrectableIssueAssert.isReportNull(report)
}

@Test
fun `truncates long message`() {
val expectedContent = readResourceContent("/reporting/long-messages-report.txt")
val longMessage = "This is just a long message that should be truncated after a given " +
"threshold is reached."
val multilineMessage = "A multiline\n\r\tmessage.\t "
val detektion = object : TestDetektion() {
override val findings: Map<String, List<Finding>> = mapOf(
"Ruleset" to listOf(
createFinding(createIssue("LongRule"), createEntity("File.kt"), longMessage),
createFinding(createIssue("MultilineRule"), createEntity("File.kt"), multilineMessage),
),
)
}
assertThat(subject.render(detektion)?.decolorized()).isEqualTo(expectedContent)
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions detekt-core/src/test/resources/reporting/findings-report.txt
View file
Open in desktop
@@ -1,7 +1,7 @@
Ruleset1 - 10min debt
TestSmell - [TestEntity] at TestFile.kt:1:1
TestSmell - [TestEntity] at TestFile.kt:1:1
TestSmell - [TestMessage] at TestFile.kt:1:1
TestSmell - [TestMessage] at TestFile.kt:1:1
Ruleset2 - 5min debt
TestSmell - [TestEntity] at TestFile.kt:1:1
TestSmell - [TestMessage] at TestFile.kt:1:1

Overall debt: 15min
6 changes: 3 additions & 3 deletions detekt-core/src/test/resources/reporting/grouped-findings-report.txt
View file
Open in desktop
@@ -1,7 +1,7 @@
File1.kt - 10min debt
TestSmell - [TestEntity] at File1.kt:1:1
TestSmell - [TestEntity] at File1.kt:1:1
TestSmell - [TestMessage] at File1.kt:1:1
TestSmell - [TestMessage] at File1.kt:1:1
File2.kt - 5min debt
TestSmell - [TestEntity] at File2.kt:1:1
TestSmell - [TestMessage] at File2.kt:1:1

Overall debt: 15min
5 changes: 5 additions & 0 deletions detekt-core/src/test/resources/reporting/long-messages-report.txt
View file
Open in desktop
@@ -0,0 +1,5 @@
Ruleset - 10min debt
LongRule - [This is just a long message that should be truncated after a given threshold is (…)] at File.kt:1:1
MultilineRule - [A multiline message.] at File.kt:1:1

Overall debt: 10min
10 changes: 5 additions & 5 deletions docs/pages/reporting.md
View file
Open in desktop
Expand Up @@ -17,11 +17,11 @@ Similar to the console output, each line of the txt output represents a finding
finding signature to help edit [baseline files](gettingstarted/gradle.md).

```
EmptyFunctionBlock - [apply] at /user/home/detekt/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt:14:42 - Signature=DetektPlugin.kt$DetektPlugin${ }
NoUnusedImports - [] at /user/home/detekt/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt:9:1 - Signature=io.gitlab.arturbosch.detekt.DetektPlugin.kt:9
NoUnusedImports - [] at /user/home/detekt/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt:10:1 - Signature=io.gitlab.arturbosch.detekt.DetektPlugin.kt:10
NoConsecutiveBlankLines - [] at /user/home/detekt/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt:86:1 - Signature=io.gitlab.arturbosch.detekt.DetektPlugin.kt:86
UnusedPrivateMember - [registerDetektJvmTasks] at /user/home/detekt/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt:17:5 - Signature=DetektPlugin.kt$DetektPlugin$private fun Project.registerDetektJvmTasks(extension: DetektExtension)
EmptyFunctionBlock - [This empty block of code can be removed.] at /user/home/detekt/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt:14:42 - Signature=DetektPlugin.kt$DetektPlugin${ }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last question: are there scenario where this text can either:

  1. Be too long
  2. Contain \n?

Askign as if so, we should escape it as it would screw up the final reporting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I'm aware of, there's no checks or anything to prevent these to be the case. These scenarios could also affect LiteFindingsReport. We could run a replace .replace('\n', ' ') for the second case but I can't think of what could be done in the first one. Truncate and ellipsis? What could be a threshold?

Copy link
Member

@cortinico cortinico Jan 30, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Truncate and ellipsis? What could be a threshold?\

I think it's something we can address should we face it. Ideally Truncate and ellipsis sounds like a decent way to clean this up, but it's something we can follow-up on.

EDIT: Just noticed you implemented it 👍 We can fine tune it if needed

NoUnusedImports - [Unused import] at /user/home/detekt/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt:9:1 - Signature=io.gitlab.arturbosch.detekt.DetektPlugin.kt:9
NoUnusedImports - [Unused import] at /user/home/detekt/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt:10:1 - Signature=io.gitlab.arturbosch.detekt.DetektPlugin.kt:10
NoConsecutiveBlankLines - [Needless blank line(s)] at /user/home/detekt/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt:86:1 - Signature=io.gitlab.arturbosch.detekt.DetektPlugin.kt:86
UnusedPrivateMember - [Private function registerDetektJvmTasks is unused.] at /user/home/detekt/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt:17:5 - Signature=DetektPlugin.kt$DetektPlugin$private fun Project.registerDetektJvmTasks(extension: DetektExtension)
```

### HTML
Expand Down