test: Move linters to test/lint, add readme #13281

This moves the checks and linters from devtools to a subfolder in test. (Motivated by my opinion that the dev tools are mostly for generating code and updating the repo whereas the linters are read-only checks.)

Also, adds a readme to clarify that checks and linters are only meant to prevent bugs and user facing issues, not merely stylistic preference or inconsistencies. (This is motivated by the diversity in developers and work flows as well as existing code styles. It would be too disruptive to change all existing code to a single style or too burdensome to force all developers to adhere to a single style. Also note that our style guide is changing, so locking in at the wrong style "too early" would only waste resources.)

The test/lint/ move is an obvious utACK, but the "add readme" part is troublesome:

The following addition …

This folder contains lint scripts that aim to prevent direct or indirect user facing bugs.
Patches that do not pass the linter checks are usually not accepted.

Note that the [recommended style](/doc/developer-notes.md) and other stylistic
inconsistencies should not be enforced unless they could result in user facing bugs.

… formalizes a new "automation skeptical" (for lack of a better word!) policy which in the light of recent discussions on linting (see for example the discussion in @kallewoof's PR #12879) does not seem to be backed by consensus.

Personally I subscribe to the automation/linting philosophy outlined by @sipa in #12879:

If there is a widespread opinion among contributors and maintainers that a particul guideline is worth the burden, then I think it should be put in the style guide - and if it is, it sounds great to enforce it.

I suggest the following text instead:

This folder contains lint scripts that automate the review process in some form.

If there is a widespread opinion among contributors and maintainers that a particular
guideline is worth the burden, then it should be put in the style guide - and if it is,
it is probably a good candidate for linting. However, the enforcement via linting should
be limited to issues that can reliably identified without false positives or false
negatives.

Patches that do not pass the linter checks are usually not accepted.

@practicalswift That was my first draft, but I realized that the style guide includes not only enforceable guidelines, but also a bunch of recommendations, that are fine to violate. For example the "Miscellaneous" coding style or the reference to the clang-format rules. While most of them can be reliably identified without false positives or false negatives, I think we should avoid enforcing them.

Also note that the wording is not strict "[...] other stylistic inconsistencies should not be enforced [...]"

We can always add a linter for stuff that is uncontroversial but not preventing any user facing bugs. See for example the linter that checks for trailing white space.

test/liny/lint-python.sh also catches python scripts in contrib/ and share/ - fine I guess?

@promag I guess that is fine or maybe even desired.

Conecpt ACK moving the linters to test/lint. No need IMO to bundle that change with a new readme setting a policy about what should be linted.

If you want to change the linter policy, then it seems appropriate to raise it in a meeting. For this PR, I recommend you limit it to just moving the files.

@practicalswift @jnewbery Done

@MarcoFalke Looks good! :-)

utACK fa3c910bfeab00703c947c5200a64c21225b50ef

utACK fa3c910bfeab00703c947c5200a64c21225b50ef

utACK fa3c910

contrib/devtools/README.md still documents check-doc, which has moved.

utACK https://github.com/bitcoin/bitcoin/commit/fa3c910bfeab00703c947c5200a64c21225b50ef , modulo Empact's comment about the check-doc documentation.

utACK fa3c910

I'm just going to merge this, so many utACKs for the current state.