Skip to content

Consider adopting detekt for static analysis #106

@dalinaum

Description

@dalinaum

Context

The project currently runs AGP-bundled Android Lint (./gradlew lint) but has no Kotlin-specific static analysis — no checks for code smells, complexity, or Kotlin idiom misuse. Rules like "no new `!!`" and "`@Immutable` on domain models" (documented in CONVENTION.md) are currently enforced by humans during review.

This issue captures the tradeoffs so the decision is recorded and doesn't need to be re-derived.

What detekt catches

  • Function length, parameter count, cyclomatic complexity
  • Empty blocks, unused imports, suspicious naming
  • Kotlin-specific: `with`/`run`/`apply` misuse, deeply nested `when`, `!!` usage
  • Custom rules via Kotlin PSI (e.g., "every domain model must carry `@Immutable`")

Benefits of adoption

  • CONVENTION §2.1 (no new `!!`) becomes CI-enforced instead of reviewer-enforced.
  • CONVENTION §5.1 (`@Immutable` on domain models) can be enforced mechanically via a custom rule.
  • `detekt-baseline.xml` supports incremental adoption — ignore existing findings, block only new ones. This matters for a codebase with any existing tech debt.

Costs and considerations

  1. Noise flood on first run. Default ruleset produces hundreds of warnings. Triage cost is real.
  2. Baseline rot. A baseline with 500 entries that no one pays down defeats the purpose. Requires an operating convention ("new violations: 0; baseline: reduced by N per quarter").
  3. Rule overlap with ktlint. Unused imports, wildcard imports, and some style rules are covered by both. Overlap means duplicate warnings unless explicitly configured.
  4. Generated code must be excluded. Apollo's generated classes and Hilt-generated code trigger false positives — add `excludes` to `detekt-config.yml`.
  5. `@Suppress` abuse risk. Without discipline, developers paper over findings with `@Suppress` rather than fix them. Start with a minimal ruleset the team agrees with.

Recommended sequence if adopted

  1. Add detekt Gradle plugin, run with defaults to inventory current findings.
  2. Pick a minimal ruleset: complexity, function length, `!!` ban, (custom) `@Immutable` rule.
  3. Generate baseline, commit, start enforcing "zero new violations".
  4. Add `./gradlew detekt` to CI.

Synergy with CONVENTION.md

Two CONVENTION rules are currently enforced by reviewer attention and could be mechanized by detekt:

  • §2.1 No new `!!` — detekt has a built-in rule; enable it (error-level) and add to baseline.
  • §5.1 Domain models must be `@Immutable` — would require a custom rule using Kotlin PSI, targeting classes under `domain/model/**`.

The first is quick to turn on. The second is a ~half-day custom rule investment but eliminates an entire class of Compose-stability regressions going forward.

Revisit triggers

Deferring is reasonable now. Revisit if any of these hold:

  • Team size grows beyond 1–2 active contributors
  • A `!!` slips past review into main (exposes the need for mechanical enforcement)
  • `@Immutable` is omitted on a new domain model and causes a measurable Compose regression
  • External contributors start sending PRs

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions