Skip to content

Add Kotlin/XML formatting and static analysis to CI #6918

Description

@chrisdebian

Summary

This repo currently has no automated formatting or static-analysis tooling: no ktlint, no Detekt, no ktfmt/Spotless, no .editorconfig anywhere in the tree, and .github/workflows/android.yml only runs unit tests + APK assembly — no lint/format step. CONTRIBUTING.md doesn't document a style convention either, so there's currently nothing enforcing or even describing consistent code style.

This surfaced concretely while reviewing #6396: a hunk in MediaClient.kt looked like it had an unbalanced brace from the diff alone. Tracing it by hand confirmed the braces do balance, but the new code is nested one level deeper than its surroundings without being re-indented, which is what made it misleading to read. A formatter would have caught or auto-fixed this for free, before it ever reached review.

Repo composition

By bytes (GitHub language stats):

Language Bytes Share
Kotlin 3,380,636 ~97%
Java 102,164 ~3%
Shell 1,449 <1%

By file count (more relevant for tooling scope — XML is byte-light but file-heavy):

Extension Files
.kt 765
.xml 644
.java 23
.yml 11
.kts 3
.sh 2

Proposed tooling, by language

Language Tool Why
Kotlin (primary) ktlint (org.jlleitschuh.gradle.ktlint) Formatting/style enforcement. Widely used in Kotlin/Android projects.
Kotlin Detekt Static analysis — complexity, code smells, and (via a configurable ForbiddenMethodCall rule) can catch things like blocking calls in production code.
Android XML (644 files) xmllint Already proposed independently in #6824 — worth landing that rather than duplicating it here.
Kotlin/Android (all) Android Lint Deprecated-API/resource/manifest checks — available via ./gradlew lint but not currently wired into CI; worth confirming whether that's deliberate.
Java (23 files, shrinking) (none proposed) Footprint too small to justify a separate tool; better left to attrition as files migrate to Kotlin.
Shell (2 files) shellcheck Trivial to add, catches real bugs, near-zero cost for 2 scripts.

Proposed way forward

Phased so nothing blocks on a disruptive mass-reformat:

  1. Add .editorconfig — cheap, tool-agnostic, and ktlint reads it directly for its own config.
  2. Add ktlint, generate a baseline against the current ~765 Kotlin files (grandfathers existing style violations; only new/touched code has to be clean going forward).
  3. Wire ktlintCheck into android.yml as a required step.
  4. Add Detekt the same way, with its own baseline.
  5. Confirm whether Android Lint should be wired into CI, or if that's intentionally left manual.
  6. Add shellcheck for the 2 shell scripts.

Happy to help with any of these steps if there's interest — flagging as an issue first given the size of the change relative to a typical bug-fix PR. No urgency; just didn't want the #6396 finding to go unrecorded.

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