From bfc5778048178a47fce19a180146651008f4eba8 Mon Sep 17 00:00:00 2001 From: mateo Date: Wed, 23 Sep 2026 02:04:35 +0000 Subject: [PATCH 1/7] feat(lint): LIT013 caps comprehensions at one for and one if clause Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- scripts/check_type_discipline.py | 44 ++++++++++++++- scripts/type_discipline_gate.py | 5 +- .../test_check_type_discipline.py | 53 +++++++++++++++++++ type-discipline-budget.json | 3 ++ 4 files changed, 101 insertions(+), 4 deletions(-) diff --git a/scripts/check_type_discipline.py b/scripts/check_type_discipline.py index a2ab4760c4f7..1965aceaea99 100644 --- a/scripts/check_type_discipline.py +++ b/scripts/check_type_discipline.py @@ -40,7 +40,8 @@ LIT004 pyright/mypy ignore without bracketed codes or without a reason. Required shape: `# pyright: ignore[reportArgumentType] # ` LIT005 A `# mutable-ok` / `# cast-ok` / `# guard-ok` / `# kwargs-ok` / - `# rebind-ok` / `# writable-ok` suppression without a reason. + `# rebind-ok` / `# writable-ok` / `# comprehension-ok` suppression + without a reason. LIT006 `cast(...)` call. typing.cast is an unchecked assertion (the moral equivalent of TypeScript's `as`); it lies to the type checker with zero runtime guarantee. Validate into a concrete frozen type at the boundary instead. @@ -99,6 +100,13 @@ the functional form (`X = TypedDict("X", {...})`) is checked too. A base imported from another module is out of reach without import resolution. Suppress with `# writable-ok: `. +LIT013 Comprehension with more than one `for` clause or more than one `if` clause, + in any of the four forms (list, set, dict, generator expression). Stacked + `for`s and `if`s read as nested loops and guards squashed onto one line; + split the comprehension into a helper generator, a named intermediate, or + a plain loop instead. A comprehension nested inside another's element or + iterable is its own node and is judged separately. Suppress with + `# comprehension-ok: `. LIT000 Setup failure: a target file could not be read, or contains a syntax error. Reported as a violation rather than crashing the run. @@ -182,6 +190,7 @@ KWARGS_OK_RE = re.compile(r"#\s*kwargs-ok(?::\s*(?P.*))?") REBIND_OK_RE = re.compile(r"#\s*rebind-ok(?::\s*(?P.*))?") WRITABLE_OK_RE = re.compile(r"#\s*writable-ok(?::\s*(?P.*))?") +COMPREHENSION_OK_RE = re.compile(r"#\s*comprehension-ok(?::\s*(?P.*))?") # Suppression tokens that must each carry a reason (LIT005). OK_SUPPRESSIONS: tuple[tuple[str, re.Pattern[str]], ...] = ( @@ -191,6 +200,7 @@ ("kwargs-ok", KWARGS_OK_RE), ("rebind-ok", REBIND_OK_RE), ("writable-ok", WRITABLE_OK_RE), + ("comprehension-ok", COMPREHENSION_OK_RE), ) @@ -214,6 +224,7 @@ class Comments: kwargs_ok_lines: frozenset[int] rebind_ok_lines: frozenset[int] writable_ok_lines: frozenset[int] + comprehension_ok_lines: frozenset[int] # --------------------------------------------------------------------------- # @@ -269,7 +280,7 @@ def scan_comments(path: Path, source: str) -> tuple[Comments, tuple[Violation, . # tokenize raises TokenError (EOF mid-construct) or a SyntaxError subclass # (IndentationError / TabError) on malformed source; defer to ast.parse below, # which re-raises and is reported as LIT000 rather than crashing the run. - return Comments(frozenset(), frozenset(), frozenset(), frozenset(), frozenset(), frozenset()), () + return Comments(frozenset(), frozenset(), frozenset(), frozenset(), frozenset(), frozenset(), frozenset()), () def _lines_with(regex: re.Pattern[str]) -> frozenset[int]: return frozenset(line for line, text in comment_toks if _valid_ok(regex, text)) @@ -282,6 +293,7 @@ def _lines_with(regex: re.Pattern[str]) -> frozenset[int]: kwargs_ok_lines=_lines_with(KWARGS_OK_RE), rebind_ok_lines=_lines_with(REBIND_OK_RE), writable_ok_lines=_lines_with(WRITABLE_OK_RE), + comprehension_ok_lines=_lines_with(COMPREHENSION_OK_RE), ), tuple(v for line, text in comment_toks for v in _comment_violations(path, line, text)), ) @@ -1033,6 +1045,33 @@ def iter_typeddict_violations(path: Path, tree: ast.AST, comments: Comments) -> ) +# --------------------------------------------------------------------------- # +# Stacked comprehension clauses (LIT013) +# --------------------------------------------------------------------------- # + +COMPREHENSION_NODES = (ast.ListComp, ast.SetComp, ast.DictComp, ast.GeneratorExp) + + +def iter_comprehension_violations(path: Path, tree: ast.AST, comments: Comments) -> Iterator[Violation]: + for node in ast.walk(tree): + if not isinstance(node, COMPREHENSION_NODES): + continue + for_count = len(node.generators) + if_count = sum(len(g.ifs) for g in node.generators) + if for_count <= 1 and if_count <= 1: + continue + if node.lineno in comments.comprehension_ok_lines: + continue + yield Violation( + path, + node.lineno, + "LIT013", + f"comprehension with {for_count} `for` clauses and {if_count} `if` clauses: " + f"at most one of each is allowed. Split it into a helper generator, a named " + f"intermediate, or a plain loop (suppress: `# comprehension-ok: `)", + ) + + # --------------------------------------------------------------------------- # # Driver # --------------------------------------------------------------------------- # @@ -1060,6 +1099,7 @@ def check_file(path: Path) -> tuple[Violation, ...]: *iter_final_violations(path, tree, comments), *iter_param_violations(path, tree, comments), *iter_typeddict_violations(path, tree, comments), + *iter_comprehension_violations(path, tree, comments), ) diff --git a/scripts/type_discipline_gate.py b/scripts/type_discipline_gate.py index 40e61cf72654..5dc0ae92714a 100644 --- a/scripts/type_discipline_gate.py +++ b/scripts/type_discipline_gate.py @@ -15,8 +15,9 @@ (assignment without a Final declaration; suppress deliberate rebinding with `# rebind-ok: `), LIT011 (parameter rebinding or in-place mutation), and LIT012 (TypedDict field without a `ReadOnly[...]` qualifier; suppress with -`# writable-ok: `) carry limits at or above their current count to -ratchet down; LIT005 (`*-ok` suppression without a reason) is frozen at limit 0 +`# writable-ok: `), and LIT013 (comprehension with more than one `for` +or `if` clause; suppress with `# comprehension-ok: `) carry limits at +or above their current count to ratchet down; LIT005 (`*-ok` suppression without a reason) is frozen at limit 0 so any net-new reasonless suppression trips the gate; and LIT007 (TypeGuard/TypeIs) is a hard zero. LIT010 and LIT011 were seeded at 1.5x the count left after the sweep that diff --git a/tests/test_litellm/test_check_type_discipline.py b/tests/test_litellm/test_check_type_discipline.py index 2d49332e6874..5f59491d82e9 100644 --- a/tests/test_litellm/test_check_type_discipline.py +++ b/tests/test_litellm/test_check_type_discipline.py @@ -687,6 +687,59 @@ def test_writable_ok_without_reason_is_lit005_and_does_not_suppress(tmp_path): assert "LIT012" in codes +# --------------------------------------------------------------------------- # +# Stacked comprehension clauses (LIT013) +# --------------------------------------------------------------------------- # + + +def test_two_for_clauses_are_flagged(tmp_path): + assert "LIT013" in _codes(tmp_path, "y = [x for a in xs for x in a]\n") + + +def test_two_ifs_on_one_generator_are_flagged(tmp_path): + assert "LIT013" in _codes(tmp_path, "y = [x for x in xs if x if x > 1]\n") + + +def test_one_if_on_each_of_two_generators_is_flagged(tmp_path): + assert "LIT013" in _codes(tmp_path, "y = [x for a in xs if a for x in a if x]\n") + + +def test_one_for_and_one_if_is_clean(tmp_path): + assert "LIT013" not in _codes(tmp_path, "y = tuple(x for x in xs if x)\n") + + +def test_dict_set_and_generator_two_fors_are_each_flagged(tmp_path): + assert "LIT013" in _codes(tmp_path, "d = {k: v for a in xs for k, v in a}\n") + assert "LIT013" in _codes(tmp_path, "s = {x for a in xs for x in a}\n") + assert "LIT013" in _codes(tmp_path, "g = (x for a in xs for x in a)\n") + + +def test_nested_comprehension_in_element_is_judged_separately(tmp_path): + assert "LIT013" not in _codes(tmp_path, "y = [[v for v in a] for a in xs]\n") + + +def test_comprehension_ok_with_reason_suppresses_lit013(tmp_path): + codes = _codes( + tmp_path, + "y = [x for a in xs for x in a] # comprehension-ok: flattens a stream of pairs, hot path\n", + ) + assert "LIT013" not in codes + + +def test_comprehension_ok_without_reason_is_lit005_and_does_not_suppress(tmp_path): + codes = _codes(tmp_path, "y = [x for a in xs for x in a] # comprehension-ok\n") + assert "LIT005" in codes + assert "LIT013" in codes + + +def test_violation_message_names_the_clause_counts(tmp_path): + f = tmp_path / "snippet.py" + f.write_text("y = [x for a in xs for x in a if x]\n", encoding="utf-8") + messages = [v.message for v in checker.check_file(f) if v.code == "LIT013"] + assert len(messages) == 1 + assert "2 `for` clauses and 1 `if` clause" in messages[0] + + # --------------------------------------------------------------------------- # # Budget integrity: every emittable LIT rule (bar the LIT000 read/parse error) is gated # --------------------------------------------------------------------------- # diff --git a/type-discipline-budget.json b/type-discipline-budget.json index 0c0952289e2b..3400c0bd729d 100644 --- a/type-discipline-budget.json +++ b/type-discipline-budget.json @@ -34,5 +34,8 @@ }, "LIT012": { "limit": 4486 + }, + "LIT013": { + "limit": 363 } } From 9d4d32a654d6fdc537436fdc183e562ca3e653b7 Mon Sep 17 00:00:00 2001 From: mateo Date: Wed, 23 Sep 2026 02:05:06 +0000 Subject: [PATCH 2/7] docs(lint): rewrap the type discipline gate rule list Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- scripts/type_discipline_gate.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/type_discipline_gate.py b/scripts/type_discipline_gate.py index 5dc0ae92714a..6ee830f82467 100644 --- a/scripts/type_discipline_gate.py +++ b/scripts/type_discipline_gate.py @@ -13,11 +13,12 @@ without codes or reason), LIT006 (cast), LIT008 (`**kwargs`), LIT009 (inert `# type: ignore`, dead syntax while enableTypeIgnoreComments is false), LIT010 (assignment without a Final declaration; suppress deliberate rebinding with -`# rebind-ok: `), LIT011 (parameter rebinding or in-place mutation), and +`# rebind-ok: `), LIT011 (parameter rebinding or in-place mutation), LIT012 (TypedDict field without a `ReadOnly[...]` qualifier; suppress with `# writable-ok: `), and LIT013 (comprehension with more than one `for` or `if` clause; suppress with `# comprehension-ok: `) carry limits at -or above their current count to ratchet down; LIT005 (`*-ok` suppression without a reason) is frozen at limit 0 +or above their current count to ratchet down; LIT005 (`*-ok` suppression +without a reason) is frozen at limit 0 so any net-new reasonless suppression trips the gate; and LIT007 (TypeGuard/TypeIs) is a hard zero. LIT010 and LIT011 were seeded at 1.5x the count left after the sweep that From c2bc11a1aed06998202e36ccb23413f3cd2d758c Mon Sep 17 00:00:00 2001 From: mateo Date: Wed, 23 Sep 2026 02:18:36 +0000 Subject: [PATCH 3/7] fix(lint): honor comprehension-ok on any line a comprehension spans Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- scripts/check_type_discipline.py | 4 ++-- .../test_check_type_discipline.py | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/scripts/check_type_discipline.py b/scripts/check_type_discipline.py index 1965aceaea99..17c311e047d2 100644 --- a/scripts/check_type_discipline.py +++ b/scripts/check_type_discipline.py @@ -106,7 +106,7 @@ split the comprehension into a helper generator, a named intermediate, or a plain loop instead. A comprehension nested inside another's element or iterable is its own node and is judged separately. Suppress with - `# comprehension-ok: `. + `# comprehension-ok: ` on any line the comprehension spans. LIT000 Setup failure: a target file could not be read, or contains a syntax error. Reported as a violation rather than crashing the run. @@ -1060,7 +1060,7 @@ def iter_comprehension_violations(path: Path, tree: ast.AST, comments: Comments) if_count = sum(len(g.ifs) for g in node.generators) if for_count <= 1 and if_count <= 1: continue - if node.lineno in comments.comprehension_ok_lines: + if not comments.comprehension_ok_lines.isdisjoint(range(node.lineno, (node.end_lineno or node.lineno) + 1)): continue yield Violation( path, diff --git a/tests/test_litellm/test_check_type_discipline.py b/tests/test_litellm/test_check_type_discipline.py index 5f59491d82e9..d15069b5b455 100644 --- a/tests/test_litellm/test_check_type_discipline.py +++ b/tests/test_litellm/test_check_type_discipline.py @@ -726,6 +726,27 @@ def test_comprehension_ok_with_reason_suppresses_lit013(tmp_path): assert "LIT013" not in codes +def test_comprehension_ok_on_any_spanned_line_suppresses_lit013(tmp_path): + src = ( + "y = [\n" + " x for a in xs\n" + " for x in a\n" + "] # comprehension-ok: cartesian product is the clearest form\n" + ) + assert "LIT013" not in _codes(tmp_path, src) + + +def test_comprehension_ok_after_the_closing_line_does_not_suppress(tmp_path): + src = ( + "y = [\n" + " x for a in xs\n" + " for x in a\n" + "]\n" + "# comprehension-ok: cartesian product is the clearest form\n" + ) + assert "LIT013" in _codes(tmp_path, src) + + def test_comprehension_ok_without_reason_is_lit005_and_does_not_suppress(tmp_path): codes = _codes(tmp_path, "y = [x for a in xs for x in a] # comprehension-ok\n") assert "LIT005" in codes From 87afc69892a2d5cab84952aca7daf4209d1d74f0 Mon Sep 17 00:00:00 2001 From: mateo Date: Wed, 23 Sep 2026 02:31:02 +0000 Subject: [PATCH 4/7] fix(lint): scope comprehension-ok to the innermost comprehension spanning it Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- scripts/check_type_discipline.py | 30 ++++++++++++++++-- .../test_check_type_discipline.py | 31 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/scripts/check_type_discipline.py b/scripts/check_type_discipline.py index 17c311e047d2..41673c0bcb77 100644 --- a/scripts/check_type_discipline.py +++ b/scripts/check_type_discipline.py @@ -106,7 +106,9 @@ split the comprehension into a helper generator, a named intermediate, or a plain loop instead. A comprehension nested inside another's element or iterable is its own node and is judged separately. Suppress with - `# comprehension-ok: ` on any line the comprehension spans. + `# comprehension-ok: ` on any line the comprehension spans; when + comprehensions nest, the comment belongs to the innermost one spanning + that line. LIT000 Setup failure: a target file could not be read, or contains a syntax error. Reported as a violation rather than crashing the run. @@ -1052,7 +1054,31 @@ def iter_typeddict_violations(path: Path, tree: ast.AST, comments: Comments) -> COMPREHENSION_NODES = (ast.ListComp, ast.SetComp, ast.DictComp, ast.GeneratorExp) +def _span(node: ast.expr) -> range: + return range(node.lineno, (node.end_lineno or node.lineno) + 1) + + +def _suppressed_comprehensions(tree: ast.AST, ok_lines: frozenset[int]) -> frozenset[int]: + """ids() of the comprehension each `# comprehension-ok` line suppresses. + + A suppression belongs to the innermost comprehension whose span contains its + line, so a comment inside a nested comprehension never silences the enclosing + one (LIT013 judges each comprehension as its own node). + """ + comps: Final = tuple(n for n in ast.walk(tree) if isinstance(n, COMPREHENSION_NODES)) + + def len_of_span(node: ast.expr) -> int: + return len(_span(node)) + + def owner(line: int) -> ast.expr | None: + containing: Final = tuple(n for n in comps if line in _span(n)) + return min(containing, key=len_of_span, default=None) + + return frozenset(id(o) for o in (owner(line) for line in ok_lines) if o is not None) + + def iter_comprehension_violations(path: Path, tree: ast.AST, comments: Comments) -> Iterator[Violation]: + suppressed: Final = _suppressed_comprehensions(tree, comments.comprehension_ok_lines) for node in ast.walk(tree): if not isinstance(node, COMPREHENSION_NODES): continue @@ -1060,7 +1086,7 @@ def iter_comprehension_violations(path: Path, tree: ast.AST, comments: Comments) if_count = sum(len(g.ifs) for g in node.generators) if for_count <= 1 and if_count <= 1: continue - if not comments.comprehension_ok_lines.isdisjoint(range(node.lineno, (node.end_lineno or node.lineno) + 1)): + if id(node) in suppressed: continue yield Violation( path, diff --git a/tests/test_litellm/test_check_type_discipline.py b/tests/test_litellm/test_check_type_discipline.py index d15069b5b455..94eef2e66f35 100644 --- a/tests/test_litellm/test_check_type_discipline.py +++ b/tests/test_litellm/test_check_type_discipline.py @@ -753,6 +753,37 @@ def test_comprehension_ok_without_reason_is_lit005_and_does_not_suppress(tmp_pat assert "LIT013" in codes +def test_suppression_inside_inner_comprehension_does_not_silence_the_outer(tmp_path): + src = ( + "y = [\n" # line 1, outer comprehension starts + " x\n" + " for a in [\n" + " z for i in ys\n" + " for z in i\n" + " ] # comprehension-ok: inner flatten is the clearest form\n" + " for x in a\n" + "]\n" + ) + f = tmp_path / "snippet.py" + f.write_text(src, encoding="utf-8") + flagged = [v for v in checker.check_file(f) if v.code == "LIT013"] + assert [v.line for v in flagged] == [1] + + +def test_suppression_on_outer_closing_line_does_not_silence_the_inner(tmp_path): + src = ( + "y = [\n" # line 1, outer comprehension starts + " x\n" + " for a in [z for i in ys for z in i]\n" # line 3, inner comprehension + " for x in a\n" + "] # comprehension-ok: outer flatten is the clearest form\n" + ) + f = tmp_path / "snippet.py" + f.write_text(src, encoding="utf-8") + flagged = [v for v in checker.check_file(f) if v.code == "LIT013"] + assert [v.line for v in flagged] == [3] + + def test_violation_message_names_the_clause_counts(tmp_path): f = tmp_path / "snippet.py" f.write_text("y = [x for a in xs for x in a if x]\n", encoding="utf-8") From 5e83bdacd22b58fb5d960ae4d62780725977fd78 Mon Sep 17 00:00:00 2001 From: mateo Date: Wed, 23 Sep 2026 02:42:00 +0000 Subject: [PATCH 5/7] fix(lint): break equal-span suppression ties toward the inner comprehension Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- scripts/check_type_discipline.py | 6 +++--- tests/test_litellm/test_check_type_discipline.py | 15 ++++++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/scripts/check_type_discipline.py b/scripts/check_type_discipline.py index 41673c0bcb77..21acd5841f1c 100644 --- a/scripts/check_type_discipline.py +++ b/scripts/check_type_discipline.py @@ -1067,12 +1067,12 @@ def _suppressed_comprehensions(tree: ast.AST, ok_lines: frozenset[int]) -> froze """ comps: Final = tuple(n for n in ast.walk(tree) if isinstance(n, COMPREHENSION_NODES)) - def len_of_span(node: ast.expr) -> int: - return len(_span(node)) + def nesting_key(node: ast.expr) -> tuple[int, int]: + return (len(_span(node)), (node.end_col_offset or node.col_offset) - node.col_offset) def owner(line: int) -> ast.expr | None: containing: Final = tuple(n for n in comps if line in _span(n)) - return min(containing, key=len_of_span, default=None) + return min(containing, key=nesting_key, default=None) return frozenset(id(o) for o in (owner(line) for line in ok_lines) if o is not None) diff --git a/tests/test_litellm/test_check_type_discipline.py b/tests/test_litellm/test_check_type_discipline.py index 94eef2e66f35..1c5c5bc843b8 100644 --- a/tests/test_litellm/test_check_type_discipline.py +++ b/tests/test_litellm/test_check_type_discipline.py @@ -755,7 +755,7 @@ def test_comprehension_ok_without_reason_is_lit005_and_does_not_suppress(tmp_pat def test_suppression_inside_inner_comprehension_does_not_silence_the_outer(tmp_path): src = ( - "y = [\n" # line 1, outer comprehension starts + "y = [\n" " x\n" " for a in [\n" " z for i in ys\n" @@ -772,9 +772,9 @@ def test_suppression_inside_inner_comprehension_does_not_silence_the_outer(tmp_p def test_suppression_on_outer_closing_line_does_not_silence_the_inner(tmp_path): src = ( - "y = [\n" # line 1, outer comprehension starts + "y = [\n" " x\n" - " for a in [z for i in ys for z in i]\n" # line 3, inner comprehension + " for a in [z for i in ys for z in i]\n" " for x in a\n" "] # comprehension-ok: outer flatten is the clearest form\n" ) @@ -784,6 +784,15 @@ def test_suppression_on_outer_closing_line_does_not_silence_the_inner(tmp_path): assert [v.line for v in flagged] == [3] +def test_equal_span_suppression_belongs_to_the_inner_comprehension(tmp_path): + src = "y = [x for a in [z for i in ys for z in i] if a if x] # comprehension-ok: inner flatten is fine\n" + f = tmp_path / "snippet.py" + f.write_text(src, encoding="utf-8") + flagged = [v for v in checker.check_file(f) if v.code == "LIT013"] + assert len(flagged) == 1 + assert "1 `for` clauses and 2 `if` clauses" in flagged[0].message + + def test_violation_message_names_the_clause_counts(tmp_path): f = tmp_path / "snippet.py" f.write_text("y = [x for a in xs for x in a if x]\n", encoding="utf-8") From c20a557c2718d8c3ef38dc5dc111394044de2b72 Mon Sep 17 00:00:00 2001 From: mateo Date: Fri, 25 Sep 2026 01:21:33 +0000 Subject: [PATCH 6/7] fix(lint): let single-line and only violating comprehensions own comprehension-ok markers Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- AGENTS.md | 1 + scripts/check_type_discipline.py | 50 ++++++++++++------- scripts/type_discipline_gate.py | 7 ++- .../test_check_type_discipline.py | 41 +++++++++++++-- 4 files changed, 75 insertions(+), 24 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 820ea64d4f90..69e034fbdead 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,6 +96,7 @@ Follow these coding conventions for new/updated code (a three-line fix in a lega - No mutation; don't reassign variables, global or local. Instead of mutable lists and dicts, prefer tuples, frozen dataclasses (with slots=True), `MappingProxyType`, etc. - Annotate every variable with `: Final` (LIT010). Unpacking and walrus targets cannot carry the annotation, so they are implicitly final. Don't rebind them. Never rebind or mutate function parameters (LIT011); `self`/`cls` attribute stores are the exception. If rebinding or in-place mutation is truly unavoidable, suppress with `# rebind-ok: ` - Qualify every TypedDict field with `ReadOnly[...]` (LIT012), which nests freely with `Required` / `NotRequired` / `Annotated` in any order. If making the key writable is truly unavoidable, suppress with `# writable-ok: ` + - Comprehensions take at most one `for` clause and one `if` clause (LIT014); split stacked clauses into a helper generator, a named intermediate, or a plain loop. Suppress with `# comprehension-ok: ` only when unavoidable - Use dependency injection - Fully typed; no `Any` or coarse types like `dict[str, Any]` or just `dict`. Every function parameter must be strongly typed - Use tagged unions + match diff --git a/scripts/check_type_discipline.py b/scripts/check_type_discipline.py index 0b12508de60a..378b8e0876a1 100644 --- a/scripts/check_type_discipline.py +++ b/scripts/check_type_discipline.py @@ -110,9 +110,9 @@ split the comprehension into a helper generator, a named intermediate, or a plain loop instead. A comprehension nested inside another's element or iterable is its own node and is judged separately. Suppress with - `# comprehension-ok: ` on any line the comprehension spans; when - comprehensions nest, the comment belongs to the innermost one spanning - that line. + `# comprehension-ok: ` on any line the comprehension spans. The + marker belongs to the innermost violating comprehension spanning that + line, and also to any single-line violating comprehension on that line. LIT000 Setup failure: a target file could not be read, or contains a syntax error. Reported as a violation rather than crashing the run. @@ -1058,25 +1058,42 @@ def _span(node: ast.expr) -> range: return range(node.lineno, (node.end_lineno or node.lineno) + 1) +def _clause_counts(node: ast.expr) -> tuple[int, int]: + return ( + len(node.generators), + sum(len(g.ifs) for g in node.generators), + ) + + +def _violates(node: ast.expr) -> bool: + for_count, if_count = _clause_counts(node) + return for_count > 1 or if_count > 1 + + def _comprehension_owners(tree: ast.AST, ok_lines: frozenset[int]) -> Mapping[int, int]: """id(node) -> marker line for each `# comprehension-ok` line's owner. - A marker belongs to the innermost comprehension whose span contains it - (line span first, column width breaks ties), so a comment inside a nested - comprehension never silences the enclosing one. + Only violating comprehensions own markers. Each marker belongs to the + innermost violating comprehension whose span contains it (line span first, + column width breaks ties) plus every violating comprehension whose whole + span is that single line, so a comment inside a nested comprehension never + silences a multi-line enclosing one and a violation sharing its only line + can still be suppressed. """ - comps: Final = tuple(n for n in ast.walk(tree) if isinstance(n, COMPREHENSION_NODES)) + violating: Final = tuple( + n for n in ast.walk(tree) if isinstance(n, COMPREHENSION_NODES) and _violates(n) + ) def nesting_key(node: ast.expr) -> tuple[int, int]: return (len(_span(node)), (node.end_col_offset or node.col_offset) - node.col_offset) - def owner(line: int) -> ast.expr | None: - containing: Final = tuple(n for n in comps if line in _span(n)) - return min(containing, key=nesting_key, default=None) + def owners(line: int) -> tuple[ast.expr, ...]: + containing: Final = tuple(n for n in violating if line in _span(n)) + innermost: Final = min(containing, key=nesting_key, default=None) + single_line: Final = tuple(n for n in violating if len(_span(n)) == 1 and n.lineno == line) + return (*single_line, *(() if innermost is None else (innermost,))) - return MappingProxyType( - {id(o): line for line in ok_lines if (o := owner(line)) is not None} - ) + return MappingProxyType({id(o): line for line in ok_lines for o in owners(line)}) def iter_comprehension_violations( @@ -1091,12 +1108,9 @@ def iter_comprehension_violations( """ owners: Final = _comprehension_owners(tree, ok_lines) for node in ast.walk(tree): - if not isinstance(node, COMPREHENSION_NODES): - continue - for_count = len(node.generators) - if_count = sum(len(g.ifs) for g in node.generators) - if for_count <= 1 and if_count <= 1: + if not isinstance(node, COMPREHENSION_NODES) or not _violates(node): continue + for_count, if_count = _clause_counts(node) yield ( Violation( path, diff --git a/scripts/type_discipline_gate.py b/scripts/type_discipline_gate.py index 2b7966922aa8..5acaf3994f75 100644 --- a/scripts/type_discipline_gate.py +++ b/scripts/type_discipline_gate.py @@ -16,7 +16,9 @@ `# rebind-ok: `), LIT011 (parameter rebinding or in-place mutation), and LIT012 (TypedDict field without a `ReadOnly[...]` qualifier; suppress with `# writable-ok: `), and LIT014 (comprehension with more than one `for` -or `if` clause; suppress with `# comprehension-ok: `) carry limits at +or `if` clause; suppress with `# comprehension-ok: ` on a spanned +line, which belongs to the innermost violating comprehension spanning it and +to any single-line violating comprehension on that line) carry limits at or above their current count to ratchet down; LIT005 (`*-ok` suppression without a reason) is frozen at limit 0 so any net-new reasonless suppression trips the gate; LIT013 (`*-ok` suppression @@ -200,7 +202,8 @@ def cmd_check(base: str) -> None: "Remove the new violations, give each a reason (`# noqa: XXX # `, " "`# pyright: ignore[rule] # `, `# mutable-ok: `, " "`# cast-ok: `, `# guard-ok: `, `# kwargs-ok: `, " - "`# rebind-ok: `, `# writable-ok: `), or remove an equal " + "`# rebind-ok: `, `# writable-ok: `, " + "`# comprehension-ok: `), or remove an equal " "number elsewhere; the ceiling " "is the limit in type-discipline-budget.json." ) diff --git a/tests/test_litellm/test_check_type_discipline.py b/tests/test_litellm/test_check_type_discipline.py index d1eb3c400ca6..30e101e5c3cb 100644 --- a/tests/test_litellm/test_check_type_discipline.py +++ b/tests/test_litellm/test_check_type_discipline.py @@ -798,13 +798,46 @@ def test_suppression_on_outer_closing_line_does_not_silence_the_inner(tmp_path): assert [v.line for v in flagged] == [3] -def test_equal_span_suppression_belongs_to_the_inner_comprehension(tmp_path): +def test_equal_span_marker_suppresses_every_violating_comprehension_on_its_line(tmp_path): src = "y = [x for a in [z for i in ys for z in i] if a if x] # comprehension-ok: inner flatten is fine\n" f = tmp_path / "snippet.py" f.write_text(src, encoding="utf-8") - flagged = [v for v in checker.check_file(f) if v.code == "LIT014"] - assert len(flagged) == 1 - assert "1 `for` clauses and 2 `if` clauses" in flagged[0].message + violations = checker.check_file(f) + assert "LIT014" not in [v.code for v in violations] + assert "LIT013" not in [v.code for v in violations] + + +def test_single_line_outer_with_violating_inner_is_suppressed(tmp_path): + src = "y = [x for a in [z for i in ys for z in i] for x in a] # comprehension-ok: nested flatten is fine\n" + f = tmp_path / "snippet.py" + f.write_text(src, encoding="utf-8") + violations = checker.check_file(f) + assert "LIT014" not in [v.code for v in violations] + assert "LIT013" not in [v.code for v in violations] + + +def test_one_marker_suppresses_two_violating_sibling_comprehensions_on_its_line(tmp_path): + src = "y = [x for a in xs for x in a] + [x for a in ys for x in a] # comprehension-ok: paired flattens\n" + f = tmp_path / "snippet.py" + f.write_text(src, encoding="utf-8") + violations = checker.check_file(f) + assert "LIT014" not in [v.code for v in violations] + assert "LIT013" not in [v.code for v in violations] + + +def test_marker_on_a_non_violating_inner_line_suppresses_the_violating_outer(tmp_path): + src = ( + "y = [\n" + " x\n" + " for a in [z for z in ys if z] # comprehension-ok: flatten stays readable\n" + " for x in a\n" + "]\n" + ) + f = tmp_path / "snippet.py" + f.write_text(src, encoding="utf-8") + violations = checker.check_file(f) + assert "LIT014" not in [v.code for v in violations] + assert "LIT013" not in [v.code for v in violations] def test_violation_message_names_the_clause_counts(tmp_path): From d47f08ca5c41b0abf002b3f79c453f2cbc3b77bf Mon Sep 17 00:00:00 2001 From: mateo Date: Fri, 25 Sep 2026 01:33:05 +0000 Subject: [PATCH 7/7] test(lint): type tmp_path in LIT014 tests Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../test_check_type_discipline.py | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/test_litellm/test_check_type_discipline.py b/tests/test_litellm/test_check_type_discipline.py index 30e101e5c3cb..aee73825d636 100644 --- a/tests/test_litellm/test_check_type_discipline.py +++ b/tests/test_litellm/test_check_type_discipline.py @@ -690,33 +690,33 @@ def test_writable_ok_without_reason_is_lit005_and_does_not_suppress(tmp_path): # --------------------------------------------------------------------------- # -def test_two_for_clauses_are_flagged(tmp_path): +def test_two_for_clauses_are_flagged(tmp_path: Path): assert "LIT014" in _codes(tmp_path, "y = [x for a in xs for x in a]\n") -def test_two_ifs_on_one_generator_are_flagged(tmp_path): +def test_two_ifs_on_one_generator_are_flagged(tmp_path: Path): assert "LIT014" in _codes(tmp_path, "y = [x for x in xs if x if x > 1]\n") -def test_one_if_on_each_of_two_generators_is_flagged(tmp_path): +def test_one_if_on_each_of_two_generators_is_flagged(tmp_path: Path): assert "LIT014" in _codes(tmp_path, "y = [x for a in xs if a for x in a if x]\n") -def test_one_for_and_one_if_is_clean(tmp_path): +def test_one_for_and_one_if_is_clean(tmp_path: Path): assert "LIT014" not in _codes(tmp_path, "y = tuple(x for x in xs if x)\n") -def test_dict_set_and_generator_two_fors_are_each_flagged(tmp_path): +def test_dict_set_and_generator_two_fors_are_each_flagged(tmp_path: Path): assert "LIT014" in _codes(tmp_path, "d = {k: v for a in xs for k, v in a}\n") assert "LIT014" in _codes(tmp_path, "s = {x for a in xs for x in a}\n") assert "LIT014" in _codes(tmp_path, "g = (x for a in xs for x in a)\n") -def test_nested_comprehension_in_element_is_judged_separately(tmp_path): +def test_nested_comprehension_in_element_is_judged_separately(tmp_path: Path): assert "LIT014" not in _codes(tmp_path, "y = [[v for v in a] for a in xs]\n") -def test_comprehension_ok_with_reason_suppresses_lit014(tmp_path): +def test_comprehension_ok_with_reason_suppresses_lit014(tmp_path: Path): codes = _codes( tmp_path, "y = [x for a in xs for x in a] # comprehension-ok: flattens a stream of pairs, hot path\n", @@ -724,7 +724,7 @@ def test_comprehension_ok_with_reason_suppresses_lit014(tmp_path): assert "LIT014" not in codes -def test_comprehension_ok_on_any_spanned_line_suppresses_lit014(tmp_path): +def test_comprehension_ok_on_any_spanned_line_suppresses_lit014(tmp_path: Path): src = ( "y = [\n" " x for a in xs\n" @@ -734,7 +734,7 @@ def test_comprehension_ok_on_any_spanned_line_suppresses_lit014(tmp_path): assert "LIT014" not in _codes(tmp_path, src) -def test_comprehension_ok_after_the_closing_line_does_not_suppress(tmp_path): +def test_comprehension_ok_after_the_closing_line_does_not_suppress(tmp_path: Path): src = ( "y = [\n" " x for a in xs\n" @@ -749,7 +749,7 @@ def test_comprehension_ok_after_the_closing_line_does_not_suppress(tmp_path): assert [v.line for v in violations if v.code == "LIT013"] == [5] -def test_comprehension_ok_on_a_compliant_comprehension_is_an_unused_marker(tmp_path): +def test_comprehension_ok_on_a_compliant_comprehension_is_an_unused_marker(tmp_path: Path): f = tmp_path / "snippet.py" f.write_text( "y = tuple(x for x in xs if x) # comprehension-ok: kept for readability\n", @@ -760,13 +760,13 @@ def test_comprehension_ok_on_a_compliant_comprehension_is_an_unused_marker(tmp_p assert "LIT014" not in [v.code for v in violations] -def test_comprehension_ok_without_reason_is_lit005_and_does_not_suppress(tmp_path): +def test_comprehension_ok_without_reason_is_lit005_and_does_not_suppress(tmp_path: Path): codes = _codes(tmp_path, "y = [x for a in xs for x in a] # comprehension-ok\n") assert "LIT005" in codes assert "LIT014" in codes -def test_suppression_inside_inner_comprehension_does_not_silence_the_outer(tmp_path): +def test_suppression_inside_inner_comprehension_does_not_silence_the_outer(tmp_path: Path): src = ( "y = [\n" " x\n" @@ -784,7 +784,7 @@ def test_suppression_inside_inner_comprehension_does_not_silence_the_outer(tmp_p assert [v.code for v in violations if v.code == "LIT013"] == [] -def test_suppression_on_outer_closing_line_does_not_silence_the_inner(tmp_path): +def test_suppression_on_outer_closing_line_does_not_silence_the_inner(tmp_path: Path): src = ( "y = [\n" " x\n" @@ -798,7 +798,7 @@ def test_suppression_on_outer_closing_line_does_not_silence_the_inner(tmp_path): assert [v.line for v in flagged] == [3] -def test_equal_span_marker_suppresses_every_violating_comprehension_on_its_line(tmp_path): +def test_equal_span_marker_suppresses_every_violating_comprehension_on_its_line(tmp_path: Path): src = "y = [x for a in [z for i in ys for z in i] if a if x] # comprehension-ok: inner flatten is fine\n" f = tmp_path / "snippet.py" f.write_text(src, encoding="utf-8") @@ -807,7 +807,7 @@ def test_equal_span_marker_suppresses_every_violating_comprehension_on_its_line( assert "LIT013" not in [v.code for v in violations] -def test_single_line_outer_with_violating_inner_is_suppressed(tmp_path): +def test_single_line_outer_with_violating_inner_is_suppressed(tmp_path: Path): src = "y = [x for a in [z for i in ys for z in i] for x in a] # comprehension-ok: nested flatten is fine\n" f = tmp_path / "snippet.py" f.write_text(src, encoding="utf-8") @@ -816,7 +816,7 @@ def test_single_line_outer_with_violating_inner_is_suppressed(tmp_path): assert "LIT013" not in [v.code for v in violations] -def test_one_marker_suppresses_two_violating_sibling_comprehensions_on_its_line(tmp_path): +def test_one_marker_suppresses_two_violating_sibling_comprehensions_on_its_line(tmp_path: Path): src = "y = [x for a in xs for x in a] + [x for a in ys for x in a] # comprehension-ok: paired flattens\n" f = tmp_path / "snippet.py" f.write_text(src, encoding="utf-8") @@ -825,7 +825,7 @@ def test_one_marker_suppresses_two_violating_sibling_comprehensions_on_its_line( assert "LIT013" not in [v.code for v in violations] -def test_marker_on_a_non_violating_inner_line_suppresses_the_violating_outer(tmp_path): +def test_marker_on_a_non_violating_inner_line_suppresses_the_violating_outer(tmp_path: Path): src = ( "y = [\n" " x\n" @@ -840,7 +840,7 @@ def test_marker_on_a_non_violating_inner_line_suppresses_the_violating_outer(tmp assert "LIT013" not in [v.code for v in violations] -def test_violation_message_names_the_clause_counts(tmp_path): +def test_violation_message_names_the_clause_counts(tmp_path: Path): f = tmp_path / "snippet.py" f.write_text("y = [x for a in xs for x in a if x]\n", encoding="utf-8") messages = [v.message for v in checker.check_file(f) if v.code == "LIT014"]