Skip to content

Commit

Permalink
Sort results in compiler plugin by column:line (detekt#7637)
Browse files Browse the repository at this point in the history
* Sort results in compiler plugin by column:line

* Add comparator to Location & SourceLocation class
  • Loading branch information
neugartf authored Oct 27, 2024
1 parent 5245fd3 commit 4328656
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 21 deletions.
11 changes: 9 additions & 2 deletions detekt-api/api/detekt-api.api
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,18 @@ public abstract interface class io/gitlab/arturbosch/detekt/api/Issue$Entity {
public abstract fun getSignature ()Ljava/lang/String;
}

public abstract interface class io/gitlab/arturbosch/detekt/api/Issue$Location {
public abstract interface class io/gitlab/arturbosch/detekt/api/Issue$Location : java/lang/Comparable {
public abstract fun compareTo (Lio/gitlab/arturbosch/detekt/api/Issue$Location;)I
public abstract fun getEndSource ()Lio/gitlab/arturbosch/detekt/api/SourceLocation;
public abstract fun getPath ()Ljava/nio/file/Path;
public abstract fun getSource ()Lio/gitlab/arturbosch/detekt/api/SourceLocation;
public abstract fun getText ()Lio/gitlab/arturbosch/detekt/api/TextLocation;
}

public final class io/gitlab/arturbosch/detekt/api/Issue$Location$DefaultImpls {
public static fun compareTo (Lio/gitlab/arturbosch/detekt/api/Issue$Location;Lio/gitlab/arturbosch/detekt/api/Issue$Location;)I
}

public final class io/gitlab/arturbosch/detekt/api/IssueKt {
public static final fun getSuppressed (Lio/gitlab/arturbosch/detekt/api/Issue;)Z
}
Expand Down Expand Up @@ -340,8 +345,10 @@ public final class io/gitlab/arturbosch/detekt/api/Severity : java/lang/Enum {
public static fun values ()[Lio/gitlab/arturbosch/detekt/api/Severity;
}

public final class io/gitlab/arturbosch/detekt/api/SourceLocation {
public final class io/gitlab/arturbosch/detekt/api/SourceLocation : java/lang/Comparable {
public fun <init> (II)V
public fun compareTo (Lio/gitlab/arturbosch/detekt/api/SourceLocation;)I
public synthetic fun compareTo (Ljava/lang/Object;)I
public fun equals (Ljava/lang/Object;)Z
public final fun getColumn ()I
public final fun getLine ()I
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ interface Issue {
val location: Location
}

interface Location {
interface Location : Comparable<Location> {
val source: SourceLocation
val endSource: SourceLocation
val text: TextLocation
val path: Path

override fun compareTo(other: Location): Int = compareValuesBy(this, other, { it.path }, { it.source })
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,15 @@ class Location(
* Stores line and column information of a location.
*/
@Poko
class SourceLocation(val line: Int, val column: Int) {
class SourceLocation(val line: Int, val column: Int) : Comparable<SourceLocation> {
init {
require(line > 0) { "The source location line must be greater than 0" }
require(column > 0) { "The source location column must be greater than 0" }
}

override fun toString(): String = "$line:$column"

override fun compareTo(other: SourceLocation): Int = compareValuesBy(this, other, { it.line }, { it.column })
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ fun MessageCollector.error(msg: String) {
fun MessageCollector.reportIssues(result: Detektion) {
result.issues
.filterNot { it.suppressed }
.sortedBy { it.location }
.forEach { issue ->
val (message, location) = issue.renderAsCompilerWarningMessage()
warn(message, location)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,46 @@ class CompilerTest {
fun `with a source file that contains violations`() {
val result = compile(
"""
class KClass {
class KClass0 {
fun foo() {
val x = 3
var x = 3
println(x)
var y = 4
println(y)
}
}
""".trimIndent(),
"""
class KClass1 {
fun foo() {
val x = "test"
}
}
""".trimIndent()
)

assertThat(result)
.passCompilation(true)
.passDetekt(false)
.withViolations(2)
.withRuleViolation("MagicNumber", "NewLineAtEndOfFile")
.withViolations(7)
.withRuleViolationInOrder(
listOf(
"VarCouldBeVal",
"MagicNumber",
"VarCouldBeVal",
"MagicNumber",
"NewLineAtEndOfFile",
"UnusedVariable",
"NewLineAtEndOfFile"
)
)
}

@Test
fun `with a source file that contains local suppression`() {
val result = compile(
"""
class KClass {
class KClass0 {
fun foo() {
@Suppress("MagicNumber")
val x = 3
Expand All @@ -52,7 +71,7 @@ class CompilerTest {
fun `with a source file that does not contain violations`() {
val result = compile(
"""
class KClass {
class KClass0 {
fun foo() {
println("Hello world :)")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,10 @@ fun assertThat(result: JvmCompilationResult) = CompilationAssert(result)
class CompilationAssert(private val result: JvmCompilationResult) :
AbstractObjectAssert<CompilationAssert, JvmCompilationResult>(result, CompilationAssert::class.java) {

private val detektMessages = result.messages.split("\n")
.dropWhile { "Running detekt" !in it }
.dropLastWhile { "Success?" !in it }
private val detektMessages = result.messages.split("\n").dropWhile { !(it.contains(Regex("KClass\\d.kt"))) }

private val regex = "\\w+\\.kt:\\d+:\\d+: .* \\[(\\w+)\\]".toRegex()
private val detektViolations = detektMessages
.mapNotNull { line -> regex.find(line)?.groupValues?.get(1) }
private val regex = "\\w+\\.kt:\\d+:\\d+ (\\w+): .*".toRegex()
private val detektViolations = detektMessages.mapNotNull { line -> regex.find(line)?.groupValues?.get(1) }

fun passCompilation(expectedStatus: Boolean = true) = apply {
val expectedErrorCode = if (expectedStatus) OK else COMPILATION_ERROR
Expand Down Expand Up @@ -62,10 +59,10 @@ class CompilationAssert(private val result: JvmCompilationResult) :
}
}

fun withRuleViolation(vararg expectedRuleName: String) = apply {
if (expectedRuleName.any { it !in detektViolations }) {
fun withRuleViolationInOrder(expectedRuleNames: List<String>) {
if (detektViolations != expectedRuleNames) {
failWithMessage(
"Expected rules ${expectedRuleName.toList()} to raise a violation but not all were found. " +
"Expected rules $expectedRuleNames to raise a violation but not all were found. " +
"Found violations are instead $detektViolations.\n" +
actual.messages,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ object CompilerTestUtils {
@Language("kotlin") vararg kotlinSources: String,
useDefaultConfig: Boolean = true,
): JvmCompilationResult {
val sourceFiles = kotlinSources.map {
SourceFile.kotlin("KClass.kt", it, trimIndent = true)
val sourceFiles = kotlinSources.mapIndexed { index, sources ->
SourceFile.kotlin("KClass$index.kt", sources, trimIndent = true)
}
return KotlinCompilation().apply {
messageOutputStream = object : OutputStream() {
Expand Down

0 comments on commit 4328656

Please sign in to comment.