DEV Community

Cover image for AI Writes Locally Correct Code. Which Is Why Your Linter Is Silent
Ibrahim A. Hemaida
Ibrahim A. Hemaida

Posted on

AI Writes Locally Correct Code. Which Is Why Your Linter Is Silent

Let me get the obvious objection out of the way first, because it's the right one.

Nothing in the list below is new. God ViewModels, layer leaks, tests that assert their own mocks: these were in Android code review checklists years before anyone shipped an LLM. If you've been reviewing mobile code for a decade, you'll read the list and think I've been catching this since 2016.

You have. Two things changed.

The first is the rate, which everyone talks about. The second is the one I want to write about: the kind of tool that can catch these has changed, and most teams haven't noticed.

Here's the claim in one sentence. A human writing bad code usually writes something locally wrong: a null dereference, a leaked scope, a wrong operator. Local tools catch local mistakes, and that is what ktlint, detekt and Android Lint are. They answer questions about one file, often one function. An assistant makes that kind of mistake far less often than a tired human does. What it produces instead is code that is completely defensible in the file you are looking at and wrong only in relation to a decision made somewhere else in the repository.

There is no line to underline. So the linter says nothing, and the review passes, because the code looks exactly like the code around it.

That mismatch is the actual story, and it has a practical consequence: adding more local rules buys you almost nothing. The checks that work sit at a different level. Below is the list, sorted by which level catches each one, then working code for each level.

The eight, and what actually catches them

# What the assistant writes Why the linter is quiet Level that catches it
1 Nine constructor dependencies, added one prompt at a time no single parameter is wrong Local
2 !! sitting next to an empty catch (e: Exception) both are legal, and one of the two rules needs type resolution Local
3 ViewModel imports Retrofit; the DTO goes straight to the UI every line is valid Kotlin Relational
4 MutableStateFlow exposed as a public property, in a codebase that exposes read-only StateFlow through asStateFlow() valid Kotlin, valid Flow API; the convention lives in other files Relational
5 collectAsState() where the codebase uses collectAsStateWithLifecycle() both are real Compose APIs Relational
6 A test that only verifies the mock was called and asserts nothing about the result it compiles, runs, passes, and has coverage Behavioural
7 isLoading + error + data as three independent flags each property is fine on its own Judgment
8 @Inject on the constructor, OrderCache() built by hand inside valid Kotlin, valid Hilt Judgment

Four levels, in increasing order of how much they cost you to set up.


Level 1. Local: rules you already have, tuned for someone else's codebase

Patterns 1 and 2 are file-local, and detekt has shipped rules for both for years. Here's the part worth knowing: LongParameterList, LargeClass, SwallowedException and TooGenericExceptionCaught are all active by default. Nobody has to turn them on.

Hardcoded Dispatchers.IO is the classic example people reach for here, and it doesn't belong in the table at all: detekt's InjectDispatcher has been on by default since 1.21. It does need type resolution, which is the trap described next, but on a task that provides it that one is Local, and it is already firing.

The !! is the same story. UnsafeCallOnNullableType is active by default too, but it needs type resolution: detekt has to see the compiler's type information to know the receiver was nullable in the first place. The plain detekt task doesn't give it that. The type-resolution tasks do: detektMain and detektTest in a JVM module, and in an Android module the per-variant ones, detektDebug, detektRelease, detektDebugUnitTest. Run ./gradlew tasks --group verification once and read what is actually there; a CI step that calls detektMain in an Android module fails loudly, but one that calls plain detekt succeeds while every type-resolution rule stays silently inert.

So if a nine-dependency ViewModel with an empty catch block is sitting in your main branch right now, there are four explanations, and it's worth knowing which one you're living with:

  1. detekt isn't actually running in CI. It's a Gradle task somebody added and no pipeline calls.
  2. A detekt-baseline.xml is suppressing it.
  3. The defaults are firing on paper and nobody reads the report.
  4. You're running plain detekt rather than one of the type-resolution tasks, so every type-resolution rule in the set is silently inert.

Explanation 2 is the common one, and it's worth understanding precisely. Someone added detekt to a project that already had thousands of violations and generated a baseline so CI could go green. That was the right call at the time. But a baseline suppresses by signature, it has no expiry, and it is now sitting underneath a steady stream of machine-written code. Open yours, look at the date, and check how many entries name files created after it.

Then there's LargeClass, whose default threshold is 600 lines. A 550-line ViewModel is silently fine by that standard. It is not fine by any standard I'd defend in review, and the default was never meant to encode your team's taste:

# detekt.yml (tightened, not enabled: these are already on)
complexity:
  LongParameterList:
    constructorThreshold: 5   # default is 7
    ignoreDefaultParameters: true
  LargeClass:
    threshold: 250            # default is 600
Enter fullscreen mode Exit fullscreen mode

Those key names are detekt 1.23.x, which is what almost everyone is on. detekt 2.0 renames them to allowedConstructorParameters and allowedLines, so check your version before you copy that block, because a config key detekt doesn't recognise is a rule you think you tightened and didn't.

That's the whole of level 1: no new tool, no new CI step. Read your baseline, check you're on a task that does type resolution, and set thresholds that reflect what your team would actually accept.

Level 2. Relational: make the repository the unit of analysis

Patterns 3, 4 and 5 have the same shape. The file is fine. The relationship between the file and a decision made elsewhere is the violation. To catch those you need a check whose input is the whole source tree, and the cheapest way to get one on Kotlin is Konsist, which runs as an ordinary JUnit test. No new CI step, no new tool in the pipeline; it just fails the test task.

The layer boundary from pattern 3, as a test:

import com.lemonappdev.konsist.api.Konsist
import com.lemonappdev.konsist.api.architecture.Layer
import org.junit.Test

class ArchitectureTest {
    @Test
    fun `layers depend only in one direction`() {
        Konsist
            .scopeFromProduction()
            .assertArchitecture {
                val presentation = Layer("Presentation", "com.acme.presentation..")
                val domain = Layer("Domain", "com.acme.domain..")
                val data = Layer("Data", "com.acme.data..")

                presentation.dependsOn(domain)
                data.dependsOn(domain)
                domain.dependsOnNothing()
            }
    }
}
Enter fullscreen mode Exit fullscreen mode

Three declarative lines, and pattern 3 is now impossible to merge. Not discouraged. Impossible. The build goes red before a human reads the diff, which matters more than it sounds: the expensive part of a layer leak isn't the leak, it's the fourteen files that get written against it before anyone notices.

Pattern 5 is the one I'd actually add first, because it isn't a style question. It's a lifecycle bug. collectAsState keeps collecting while the app is in the background; collectAsStateWithLifecycle doesn't. Assistants reach for the first one constantly, because their training data is full of Compose code written before the lifecycle-aware version was the default advice. Both compile. Both work in a demo. One of them wakes up your users' phones.

private val bareCollectAsState = Regex("""\bcollectAsState\(""")

@Test
fun `no file collects a flow without lifecycle awareness`() {
    Konsist
        .scopeFromProduction()
        .files
        .assertFalse {
            // `text` is the file's source; verify against your Konsist version
            bareCollectAsState.containsMatchIn(it.text)
        }
}
Enter fullscreen mode Exit fullscreen mode

Check the call, not the import. A wildcard import or a fully-qualified call walks straight past an import check, and assistants produce both. The regex is deliberately narrow: collectAsState( matches and collectAsStateWithLifecycle( doesn't, because the character after State is different.

Pattern 4 is the same shape with a different subject. Every property of type MutableStateFlow declared in a class whose name ends in ViewModel must be private, because this codebase exposes read-only StateFlow through asStateFlow(). Nothing in the file says so. The convention lives in the other forty ViewModels, which is exactly the kind of decision an assistant reads past and a repository-wide check doesn't. Same for anything else your team has settled once and doesn't want to re-litigate in review.

Flutter gets the same treatment, and it's arguably easier because it runs inside the analyzer. The BLoC that imports dio directly is the exact same failure as the ViewModel that imports Retrofit, and import_rules turns it into a normal dart analyze error:

# analysis_options.yaml
plugins:
  import_rules: ^0.0.8

import_rules:
  rules:
    - target: lib/domain/**
      disallow: "**"
      exclude_disallow:
        - lib/domain/**
        - package:meta/**
        - package:equatable/**
        - package:freezed_annotation/**
        - package:json_annotation/**
      reason: The domain layer depends on nothing but itself and pure
              annotation packages.

    - target: lib/presentation/**
      disallow:
        - package:dio/**
        - lib/data/**
      reason: Presentation talks to domain, never to transport.
Enter fullscreen mode Exit fullscreen mode

This is deliberately stricter than the Konsist version above. dependsOnNothing() only speaks about the layers you defined; the Dart rule speaks about every import, so pure-value packages have to be whitelisted explicitly or the first real domain file fails the check.

Check your SDK before you commit that, though: Dart's analyzer plugin system is new, and import_rules needs Dart 3.10 or later (Flutter 3.38). On an older SDK the config is accepted and does nothing at all, which is the worst failure mode available for a rule you added in order to feel safe.

On a supported SDK: red squiggle in the IDE, failure in CI, and the assistant sees the error the next time it reads the file.

Level 3. Behavioural: the test that proves your tests are theatre

Pattern 6 is the one I'd fix first if I could only fix one, because it's the only one that actively lies to you. Everything else at least looks suspicious in a diff. This one produces a green checkmark and a rising coverage number.

@Test
fun `loads user`() = runTest {
    val user = User("1", "Ibrahim")
    coEvery { repository.getUser("1") } returns user

    viewModel.load("1")

    coVerify { repository.getUser("1") }
}
Enter fullscreen mode Exit fullscreen mode

This passes, it has coverage, and it checks nothing about what load did with the result. Delete the state assignment inside load and it still passes. Return the wrong user and it still passes. PIT will report every mutant in the result-handling path as survived, and that is the whole finding.

Note what the tempting "fix" looks like:

assertEquals(user, viewModel.state.value.user)
Enter fullscreen mode Exit fullscreen mode

That one actually does kill mutants: break the state write and it fails. It is thin, not empty. The line between the two is exactly what line coverage cannot see and mutation score can.

Here's the check. It takes thirty seconds and needs no tooling at all:

Comment out the body of the function under test one statement at a time, and run the test after each. Every statement whose removal leaves the test green is a statement nothing is protecting.

On the test above: remove the repository call and it goes red; remove the state write and it stays green. That second one is the entire result-handling path, and no test in the file would notice it vanish.

Do this on three or four AI-written tests in your codebase before you read any further. I'd rather you saw the result yourself than took my word for it.

What that exercise is doing by hand is mutation testing, and the distinction it exposes is the one that matters:

Line coverage measures whether a line executed. Mutation score measures whether anything would have noticed if that line were wrong.

Human-written test suites usually have a gap between those two numbers. My read, and it's a read from my own projects rather than a study, is that AI-written suites have a chasm, and that the reason is structural rather than accidental: the assistant writes the mock setup and the check in the same breath, from the same intention, so the check is derived from the setup rather than from the behaviour. It cannot help but agree with itself.

Automating it on the JVM means PIT:

plugins {
    id("info.solidsoft.pitest")
}

pitest {
    targetClasses.set(listOf("com.acme.*ViewModel"))
    targetTests.set(listOf("com.acme.*Test"))
    mutators.set(listOf("STRONGER"))
}
Enter fullscreen mode Exit fullscreen mode

Two honest caveats, because this is the part where articles usually oversell.

PIT mutates bytecode, and Kotlin's bytecode is full of things you never wrote: null-check intrinsics, inlined function bodies copied into their call sites, coroutine state machines. Plain open-source PIT will report mutants against all of it, and a meaningful share of your first report will be junk. There's a commercial Kotlin plugin that filters most of it, and on Android you'll need pl.droidsonroids.pitest rather than the plain Gradle plugin.

So: don't start there. Start with the thirty-second manual check on the tests you already distrust. Reach for PIT when you want the number in CI, and go in expecting to spend an afternoon tuning it.

Level 4. Judgment: what none of this catches

Patterns 7 and 8 are still on the table, and they're there for different reasons.

Pattern 7 first.

private val _isLoading = MutableStateFlow(false)
private val _error = MutableStateFlow<String?>(null)
private val _user = MutableStateFlow<User?>(null)
Enter fullscreen mode Exit fullscreen mode

Three properties. Eight combinations. Perhaps three of them correspond to states your feature actually has. No rule catches this, and I don't think one can, because the defect isn't structural. It's that these three properties should have been one sealed interface, and knowing that requires knowing which states your domain permits. A linter can count properties. It can't know that "loaded, and also failed, and also still loading" is nonsense in your product.

Pattern 8 is a different kind of miss, and I'll be straight about it: it looks like a level 2 problem, and that's where I filed it in my first draft. It isn't one. Konsist reads declarations: classes, functions, properties, imports, signatures. The OrderCache() built by hand three lines into a function body is an expression, and declaration-level tools cannot see it. If you want that one automated, the honest answer is a custom detekt rule, which works on the PSI tree and can see call expressions. Worth the afternoon if manual DI is a recurring pattern on your team; not worth it if it happened twice.

And writing your own check is less exotic than it sounds. A real one from a Flutter codebase: Image.file and Image.memory with no cacheWidth/cacheHeight. Behind a 72dp thumbnail that decodes a 4032×3024 photo at full size, about 48 MB of bitmap for a tile the size of a fingernail, and a list of them is an out-of-memory crash. Every one of those lines is valid Flutter, no shipped linter has a rule for it, and it is entirely local: the fix is on the same line as the fault. A twenty-line script that walks lib/ and fails the build on any Image.file( or Image.memory( without a cacheWidth: argument closed the whole class of bug, not the one instance. That is what "custom rule" means in practice: you find one, you write the check, and it never comes back.

The same is true of the family pattern 7 belongs to: a new UserUiModel that duplicates a mapper two modules away; a use case that reimplements something already in the domain layer; an abstraction introduced for one caller. Every one of these is locally reasonable. Every one of them is a decision the repository already made, differently.

That's the layer where a reviewer earns its keep, human or otherwise, and it's the only part of the review that a rule engine can't take off your plate, which is why it's worth being deliberate about what you spend it on. Levels 1 to 3 exist to stop the mechanical findings from consuming a reviewer's attention before it reaches level 4.

So what does level 4? A person, when you have one to spare. When you don't, and on most teams shipping at this rate you don't, the next best thing is a reviewer that reads the same repository the assistant read and holds the diff to the conventions already in it.

Mine is a set of skills that do that, which I open-sourced:

github.com/IbrahimHemaida/mobile-engineering-skills

Fourteen skills for Kotlin/Android and Flutter across three jobs: scaffold (generate features with the layers, DI and tests already in place), guard (the pre-merge reviews: architecture, TDD, security, accessibility, KMM, release compliance), and doctor (work out why the build broke).

curl -fsSL https://raw.githubusercontent.com/IbrahimHemaida/mobile-engineering-skills/main/install.sh | bash
Enter fullscreen mode Exit fullscreen mode

Add -s -- --project to commit it to a repo and share it with your team. Cursor rules for .kt and .dart ship in the same repo.

But install the Konsist test first. It's twenty lines, it runs in your existing test task, and unlike any reviewer I know of, it is never tired, never in a hurry, and never persuaded that this one time is fine.


The summary, if you skipped here: local tools catch local mistakes, and assistants mostly don't make local mistakes. They do make some, and the ones they make are legal-but-wrong, a quality constant set to 30 or a decode with no size bound, so they miss every syntax rule and hit only policy rules, which is to say rules you had to write. Move your rules up a level, to the repository for structure and to mutation for tests, and keep human attention for the one level that can't be automated.

If you run the thirty-second check on your own test suite, I'd like to hear what came back. That number is the most interesting thing in this article and I only have my own.

Top comments (0)