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

Remove logback specific configuration from ktlint-core package #1424

Merged
merged 4 commits into from
Mar 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
### Added

### Fixed
- Remove logback dependency from ktlint-core module ([#1421](https://github.com/pinterest/ktlint/issues/1421))

### Changed

Expand Down
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ ext.deps = [
'logging' : 'io.github.microutils:kotlin-logging-jvm:2.1.21',
// Use logback-classic as the logger for kotlin-logging / slf4j as it allow changing the log level at runtime.
'logback' : 'ch.qos.logback:logback-classic:1.2.9',
// Required for logback.xml conditional configuration
'janino' : 'org.codehaus.janino:janino:3.1.4',
// Testing libraries
'junit5' : 'org.junit.jupiter:junit-jupiter:5.8.2',
'assertj' : 'org.assertj:assertj-core:3.12.2',
Expand Down
3 changes: 3 additions & 0 deletions gradle/verification-metadata.xml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@
<trusting group="org.opentest4j"/>
<trusting group="^org[.]junit($|([.].*))" regex="true"/>
</trusted-key>
<trusted-key id="37d75e23e6b6a6c573c9ae6fdfbf27530b637783">
<trusting group="org.codehaus.janino" />
</trusted-key>
</trusted-keys>
</configuration>
<components>
Expand Down
2 changes: 1 addition & 1 deletion ktlint-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ dependencies {
api deps.kotlin.compiler
api deps.ec4j
api deps.logging
api deps.logback

// Standard ruleset is required for EditConfigLoaderTest only
testImplementation projects.ktlintRulesetStandard
testImplementation projects.ktlintTestLogging
testImplementation deps.junit5
testImplementation deps.assertj
testImplementation deps.jimfs
Expand Down
Original file line number Diff line number Diff line change
@@ -1,58 +1,19 @@
package com.pinterest.ktlint.core

import ch.qos.logback.classic.Level
import ch.qos.logback.classic.Logger
import mu.KLogger
import org.jetbrains.kotlin.utils.addToStdlib.ifTrue

public enum class LogLevel { TRACE, DEBUG, INFO }

public var logLevel: LogLevel = LogLevel.INFO

@Deprecated("Environment variable is replace by new variables below")
@Deprecated(message = "No longer in use for the public API. Constant is marked for removal in Ktlint 0.46.")
private const val KTLINT_DEBUG = "KTLINT_DEBUG"

// Via command line parameters "--trace" and "--print-ast" the end user of ktlint can change the logging behavior. As
// unit tests are not invoked via the main ktlint runtime, those command line parameters can not be used to change the
// logging behavior while running the unit tests. Instead, the environment variables below can be used by ktlint
// developers to change the logging behavior. Note, when set the environment variables also affect the runtinme of
// ktlint. As of that the name of the variables start with KTLINT_UNIT_TEST to clarify the intent.
@Deprecated(message = "No longer in use for the public API. Constant is marked for removal in Ktlint 0.46.")
public const val KTLINT_UNIT_TEST_DUMP_AST = "KTLINT_UNIT_TEST_DUMP_AST"
public const val KTLINT_UNIT_TEST_TRACE = "KTLINT_UNIT_TEST_TRACE"
public const val KTLINT_UNIT_TEST_ON_PROPERTY = "ON"

public fun KLogger.initKtLintKLogger(): KLogger =
also { logger ->
System
.getenv(KTLINT_UNIT_TEST_TRACE)
.orEmpty()
.equals(KTLINT_UNIT_TEST_ON_PROPERTY, ignoreCase = true)
.ifTrue {
// The log level of the kotlin-logging framework can only be altered by modifying the underling logging
// library. Also note that the default SLF4J implementation does not allow the log level to be changes.
// Therefore, a fall back on the logback-core is required. See
// https://github.com/MicroUtils/kotlin-logging/issues/20
logger.trace { "Enable TRACE logging as System environment variable $KTLINT_UNIT_TEST_TRACE is set to 'on'" }
logLevel = LogLevel.TRACE
}
(logger.underlyingLogger as Logger).level = when (logLevel) {
LogLevel.INFO -> Level.INFO
LogLevel.DEBUG -> Level.DEBUG
LogLevel.TRACE -> Level.TRACE
}
@Deprecated(message = "No longer in use for the public API. Constant is marked for removal in Ktlint 0.46.")
public const val KTLINT_UNIT_TEST_ON_PROPERTY = "ON"

System
.getenv(KTLINT_DEBUG)
.orEmpty()
.takeIf { it.isNotEmpty() }
?.let {
logger.error {
"""
System environment variable $KTLINT_DEBUG is no longer used to change the logging behavior while running unit tests.
Now set one or more of environment variables below:
$KTLINT_UNIT_TEST_TRACE=[on|off]
$KTLINT_UNIT_TEST_DUMP_AST=[on|off]
""".trimIndent()
}
}
paul-dingemans marked this conversation as resolved.
Show resolved Hide resolved
}
/**
* Initializes the logger. Optionally the logger can be modified using the [loggerModifier].
*/
public fun KLogger.initKtLintKLogger(
loggerModifier: (KLogger) -> Unit = { _ -> }
): KLogger = apply { loggerModifier(this) }
9 changes: 9 additions & 0 deletions ktlint-test-logging/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
plugins {
id 'ktlint-kotlin-common'
id 'ktlint-publication'
}

dependencies {
api deps.logback
api deps.janino
}
4 changes: 4 additions & 0 deletions ktlint-test-logging/gradle.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
GROUP=com.pinterest.ktlint
POM_NAME=ktlint-test-logging
POM_ARTIFACT_ID=ktlint-test-logging
POM_PACKAGING=jar
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
<configuration>
<if condition='property("KTLINT_UNIT_TEST_TRACE").equalsIgnoreCase("on")'>
<then>
<property name="level" value="trace" />
</then>
<else>
<property name="level" value="debug" />
</else>
</if>
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<!-- encoders are assigned the type
ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
Expand All @@ -8,7 +16,7 @@
</encoder>
</appender>

<root level="debug">
<root level="${level}">
<appender-ref ref="STDOUT" />
</root>
</configuration>
1 change: 1 addition & 0 deletions ktlint-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ plugins {
dependencies {
api projects.ktlintCore
api projects.ktlintRulesetTest
api projects.ktlintTestLogging

implementation deps.junit5
implementation deps.assertj
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package com.pinterest.ktlint.test

import com.pinterest.ktlint.core.KTLINT_UNIT_TEST_DUMP_AST
import com.pinterest.ktlint.core.KTLINT_UNIT_TEST_ON_PROPERTY
import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.core.Rule
Expand All @@ -15,7 +13,34 @@ import org.assertj.core.util.diff.DiffUtils.diff
import org.assertj.core.util.diff.DiffUtils.generateUnifiedDiff
import org.jetbrains.kotlin.utils.addToStdlib.ifTrue

private val logger = KotlinLogging.logger {}.initKtLintKLogger()
private val logger =
KotlinLogging
.logger {}
.initKtLintKLogger { logger ->
if (!logger.isTraceEnabled || !logger.isDebugEnabled) {
logger.info {
"""
Additional information can be printed during running of unit tests, by setting one or more of environment variables below:
$KTLINT_UNIT_TEST_TRACE=[on|off]
$KTLINT_UNIT_TEST_DUMP_AST=[on|off]
""".trimIndent()
}
}
}

// Via command line parameter "--trace" the end user of ktlint can change the logging behavior. As unit tests are not
// invoked via the main ktlint runtime, this command line parameter can not be used to change the logging behavior while
// running the unit tests. Instead, the environment variable below can be set by ktlint developers to change the logging
// behavior.
// Keep value in sync with value in 'logback-test.xml' source in module 'ktlint-test-logging'
private const val KTLINT_UNIT_TEST_TRACE = "KTLINT_UNIT_TEST_TRACE"

// Via command line parameter "--print-ast" the end user of ktlint can change the logging behavior. As unit tests are
// not invoked via the main ktlint runtime, this command line parameter can not be used to change the logging behavior
// while running the unit tests. Instead, the environment variable below can be used by ktlint developers to change the
// logging behavior.
private const val KTLINT_UNIT_TEST_DUMP_AST = "KTLINT_UNIT_TEST_DUMP_AST"
private const val KTLINT_UNIT_TEST_ON_PROPERTY = "ON"

private fun List<Rule>.toRuleSets(): List<RuleSet> {
val dumpAstRuleSet = System
Expand Down
1 change: 1 addition & 0 deletions ktlint/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ dependencies {
implementation deps.kotlin.compiler
implementation deps.klob
implementation deps.picocli
implementation deps.logback


testImplementation deps.junit5
Expand Down
52 changes: 38 additions & 14 deletions ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

package com.pinterest.ktlint

import ch.qos.logback.classic.Level
import ch.qos.logback.classic.Logger
import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.core.LogLevel
import com.pinterest.ktlint.core.ParseException
import com.pinterest.ktlint.core.Reporter
import com.pinterest.ktlint.core.ReporterProvider
Expand All @@ -16,7 +17,6 @@ import com.pinterest.ktlint.core.initKtLintKLogger
import com.pinterest.ktlint.core.internal.containsLintError
import com.pinterest.ktlint.core.internal.loadBaseline
import com.pinterest.ktlint.core.internal.relativeRoute
import com.pinterest.ktlint.core.logLevel
import com.pinterest.ktlint.internal.ApplyToIDEAGloballySubCommand
import com.pinterest.ktlint.internal.ApplyToIDEAProjectSubCommand
import com.pinterest.ktlint.internal.GenerateEditorConfigSubCommand
Expand Down Expand Up @@ -254,12 +254,7 @@ class KtlintCommandLine {
if (verbose) {
debug = true
}
logLevel = when {
trace -> LogLevel.TRACE
debug -> LogLevel.DEBUG
else -> LogLevel.INFO
}
logger = KotlinLogging.logger {}.initKtLintKLogger()
logger = configureLogger()

failOnOldRulesetProviderUsage()

Expand Down Expand Up @@ -299,6 +294,17 @@ class KtlintCommandLine {
}
}

private fun configureLogger() =
KotlinLogging
.logger {}
.initKtLintKLogger { logger ->
(logger.underlyingLogger as Logger).level = when {
trace -> Level.TRACE
debug -> Level.DEBUG
else -> Level.INFO
}
}

private fun lintFiles(
ruleSetProviders: Map<String, RuleSetProvider>,
visitorProvider: VisitorProvider,
Expand Down Expand Up @@ -461,7 +467,11 @@ class KtlintCommandLine {
ReporterTemplate(
reporterId,
split.lastOrNull { it.startsWith("artifact=") }?.let { it.split("=")[1] },
mapOf("verbose" to verbose.toString(), "color" to color.toString(), "color_name" to colorName) + parseQuery(rawReporterConfig),
mapOf(
"verbose" to verbose.toString(),
"color" to color.toString(),
"color_name" to colorName
) + parseQuery(rawReporterConfig),
split.lastOrNull { it.startsWith("output=") }?.let { it.split("=")[1] }
)
}
Expand Down Expand Up @@ -583,11 +593,25 @@ class KtlintCommandLine {
numberOfThreads: Int = Runtime.getRuntime().availableProcessors()
) {
val pill = object : Future<T> {
override fun isDone(): Boolean { throw UnsupportedOperationException() }
override fun get(timeout: Long, unit: TimeUnit): T { throw UnsupportedOperationException() }
override fun get(): T { throw UnsupportedOperationException() }
override fun cancel(mayInterruptIfRunning: Boolean): Boolean { throw UnsupportedOperationException() }
override fun isCancelled(): Boolean { throw UnsupportedOperationException() }
override fun isDone(): Boolean {
throw UnsupportedOperationException()
}

override fun get(timeout: Long, unit: TimeUnit): T {
throw UnsupportedOperationException()
}

override fun get(): T {
throw UnsupportedOperationException()
}

override fun cancel(mayInterruptIfRunning: Boolean): Boolean {
throw UnsupportedOperationException()
}

override fun isCancelled(): Boolean {
throw UnsupportedOperationException()
}
}
val q = ArrayBlockingQueue<Future<T>>(numberOfThreads)
val producer = thread(start = true) {
Expand Down
1 change: 1 addition & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ include ':ktlint-ruleset-standard'
include ':ktlint-ruleset-template'
include ':ktlint-ruleset-test'
include ':ktlint-test'
include ':ktlint-test-logging'