[checks] Implement Python checks and fix some violations by eisenwave · Pull Request #9146 · cplusplus/draft

eisenwave · 2026-06-17T18:17:38Z

I've mentioned the option to improve the checks in Croydon to @tkoeppe, and he seemed generally open to the idea.

In summary, this PR converts all the checks we have in check-source.sh (plus some additional once adopted from check-output.sh) and turns them into a Python script. The benefits are:

Much nicer formatting for errors. You get VSCode-friendly source locations that you can use to navigate directly to the line and column in an editor. You also get a citation of the source with the affected column span highlighted, so you're not left guessing where the error is located.
No more hieroglyphic sed and grep piping. The code is relatively simple Python. It still uses a bunch of regular expressions for a lot of checks; the greatest benefits are in the more complex checks.
Ability to write test cases for checks using EXPECTCHECKNEXTLINE.
Ability to enable and disable individual checks for source spans using NOCHECKBEGIN and other directives. This makes it possible for us to add much more aggressive checks that would have previously been infeasible because they generate too much noise in old TeX sources.
- A good example of that is in this PR, where I disable the check for \tcode{\exposid{...}} in a part of numerics.tex because there are too many violations.
- We could also use this to e.g. disallow use of \textit in "newer" TeX ranges for example without having to fix every single violation. We could also use it to catch many "bad words" or "bad phrases" that appear too often to be checked right now, but that we don't want perpetuated.
- We could also use this to enforce a little bit of auto-enforcement of semantic line breaks (such as requiring a new line after a sentence period) without needing to fix the countless existing violations.
With check suppression, it is possible to suppress checks that would have inevitably failed during motions. For example, if there is a dangling \ref to a section that is introduced by another paper, you can still get a green build by suppressing the \ref check (that is, at least for the check-source checks).

There are a few notable new checks that make things a lot easier in the future:

base-env-balancing checks for balancing of \begin and \end commands. It's quite easy to get it wrong, and when you do, it tends to blow up after minutes during the TeX build in bizarre and hard-to-debug ways.
base-unknown-command checks for use of TeX commands that have never appeared before in the code base. It's very unlikely that this is intentional in most PRs, and very likely that someone just wrote \tocde instead of \tcode or something else stupid. If we need a new TeX command, we can just add it to the list.

eisenwave · 2026-06-17T19:07:32Z

Some more neat examples:

eisenwave · 2026-06-17T19:43:55Z

Latest innovation:

Tsche · 2026-06-19T11:27:05Z

+%EXPECTCHECKNEXTLINE(base-tcode-exposid)
+\tcode{\exposid{snake-case}}
+%EXPECTCHECKNEXTLINE(base-tcode-exposid)
+\tcode{\exposid{kebak-case}}


did you mean "kebab-case"? Also, that snake-case one isn't actually snake_case.

Tsche · 2026-06-19T11:29:28Z

 *.synctex.gz
 *.synctex*
 .check.stamp
+__pycache__


While it won't matter until we have multiple Python modules, you might want to also ignore *.pyc, *.egg and *.egg-info.

Tsche · 2026-06-19T11:32:43Z

+    "reportUnknownParameterType": false,
+    "reportUnusedImport": true,
+    "reportUnusedVariable": true,
+    "pythonVersion": "3.11"


why 3.11 specifically? That version of Python is EoL in about a year.

For the CI we use Ubuntu 24.04. This ships with Python 3.12 - the current Python release is 3.14

I just blindly copied and pasted this from another project, so no particular reason.

Tsche · 2026-06-19T11:33:58Z

+      - name: setup Node
+        uses: actions/setup-node@v6
+        with:
+          node-version: "20"
+
+      - name: install pyright
+        run: npm install -g pyright


I'm not sure pyright is a good call. There are other alternatives that do not require pulling in node as dependency.

I just use it because it integrates nicely with VSCode out of the box. What would you recommend instead?

Tsche · 2026-06-19T11:36:24Z

+# ==================================================================================================
+# Lookup tables
+# ==================================================================================================


I'm really not a fan of those section comment headers. Would it be viable to chop this >2k LoC module up into several smaller modules?

There may be some merit to pulling the lookup tables into a separate Python file just because they are a bit of clutter and make the script look a lot bigger than it actually is. The rest is just around 1K lines, and modulerization probably wouldn't improve readability much.

Tsche · 2026-06-19T12:12:19Z

+        message: str,
+    ) -> None:
+        """Report a failure at `self.file_path`."""
+        emit_check_failure(


emit_check_failure is only called from within classes deriving from Check. Can you fold it into fail and consistently use this one?

Tsche · 2026-06-19T12:18:22Z

+class Check(ABC):
+    """Base class for all checks."""
+
+    id: str = ""


id is a Python builtin, please don't use it for identifiers. Also I'm not entirely sure what you're trying to do here - this id is a class attribute not an instance attribute.

Did you mean to say

class Check(ABC): uid: str def __init__(self, uid: str = ""): self.uid = uid # ... class Foo(Check): def __init__(self, uid: str, **whatever): super().__init__(uid)

?

I didn't even know that there is such a thing as class attributes and instance attributes, but okay. I thought the "" is a default member initializer or something, and I think I used that at some point but then it became pointless.

Tsche · 2026-06-19T12:20:21Z

+    file_locations.clear()
+
+    for fp in tex_files:
+        file_locations[os.path.relpath(fp)] = fp


please don't mix os.path and pathlib usage. pathlib.Path exposes this functionality as relative_to().

Tsche · 2026-06-19T12:21:05Z

+                if directive == "BEGIN":
+                    if cid == "*":
+                        active.clear()
+                    else:
+                        active = {a for a in active if not fnmatch.fnmatch(a, cid)}
+                elif directive == "END":
+                    if cid == "*":
+                        active = set(all_ids)
+                    else:
+                        active |= {a for a in all_ids if fnmatch.fnmatch(a, cid)}
+                elif directive == "NEXTLINE":
+                    if cid == "*":
+                        for c in all_ids:
+                            skip_next[c].add(line_num + 1)
+                    else:
+                        matched = {a for a in all_ids if fnmatch.fnmatch(a, cid)}
+                        for c in matched:
+                            skip_next[c].add(line_num + 1)


you have some code duplication here

Tsche · 2026-06-19T13:19:36Z

+            file=sys.stderr,
+        )
+
+    exit_code = 1 if num_failures > 0 else 0


you can just do num_failures > 0 here. If you insist on getting an int, do int(num_failures > 0) but it's not necessary.

eisenwave · 2026-06-19T13:37:05Z

@Tsche thanks a lot for the review. It'll take some time to clean it all up.

jensmaurer · 2026-06-19T14:26:09Z

+%NOCHECKBEGIN(base-tcode-exposid)
 Every type in the parameter pack \tcode{Flags} is one of \tcode{\exposid{convert-flag}},
 \tcode{\exposid{aligned-flag}}, or \tcode{\exposid{over\-aligned-\brk{}flag}<N>}.


I think we can just remove the \tcode wrapping around \exposid here?

Yeah, we could. It would be something like 10 separate fixes.

I think I'll just do a surgical fix in a separate PR for these cleanups so that this PR doesn't have to clean up any existing sources.

The problem with the existing check is that the regex pattern doesn't work on hyphenated identifiers.

jensmaurer · 2026-06-19T14:26:31Z

 \pnum
-\recommended Implementations should support implicit conversions between
+\recommended
+Implementations should support implicit conversions between


Please move all specific fixes to a separate commit.

eisenwave added 2 commits June 17, 2026 20:05

Implement Python checks and fix some violations

51dfd10

Improve naming consistency

50fc65d

eisenwave requested a review from jensmaurer June 17, 2026 18:28

Add definition checks for \ref and \iref

9e27d06

eisenwave changed the title ~~Implement Python checks and fix some violations~~ [checks] Implement Python checks and fix some violations Jun 17, 2026

Tsche suggested changes Jun 19, 2026

View reviewed changes

jensmaurer requested changes Jun 19, 2026

View reviewed changes

Conversation

eisenwave commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eisenwave commented Jun 17, 2026

Uh oh!

eisenwave commented Jun 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eisenwave commented Jun 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eisenwave commented Jun 17, 2026 •

edited

Loading