spack/share/spack/qa/flake8_formatter.py

149 lines
4.8 KiB
Python
Raw Normal View History

Refactor flake8 handling and tool compatibility (#20376) This PR does three related things to try to improve developer tooling quality of life: 1. Adds new options to `.flake8` so it applies the rules of both `.flake8` and `.flake_package` based on paths in the repository. 2. Adds a re-factoring of the `spack flake8` logic into a flake8 plugin so using flake8 directly, or through editor or language server integration, only reports errors that `spack flake8` would. 3. Allows star import of `spack.pkgkit` in packages, since this is now the thing that needs to be imported for completion to work correctly in package files, it's nice to be able to do that. I'm sorely tempted to sed over the whole repository and put `from spack.pkgkit import *` in every package, but at least being allowed to do it on a per-package basis helps. As an example of what the result of this is: ``` ~/Workspace/Projects/spack/spack develop* ⇣ ❯ flake8 --format=pylint ./var/spack/repos/builtin/packages/kripke/package.py ./var/spack/repos/builtin/packages/kripke/package.py:6: [F403] 'from spack.pkgkit import *' used; unable to detect undefined names ./var/spack/repos/builtin/packages/kripke/package.py:25: [E501] line too long (88 > 79 characters) ~/Workspace/Projects/spack/spack refactor-flake8* 1 ❯ flake8 --format=spack ./var/spack/repos/builtin/packages/kripke/package.py ~/Workspace/Projects/spack/spack refactor-flake8* ❯ flake8 ./var/spack/repos/builtin/packages/kripke/package.py ``` * qa/flake8: update .flake8, spack formatter plugin Adds: * Modern flake8 settings for per-path/glob error ignores, allows packages to use the same `.flake8` as the rest of spack * A spack formatter plugin to flake8 that implements the behavior of `spack flake8` for direct invocations. Makes integration with developer tooling nicer, linting with flake8 reports only errors that `spack flake8` would report. Using pyls and pyls-flake8, or any other non-format-dependent flake8 integration, now works with spack's rules. * qa/flake8: allow star import of spack.pkgkit To get working completion of directives and spack components it's necessary to import the contents of spack.pkgkit. At the moment doing this makes flake8 displeased. For now, allow spack.pkgkit and spack both, next step is to ban spack * and require spack.pkgkit *. * first cut at refactoring spack flake8 This version still copies all of the files to be checked as befire, and some other things that probably aren't necessary, but it relies on the spack formatter plugin to implement the ignore logic. * keep flake8 from rejecting itself * remove separate packages flake8 config * fix failures from too many files I ran into this in the PR converting pkgkit to std. The solution in that branch does not work in all cases as it turns out, and all the workarounds I tried to use generated configs to get a single invocation of flake8 with a filename optoion to work failed. It's an astonishingly frustrating config option. Regardless, this removes all temporary file creation from the command and relies on the plugin instead. To work around the huge number of files in spack and still allow the command to control what gets checked, it scans files in batches of 100. This is a completely arbitrary number but was chosen to be safely under common line-length limits. One side-effect of this is that every 100 files the command will produce output, rather than only at the end, which doesn't seem like a terrible thing.
2020-12-23 01:28:46 +08:00
import re
import sys
from collections import defaultdict
import pycodestyle
Refactor flake8 handling and tool compatibility (#20376) This PR does three related things to try to improve developer tooling quality of life: 1. Adds new options to `.flake8` so it applies the rules of both `.flake8` and `.flake_package` based on paths in the repository. 2. Adds a re-factoring of the `spack flake8` logic into a flake8 plugin so using flake8 directly, or through editor or language server integration, only reports errors that `spack flake8` would. 3. Allows star import of `spack.pkgkit` in packages, since this is now the thing that needs to be imported for completion to work correctly in package files, it's nice to be able to do that. I'm sorely tempted to sed over the whole repository and put `from spack.pkgkit import *` in every package, but at least being allowed to do it on a per-package basis helps. As an example of what the result of this is: ``` ~/Workspace/Projects/spack/spack develop* ⇣ ❯ flake8 --format=pylint ./var/spack/repos/builtin/packages/kripke/package.py ./var/spack/repos/builtin/packages/kripke/package.py:6: [F403] 'from spack.pkgkit import *' used; unable to detect undefined names ./var/spack/repos/builtin/packages/kripke/package.py:25: [E501] line too long (88 > 79 characters) ~/Workspace/Projects/spack/spack refactor-flake8* 1 ❯ flake8 --format=spack ./var/spack/repos/builtin/packages/kripke/package.py ~/Workspace/Projects/spack/spack refactor-flake8* ❯ flake8 ./var/spack/repos/builtin/packages/kripke/package.py ``` * qa/flake8: update .flake8, spack formatter plugin Adds: * Modern flake8 settings for per-path/glob error ignores, allows packages to use the same `.flake8` as the rest of spack * A spack formatter plugin to flake8 that implements the behavior of `spack flake8` for direct invocations. Makes integration with developer tooling nicer, linting with flake8 reports only errors that `spack flake8` would report. Using pyls and pyls-flake8, or any other non-format-dependent flake8 integration, now works with spack's rules. * qa/flake8: allow star import of spack.pkgkit To get working completion of directives and spack components it's necessary to import the contents of spack.pkgkit. At the moment doing this makes flake8 displeased. For now, allow spack.pkgkit and spack both, next step is to ban spack * and require spack.pkgkit *. * first cut at refactoring spack flake8 This version still copies all of the files to be checked as befire, and some other things that probably aren't necessary, but it relies on the spack formatter plugin to implement the ignore logic. * keep flake8 from rejecting itself * remove separate packages flake8 config * fix failures from too many files I ran into this in the PR converting pkgkit to std. The solution in that branch does not work in all cases as it turns out, and all the workarounds I tried to use generated configs to get a single invocation of flake8 with a filename optoion to work failed. It's an astonishingly frustrating config option. Regardless, this removes all temporary file creation from the command and relies on the plugin instead. To work around the huge number of files in spack and still allow the command to control what gets checked, it scans files in batches of 100. This is a completely arbitrary number but was chosen to be safely under common line-length limits. One side-effect of this is that every 100 files the command will produce output, rather than only at the end, which doesn't seem like a terrible thing.
2020-12-23 01:28:46 +08:00
from flake8.formatting.default import Pylint
from flake8.style_guide import Violation
#: This is a dict that maps:
#: filename pattern ->
#: flake8 exemption code ->
#: list of patterns, for which matching lines should have codes applied.
#:
#: For each file, if the filename pattern matches, we'll add per-line
#: exemptions if any patterns in the sub-dict match.
pattern_exemptions = {
# exemptions applied only to package.py files.
r"package.py$": {
# Allow 'from spack import *' in packages, but no other wildcards
"F403": [
r"^from spack import \*$",
r"^from spack.pkgkit import \*$",
],
# Exempt lines with urls and descriptions from overlong line errors.
"E501": [
r"^\s*homepage\s*=",
r"^\s*url\s*=",
r"^\s*git\s*=",
r"^\s*svn\s*=",
r"^\s*hg\s*=",
r"^\s*pypi\s*=",
Refactor flake8 handling and tool compatibility (#20376) This PR does three related things to try to improve developer tooling quality of life: 1. Adds new options to `.flake8` so it applies the rules of both `.flake8` and `.flake_package` based on paths in the repository. 2. Adds a re-factoring of the `spack flake8` logic into a flake8 plugin so using flake8 directly, or through editor or language server integration, only reports errors that `spack flake8` would. 3. Allows star import of `spack.pkgkit` in packages, since this is now the thing that needs to be imported for completion to work correctly in package files, it's nice to be able to do that. I'm sorely tempted to sed over the whole repository and put `from spack.pkgkit import *` in every package, but at least being allowed to do it on a per-package basis helps. As an example of what the result of this is: ``` ~/Workspace/Projects/spack/spack develop* ⇣ ❯ flake8 --format=pylint ./var/spack/repos/builtin/packages/kripke/package.py ./var/spack/repos/builtin/packages/kripke/package.py:6: [F403] 'from spack.pkgkit import *' used; unable to detect undefined names ./var/spack/repos/builtin/packages/kripke/package.py:25: [E501] line too long (88 > 79 characters) ~/Workspace/Projects/spack/spack refactor-flake8* 1 ❯ flake8 --format=spack ./var/spack/repos/builtin/packages/kripke/package.py ~/Workspace/Projects/spack/spack refactor-flake8* ❯ flake8 ./var/spack/repos/builtin/packages/kripke/package.py ``` * qa/flake8: update .flake8, spack formatter plugin Adds: * Modern flake8 settings for per-path/glob error ignores, allows packages to use the same `.flake8` as the rest of spack * A spack formatter plugin to flake8 that implements the behavior of `spack flake8` for direct invocations. Makes integration with developer tooling nicer, linting with flake8 reports only errors that `spack flake8` would report. Using pyls and pyls-flake8, or any other non-format-dependent flake8 integration, now works with spack's rules. * qa/flake8: allow star import of spack.pkgkit To get working completion of directives and spack components it's necessary to import the contents of spack.pkgkit. At the moment doing this makes flake8 displeased. For now, allow spack.pkgkit and spack both, next step is to ban spack * and require spack.pkgkit *. * first cut at refactoring spack flake8 This version still copies all of the files to be checked as befire, and some other things that probably aren't necessary, but it relies on the spack formatter plugin to implement the ignore logic. * keep flake8 from rejecting itself * remove separate packages flake8 config * fix failures from too many files I ran into this in the PR converting pkgkit to std. The solution in that branch does not work in all cases as it turns out, and all the workarounds I tried to use generated configs to get a single invocation of flake8 with a filename optoion to work failed. It's an astonishingly frustrating config option. Regardless, this removes all temporary file creation from the command and relies on the plugin instead. To work around the huge number of files in spack and still allow the command to control what gets checked, it scans files in batches of 100. This is a completely arbitrary number but was chosen to be safely under common line-length limits. One side-effect of this is that every 100 files the command will produce output, rather than only at the end, which doesn't seem like a terrible thing.
2020-12-23 01:28:46 +08:00
r"^\s*list_url\s*=",
r"^\s*version\(",
r"^\s*variant\(",
r"^\s*provides\(",
r"^\s*extends\(",
r"^\s*depends_on\(",
r"^\s*conflicts\(",
r"^\s*resource\(",
r"^\s*patch\(",
],
# Exempt '@when' decorated functions from redefinition errors.
"F811": [
r"^\s*@when\(.*\)",
],
},
# exemptions applied to all files.
r".py$": {
"E501": [
r"(https?|ftp|file)\:", # URLs
r'([\'"])[0-9a-fA-F]{32,}\1', # long hex checksums
]
},
}
# compile all regular expressions.
pattern_exemptions = dict(
(
re.compile(file_pattern),
dict(
(code, [re.compile(p) for p in patterns])
for code, patterns in error_dict.items()
),
)
for file_pattern, error_dict in pattern_exemptions.items()
)
class SpackFormatter(Pylint):
def __init__(self, options):
self.spack_errors = {}
self.error_seen = False
super().__init__(options)
def after_init(self): # type: () -> None
"""Overriding to keep format string from being unset in Default"""
pass
def beginning(self, filename):
self.filename = filename
self.file_lines = None
self.spack_errors = defaultdict(list)
for file_pattern, errors in pattern_exemptions.items():
if file_pattern.search(filename):
for code, pat_arr in errors.items():
self.spack_errors[code].extend(pat_arr)
def handle(self, error): # type: (Violation) -> None
"""Handle an error reported by Flake8.
This defaults to calling :meth:`format`, :meth:`show_source`, and
then :meth:`write`. This version implements the pattern-based ignore
behavior from `spack flake8` as a native flake8 plugin.
:param error:
This will be an instance of
:class:`~flake8.style_guide.Violation`.
:type error:
flake8.style_guide.Violation
"""
# print(error.code)
# print(error.physical_line)
# get list of patterns for this error code
pats = self.spack_errors.get(error.code, None)
# if any pattern matches, skip line
if pats is not None and any(
(pat.search(error.physical_line) for pat in pats)
):
return
# Special F811 handling
# Prior to Python 3.8, `noqa: F811` needed to be placed on the `@when`
# line
# Starting with Python 3.8, it must be placed on the `def` line
# https://gitlab.com/pycqa/flake8/issues/583
# we can only determine if F811 should be ignored given the previous
# line, so get the previous line and check it
if (
self.spack_errors.get("F811", False)
and error.code == "F811"
and error.line_number > 1
):
if self.file_lines is None:
if self.filename in {"stdin", "-", "(none)", None}:
self.file_lines = pycodestyle.stdin_get_value().splitlines(
True
)
else:
self.file_lines = pycodestyle.readlines(self.filename)
for pat in self.spack_errors["F811"]:
if pat.search(self.file_lines[error.line_number - 2]):
return
self.error_seen = True
line = self.format(error)
source = self.show_source(error)
self.write(line, source)
def stop(self):
"""Override stop to check whether any errors we consider to be errors
were reported.
This is a hack, but it makes flake8 behave the desired way.
"""
if not self.error_seen:
sys.exit(0)