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

Make detekt less noisy #5448

Merged
merged 2 commits 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 @@ -32,7 +32,7 @@ class ProcessingSettings(
Closeable,
LoggingAware by LoggingFacade(spec.loggingSpec),
PropertiesAware by PropertiesFacade(),
EnvironmentAware by EnvironmentFacade(spec.projectSpec, spec.compilerSpec),
EnvironmentAware by EnvironmentFacade(spec.projectSpec, spec.compilerSpec, spec.loggingSpec),
ClassloaderAware by ExtensionFacade(spec.extensionsSpec.plugins),
SetupContext {

Expand Down
Expand Up @@ -3,6 +3,7 @@ package io.gitlab.arturbosch.detekt.core.settings
import io.github.detekt.parser.createCompilerConfiguration
import io.github.detekt.parser.createKotlinCoreEnvironment
import io.github.detekt.tooling.api.spec.CompilerSpec
import io.github.detekt.tooling.api.spec.LoggingSpec
import io.github.detekt.tooling.api.spec.ProjectSpec
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.jetbrains.kotlin.com.intellij.openapi.Disposable
Expand All @@ -11,6 +12,8 @@ import org.jetbrains.kotlin.config.JvmTarget
import org.jetbrains.kotlin.config.LanguageVersion
import java.io.Closeable
import java.io.File
import java.io.OutputStream
import java.io.PrintStream

interface EnvironmentAware {

Expand All @@ -21,7 +24,8 @@ interface EnvironmentAware {

internal class EnvironmentFacade(
private val projectSpec: ProjectSpec,
private val compilerSpec: CompilerSpec
private val compilerSpec: CompilerSpec,
private val loggingSpec: LoggingSpec,
) : AutoCloseable, Closeable, EnvironmentAware {

override val disposable: Disposable = Disposer.newDisposable()
Expand All @@ -36,7 +40,11 @@ internal class EnvironmentFacade(
compilerSpec.parseJvmTarget(),
compilerSpec.jdkHome,
)
createKotlinCoreEnvironment(compilerConfiguration, disposable)
createKotlinCoreEnvironment(
compilerConfiguration,
disposable,
if (loggingSpec.debug) loggingSpec.errorChannel.asPrintStream() else NullPrintStream,
)
}

override fun close() {
Expand All @@ -61,3 +69,26 @@ internal fun CompilerSpec.parseJvmTarget(): JvmTarget {
" must be one of ${JvmTarget.values().map(JvmTarget::description)}"
}
}

private object NullPrintStream : PrintStream(
object : OutputStream() {
override fun write(b: Int) {
// no-op
}
}
)

private fun Appendable.asPrintStream(): PrintStream {
val appendable = this
return if (appendable is PrintStream) {
appendable
} else {
PrintStream(
object : OutputStream() {
override fun write(b: Int) {
appendable.append(b.toChar())
}
}
)
}
}
Expand Up @@ -23,22 +23,39 @@ import org.jetbrains.kotlin.config.LanguageVersion
import org.jetbrains.kotlin.config.LanguageVersionSettings
import org.jetbrains.kotlin.config.LanguageVersionSettingsImpl
import java.io.File
import java.io.PrintStream
import java.net.URLClassLoader
import java.nio.file.Path

/**
* Creates an environment instance which can be used to compile source code to KtFile's.
* This environment also allows to modify the resulting AST files.
*/
@Deprecated(
"You should pass a printStream",
ReplaceWith("createKotlinCoreEnvironment(configuration, disposable, System.err)")
)
Comment on lines +34 to +37
Copy link
Member

Choose a reason for hiding this comment

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

    printStream: PrintStream = System.err,

wouldn't work?

Copy link
Member Author

Choose a reason for hiding this comment

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

It would. But that would break binary compatibility.

And I'm not a big fan of the default values, too be honest.

Copy link
Member

Choose a reason for hiding this comment

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

agreed there, but when there's two other default values already :)

a @jvmoverloads would help with binary compat I think, and one less method, and one less deprecation warning to action for users.

fun createKotlinCoreEnvironment(
configuration: CompilerConfiguration = CompilerConfiguration(),
disposable: Disposable = Disposer.newDisposable()
): KotlinCoreEnvironment {
return createKotlinCoreEnvironment(configuration, disposable, System.err)
}

/**
* Creates an environment instance which can be used to compile source code to KtFile's.
* This environment also allows to modify the resulting AST files.
*/
fun createKotlinCoreEnvironment(
configuration: CompilerConfiguration = CompilerConfiguration(),
disposable: Disposable = Disposer.newDisposable(),
printStream: PrintStream,
): KotlinCoreEnvironment {
// https://github.com/JetBrains/kotlin/commit/2568804eaa2c8f6b10b735777218c81af62919c1
setIdeaIoUseFallback()
configuration.put(
CLIConfigurationKeys.MESSAGE_COLLECTOR_KEY,
PrintingMessageCollector(System.err, MessageRenderer.PLAIN_FULL_PATHS, false)
PrintingMessageCollector(printStream, MessageRenderer.PLAIN_FULL_PATHS, false)
)
configuration.put(CommonConfigurationKeys.MODULE_NAME, "detekt")

Expand Down
Expand Up @@ -11,7 +11,7 @@ import java.nio.file.Files
import java.nio.file.Path

open class KtCompiler(
protected val environment: KotlinCoreEnvironment = createKotlinCoreEnvironment()
protected val environment: KotlinCoreEnvironment = createKotlinCoreEnvironment(printStream = System.err)
) {

protected val psiFileFactory = KtPsiFactory(environment.project, markGenerated = false)
Expand Down