openstack/cinder
Code Issues Proposed changes

Simplify running pylint

Browse Source 
This commit does several things:

- Setup and run pylint directly rather than running
  through a script. This allows the user to see what is happening
  while the user is running through pylint.
- Allow the user to either run pylint on a particular changeset,
  or the entire cinder tree.
- Allow the user to run on a particular changeset. Using like
  HEAD~1, etc.
- Since the pylint gate check I disabled the tests that were
  reported by pylint. The thought here would be go through
  the failures and correct them.
- Update pylint to 2.1.1.
- Removed old pylintrc.

Change-Id: I708c93843d991e8c0ac114b0b2d6ede373b96d0a
Signed-off-by: Chuck Short <chucks@redhat.com>
This commit is contained in:
Chuck Short 2018-08-28 10:01:51 -04:00
parent ccb3ff90c5
commit adcfb4f97e
6 changed files with 290 additions and 378 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

231
.pylintrc Normal file
View File

@ -0,0 +1,231 @@
[MASTER]
# A comma-separated list of package or module names from where C extensions may
# be loaded. Extensions are loading into the active Python interpreter and may
# run arbitrary code.
extension-pkg-whitelist=
# Add files or directories to the blacklist. They should be base names, not
# paths.
ignore=CVS,tests,test
# Add files or directories matching the regex patterns to the blacklist. The
# regex matches against base names, not paths.
ignore-patterns=
# Python code to execute, usually for sys.path manipulation such as
# pygtk.require().
#init-hook=
# Use multiple processes to speed up Pylint. Specifying 0 will auto-detect the
# number of processors available to use.
jobs=1
# Control the amount of potential inferred values when inferring a single
# object. This can help the performance when dealing with large functions or
# complex, nested conditions.
limit-inference-results=100
# List of plugins (as comma separated values of python modules names) to load,
# usually to register additional checkers.
load-plugins=
# Pickle collected data for later comparisons.
persistent=yes
# Specify a configuration file.
#rcfile=
# When enabled, pylint would attempt to guess common misconfiguration and emit
# user-friendly hints instead of false-positive error messages.
suggestion-mode=yes
# Allow loading of arbitrary C extensions. Extensions are imported into the
# active Python interpreter and may run arbitrary code.
unsafe-load-any-extension=no
[MESSAGES CONTROL]
# Only show warnings with the listed confidence levels. Leave empty to show
# all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED.
confidence=
# Disable the message, report, category or checker with the given id(s). You
# can either give multiple identifiers separated by comma (,) or put this
# option multiple times (only on the command line, not in the configuration
# file where it should appear only once). You can also use "--disable=all" to
# disable everything first and then reenable specific checks. For example, if
# you want to run only the similarities checker, you can use "--disable=all
# --enable=similarities". If you want to run only the classes checker, but have
# no Warning level messages displayed, use "--disable=all --enable=classes
# --disable=W".
disable=
# "F" Fatal errors that prevent further processing
import-error,
# "I" Informational noise
locally-disabled,
c-extension-no-member,
# "E" Error for important programming issues (likely bugs)
access-member-before-definition,
bad-super-call,
no-member,
no-method-argument,
no-name-in-module,
no-self-argument,
no-value-for-parameter,
unsubscriptable-object,
method-hidden,
not-callable,
keyword-arg-before-vararg,
too-many-function-args,
unsupported-assignment-operation,
misplaced-bare-raise,
not-an-iterable,
unsupported-membership-test,
unsupported-assignment-operation,
raising-bad-type,
bad-option-value,
unexpected-keyword-arg,
assignment-from-none,
assignment-from-no-return,
# "W" Warnings for stylistic problems or minor programming issues
exec-used,
pointless-statement,
unnecessary-lambda,
abstract-method,
arguments-differ,
attribute-defined-outside-init,
bad-builtin,
bad-indentation,
broad-except,
deprecated-lambda,
expression-not-assigned,
fixme,
global-statement,
global-variable-not-assigned,
no-init,
non-parent-init-called,
protected-access,
redefined-builtin,
redefined-outer-name,
reimported,
signature-differs,
star-args,
super-init-not-called,
unnecessary-pass,
unpacking-non-sequence,
unused-argument,
unused-import,
undefined-loop-variable,
bad-staticmethod-argument,
deprecated-method,
useless-else-on-loop,
lost-exception,
pointless-string-statement,
useless-super-delegation,
deprecated-method,
dangerous-default-value,
wildcard-import,
bad-staticmethod-argument,
eval-used,
blacklisted-name,
pointless-statement,
try-except-raise,
# "C" Coding convention violations
bad-continuation,
invalid-name,
missing-docstring,
old-style-class,
superfluous-parens,
wrong-import-position,
wrong-import-order,
ungrouped-imports,
unused-variable,
len-as-condition,
cell-var-from-loop,
unneeded-not,
singleton-comparison,
misplaced-comparison-constant,
unidiomatic-typecheck,
consider-using-enumerate,
bad-whitespace,
consider-iterating-dictionary,
line-too-long,
useless-super-delegation,
pointless-string-statement,
unsupported-membership-test,
bad-classmethod-argument,
bad-mcs-classmethod-argument,
# "R" Refactor recommendations
abstract-class-little-used,
abstract-class-not-used,
duplicate-code,
interface-not-implemented,
no-self-use,
too-few-public-methods,
too-many-ancestors,
too-many-arguments,
too-many-branches,
too-many-instance-attributes,
too-many-lines,
too-many-locals,
too-many-public-methods,
too-many-return-statements,
too-many-statements,
too-many-nested-blocks,
no-else-return,
inconsistent-return-statements,
simplifiable-if-statement,
too-many-boolean-expressions,
cyclic-import,
redefined-argument-from-local,
consider-using-ternary,
literal-comparison,
consider-merging-isinstance,
consider-merging-isinstance,
too-many-boolean-expressions,
useless-object-inheritance,
trailing-comma-tuple,
useless-object-inheritance,
consider-using-set-comprehension,
consider-using-in,
useless-return,
chained-comparison
[REPORTS]
# Tells whether to display a full report or only the messages.
reports=no
[BASIC]
# Variable names can be 1 to 31 characters long, with lowercase and underscores
variable-rgx=[a-z_][a-z0-9_]{0,30}$
# Argument names can be 2 to 31 characters long, with lowercase and underscores
argument-rgx=[a-z_][a-z0-9_]{1,30}$
# Method names should be at least 3 characters long
# and be lowercased with underscores
method-rgx=([a-z_][a-z0-9_]{2,}|setUp|tearDown)$
# Module names matching neutron-* are ok (files in bin/)
module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+)|(neutron-[a-z0-9_-]+))$
# Don't require docstrings on tests.
no-docstring-rgx=((__.*__)|([tT]est.*)|setUp|tearDown)$
[FORMAT]
# Maximum number of characters on a single line.
max-line-length=79
[VARIABLES]
# List of additional names supposed to be defined in builtins. Remember that
# you should avoid to define new builtins when possible.
additional-builtins=_
[TYPECHECK]
# List of module names for which member attributes should not be checked
ignored-modules=six.moves,_MovedItems

41
pylintrc
View File

@ -1,41 +0,0 @@
# The format of this file isn't really documented; just use --generate-rcfile
[Messages Control]
# NOTE(justinsb): We might want to have a 2nd strict pylintrc in future
# C0111: Don't require docstrings on every method
# W0511: TODOs in code comments are fine.
# W0142: *args and **kwargs are fine.
# W0622: Redefining id is fine.
disable=C0111,W0511,W0142,W0622
[Basic]
# Variable names can be 1 to 31 characters long, with lowercase and underscores
variable-rgx=[a-z_][a-z0-9_]{0,30}$
# Argument names can be 2 to 31 characters long, with lowercase and underscores
argument-rgx=[a-z_][a-z0-9_]{1,30}$
# Method names should be at least 3 characters long
# and be lowercased with underscores
method-rgx=([a-z_][a-z0-9_]{2,50}|setUp|tearDown)$
# Module names matching cinder-* are ok (files in bin/)
module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+)|(cinder-[a-z0-9_-]+))$
# Don't require docstrings on tests.
no-docstring-rgx=((__.*__)|([tT]est.*)|setUp|tearDown)$
[Design]
max-public-methods=100
min-public-methods=0
max-args=6
[Variables]
dummy-variables-rgx=_
[Typecheck]
# Disable warnings on the HTTPSConnection classes because pylint doesn't
# support importing from six.moves yet, see:
# https://bitbucket.org/logilab/pylint/issue/550/
ignored-classes=HTTPSConnection

56
tools/coding-checks.sh Executable file
View File

@ -0,0 +1,56 @@
#!/bin/sh
set -eu
usage() {
echo "Usage: $0 [OPTION]..."
echo "Run Cinder's coding check(s)"
echo ""
echo " -Y, --pylint [<basecommit>] Run pylint check on the entire cinder module or just files changed in basecommit (e.g. HEAD~1)"
echo " -h, --help Print this usage message"
echo
exit 0
}
process_options() {
i=1
while [ $i -le $# ]; do
eval opt=\$$i
case $opt in
-h|--help) usage;;
-Y|--pylint) pylint=1;;
*) scriptargs="$scriptargs $opt"
esac
i=$((i+1))
done
}
run_pylint() {
local target="${scriptargs:-HEAD~1}"
if [[ "$target" = *"all"* ]]; then
files="cinder"
else
files=$(git diff --name-only --diff-filter=ACMRU $target "*.py")
fi
if [ -n "${files}" ]; then
echo "Running pylint against:"
printf "\t%s\n" "${files[@]}"
pylint --rcfile=.pylintrc --output-format=colorized ${files} -E \
-j `python -c 'import multiprocessing as mp; print(mp.cpu_count())'`
else
echo "No python changes in this commit, pylint check not required."
exit 0
fi
}
scriptargs=
pylint=1
process_options $@
if [ $pylint -eq 1 ]; then
run_pylint
exit 0
fi

276
tools/lintstack.py
View File

@ -1,276 +0,0 @@
#!/usr/bin/env python
# Copyright (c) 2013, AT&T Labs, Yun Mao <yunmao@gmail.com>
# 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.
"""pylint error checking."""
import json
import multiprocessing
import re
import sys
from pylint import lint
from pylint.reporters import text
from six.moves import cStringIO as StringIO
ignore_codes = [
# Note(maoy): E1103 is error code related to partial type inference
"E1103"
]
ignore_messages = [
# Note(maoy): this error message is the pattern of E0202. It should be
# ignored for cinder.tests modules
"An attribute affected in cinder.tests",
# Note(fengqian): this error message is the pattern of [E0611].
"No name 'urllib' in module '_MovedItems'",
# Note(e0ne): this error message is for SQLAlchemy update() calls
# It should be ignored because use six module to keep py3.X compatibility.
# in DB schema migrations.
"No value passed for parameter 'dml'",
# Note(xyang): these error messages are for the code [E1101].
# They should be ignored because 'sha256' and 'sha224' are functions in
# 'hashlib'.
"Module 'hashlib' has no 'sha256' member",
"Module 'hashlib' has no 'sha224' member",
# Note(aarefiev): this error message is for SQLAlchemy rename calls in
# DB migration(033_add_encryption_unique_key).
"Instance of 'Table' has no 'rename' member",
# NOTE(geguileo): these error messages are for code [E1101], and they can
# be ignored because a SQLAlchemy ORM class will have __table__ member
# during runtime.
"Class 'ConsistencyGroup' has no '__table__' member",
"Class 'CGSnapshot' has no '__table__' member",
"Class 'Group' has no '__table__' member",
"Class 'GroupSnapshot' has no '__table__' member",
# NOTE(xyang): this error message is for code [E1120] when checking if
# there are already 'groups' entries in 'quota_classes' `in DB migration
# (078_add_groups_and_group_volume_type_mapping_table).
"No value passed for parameter 'functions' in function call",
# NOTE(dulek): This one is related to objects.
"No value passed for parameter 'id' in function call",
# NOTE(geguileo): v3 common manage class for volumes and snapshots
"Instance of 'ManageResource' has no 'volume_api' member",
"Instance of 'ManageResource' has no '_list_manageable_view' member",
]
# Note(maoy): We ignore cinder.tests for now due to high false
# positive rate.
ignore_modules = ["cinder/tests/"]
# Note(thangp): E0213, E1101, and E1102 should be ignored for only
# cinder.object modules. E0213 and E1102 are error codes related to
# the first argument of a method, but should be ignored because the method
# is a remotable class method. E1101 is error code related to accessing a
# non-existent member of an object, but should be ignored because the object
# member is created dynamically.
objects_ignore_codes = ["E0213", "E1101", "E1102"]
# NOTE(dulek): We're ignoring messages related to non-existent objects in
# cinder.objects namespace. This is because this namespace is populated when
# registering the objects, and pylint is unable to detect that.
objects_ignore_regexp = "Module 'cinder.objects' has no '.*' member"
objects_ignore_modules = ["cinder/objects/"]
KNOWN_PYLINT_EXCEPTIONS_FILE = "tools/pylint_exceptions"
class LintOutput(object):
_cached_filename = None
_cached_content = None
def __init__(self, filename, lineno, line_content, code, message,
lintoutput):
self.filename = filename
self.lineno = lineno
self.line_content = line_content
self.code = code
self.message = message
self.lintoutput = lintoutput
@classmethod
def from_line(cls, line):
m = re.search(r"(\S+):(\d+): \[(\S+)(, \S+)?] (.*)", line)
if m is None:
return None
matched = m.groups()
filename, lineno, code, message = (matched[0], int(matched[1]),
matched[2], matched[-1])
if cls._cached_filename != filename:
with open(filename) as f:
cls._cached_content = list(f.readlines())
cls._cached_filename = filename
line_content = cls._cached_content[lineno - 1].rstrip()
return cls(filename, lineno, line_content, code, message,
line.rstrip())
@classmethod
def from_msg_to_dict(cls, msg):
"""Convert pylint output to a dict
From the output of pylint msg, to a dict, where each key
is a unique error identifier, value is a list of LintOutput
"""
result = {}
for line in msg.splitlines():
obj = cls.from_line(line)
if obj is None or obj.is_ignored():
continue
key = obj.key()
if key not in result:
result[key] = []
result[key].append(obj)
return result
def is_ignored(self):
if self.code in ignore_codes:
return True
if any(self.filename.startswith(name) for name in ignore_modules):
return True
if any(msg in self.message for msg in ignore_messages):
return True
if re.match(objects_ignore_regexp, self.message):
return True
if (self.code in objects_ignore_codes and
any(self.filename.startswith(name)
for name in objects_ignore_modules)):
return True
if (self.code in objects_ignore_codes and
any(self.filename.startswith(name)
for name in objects_ignore_modules)):
return True
return False
def key(self):
if self.code in ["E1101", "E1103"]:
# These two types of errors are like Foo class has no member bar.
# We discard the source code so that the error will be ignored
# next time another Foo.bar is encountered.
return self.message, ""
return self.message, self.line_content.strip()
def json(self):
return json.dumps(self.__dict__)
def review_str(self):
return ("File %(filename)s\nLine %(lineno)d:%(line_content)s\n"
"%(code)s: %(message)s" %
{'filename': self.filename,
'lineno': self.lineno,
'line_content': self.line_content,
'code': self.code,
'message': self.message})
class ErrorKeys(object):
@classmethod
def print_json(cls, errors, output=sys.stdout):
print("# automatically generated by tools/lintstack.py", file=output)
for i in sorted(errors.keys()):
print(json.dumps(i), file=output)
@classmethod
def from_file(cls, filename):
keys = set()
for line in open(filename):
if line and line[0] != "#":
d = json.loads(line)
keys.add(tuple(d))
return keys
def run_pylint():
buff = StringIO()
reporter = text.TextReporter(output=buff)
args = [
"--msg-template='{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}'",
"-j", "%s" % multiprocessing.cpu_count(),
"-E", "cinder"]
lint.Run(args, reporter=reporter, exit=False)
val = buff.getvalue()
buff.close()
return val
def generate_error_keys(msg=None):
print("Generating", KNOWN_PYLINT_EXCEPTIONS_FILE)
if msg is None:
msg = run_pylint()
errors = LintOutput.from_msg_to_dict(msg)
with open(KNOWN_PYLINT_EXCEPTIONS_FILE, "w") as f:
ErrorKeys.print_json(errors, output=f)
def validate(newmsg=None):
print("Loading", KNOWN_PYLINT_EXCEPTIONS_FILE)
known = ErrorKeys.from_file(KNOWN_PYLINT_EXCEPTIONS_FILE)
if newmsg is None:
print("Running pylint. Be patient...")
newmsg = run_pylint()
errors = LintOutput.from_msg_to_dict(newmsg)
print("Unique errors reported by pylint: was %d, now %d."
% (len(known), len(errors)))
passed = True
for err_key, err_list in errors.items():
for err in err_list:
if err_key not in known:
print(err.lintoutput)
print()
passed = False
if passed:
print("Congrats! pylint check passed.")
redundant = known - set(errors.keys())
if redundant:
print("Extra credit: some known pylint exceptions disappeared.")
for i in sorted(redundant):
print(json.dumps(i))
print("Consider regenerating the exception file if you will.")
else:
print("Please fix the errors above. If you believe they are false "
"positives, run 'tools/lintstack.py generate' to overwrite.")
sys.exit(1)
def usage():
print("""Usage: tools/lintstack.py [generate|validate]
To generate pylint_exceptions file: tools/lintstack.py generate
To validate the current commit: tools/lintstack.py
""")
def main():
option = "validate"
if len(sys.argv) > 1:
option = sys.argv[1]
if option == "generate":
generate_error_keys()
elif option == "validate":
validate()
else:
usage()
if __name__ == "__main__":
main()

59
tools/lintstack.sh
View File

@ -1,59 +0,0 @@
#!/usr/bin/env bash
# Copyright (c) 2012-2013, AT&T Labs, Yun Mao <yunmao@gmail.com>
# 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.
# Use lintstack.py to compare pylint errors.
# We run pylint twice, once on HEAD, once on the code before the latest
# commit for review.
set -e
TOOLS_DIR=$(cd $(dirname "$0") && pwd)
# Get the current branch name.
GITHEAD=`git rev-parse --abbrev-ref HEAD`
if [[ "$GITHEAD" == "HEAD" ]]; then
# In detached head mode, get revision number instead
GITHEAD=`git rev-parse HEAD`
echo "Currently we are at commit $GITHEAD"
else
echo "Currently we are at branch $GITHEAD"
fi
cp -f $TOOLS_DIR/lintstack.py $TOOLS_DIR/lintstack.head.py
if git rev-parse HEAD^2 2>/dev/null; then
# The HEAD is a Merge commit. Here, the patch to review is
# HEAD^2, the master branch is at HEAD^1, and the patch was
# written based on HEAD^2~1.
PREV_COMMIT=`git rev-parse HEAD^2~1`
git checkout HEAD~1
# The git merge is necessary for reviews with a series of patches.
# If not, this is a no-op so won't hurt either.
git merge $PREV_COMMIT
else
# The HEAD is not a merge commit. This won't happen on gerrit.
# Most likely you are running against your own patch locally.
# We assume the patch to examine is HEAD, and we compare it against
# HEAD~1
git checkout HEAD~1
fi
# First generate tools/pylint_exceptions from HEAD~1
$TOOLS_DIR/lintstack.head.py generate
# Then use that as a reference to compare against HEAD
git checkout $GITHEAD
$TOOLS_DIR/lintstack.head.py
echo "Check passed. FYI: the pylint exceptions are:"
cat $TOOLS_DIR/pylint_exceptions

5
tox.ini
View File

@ -85,8 +85,9 @@ commands =
[testenv:pylint]
basepython = python3
deps = -r{toxinidir}/requirements.txt
pylint==1.9.1
commands = bash tools/lintstack.sh
pylint==2.1.1
commands =
bash ./tools/coding-checks.sh --pylint {posargs}
[testenv:cover]
# Also do not run test_coverage_ext tests while gathering coverage as those