From 65d8f5f9c6d11c595512dcff497e29c821585763 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 19 Aug 2026 15:51:12 -0400 Subject: [PATCH] android: the import ktlint and detekt cannot see MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `750d11d` failed CI at `compileDebugKotlin` with `Unresolved reference 'Build'`. `defaultDeviceName()` reads `android.os.Build`, and the import was lost when `SyncPairing.kt` was split out of `SyncScreen.kt`. One line to fix. The interesting part is that ktlint and detekt had both passed it, locally and in CI. Neither resolves symbols — they parse — so a file that cannot compile is indistinguishable to them from one that can. A clean analyzer run is not evidence the code builds, and on this repo `compileDebugKotlin` is the only gate that type-checks at all, since there is no Android SDK on the workstation. So: `android/tools/check-symbols.py`, covering that one blind spot. It flags any capitalised identifier that is neither imported, declared in the same package, a type parameter, nor implicitly available. Not a type checker and not pretending to be — a pre-push filter for the single mistake that survives every other local gate, erring toward false positives. Verified against a known-bad tree rather than trusted on a green: deleting the `Build` import from a copy makes it fail with the same two references the Kotlin compiler reported. That step is not ceremony. An earlier attempt at this check stripped line comments with `re.S`, where `//.*` eats each file from its first comment to EOF — it examined almost nothing and reported everything clean. ci-requirements.md now documents all three Kotlin checks, and its claim that no workflow consumes the Android image yet is gone; the lane has been running since step 5. --- .../fabledsword/thoughtsync/ui/SyncPairing.kt | 1 + android/tools/check-symbols.py | 127 ++++++++++++++++++ ci-requirements.md | 44 +++++- 3 files changed, 171 insertions(+), 1 deletion(-) create mode 100755 android/tools/check-symbols.py diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/SyncPairing.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/SyncPairing.kt index e032cef..651ecdb 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/SyncPairing.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/SyncPairing.kt @@ -1,5 +1,6 @@ package com.fabledsword.thoughtsync.ui +import android.os.Build import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth diff --git a/android/tools/check-symbols.py b/android/tools/check-symbols.py new file mode 100755 index 0000000..f0862dd --- /dev/null +++ b/android/tools/check-symbols.py @@ -0,0 +1,127 @@ +#!/usr/bin/env python3 +"""Flag capitalised identifiers that are neither imported nor declared locally. + +This exists because ktlint and detekt are both structurally blind to it: they +parse Kotlin without resolving symbols, so a *missing import* is invisible to +them and both pass a file that cannot compile. The first sync-screen push failed +in CI on exactly that (`Unresolved reference 'Build'` — `android.os.Build` was +lost in a file split), after a clean local analyzer run. + +Not a type checker and not trying to be. `compileDebugKotlin` in CI is the real +one; this is a cheap pre-push filter for the single mistake that survives every +other local gate. It errs toward false positives — anything it cannot account +for is reported rather than assumed fine. + + python3 android/tools/check-symbols.py [source-root] + +Exits non-zero when something is unaccounted for. +""" + +import collections +import os +import re +import sys + +DEFAULT_ROOT = os.path.join( + os.path.dirname(os.path.abspath(__file__)), "..", "app", "src", "main", "java" +) + +# Available without an import: kotlin.* and kotlin.collections.*, plus +# java.lang.* which Kotlin/JVM also imports by default. +IMPLICIT = set( + """ +String Int Long Short Byte Boolean Char Float Double Unit Any Nothing Number +UInt ULong UShort UByte Array List Set Map MutableList MutableSet MutableMap +Collection Iterable Iterator Sequence Pair Triple Comparable Comparator +Throwable Exception RuntimeException IllegalArgumentException IllegalStateException +Error Result Regex StringBuilder CharSequence Enum Annotation Function +Deprecated Suppress OptIn JvmStatic JvmField JvmName JvmOverloads Volatile +Synchronized Throws Target Retention Repeatable MustBeDocumented +System Math Object Class Thread Runnable Void Integer Character +StringBuffer +""".split() +) + +# `R` is generated at build time and never imported from the app's own package. +GENERATED = {"R"} + +DECL = re.compile( + r"^\s*(?:@\w+\s+)*(?:public |private |internal |protected )?" + r"(?:expect |actual |external |abstract |final |open |sealed |data |value |" + r"inline |enum |annotation |fun |companion |const |lateinit )*" + r"(?:class|interface|object|typealias|fun|val|var)\s+" + r"(?:<[^>]*>\s*)?([A-Za-z_]\w*)", + re.M, +) + + +def strip(src: str) -> str: + """Blank out comments and string literals. + + Order matters: raw strings before block comments, and line comments must NOT + use DOTALL — `//.*` with re.S eats from the first comment to end of file, + which silently empties the input and makes the whole check pass vacuously. + """ + src = re.sub(r'"""(?:.|\n)*?"""', '""', src) + src = re.sub(r"/\*(?:.|\n)*?\*/", " ", src) + src = re.sub(r"//[^\n]*", " ", src) + src = re.sub(r'"(?:\\.|[^"\\\n])*"', '""', src) + return src + + +def main() -> int: + root = sys.argv[1] if len(sys.argv) > 1 else DEFAULT_ROOT + files = [ + os.path.join(d, n) + for d, _, names in os.walk(root) + for n in names + if n.endswith(".kt") + ] + + declared = collections.defaultdict(set) + parsed = {} + for path in files: + with open(path, encoding="utf-8") as fh: + raw = fh.read() + package = re.search(r"^package\s+([\w.]+)", raw, re.M).group(1) + src = strip(raw) + parsed[path] = (package, src, raw) + for match in DECL.finditer(src): + declared[package].add(match.group(1)) + # Enum entries are declarations too; DECL only sees the class itself. + for match in re.finditer(r"enum class \w+[^{]*\{([^};]*)", src): + for entry in match.group(1).split(","): + name = entry.strip().split("(")[0].strip() + if re.fullmatch(r"[A-Z]\w*", name): + declared[package].add(name) + + problems = 0 + for path in sorted(files): + package, src, raw = parsed[path] + imported = set() + for match in re.finditer(r"^import\s+([\w.]+)(?:\s+as\s+(\w+))?", raw, re.M): + imported.add(match.group(2) or match.group(1).split(".")[-1]) + # Type parameters are declared inline at their use site. + type_params = set() + for match in re.finditer(r"(?:fun|class|interface)\s*<([^>]*)>", src): + type_params |= set( + re.findall(r"\b([A-Z]\w*)\b(?=\s*(?::|,|$))", match.group(1)) + ) + known = imported | declared[package] | IMPLICIT | GENERATED | type_params + + # Capitalised tokens NOT preceded by a dot: `Icons.Filled` resolves + # through `Icons`, so only the leading segment needs to be accounted for. + for match in re.finditer(r"(?