Add basic_style.py to automate some style checking. #9603

As discussed recently in #9581 and in referenced past discussions #6839, #6156, and probably a bunch of other places too, there is concern about the long tail of effort and risk involved with these style fix-up PRs. To summarize what has been articulated by the experienced maintainers:

1. it is a lot of PR clutter for reviewers/maintainers to sift through
2. it impedes release management since style fix-ups often have difficult merge conflicts with more important content
3. they are sometimes hard to review and bugs can slip in
4. they permanently clutter up output from git blame and other history tools

My suggestion is that the ideal long-term strategy is to automate detection of these style issues as much as possible such that the CI system can auto-check every PR. This will ensure that the code is kept clean and consistent at source and is kept that way for all time.

Additionally, the message I am getting from what has been written is that the strategy for bringing this online needs to be done properly or not at all. It will require a conscientious effort to minimize the impact to ongoing activities and steps should be taken to make damn sure it is not the cause of any bugs. I have some thoughts, but collecting ideas from all around is good (and thanks for all that has already been written, it is quite helpful for thinking this through). I am content leaving this PR to sit until a larger collection of automation scripts are complete and we have a strategy that the maintainers are on board with.

This PR provides an example script for very basic style checks which are expressed as simple regular expressions:

STYLE_RULES = [
    {'title':   'No tabstops',
     'applies': ['*.c', '*.cpp', '*.h', '*.py', '*.sh'],
     'regex':   '\t',
     'fix':     '    '},
    {'title':   'No trailing whitespace on a line',
     'applies': ['*.c', '*.cpp', '*.h', '*.py', '*.sh'],
     'regex':   ' \n',
     'fix':     '\n'},
    {'title':   'No more than three consecutive newlines',
     'applies': ['*.c', '*.cpp', '*.h', '*.py', '*.sh'],
     'regex':   '\n\n\n\n',
     'fix':     '\n\n\n'},
    {'title':   'Do not end a line with a semicolon',
     'applies': ['*.py'],
     'regex':   ';\n',
     'fix':     '\n'},
    {'title':   'Do not end a line with two semicolons',
     'applies': ['*.c', '*.cpp', '*.h'],
     'regex':   ';;\n',
     'fix':     ';\n'},
]

The script provides a report subcommand that gives an overview of the state of the repository. It also provides a check command, which like in #9459, provides a shell status code (to make it easy to hook to TravisCI) and also gives pointers to what needs to be done to resolve the issue. Finally it provides a fix command which just does a search-and-replace according to the 'fix' string in the above STYLE_RULES dictionary.

The master branch has 141 files that fail this set of rules (19 + 93 + 16 + 13 + 0). However, I did not include the fixup in this PR due to the aforementioned reasons. The script can do the edit by invoking contrib/devtools/basic_style.py fix ..

clang-format, pep8, pylint, coverity, clang static analyzer, etc. are what I would define as 'non-basic' style checks that can be similarly automated (and might actually make this one redundant). Also, the copyright_header.py script in #9452/#9459 could be part of the end result too.

First, let's define an cosmetic issue as an issue for which a fix wouldn't change the possible code paths taken at runtime (think unchanged binary). Cosmetic issues would then include things such as trailing whitespace, tabs instead of spaces, etc.

Second,

• Why do we want to add a linter? Is the goal to catch cosmetic problems, non-cosmetic problems or both?
• What would be the incremental gain of adding a C++ linter?
• What would be the incremental gain of adding a Python linter?
• Do we strive to achieve the same level of assurance from Travis CI regarding the code quality after a passed Travis CI build regardless of the language of the code changed in a newly submitted PR?

In C++ land we can assume that most non-cosmetic problems ("real issues") that would be uncovered by a good linter are likely already covered by the compiler's warnings/errors. The incremental gain of adding a C++ linter would likely be limited to improved style consistency of the code base.

In Python land the situation is quite different. Adding a Python linter to the Travis CI build process would help us uncover real non-cosmetic issues that otherwise could go unnoticed until runtime (NameError:s, logic errors due to inconsistent indentation, etc.).

I think there is a really strong case for adding a Python linter that would be finely tuned to catch only real non-cosmetic issues and disregard all styling issues. That would increase the level of assurance given by a passed Travis CI build at a very low cost.

Closing.

The plan is to submit script likes this to https://github.com/bitcoin-core/bitcoin-maintainer-tools as that now seems to be the preferred location.

I will be touching up this script to share some code with the others and submitting it all as a PR there.