diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-10-31 18:24:45 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-10-31 18:24:45 +0000 |
| commit | 5664a342960151be9ebecf5d36119f464f17cc32 (patch) | |
| tree | 11d0f5e179eb7c170cd301b7374280de7ed73afc | |
| parent | 3cba8487e76694da4f70bb4942faa7671d3e8b46 (diff) | |
Close the reachability guard's blind spot
It matched `fun` at four spaces, so every `internal fun` on a helper object was
invisible to it -- which is most of what this app's UI layer exposes to its own
module. The guard that exists because findFeedLink and QueueRepository.advance
were written, tested, documented and never called could not see that whole
category.
It also credited only cross-file callers, so a helper called by its own object's
public entry point looked dead and needed an allowlist entry explaining that it
was not. Same-file callers now count, with declarations stripped first so a
function cannot find itself -- the trick the DAO half of this test already used.
Both together: a wider net and one fewer exception. The allowlist is down to a
single entry.
Watched failing on a deliberately dead `internal fun` before being kept, and
watched passing with the entry removed -- so the widening caught nothing that
was actually live.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
| -rw-r--r-- | app/src/test/java/org/terst/listen/ReachabilityTest.kt | 39 |
1 files changed, 28 insertions, 11 deletions
diff --git a/app/src/test/java/org/terst/listen/ReachabilityTest.kt b/app/src/test/java/org/terst/listen/ReachabilityTest.kt index ec603fa..e1e93b3 100644 --- a/app/src/test/java/org/terst/listen/ReachabilityTest.kt +++ b/app/src/test/java/org/terst/listen/ReachabilityTest.kt @@ -27,22 +27,31 @@ class ReachabilityTest { "YouTubeAudio.ytDlpVersion" to "Called from YouTubeAudioTest, which fails on a stale bundled yt-dlp. That guard " + "is the whole point; it has no job in production.", - "SkipSummary.describeKind" to - "Only called from SkipSummary.breakLines, in the same file, since the episode " + - "screen's break-list labels and its ad/furniture colour split (FIX 3) were " + - "deliberately decoupled -- the colour split now uses isAdvertising, not this " + - "display string. This heuristic only credits a cross-file call as a caller, so " + - "it cannot see that breakLines is reached from EpisodeScreen.kt via " + - "SkipSummary.episode(); the function is genuinely live, just one hop further " + - "than the check looks.", ) private val mainDir = File("src/main/java/org/terst/listen") private val objectDecl = Regex("""^(?:internal )?object (\w+)""", RegexOption.MULTILINE) - /** Top-level members of an object are indented exactly four spaces. */ - private val memberFun = Regex("""^ (?:suspend )?fun (\w+)""", RegexOption.MULTILINE) + /** + * Top-level members of an object are indented exactly four spaces. + * + * `internal` is included because it was the hole: every `internal fun` on an object was + * invisible to this check, which is most of what the UI layer exposes to its own module. + * `private` is left out -- the compiler and detekt already refuse an unused one. + */ + private val memberFun = Regex( + """^ (?:internal )?(?:suspend )?fun (\w+)""", + RegexOption.MULTILINE, + ) + + /** + * A declaration, so a function is not credited as its own caller. + * + * The same trick the DAO check uses. Without it, scanning a file for `name(` always finds + * `fun name(` and nothing is ever dead. + */ + private val anyDeclaration = Regex("""(?:internal |private )?(?:suspend )?fun (\w+)\(""") /** Any declaration starting at column zero ends the preceding object's body. */ private val nextTopLevel = Regex( @@ -74,9 +83,17 @@ class ReachabilityTest { // method reference such as `map(LibraryTree::subscription)`, which has no parens at // all and was reported as dead while being called. .filterNot { qualified -> + val bare = qualified.substringAfter(".") val reference = qualified.replace(".", "::") bodies.any { (other, b) -> - other != file && (b.contains("$qualified(") || b.contains(reference)) + if (other != file) { + b.contains("$qualified(") || b.contains(reference) + } else { + // Same file: a helper called only by its own object's public entry + // point is live, and used to need an allowlist entry saying so. + // Declarations are stripped first or every function finds itself. + anyDeclaration.replace(b, "DECL(").contains("$bare(") + } } } .toList() |
