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.

Sign up for GitHub

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

Jump to bottom

Indroduce Rule.Id #7468

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Indroduce Rule.Id #7468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift click to select a range
c35eb3c
Introduce Rule.Id
BraisGabin Jul 19, 2024
5867fc2
Improve wording
BraisGabin Jul 19, 2024
620723a
Check Rule.Id
BraisGabin Jul 19, 2024
c317c4e
Improve tests
BraisGabin Jul 19, 2024
0f8b39f
Add missing tests
BraisGabin Jul 19, 2024
95aec44
Reduce number of internal files in api
BraisGabin Jul 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions detekt-api/api/detekt-api.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 184,10 @@ public final class io/gitlab/arturbosch/detekt/api/Issue$DefaultImpls {
public static fun getLocation (Lio/gitlab/arturbosch/detekt/api/Issue;)Lio/gitlab/arturbosch/detekt/api/Location;
}

public final class io/gitlab/arturbosch/detekt/api/IssueKt {
public static final fun getName (Lio/gitlab/arturbosch/detekt/api/RuleInstance;)Lio/gitlab/arturbosch/detekt/api/Rule$Name;
}

public final class io/gitlab/arturbosch/detekt/api/Location {
public static final field Companion Lio/gitlab/arturbosch/detekt/api/Location$Companion;
public fun <init> (Lio/gitlab/arturbosch/detekt/api/SourceLocation;Lio/gitlab/arturbosch/detekt/api/SourceLocation;Lio/gitlab/arturbosch/detekt/api/TextLocation;Ljava/nio/file/Path;)V
Expand Down Expand Up @@ -285,6 289,15 @@ public class io/gitlab/arturbosch/detekt/api/Rule : io/gitlab/arturbosch/detekt/
public static synthetic fun visitFile$default (Lio/gitlab/arturbosch/detekt/api/Rule;Lorg/jetbrains/kotlin/psi/KtFile;Lorg/jetbrains/kotlin/resolve/BindingContext;Lio/gitlab/arturbosch/detekt/api/CompilerResources;ILjava/lang/Object;)Ljava/util/List;
}

public final class io/gitlab/arturbosch/detekt/api/Rule$Id {
public fun <init> (Ljava/lang/String;)V
public fun equals (Ljava/lang/Object;)Z
public final fun getRuleName ()Lio/gitlab/arturbosch/detekt/api/Rule$Name;
public final fun getValue ()Ljava/lang/String;
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
}

public final class io/gitlab/arturbosch/detekt/api/Rule$Name {
public fun <init> (Ljava/lang/String;)V
public fun equals (Ljava/lang/Object;)Z
Expand All @@ -295,8 308,7 @@ public final class io/gitlab/arturbosch/detekt/api/Rule$Name {

public abstract interface class io/gitlab/arturbosch/detekt/api/RuleInstance {
public abstract fun getDescription ()Ljava/lang/String;
public abstract fun getId ()Ljava/lang/String;
public abstract fun getName ()Lio/gitlab/arturbosch/detekt/api/Rule$Name;
public abstract fun getId ()Lio/gitlab/arturbosch/detekt/api/Rule$Id;
public abstract fun getRuleSetId ()Lio/gitlab/arturbosch/detekt/api/RuleSet$Id;
}

Expand Down
5 changes: 3 additions & 2 deletions detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/Issue.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 19,9 @@ interface Issue {
}

interface RuleInstance {
val id: String
val name: Rule.Name
val id: Rule.Id
val ruleSetId: RuleSet.Id
val description: String
}

val RuleInstance.name: Rule.Name get() = id.ruleName
17 changes: 15 additions & 2 deletions detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/Rule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 1,6 @@
package io.gitlab.arturbosch.detekt.api

import dev.drewhamilton.poko.Poko
import io.gitlab.arturbosch.detekt.api.internal.validateIdentifier
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.resolve.BindingContext

Expand Down Expand Up @@ -91,9 90,23 @@ open class Rule(
@Poko
class Name(val value: String) {
init {
validateIdentifier(value)
require(value.matches(nameRegex)) { "Name '$value' must match ${nameRegex.pattern}}" }
}

override fun toString(): String = value
}

@Poko
class Id(val value: String) {
init {
require(value.matches(idRegex)) { "Id '$value' must match ${idRegex.pattern}" }
}

val ruleName: Name get() = Name(value.split("/", limit = 2).first())

override fun toString(): String = value
}
}

private val nameRegex = Regex("[aA-zZ] (?:[aA-zZ0-9-] )*[aA-zZ0-9]")
private val idRegex = Regex("${nameRegex.pattern}(/${nameRegex.pattern})?")
5 changes: 3 additions & 2 deletions detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/RuleSet.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 1,6 @@
package io.gitlab.arturbosch.detekt.api

import dev.drewhamilton.poko.Poko
import io.gitlab.arturbosch.detekt.api.internal.validateIdentifier

/**
* A rule set is a collection of rules and must be defined within a rule set provider implementation.
Expand All @@ -15,9 14,11 @@ class RuleSet(val id: Id, val rules: Map<Rule.Name, (Config) -> Rule>) {
@Poko
class Id(val value: String) {
init {
validateIdentifier(value)
require(value.matches(idRegex)) { "Id '$value' must match ${idRegex.pattern}" }
}

override fun toString(): String = value
}
}

private val idRegex = Regex("[aA-zZ] (?:[aA-zZ0-9-] )*[aA-zZ0-9]")
12 changes: 0 additions & 12 deletions detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/Validation.kt
View file
Open in desktop

This file was deleted.

74 changes: 32 additions & 42 deletions detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/RuleSetSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,53 1,43 @@
package io.gitlab.arturbosch.detekt.api

import org.assertj.core.api.Assertions.assertThatCode
import org.assertj.core.api.Assertions.assertThatExceptionOfType
import org.junit.jupiter.api.Nested
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.junit.jupiter.params.provider.ValueSource

class RuleSetSpec {
@ParameterizedTest(name = "should allow RuleSet with name {0}")
@MethodSource("getValidNames")
fun shouldAllowValidNames(ruleSetId: String) {
assertThatCode { RuleSet(RuleSet.Id(ruleSetId), emptyList()) }.doesNotThrowAnyException()
}

@ParameterizedTest(name = "should not allow RuleSet with name {0}")
@MethodSource("getInvalidNames")
fun shouldNotAllowInvalidNames(ruleSetId: String) {
assertThatExceptionOfType(IllegalArgumentException::class.java).isThrownBy {
RuleSet(
RuleSet.Id(ruleSetId),
emptyList()
)
}.withMessageStartingWith("id '$ruleSetId' must match")
}

companion object {
@JvmStatic
fun getValidNames() = listOf(
"abc-def",
"abc-def",
"abc1-def",
"ab1c-def",
"abc1",
"abc-1",
"abc-def1",
"abc-de1f",
"abcDef",
"abcDef1",
@Nested
inner class Id {
@ParameterizedTest(name = "should allow RuleSet with id {0}")
@ValueSource(
strings = [
"abc-def",
"abc1-def",
"abc-1",
"abc-def1",
"abc-de1f",
"abcDef",
"abcDef1",
]
)
fun shouldAllowValidIds(ruleSetId: String) {
assertThatCode { RuleSet.Id(ruleSetId) }
.doesNotThrowAnyException()
}

@JvmStatic
fun getInvalidNames() = listOf(
"abc def",
"abc1 def",
"ab1c def",
"abc 1",
"abc-",
"abc-def-",
"-abcDef",
"1abcDef",
@ParameterizedTest(name = "should not allow RuleSet with id {0}")
@ValueSource(
strings = [
"abc def",
"abc-",
"-abcDef",
"1abcDef",
]
)
fun shouldNotAllowInvalidIds(ruleSetId: String) {
assertThatCode { RuleSet.Id(ruleSetId) }
.hasMessageStartingWith("Id '$ruleSetId' must match")
.isInstanceOf(IllegalArgumentException::class.java)
}
}
}
101 changes: 101 additions & 0 deletions detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/RuleSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1,101 @@
package io.gitlab.arturbosch.detekt.api

import org.assertj.core.api.Assertions.assertThatCode
import org.junit.jupiter.api.Nested
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.ValueSource

class RuleSpec {
@Nested
inner class Id {
@ParameterizedTest(name = "should allow Rule with id {0}")
@ValueSource(
strings = [
"abc-def",
"abc1-def",
"abc-1",
"abc-def1",
"abc-de1f",
"abcDef",
"abcDef1",

"abc-def/abcDef1",
"abc1-def/abc-def",
"abc-1/abc1-def",
"abc-def1/abc-1",
"abc-de1f/abc-def1",
"abcDef/abc-de1f",
"abcDef1/abcDef",
]
)
fun shouldAllowValidIds(ruleName: String) {
assertThatCode { Rule.Id(ruleName) }
.doesNotThrowAnyException()
}

@ParameterizedTest(name = "should not allow Rule with id {0}")
@ValueSource(
strings = [
"abc def",
"abc1 def",
"ab1c def",
"abc 1",
"abc-",
"abc-def-",
"-abcDef",
"1abcDef",

"abc-def/abc def",
"abc1-def/abc1 def",
"abc-1/ab1c def",
"abc-def1/abc 1",
"abc-de1f/abc-",
"abcDef/abc-def-",

"abcDef1/abc/def",
"abcDef1/abc/",
"abcDef1/",
]
)
fun shouldNotAllowInvalidIds(ruleName: String) {
assertThatCode { Rule.Id(ruleName) }
.hasMessageStartingWith("Id '$ruleName' must match")
.isInstanceOf(IllegalArgumentException::class.java)
}
}

@Nested
inner class Name {
@ParameterizedTest(name = "should allow Rule with name {0}")
@ValueSource(
strings = [
"abc-def",
"abc1-def",
"abc-1",
"abc-def1",
"abc-de1f",
"abcDef",
"abcDef1",
]
)
fun shouldAllowValidNames(ruleName: String) {
assertThatCode { Rule.Name(ruleName) }
.doesNotThrowAnyException()
}

@ParameterizedTest(name = "should not allow Rule with name {0}")
@ValueSource(
strings = [
"abc def",
"abc-",
"-abcDef",
"1abcDef",
]
)
fun shouldNotAllowInvalidNames(ruleName: String) {
assertThatCode { Rule.Name(ruleName) }
.hasMessageStartingWith("Name '$ruleName' must match")
.isInstanceOf(IllegalArgumentException::class.java)
}
}
}
17 changes: 6 additions & 11 deletions detekt-api/src/testFixtures/kotlin/io/gitlab/arturbosch/detekt/test/TestFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 60,11 @@ fun createRuleInstance(
id: String = "TestSmell/id",
ruleSetId: String = "RuleSet${id.split("/", limit = 2).first()}",
description: String = "Description ${id.split("/", limit = 2).first()}",
): RuleInstance {
val split = id.split("/", limit = 2)
return RuleInstanceImpl(
id = id,
name = Rule.Name(split.first()),
ruleSetId = RuleSet.Id(ruleSetId),
description = description
)
}
): RuleInstance = RuleInstanceImpl(
id = Rule.Id(id),
ruleSetId = RuleSet.Id(ruleSetId),
description = description
)

fun createIssueForRelativePath(
ruleInstance: RuleInstance,
Expand Down Expand Up @@ -125,8 121,7 @@ private data class IssueImpl(
) : Issue

private data class RuleInstanceImpl(
override val id: String,
override val name: Rule.Name,
override val id: Rule.Id,
override val ruleSetId: RuleSet.Id,
override val description: String,
) : RuleInstance
3 changes: 2 additions & 1 deletion detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/Spec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 5,7 @@ import io.github.detekt.tooling.api.spec.RulesSpec
import io.github.detekt.tooling.api.spec.RulesSpec.RunPolicy.DisableDefaultRuleSets
import io.github.detekt.tooling.api.spec.RulesSpec.RunPolicy.NoRestrictions
import io.github.detekt.utils.PathFilters
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.RuleSet
import java.nio.file.Path
import kotlin.io.path.ExperimentalPathApi
Expand Down Expand Up @@ -103,5 104,5 @@ private fun CliArgs.toRunPolicy(): RulesSpec.RunPolicy {
val parts = runRule?.split(":")
?: return if (disableDefaultRuleSets) DisableDefaultRuleSets else NoRestrictions
require(parts.size == 2) { "Pattern 'RuleSetId:RuleName' expected." }
return RulesSpec.RunPolicy.RestrictToSingleRule(RuleSet.Id(parts[0]), parts[1])
return RulesSpec.RunPolicy.RestrictToSingleRule(RuleSet.Id(parts[0]), Rule.Id(parts[1]))
}