Skip to content

Commit 94fe530

Browse files
chore: add a ktlint namespace-import rule
kotlinter runs ktlint over every source set, so a custom ruleset enforces the import rule everywhere: androidMain, commonMain, iosMain and plain JVM modules alike. Android Lint would be the more precise tool, since it resolves types, but it cannot reach KMP modules at all, and 19 of the violations just cleaned up live in one. AGP's com.android.kotlin.multiplatform.library registers a lint {} DSL but no task that runs it, and it sits outside com.android.base, so plugin logic keying off the Android plugins does nothing there. Applying the standalone com.android.lint plugin alongside it does produce a working lint that reads commonMain, but every generateAndroidMainLintModel then depends on itself and the build fails with a circular dependency the moment two such modules depend on each other. Neither checkDependencies=false nor android.experimental.lint.analysisPerComponent=false avoids it, and com.android.internal.lint only ever analyses the JVM components, which do not include commonMain. Google issue 246751841 has tracked this since 2022. Having no type resolution means this cannot ask whether an owner is a class or a package. CAPITALIZED_PACKAGES carries that cost: Kotlin/Native interop packages are named after the framework they bind, so platform.Foundation.systemLocale is shaped exactly like a member import and has to be excluded by prefix.
1 parent b297f26 commit 94fe530

7 files changed

Lines changed: 110 additions & 0 deletions

File tree

‎build-logic/convention/src/main/kotlin/HedvigGradlePlugin.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@ private fun Project.configureKtlint(libs: LibrariesForLibs) {
3838
reporters = arrayOf(ReporterType.checkstyle.name)
3939
}
4040

41+
// Our own rules run on every source set, which is what gives KMP modules the coverage that
42+
// Android Lint cannot reach.
43+
if (name != "hedvig-ktlint") {
44+
dependencies {
45+
add("ktlint", project(":hedvig-ktlint"))
46+
}
47+
}
48+
4149
tasks.withType<org.jmailen.gradle.kotlinter.tasks.LintTask>().configureEach {
4250
exclude { it.file.path.contains("generated/") }
4351
reports.set(

‎gradle/libs.versions.toml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ kmpNativeCoroutines = "1.0.5"
2727
kotlin = "2.4.10"
2828
kotlinpoet = "2.3.0"
2929
kotlinter = "5.6.0"
30+
# Must match the ktlint that kotlinter bundles, so the custom ruleset links against the same API
31+
ktlintCore = "1.8.0"
3032
ksp = "2.3.10"
3133
ktor = "3.5.1"
3234
license = "0.9.9"
@@ -270,6 +272,8 @@ kmpNativeCoroutines-gradlePlugin = { module = "com.rickclephas.kmp.nativecorouti
270272
kotlin-gradlePlugin = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "kotlin" }
271273
kotlinSerialization-gradlePlugin = { module = "org.jetbrains.kotlin:kotlin-serialization", version.ref = "kotlin" }
272274
kotlinter-gradlePlugin = { module = "org.jmailen.gradle:kotlinter-gradle", version.ref = "kotlinter" }
275+
ktlint-ruleEngineCore = { module = "com.pinterest.ktlint:ktlint-rule-engine-core", version.ref = "ktlintCore" }
276+
ktlint-cliRulesetCore = { module = "com.pinterest.ktlint:ktlint-cli-ruleset-core", version.ref = "ktlintCore" }
273277
ksp-gradlePlugin = { module = "com.google.devtools.ksp:com.google.devtools.ksp.gradle.plugin", version.ref = "ksp" }
274278
license-gradlePlugin = { module = "com.jaredsburrows.license:com.jaredsburrows.license.gradle.plugin", version.ref = "license" }
275279
metro-gradlePlugin = { module = "dev.zacsweers.metro:dev.zacsweers.metro.gradle.plugin", version.ref = "metro" }

‎hedvig-ktlint/build.gradle.kts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
plugins {
2+
id("hedvig.jvm.library")
3+
id("hedvig.gradle.plugin")
4+
}
5+
6+
dependencies {
7+
compileOnly(libs.ktlint.ruleEngineCore)
8+
compileOnly(libs.ktlint.cliRulesetCore)
9+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package com.hedvig.android.ktlint
2+
3+
import com.pinterest.ktlint.cli.ruleset.core.api.RuleSetProviderV3
4+
import com.pinterest.ktlint.rule.engine.core.api.RuleProvider
5+
import com.pinterest.ktlint.rule.engine.core.api.RuleSetId
6+
7+
internal const val CUSTOM_RULE_SET_ID = "hedvig"
8+
9+
class HedvigRuleSetProvider : RuleSetProviderV3(RuleSetId(CUSTOM_RULE_SET_ID)) {
10+
override fun getRuleProviders(): Set<RuleProvider> = setOf(
11+
RuleProvider { NamespaceImportRule() },
12+
)
13+
}
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package com.hedvig.android.ktlint
2+
3+
import com.pinterest.ktlint.rule.engine.core.api.ElementType
4+
import com.pinterest.ktlint.rule.engine.core.api.Rule
5+
import com.pinterest.ktlint.rule.engine.core.api.RuleId
6+
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
7+
8+
/**
9+
* Reports imports that shorten a qualified reference past the point where the short name still says
10+
* what it is, such as `import hedvig.resources.Res.string` turning `Res.string.FOO` into `string.FOO`.
11+
*
12+
* This runs through kotlinter on every source set, `commonMain` and `iosMain` included, which is why
13+
* the rule lives here rather than as an Android Lint check: AGP's KMP library plugin registers no
14+
* task that runs Android Lint (https://issuetracker.google.com/issues/246751841), so a lint check
15+
* cannot see a KMP module at all, and the design system is one.
16+
*
17+
* The cost of that reach is precision. ktlint has no type resolution, so an owner is recognized by
18+
* the shape of the import path rather than by resolving it, and [CAPITALIZED_PACKAGES] carries the
19+
* exceptions. Trading the heuristic for a resolved owner across the whole codebase needs an analyzer
20+
* that both resolves types and runs on every source set: Android Lint, once the issue above is
21+
* fixed, or detekt with type resolution.
22+
*/
23+
internal class NamespaceImportRule :
24+
Rule(
25+
ruleId = RuleId("$CUSTOM_RULE_SET_ID:namespace-import"),
26+
about = About(
27+
maintainer = "Hedvig",
28+
repositoryUrl = "https://github.com/HedvigInsurance/android",
29+
issueTrackerUrl = "https://github.com/HedvigInsurance/android/issues",
30+
),
31+
) {
32+
override fun beforeVisitChildNodes(
33+
node: ASTNode,
34+
autoCorrect: Boolean,
35+
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
36+
) {
37+
if (node.elementType != ElementType.IMPORT_DIRECTIVE) return
38+
val text = node.text
39+
// An alias is a deliberate act of renaming, and gives the use site a name of its own.
40+
if (text.contains(" as ")) return
41+
val qualifiedName = text.removePrefix("import").trim()
42+
if (qualifiedName.isEmpty() || qualifiedName.endsWith("*")) return
43+
44+
val importedName = qualifiedName.substringAfterLast('.')
45+
val ownerPath = qualifiedName.substringBeforeLast('.', "")
46+
val ownerName = ownerPath.substringAfterLast('.')
47+
if (importedName.isEmpty() || ownerName.isEmpty()) return
48+
if (!ownerName.first().isUpperCase()) return
49+
// `Duration.Companion.seconds` and friends exist to enable the `5.seconds` receiver idiom.
50+
if (ownerName == "Companion") return
51+
if (CAPITALIZED_PACKAGES.any { qualifiedName.startsWith(it) }) return
52+
53+
val importsAMember = importedName.first().isLowerCase()
54+
if (!importsAMember && qualifiedName !in DENIED_IMPORTS) return
55+
56+
emit(
57+
node.startOffset,
58+
"Import $ownerName and write $ownerName.$importedName at the use site. " +
59+
"On its own, $importedName no longer says what it is.",
60+
false,
61+
)
62+
}
63+
64+
private companion object {
65+
/**
66+
* Kotlin/Native interop packages are capitalized after the framework they bind, so their
67+
* top-level declarations look identical to members of a class.
68+
*/
69+
val CAPITALIZED_PACKAGES = listOf("platform.")
70+
71+
/** Imports that read as a type but still leave nothing meaningful at the use site. */
72+
val DENIED_IMPORTS = setOf("kotlin.time.Clock.System")
73+
}
74+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
com.hedvig.android.ktlint.HedvigRuleSetProvider

‎settings.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,4 @@ include("design-showcase-desktop")
6969
project(":design-showcase-desktop").projectDir =
7070
rootProject.projectDir.resolve("micro-apps").resolve("design-showcase-desktop")
7171
include("hedvig-lint")
72+
include("hedvig-ktlint")

0 commit comments

Comments
 (0)