From 4d0abb4a149eea597e51f79c8e76d9d2284d65ee Mon Sep 17 00:00:00 2001 From: Chuck Short Date: Thu, 11 Oct 2018 15:08:16 -0400 Subject: [PATCH] Simplify running pylint 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 manila tree. - Allow the user to run on a particular changeset. Using like HEAD~1, etc. - 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. Change-Id: I398d1ba01a3fd7f1c86ad4065a7984d276f33383 Signed-off-by: Chuck Short --- .pylintrc | 189 +++++++++++++++++++++++++++++++++ tools/coding-checks.sh | 56 ++++++++++ tools/lintstack.py | 232 ----------------------------------------- tools/lintstack.sh | 59 ----------- tox.ini | 4 +- 5 files changed, 247 insertions(+), 293 deletions(-) create mode 100644 .pylintrc create mode 100755 tools/coding-checks.sh delete mode 100755 tools/lintstack.py delete mode 100755 tools/lintstack.sh diff --git a/.pylintrc b/.pylintrc new file mode 100644 index 0000000000..cad77928ee --- /dev/null +++ b/.pylintrc @@ -0,0 +1,189 @@ +[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) + no-member, + too-many-function-args, + not-callable, + assignment-from-none, + unsubscriptable-object, + used-prior-global-declaration, + not-an-iterable, +# "W" Warnings for stylistic problems or minor programming issues + unused-argument, + bad-indentation, + unused-variable, + useless-else-on-loop, + pointless-string-statement, + unused-import, + redefined-outer-name, + redefined-builtin, + attribute-defined-outside-init, + abstract-method, + fixme, + exec-used, + anomalous-backslash-in-string, + broad-except, + protected-access, + arguments-differ, + undefined-loop-variable, + try-except-raise, + global-statement, + super-init-not-called, + pointless-statement, + global-statement, + unnecessary-lambda, + keyword-arg-before-vararg, + deprecated-method, + useless-super-delegation, + eval-used, + wildcard-import, + reimported, + expression-not-assigned, + cell-var-from-loop, + signature-differs, +# "C" Coding convention violations + missing-docstring, + invalid-name, + wrong-import-order, + len-as-condition, + wrong-import-position, + bad-continuation, + too-many-lines, + misplaced-comparison-constant, + bad-mcs-classmethod-argument, + ungrouped-imports, + superfluous-parens, + unidiomatic-typecheck, + consider-iterating-dictionary, + bad-whitespace, + dangerous-default-value, + line-too-long, + consider-using-enumerate, + useless-import-alias, + singleton-comparison, +# "R" Refactor recommendations + no-self-use, + no-else-return, + too-many-locals, + too-many-public-methods, + consider-using-set-comprehension, + inconsistent-return-statements, + useless-object-inheritance, + too-few-public-methods, + too-many-boolean-expressions, + too-many-instance-attributes, + too-many-return-statements, + literal-comparison, + too-many-statements, + too-many-ancestors, + literal-comparison, + consider-merging-isinstance, + too-many-nested-blocks, + trailing-comma-tuple, + simplifiable-if-statement, + consider-using-in, + consider-using-ternary, + too-many-arguments + +[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 + diff --git a/tools/coding-checks.sh b/tools/coding-checks.sh new file mode 100755 index 0000000000..8a41223ee9 --- /dev/null +++ b/tools/coding-checks.sh @@ -0,0 +1,56 @@ +#!/bin/sh + +set -eu + +usage() { + echo "Usage: $0 [OPTION]..." + echo "Run Cinder's coding check(s)" + echo "" + echo " -Y, --pylint [] Run pylint check on the entire manila 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="manila" + 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 diff --git a/tools/lintstack.py b/tools/lintstack.py deleted file mode 100755 index 89d9e9454a..0000000000 --- a/tools/lintstack.py +++ /dev/null @@ -1,232 +0,0 @@ -#!/usr/bin/env python - -# Copyright (c) 2013, AT&T Labs, Yun Mao -# 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 re -import sys - -import six - -from pylint import lint -from pylint.reporters import text - -# Note(maoy): E1103 is error code related to partial type inference -ignore_codes = ["E1103"] -# Note(maoy): the error message is the pattern of E0202. It should be ignored -# for manila.tests modules -# Note(chen): the second error message is the pattern of [E0611] -# It should be ignored because use six module to keep py3.X compatibility. -ignore_messages = ["An attribute affected in manila.tests", - "No name 'urllib' in module '_MovedItems'"] -# Note(maoy): we ignore all errors in openstack.common because it should be -# checked elsewhere. We also ignore manila.tests for now due to high false -# positive rate. -ignore_modules = ["manila/openstack/common/", "manila/tests/"] - -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): - """From the output of pylint msg, to a dict. - - 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 - 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 = six.StringIO() - reporter = text.TextReporter(output=buff) - args = [ - "--msg-template='{path}:{line}: [{msg_id}i({symbol}), {obj}] {msg}'", - "-E", "manila"] - 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 check(): - print("Running pylint. Be patient...") - newmsg = run_pylint() - errors = LintOutput.from_msg_to_dict(newmsg) - - passed = True - for err_key, err_list in errors.items(): - for err in err_list: - print(err.review_str() + "\n") - passed = False - - if passed: - print("Congrats! pylint check passed.") - else: - print("\nPlease fix the errors above. If you believe they are false " - "positives, run 'tools/lintstack.py generate' to overwrite.") - sys.exit(1) - - -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() - elif option == "check": - check() - else: - usage() - - -if __name__ == "__main__": - main() diff --git a/tools/lintstack.sh b/tools/lintstack.sh deleted file mode 100755 index d8591d03dd..0000000000 --- a/tools/lintstack.sh +++ /dev/null @@ -1,59 +0,0 @@ -#!/usr/bin/env bash - -# Copyright (c) 2012-2013, AT&T Labs, Yun Mao -# 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 - diff --git a/tox.ini b/tox.ini index 98dad5edb1..394e340c4d 100644 --- a/tox.ini +++ b/tox.ini @@ -104,9 +104,9 @@ commands = [testenv:pylint] basepython = python3 deps = -r{toxinidir}/requirements.txt - pylint==1.9.0 + pylint==2.1.1 whitelist_externals = bash -commands = bash tools/lintstack.sh +commands = bash ./tools/coding-checks.sh --pylint {posargs} [testenv:lint] basepython = python3