diff --git a/.gitignore b/.gitignore index 7737ac4af45..5c7fdbd5efe 100644 --- a/.gitignore +++ b/.gitignore @@ -23,4 +23,6 @@ keeper keys local_settings.py tools/conf/cinder.conf* +tools/lintstack.head.py +tools/pylint_exceptions tags diff --git a/tools/lintstack.py b/tools/lintstack.py new file mode 100755 index 00000000000..4130d105f67 --- /dev/null +++ b/tools/lintstack.py @@ -0,0 +1,199 @@ +#!/usr/bin/env python +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# 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 cStringIO as StringIO +import json +import re +import sys + +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 cinder.tests modules +ignore_messages = ["An attribute affected in cinder.tests"] +# Note(maoy): we ignore all errors in openstack.common because it should be +# checked elsewhere. We also ignore cinder.tests for now due to high false +# positive rate. +ignore_modules = ["cinder/openstack/common/", "cinder/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) + 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.StringIO() + reporter = text.ParseableTextReporter(output=buff) + args = ["--include-ids=y", "-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() diff --git a/tools/lintstack.sh b/tools/lintstack.sh new file mode 100755 index 00000000000..d8591d03dd1 --- /dev/null +++ b/tools/lintstack.sh @@ -0,0 +1,59 @@ +#!/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 666001a36c9..b84c2f42161 100644 --- a/tox.ini +++ b/tox.ini @@ -35,3 +35,9 @@ 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