[blivet:master 1/2] Add a simple pylint checker for pointless overrides.

mulhern amulhern at redhat.com
Fri Nov 21 18:07:33 UTC 2014


Invoke it when running pylint.

Signed-off-by: mulhern <amulhern at redhat.com>
---
 tests/pylint/pointless-override.py | 194 +++++++++++++++++++++++++++++++++++++
 tests/pylint/pylint-one.sh         |   2 +-
 2 files changed, 195 insertions(+), 1 deletion(-)
 create mode 100644 tests/pylint/pointless-override.py

diff --git a/tests/pylint/pointless-override.py b/tests/pylint/pointless-override.py
new file mode 100644
index 0000000..79795ce
--- /dev/null
+++ b/tests/pylint/pointless-override.py
@@ -0,0 +1,194 @@
+# 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 astroid
+
+from pylint.checkers import BaseChecker
+from pylint.checkers.utils import check_messages
+from pylint.interfaces import IAstroidChecker
+
+_VALUE_CLASSES = (
+    astroid.Const,
+    astroid.Dict,
+    astroid.List,
+    astroid.Tuple
+)
+
+def _get_function_definition_data(node, restrict=True):
+    """ Generate name * bool pair for for functions in class.
+
+        :param node: an AST node
+        :type node: astroid.Class
+        :param restrict bool: True if results returned should be restricted
+
+        :rtype: generator of str * bool
+        :returns: the name of the function and a representation of the function's
+           definition.
+
+        Returns data for function definitions in the class. The first part is
+        the name, the second part is a Boolean value which is True if the
+        body of the function is a single pass statement, otherwise False.
+        If restrict is True, only returns function definitions where the
+        body is pass.
+    """
+    stmts = (s for s in node.body if isinstance(s, astroid.Function))
+    for stmt in stmts:
+        if len(stmt.body) == 1 and isinstance(stmt.body[0], astroid.Pass):
+            yield (stmt, True)
+        else:
+            if not restrict:
+                yield (stmt, False)
+
+def _get_assignment_data(node, restrict=True):
+    """ Generate target * value pair for subset of interesting
+        assignments in a class.
+
+        :param node: an AST node
+        :type node: astroid.Class
+        :param restrict bool: True if results returned should be restricted
+
+        :rtype: generator of node * node
+        :returns: target and value for interesting assignment statements
+
+        Returns data for all simple statements, i.e., assignments to a single
+        target. If restrict is True, only return assignments where the
+        value has one of the types in _VALUE_CLASSES.
+    """
+    stmts = (s for s in node.body if isinstance(s, astroid.Assign))
+    for stmt in stmts:
+        targets = stmt.targets
+        if len(targets) != 1:
+            continue
+        target = targets[0]
+
+        value = stmt.value
+        if restrict and not isinstance(value, _VALUE_CLASSES):
+            continue
+        yield (target, value)
+
+def _check_equal(node, other):
+    """ Check whether the two nodes have equal values.
+
+        :param node: some ast node
+        :param other: some ast node
+
+        :rtype: bool
+        :returns: True if the nodes have equal values, otherwise False
+
+        Only checks nodes of interest, having types in _VALUE_CLASSES.
+
+        The algorithm is not complete and considers two dicts to be equal
+        only if they are both empty.
+    """
+    if type(node) != type(other):
+        return False
+
+    if not isinstance(node, _VALUE_CLASSES):
+        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(_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)
+
+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, n = l (where l is a literal):
+              * Traverse the linearization of the MRO until the first 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 deleting 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.
+    """
+
+    __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 (target, value) in _get_assignment_data(node):
+            for a in node.ancestors():
+                match = next((v for (n, v) in _get_assignment_data(a, False) if n.name == target.name), None)
+                if match is not None:
+                    if _check_equal(value, match):
+                        self.add_message("W9951", node=target, args=(target.name,))
+                    break
+
+        for (stmt, value) in _get_function_definition_data(node):
+            for a in node.ancestors():
+                match = next((v for (n, v) in _get_function_definition_data(a, False) if n.name == stmt.name), None)
+                if match is not None:
+                    if match == True:
+                        self.add_message("W9952", node=stmt, args=(stmt.name,))
+                    break
+
+def register(linter):
+    linter.register_checker(PointlessClassAttributeOverrideChecker(linter))
diff --git a/tests/pylint/pylint-one.sh b/tests/pylint/pylint-one.sh
index 82de490..9b1e79b 100755
--- a/tests/pylint/pylint-one.sh
+++ b/tests/pylint/pylint-one.sh
@@ -16,7 +16,7 @@ pylint_output="$(pylint \
     --dummy-variables-rgx=_ \
     --ignored-classes=Popen,TransactionSet \
     --defining-attr-methods=__init__,_grabObjects,initialize,reset,start,setUp \
-    --load-plugins=intl \
+    --load-plugins=intl,pointless-override \
     $DISABLED_WARN_OPTIONS \
     $DISABLED_ERR_OPTIONS \
     $NON_STRICT_OPTIONS "$@" 2>&1 | \
-- 
1.9.3



More information about the anaconda-patches mailing list