android: restore the dismiss I deleted, and teach the checker to see it
`785ebdb` failed at compileDebugKotlin with two `Unresolved reference 'dismiss'`. Splitting the reminder notification code into its own object, I removed `dismiss` from `Reminders` and never pasted it into `ReminderNotification`. The call sites were correctly qualified; the function simply was not there. All four local gates passed it, and `check-symbols.py` passed it for a reason it documented about itself: it only resolved the LEADING segment of a dotted expression, because that is the part a regex can resolve. `ReminderNotification` existed, so `ReminderNotification.dismiss(...)` looked fine. That was a real gap rather than an inherent one, so the checker now indexes the members of every `object` declared in the package and verifies `Foo.bar` against them. Brace-counted, not regex-matched — an object body is full of nested braces from lambdas and apply blocks, and no regex closes correctly over them. Verified by deleting `dismiss` from a copy of the tree again: it reports the same two call sites the Kotlin compiler did. What it still cannot see is narrowed and written down rather than left implied — members of anything declared outside this package, members reached through a variable rather than a type name, and every question about types.
This commit is contained in:
@@ -78,6 +78,12 @@ internal object ReminderNotification {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Take a reminder off the shade, once it has been acted on. */
|
||||||
|
fun dismiss(
|
||||||
|
context: Context,
|
||||||
|
noteId: String,
|
||||||
|
) = NotificationManagerCompat.from(context).cancel(noteId.hashCode())
|
||||||
|
|
||||||
private fun openIntent(
|
private fun openIntent(
|
||||||
context: Context,
|
context: Context,
|
||||||
note: Note,
|
note: Note,
|
||||||
|
|||||||
@@ -12,6 +12,16 @@ 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
|
other local gate. It errs toward false positives — anything it cannot account
|
||||||
for is reported rather than assumed fine.
|
for is reported rather than assumed fine.
|
||||||
|
|
||||||
|
It also checks MEMBERS of this package's own `object` declarations — `Foo.bar()`
|
||||||
|
where `Foo` is an object declared here. That case was added after moving a
|
||||||
|
function between two objects and forgetting to paste it into the second: the
|
||||||
|
call site read `Other.thing()`, resolved fine as far as the leading token, and
|
||||||
|
failed in CI (785ebdb).
|
||||||
|
|
||||||
|
Still NOT caught, so a clean run is not over-read: members of anything declared
|
||||||
|
outside this package, members reached through a variable rather than a type
|
||||||
|
name, and every question about types. Those are what `compileDebugKotlin` is for.
|
||||||
|
|
||||||
python3 android/tools/check-symbols.py [source-root]
|
python3 android/tools/check-symbols.py [source-root]
|
||||||
|
|
||||||
Exits non-zero when something is unaccounted for.
|
Exits non-zero when something is unaccounted for.
|
||||||
@@ -69,6 +79,44 @@ def strip(src: str) -> str:
|
|||||||
return src
|
return src
|
||||||
|
|
||||||
|
|
||||||
|
def object_members(src: str) -> dict:
|
||||||
|
"""Map each `object Foo` declared here to the names declared directly in it.
|
||||||
|
|
||||||
|
Brace-counted rather than regex-matched: an object body contains nested
|
||||||
|
braces (lambdas, apply blocks, companions) and no regex closes correctly over
|
||||||
|
them. Only top-level members count — anything nested deeper is not reachable
|
||||||
|
as `Foo.member` anyway.
|
||||||
|
"""
|
||||||
|
members = {}
|
||||||
|
for match in re.finditer(r"^(?:internal |private )?object (\w+)\s*\{", src, re.M):
|
||||||
|
name = match.group(1)
|
||||||
|
depth = 0
|
||||||
|
body_start = match.end() - 1
|
||||||
|
for i in range(body_start, len(src)):
|
||||||
|
if src[i] == "{":
|
||||||
|
depth += 1
|
||||||
|
elif src[i] == "}":
|
||||||
|
depth -= 1
|
||||||
|
if depth == 0:
|
||||||
|
break
|
||||||
|
body = src[body_start + 1 : i]
|
||||||
|
own = set()
|
||||||
|
depth = 0
|
||||||
|
for line in body.splitlines():
|
||||||
|
if depth == 0:
|
||||||
|
decl = re.match(
|
||||||
|
r"\s*(?:@\w+\s+)*(?:public |private |internal |protected )?"
|
||||||
|
r"(?:const |lateinit |inline |suspend )*(?:fun|val|var)\s+"
|
||||||
|
r"(?:<[^>]*>\s*)?(\w+)",
|
||||||
|
line,
|
||||||
|
)
|
||||||
|
if decl:
|
||||||
|
own.add(decl.group(1))
|
||||||
|
depth += line.count("{") - line.count("}")
|
||||||
|
members[name] = own
|
||||||
|
return members
|
||||||
|
|
||||||
|
|
||||||
def main() -> int:
|
def main() -> int:
|
||||||
root = sys.argv[1] if len(sys.argv) > 1 else DEFAULT_ROOT
|
root = sys.argv[1] if len(sys.argv) > 1 else DEFAULT_ROOT
|
||||||
files = [
|
files = [
|
||||||
@@ -79,6 +127,7 @@ def main() -> int:
|
|||||||
]
|
]
|
||||||
|
|
||||||
declared = collections.defaultdict(set)
|
declared = collections.defaultdict(set)
|
||||||
|
objects = {}
|
||||||
parsed = {}
|
parsed = {}
|
||||||
for path in files:
|
for path in files:
|
||||||
with open(path, encoding="utf-8") as fh:
|
with open(path, encoding="utf-8") as fh:
|
||||||
@@ -86,6 +135,7 @@ def main() -> int:
|
|||||||
package = re.search(r"^package\s+([\w.]+)", raw, re.M).group(1)
|
package = re.search(r"^package\s+([\w.]+)", raw, re.M).group(1)
|
||||||
src = strip(raw)
|
src = strip(raw)
|
||||||
parsed[path] = (package, src, raw)
|
parsed[path] = (package, src, raw)
|
||||||
|
objects.update(object_members(src))
|
||||||
for match in DECL.finditer(src):
|
for match in DECL.finditer(src):
|
||||||
declared[package].add(match.group(1))
|
declared[package].add(match.group(1))
|
||||||
# Enum entries are declarations too; DECL only sees the class itself.
|
# Enum entries are declarations too; DECL only sees the class itself.
|
||||||
@@ -119,6 +169,18 @@ def main() -> int:
|
|||||||
print(f"{os.path.relpath(path, root)}:{line}: unresolved '{name}'")
|
print(f"{os.path.relpath(path, root)}:{line}: unresolved '{name}'")
|
||||||
problems += 1
|
problems += 1
|
||||||
|
|
||||||
|
# Members of objects declared in this package.
|
||||||
|
for match in re.finditer(r"(?<![\w.])([A-Z][A-Za-z0-9_]*)\.(\w+)", src):
|
||||||
|
owner, member = match.group(1), match.group(2)
|
||||||
|
if owner not in objects or member in objects[owner]:
|
||||||
|
continue
|
||||||
|
line = src[: match.start()].count("\n") + 1
|
||||||
|
print(
|
||||||
|
f"{os.path.relpath(path, root)}:{line}: "
|
||||||
|
f"'{owner}' has no member '{member}'"
|
||||||
|
)
|
||||||
|
problems += 1
|
||||||
|
|
||||||
print(f"\n{len(files)} files, {problems} unresolved")
|
print(f"\n{len(files)} files, {problems} unresolved")
|
||||||
return 1 if problems else 0
|
return 1 if problems else 0
|
||||||
|
|
||||||
|
|||||||
+6
-2
@@ -219,8 +219,12 @@ python3 android/tools/check-strings.py
|
|||||||
```
|
```
|
||||||
|
|
||||||
`check-symbols.py` flags any capitalised identifier that is neither imported,
|
`check-symbols.py` flags any capitalised identifier that is neither imported,
|
||||||
declared in the same package, a type parameter, nor implicitly available. Not a
|
declared in the same package, a type parameter, nor implicitly available — and
|
||||||
type checker — `compileDebugKotlin` in CI remains the only real one, and it is
|
members of this package's own `object` declarations, so that `Foo.bar()` fails
|
||||||
|
here when `Foo` has no `bar`. That second case exists because moving a function
|
||||||
|
between two objects and forgetting to paste it into the second cost a red run
|
||||||
|
(785ebdb): the call site was correctly qualified and every other gate passed.
|
||||||
|
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
|
also the ONLY lane that type-checks at all, since there is no Android SDK on the
|
||||||
workstation.
|
workstation.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user