Files
LibreMediaConverter/config/detekt/detekt.yml
T
JMR-devandClaude Opus 5 97fdc49f01 Guard the native boundary against what it actually throws
D14: picking a file died instead of reporting an unreadable one when
FFmpegKit's native library could not load. `probeWithFFprobe` guarded its
call with `catch (e: Exception)`, and `ConversionViewModel.onInputPicked`
guarded nothing, so the failure escaped a `viewModelScope.launch` -- which
has no handler, and on a device ends the process.

All three of the obvious narrow guards catch nothing, which is why this
needed reading the AAR rather than guessing. `NativeLoader.loadLibrary`
catches the `UnsatisfiedLinkError` that `System.loadLibrary` raises and
rethrows a *bare* `java.lang.Error` wrapping it, so `UnsatisfiedLinkError`
never escapes and the escaping type carries no information at all. Every
touch of the class after the first is a different type again --
`NoClassDefFoundError` -- so a guard written for the first shape lets the
second pick onwards crash, which is the harder half to notice. Both are in
the test output verbatim.

`catch (Throwable)` was the wrong answer for the reason the audit gave: it
would swallow a genuine `OutOfMemoryError` in a method that spawns a native
process, turning "this device is out of memory" into "this file looks
unreadable" and letting the app act on it. So the line is drawn by a named
predicate, `isNativeLoadFailure`, rather than by the catch clause -- every
class-loading shape is a `LinkageError`, and nothing that means the JVM is
failing is one. That disjointness is what makes the guard narrow.

This is consistent with the position `config/detekt/detekt.yml` already
takes for `TooGenericExceptionCaught`: the boundary's failure types are
undocumented, so guessing crashes the app on a file it could have reported.
One level up the opposite mistake is available too, and the predicate is
what lets both be avoided at once.

`TooGenericExceptionThrown` is relaxed for the test source sets only. A test
that reproduces a failed native load has to throw what the library throws,
and a tidier subclass would leave it passing against a defect it no longer
reproduces. Main source is untouched by that and throws nothing generic.

`ConversionDependencies.probe`'s KDoc is rewritten rather than left. It said
this hazard was "deliberately not fixed here ... its own commit, with its
own test", which this is -- leaving it would have replaced one true comment
with a false one, which is the same defect class as the D11 work.

Both halves are covered independently: reverting the `MediaProbe` catch
reds only the two `MediaProbeNativeLoadTest` cases, reverting the ViewModel
guard reds only the two injected-seam cases, and widening the ViewModel
guard to `Throwable` reds the OutOfMemoryError case -- so the narrowness is
pinned, not just the catch.

Audited the sibling boundaries named in the audit and left all three alone:
`FFmpegEngine` and `ConcatEngine` both construct and run under
`catch (e: Throwable)` in their workers, and `Media3Engine` has no native
loader of this kind and already routes failures through `runCatching`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 19:52:55 -05:00

116 lines
5.9 KiB
YAML

# Project overrides merged onto detekt's bundled defaults (buildUponDefaultConfig = true in
# app/build.gradle.kts). Only rules that need tuning for this project appear here.
#
# Guiding principle: Compose UI intentionally breaks several OOP-era metrics, and a few default
# thresholds are stricter than this project's chosen style. Those are relaxed here with a reason.
# Genuine smells (swallowed exceptions, an over-complex condition, a misnamed file) are fixed in
# the code, not silenced.
#
# Formatting is deliberately absent: ktlint owns it. detekt's formatting ruleset is not enabled,
# so the two tools can never disagree about the same line.
complexity:
LongMethod:
# Declarative @Composable functions are read top-to-bottom and are legitimately long.
ignoreAnnotated: ['Composable']
CyclomaticComplexMethod:
# Branchy layout code (when/if inside a UI tree) isn't algorithmic complexity.
ignoreAnnotated: ['Composable']
# A flat `when` used as a lookup table scores one point per entry, so MediaProbe's
# demuxer-name -> Container map reads as complexity 21 while having no nesting, no
# state and nothing to follow. That is the metric measuring table size. A `when` with
# real logic in its branches still counts.
ignoreSingleWhenExpression: true
ignoreSimpleWhenEntries: true
# Same reason the model package is excluded from ReturnCount below: there, one branch
# is one documented outcome. ConversionRouter.route scores 17 because the app can give
# 17 distinct answers to "which engine, and why", not because it is hard to follow --
# it is a flat guard chain with a comment per rule. Only this metric and ReturnCount
# are relaxed for that package; LongMethod, NestedBlockDepth, ComplexCondition and the
# rest still apply there.
excludes: ['**/model/**']
TooManyFunctions:
# Screen files group many small @Composable helpers next to their screen, and the codec /
# container matrix files are intentionally operation-rich cohesive APIs. detekt's default of
# 11 is far too low for either. Files past ~40 functions still flag as genuinely bloated.
ignoreAnnotated: ['Composable']
allowedFunctionsPerFile: 40
allowedFunctionsPerClass: 40
allowedFunctionsPerInterface: 40
# ContainerCapabilities is a single object holding the container x codec matrix and the
# queries over it. Splitting it to satisfy a count would scatter one table across files.
allowedFunctionsPerObject: 40
allowedFunctionsPerEnum: 40
naming:
FunctionNaming:
# @Composable functions are PascalCase by Compose convention.
ignoreAnnotated: ['Composable']
style:
MagicNumber:
# dp / sp / duration literals are idiomatic inline in Compose.
ignoreAnnotated: ['Composable']
ignorePropertyDeclaration: true
ignoreNamedArgument: true
# SI thresholds in the byte formatter. Each literal sits on the same line as the unit
# string it belongs to -- `bytes >= 1_000_000 -> "%.1f MB"` -- so a BYTES_PER_MB pair
# (Long and Double, since the compare and the divide need different types) would add
# six names and say nothing the line does not already say. Counts, limits and tuning
# values still flag; these are unit boundaries.
ignoreNumbers:
- '-1'
- '0'
- '1'
- '2'
- '1e3'
- '1e6'
- '1e9'
- '1_000'
- '1_000_000'
- '1_000_000_000'
ReturnCount:
# Allow guard-clause-style early returns; detekt's default of 2 is overly strict.
max: 4
excludeGuardClauses: true
# The model package IS the decision layer: ConversionRouter picks an engine,
# ContainerCapabilities validates a container x codec pair, the planners pick a
# strategy. Each `return` there is one specific, user-visible reason, and the count
# equals the number of reasons the app can give -- ConversionRouter.route even
# documents that their ORDER is what decides which message the user sees. Collapsing
# them into one exit would bury that. Everywhere else -- UI, workers, engines -- the
# limit still applies.
excludes: ['**/model/**']
LoopWithTooManyJumpStatements:
# Clear early-continue / early-return loops read fine; the default of 1 is strict.
maxJumpCount: 3
ThrowsCount:
# Guard-clause throws don't count; allow a few more for functions validating several
# preconditions.
excludeGuardClauses: true
max: 3
exceptions:
TooGenericExceptionCaught:
# The engine boundaries catch broadly on purpose. MediaProbe drives the platform
# extractor and MediaMetadataRetriever over arbitrary user files; ConversionWorker and
# ConcatWorker wrap Media3 and FFmpeg. All three sit in front of native code that
# reports a malformed file as anything from IllegalArgumentException to
# IllegalStateException to a bare RuntimeException, and the list is not documented.
# Enumerating it would mean guessing, and a guess that is wrong crashes the app on a
# file it could have simply reported as unreadable. Every one of these catches logs
# and handles -- falls back to FFmpeg, or fails the job with a reason -- and the
# SwallowedException rule stays active to keep it that way.
active: false
TooGenericExceptionThrown:
# Still on for main source, which throws nothing generic and should not start.
#
# Relaxed for tests only, and for one reason: a test that reproduces a failed native
# load has to throw what the library actually throws, and FFmpegKit throws a *bare*
# `java.lang.Error` -- `NativeLoader` catches the UnsatisfiedLinkError from
# System.loadLibrary and rethrows `Error(message, cause)`. That is not incidental, it
# is the whole finding `ffmpeg/NativeLoadFailure.kt` exists to handle, and the reason
# the obvious narrower guards catch nothing. Substituting a tidier subclass here would
# leave the test passing against a defect it no longer reproduces.
excludes: ['**/test/**', '**/androidTest/**']