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

Support logging and exception throwing when loading baseline #2362

Merged
merged 3 commits into from
Nov 19, 2023
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 gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ slf4j = "org.slf4j:slf4j-simple:2.0.9"
poko = "dev.drewhamilton.poko:poko-gradle-plugin:0.15.0"
# 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.3.5"
logcaptor = "io.github.hakky54:logcaptor:2.9.0"
# Required for logback.xml conditional configuration
janino = "org.codehaus.janino:janino:3.1.10"
# Testing libraries
Expand Down
14 changes: 14 additions & 0 deletions ktlint-cli-reporter-baseline/api/ktlint-cli-reporter-baseline.api
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,24 @@ public final class com/pinterest/ktlint/cli/reporter/baseline/Baseline$Status :
public static fun values ()[Lcom/pinterest/ktlint/cli/reporter/baseline/Baseline$Status;
}

public final class com/pinterest/ktlint/cli/reporter/baseline/BaselineErrorHandling : java/lang/Enum {
public static final field EXCEPTION Lcom/pinterest/ktlint/cli/reporter/baseline/BaselineErrorHandling;
public static final field LOG Lcom/pinterest/ktlint/cli/reporter/baseline/BaselineErrorHandling;
public static fun getEntries ()Lkotlin/enums/EnumEntries;
public static fun valueOf (Ljava/lang/String;)Lcom/pinterest/ktlint/cli/reporter/baseline/BaselineErrorHandling;
public static fun values ()[Lcom/pinterest/ktlint/cli/reporter/baseline/BaselineErrorHandling;
}

public final class com/pinterest/ktlint/cli/reporter/baseline/BaselineKt {
public static final fun containsLintError (Ljava/util/List;Lcom/pinterest/ktlint/cli/reporter/core/api/KtlintCliError;)Z
public static final fun doesNotContain (Ljava/util/List;Lcom/pinterest/ktlint/cli/reporter/core/api/KtlintCliError;)Z
public static final fun loadBaseline (Ljava/lang/String;)Lcom/pinterest/ktlint/cli/reporter/baseline/Baseline;
public static final fun loadBaseline (Ljava/lang/String;Lcom/pinterest/ktlint/cli/reporter/baseline/BaselineErrorHandling;)Lcom/pinterest/ktlint/cli/reporter/baseline/Baseline;
public static synthetic fun loadBaseline$default (Ljava/lang/String;Lcom/pinterest/ktlint/cli/reporter/baseline/BaselineErrorHandling;ILjava/lang/Object;)Lcom/pinterest/ktlint/cli/reporter/baseline/Baseline;
}

public final class com/pinterest/ktlint/cli/reporter/baseline/BaselineLoaderException : java/lang/RuntimeException {
public fun <init> (Ljava/lang/String;Ljava/lang/Throwable;)V
}

public final class com/pinterest/ktlint/cli/reporter/baseline/BaselineReporter : com/pinterest/ktlint/cli/reporter/core/api/ReporterV2 {
Expand Down
2 changes: 2 additions & 0 deletions ktlint-cli-reporter-baseline/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@ dependencies {
implementation(projects.ktlintCliReporterCore)

testImplementation(projects.ktlintTest)
testImplementation(libs.logback)
testImplementation(libs.logcaptor)
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,23 +58,75 @@ public class Baseline(
}
}

public enum class BaselineErrorHandling {
/**
* Log an error message. Does not throw an exception.
*/
LOG,

/**
* Throws an exception on error. Does not log the error.
*/
EXCEPTION,
}

/**
* Loads the [Baseline] from the file located on [path].
* Loads the [Baseline] from the file located on [path]. Exceptions are swallowed and log message is written. On error, the baseline file is
* deleted.
*/
public fun loadBaseline(path: String): Baseline = BaselineLoader(path).load()
@Deprecated(
message = "Marked for removal in Ktlint 2.0",
replaceWith = ReplaceWith("loadBaseline(path, BaselineErrorHandling.LOG)"),
)
public fun loadBaseline(path: String): Baseline = loadBaseline(path, BaselineErrorHandling.LOG)

/**
* Loads the [Baseline] from the file located on [path]. In case the baseline file can not be loaded successfully, it will be deleted.
*/
public fun loadBaseline(
path: String,
errorHandling: BaselineErrorHandling = BaselineErrorHandling.EXCEPTION,
): Baseline =
with(BaselineLoader(path)) {
try {
load()
} catch (e: Exception) {
// Delete baseline as it contains an error
try {
delete()
} catch (e: Exception) {
if (errorHandling == BaselineErrorHandling.LOG) {
LOGGER.error { e.message }
} else {
// Swallow as original exception from loading is to be returned only
}
}

// Handle original exception
if (errorHandling == BaselineErrorHandling.EXCEPTION) {
throw e
} else {
LOGGER.error { e.message }
Baseline(path = path, status = INVALID)
}
}
}

private class BaselineLoader(
private val path: String,
) {
private val baselinePath =
Paths
.get(path)
.toFile()
.takeIf { it.exists() }

var ruleReferenceWithoutRuleSetIdPrefix = 0

fun load(): Baseline {
require(path.isNotBlank()) { "Path for loading baseline may not be blank or empty" }

Paths
.get(path)
.toFile()
.takeIf { it.exists() }
baselinePath
?.let { baselineFile ->
try {
return Baseline(
Expand All @@ -91,20 +143,12 @@ private class BaselineLoader(
}
}
} catch (e: IOException) {
LOGGER.error { "Unable to parse baseline file: $path" }
throw BaselineLoaderException("Unable to parse baseline file: $path", e)
} catch (e: ParserConfigurationException) {
LOGGER.error { "Unable to parse baseline file: $path" }
throw BaselineLoaderException("Unable to parse baseline file: $path", e)
} catch (e: SAXException) {
LOGGER.error { "Unable to parse baseline file: $path" }
throw BaselineLoaderException("Unable to parse baseline file: $path", e)
}

// Baseline can not be parsed.
try {
baselineFile.delete()
} catch (e: IOException) {
LOGGER.error { "Unable to delete baseline file: $path" }
}
return Baseline(path = path, status = INVALID)
}

return Baseline(path = path, status = NOT_FOUND)
Expand Down Expand Up @@ -172,8 +216,21 @@ private class BaselineLoader(
detail = "",
status = BASELINE_IGNORED,
)

fun delete() {
try {
baselinePath?.delete()
} catch (e: IOException) {
throw BaselineLoaderException("Unable to delete baseline file: $path", e)
}
}
}

public class BaselineLoaderException(
message: String,
throwable: Throwable,
) : RuntimeException(message, throwable)

/**
* Checks if the list contains the given [KtlintCliError]. The [List.contains] function can not be used as [KtlintCliError.detail] is not
* available in the baseline file and a normal equality check on the [KtlintCliError] fails.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
package com.pinterest.ktlint.cli.reporter.baseline

import ch.qos.logback.classic.Level
import ch.qos.logback.classic.Logger
import com.pinterest.ktlint.cli.reporter.core.api.KtlintCliError
import com.pinterest.ktlint.cli.reporter.core.api.KtlintCliError.Status.BASELINE_IGNORED
import com.pinterest.ktlint.logger.api.initKtLintKLogger
import com.pinterest.ktlint.logger.api.setDefaultLoggerModifier
import io.github.oshai.kotlinlogging.DelegatingKLogger
import io.github.oshai.kotlinlogging.KLogger
import io.github.oshai.kotlinlogging.KotlinLogging
import nl.altindag.log.LogCaptor
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatExceptionOfType
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.io.TempDir
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.EnumSource
import org.xml.sax.SAXParseException
import java.nio.file.Path
import java.nio.file.Paths
import kotlin.io.path.Path
import kotlin.io.path.copyTo
import kotlin.io.path.pathString

@Suppress("unused")
private val LOGGER =
KotlinLogging
.logger {}
.setDefaultLoggerModifier { logger -> logger.level = Level.DEBUG }
.initKtLintKLogger()

private var KLogger.level: Level?
get() = underlyingLogger()?.level
set(value) {
underlyingLogger()?.level = value
}

private fun KLogger.underlyingLogger(): Logger? =
@Suppress("UNCHECKED_CAST")
(this as? DelegatingKLogger<Logger>)
?.underlyingLogger

class BaselineTest {
@Nested
inner class `Given a valid baseline file` {
@Test
fun `Given that the baseline is loaded with classic loader`(
@TempDir
tempDir: Path,
) {
val path = "baseline-valid.xml".copyResourceToFileIn(tempDir)

val actual = loadBaseline(path)

assertThat(actual)
.usingRecursiveComparison()
.isEqualTo(
Baseline(
path = path,
status = Baseline.Status.VALID,
lintErrorsPerFile =
mapOf(
"src/main/kotlin/Foo.kt" to
listOf(
KtlintCliError(1, 1, "standard:max-line-length", "", BASELINE_IGNORED),
KtlintCliError(2, 1, "standard:max-line-length", "", BASELINE_IGNORED),
KtlintCliError(4, 9, "standard:property-naming", "", BASELINE_IGNORED),
),
),
),
)
}

@ParameterizedTest(name = "BaselineErrorHandling: {0}")
@EnumSource(value = BaselineErrorHandling::class)
fun `Given that the baseline is loaded with the new loader with LOG error handling`(
baselineErrorHandling: BaselineErrorHandling,
@TempDir
tempDir: Path,
) {
val path = "baseline-valid.xml".copyResourceToFileIn(tempDir)

val actual = loadBaseline(path, baselineErrorHandling)

assertThat(actual)
.usingRecursiveComparison()
.isEqualTo(
Baseline(
path = path,
status = Baseline.Status.VALID,
lintErrorsPerFile =
mapOf(
"src/main/kotlin/Foo.kt" to
listOf(
KtlintCliError(1, 1, "standard:max-line-length", "", BASELINE_IGNORED),
KtlintCliError(2, 1, "standard:max-line-length", "", BASELINE_IGNORED),
KtlintCliError(4, 9, "standard:property-naming", "", BASELINE_IGNORED),
),
),
),
)
}
}

@Nested
inner class `Baseline has invalid xml structure` {
@Test
fun `Given that the baseline is loaded with classic loader then the log contains an error message`(
@TempDir
tempDir: Path,
) {
val path = "baseline-invalid.xml".copyResourceToFileIn(tempDir)

val logCaptor = LogCaptor.forClass(Baseline::class.java)

loadBaseline(path)

assertThat(logCaptor.errorLogs).contains("Unable to parse baseline file: $path")
}

@Test
fun `Given that the baseline is loaded with new loader then an exception is thrown on error`(
@TempDir
tempDir: Path,
) {
val path = "baseline-invalid.xml".copyResourceToFileIn(tempDir)

assertThatExceptionOfType(BaselineLoaderException::class.java)
.isThrownBy { loadBaseline(path, BaselineErrorHandling.EXCEPTION) }
.withMessage("Unable to parse baseline file: $path")
.withCauseInstanceOf(SAXParseException::class.java)
.havingCause()
.withMessage("The element type \"file\" must be terminated by the matching end-tag \"</file>\".")
}

@Test
fun `Given that the baseline is loaded with new loader then the log message is printed and no exception is thrown`(
@TempDir
tempDir: Path,
) {
val path = "baseline-invalid.xml".copyResourceToFileIn(tempDir)

val logCaptor = LogCaptor.forClass(Baseline::class.java)

loadBaseline(path)

assertThat(logCaptor.errorLogs).contains("Unable to parse baseline file: $path")
}
}

/**
* The resource needs to be copied in each test, because the file will be deleted in case it contains an error.
*/
private fun String.copyResourceToFileIn(tempDir: Path) =
Paths
.get("$TEST_RESOURCE_PATH/$this")
.copyTo(tempDir, overwrite = true)
.pathString

private companion object {
val TEST_RESOURCE_PATH: Path = Path("src", "test", "resources")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<baseline version="1.0">
<file name="src/main/kotlin/Foo2.kt">
<error line="1" column="1" source="standard:max-line-length" />
<error line="2" column="1" source="standard:max-line-length" />
<error line="4" column="9" source="standard:property-naming" />
</baseline>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="utf-8"?>
<baseline version="1.0">
<file name="src/main/kotlin/Foo.kt">
<error line="1" column="1" source="standard:max-line-length" />
<error line="2" column="1" source="standard:max-line-length" />
<error line="4" column="9" source="standard:property-naming" />
</file>
</baseline>
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# SLF4J's SimpleLogger configuration file

# This file contains the configuration for the logging framework which is added as dependency of the module (see build.gradle.kts). When you
# use another framework than "slf4j-simple", the configuration below will not work. Check out the documentation of the chosen framework how
# it should be configured. Most likely, it has comparable capabilities.

# Default logging detail level for all instances of SimpleLogger.
# Must be one of ("trace", "debug", "info", "warn", or "error").
# If not specified, defaults to "info".
#org.slf4j.simpleLogger.defaultLogLevel=warn

# Logging detail level which can either be set for a specific class or all classes in a specific package. If your project already contains
# a logger, you might want to set the logging for ktlint classes only.
#
# Must be one of ("trace", "debug", "info", "warn", or "error").
# If not specified, the default logging detail level is used. Specifying another loglevel (for example the value "off") results in
# suppressing all logging for the class or package. Although this can be abused to suppress all logging from KtLint, you are advised no to
# do this. It is better to set the loglevel to "debug" to reduce all logging except the error messages that you don't want to miss.
org.slf4j.simpleLogger.log.com.pinterest.ktlint=debug
# Additional logging detail levels can be set. Although rule above suppresses all log messages at levels warn, info, debug and trace, the
# line below adds an exception for a specific class which might be relevant for you.
org.slf4j.simpleLogger.log.com.pinterest.ktlint.rule.engine.internal.EditorConfigDefaultsLoader=warn

# Set to true if you want the current date and time to be included in output messages.
# Default is false, and will output the number of milliseconds elapsed since startup.
#org.slf4j.simpleLogger.showDateTime=false

# The date and time format to be used in the output messages.
# The pattern describing the date and time format is the same that is used in java.text.SimpleDateFormat.
# If the format is not specified or is invalid, the default format is used.
# The default format is yyyy-MM-dd HH:mm:ss:SSS Z.
#org.slf4j.simpleLogger.dateTimeFormat=yyyy-MM-dd HH:mm:ss:SSS Z

# Set to true if you want to output the current thread name.
# Defaults to true.
#org.slf4j.simpleLogger.showThreadName=true

# Set to true if you want the Logger instance name to be included in output messages.
# Defaults to true.
#org.slf4j.simpleLogger.showLogName=true
Loading