Port lint scripts to Python #24783

To prevent endless issues when running the linters locally (say, on specific versions of bash, or system tools having certain names, or a certain implementation of sed, etc) , as well as for maintainability/reviewability, it's preferred for linter scripts to be written in Python.

Here is a list of scripts, feel free to pick up any that haven't been ported:

• test/lint/check-doc.py (added in #7280, was always Python)
• test/lint/git-subtree-check.sh (in progress in #25039)
• test/lint/lint-all.sh (ported in #24982)
• test/lint/lint-assertions.sh (ported in #24856)
• test/lint/lint-circular-dependencies.sh (ported in #24915)
• test/lint/lint-cpp.sh (removed in #24785)
• test/lint/lint-files.py (added in #21740, was always Python)
• test/lint/lint-format-strings.sh (ported in #24802)
• test/lint/lint-git-commit-check.sh (ported in #24853)
• test/lint/lint-include-guards.sh (ported in #24902)
• test/lint/lint-includes.sh (ported in #24895)
• test/lint/lint-locale-dependence.sh (ported in #24932)
• test/lint/lint-logs.sh (ported in #24849)
• test/lint/lint-python-dead-code.py (ported in #24778)
• test/lint/lint-python-mutable-default-parameters.sh (ported in #24800)
• test/lint/lint-python.sh (ported in #24794)
• test/lint/lint-python-utf8-encoding.sh (ported in #24916)
• test/lint/lint-qt.sh (removed in #24790)
• test/lint/lint-shell-locale.sh (ported in #24929)
• test/lint/lint-shell.sh (ported in #24840)
• test/lint/lint-spelling.py (ported in #24766)
• test/lint/lint-submodule.sh (ported in #24803)
• test/lint/lint-tests.sh (ported in #24815)
• test/lint/lint-whitespace.sh (ported in #24844)
• test/lint/run-lint-format-strings.py (added in #13705, was always Python)

Although I try to keep this post up to date, before starting on a linter, please first do a github search in PRs to see if a PR for that already exists.

Guidelines

• Don't forget to update test/README.md when the name of a script changes.
• Avoid shell=true on subprocess calls. Remember that the point here is to avoid shell ambiguities.
• If possible, avoid calling out to subprocesses at all and use what Python APIs provide (say, grep sed awk sha256sum). An exception if this would introduce further dependencies that need to be installed. These are OK to use subprocess for:
  • git subcommands including git grep—the alternative would be to require an extra dependency on a Python git module, also, git can be assumed to be the same implementation everywhere
  • Other scripts in the source tree.
  • External linters (vulture, codespell, etc) that provide only a command-based interface.
• Some further recommendations about Python usage here: #24766#pullrequestreview-932953476

Useful skills:

• Python
• Being able to read shell script

Want to work on this issue?

For guidance on contributing, please read CONTRIBUTING.md before opening your pull request.

Picking lint-format-strings.sh to start with.

Picking lint-format-strings.sh to start with.

Thanks! It should be a fairly straightforward one, as it already calls into a Python script, run-lint-format-strings.py, to do the brunt of the work. Could turn that into a module instead of subprocess. Mostlyt a matter of replacing the grep with Python code.

I'll try lint-python-mutable-default-parameters.sh

Reminder that review is more important than porting, so I encourage and recommend that anyone who opened a pull request to port to also review at least one other port pull request.

I'd like to port lint-whitespace.sh

I'll do lint-tests.sh

Picking lint-shell-locale.sh next

I'll give a stab at test/lint/lint-shell.sh.

Also, if you port a script, make sure to test that a failure looks similar before and after the change. Ideally, leave a comment with the output copy-pasted or a screenshot in the pull request.

Giving a port of /test/lint/lint-git-commit-check.sh a go next..

I'll port the test/lint/lint-assertions.sh

Working on a port of /test/lint/lint-includes.sh now..

Since python 3.5, run() API is available in subprocess module. In a situation where one can use both the APIs run() and check_output() is there a preference to one over the other? Backward compatibility maybe?

Giving test/lint/lint-include-guards.sh a shot.

Since python 3.5, run() API is available in subprocess module. In a situation where one can use both the APIs run() and check_output() is there a preference to one over the other? Backward compatibility maybe?

It's fine to use either imo. But the general tendency I'm seeing is that check_output is used in the linters when stdout is needed. run is more flexible, and also allows for other combinations of settings.

Although I try to keep the OP up to date, before starting on a linter, please first do a github search in PRs to see if a PR for that already exists.

Hey 👋 I'll port test/lint/lint-circular-dependencies.sh

Edit: done (https://github.com/bitcoin/bitcoin/pull/24915)

I'll be working on the port of lint-locale-depence.sh next..

I've removed commit-script-check.sh from the list as it's a) not a linter b) messes with the git tree c) intentionally invokes shell script, so there's little to pythonize about it unless we change scripted-diffs to use Python, which is way outside scope of this issue.

Not sure what the status of "extended-lint" is. Can this be removed?

Not sure what the status of "extended-lint" is. Can this be removed?

Good point. It doesn't seem to be invoked as part of the CI at all?

I guess the cppcheck linter can be part of the normal lints, or removed completely? I'm a bit confused. I would have assumed it was still active as of #20530, when the cppcheck version was upgraded. But also looks like fa2941bbf47a8a6b79b8db4a87e1aedcf6a29a5e already removed it from the CI in 2019.

Anyhow, removed from the list, there's no point in wasting anyone's time porting it if it's probably going away.

I don't have access to logs at this point, but I am assuming I removed it because it was failing.

I'd like to convert test/lint/git-subtree-check.sh.

Looks like we're almost done here!

I'll convert the test/lint/lint-all.sh to finish it!

I'm going to close this issue. The test/lint/git-subtree-check.sh port is still under review, but all the linters have been ported and merged, so there's no need for this issue to attract attention anymore.

Thanks everyone for the help!