diff --git a/.pylintrc b/.pylintrc new file mode 100644 index 00000000000..dc887074eca --- /dev/null +++ b/.pylintrc @@ -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 + diff --git a/pylintrc b/pylintrc deleted file mode 100644 index a5064360fdd..00000000000 --- a/pylintrc +++ /dev/null @@ -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 diff --git a/tools/coding-checks.sh b/tools/coding-checks.sh new file mode 100755 index 00000000000..71e2195c6d0 --- /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 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 diff --git a/tools/lintstack.py b/tools/lintstack.py deleted file mode 100755 index d833fdd19a9..00000000000 --- a/tools/lintstack.py +++ /dev/null @@ -1,276 +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 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() diff --git a/tools/lintstack.sh b/tools/lintstack.sh deleted file mode 100755 index d8591d03dd1..00000000000 --- 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 51b4c61a373..57ebbc3578d 100644 --- a/tox.ini +++ b/tox.ini @@ -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