A Portuguese user reported "Skip Intro" rendering in English on Android TV.
The locale files were not the problem - all 22 were structurally complete.
skip_marker_button.dart simply never imported strings.g.dart and assigned
'Skip Intro' / 'Skip Credits' / 'Next Episode' as plain literals. An audit of
lib/ found ~120 more sites in the same state, in four shapes that need
different fixes:
A literal in a file that never imported the i18n layer is the easy one -
skip_marker_button, performance_stats, track_label_builder and codec_utils all
render text with no `t` in the file at all. TrackLabelBuilder._compose now takes
a fallbackLabel builder instead of an English fallbackPrefix, so the caller
supplies t.audioTracks.track / t.videoControls.subtitleTrack and every unnamed
audio and subtitle row in the track menus is localized.
English reaching the user through an exception message is the widest one, and
it needs care: MediaServerException.message feeds both toString() - logs and
Sentry grouping - and verbatim UI display. Localizing it in place would make
bug-report logs follow the user's locale and split one Sentry issue into 22.
The MediaServer and Seerr families instead gain a nullable `display` alongside
the English `message`, and the six screens that print these errors read
`display ?? message`. PlaybackException keeps the opposite rule, because it
already carries a PlaybackFailureReason for logic and classifyPlaybackFailure
already builds it from t.messages: its stragglers are localized at the throw
site. That also removes the literal "Exception: " prefix Live TV users saw on
a tune failure, since PlaybackException.toString() returns the bare message.
Localized parts hand-concatenated with bare English are the shape no search for
Text('...') can find: '${t.common.pause} auto-scroll' on the home carousel,
'${day} at ${time}' on the Live TV schedule row, and an actor-screen count that
hand-rolled its plural as `n == 1 ? 'title' : 'titles'` - wrong for ru and pl
regardless of translation, now a real Slang plural.
Finally a literal assigned to provider state that a widget renders later:
DownloadProgress.errorMessage, and the four background_downloader notification
bodies, which sit inside a plugin config call where no widget-shaped search
reaches them.
Two things surfaced while converting. track_chapter_controls compared a track
label against 'Audio Track N' to swap in a localized version; once the builder
localized its own fallback that branch became unreachable, so it and the
orphaned _joinTrackLabel are gone. And discovery_view's PeerError fallback arm
looks like a leak but is not - its producers already localize, and a test says
so - so it stays as it is.
All 21 non-base locales are translated, including the 21 keys left empty by
earlier commits that were falling back to English. No locale has an empty value.
scripts/check_hardcoded_strings.py guards the three shapes a structural check
can see, and runs in ci_checks.sh after translation hygiene. Its first draft
passed its own tests while missing this very bug, because 'Skip Intro' is bound
to a local rather than handed to Text(); the name-bound rule that closes that
gap is restricted to phrase-shaped literals, or it cannot tell copy from the
identifiers this codebase binds constantly ('cast_row', 'auto', 'liveTv'). It
cannot see English inside a throw or assigned to a provider field - neither is
distinguishable from a log message without dataflow analysis - and the docstring
says so. label: and actionLabel: are deliberately unscanned: here they name a
diagnostic operation, and a check that is chronically red is a check that gets
switched off.
One commit rather than one per area: the keys, the 22 locale files and the
generated output are a single unit, and any partial split fails the repo's own
unused-key scan on the way through.
close #1856
409 lines
14 KiB
Python
Executable File
409 lines
14 KiB
Python
Executable File
#!/usr/bin/env python3
|
|
"""Reject common user-facing English string literals that bypass Slang.
|
|
|
|
This is deliberately a structural check rather than a Dart/dataflow analyzer. It
|
|
cannot see English inside a ``throw`` that a screen later renders, nor a literal
|
|
assigned to a provider field that a widget later renders, because neither is
|
|
distinguishable from a log message without dataflow analysis.
|
|
|
|
Bare ``label:`` and ``actionLabel:`` are deliberately NOT scanned. In this
|
|
codebase they overwhelmingly name a diagnostic operation rather than UI text
|
|
(``_broadcastToDvrs(actionLabel: 'Reload guide', successMessage: t....)``,
|
|
``raceEndpointCandidates(label: ...)``, ``systemShelf(label: 'Failed to ...')``),
|
|
so scanning them yields only false positives, and a check that is chronically
|
|
red is a check that gets switched off. Widget ``label:`` text is still covered
|
|
when it reaches a ``Text`` (rule 1) or mixes with ``t.`` (rule 3).
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import argparse
|
|
import json
|
|
import re
|
|
from dataclasses import dataclass
|
|
from pathlib import Path
|
|
|
|
ROOT = Path(__file__).resolve().parents[1]
|
|
LIB_DIR = ROOT / "lib"
|
|
ALLOWLIST_PATH = Path(__file__).with_name("hardcoded_strings_allowlist.json")
|
|
|
|
_UI_ARGUMENTS = (
|
|
"tooltip",
|
|
"semanticLabel",
|
|
"labelText",
|
|
"hintText",
|
|
"helperText",
|
|
"errorText",
|
|
"dialogTitle",
|
|
)
|
|
_UI_ARGUMENT_RE = re.compile(r"(?<![A-Za-z0-9_])(?:" + "|".join(_UI_ARGUMENTS) + r")\s*:\s*$")
|
|
_TEXT_RE = re.compile(r"(?:\bText|\bSelectableText)\s*\(\s*$")
|
|
_EXCLUDED_ARGUMENT_RE = re.compile(r"(?:debugLabel|fontFamily)\s*:\s*$")
|
|
_KEY_RE = re.compile(r"(?:\bKey|\bValueKey)\s*\(\s*$")
|
|
# `createdLog:`/`log:` name a diagnostic sink, never rendered copy. Matched
|
|
# unanchored because the literal is usually behind a lambda
|
|
# (`createdLog: (playlist) => 'Created ...'`), so the argument name is not
|
|
# the text immediately preceding the literal. The `[a-z]Log` arm keeps
|
|
# innocent names such as `catalog:` out of the exclusion.
|
|
_DIAGNOSTIC_RE = re.compile(
|
|
r"(?:\bappLogger\.|\bSentry\.|\bassert\s*\(|\bthrow\b|(?:\blog|[a-z]Log)\s*:)"
|
|
)
|
|
_TRANSLATION_INTERPOLATION_RE = re.compile(
|
|
r"\$\{\s*(?:t\.|context\.t\b|Translations\.of\s*\()"
|
|
)
|
|
_T_PARAMETER_RE = re.compile(r"(?:\(\s*t\s*\)|\bt)\s*=>[^;]*$")
|
|
# Rule 4 support: a literal bound to a name or returned, rather than handed
|
|
# straight to a widget. This is the shape issue #1856 actually had --
|
|
# `baseButtonText = 'Skip Intro';` a few lines above the `Text(buttonText)`
|
|
# that renders it -- which rules 1-3 structurally cannot see.
|
|
#
|
|
# Restricted to phrase-shaped literals: at least two letter-words separated by
|
|
# whitespace. Without that restriction the rule cannot tell display copy from
|
|
# the identifier strings this codebase binds constantly ('cast_row', 'auto',
|
|
# 'liveTv', 'HDR_UNSUPPORTED'), and it drowns in false positives. The cost is
|
|
# that a bound single-word label ('Software', 'Stereo') slips through; those
|
|
# are indistinguishable from an identifier without dataflow analysis.
|
|
_PHRASE_RE = re.compile(r"[A-Za-z]\s+[A-Za-z]")
|
|
_BOUND_LITERAL_RE = re.compile(r"(?:\breturn|=>|(?<![=!<>+\-*/%&|^~])=)\s*$")
|
|
_RENDERS_UI_RE = re.compile(
|
|
r"(?:\bText|\bSelectableText)\s*\(|(?<![A-Za-z0-9_])(?:"
|
|
+ "|".join(_UI_ARGUMENTS)
|
|
+ r")\s*:"
|
|
)
|
|
_WORD_RE = re.compile(r"[A-Za-z]{3,}")
|
|
_UNITS = {
|
|
"bit",
|
|
"bits",
|
|
"bps",
|
|
"dp",
|
|
"fps",
|
|
"gb",
|
|
"gbps",
|
|
"hz",
|
|
"kb",
|
|
"kbps",
|
|
"khz",
|
|
"mb",
|
|
"mbps",
|
|
"mhz",
|
|
"min",
|
|
"mins",
|
|
"ms",
|
|
"px",
|
|
"sec",
|
|
"secs",
|
|
"sp",
|
|
"tb",
|
|
}
|
|
|
|
|
|
@dataclass(frozen=True)
|
|
class Literal:
|
|
line: int
|
|
value: str
|
|
start: int
|
|
end: int
|
|
raw: bool
|
|
|
|
|
|
@dataclass(frozen=True)
|
|
class Finding:
|
|
path: str
|
|
line: int
|
|
literal: str
|
|
rule: str
|
|
|
|
|
|
def _interpolation_end(source: str, start: int) -> int:
|
|
"""Return the first offset after a balanced ``${...}`` expression."""
|
|
cursor = start + 2
|
|
depth = 1
|
|
while cursor < len(source) and depth:
|
|
if source.startswith("//", cursor):
|
|
newline = source.find("\n", cursor + 2)
|
|
cursor = len(source) if newline < 0 else newline + 1
|
|
continue
|
|
if source.startswith("/*", cursor):
|
|
end = source.find("*/", cursor + 2)
|
|
cursor = len(source) if end < 0 else end + 2
|
|
continue
|
|
raw = (
|
|
source[cursor] in "rR"
|
|
and cursor + 1 < len(source)
|
|
and source[cursor + 1] in "'\""
|
|
)
|
|
quote_start = cursor + 1 if raw else cursor
|
|
if source[quote_start] in "'\"":
|
|
quote_char = source[quote_start]
|
|
quote = (
|
|
quote_char * 3
|
|
if source.startswith(quote_char * 3, quote_start)
|
|
else quote_char
|
|
)
|
|
cursor = quote_start + len(quote)
|
|
while cursor < len(source):
|
|
if not raw and source[cursor] == "\\":
|
|
cursor += 2
|
|
elif source.startswith(quote, cursor):
|
|
cursor += len(quote)
|
|
break
|
|
else:
|
|
cursor += 1
|
|
continue
|
|
if source[cursor] == "{":
|
|
depth += 1
|
|
elif source[cursor] == "}":
|
|
depth -= 1
|
|
cursor += 1
|
|
return cursor
|
|
|
|
|
|
def _string_literals(source: str) -> list[Literal]:
|
|
"""Return Dart string literals while ignoring line and block comments."""
|
|
literals: list[Literal] = []
|
|
index = 0
|
|
length = len(source)
|
|
while index < length:
|
|
if source.startswith("//", index):
|
|
newline = source.find("\n", index + 2)
|
|
index = length if newline < 0 else newline + 1
|
|
continue
|
|
if source.startswith("/*", index):
|
|
depth = 1
|
|
index += 2
|
|
while index < length and depth:
|
|
if source.startswith("/*", index):
|
|
depth += 1
|
|
index += 2
|
|
elif source.startswith("*/", index):
|
|
depth -= 1
|
|
index += 2
|
|
else:
|
|
index += 1
|
|
continue
|
|
|
|
raw = False
|
|
token_start = index
|
|
if (
|
|
source[index] in "rR"
|
|
and index + 1 < length
|
|
and source[index + 1] in "'\""
|
|
and (index == 0 or not (source[index - 1].isalnum() or source[index - 1] == "_"))
|
|
):
|
|
raw = True
|
|
index += 1
|
|
if source[index] not in "'\"":
|
|
index += 1
|
|
continue
|
|
|
|
quote_char = source[index]
|
|
quote = quote_char * 3 if source.startswith(quote_char * 3, index) else quote_char
|
|
body_start = index + len(quote)
|
|
cursor = body_start
|
|
while cursor < length:
|
|
if not raw and source[cursor] == "\\":
|
|
cursor += 2
|
|
continue
|
|
if not raw and source.startswith("${", cursor):
|
|
cursor = _interpolation_end(source, cursor)
|
|
continue
|
|
if source.startswith(quote, cursor):
|
|
end = cursor + len(quote)
|
|
literals.append(
|
|
Literal(
|
|
line=source.count("\n", 0, token_start) + 1,
|
|
value=source[body_start:cursor],
|
|
start=token_start,
|
|
end=end,
|
|
raw=raw,
|
|
)
|
|
)
|
|
index = end
|
|
break
|
|
if len(quote) == 1 and source[cursor] == "\n":
|
|
index = cursor + 1
|
|
break
|
|
cursor += 1
|
|
else:
|
|
index = length
|
|
return literals
|
|
|
|
|
|
def _literal_text(value: str) -> str:
|
|
"""Return the literal with every interpolation replaced by whitespace."""
|
|
literal_parts: list[str] = []
|
|
cursor = 0
|
|
while cursor < len(value):
|
|
if value.startswith("${", cursor):
|
|
literal_parts.append(" ")
|
|
cursor = _interpolation_end(value, cursor)
|
|
elif value[cursor] == "$" and cursor + 1 < len(value) and (
|
|
value[cursor + 1].isalpha() or value[cursor + 1] == "_"
|
|
):
|
|
cursor += 2
|
|
while cursor < len(value) and (value[cursor].isalnum() or value[cursor] == "_"):
|
|
cursor += 1
|
|
else:
|
|
literal_parts.append(value[cursor])
|
|
cursor += 1
|
|
return "".join(literal_parts)
|
|
|
|
|
|
def _literal_words(value: str) -> list[str]:
|
|
return [word.lower() for word in _WORD_RE.findall(_literal_text(value))]
|
|
|
|
|
|
def _contains_english(value: str) -> bool:
|
|
words = _literal_words(value)
|
|
return bool(words) and not all(word in _UNITS for word in words)
|
|
|
|
|
|
def _statement_prefix(source: str, start: int) -> str:
|
|
boundary = source.rfind(";", 0, start)
|
|
return source[boundary + 1 : start]
|
|
|
|
|
|
def _is_excluded_context(source: str, literal: Literal) -> bool:
|
|
statement_prefix = _statement_prefix(source, literal.start)
|
|
if re.match(r"\s*(?:import|part)\b", statement_prefix):
|
|
return True
|
|
|
|
prefix = source[max(0, literal.start - 300) : literal.start]
|
|
if _EXCLUDED_ARGUMENT_RE.search(prefix) or _KEY_RE.search(prefix):
|
|
return True
|
|
return bool(_DIAGNOSTIC_RE.search(statement_prefix))
|
|
|
|
|
|
def _rule_for_literal(source: str, literal: Literal, renders_ui: bool = False) -> str | None:
|
|
if not _contains_english(literal.value) or _is_excluded_context(source, literal):
|
|
return None
|
|
|
|
prefix = source[max(0, literal.start - 300) : literal.start]
|
|
if not literal.raw and _TRANSLATION_INTERPOLATION_RE.search(literal.value):
|
|
if not (re.search(r"\$\{\s*t\.", literal.value) and _T_PARAMETER_RE.search(prefix)):
|
|
return "mixed translation interpolation"
|
|
if _TEXT_RE.search(prefix):
|
|
return "Text first argument"
|
|
if _UI_ARGUMENT_RE.search(prefix):
|
|
return "UI named argument"
|
|
if (
|
|
renders_ui
|
|
and _PHRASE_RE.search(_literal_text(literal.value))
|
|
and _BOUND_LITERAL_RE.search(_statement_prefix(source, literal.start))
|
|
):
|
|
return "display string bound to a name"
|
|
return None
|
|
|
|
|
|
def load_allowlist(path: Path | None = None) -> dict[str, dict[str, str]]:
|
|
path = ALLOWLIST_PATH if path is None else path
|
|
data = json.loads(path.read_text(encoding="utf-8"))
|
|
if not isinstance(data, dict):
|
|
raise ValueError("allowlist root must be an object")
|
|
for relative_path, entries in data.items():
|
|
if not isinstance(relative_path, str) or not isinstance(entries, dict):
|
|
raise ValueError("allowlist entries must map paths to objects")
|
|
for literal, reason in entries.items():
|
|
if not isinstance(literal, str) or not isinstance(reason, str) or not reason.strip():
|
|
raise ValueError(f"allowlist entry {relative_path!r} / {literal!r} needs a reason")
|
|
return data
|
|
|
|
|
|
def _is_allowlisted(path: str, literal: str, allowlist: dict[str, dict[str, str]]) -> bool:
|
|
return literal in allowlist.get(path, {}) or literal in allowlist.get("*", {})
|
|
|
|
|
|
def scan(
|
|
lib_dir: Path | None = None,
|
|
allowlist: dict[str, dict[str, str]] | None = None,
|
|
root: Path | None = None,
|
|
) -> tuple[list[Finding], dict[str, set[str]]]:
|
|
lib_dir = LIB_DIR if lib_dir is None else lib_dir
|
|
root = ROOT if root is None else root
|
|
if allowlist is None:
|
|
allowlist = load_allowlist()
|
|
findings: list[Finding] = []
|
|
seen_literals: dict[str, set[str]] = {}
|
|
for path in sorted(lib_dir.rglob("*.dart")):
|
|
relative = path.relative_to(root).as_posix()
|
|
# lib/dev is a separate measurement entrypoint, explicitly "NOT part of the app".
|
|
if (
|
|
relative.startswith(("lib/i18n/", "lib/dev/"))
|
|
or path.name.endswith((".g.dart", ".freezed.dart"))
|
|
):
|
|
continue
|
|
source = path.read_text(encoding="utf-8")
|
|
literals = _string_literals(source)
|
|
renders_ui = bool(_RENDERS_UI_RE.search(source))
|
|
seen_literals[relative] = {literal.value for literal in literals}
|
|
for literal in literals:
|
|
rule = _rule_for_literal(source, literal, renders_ui)
|
|
if rule is None or _is_allowlisted(relative, literal.value, allowlist):
|
|
continue
|
|
findings.append(Finding(relative, literal.line, literal.value, rule))
|
|
return findings, seen_literals
|
|
|
|
|
|
def stale_allowlist_entries(
|
|
allowlist: dict[str, dict[str, str]], seen_literals: dict[str, set[str]]
|
|
) -> list[tuple[str, str]]:
|
|
all_literals = set().union(*seen_literals.values()) if seen_literals else set()
|
|
stale: list[tuple[str, str]] = []
|
|
for path, entries in allowlist.items():
|
|
present = all_literals if path == "*" else seen_literals.get(path, set())
|
|
stale.extend((path, literal) for literal in entries if literal not in present)
|
|
return sorted(stale)
|
|
|
|
|
|
def _print_findings(findings: list[Finding]) -> None:
|
|
current_path = None
|
|
for finding in findings:
|
|
if finding.path != current_path:
|
|
if current_path is not None:
|
|
print()
|
|
current_path = finding.path
|
|
print(f"{finding.path}:")
|
|
print(f" {finding.line}: [{finding.rule}] {finding.literal!r}")
|
|
print(f"\nFound {len(findings)} hardcoded user-facing string(s).")
|
|
|
|
|
|
def main(argv: list[str] | None = None) -> int:
|
|
parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
|
|
parser.add_argument("--root", type=Path, default=ROOT)
|
|
parser.add_argument(
|
|
"--allowlist", type=Path, default=ALLOWLIST_PATH, help="path to the JSON allowlist"
|
|
)
|
|
parser.add_argument(
|
|
"--allowlist-missing",
|
|
action="store_true",
|
|
help="report allowlist entries whose literal no longer exists",
|
|
)
|
|
args = parser.parse_args(argv)
|
|
|
|
try:
|
|
allowlist = load_allowlist(args.allowlist)
|
|
findings, seen_literals = scan(args.root / "lib", allowlist, args.root)
|
|
except (OSError, ValueError, json.JSONDecodeError) as error:
|
|
print(f"error: {error}")
|
|
return 1
|
|
|
|
if args.allowlist_missing:
|
|
stale = stale_allowlist_entries(allowlist, seen_literals)
|
|
if stale:
|
|
print("Allowlist entries that no longer match a source literal:")
|
|
for path, literal in stale:
|
|
print(f" {path}: {literal!r}")
|
|
return 1
|
|
print("All hardcoded-string allowlist entries still match source literals.")
|
|
return 0
|
|
|
|
if findings:
|
|
_print_findings(findings)
|
|
return 1
|
|
print("No hardcoded user-facing English strings found.")
|
|
return 0
|
|
|
|
|
|
if __name__ == "__main__":
|
|
raise SystemExit(main())
|