Skip to content

Import the type, never the namespace - #3122

Draft
StylianosGakis wants to merge 3 commits into
chore/lint-config-fixesfrom
chore/namespace-import-ktlint
Draft

Import the type, never the namespace#3122
StylianosGakis wants to merge 3 commits into
chore/lint-config-fixesfrom
chore/namespace-import-ktlint

Conversation

@StylianosGakis

@StylianosGakis StylianosGakis commented Sep 3, 2026

Copy link
Copy Markdown
Member

Top of stack #3127, on top of #3121. Documents the import rule, fixes every existing violation, and enforces it on every source set.

Why

PR #3100 was one screen refactor in one file, +250/-139. Most of that was not the refactor: 29 gratuitous new imports and ~60 rewritten call sites turning Res.string.FOO into string.FOO, Clock.System.now() into System.now(), and HomeEvent.RefreshData into RefreshData. The real change was buried under the churn, and the shortened forms read worse than what they replaced. string.FOO names nothing, and System.now() reads as java.lang.System.

The rule

Import the type, never the namespace. An import may shorten a qualified reference only when the short name still says what it is to someone reading that line cold.

Types pass, and this stays the house style (~1,200 such imports already exist):

import ...HomeUiState.Success                 // `is Success ->` reads fine
import kotlin.time.Duration.Companion.seconds // enables the `5.seconds` idiom

Members reached through a receiver that carries the meaning do not:

import hedvig.resources.Res.string   // ❌ use Res.string.FOO
import kotlin.time.Clock.System      // ❌ use Clock.System.now()

The rule also bans import-only changes to lines you are not otherwise editing.

Enforcement

kotlinter already runs ktlint over every source set, so hedvig-ktlint implements the rule as a RuleSetProviderV3 custom ruleset against ktlint 1.8.0, the version kotlinter 5.6.0 bundles. It is wired into every module through kotlinter's ktlint configuration, and covers androidMain, commonMain, iosMain and plain JVM modules alike.

Android Lint would be the more precise tool, since it resolves types. It is not used as the mechanism of record because it cannot reach KMP modules at all, and 19 of the violations fixed here live in one. What was tried first:

Attempt Result
Apply standalone com.android.lint alongside the KMP plugin Works, catches commonMain. But every generateAndroidMainLintModel then depends on itself, and the build dies with a circular dependency as soon as two such modules depend on each other
checkDependencies = false Same cycle
android.experimental.lint.analysisPerComponent=false Same cycle
com.android.internal.lint Runnable lint, but JVM components only. A full run with a planted violation still in commonMain came back green

The root cause is upstream. AGP's com.android.kotlin.multiplatform.library contributes a lint {} DSL but registers no task that runs it, and it sits outside com.android.base, so plugin logic keying off the Android plugins does nothing there. Google issue 246751841 has tracked this since 2022, AGP release notes through 9.3.2 never mention KMP lint, and the KMP plugin docs don't mention lint in either the supported or the unsupported section. The consistent community answer is to use ktlint or detekt for common code.

An Android Lint version of the same check was written and then dropped (#3123, closed). It could only ever be additive, and it cost a second implementation of the same policy with a duplicated denied list. Its one substantive advantage, resolving the owner rather than inferring it, turned out to be worth almost nothing here: the only imports needing the CAPITALIZED_PACKAGES escape hatch are platform.*, which appear in 8 files, all in nativeMain, and the repo contains no other capitalized-package member imports at all. So this rule is the single mechanism, and it covers everything.

The tradeoff

ktlint has no type resolution, so it 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.

The fixes

26 imports across 12 files. Two of them were latent bugs rather than style:

  • TopAppBarLayoutForActions has no windowInsets parameter and was silently reading the import. Only the compiler caught it, after the import was removed.
  • TopAppBar.kt's import was otherwise entirely unused, the same class of miss as an unused kotlin.time.Clock import elsewhere. ktlint's no-unused-imports catches neither.

For the 19 design system Defaults members, the owning object lives in the same file, so they are now qualified as TooltipDefaults.defaultStyle, matching what TopAppBar.kt already did. References from inside an owning object's own body are untouched, since they resolve without an import.

The commits are ordered doc, then fixes, then enforcement, so every commit is green and the enforcing commit never lands on a dirty tree.

Verification

  • Fires on KMP commonMain: a planted import in Tooltip.kt reports [hedvig:namespace-import]. Also verified on a plain JVM module.
  • Zero false positives. A full ktlintCheck sweep across every module and source set returns no namespace-import hits.
  • Edge cases pass: platform.Foundation.systemLocale in core-locale's nativeMain and every Duration.Companion.seconds import are clean.
  • ./gradlew lint across the repo passes, which is what caught a lowercase resource reference the first pass at the cross-sells fixes had missed.

Pre-existing develop failure, handled below this PR

ktlintCheck was red on develop with 37 errors in :feature-insurances and :feature-terminate-insurance, on standard:indent (27), standard:function-signature (4), standard:multiline-if-else (2), standard:max-line-length (2) and standard:if-else-wrapping (2). None of those files are touched by this PR.

#3111 sits at the bottom of this stack and fixes them, so ktlintCheck reports 0 errors from here upward. That ordering is deliberate: this PR adds a new ktlint rule, and that rule is only trustworthy if the task it runs under is otherwise clean.

Import the type, never the namespace. An import may shorten a qualified
reference only when the short name still says what it is to someone
reading that line cold.

Sealed subclasses and enum entries pass that test and stay allowed;
around 1200 such imports already exist and they are the house style.
Members reached through a receiver that carries the meaning do not:
Res.string, Res.drawable and Clock.System are banned, since
string.FOO and System.now() lose what Res.string.FOO and
Clock.System.now() tell the reader.

Also bans import-only changes to lines that are not otherwise being
edited. PR #3100 carried 29 gratuitous new imports and ~60 rewritten
call sites through a single screen refactor, which buried the real
change under churn.
Fixes every existing violation of the import rule: 26 imports across 12
files.

Res.string.FOO and Clock.System.now() are restored at their use sites in
the feature files that had shortened them away. Nineteen of the rest are
design system Defaults members whose owning object lives in the same
file, so they are now qualified as TooltipDefaults.defaultStyle and the
like, matching what TopAppBar.kt already did. References from inside an
owning object's own body are untouched, since they resolve without an
import and gain nothing from the prefix.

TopAppBar.kt's import turned out to be unused except for one call in
TopAppBarLayoutForActions, which has no windowInsets parameter of its
own and was silently reading the import.
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.
@StylianosGakis
StylianosGakis force-pushed the chore/namespace-import-ktlint branch from d7124e5 to b3d6313 Compare September 3, 2026 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant