Adopting use of flake8 plugins

[pradyunsg]

In general, there's some flake8 plugins that might be useful to us. In case of some of these plugins, it means adopting a newer code style so we should probably start with that discussion.

• flake8-commas PyPI: enforce commas after the last item of multiline literals/function calls.
• flake8-mutable PyPI: enforce not using mutable values as defaults
  • TODO: check if flake8 does this check by default
• flake8-logging-format PyPI: check that strings aren't formatted while passing to logging (should depend on logging doing the interpolation)
• flake8-pep3101 PyPI: enforces the use of .format instead of % in the codebase
• flake8-gramex PyPI: checks for magic numbers and required encoding for calls to file opening functions + 2/3 more (but we'll ignore those)
  • It's documentation is lacking

I think some of these might be good additions. The only thing is that adopting them would mean going through the entire codebase to make things consistent according to them wherever there's a violation (which is kind of the point of doing this).

Thoughts @pypa/pip-committers?

[xavfernandez]

flake8-commas 👍
flake8-mutable and flake8-logging-format I'm wondering if pylint does not already do that (and hence, wouldn't we want to also use pylint in our lint target ?)
flake8-pep3101 I'm 0-.
flake8-gramex magic numbers and encoding seem like sensible checks.
I've also heard of flake8-bugbear with some additionnal checks: https://github.com/PyCQA/flake8-bugbear#list-of-warnings which currently output this list of warnings:

./src/pip/_internal/wheel.py:509:20: E741 ambiguous variable name 'l'
./src/pip/_internal/wheel.py:531:5: E722 do not use bare except'
./src/pip/_internal/wheel.py:656:17: E722 do not use bare except'
./src/pip/_internal/wheel.py:688:13: E722 do not use bare except'
./src/pip/_internal/wheel.py:701:9: E722 do not use bare except'
./src/pip/_internal/basecommand.py:243:9: E722 do not use bare except'
./src/pip/_internal/compat.py:220:13: E722 do not use bare except'
./src/pip/_internal/compat.py:231:13: E722 do not use bare except'
./src/pip/_internal/req/__init__.py:51:13: E722 do not use bare except'
./tests/unit/test_req_uninstall.py:124:9: E741 ambiguous variable name 'l'
./tests/unit/test_req_file.py:642:13: E722 do not use bare except'

[pradyunsg]

Actually, all of those are errors from a newer version of flake8. I have a local branch that fixes that. It might also be a PR already but in case it isn't, I'll make one for that in the evening today.

(on mobile, it's 3am, I'm too sleepy to do anything)
(also hi o/)

[xavfernandez]

Actually, all of those are errors from a newer version of flake8.

Indeed >.<

[pradyunsg]

and hence, wouldn't we want to also use pylint in our lint target

Yeah. FWIW, pylint doesn't really have sane defaults and it just comes down to actually sitting down and deciding what checks should be a part of your code-style and then enforcing them. I'm personally not convinced that to do this though.

Though, I do think if someone wants to put in the effort to do that, it'd be a net positive at the end of it for everyone.

I have a local branch that fixes that.

#5543

[pradyunsg]

Bumping this, to check if folks are still interested in these checks.

[pradyunsg]

Noting flake8-printf-formatting from #6973.

[pradyunsg]

If anyone wants to work on this, the way we'd add a flake8 plugin to our linting setup, is to add an additional_dependencies key to our .pre-commit-config.yaml file:

        additional_dependencies: [
            'flake8-bugbear==19.8.0',
            'flake8-coding==1.3.2',
            'flake8-comprehensions==3.0.1',
            'flake8-debugger==3.2.1',
            'flake8-deprecated==1.3',
            'flake8-docstrings==1.5.0',
            'flake8-pep3101==1.2.1',
            'flake8-polyfill==1.0.2',
            'flake8-print==3.1.4',
            'flake8-quotes==2.1.1',
            'flake8-string-format==0.2.3',
        ]

Note: Do not use the exact versions/plugins as in the example above -- please check what plugins have been discussed in this issue and what their current versions are.

Relevant: pre-commit/pre-commit-hooks#311

[webknjaz]

@pradyunsg I recently discovered a flake8 plugin collection that I mostly like: https://wemake-python-stylegui.de/. It even supports a legacy-first mode (where it doesn't yell at you for the errors that existed prior to the integration of the linter).

[pradyunsg]

Yea, I've seen that. Sadly, one of the objectives is to "Enforce python3.6+ usage". :(

[webknjaz]

Sadly, one of the objectives is to "Enforce python3.6+ usage". :(

Where? I'm pretty sure it's Python 2.7 compatible, I'm trying it out on Cheroot now. If there's some plugins that are incompatible, they are easily disabled.