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 <chucks@redhat.com>

.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
+

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 [<basecommit>] 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

tools/lintstack.py

@@ -1,232 +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 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()

tools/lintstack.sh

@@ -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
-

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