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

Prevents jvm incompat dataclass constructors with value class params #329

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
790 changes: 790 additions & 0 deletions config/detekt/detekt.yml

Large diffs are not rendered by default.

28 changes: 27 additions & 1 deletion core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@ plugins {
alias(libs.plugins.dokka)
alias(libs.plugins.arrow.gradle.publish)
alias(libs.plugins.semver.gradle)
alias(libs.plugins.detekt)
//id("com.xebia.asfuture").version("0.0.1")
}

dependencies {
detektPlugins(project(":detekt-rules"))
}

java {
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
Expand All @@ -26,6 +31,13 @@ java {
}
}

detekt {
toolVersion = "1.23.1"
source = files("src/commonMain/kotlin", "src/jvmMain/kotlin")
config.setFrom("../config/detekt/detekt.yml")
autoCorrect = true
}

kotlin {
jvm {
compilations {
Expand Down Expand Up @@ -87,7 +99,7 @@ kotlin {
implementation(libs.logback)
implementation(libs.skrape)
implementation(libs.rss.reader)
api(libs.jackson)
api(libs.jackson)
api(libs.jackson.schema)
api(libs.jackson.schema.jakarta)
api(libs.jakarta.validation)
Expand Down Expand Up @@ -161,6 +173,20 @@ spotless {
}

tasks {

withType<io.gitlab.arturbosch.detekt.Detekt>().configureEach {
dependsOn(":detekt-rules:assemble")
autoCorrect = true
}
named("detektJvmMain") {
dependsOn(":detekt-rules:assemble")
getByName("build").dependsOn(this)
}
named("detekt") {
dependsOn(":detekt-rules:assemble")
getByName("build").dependsOn(this)
}

withType<Test>().configureEach {
maxParallelForks = Runtime.getRuntime().availableProcessors()
useJUnitPlatform()
Expand Down
40 changes: 40 additions & 0 deletions detekt-rules/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
@file:Suppress("DSL_SCOPE_VIOLATION")

repositories { mavenCentral() }

plugins {
`kotlin-dsl`
base
alias(libs.plugins.spotless)
}

spotless {
kotlin {
target("**/*.kt")
ktfmt().googleStyle()
}
}


dependencies {
api(libs.detekt.api)
testImplementation(libs.detekt.test)
testImplementation(libs.kotest.assertions)
testImplementation("org.junit.jupiter:junit-jupiter:5.10.0")
implementation(libs.klogging)
}

tasks.withType<Jar>() {
duplicatesStrategy = DuplicatesStrategy.EXCLUDE
}

tasks.withType<Test>().configureEach {
maxParallelForks = Runtime.getRuntime().availableProcessors()
useJUnitPlatform()
systemProperty("junit.jupiter.testinstance.lifecycle.default", "per_class")
systemProperty("compile-snippet-tests", project.hasProperty("compile-test-snippets"))
testLogging {
setExceptionFormat("full")
setEvents(listOf("passed", "skipped", "failed", "standardOut", "standardError"))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package org.example.detekt

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.CorrectableCodeSmell
import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.Severity
import org.jetbrains.kotlin.psi.KtClass

/** Fixes occurrences of inline data class when enabled to allow for jvm language interop. */
class JvmInlineAnnotation(config: Config) : Rule(config) {

override val issue: Issue =
Issue(
javaClass.simpleName,
Severity.Defect,
"This rule reports an public inline value class as incompatible with other jvm languages.",
Debt.FIVE_MINS
)

/**
* * Reports inline value classes not marked @JvmInline as a lint warning, and autofixes the
* declaration to include @JvmInline.
*/
override fun visitClass(klass: KtClass) {

if (
klass.isValue() &&
klass.annotationEntries.filter { e -> e.shortName?.asString() == "JvmInline" }.size < 1
) {
report(
CorrectableCodeSmell(
issue,
Entity.from(klass),
"Kotlin inline value classes are not compatible with other jvm lanugages.",
emptyList(),
listOf(Entity.from(klass)),
false
)
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package org.example.detekt

import io.gitlab.arturbosch.detekt.api.*
import org.jetbrains.kotlin.psi.KtClass
import org.jetbrains.kotlin.psi.KtElement
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.KtVisitorVoid
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.isValueClassType
import org.jetbrains.kotlin.types.KotlinType

/** Fixes occurrences of inline data class when enabled to allow for jvm language interop. */
class PublicDataClassConstructorWithValueParameters(config: Config) : Rule(config) {

override val issue: Issue =
Issue(
javaClass.simpleName,
Severity.Defect,
"This rule reports public data classes that are incompatible with other jvm languages.",
Debt.FIVE_MINS
)

/** * Reports inline value classes not marked @JvmInline as a lint warning. */
override fun visitClass(ktClass: KtClass) {
try {
if (
ktClass.isData() &&
ktClass.hasPrimaryConstructor() &&
ktClass.primaryConstructor?.valueParameters?.any {
val ktpe = bindingContext.get(BindingContext.TYPE, it.typeReference)

(ktpe as KotlinType).unwrap().isValueClassType()
} == true
) {
var hasJvmStaticConstructor = false
val hasCompanionObjects = ktClass.companionObjects.isNotEmpty()
if (!hasCompanionObjects) {
hasJvmStaticConstructor = false
} else {
ktClass.companionObjects.forEach { ktObjectDeclaration ->
if (!hasJvmStaticConstructor) {
ktObjectDeclaration.acceptChildren(
object : KtVisitorVoid() {
override fun visitKtElement(element: KtElement) {
try {
element.acceptChildren(this)
} catch (_: UnsupportedOperationException) {}
}

override fun visitNamedFunction(function: KtNamedFunction) {
if (!hasJvmStaticConstructor) {
val klassParametersAsString =
ktClass.primaryConstructor?.valueParameterList?.parameters?.fold("") {
acc,
parameter ->
if (acc == "") {
acc + parameter.name + ":" + parameter.typeReference?.text
} else {
acc + ", " + parameter.name + ":" + parameter.typeReference?.text
}
}
val functionParametersAsStrings =
function.valueParameterList?.parameters?.fold("") { acc, parameter ->
if (acc == "") {
acc + parameter.name + ":" + parameter.typeReference?.text
} else {
acc + ", " + parameter.name + ":" + parameter.typeReference?.text
}
}
val functionHasJvmStatic =
function.annotationEntries.any { it.shortName?.asString() == "JvmStatic" }
val functionHasAllButLastDefaultConstructorParameter =
klassParametersAsString == functionParametersAsStrings
hasJvmStaticConstructor =
functionHasJvmStatic && functionHasAllButLastDefaultConstructorParameter
}
}
}
)
}
}
}
if (!hasJvmStaticConstructor) {
report(
CorrectableCodeSmell(
issue,
Entity.from(ktClass),
"Kotlin data classes containing parameters that are value classes must have a JVM static factory method to be compatible with other jvm languages.",
emptyList(),
listOf(Entity.from(ktClass)),
true
)
)
}
}
} catch (_: Exception) {}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.example.detekt

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.api.RuleSetProvider

class XefRuleSetProvider : RuleSetProvider {
override val ruleSetId: String = "XefRuleSet"

override fun instance(config: Config): RuleSet {
return RuleSet(
ruleSetId,
listOf(JvmInlineAnnotation(config), PublicDataClassConstructorWithValueParameters(config)),
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.example.detekt.XefRuleSetProvider
5 changes: 5 additions & 0 deletions detekt-rules/src/main/resources/config/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
XefRuleSet:
JvmInlineAnnotation:
active: true
PublicDataClassConstructorWithValueParameters:
active: true
11 changes: 11 additions & 0 deletions detekt-rules/src/main/resources/logback.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<configuration>
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<encoder>
<pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} MDC=%X{user} - %msg%n</pattern>
</encoder>
</appender>

<root level="debug">
<appender-ref ref="STDOUT" />
</root>
</configuration>
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package org.example.detekt

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
import io.kotest.matchers.collections.shouldHaveSize
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.junit.jupiter.api.Test

@KotlinCoreEnvironmentTest
internal class JvmInlineAnnotationTest(private val env: KotlinCoreEnvironment) {

@Test
fun `reports missing jvminline annotations`() {
val code = """
value class User(val id: String)
"""
val findings = JvmInlineAnnotation(Config.empty).compileAndLintWithContext(env, code)
findings shouldHaveSize 1
}

@Test
fun `doesn't report annotated value classes`() {
val code =
"""
import kotlin.jvm.JvmInline
@JvmInline value class User(val id: String)
"""
val findings = JvmInlineAnnotation(Config.empty).compileAndLintWithContext(env, code)
findings shouldHaveSize 0
}
}