From: Alexander Todorov atodorov@redhat.com

--- tests/pylint/eintr.py | 65 +++++++++ tests/pylint/environ.py | 107 ++++++++++++++ tests/pylint/intl.py | 2 - tests/pylint/pointless-override.py | 268 ++++++++++++++++++++++++++++++++++++ tests/pylint/pylint-false-positives | 4 +- tests/pylint/pylint-one.sh | 12 +- 6 files changed, 448 insertions(+), 10 deletions(-) create mode 100644 tests/pylint/eintr.py create mode 100644 tests/pylint/environ.py create mode 100644 tests/pylint/pointless-override.py

diff --git a/tests/pylint/eintr.py b/tests/pylint/eintr.py new file mode 100644 index 0000000..8fae96c --- /dev/null +++ b/tests/pylint/eintr.py @@ -0,0 +1,65 @@ +# Interuptible system call pylint module +# +# Copyright (C) 2014 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# +# Red Hat Author(s): David Shea dhsea@redhat.com +# + +import astroid + +from pylint.checkers import BaseChecker +from pylint.checkers.utils import check_messages, safe_infer +from pylint.interfaces import IAstroidChecker + +import os + +# These are all of the system calls exposed through the os module that are +# documented in SUSv4 as *may* set EINTR. Some of them probably don't in Linux, +# but who knows. lchmod, wait3 and wait4 aren't documented much anywhere but +# are here just in case. +interruptible = ("tmpfile", "close", "dup2", "fchmod", "fchown", "fstatvfs", + "fsync", "ftruncate", "open", "read", "write", "fchdir", + "chmod", "chown", "lchmod", "lchown", "statvfs", "wait", + "waitpid", "wait3", "wait4") + +class EintrChecker(BaseChecker): + __implements__ = (IAstroidChecker,) + name = "retry-interruptible" + msgs = {"W9930" : ("Found interruptible system call %s", + "interruptible-system-call", + "A system call that may raise EINTR is not wrapped in eintr_retry_call"), + } + + @check_messages("interruptible-system-call") + def visit_callfunc(self, node): + if not isinstance(node, astroid.CallFunc): + return + + # Skip anything not a function or not in os. os redirects most of its + # content to an OS-dependent module, named in os.name, so check that + # one too. + function_node = safe_infer(node.func) + if not isinstance(function_node, astroid.Function) or \ + function_node.root().name not in ("os", os.name): + return + + if function_node.name in interruptible: + self.add_message("interruptible-system-call", node=node, args=function_node.name) + +def register(linter): + """required method to auto register this checker """ + linter.register_checker(EintrChecker(linter)) diff --git a/tests/pylint/environ.py b/tests/pylint/environ.py new file mode 100644 index 0000000..ffbed49 --- /dev/null +++ b/tests/pylint/environ.py @@ -0,0 +1,107 @@ +# setenv pylint module +# +# Copyright (C) 2015 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# +# Red Hat Author(s): David Shea dshea@redhat.com +# + +import astroid + +from pylint.checkers import BaseChecker +from pylint.checkers.utils import check_messages, safe_infer +from pylint.interfaces import IAstroidChecker + +import os + +class EnvironChecker(BaseChecker): + __implements__ = (IAstroidChecker,) + name = "environ" + msgs = {"W9940" : ("Found potentially unsafe modification of environment", + "environment-modify", + "Potentially thread-unsafe modification of environment")} + + def _is_environ(self, node): + # Guess whether a node being modified is os.environ + + if isinstance(node, astroid.Getattr): + if node.attrname == "environ": + expr_node = safe_infer(node.expr) + if isinstance(expr_node, astroid.Module) and expr_node.name == "os": + return True + + # If the node being modified is just "environ" assume that it's os.environ + if isinstance(node, astroid.Name): + if node.name == "environ": + return True + + return False + + @check_messages("environment-modify") + def visit_assign(self, node): + if not isinstance(node, astroid.Assign): + return + + # Look for os.environ["WHATEVER"] = something + for target in node.targets: + if not isinstance(target, astroid.Subscript): + continue + + if self._is_environ(target.value): + self.add_message("environment-modify", node=node) + + @check_messages("environment-modify") + def visit_callfunc(self, node): + # Check both for uses of os.putenv and os.setenv and modifying calls + # to the os.environ object, such as os.environ.update + + if not isinstance(node, astroid.CallFunc): + return + + function_node = safe_infer(node.func) + if not isinstance(function_node, (astroid.Function, astroid.BoundMethod)): + return + + # If the function is from the os or posix modules, look for calls that + # modify the environment + if function_node.root().name in ("os", os.name) and \ + function_node.name in ("putenv", "unsetenv"): + self.add_message("environment-modify", node=node) + + # Look for methods bound to the environ dict + if isinstance(function_node, astroid.BoundMethod) and \ + isinstance(function_node.bound, astroid.Dict) and \ + function_node.bound.root().name in ("os", os.name) and \ + function_node.bound.name == "environ" and \ + function_node.name in ("clear", "pop", "popitem", "setdefault", "update"): + self.add_message("environment-modify", node=node) + + @check_messages("environment-modify") + def visit_delete(self, node): + if not isinstance(node, astroid.Delete): + return + + # Look for del os.environ["WHATEVER"] + for target in node.targets: + if not isinstance(target, astroid.Subscript): + continue + + if self._is_environ(target.value): + self.add_message("environment-modify", node=node) + +def register(linter): + """required method to auto register this checker """ + linter.register_checker(EnvironChecker(linter)) diff --git a/tests/pylint/intl.py b/tests/pylint/intl.py index 4a2a84d..ac27bb6 100644 --- a/tests/pylint/intl.py +++ b/tests/pylint/intl.py @@ -129,8 +129,6 @@ class IntlStringFormatChecker(StringFormatChecker): string format messages extended for translated strings") }

- options = () - @check_messages('translated-format') def visit_binop(self, node): if node.op != '%': diff --git a/tests/pylint/pointless-override.py b/tests/pylint/pointless-override.py new file mode 100644 index 0000000..e34abce --- /dev/null +++ b/tests/pylint/pointless-override.py @@ -0,0 +1,268 @@ +# Pylint checker for pointless class attributes overrides. +# +# Copyright (C) 2014 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# +# Red Hat Author(s): Anne Mulhern amulhern@redhat.com +# + +import abc + +import astroid + +from pylint.checkers import BaseChecker +from pylint.checkers.utils import check_messages +from pylint.interfaces import IAstroidChecker + +# taken from python-six-1.7.3-2 +# while RHEL 7 has python-six-1.3.0-4 +def add_metaclass(metaclass): + """Class decorator for creating a class with a metaclass.""" + def wrapper(cls): + orig_vars = cls.__dict__.copy() + orig_vars.pop('__dict__', None) + orig_vars.pop('__weakref__', None) + slots = orig_vars.get('__slots__') + if slots is not None: + if isinstance(slots, str): + slots = [slots] + for slots_var in slots: + orig_vars.pop(slots_var) + return metaclass(cls.__name__, cls.__bases__, orig_vars) + return wrapper + + +@add_metaclass(abc.ABCMeta) +class PointlessData(object): + + _DEF_CLASS = abc.abstractproperty(doc="Class of interesting definitions.") + message_id = abc.abstractproperty(doc="Pylint message identifier.") + + @classmethod + @abc.abstractmethod + def _retain_node(cls, node, restrict=True): + """ Determines whether to retain a node for the analysis. + + :param node: an AST node + :type node: astroid.Class + :param restrict bool: True if results returned should be restricted + :returns: True if the node should be kept, otherwise False + :rtype: bool + + Restricted nodes are candidates for being marked as overridden. + Only restricted nodes are put into the initial pool of candidates. + """ + raise NotImplementedError() + + @staticmethod + @abc.abstractmethod + def _extract_value(node): + """ Return the node that contains the assignment's value. + + :param node: an AST node + :type node: astroid.Class + :returns: the node corresponding to the value + :rtype: bool + """ + raise NotImplementedError() + + @staticmethod + @abc.abstractmethod + def _extract_targets(node): + """ Generates the names being assigned to. + + :param node: an AST node + :type node: astroid.Class + :returns: a list of assignment target names + :rtype: generator of str + """ + raise NotImplementedError() + + @classmethod + def get_data(cls, node, restrict=True): + """ Find relevant nodes for this analysis. + + :param node: an AST node + :type node: astroid.Class + :param restrict bool: True if results returned should be restricted + + :rtype: generator of astroid.Class + :returns: a generator of interesting nodes. + + Note that all nodes returned are guaranteed to be instances of + some class in self._DEF_CLASS. + """ + nodes = (n for n in node.body if isinstance(n, cls._DEF_CLASS)) + for n in nodes: + if cls._retain_node(n, restrict): + for name in cls._extract_targets(n): + yield (name, cls._extract_value(n)) + + @classmethod + @abc.abstractmethod + def check_equal(cls, node, other): + """ Check whether the two nodes are considered equal. + + :param node: some ast node + :param other: some ast node + + :rtype: bool + :returns: True if the nodes are considered equal, otherwise False + + If the method returns True, the nodes are actually equal, but it + may return False when the nodes are equal. + """ + raise NotImplementedError() + +class PointlessFunctionDefinition(PointlessData): + """ Looking for pointless function definitions. """ + + _DEF_CLASS = astroid.Function + message_id = "W9952" + + @classmethod + def _retain_node(cls, node, restrict=True): + return not restrict or \ + (len(node.body) == 1 and isinstance(node.body[0], astroid.Pass)) + + @classmethod + def check_equal(cls, node, other): + return len(node.body) == 1 and isinstance(node.body[0], astroid.Pass) and \ + len(other.body) == 1 and isinstance(other.body[0], astroid.Pass) + + @staticmethod + def _extract_value(node): + return node + + @staticmethod + def _extract_targets(node): + yield node.name + +class PointlessAssignment(PointlessData): + + _DEF_CLASS = astroid.Assign + message_id = "W9951" + + _VALUE_CLASSES = ( + astroid.Const, + astroid.Dict, + astroid.List, + astroid.Tuple + ) + + @classmethod + def _retain_node(cls, node, restrict=True): + return not restrict or isinstance(node.value, cls._VALUE_CLASSES) + + @classmethod + def check_equal(cls, node, other): + if type(node) != type(other): + return False + if isinstance(node, astroid.Const): + return node.value == other.value + if isinstance(node, (astroid.List, astroid.Tuple)): + return len(node.elts) == len(other.elts) and \ + all(cls.check_equal(n, o) for (n, o) in zip(node.elts, other.elts)) + if isinstance(node, astroid.Dict): + return len(node.items) == len(other.items) + return False + + @staticmethod + def _extract_value(node): + return node.value + + @staticmethod + def _extract_targets(node): + for target in node.targets: + yield target.name + +class PointlessClassAttributeOverrideChecker(BaseChecker): + """ If the nearest definition of the class attribute in the MRO assigns + it the same value, then the overriding definition is said to be + pointless. + + The algorithm for detecting a pointless attribute override is the following. + + * For each class, C: + - For each attribute assignment, + name_1 = name_2 ... name_n = l (where l is a literal): + * For each n in (n_1, n_2): + - Traverse the linearization of the MRO until the first + matching assignment n = l' is identified. If l is equal to l', + then consider that the assignment to l in C is a + pointless override. + + The algorithm for detecting a pointless method override has the same + general structure, and the same defects discussed below. + + Note that this analysis is neither sound nor complete. It is unsound + under multiple inheritance. Consider the following class hierarchy:: + + class A(object): + _attrib = False + + class B(A): + _attrib = False + + class C(A): + _attrib = True + + class D(B,C): + pass + + In this case, starting from B, B._attrib = False would be considered + pointless. However, for D the MRO is B, C, A, and removing the assignment + B._attrib = False would change the inherited value of D._attrib from + False to True. + + The analysis is incomplete because it will find some values unequal when + actually they are equal. + + The analysis is both incomplete and unsound because it expects that + assignments will always be made by means of the same syntax. + """ + + __implements__ = (IAstroidChecker,) + + name = "pointless class attribute override checker" + msgs = { + "W9951": + ( + "Assignment to class attribute %s overrides identical assignment in ancestor.", + "pointless-class-attribute-override", + "Assignment to class attribute that overrides assignment in ancestor that assigns identical value has no effect." + ), + "W9952": + ( + "definition of %s method overrides identical method definition in ancestor", + "pointless-method-definition-override", + "Overriding empty method definition with another empty method definition has no effect." + ) + } + + @check_messages("W9951", "W9952") + def visit_class(self, node): + for checker in (PointlessAssignment, PointlessFunctionDefinition): + for (name, value) in checker.get_data(node): + for a in node.ancestors(): + match = next((v for (n, v) in checker.get_data(a, False) if n == name), None) + if match is not None: + if checker.check_equal(value, match): + self.add_message(checker.message_id, node=value, args=(name,)) + break + +def register(linter): + linter.register_checker(PointlessClassAttributeOverrideChecker(linter)) diff --git a/tests/pylint/pylint-false-positives b/tests/pylint/pylint-false-positives index 5b839e6..ef96849 100644 --- a/tests/pylint/pylint-false-positives +++ b/tests/pylint/pylint-false-positives @@ -1,9 +1,7 @@ ^E1101:[ 0-9]*,[0-9]*:.*: Instance of '.*' has no 'get_property' member$ ^E1101:[ 0-9]*,[0-9]*:.*: Instance of '.*' has no 'set_property' member$ -^E0611:[ 0-9]*,[0-9]*: No name 'GLib' in module 'gi.repository'$ -^E0611:[ 0-9]*,[0-9]*:.*: No name 'GLib' in module 'gi.repository'$ ^E0611:[ 0-9]*,[0-9]*: No name '_isys' in module 'pyanaconda'$ ^E0611:[ 0-9]*,[0-9]*:.*: No name '_isys' in module 'pyanaconda'$ ^E0712:[ 0-9]*,[0-9]*:.*: Catching an exception which doesn't inherit from BaseException: GError$ -gi/module.py:[0-9]*: Warning: g_hash_table_insert_internal: assertion `hash_table != NULL' failed$ +gi/module.py:[0-9]*: Warning: g_hash_table_insert_internal: assertion 'hash_table != NULL' failed$ ^ g_type = info.get_g_type()$ diff --git a/tests/pylint/pylint-one.sh b/tests/pylint/pylint-one.sh index 6aed441..8a88f59 100755 --- a/tests/pylint/pylint-one.sh +++ b/tests/pylint/pylint-one.sh @@ -12,20 +12,20 @@ if grep -q '# pylint: skip-file' $1; then exit 0 fi

-file_suffix="$(eval echo $$#|sed s?/?_?g)" - pylint_output="$(pylint \ --msg-template='{msg_id}:{line:3d},{column}: {obj}: {msg}' \ -r n --disable=C,R --rcfile=/dev/null \ --dummy-variables-rgx=_ \ --ignored-classes=DefaultInstall,Popen,QueueFactory,TransactionSet \ --defining-attr-methods=__init__,_grabObjects,initialize,reset,start,setUp \ - --load-plugins=intl,preconf,markup \ + --load-plugins=intl,preconf,markup,eintr,pointless-override,environ \ + --init-import=y \ --init-hook=\ 'import gi.overrides, os; gi.overrides.__path__[0:0] = (os.environ["ANACONDA_WIDGETS_OVERRIDES"].split(":") if "ANACONDA_WIDGETS_OVERRIDES" in os.environ else [])' \ $DISABLED_WARN_OPTIONS \ $DISABLED_ERR_OPTIONS \ + $EXTRA_OPTIONS \ $NON_STRICT_OPTIONS "$@" 2>&1 | \ egrep -v -f "$FALSE_POSITIVES" \ )" @@ -33,6 +33,8 @@ gi.overrides.__path__[0:0] = (os.environ["ANACONDA_WIDGETS_OVERRIDES"].split(":" if [ -n "$(echo "$pylint_output" | fgrep -v '************* Module ')" ]; then # Replace the Module line with the actual filename pylint_output="$(echo "$pylint_output" | sed "s|* Module .*|* Module $(eval echo $$#)|")" - echo "$pylint_output" > pylint-out_$file_suffix - touch "pylint-$file_suffix-failed" + echo "$pylint_output" + exit 1 +else + exit 0 fi