android: the import ktlint and detekt cannot see
`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.
This commit is contained in:
@@ -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
|
||||
|
||||
Executable
+127
@@ -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"(?<![\w.])@?([A-Z][A-Za-z0-9_]*)\b", src):
|
||||
name = match.group(1)
|
||||
if name in known:
|
||||
continue
|
||||
line = src[: match.start()].count("\n") + 1
|
||||
print(f"{os.path.relpath(path, root)}:{line}: unresolved '{name}'")
|
||||
problems += 1
|
||||
|
||||
print(f"\n{len(files)} files, {problems} unresolved")
|
||||
return 1 if problems else 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
+43
-1
@@ -187,7 +187,49 @@ The Rust pin is in LOCKSTEP with `ci-tauri` and `ci-tauri-win`. All three build
|
||||
mismatched Rust minor across the lanes would mean divergent resolution for no
|
||||
reason. Bump the three together or not at all.
|
||||
|
||||
No workflow consumes it yet; the lane arrives with the app skeleton (M12 step 5).
|
||||
## Checking the Kotlin lane before pushing
|
||||
|
||||
Same authorisation and same reasoning as the Rust section below — analyzers, run
|
||||
in the CI image, with the workflow's exact arguments. From `android/`:
|
||||
|
||||
```
|
||||
IMG=git.fabledsword.com/bvandeusen/ci-rust-android:1.97
|
||||
DOCK="docker run --rm --user $(id -u):$(id -g) -e HOME=/tmp -v $PWD:/w -w /w"
|
||||
|
||||
$DOCK $IMG ktlint "app/src/main/**/*.kt"
|
||||
$DOCK $IMG detekt --build-upon-default-config --config config/detekt.yml \
|
||||
--input app/src/main/java
|
||||
```
|
||||
|
||||
`HOME=/tmp` because both tools want a writable home for their caches and
|
||||
`--user` has taken the image's away.
|
||||
|
||||
**Neither of these can see a missing import.** They parse Kotlin without
|
||||
resolving symbols, so a file that cannot possibly compile passes both. That is
|
||||
not a gap to work around — it is what these tools are — but it means a clean
|
||||
local run says nothing about whether the code builds. It cost a red CI run on
|
||||
`750d11d`, where `android.os.Build` was lost in a file split and both analyzers
|
||||
were happy.
|
||||
|
||||
So there is a third local check, covering exactly that one blind spot:
|
||||
|
||||
```
|
||||
python3 android/tools/check-symbols.py
|
||||
```
|
||||
|
||||
It flags any capitalised identifier that is neither imported, declared in the
|
||||
same package, a type parameter, nor implicitly available. Not a type checker —
|
||||
`compileDebugKotlin` in CI remains the only real one, and it is also the ONLY
|
||||
lane that type-checks at all, since there is no Android SDK on the workstation.
|
||||
Run all three before a push that touches Kotlin.
|
||||
|
||||
A caution worth keeping, because it bit twice: a checker of this shape is itself
|
||||
easy to get vacuously right. The first version stripped line comments with
|
||||
`re.sub(r'//.*', src, flags=re.S)`, and DOTALL makes `//.*` swallow each file
|
||||
from its first comment to EOF — so it reported everything clean by examining
|
||||
almost nothing. **Test a checker against a known-bad tree before trusting a
|
||||
green from it**; this one is verified by deleting the `Build` import from a copy
|
||||
of the source and confirming it fails.
|
||||
|
||||
## Checking the Rust lane before pushing
|
||||
|
||||
|
||||
Reference in New Issue
Block a user