From bfec80ee7bc35247422a588ef63b03449d63731b Mon Sep 17 00:00:00 2001 From: Aleksandr Pilipenko <2481047+z3d1k@users.noreply.github.com> Date: Sun, 20 Mar 2022 14:43:29 +0000 Subject: [PATCH] Remove logback specific configuration from ktlint-core package (#1424) Closes #1421 Co-authored-by: paul-dingemans --- CHANGELOG.md | 1 + build.gradle | 2 + gradle/verification-metadata.xml | 3 + ktlint-core/build.gradle | 2 +- .../ktlint/core/KtLintKLoggerInitializer.kt | 59 ++++--------------- ktlint-test-logging/build.gradle | 9 +++ ktlint-test-logging/gradle.properties | 4 ++ .../src/main/resources/logback-test.xml | 10 +++- ktlint-test/build.gradle | 1 + .../pinterest/ktlint/test/RuleExtension.kt | 31 +++++++++- ktlint/build.gradle | 1 + .../main/kotlin/com/pinterest/ktlint/Main.kt | 52 +++++++++++----- settings.gradle | 1 + 13 files changed, 108 insertions(+), 68 deletions(-) create mode 100644 ktlint-test-logging/build.gradle create mode 100644 ktlint-test-logging/gradle.properties rename {ktlint-test => ktlint-test-logging}/src/main/resources/logback-test.xml (62%) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2a4219cda..9fc4e9b143 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/build.gradle b/build.gradle index 71af4ee97a..330a13f452 100644 --- a/build.gradle +++ b/build.gradle @@ -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', diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index 94edd6be6e..bbd2eca4f7 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -137,6 +137,9 @@ + + + diff --git a/ktlint-core/build.gradle b/ktlint-core/build.gradle index 0adeac129a..23c5e3e010 100644 --- a/ktlint-core/build.gradle +++ b/ktlint-core/build.gradle @@ -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 diff --git a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLintKLoggerInitializer.kt b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLintKLoggerInitializer.kt index 3c0dfe596a..c2bd81382f 100644 --- a/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLintKLoggerInitializer.kt +++ b/ktlint-core/src/main/kotlin/com/pinterest/ktlint/core/KtLintKLoggerInitializer.kt @@ -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() - } - } - } +/** + * Initializes the logger. Optionally the logger can be modified using the [loggerModifier]. + */ +public fun KLogger.initKtLintKLogger( + loggerModifier: (KLogger) -> Unit = { _ -> } +): KLogger = apply { loggerModifier(this) } diff --git a/ktlint-test-logging/build.gradle b/ktlint-test-logging/build.gradle new file mode 100644 index 0000000000..ac1297a346 --- /dev/null +++ b/ktlint-test-logging/build.gradle @@ -0,0 +1,9 @@ +plugins { + id 'ktlint-kotlin-common' + id 'ktlint-publication' +} + +dependencies { + api deps.logback + api deps.janino +} diff --git a/ktlint-test-logging/gradle.properties b/ktlint-test-logging/gradle.properties new file mode 100644 index 0000000000..659751d642 --- /dev/null +++ b/ktlint-test-logging/gradle.properties @@ -0,0 +1,4 @@ +GROUP=com.pinterest.ktlint +POM_NAME=ktlint-test-logging +POM_ARTIFACT_ID=ktlint-test-logging +POM_PACKAGING=jar diff --git a/ktlint-test/src/main/resources/logback-test.xml b/ktlint-test-logging/src/main/resources/logback-test.xml similarity index 62% rename from ktlint-test/src/main/resources/logback-test.xml rename to ktlint-test-logging/src/main/resources/logback-test.xml index 39df24dbd0..1e63a15073 100644 --- a/ktlint-test/src/main/resources/logback-test.xml +++ b/ktlint-test-logging/src/main/resources/logback-test.xml @@ -1,4 +1,12 @@ + + + + + + + + @@ -8,7 +16,7 @@ - + diff --git a/ktlint-test/build.gradle b/ktlint-test/build.gradle index 42647d2b52..84f888f74e 100644 --- a/ktlint-test/build.gradle +++ b/ktlint-test/build.gradle @@ -6,6 +6,7 @@ plugins { dependencies { api projects.ktlintCore api projects.ktlintRulesetTest + api projects.ktlintTestLogging implementation deps.junit5 implementation deps.assertj diff --git a/ktlint-test/src/main/kotlin/com/pinterest/ktlint/test/RuleExtension.kt b/ktlint-test/src/main/kotlin/com/pinterest/ktlint/test/RuleExtension.kt index c0ffb142c0..069e71015f 100644 --- a/ktlint-test/src/main/kotlin/com/pinterest/ktlint/test/RuleExtension.kt +++ b/ktlint-test/src/main/kotlin/com/pinterest/ktlint/test/RuleExtension.kt @@ -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 @@ -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.toRuleSets(): List { val dumpAstRuleSet = System diff --git a/ktlint/build.gradle b/ktlint/build.gradle index 49c4faf283..cd9fe89812 100644 --- a/ktlint/build.gradle +++ b/ktlint/build.gradle @@ -33,6 +33,7 @@ dependencies { implementation deps.kotlin.compiler implementation deps.klob implementation deps.picocli + implementation deps.logback testImplementation deps.junit5 diff --git a/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt b/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt index be50680f77..e76cb034ef 100644 --- a/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt +++ b/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt @@ -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 @@ -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 @@ -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() @@ -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, visitorProvider: VisitorProvider, @@ -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] } ) } @@ -583,11 +593,25 @@ class KtlintCommandLine { numberOfThreads: Int = Runtime.getRuntime().availableProcessors() ) { val pill = object : Future { - 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>(numberOfThreads) val producer = thread(start = true) { diff --git a/settings.gradle b/settings.gradle index 282dd7c01b..ae67e01f94 100644 --- a/settings.gradle +++ b/settings.gradle @@ -28,3 +28,4 @@ include ':ktlint-ruleset-standard' include ':ktlint-ruleset-template' include ':ktlint-ruleset-test' include ':ktlint-test' +include ':ktlint-test-logging'