logger initialization improvement (#1430)

Hide variable defaultLoggerModifier and enforce that it can be set via function setDefaultLoggerModifier only. In this way, a warning can be printed if it is set more than once which could lead to unpredictable behavior.
pinterest · Mar 21, 2022 · 5fd23c0 · 5fd23c0
1 parent 83fb270
commit 5fd23c0
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 25 deletions.
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
@@ -12,26 +12,41 @@ public const val KTLINT_UNIT_TEST_DUMP_AST = "KTLINT_UNIT_TEST_DUMP_AST"
 public const val KTLINT_UNIT_TEST_ON_PROPERTY = "ON"
 
 /**
- * Default modifier for the KLogger of new instances of classes calling [initKtLintKLogger]. Classes for which
- * [initKtLintKLogger] has been called before setting this variable will not be changed. Also note, that this modifier
- * can only be set once.
+ * Default modifier for the KLogger. It can be set only once via [setDefaultLoggerModifier] but it should be set before
+ * the first invocation of [initKtLintKLogger].
  */
-public lateinit var defaultLoggerModifier: (KLogger) -> Unit
+private var defaultLoggerModifier: ((KLogger) -> Unit)? = null
 
 /**
- * Initializes the logger with the [defaultLoggerModifier] when set.
+ * Set the [defaultLoggerModifier]. Note that it can only be set once. It should be set before the first invocation to
+ * [initKtLintKLogger].
  */
-public fun KLogger.initKtLintKLogger(): KLogger {
-    return if (::defaultLoggerModifier.isInitialized) {
-        apply { defaultLoggerModifier(this) }
-    } else {
-        this
+public fun KLogger.setDefaultLoggerModifier(
+    loggerModifier: (KLogger) -> Unit
+): KLogger {
+    if (defaultLoggerModifier != null) {
+        warn {
+            """
+            The defaultLoggerModifier has been set before and might already have been applied when initializing
+            Loggers. Loggers which will be initialized after resetting the defaultLoggerModifier will be initialized
+            with the new value. This might result in unpredictable behavior. Except for in unit tests, it is
+            recommended to ensure to call this function only once.
+            """.trimIndent()
+        }
     }
+    defaultLoggerModifier = loggerModifier
+    return this
 }
 
 /**
- * Initializes the logger with the [loggerModifier].
+ * Initializes the logger with the [defaultLoggerModifier].
  */
-public fun KLogger.initKtLintKLogger(
-    loggerModifier: (KLogger) -> Unit
-): KLogger = apply { loggerModifier(this) }
+public fun KLogger.initKtLintKLogger(): KLogger {
+    if (defaultLoggerModifier == null) {
+        // Initialize the defaultLoggerModifier on the first invocation of initKtlintLogger when it is not yet set.
+        // In this way it can be ensured that all loggers are initialized with the exact same logger modifier.
+        defaultLoggerModifier = { _ -> }
+    }
+
+    return apply { defaultLoggerModifier?.invoke(this) }
+}
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
@@ -6,6 +6,7 @@ import com.pinterest.ktlint.core.Rule
 import com.pinterest.ktlint.core.RuleSet
 import com.pinterest.ktlint.core.api.FeatureInAlphaState
 import com.pinterest.ktlint.core.initKtLintKLogger
+import com.pinterest.ktlint.core.setDefaultLoggerModifier
 import com.pinterest.ruleset.test.DumpASTRule
 import mu.KotlinLogging
 import org.assertj.core.api.Assertions.assertThat
@@ -16,7 +17,7 @@ import org.jetbrains.kotlin.utils.addToStdlib.ifTrue
 private val logger =
     KotlinLogging
         .logger {}
-        .initKtLintKLogger { logger ->
+        .setDefaultLoggerModifier { logger ->
             if (!logger.isTraceEnabled || !logger.isDebugEnabled) {
                 logger.info {
                     """
@@ -26,7 +27,7 @@ private val logger =
                     """.trimIndent()
                 }
             }
-        }
+        }.initKtLintKLogger()
 
 // 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

diff --git a/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt b/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt
@@ -13,11 +13,11 @@ import com.pinterest.ktlint.core.RuleExecutionException
 import com.pinterest.ktlint.core.RuleSet
 import com.pinterest.ktlint.core.RuleSetProvider
 import com.pinterest.ktlint.core.VisitorProvider
-import com.pinterest.ktlint.core.defaultLoggerModifier
 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.setDefaultLoggerModifier
 import com.pinterest.ktlint.internal.ApplyToIDEAGloballySubCommand
 import com.pinterest.ktlint.internal.ApplyToIDEAProjectSubCommand
 import com.pinterest.ktlint.internal.GenerateEditorConfigSubCommand
@@ -255,14 +255,7 @@ class KtlintCommandLine {
         if (verbose) {
             debug = true
         }
-        defaultLoggerModifier = { logger ->
-            (logger.underlyingLogger as Logger).level = when {
-                trace -> Level.TRACE
-                debug -> Level.DEBUG
-                else -> Level.INFO
-            }
-        }
-        logger = KotlinLogging.logger {}.initKtLintKLogger()
+        logger = configureLogger()
 
         failOnOldRulesetProviderUsage()
 
@@ -302,6 +295,18 @@ class KtlintCommandLine {
         }
     }
 
+    private fun configureLogger() =
+        KotlinLogging
+            .logger {}
+            .setDefaultLoggerModifier { logger ->
+                (logger.underlyingLogger as Logger).level = when {
+                    trace -> Level.TRACE
+                    debug -> Level.DEBUG
+                    else -> Level.INFO
+                }
+            }
+            .initKtLintKLogger()
+
     private fun lintFiles(
         ruleSetProviders: Map<String, RuleSetProvider>,
         visitorProvider: VisitorProvider,