From 0260d2accbc047dda144b4738fdd8a15b9a9e69d Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Wed, 26 Aug 2026 17:10:30 +0200 Subject: [PATCH] Stop the textFormat test from passing when it has not checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The root rule matched only a file-level root Text, of which this tree has exactly one. QML inline components are roots for the same reason — the `text` of `component InfoValue: Text {` comes from every caller, so the file it lives in never binds it — but they sit inside another element, so the depth-1 test never saw them. Six went uncovered while the test reported green, among them the network panel's InfoValue, which callers bind to the IP address and gateway. Six more ways to write a Text were read as clean rather than as unreadable: an opening brace that is not last on its line, a brace on the line after `Text`, a one-line block containing nested braces, a wrapped binding split by a comment or a blank line before its `+` (which exempted a dynamic binding as a literal), and a root Text indented from column zero. Require the forms a line scanner can read instead of parsing QML; the tree already writes every Text that way. Last, a run that read no files reported success. A checkout with no shell/ QML now fails instead, since an all-clear from a scan that opened nothing is the one answer this test must never give. Each case is covered by a fixture that fails without its fix. Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: OpenAI Codex (gpt-5, xhigh) --- shell/plugins/panels/dropbox/Panel.qml | 2 + shell/plugins/panels/network/Panel.qml | 2 + shell/plugins/panels/power/Panel.qml | 2 + test/shell.d/qml-text-format-test.sh | 105 ++++++++++++++++++++++++- 4 files changed, 109 insertions(+), 2 deletions(-) diff --git a/shell/plugins/panels/dropbox/Panel.qml b/shell/plugins/panels/dropbox/Panel.qml index b470cee2..515b43ce 100644 --- a/shell/plugins/panels/dropbox/Panel.qml +++ b/shell/plugins/panels/dropbox/Panel.qml @@ -530,6 +530,7 @@ Panel { } component InfoLabel: Text { + textFormat: Text.PlainText color: root.foreground opacity: 0.6 font.family: root.fontFamily @@ -537,6 +538,7 @@ Panel { } component InfoValue: Text { + textFormat: Text.PlainText color: root.foreground font.family: root.fontFamily font.pixelSize: Style.font.bodySmall diff --git a/shell/plugins/panels/network/Panel.qml b/shell/plugins/panels/network/Panel.qml index c4da0afe..d1e41149 100644 --- a/shell/plugins/panels/network/Panel.qml +++ b/shell/plugins/panels/network/Panel.qml @@ -1954,6 +1954,7 @@ Panel { } component InfoLabel: Text { + textFormat: Text.PlainText color: root.bar.foreground opacity: 0.6 font.family: root.bar.fontFamily @@ -1961,6 +1962,7 @@ Panel { } component InfoValue: Text { + textFormat: Text.PlainText color: root.bar.foreground font.family: root.bar.fontFamily font.pixelSize: Style.font.bodySmall diff --git a/shell/plugins/panels/power/Panel.qml b/shell/plugins/panels/power/Panel.qml index 7733bb37..b1c34da2 100644 --- a/shell/plugins/panels/power/Panel.qml +++ b/shell/plugins/panels/power/Panel.qml @@ -520,6 +520,7 @@ Panel { } component InfoLabel: Text { + textFormat: Text.PlainText color: root.bar.foreground opacity: 0.6 font.family: root.bar.fontFamily @@ -527,6 +528,7 @@ Panel { } component InfoValue: Text { + textFormat: Text.PlainText color: root.bar.foreground font.family: root.bar.fontFamily font.pixelSize: Style.font.bodySmall diff --git a/test/shell.d/qml-text-format-test.sh b/test/shell.d/qml-text-format-test.sh index 2d7774f0..0b286a76 100755 --- a/test/shell.d/qml-text-format-test.sh +++ b/test/shell.d/qml-text-format-test.sh @@ -31,6 +31,8 @@ import re from pathlib import Path OPEN_ELEMENT = re.compile(r'(?:^|[:\s])([A-Z][A-Za-z0-9_.]*)\s*\{\s*$') +INLINE_COMPONENT = re.compile(r'^\s*component\s+[A-Za-z_][A-Za-z0-9_]*\s*:\s*Text\s*\{\s*$') +INLINE_COMPONENT_ONELINE = re.compile(r'^\s*component\s+[A-Za-z_][A-Za-z0-9_]*\s*:\s*Text\s*\{') PROP = re.compile(r'^\s*([A-Za-z_][A-Za-z0-9_.]*)\s*:') STRING_LITERAL = re.compile(r'"(?:[^"\\]|\\.)*"|\'(?:[^\'\\]|\\.)*\'') PROPERTY_DECL = re.compile(r'^\s*(?:readonly\s+)?property\b') @@ -96,7 +98,17 @@ def binding_expression(lines, start): counted = strip_noise(lines[i]) parens += counted.count('(') - counted.count(')') brackets += counted.count('[') - counted.count(']') - following = strip_noise(lines[i + 1]) if i + 1 < len(lines) else '' + # Look past blank and comment-only lines for the continuation. A + # comment or a blank line dropped into a wrapped expression does not + # end it, and stopping there would read `text: "prefix"` as the whole + # binding and exempt it as a literal while `+ externalValue` waits + # below — the exact misreading this function exists to prevent. + following = '' + for ahead in range(i + 1, len(lines)): + candidate = strip_noise(lines[ahead]) + if candidate.strip(): + following = candidate + break continues = (parens > 0 or brackets > 0 or TRAILING_OPERATOR.search(counted.rstrip()) or LEADING_OPERATOR.match(following)) @@ -156,6 +168,12 @@ def inline_violations(lines, rel): body = match.group(1) if 'textFormat' in body: continue + # A component root written on one line needs the default whether or + # not this line binds `text`, for the same reason the block form + # does: every caller supplies the binding. + if INLINE_COMPONENT_ONELINE.match(code): + out.append(f'{rel}:{idx + 1}: inline component root Text declares no textFormat') + continue binding = INLINE_BINDING.search(body) if not binding or is_pure_literal(binding.group(1)): continue @@ -163,12 +181,75 @@ def inline_violations(lines, rel): return out +# `Text { text: someValue` with the block carrying on below is valid QML and is +# invisible to both scanners: OPEN_ELEMENT anchors its `{` at the end of the +# line so the brace tracker never opens the block, and INLINE_TEXT needs the +# closing brace on the same line. A dynamic AutoText binding written that way +# passes this file in silence, which is the one failure a test like this must +# not have. +# +# Rather than teach a line scanner to parse QML, require the two forms it can +# read: the whole block on one line, or nothing after the opening brace. Every +# Text in this tree is already written that way, so keeping to it costs nothing. +UNSCANNABLE_TEXT = re.compile(r'(?:^|[:\s])Text\s*\{\s*\S') +BARE_TEXT_OPENER = re.compile(r'(?:^|[:\s])Text\s*$') + +UNSCANNABLE = ('Text block written in a form this scanner cannot read; put the ' + 'opening brace last on the line, or write the whole block on ' + 'one line with no nested braces') + + +def unscannable_violations(lines, rel): + out = [] + for idx, raw in enumerate(lines): + code = strip_noise(raw) + + # `Text` with its brace on the next line. OPEN_ELEMENT needs both on + # one line, so the block is never opened and everything in it is + # attributed to the enclosing element instead. + if BARE_TEXT_OPENER.search(code): + following = '' + for ahead in range(idx + 1, len(lines)): + candidate = strip_noise(lines[ahead]).strip() + if candidate: + following = candidate + break + if following.startswith('{'): + out.append(f'{rel}:{idx + 1}: {UNSCANNABLE}') + continue + + for match in UNSCANNABLE_TEXT.finditer(code): + # A complete one-line block with no nested braces is fine — + # inline_violations reads those. Count rather than looking for a + # `}`, because `Text { text: ({ a: external }).a }` closes on this + # line yet INLINE_TEXT's brace-free body pattern cannot match it, + # so treating any `}` as "handled elsewhere" would drop it. + rest = code[match.end() - 1:] + depth = 1 + closed = False + for char in rest: + if char == '{': + depth += 1 + elif char == '}': + depth -= 1 + if depth == 0: + closed = True + break + if closed and '{' not in rest: + continue + out.append(f'{rel}:{idx + 1}: {UNSCANNABLE}') + return out + + root = Path(os.environ['ROOT']) found = [] +scanned = 0 for path in sorted((root / 'shell').rglob('*.qml')): + scanned += 1 lines = path.read_text().splitlines() rel = path.relative_to(root) found.extend(inline_violations(lines, rel)) + found.extend(unscannable_violations(lines, rel)) for b in blocks(lines): if b['name'] != 'Text' or 'textFormat' in b['props']: @@ -182,10 +263,24 @@ for path in sorted((root / 'shell').rglob('*.qml')): # depth 1 and column 0: the scanner attributes one element per line, so # a `Row { Text {` line would report depth 1 for a nested block, and # falling through to the binding check below is the safe reading. - if b['depth'] == 1 and lines[b['start']].startswith('Text'): + # Indentation is not what makes it a root; depth 1 is. A `Row { Text {` + # line still reads as `Row` here, so leading whitespace can be ignored + # without letting a nested block be mistaken for the file's root. + if b['depth'] == 1 and lines[b['start']].lstrip().startswith('Text'): found.append(f'{rel}:{b["start"] + 1}: root Text element declares no textFormat') continue + # A QML inline component is a root for the same reason, and the rule + # above cannot see one: `component InfoValue: Text {` sits inside + # another element, so its depth is not 1 and its line does not start + # with `Text`. Its `text` comes from every caller, so the file it lives + # in never binds it and the binding check below lets it through in + # silence. Only one file-level root Text exists in this tree, so + # without this the root rule is very nearly dead code. + if INLINE_COMPONENT.match(lines[b['start']]): + found.append(f'{rel}:{b["start"] + 1}: inline component root Text declares no textFormat') + continue + if 'text' not in b['props']: continue tline = b['props']['text'] @@ -193,6 +288,12 @@ for path in sorted((root / 'shell').rglob('*.qml')): continue found.append(f'{rel}:{tline + 1}: text binding without textFormat') +# A scan that read nothing reports nothing, and an all-clear from a run that +# never opened a file is the one result this test must never give. Only a +# checkout with no shell/ QML at all reaches this. +if scanned == 0: + raise SystemExit('no .qml files found under shell/; the scan read nothing') + for line in found: print(line) PY