Add lint, formatting and static analysis; move the toolchain to Java 25 #5
Merged
JMR-dev
merged 11 commits from 2026-08-22 21:22:40 +00:00
chore/lint-and-format-parity into main
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  JMR-devandClaude Opus 5
|
39e0900928 |
Adapt LibreMail's emulator instrumentation for the E2E matrix
The E2E legs could fail with almost nothing to show for it. The previous handler
was a single line of semicolons printing meminfo and 60 lines of crash logcat,
and it only ran when gradle RETURNED non-zero -- a hang left nothing at all, and
`adb logcat -d` at the end only holds whatever survived in the ring buffer, which
a chatty run evicts.
The two failure shapes want different evidence, so they are handled separately:
FAILED -- gradle returned non-zero. The test reports already say which test and
why, so this captures the surrounding state: guest memory and
storage, whether the app even installed, native crashes, and the
runner's own kvm/memory/disk.
WEDGED -- gradle never returned and the wrapper timeout killed it. There are no
reports, so the evidence has to come off the live device: which test
was in flight per the TestRunner logcat, whether the binder services
are published, and SIGQUIT thread dumps of both processes. That last
one is the point -- ART writes full stacks to logcat and /data/anr,
which is what separates a deadlocked test from a stuck MediaCodec
from a device that stopped answering. dumpsys media.player is in
there because both engines transcode through MediaCodec, so a hung
conversion shows up in it.
Logcat is now streamed to a file from the start of the step and uploaded whichever
way the leg goes, since the leg worth reading is usually the one that went red once
and green on re-run -- by which time the emulator is gone.
It is a script rather than inline YAML because it has to be. The action splits its
`script` input on newlines and runs each line as its own `sh -c`, so functions and
`if` blocks cannot survive there; that constraint is what produced the one-line
handler in the first place. One line calls the script now.
The wrapper timeout is 1200s against measured ~5-minute healthy legs, so it cannot
trip on a slow-but-working run, and sits far enough under the 60-minute cap to
leave room for the capture. It wraps only the foreground gradle client, never the
emulator the action owns, so it cannot hang the leg itself.
Not adopted from LibreMail: the hand-provisioned AVD boot, its SDK-integrity
installer and its focus gate. Those answer failures this repo has not had, and
replacing a boot path that works to fix problems we do not have is how a working
matrix breaks. Every emulator setting here -- ram-size, disk-size, the ABI filter,
swiftshader -- is untouched, along with the reasoning already written next to it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
3c7b4b9063 |
Stop the prerelease guard from rejecting AGP's own test platform
All four E2E legs failed to resolve :app:connectedDebugAndroidTest: Could not find com.google.testing.platform:android-device-provider-local:0.0.9-alpha04 That is AGP's Unified Test Platform, the thing that actually runs instrumented tests, and the guard added two commits ago was rejecting it. The guard was written as a blanket rule over every configuration, and AGP resolves its own tooling through this project's configurations. There is no stable version to move to, and there never has been: every module in com.google.testing.platform has only -dev and -alpha releases, going back to 0.0.1-dev. Nor is the version ours to choose -- AGP 9.3.1 pins it through com.android.tools.utp:android-test-plugin-host-additional-test-output:32.3.1. So an allowlist entry would have been the first of several, one per prerelease tool AGP happens to depend on, discovered one red matrix at a time. Scoping the guard to the groups this project actually floats fixes the class rather than the instance. It also retires the detekt exception: we do not float dev.detekt, so the guard now has no opinion about it, where before it passed only because "alpha.6" has a dot the pattern missed. Worth naming the shape of this bug, because it is the second time in this branch that a change looked complete locally and was not. Nothing that runs without a device resolves the UTP configurations -- unit tests, ktlint, detekt, lint, assembleDebug and assembleRelease were all green while connectedAndroidTest could not resolve at all. Verified now by resolving that exact configuration directly, and by confirming the guard still does its job: lifecycle 2.+ resolves to 2.11.0, not 2.12.0-alpha01. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
8727fccba1 |
Declare read-only permissions on the status-check workflow
CodeQL flagged the new static-analysis job for relying on the repository's default GITHUB_TOKEN scope. Fair, and the repo already holds the opposite opinion elsewhere: build.yml's release job spells out contents: write with a comment saying the token's reach should be visible at the point of use. Set at workflow level rather than on the one job that was flagged, because none of these four write anything -- they read the code, build it and attach reports. It also means a repository default that widens later cannot quietly widen these jobs with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
fd6e5325cf |
Raise Kotlin to 2.4.10 so the bytecode can join the toolchain on Java 25
The previous commit settled for Java 24 everywhere because Kotlin 2.2.10 refuses jvmTarget 25. That was the wrong constraint to accept, for two reasons. The first is that 24 turned out to be unbuyable. Adoptium's repository carries 8, 11, 17, 21, 25 and 26 -- no 24, because it is a non-LTS that went end of life in July 2025. The builds passed only because Gradle quietly auto-provisioned 24.0.2+12 through foojay, and .idea/misc.xml had been pointed at a temurin-24 that cannot be installed. A toolchain nobody can install is not pinned, it is lucky. The second is that the cap was never on the toolchain at all. Kotlin's ceiling applies to jvmTarget -- the bytecode -- and the JDK running the build is a separate axis. Conflating them is what steered this at 24 in the first place. So the fix is the one the sibling repo already uses: put KGP on the root buildscript classpath, where AGP's built-in Kotlin picks it up instead of the 2.2.10 it bundles. Kotlin 2.4.10 supports jvmTarget through 26, which lifts the ceiling above the toolchain rather than under it. The Compose compiler plugin is versioned in lockstep and reads the same catalog entry, so the two cannot drift, and the module now applies both by id() because they come from the classpath rather than from plugin resolution. Checked rather than assumed, since a silent downgrade would look identical to success: compiled classes report major version 69, which is Java 25. D8 dexes them, R8 minifies them, and ktlint, detekt, lint, the unit tests and the androidTest compile are all green on top. 25 is the right landing place independent of all this: it is LTS, it is in the Adoptium repository, and temurin-25-jdk is already installed here -- so the daemon runs on a real system JDK rather than a provisioned copy of an unpatched one. Two catalog plugin aliases went with it. android-application and kotlin-compose now resolve from the buildscript classpath, so leaving aliases behind would have left two entries that read like the source of truth and control nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
a7aa003020 |
Fix three ways the floating versions could have gone wrong quietly
All three shared a failure mode: the build stays green while doing something other than what the config says. composeBom was "2026.+". The Compose BOM numbers as YYYY.MM.PP, so the year is the major -- that float stops finding releases on 1 January 2027 and keeps building happily against a frozen BOM, with nothing in CI or the diff to say so. Bare "+" now, which is safe only because the prerelease guard is there. smart-exception was floating on "0.+". Under semver a 0.x minor may break, and this library is load-bearing precisely where breakage hides: the ffmpeg-kit wrapper reaches for smartexception.java.Exceptions only when a call FAILS, so a moved class shows up as an R8 missing-class error at release, or as a crash on the error path -- the least-exercised code in the app, by its own comment. Pinned, with that written down. It was noticed while the float was being written and shipped anyway, which is the actual mistake here. The prerelease guard permitted detekt's alpha by accident. The pattern wanted digits straight after the marker word, and detekt reads "2.0.0-alpha.6" with a dot -- so it passed on punctuation. Had it read "alpha6" the build would have broken with no way to see why from the config. There is now an explicit prereleasePermitted set, and the pattern tolerates both spellings, so the exemption is a decision instead of a coincidence. Also corrects a comment that was confidently wrong: componentSelection rejects STATIC prerelease versions too, not only floating ones. Naming "2.12.0-alpha01" in the catalog does not get you that alpha, it fails to resolve -- verified, not assumed, because the obvious guess is the opposite. Prereleases are taken by adding the group to prereleasePermitted. Two stale references to Gradle 9.5 updated to 9.7.1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
e9542d2223 |
Let the libraries float on minor and patch
Library versions now read "1.+" instead of "1.19.0". Three groups stay pinned, and the reasons differ: agp/kotlin/ksp are version-locked to each other -- AGP 9.3.1's POM declares kotlin-gradle-plugin 2.2.10, so a float that picked up Kotlin 2.4.x would put the Compose compiler ahead of the Kotlin AGP actually compiles with. ktlint/detekt/jacoco because a linter is not a library. A library bump that misbehaves usually still compiles; a new lint rule makes files nobody touched stop passing, turning a PR red for something absent from its diff. Upgrading those is worth a commit that reads the new findings. The FFmpeg AAR is a committed file, not a coordinate. The componentSelection block is the part that makes this safe rather than the part that makes it work. Gradle resolves "+" to the highest version it can find and does not skip prereleases, and androidx routinely publishes alphas numbered above the current stable: lifecycle 2.12.0-alpha01, work 2.12.0-rc01, navigation 2.10.0-rc01, datastore 1.3.0-alpha10, annotation 1.11.0-alpha01 all outrank the releases this app uses. Without the guard, five dependencies would have moved onto unreleased code on the next build with nothing in the diff to say so. With it, every float resolves to exactly the version that was pinned before -- checked against :app:dependencies, not assumed. So this changes nothing today. Every library was already at its newest stable when the catalog was audited; floating is about what happens next month, not this commit. Trying a prerelease is still possible: name the exact version, which pins it rather than floating it. That is the right way round -- an alpha should be a deliberate act with a version number attached to it. Verified: ktlint, detekt, lint, unit tests, androidTest compile, assembleDebug all green, and the configuration cache still reuses across runs of the same task set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
3841f58c74 |
Put the whole toolchain on Java 24, and take Gradle to 9.7.1
Java was scattered across four numbers that nobody had chosen together: the
daemon ran on 25 (pinned in gradle-daemon-jvm.properties), CI installed 17, the
IDE was set to 25, and the app compiled to 17 bytecode. Now all four say 24.
24 rather than 25 because 25 is not reachable end to end. Kotlin 2.2.10 refuses
jvmTarget 25 outright -- "available targets are 1.8 ... 23, 24" -- so the app's
bytecode could never have joined a 25 toolchain, and "everything on the same
version" would have stayed false in the one place it is hardest to notice. 24 is
the highest number all four can actually hold. Checked, not assumed: D8 dexes
Java 24 class files, and R8 full mode minifies them, so the shipped artifact
builds on this too.
Floating where floating is native:
- java-version: '24' -- setup-java resolves the newest 24.x at run time.
- toolchainVersion=24 -- Gradle reports it as "Compatible with Java 24, any
vendor", and provisions whatever 24.x it finds or downloads.
The Gradle wrapper deliberately does NOT float, because it cannot: distributionUrl
names one archive and distributionSha256Sum is the checksum of that exact file.
That pairing is the wrapper's integrity check, and it is the same reasoning the
workflows already apply to action SHAs. Set via `./gradlew wrapper`, not by hand,
so the checksum matches the URL.
Dependencies were audited against Google Maven and Maven Central rather than
guessed at, and almost everything was already current: AGP, the Compose BOM,
core-ktx, activity, lifecycle, navigation, work, datastore, media3, room,
documentfile, annotation, espresso, androidx-junit, junit and ktlint are all at
their newest stable. Only two had moved -- detekt to 2.0.0-alpha.6 and JaCoCo to
0.8.15 -- and both are here.
Kotlin stays at 2.2.10 and that is now recorded as a verified fact rather than a
warning: the AGP 9.3.1 POM declares kotlin-gradle-plugin 2.2.10 at runtime scope,
which is what AGP's built-in Kotlin actually compiles with. Android lint suggests
2.4.10 and taking that suggestion breaks the build unless KGP is also forced onto
the root buildscript classpath. agp, kotlin and ksp move together or not at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
f4962913e1 |
Correct the JDK claim, and finish the @UnstableApi propagation
Two things the first pass got wrong. CLAUDE.md said "use a JDK 17-21, AGP 9 does not support 25+". That was carried over from the sibling repo and is not true here: gradle-daemon-jvm.properties pins toolchainVersion=25, so Gradle provisions and runs the daemon on Java 25 whatever JAVA_HOME says -- JAVA_HOME only picks the launcher. `gradlew --version` prints both, and shows them differing on this machine right now. It also means CI's java-version: '17' is not the JDK that compiles anything, and that the daemon JVM is the same on a laptop as on a runner, which is a better guarantee than the one the file claimed. Marking ConversionDependencies @UnstableApi propagates to its callers, and FakeFailures in androidTest calls it. That is a warning rather than an error in Kotlin, and lint does not read the androidTest source set, so the previous commit compiled clean while leaving one file inconsistent with the very pattern it described. Marked now. Left alone deliberately: gradlew.bat. The new `*.bat text eol=crlf` attribute governs how it is checked out from here on, which is the point of adding it, and the file already has CRLF in both the tree and the index. Rewriting the stored bytes of the wrapper script to prove the attribute works is not this branch's business. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
65a94b4ec1 |
Clear the 35 findings the new tools reported
detekt found 29 and Android lint 6, on a codebase neither had ever seen. Each
one was either fixed or relaxed with the reason written next to it; nothing was
suppressed to make the build quiet.
Fixed, because the tool was right:
- ConversionDependencies constructs Media3Engine, which is @UnstableApi, and
was not marked. Every other type here that touches Media3 propagates the
marker rather than swallowing it with @OptIn, so this one does too. Lint
was the only thing that had ever noticed.
- MediaProbe converted microseconds to milliseconds with a bare 1000, twice,
in a file that also handles a seconds-based duration from a different API.
US_PER_MS and MS_PER_SECOND now say which is which -- that confusion is a
real bug source in media code, not a style question.
- Foreground service types compared SDK_INT against 34 and 35 as raw ints
while the doc comment above spelled the version names out. VERSION_CODES
says it in the code.
- take(3) is a product decision about how many alternatives an error offers.
It means nothing until it is named; MAX_SUGGESTIONS does.
- setProgress(100, ...) is a percentage max, now PERCENT_MAX.
Relaxed, because the rule did not fit:
- The model package is excluded from ReturnCount and CyclomaticComplexMethod
ONLY. It is the decision layer: ConversionRouter.route scores 17 because
the app can give 17 distinct answers to "which engine, and why", each with
its own user-visible reason, and route's own comment records that their
ORDER decides which message is shown. Counting those as complexity measures
how many answers exist, not how hard the code is to follow. Everything else
-- LongMethod, NestedBlockDepth, ComplexCondition -- still applies there.
- A flat `when` used as a lookup table scores a point per entry, so
MediaProbe's demuxer-name to Container map read as complexity 21 with no
nesting and no state. ignoreSingleWhenExpression is the rule's own answer.
- TooGenericExceptionCaught off. MediaProbe, ConversionWorker and ConcatWorker
sit in front of native code that reports a malformed file as anything from
IllegalArgumentException to a bare RuntimeException, undocumented.
Enumerating that list means guessing, and a wrong guess crashes the app on a
file it could have reported as unreadable. SwallowedException stays on, so
these still have to log and handle.
- SI thresholds in the byte formatter, via ignoreNumbers. Each literal sits on
the line with the unit string it belongs to; BYTES_PER_MB would need a Long
and a Double and say nothing the line does not.
- allowedFunctionsPerObject, which the first pass simply missed.
Lint's three version-freshness nags are off. They do not describe this code --
they go red the day someone else publishes a release, which turns a PR red for
something its author cannot see in their diff, and they want the network at
lint time. Upgrades here are deliberate; Kotlin in particular is pinned to AGP's
bundled KGP and is not free to follow the newest release.
UsableSpace is informational rather than disabled, because it is a real finding
that this commit is choosing not to act on. hasSpaceFor reads File.usableSpace,
which ignores reclaimable cache, so the app can refuse a conversion it had room
for. StorageManager.getAllocatableBytes is the better answer, but it changes
when a job is rejected and can throw -- a behaviour change to a safety check,
which deserves its own commit and its own test rather than a drive-by here.
informational keeps it in every lint report instead of hiding it.
Also adds the CI gate and a CLAUDE.md. Coverage is reported and not gated: the
measured baseline is 31% of lines, which is exactly why LibreMail's 0.84 floor
was evidence about LibreMail and not a number to copy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
496f1c7e73 |
Apply ktlintFormat
Tool output only, no hand edits, so this is safe to read with whitespace diffing off. It is its own commit for exactly that reason: a whole-repo reformat folded into the commit that configured the formatter would have made both unreviewable. What it did, mostly: trailing commas on wrapped argument lists, signatures collapsed onto one line where they fit inside 120 columns, import order, and four genuinely unused imports removed. Unit tests pass unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
93b1fbd5a9 |
Give this project the lint and formatting setup LibreMail already has
There was none: no .editorconfig, no static analysis, and CI ran only tests. Style was whatever the IDE happened to do, which is fine until two of them disagree. The split is LibreMail's, because it is the one that avoids arguments between tools: ktlint owns formatting, detekt owns static analysis with its formatting ruleset left off. Neither can contradict the other about the same line. Adapted rather than copied. LibreMail is Gradle 9.6 / JDK 21 with the configuration cache off; this is Gradle 9.5 / JDK 17 with it on, and Kotlin lives under src/main/java rather than src/main/kotlin -- so its plugin versions were evidence, not proof. Verified here before committing: both plugins resolve, ktlint reads src/main/java, and the run stores a configuration cache entry rather than tripping over it. detekt.yml carries only what applies. The Compose relaxations transfer intact -- a @Composable function is legitimately long, PascalCase, and full of dp literals no matter which app it is in. LibreMail's ForbiddenImport guard and its LargeClass exclusions do not: they name an AppLog facade and two test files that exist over there and nowhere here, and config that guards nothing is worse than no config, because the next reader has to work out that it is dead. detekt 2.0 is an alpha. That is not a preference: stable 1.23.x stops at Gradle 8.12 and this project is on 9.5, so there is no other line to be on. Coverage is reported, not gated. A floor needs a measured baseline, and the JVM test stack here is still junit-only -- a number picked before measuring would either fail on day one or mean nothing. Android lint gets warningsAsErrors because the other two tools fail on any finding, and a gate that stays green while its report fills up is not a gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |