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

Don't show the number of issues generating the BindingContext #5449

Merged
merged 1 commit into from Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -18,7 +18,6 @@ internal fun generateBindingContext(
classpath: List<String>,
files: List<KtFile>,
debugPrinter: (() -> String) -> Unit,
warningPrinter: (String) -> Unit,
): BindingContext {
if (classpath.isEmpty()) {
return BindingContext.EMPTY
Expand All @@ -27,7 +26,6 @@ internal fun generateBindingContext(
val messageCollector = DetektMessageCollector(
minSeverity = CompilerMessageSeverity.ERROR,
debugPrinter = debugPrinter,
warningPrinter = warningPrinter,
)

val analyzer = AnalyzerWithCompilerReport(
Expand All @@ -46,15 +44,12 @@ internal fun generateBindingContext(
)
}

messageCollector.printIssuesCountIfAny()

return analyzer.analysisResult.bindingContext
}

internal class DetektMessageCollector(
private val minSeverity: CompilerMessageSeverity,
private val debugPrinter: (() -> String) -> Unit,
private val warningPrinter: (String) -> Unit,
) : MessageCollector by MessageCollector.NONE {
private var messages = 0

Expand All @@ -64,15 +59,6 @@ internal class DetektMessageCollector(
messages++
}
}

fun printIssuesCountIfAny() {
if (messages > 0) {
warningPrinter(
Copy link
Member

Choose a reason for hiding this comment

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

rather than fully killing put it behind the debug? or does that already print the number of problems? I should probably check 😂

Copy link
Member

Choose a reason for hiding this comment

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

I double-checked, when running with debug mode, it doesn't print the number of compilation errors, just lists them all. Since the numbers are high, and they can fluctuate, I think it would be beneficial to print the number of them, although that's probably not in scope here. (javac does print error and apt problem count, but kotlinc doesn't)

Copy link
Member Author

Choose a reason for hiding this comment

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

Out of scope as you said. We are removing this because no one cares. If someone wants it I'll be happy to review the pr but otherwise it seems a YAGNI.

"The BindingContext was created with $messages issues. " +
"Run detekt CLI with --debug or set `detekt { debug = true }` in Gradle to see the error messages."
)
}
}
}

private object DetektMessageRenderer : PlainTextMessageRenderer() {
Expand Down
Expand Up @@ -63,7 +63,7 @@ internal class DefaultLifecycle(
override val settings: ProcessingSettings,
override val parsingStrategy: ParsingStrategy,
override val bindingProvider: (files: List<KtFile>) -> BindingContext =
{ generateBindingContext(settings.environment, settings.classpath, it, settings::debug, settings::info) },
{ generateBindingContext(settings.environment, settings.classpath, it, settings::debug) },
override val processorsProvider: () -> List<FileProcessListener> =
{ FileProcessorLocator(settings).load() },
override val ruleSetsProvider: () -> List<RuleSetProvider> =
Expand Down
Expand Up @@ -14,17 +14,14 @@ import org.junit.jupiter.api.Test
class DetektMessageCollectorSpec {

private lateinit var debugPrinter: (() -> String) -> Unit
private lateinit var warningPrinter: ((String) -> Unit)
private lateinit var subject: DetektMessageCollector

@BeforeEach
fun setupMocksAndSubject() {
debugPrinter = mockk { every { this@mockk.invoke(any()) } returns Unit }
warningPrinter = mockk { every { this@mockk.invoke(any()) } returns Unit }
subject = DetektMessageCollector(
minSeverity = CompilerMessageSeverity.INFO,
debugPrinter = debugPrinter,
warningPrinter = warningPrinter,
)
}

Expand All @@ -41,18 +38,6 @@ class DetektMessageCollectorSpec {
verify { debugPrinter.invoke(capture(slot)) }
assertThat(slot.captured()).isEqualTo("info: message")
}

@Test
fun `adds up to the message count`() {
subject.printIssuesCountIfAny()

verify {
warningPrinter(
"The BindingContext was created with 1 issues. " +
"Run detekt CLI with --debug or set `detekt { debug = true }` in Gradle to see the error messages."
)
}
}
}

@Nested
Expand All @@ -68,18 +53,6 @@ class DetektMessageCollectorSpec {
verify { debugPrinter.invoke(capture(slot)) }
assertThat(slot.captured()).isEqualTo("warning: message")
}

@Test
fun `adds up to the message count`() {
subject.printIssuesCountIfAny()

verify {
warningPrinter(
"The BindingContext was created with 1 issues. " +
"Run detekt CLI with --debug or set `detekt { debug = true }` in Gradle to see the error messages."
)
}
}
}

@Nested
Expand All @@ -93,12 +66,5 @@ class DetektMessageCollectorSpec {
fun `ignores the message`() {
verify { debugPrinter wasNot Called }
}

@Test
fun `doesn't add up to the message count`() {
subject.printIssuesCountIfAny()

verify { warningPrinter wasNot Called }
}
}
}