Why not semgrep?
If someone adds a new function to the code six months later, they would be unaware of the rule and nothing will complain because code will still work and tests will pass. Linters will not help here because these are not style rules, but project-specific invariants. "All user-visible strings must go through translation." "This whitelist must match what is stated in the privacy documents." A linter does not know what your translation is. Therefore, create tests that check the code:
import ast, pathlib
def test_invariant():
offenders = []
for path in pathlib.Path("myapp").rglob("*.py"):
tree = ast.parse(path.read_text(encoding="utf-8"), str(path))
for node in ast.walk(tree):
if violates(node):
offenders.append(f"{path}:{node.lineno}")
assert not offenders, f"rule broken at: {offenders}"
That's the whole pattern. Everything below is just violates().
Example 1: no untranslated UI strings
Every string the user sees should come from t("some.key"), never a literal. Easy to enforce, easy to forget.
UI_SETTERS = {"setText", "setToolTip", "setTitle"}
def hardcoded_ui_strings(source: str, filename: str = "<mem>"):
"""(lineno, text) for every literal passed straight into a UI setter."""
found = []
for node in ast.walk(ast.parse(source, filename)):
if not isinstance(node, ast.Call):
continue
if not (isinstance(node.func, ast.Attribute)
and node.func.attr in UI_SETTERS):
continue
for arg in node.args:
if isinstance(arg, ast.Constant) and isinstance(arg.value, str) and arg.value.strip():
found.append((arg.lineno, arg.value))
return found
self.btn.setText(t("btn.save")) is not reported, since the argument is a Call and not a Constant. self.lbl.setText("Save changes") is reported.
This one revealed the bug to me which unit tests could never see, that is, a warning message that had been completely developed, correct, and tested for the application, but only in one language out of the six ones that were supposed to be used.
Example 2: make the naming convention be the rule
This is the one I'd actually argue for. Say you have a dict of operations that cost the user money:
BILLABLE_OPS = {
"cloud_assist": 1,
"cloud_render": 5,
}
The naive guardian compares the dict against a known-good copy of itself, which is self-defeating because you are only claiming that the list equals the list, and whoever adds an entry just updates both places.
Instead do it by coding the rule into the name where everything that one can bill is cloud_* and the guardian does only the check for the prefix without checking for what follows.
def billable_ops_violating_prefix(source: str, dict_name="BILLABLE_OPS", prefix="cloud_"):
bad = []
for node in ast.walk(ast.parse(source)):
if not isinstance(node, ast.Assign) or not isinstance(node.value, ast.Dict):
continue
if not any(isinstance(t, ast.Name) and t.id == dict_name for t in node.targets):
continue
for key in node.value.keys:
if isinstance(key, ast.Constant) and isinstance(key.value, str):
if not key.value.startswith(prefix):
bad.append(key.value)
return bad
The guardian now applies even when operations are not yet present. Usage of "local_export": 2 does not work. Billing something local would force you to name it cloud_export instead — a lie simple enough for a human reviewer to catch.
The overall point is to use the naming scheme that would allow for the rule to be checked automatically as the test checks the property instead of the exact data.
Example 3: a constant that will not deviate from the document
You state in your privacy statement that these exact fields will be collected. The information is coded elsewhere. The latter will inevitably differ, not because anyone did anything wrong, but more likely because of an ordinary Tuesday afternoon.
def declared_fields(source: str, name="TELEMETRY_FIELDS"):
for node in ast.walk(ast.parse(source)):
if isinstance(node, ast.Assign):
if any(isinstance(t, ast.Name) and t.id == name for t in node.targets):
return {n.value for n in ast.walk(node.value)
if isinstance(n, ast.Constant) and isinstance(n.value, str)}
return set()
def documented_fields(markdown: str):
return {line.strip().lstrip("-* ").strip("`")
for line in markdown.splitlines()
if line.strip().startswith(("- `", "* `"))}
Then assert both directions:
assert declared_fields(src) == documented_fields(doc)
Both directions matters. One direction catches "we collect something undocumented" - the compliance problem. The other catches "we document something we no longer collect" - the trust problem, and the one people forget.
When this is the wrong tool:
The niche is narrow but real: project-specific structural invariants that no off-the-shelf tool knows about.
Two gotchas that bit me:
Make a guardian that finds nothing loudly report that it found nothing. A typo in the path means it's not scanning any files, and silently succeeding. Assert that you scanned a reasonable number of files, or that the one known-bad fixture is indeed being caught by the guardian. A guardian nobody runs is not a guardian at all, and one that scans zero files is even worse than that, because it reports success.
Do not have a comment-based disablement of checks. I had a case where writing a comment in a certain way caused the line to be ignored, and it was propagated around the codebase. If a check needs exceptions, list them in the test as paths to scan, so that adding a new exception is visible in the git diff.
Curious if anyone else does something similar, and what they tend to check. The only somewhat similar thing I've seen in the wild is import-linter with rules for layering, which is more or less the same idea but for a narrower class of checks.
Why not semgrep?
While I recognize the need for these tests, I feel like much of these can be captured via pydantic and choosing to parse instead of validate.
Why not just tell your AI code reviewer to enforce the rule, in plain English?
Why do you think it might be a bad idea to make tests non-deterministic
eww
Your submission has been automatically queued for manual review by the moderation team because it has been reported too many times.
Please wait until the moderation team reviews your post.
I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.