Add a type-resolving NamespaceImport lint check - #3123
Closed
StylianosGakis wants to merge 1 commit into
Closed
Conversation
This was referenced Sep 3, 2026
StylianosGakis
force-pushed
the
chore/namespace-import-ktlint
branch
from
September 3, 2026 08:31
d7124e5 to
b3d6313
Compare
StylianosGakis
force-pushed
the
chore/namespace-import-lint-check
branch
from
September 3, 2026 08:31
a641b95 to
a933462
Compare
The ktlint rule already enforces the import rule on every source set, so this adds precision and IDE feedback rather than coverage. Two things it does that ktlint cannot. It resolves the owner through context.evaluator.findClass instead of inferring one from the shape of the import path, so platform.Foundation.systemLocale needs no prefix exception to stay unflagged. And Android Lint reports inline in the IDE while the import is being typed, which is where a style rule is cheapest to obey. It cannot run on KMP modules, so the ktlint rule stays the mechanism of record and the two denied lists have to be kept in step. Deny and allow lists are configurable through the shared lint.xml. hedvig-lint had no test source set, so this adds one plus the lint-tests dependency. Seven cases cover both directions of the rule.
StylianosGakis
force-pushed
the
chore/namespace-import-lint-check
branch
from
September 3, 2026 12:36
a933462 to
3cc093c
Compare
Member
Author
|
Closing: not worth its complexity for an import style rule. It was only ever additive. #3122's ktlint rule enforces the same policy on every source set, so nothing is left unenforced by dropping this. What it bought, and why that wasn't enough:
The 34 modules it cannot reach are all KMP, including Worth reopening if AGP issue 246751841 is ever fixed. At that point Android Lint could cover the whole repository and would be the better single mechanism, making the ktlint rule the redundant half instead. The code here still works and is fully tested if that day comes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #3122. Review that one first.
The ktlint rule in #3122 already enforces the import rule on every source set, so this adds precision and IDE feedback rather than coverage. It is entirely optional: nothing regresses if it is dropped.
What it adds over the ktlint rule
Type resolution. It resolves the owner through
context.evaluator.findClassinstead of inferring one from the shape of the import path.platform.Foundation.systemLocaletherefore needs no prefix exception to stay unflagged, whereas the ktlint rule carriesCAPITALIZED_PACKAGESfor exactly that case.Inline IDE reporting. Android Lint surfaces the problem while the import is being typed, which is where a style rule is cheapest to obey. Catching it at CI is much later.
Deny and allow lists are configurable through the shared
lint.xml, alongside the existingComposeM2Api.The coupling this introduces
Two mechanisms now encode the same policy, and Android Lint cannot run on KMP modules, so the ktlint rule stays the mechanism of record. The default denied list (
kotlin.time.Clock.System) exists in both and has to be kept in step. Both files say so in their KDoc.If that bookkeeping is judged not worth the IDE benefit, the right move is to close this PR and keep #3122 alone. Driving both from one shared configuration, for instance
.editorconfig, is the alternative worth considering, and is deliberately not attempted here.On Android modules a violation is now reported twice, which only happens when someone is already wrong.
Test source set
hedvig-linthad no test source set, so this adds one plus thelint-testsdependency. Seven cases cover both directions of the rule.Note that these tests need an Android SDK: they fail locally with "This test requires an Android SDK: No SDK configured" unless
ANDROID_HOMEis set. CI'scommon-setupprovides it.Verification
import hedvig.resources.Res.stringinCrossSells.ktfails:cross-sells:lintwithError: Import Res and write Res.string at the use site ... [NamespaceImport]. Reverted, lint clean../gradlew lintacross the repo passes.