[PATCH rhel7-branch 5/7] Backport pylint updates from master (atodorov).

Chris Lumens clumens at redhat.com
Wed Jun 3 13:55:42 UTC 2015


Related: rhbz#1196721
---
 tests/pylint/environ.py             | 107 +++++++++++++++
 tests/pylint/intl.py                |   2 -
 tests/pylint/markup.py              |   2 +-
 tests/pylint/pointless-override.py  | 252 ++++++++++++++++++++++++++++++++++++
 tests/pylint/pylint-false-positives |   8 +-
 tests/pylint/pylint-one.sh          |  12 +-
 6 files changed, 372 insertions(+), 11 deletions(-)
 create mode 100644 tests/pylint/environ.py
 create mode 100644 tests/pylint/pointless-override.py

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 at 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/markup.py b/tests/pylint/markup.py
index 690eb13..3bb85dd 100644
--- a/tests/pylint/markup.py
+++ b/tests/pylint/markup.py
@@ -112,7 +112,7 @@ class MarkupChecker(BaseChecker):
 
     @check_messages("invalid-markup", "invalid-markup-element", "unescaped-markup", "invalid-translated-markup", "invalid-translated-markup-element", "invalid-pango-translation", "unnecessary-markup")
     def visit_const(self, node):
-        if type(node.value) not in (types.StringType, types.UnicodeType):
+        if not isinstance(node.value, types.StringType) and not isinstance(node.value, types.UnicodeType):
             return
 
         if not is_markup(node.value):
diff --git a/tests/pylint/pointless-override.py b/tests/pylint/pointless-override.py
new file mode 100644
index 0000000..d10ba42
--- /dev/null
+++ b/tests/pylint/pointless-override.py
@@ -0,0 +1,252 @@
+# 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 at redhat.com>
+#
+
+import abc
+
+from six import add_metaclass
+
+import astroid
+
+from pylint.checkers import BaseChecker
+from pylint.checkers.utils import check_messages
+from pylint.interfaces import IAstroidChecker
+
+ at 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 not isinstance(node, other.__class__):
+            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 bc020fd..8eb467a 100644
--- a/tests/pylint/pylint-false-positives
+++ b/tests/pylint/pylint-false-positives
@@ -1,9 +1,11 @@
 ^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$
+^E0712:[ 0-9]*,[0-9]*:.*: Catching an exception which doesn't inherit from BaseException: S390Error$
+PyGIDeprecationWarning: GLib\..* is deprecated
+PyGIDeprecationWarning: GObject\..* is deprecated
+obj = getattr\(parent, name\)$
 gi/module.py:[0-9]*: Warning: g_hash_table_insert_internal: assertion 'hash_table != NULL' failed$
-^  g_type = info\.get_g_type\(\)$
+^  g_type = info\.get_g_type\(\)$
\ No newline at end of file
diff --git a/tests/pylint/pylint-one.sh b/tests/pylint/pylint-one.sh
index 4b155b8..6f06a22 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,eintr \
+    --load-plugins=intl,markup,eintr,pointless-override,environ,preconf \
+    --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
-- 
2.2.2



More information about the anaconda-patches mailing list