Skip to content

[checks] Create a better check-source.sh in Python #8086

@Eisenwave

Description

@Eisenwave

This is an idea that has been on my mind for a while, and which may be a decent productivity booster. There are various contributors to our repo, some of whom only do a little bit of work seasonally, and so having high-quality checks can make contributing quite a bit easier.

With the current checks, there are a few issues:

Maintainability problems

They are based on sed and grep, and our checking code is far from expressive:

# "Class" heading without namespace
for f in $texlib; do
    sed -n '/rSec[0-9].*{Class/,/\\end{codeblock}/{/\\begin{example}/,/\\end{example}/b;/\\begin{codeblock}/,/\(^namespace\)\|\(\\end{codeblock}\)/{s/template<[^>]*>//;/\(class\|struct\)[A-Za-z0-9_: ]*{/{=;p;};};}' $f |
    # prefix output with filename and line
    sed '/^[0-9]\+$/{N;s/\n/:/;}' | sed "s/.*/$f:&/"
done |
    fail 'No namespace around class definition' || failed=1

This kind of code borders on hieroglyphic. No offense to the author; it's just a limitation of shell tools.

Realistically, this should be something like:

# Somewhere in loop that goes over each Tex line ...
if inside_declaration_code_block and not inside_namespace:
    if re.match(r"(struct|class)\s+[a-zA-Z_]+\b", current_line) is not None:
        raise_error("No namespace around class definition.")

Error output quality

Secondly, the output quality could be improved:

-Error: future.tex:881:\iref must be flush against the preceding word, not at the start of a line: \iref{fs.path.member}:
+Error: future.tex:881:13: \iref must be flush against the preceding word, not at the start of the line:
+     | \pnum
+     | The following members are declared in addition to those members
+ 881 | specified in \iref{fs.path.member}:
+                   ^~~~~~~~~~~~~~~~~~~~~~ 

I find that getting a bit of a bigger code citation is helpful. It's also helpful to get suggested improvements. For example, in the constexpr static checks, the error message could include a code citation with static constexpr, i.e. with the problem fixed.

No linter control

There are numerous checks which we could silence via something like %NOLINTBEGIN(check-id) within certain locations. For example, we can have a global check which outlaws raw use of \textit (we usually want \exposid or various other semantic macros), but there are still 150 or so \textits, and it would take some time to dismantle them.

The easy way forward is to wrap those "bad sections" in NOLINTBEGIN, NOLINTEND, which would allow us to enable many more helpful checks in new markup, without worrying about not-yet-fixed old markup.

Other examples include enforcing ~\ref instead of \ref within certain regions (possibly disabling the checks in synopses or code comments), enforcing that \returns has to be wrapped in \begin{itemdescr}, etc.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions