Remove logback specific configuration from ktlint-core package (#1424)

Closes #1421 Co-authored-by: paul-dingemans <paul-dingemans@users.noreply.github.com>
pinterest · Mar 20, 2022 · bfec80e · bfec80e
1 parent 9c7c4ca
commit bfec80e
Show file tree

Hide file tree

Showing 13 changed files with 108 additions and 68 deletions.
diff --git 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
@@ -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
@@ -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>

diff --git 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
@@ -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
@@ -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
@@ -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/...-test/src/main/resources/logback-test.xml → ...gging/src/main/resources/logback-test.xml b/...-test/src/main/resources/logback-test.xml → ...gging/src/main/resources/logback-test.xml
@@ -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 -->
@@ -8,7 +16,7 @@
         </encoder>
     </appender>
 
-    <root level="debug">
+    <root level="${level}">
         <appender-ref ref="STDOUT" />
     </root>
 </configuration>
diff --git 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
@@ -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<Rule>.toRuleSets(): List<RuleSet> {
     val dumpAstRuleSet = System

diff --git 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
@@ -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<String, RuleSetProvider>,
         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<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) {

diff --git 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'