summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2017-10-23 14:57:46 +0200
committerTodd Gamblin <tgamblin@llnl.gov>2017-10-24 10:05:36 +0200
commit7757ebc0bc5b46cb3cfa41e5bebd4754978fd3b0 (patch)
tree2fdefbf27a3b392aedb6323d013bb842c8fe304c
parentbeab0cb92e61ba4e33124ff17f244c121973117d (diff)
downloadspack-7757ebc0bc5b46cb3cfa41e5bebd4754978fd3b0.tar.gz
spack-7757ebc0bc5b46cb3cfa41e5bebd4754978fd3b0.tar.bz2
spack-7757ebc0bc5b46cb3cfa41e5bebd4754978fd3b0.tar.xz
spack-7757ebc0bc5b46cb3cfa41e5bebd4754978fd3b0.zip
flake8: no wildcards in core; only `import *` from spack in packages
There are now separate flake8 configs for core vs. packages: - core has a smaller set of flake8 exceptions - packages allow `from spack import *` and module globals - Allows core to take advantage of static checking for undefined names - Allows packages to keep using Spack tricks like `from spack import *` and dependencies setting globals for dependents.
-rw-r--r--.flake813
-rw-r--r--.flake8_packages22
-rw-r--r--lib/spack/spack/cmd/flake8.py71
-rw-r--r--lib/spack/spack/test/cmd/flake8.py1
4 files changed, 78 insertions, 29 deletions
diff --git a/.flake8 b/.flake8
index e697d5ea04..49199b35c8 100644
--- a/.flake8
+++ b/.flake8
@@ -1,8 +1,8 @@
# -*- conf -*-
-# flake8 settings for Spack.
+# flake8 settings for Spack core files.
#
-# Below we describe which flake8 checks Spack ignores and what the
-# rationale is.
+# These exceptions ar for Spack core files. We're slightly more lenient
+# with packages. See .flake8_packages for that.
#
# Let people line things up nicely:
# - E129: visually indented line with same indent as next logical line
@@ -13,14 +13,9 @@
# Let people use terse Python features:
# - E731: lambda expressions
#
-# Spack allows wildcard imports:
-# - F403: disable wildcard import
-#
# These are required to get the package.py files to test clean:
-# - F405: `name` may be undefined, or undefined from star imports: `module`
-# - F821: undefined name `name` (needed for cmake, configure, etc.)
# - F999: syntax error in doctest
#
[flake8]
-ignore = E129,E221,E241,E272,E731,F403,F405,F821,F999
+ignore = E129,E221,E241,E272,E731,F999
max-line-length = 79
diff --git a/.flake8_packages b/.flake8_packages
new file mode 100644
index 0000000000..9fcc3b86d4
--- /dev/null
+++ b/.flake8_packages
@@ -0,0 +1,22 @@
+# -*- conf -*-
+# flake8 settings for Spack package files.
+#
+# This should include all the same exceptions that we use for core files.
+#
+# In Spack packages, we also allow the single `from spack import *`
+# wildcard import and dependencies can set globals for their
+# dependents. So we add exceptions for checks related to undefined names.
+#
+# Note that we also add *per-line* exemptions for certain patters in the
+# `spack flake8` command. This is where F403 for `from spack import *`
+# is added (beause we *only* allow that wildcard).
+#
+# See .flake8 for regular exceptions.
+#
+# Redefinition exceptions:
+# - F405: `name` may be undefined, or undefined from star imports: `module`
+# - F821: undefined name `name` (needed for cmake, configure, etc.)
+#
+[flake8]
+ignore = E129,E221,E241,E272,E731,F999,F405,F821
+max-line-length = 79
diff --git a/lib/spack/spack/cmd/flake8.py b/lib/spack/spack/cmd/flake8.py
index 8f9e311734..01e642235d 100644
--- a/lib/spack/spack/cmd/flake8.py
+++ b/lib/spack/spack/cmd/flake8.py
@@ -42,18 +42,34 @@ section = "developer"
level = "long"
-"""List of directories to exclude from checks."""
+def is_package(f):
+ """Whether flake8 should consider a file as a core file or a package.
+
+ We run flake8 with different exceptions for the core and for
+ packages, since we allow `from spack import *` and poking globals
+ into packages.
+ """
+ return f.startswith('var/spack/repos/') or 'docs/tutorial/examples' in f
+
+
+#: List of directories to exclude from checks.
exclude_directories = [spack.external_path]
-"""
-This is a dict that maps:
- filename pattern ->
- a flake8 exemption code ->
- list of patterns, for which matching lines should have codes applied.
-"""
-exemptions = {
+
+#: 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 \*$'
+ ],
# Exempt lines with urls and descriptions from overlong line errors.
'E501': [
r'^\s*homepage\s*=',
@@ -87,10 +103,11 @@ exemptions = {
}
# compile all regular expressions.
-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 exemptions.items())
+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())
def changed_files(args):
@@ -135,7 +152,7 @@ def changed_files(args):
return sorted(changed)
-def add_exemptions(line, codes):
+def add_pattern_exemptions(line, codes):
"""Add a flake8 exemption to a line."""
if line.startswith('#'):
return line
@@ -163,7 +180,7 @@ def add_exemptions(line, codes):
def filter_file(source, dest, output=False):
- """Filter a single file through all the patterns in exemptions."""
+ """Filter a single file through all the patterns in pattern_exemptions."""
with open(source) as infile:
parent = os.path.dirname(dest)
mkdirp(parent)
@@ -171,7 +188,9 @@ def filter_file(source, dest, output=False):
with open(dest, 'w') as outfile:
for line in infile:
line_errors = []
- for file_pattern, errors in exemptions.items():
+
+ # pattern exemptions
+ for file_pattern, errors in pattern_exemptions.items():
if not file_pattern.search(source):
continue
@@ -182,7 +201,7 @@ def filter_file(source, dest, output=False):
break
if line_errors:
- line = add_exemptions(line, line_errors)
+ line = add_pattern_exemptions(line, line_errors)
outfile.write(line)
if output:
@@ -226,7 +245,6 @@ def flake8(parser, args):
with working_dir(spack.prefix):
if not file_list:
file_list = changed_files(args)
- shutil.copy('.flake8', os.path.join(temp, '.flake8'))
print('=======================================================')
print('flake8: running flake8 code checks on spack.')
@@ -242,10 +260,23 @@ def flake8(parser, args):
dest_path = os.path.join(temp, filename)
filter_file(src_path, dest_path, args.output)
- # run flake8 on the temporary tree.
+ # run flake8 on the temporary tree, once for core, once for pkgs
+ package_file_list = [f for f in file_list if is_package(f)]
+ file_list = [f for f in file_list if not is_package(f)]
+
with working_dir(temp):
- output = flake8('--format', 'pylint', *file_list,
- fail_on_error=False, output=str)
+ output = ''
+ if file_list:
+ output += flake8(
+ '--format', 'pylint',
+ '--config=%s' % os.path.join(spack.prefix, '.flake8'),
+ *file_list, fail_on_error=False, output=str)
+ if package_file_list:
+ output += flake8(
+ '--format', 'pylint',
+ '--config=%s' % os.path.join(spack.prefix,
+ '.flake8_packages'),
+ *package_file_list, fail_on_error=False, output=str)
if args.root_relative:
# print results relative to repo root.
diff --git a/lib/spack/spack/test/cmd/flake8.py b/lib/spack/spack/test/cmd/flake8.py
index 8556a0a036..b6433e95c9 100644
--- a/lib/spack/spack/test/cmd/flake8.py
+++ b/lib/spack/spack/test/cmd/flake8.py
@@ -34,6 +34,7 @@ from spack.cmd.flake8 import *
from spack.repository import Repo
+
@pytest.fixture(scope='module')
def parser():
"""Returns the parser for the ``flake8`` command"""