From: Monty Taylor Date: Sun, 21 Apr 2013 04:44:09 +0000 (-0700) Subject: Use flake8 and hacking. X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=c23f620f2ce2d2dd5079fe6b258b1df7a6662240;p=openstack-build%2Fcinder-build.git Use flake8 and hacking. Fixes bug 1172444 Change-Id: I46b733bd16fa49c58c642348988233064d2b673b --- diff --git a/cinder/backup/services/__init__.py b/cinder/backup/services/__init__.py index 5350b06ea..f745a135a 100644 --- a/cinder/backup/services/__init__.py +++ b/cinder/backup/services/__init__.py @@ -1,14 +1,14 @@ -# Copyright (C) 2012 Hewlett-Packard Development Company, L.P. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. +# Copyright (c) 2013 Hewlett-Packard Development Company, L.P. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py index cad040a02..79e27462d 100644 --- a/cinder/tests/test_storwize_svc.py +++ b/cinder/tests/test_storwize_svc.py @@ -565,7 +565,7 @@ port_speed!N/A return self._errors['CMMVC5701E'] vol_name = kwargs['obj'].strip('\'\'') - if not vol_name in self._volumes_list: + if vol_name not in self._volumes_list: return self._errors['CMMVC5753E'] if not force: @@ -870,10 +870,10 @@ port_speed!N/A return self._errors['CMMVC5707E'] mapping_info['vol'] = kwargs['obj'].strip('\'\'') - if not mapping_info['vol'] in self._volumes_list: + if mapping_info['vol'] not in self._volumes_list: return self._errors['CMMVC5753E'] - if not mapping_info['host'] in self._hosts_list: + if mapping_info['host'] not in self._hosts_list: return self._errors['CMMVC5754E'] if mapping_info['vol'] in self._mappings_list: @@ -898,7 +898,7 @@ port_speed!N/A return self._errors['CMMVC5701E'] vol = kwargs['obj'].strip('\'\'') - if not vol in self._mappings_list: + if vol not in self._mappings_list: return self._errors['CMMVC5753E'] if self._mappings_list[vol]['host'] != host: @@ -939,13 +939,13 @@ port_speed!N/A if 'source' not in kwargs: return self._errors['CMMVC5707E'] source = kwargs['source'].strip('\'\'') - if not source in self._volumes_list: + if source not in self._volumes_list: return self._errors['CMMVC5754E'] if 'target' not in kwargs: return self._errors['CMMVC5707E'] target = kwargs['target'].strip('\'\'') - if not target in self._volumes_list: + if target not in self._volumes_list: return self._errors['CMMVC5754E'] if source == target: diff --git a/cinder/volume/drivers/storwize_svc.py b/cinder/volume/drivers/storwize_svc.py index 4da610e02..1a0bf7a86 100644 --- a/cinder/volume/drivers/storwize_svc.py +++ b/cinder/volume/drivers/storwize_svc.py @@ -1420,7 +1420,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): 'also be set (not equal to -1)')) # Check that the requested protocol is enabled - if not opts['protocol'] in self._enabled_protocols: + if opts['protocol'] not in self._enabled_protocols: raise exception.InvalidInput( reason=_('Illegal value %(prot)s specified for ' 'storwize_svc_connection_protocol: ' diff --git a/tools/flakes.py b/tools/flakes.py deleted file mode 100644 index 191bd6eab..000000000 --- a/tools/flakes.py +++ /dev/null @@ -1,24 +0,0 @@ -""" - wrapper for pyflakes to ignore gettext based warning: - "undefined name '_'" - - Synced in from openstack-common -""" - -__all__ = ['main'] - -import __builtin__ as builtins -import sys - -import pyflakes.api -from pyflakes import checker - - -def main(): - checker.Checker.builtIns = (set(dir(builtins)) | - set(['_']) | - set(checker._MAGIC_GLOBALS)) - sys.exit(pyflakes.api.main()) - -if __name__ == "__main__": - main() diff --git a/tools/hacking.py b/tools/hacking.py deleted file mode 100755 index 680443abd..000000000 --- a/tools/hacking.py +++ /dev/null @@ -1,405 +0,0 @@ -#!/usr/bin/env python -# vim: tabstop=4 shiftwidth=4 softtabstop=4 - -# Copyright (c) 2012, Cloudscaling -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -"""cinder HACKING file compliance testing - -built on top of pep8.py -""" - -import inspect -import logging -import os -import re -import sys -import tokenize -import warnings - -import pep8 - -# Don't need this for testing -logging.disable('LOG') - -#N1xx comments -#N2xx except -#N3xx imports -#N4xx docstrings -#N5xx dictionaries/lists -#N6xx Calling methods -#N7xx localization - -IMPORT_EXCEPTIONS = ['sqlalchemy', 'migrate', 'cinder.db.sqlalchemy.session'] -DOCSTRING_TRIPLE = ['"""', "'''"] -VERBOSE_MISSING_IMPORT = False - - -def is_import_exception(mod): - return mod in IMPORT_EXCEPTIONS or \ - any(mod.startswith(m + '.') for m in IMPORT_EXCEPTIONS) - - -def import_normalize(line): - # convert "from x import y" to "import x.y" - # handle "from x import y as z" to "import x.y as z" - split_line = line.split() - if (line.startswith("from ") and "," not in line and - split_line[2] == "import" and split_line[3] != "*" and - split_line[1] != "__future__" and - (len(split_line) == 4 or - (len(split_line) == 6 and split_line[4] == "as"))): - return "import %s.%s" % (split_line[1], split_line[3]) - else: - return line - - -def cinder_todo_format(physical_line): - """Check for 'TODO()'. - - cinder HACKING guide recommendation for TODO: - Include your name with TODOs as in "#TODO(termie)" - N101 - """ - pos = physical_line.find('TODO') - pos1 = physical_line.find('TODO(') - pos2 = physical_line.find('#') # make sure its a comment - if (pos != pos1 and pos2 >= 0 and pos2 < pos): - return pos, "CINDER N101: Use TODO(NAME)" - - -def cinder_except_format(logical_line): - """Check for 'except:'. - - cinder HACKING guide recommends not using except: - Do not write "except:", use "except Exception:" at the very least - N201 - """ - if logical_line.startswith("except:"): - yield 6, "CINDER N201: no 'except:' at least use 'except Exception:'" - - -def cinder_except_format_assert(logical_line): - """Check for 'assertRaises(Exception'. - - cinder HACKING guide recommends not using assertRaises(Exception...): - Do not use overly broad Exception type - N202 - """ - if logical_line.startswith("self.assertRaises(Exception"): - yield 1, "CINDER N202: assertRaises Exception too broad" - - -def cinder_one_import_per_line(logical_line): - """Check for import format. - - cinder HACKING guide recommends one import per line: - Do not import more than one module per line - - Examples: - BAD: from cinder.rpc.common import RemoteError, LOG - N301 - """ - pos = logical_line.find(',') - parts = logical_line.split() - if (pos > -1 and (parts[0] == "import" or - parts[0] == "from" and parts[2] == "import") and - not is_import_exception(parts[1])): - yield pos, "CINDER N301: one import per line" - -_missingImport = set([]) - - -def cinder_import_module_only(logical_line): - """Check for import module only. - - cinder HACKING guide recommends importing only modules: - Do not import objects, only modules - N302 import only modules - N303 Invalid Import - N304 Relative Import - """ - def importModuleCheck(mod, parent=None, added=False): - """ - If can't find module on first try, recursively check for relative - imports - """ - current_path = os.path.dirname(pep8.current_file) - try: - with warnings.catch_warnings(): - warnings.simplefilter('ignore', DeprecationWarning) - valid = True - if parent: - if is_import_exception(parent): - return - parent_mod = __import__(parent, - globals(), - locals(), - [mod], - -1) - valid = inspect.ismodule(getattr(parent_mod, mod)) - else: - __import__(mod, globals(), locals(), [], -1) - valid = inspect.ismodule(sys.modules[mod]) - if not valid: - if added: - sys.path.pop() - added = False - return (logical_line.find(mod), - ("CINDER N304: No " - "relative imports. '%s' is a relative import" - % logical_line)) - return (logical_line.find(mod), - ("CINDER N302: import only " - "modules. '%s' does not import a module" - % logical_line)) - - except (ImportError, NameError) as exc: - if not added: - added = True - sys.path.append(current_path) - return importModuleCheck(mod, parent, added) - else: - name = logical_line.split()[1] - if name not in _missingImport: - if VERBOSE_MISSING_IMPORT: - print >> sys.stderr, ("ERROR: import '%s' failed: %s" % - (name, exc)) - _missingImport.add(name) - added = False - sys.path.pop() - return - - except AttributeError: - # Invalid import - return logical_line.find(mod), ("CINDER N303: Invalid import, " - "AttributeError raised") - - # convert "from x import y" to " import x.y" - # convert "from x import y as z" to " import x.y" - import_normalize(logical_line) - split_line = logical_line.split() - - if (logical_line.startswith("import ") and - "," not in logical_line and - (len(split_line) == 2 or - (len(split_line) == 4 and split_line[2] == "as"))): - mod = split_line[1] - rval = importModuleCheck(mod) - if rval is not None: - yield rval - - # TODO(jogo) handle "from x import *" - -#TODO(jogo): import template: N305 - - -def cinder_import_alphabetical(physical_line, line_number, lines): - """Check for imports in alphabetical order. - - cinder HACKING guide recommendation for imports: - imports in human alphabetical order - N306 - """ - # handle import x - # use .lower since capitalization shouldn't dictate order - split_line = import_normalize(physical_line.strip()).lower().split() - split_previous = import_normalize( - lines[line_number - 2]).strip().lower().split() - # with or without "as y" - length = [2, 4] - if (len(split_line) in length and len(split_previous) in length and - split_line[0] == "import" and split_previous[0] == "import"): - if split_line[1] < split_previous[1]: - return (0, - "CINDER N306: imports not in alphabetical order (%s, %s)" - % (split_previous[1], split_line[1])) - - -def cinder_docstring_start_space(physical_line): - """Check for docstring not start with space. - - cinder HACKING guide recommendation for docstring: - Docstring should not start with space - N401 - """ - pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start - if (pos != -1 and len(physical_line) > pos + 1): - if (physical_line[pos + 3] == ' '): - return (pos, - "CINDER N401: one line docstring should not start with" - " a space") - - -def cinder_docstring_one_line(physical_line): - """Check one line docstring end. - - cinder HACKING guide recommendation for one line docstring: - A one line docstring looks like this and ends in a period. - N402 - """ - pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start - end = max([physical_line[-4:-1] == i for i in DOCSTRING_TRIPLE]) # end - if (pos != -1 and end and len(physical_line) > pos + 4): - if (physical_line[-5] != '.' and physical_line): - return pos, "CINDER N402: one line docstring needs a period" - - -def cinder_docstring_multiline_end(physical_line): - """Check multi line docstring end. - - cinder HACKING guide recommendation for docstring: - Docstring should end on a new line - N403 - """ - pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start - if (pos != -1 and len(physical_line) == pos): - print physical_line - if (physical_line[pos + 3] == ' '): - return (pos, "CINDER N403: multi line docstring end on new line") - - -FORMAT_RE = re.compile("%(?:" - "%|" # Ignore plain percents - "(\(\w+\))?" # mapping key - "([#0 +-]?" # flag - "(?:\d+|\*)?" # width - "(?:\.\d+)?" # precision - "[hlL]?" # length mod - "\w))") # type - - -class LocalizationError(Exception): - pass - - -def check_l18n(): - """Generator that checks token stream for localization errors. - - Expects tokens to be ``send``ed one by one. - Raises LocalizationError if some error is found. - """ - while True: - try: - token_type, text, _, _, _ = yield - except GeneratorExit: - return - if token_type == tokenize.NAME and text == "_": - while True: - token_type, text, start, _, _ = yield - if token_type != tokenize.NL: - break - if token_type != tokenize.OP or text != "(": - continue # not a localization call - - format_string = '' - while True: - token_type, text, start, _, _ = yield - if token_type == tokenize.STRING: - format_string += eval(text) - elif token_type == tokenize.NL: - pass - else: - break - - if not format_string: - raise LocalizationError( - start, - "CINDER N701: Empty localization string") - if token_type != tokenize.OP: - raise LocalizationError( - start, - "CINDER N701: Invalid localization call") - if text != ")": - if text == "%": - raise LocalizationError( - start, - "CINDER N702: Formatting operation should be outside" - " of localization method call") - elif text == "+": - raise LocalizationError( - start, - "CINDER N702: Use bare string concatenation instead" - " of +") - else: - raise LocalizationError( - start, - "CINDER N702: Argument to _ must be just a string") - - format_specs = FORMAT_RE.findall(format_string) - positional_specs = [(key, spec) for key, spec in format_specs - if not key and spec] - # not spec means %%, key means %(smth)s - if len(positional_specs) > 1: - raise LocalizationError( - start, - "CINDER N703: Multiple positional placeholders") - - -def cinder_localization_strings(logical_line, tokens): - """Check localization in line. - - N701: bad localization call - N702: complex expression instead of string as argument to _() - N703: multiple positional placeholders - """ - - gen = check_l18n() - next(gen) - try: - map(gen.send, tokens) - gen.close() - except LocalizationError as e: - yield e.args - -#TODO(jogo) Dict and list objects - -current_file = "" - - -def readlines(filename): - """Record the current file being tested.""" - pep8.current_file = filename - return open(filename).readlines() - - -def add_cinder(): - """Monkey patch in cinder guidelines. - - Look for functions that start with cinder_ and have arguments - and add them to pep8 module - Assumes you know how to write pep8.py checks - """ - for name, function in globals().items(): - if not inspect.isfunction(function): - continue - args = inspect.getargspec(function)[0] - if args and name.startswith("cinder"): - exec("pep8.%s = %s" % (name, name)) - -if __name__ == "__main__": - #include cinder path - sys.path.append(os.getcwd()) - #CINDER error codes start with an N - pep8.ERRORCODE_REGEX = re.compile(r'[EWN]\d{3}') - add_cinder() - pep8.current_file = current_file - pep8.readlines = readlines - try: - pep8._main() - finally: - if len(_missingImport) > 0: - print >> sys.stderr, ("%i imports missing in this test environment" - % len(_missingImport)) diff --git a/tools/test-requires b/tools/test-requires index 0ff933c69..640ae2720 100644 --- a/tools/test-requires +++ b/tools/test-requires @@ -1,4 +1,9 @@ -# This file is managed by openstack-depends +# Install bounded pep8/pyflakes first, then let flake8 install +pep8==1.4.5 +pyflakes==0.7.2 +flake8==2.0 +hacking>=0.5.3,<0.6 + coverage>=3.6 distribute>=0.6.24 hp3parclient>=1.0.0 @@ -9,8 +14,5 @@ nose nosehtmloutput>=0.0.3 nosexcover openstack.nose_plugin>=0.7 -pep8==1.3.3 psycopg2 -pylint==0.25.2 -setuptools_git>=0.4 sphinx>=1.1.2 diff --git a/tox.ini b/tox.ini index b84c2f421..ac75259b6 100644 --- a/tox.ini +++ b/tox.ini @@ -21,10 +21,8 @@ commands = [testenv:pep8] commands = - python tools/hacking.py --ignore=N4,E125,E126,E711,E712 --repeat --show-source \ - --exclude=.venv,.tox,dist,doc,openstack,*egg . - python tools/hacking.py --ignore=N4,E125,E126,E711,E712 --repeat --show-source \ - --filename=cinder* bin + flake8 + flake8 --filename=cinder* bin [testenv:venv] commands = {posargs} @@ -32,12 +30,13 @@ commands = {posargs} [testenv:cover] setenv = NOSE_WITH_COVERAGE=1 -[testenv:pyflakes] -deps = pyflakes -commands = python tools/flakes.py cinder - [testenv:pylint] setenv = VIRTUAL_ENV={envdir} deps = -r{toxinidir}/tools/pip-requires pylint==0.26.0 commands = bash tools/lintstack.sh + +[flake8] +ignore = E12,E711,E712,H3,H4,F +builtins = _ +exclude = .venv,.tox,dist,doc,openstack,*egg