From b76c3facdf809644ab8207288b82a7cf214c2dca Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 4 Oct 2026 17:35:37 -0700 Subject: [PATCH] fix(code-health): profile regex rules match executable code, not prose F21 (andrexibiza): PS-P05/PS-P06 ran their regexes over raw source lines, so a comment such as `# Avoid env = os.environ.copy(); use the scoped builder.`, the same sentence in a docstring, or a comment warning against `os.getenv("DISCORD_TOKEN")` produced a blocking hit although nothing builds an env or reads a credential. Before the regexes run, comment tokens (tokenize) and string statements (ast: docstrings and bare string examples) are blanked in place, preserving every line number and line break. Ordinary string literals inside code are kept, so the env-key argument of `os.getenv("DISCORD_TOKEN")` still matches; the real `env = os.environ.copy()` / `dict(os.environ)` / platform getenv stay flagged. If tokenizing fails, the raw lines are used (more hits, never fewer). --- scripts/code_health/measure.py | 58 +++++++++++++++++++++++++-- tests/scripts/test_code_health_cli.py | 27 +++++++++++++ 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/scripts/code_health/measure.py b/scripts/code_health/measure.py index 184895dad2..3ff7692d68 100644 --- a/scripts/code_health/measure.py +++ b/scripts/code_health/measure.py @@ -57,6 +57,55 @@ def _python_comments(text: str) -> dict[int, str]: return comments +# Characters str.splitlines() breaks on; never blanked, so the blanked text keeps its line numbers. +_LINE_BREAKS = frozenset("\n\r\x0b\x0c\x1c\x1d\x1e\x85\u2028\u2029") + + +def _string_statements(tree: ast.Module): + """Docstrings and other bare string statements: prose, never executed.""" + for node in ast.walk(tree): + if (isinstance(node, ast.Expr) and isinstance(node.value, ast.Constant) + and isinstance(node.value.value, str)): + yield node + + +def _executable_lines(text: str, tree: ast.Module) -> list[str]: + """``text.splitlines()`` with comments and string statements blanked out. + + The profile regex rules describe operations (`env = os.environ.copy()`), so a comment or a + docstring that mentions one must not count as doing it. String literals inside code stay: + `os.getenv("DISCORD_TOKEN")` is matched by its argument. + """ + rows = [list(line) for line in io.StringIO(text).readlines()] # ast/tokenize line numbering + + def blank(line: int, start: int, end: int | None = None) -> None: + row = rows[line - 1] + for col in range(start, len(row) if end is None else end): + if row[col] not in _LINE_BREAKS: + row[col] = " " + + def char_col(line: int, byte_col: int) -> int: # ast columns are UTF-8 byte offsets + return len("".join(rows[line - 1]).encode("utf-8")[:byte_col].decode("utf-8", "ignore")) + + try: + for tok in tokenize.generate_tokens(io.StringIO(text).readline): + if tok.type == tokenize.COMMENT: + blank(tok.start[0], tok.start[1], tok.end[1]) + except (tokenize.TokenError, SyntaxError): + return text.splitlines() # fail closed: every line stays visible to the rules + for node in _string_statements(tree): + last = node.end_lineno or node.lineno + end = char_col(last, node.end_col_offset or 0) + if last == node.lineno: + blank(node.lineno, char_col(node.lineno, node.col_offset), end) + continue + blank(node.lineno, char_col(node.lineno, node.col_offset)) + for line in range(node.lineno + 1, last): + blank(line, 0) + blank(last, 0, end) + return "".join("".join(row) for row in rows).splitlines() + + class Measurer: def __init__(self, repo: Path, ruff: list[str], known_env: set[str]) -> None: self.repo = repo @@ -116,15 +165,18 @@ class Measurer: if rule_applies(RULES_BY_ID[rule_id], fm.path): for row in sorted(set(checker(rules_tree, self.ctx))): fm.add_hit(rule_id, scopes.scope(row), row) - self._regex(fm, scopes) + self._regex(fm, scopes, text, tree) - def _regex(self, fm: FileMeasure, scopes) -> None: + def _regex(self, fm: FileMeasure, scopes, text: str, tree: ast.Module) -> None: + code_lines: list[str] | None = None for rule_id, pattern, path_re in self.regex_rules: if not rule_applies(RULES_BY_ID[rule_id], fm.path): continue if path_re and not path_re.search(fm.path): continue - for index, line in enumerate(fm.lines, start=1): + if code_lines is None: + code_lines = _executable_lines(text, tree) + for index, line in enumerate(code_lines, start=1): if pattern.search(line): fm.add_hit(rule_id, scopes.scope(index), index) diff --git a/tests/scripts/test_code_health_cli.py b/tests/scripts/test_code_health_cli.py index b40ca698b9..1df1b2f9ea 100644 --- a/tests/scripts/test_code_health_cli.py +++ b/tests/scripts/test_code_health_cli.py @@ -153,6 +153,33 @@ def test_json_stdout_is_json_on_every_path(tmp_path, capsys, case, switch, files assert "code health:" in err # the human explanation moves to stderr +# --- F21: profile regex rules see executable code, not prose ----------------------------------- + + +_FUNC = "import os\n\n\ndef build(cmd):\n{} return cmd\n" + + +@pytest.mark.parametrize("body, blocks", [ + (" # Avoid env = os.environ.copy(); use the scoped builder.\n", False), + (' """Avoid env = os.environ.copy(); use the scoped builder."""\n', False), + (' """Builder.\n\n Never env = os.environ.copy() here.\n """\n', False), + (" # don't read os.getenv(\"DISCORD_TOKEN\") directly\n", False), + (" cmd = [cmd] # not os.environ.get('SLACK_TOKEN')\n", False), + (" 'inert example: env = os.environ.copy()'\n", False), + # positive controls: the real operations, including a literal env-key argument + (" env = os.environ.copy()\n cmd = (cmd, env)\n", True), + (" token = os.getenv(\"DISCORD_TOKEN\")\n cmd = (cmd, token)\n", True), + (" env = dict(os.environ) # copy it\n cmd = (cmd, env)\n", True), +]) +def test_profile_regex_rules_ignore_comments_and_docstrings(tmp_path, capsys, body, blocks): + repo, base = _ratchet_repo(tmp_path) + head = _commit(repo, {"pkg/c.py": _FUNC.format(body)}) + code = cli.run(repo, base, head) + out = capsys.readouterr().out + assert code == (1 if blocks else 0), out + assert ("PS-P05" in out or "PS-P06" in out) == blocks, out + + # --- allow comments may list several rules ----------------------------------------------------