Adds pylint check for critical error in new patches

Pylint usage pattern is to take a look on a diff for pylint results between
previous code version and current patch. It's needed to critical issues only.

Change-Id: I4fc2ae732a7eaef2bc7bfacfd111ef849d003268

.gitignore

@@ -14,3 +14,5 @@ doc/build
 doc/source/sourcecode
 *.mo
 etc/ceilometer/ceilometer.conf.sample
+tools/lintstack.head.py
+tools/pylint_exceptions

pylintrc

@@ -0,0 +1,48 @@
+# The format of this file isn't really documented; just use --generate-rcfile
+[MASTER]
+# Add <file or directory> to the black list. It should be a base name, not a
+# path.
+ignore=openstack
+
+[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.
+# W0703: Catch "Exception".
+disable=C0111,W0511,W0142,W0622,W0703
+
+[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}$
+
+# Type attributes names can be 2 to 31 characters long, with lowercase and underscores
+attr-rgx=[a-z_][a-z0-9_]{1,30}$
+
+# Method names shold be at least 3 characters long and be lowercased with underscores
+method-rgx=([a-z_][a-z0-9_]{1,30}|setUp|tearDown)$
+
+# Module names matching sahara-* are ok (files in bin/)
+module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+)|(sahara-[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]
+
+# List of additional names supposed to be defined in builtins. Remember that
+# you should avoid to define new builtins when possible.
+# _ is used by our localization
+additional-builtins=_
+
+[TYPECHECK]
+generated-members=query,node_template,status_code,data

test-requirements.txt

@@ -16,6 +16,7 @@ MySQL-python
 oslosphinx>=2.2.0  # Apache-2.0
 oslotest>=1.1.0  # Apache-2.0
 psycopg2
+pylint==0.25.2
 pymongo>=2.5
 python-subunit>=0.0.18
 sphinx>=1.1.2,!=1.2.0,<1.3

tools/lintstack.py

@@ -0,0 +1,195 @@
+#!/usr/bin/env python
+
+# Copyright (c) 2012, 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
+
+from pylint import lint
+from pylint.reporters import text
+from six.moves import cStringIO as StringIO
+
+# These variables will be useful if we will need to skip some pylint checks
+ignore_codes = []
+ignore_messages = []
+# We ignore all errors in openstack.common because it should be checked
+# elsewhere.
+ignore_modules = ["ceilometer/openstack/common/"]
+
+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)
+        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, 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_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" % self.__dict__)
+
+
+class ErrorKeys(object):
+
+    @classmethod
+    def print_json(cls, errors, output=sys.stdout):
+        print >>output, "# automatically generated by tools/lintstack.py"
+        for i in sorted(errors.keys()):
+            print >>output, json.dumps(i)
+
+    @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.ParseableTextReporter(output=buff)
+    args = ["--include-ids=y", "-E", "ceilometer"]
+    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()

tools/lintstack.sh

@@ -0,0 +1,57 @@
+#!/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

@@ -47,6 +47,9 @@ commands =
 [testenv:docs]
 commands = python setup.py build_sphinx
 
+[testenv:pylint]
+commands = bash tools/lintstack.sh
+
 [testenv:venv]
 commands = {posargs}