Add more pre-commit hooks and run them in CI

[MarkKoz]

As per python-discord/organisation#138, new hooks were added for pre-commit and they will run in CI too. The pipenv run lint script will now run all the new hooks, including flake8.

Hooks added

A couple of these hooks automatically apply fixes. However, they still report failure and leave any changes they make uncommitted. Therefore, the user has to commit the automatic fixes.

• check-merge-conflict - Check for files that contain merge conflict strings.
• check-toml - Attempts to load all toml files to verify syntax.
• check-yaml - Attempts to load all yaml files to verify syntax.
• end-of-file-fixer - Makes sure files end in a newline and only a newline.
• mixed-line-ending - Replaces mixed line endings with LF.
• trailing-whitespace - Trims trailing whitespace.
• python-check-blanket-noqa - Enforce that noqa annotations always occur with specific codes

CI

The pre-commit venv is cached. There should only be a cache miss if the .pre-commit-config.yaml file changes or if the location of the Python interpreter changes (more realistically, if the Python version changes). Maybe Azure wipes the cache sometimes too, but in that case pre-commit will simply re-create the environment.

ELA edit:
Also closes #775

[SebastiaanZ]

It's worth noting that moving flake8 out of the virtual environment also comes with a disadvantage for local development. While it's still going to be possible to run the linter from the command line using pre-commit, it will break editors that use flake8 to lint within the editor. There may be ways to deal with this, as I haven't looked into whether or not it's possible to configure editors in such a way that they're able to use pre-commit's flake8, but this will be a more complicated and non-standard set-up.

For me, personally, it probably means that I'm going to maintain a separate dependency listing locally to be able to create a pipenv or other Python virtual environment where flake8 and the plugins we use are installed to be able to use a linter in editor.

For reference, this is what you'd get with a default linter set-up in Sublime Text:

SublimeLinter: #2 flake8 test_base.py ERROR:
============================================

/home/sebastiaan/.local/share/virtualenvs/bot-I8fh9oki/bin/python: No module named flake8

[lemonsaurus]

I think what @SebastiaanZ is saying here is a dealbreaker for removing these dependencies from the pipfile.

I think it would be better to maintain duplicate dependencies in two files than to require a weird non-standard setup to get linting working on sublime or vscode. These are popular editors, and it should be frictionless to set up linting in our repos.

[MarkKoz]

OK. What do you all think of using this hack in CI?

printf '%s\n%s' '#!/bin/bash' '"${@:2}"' >> pipenv && chmod +x pipenv

It creates a mock pipenv run executable. It will ignore the first arg (i.e. ignore the run in pipenv run) and interpret the rest as if it was a command. It'll get prepended to PATH somewhere so that when pre-commit runs the flake8 hook, it will prefer it as the pipenv executable over the normal one.

[sco1]

Per #775 I've also added pep8-naming to the linting toolchain and updated the codebase to fix the handful of new linting issues.

[SebastiaanZ]

Looks good and this works well with local linting set-ups.

This PR does make me realize that I badly need to update the tests README.

[lemonsaurus]

@MarkKoz I like it. It's hacky and clever and I don't mind at all because you've documented it well and it only happens in the CI. Excellent work.