Code Style Checker

Sat, 03 Apr 2021 14:51:08 +0200

author
Detlev Offenbach <detlev@die-offenbachs.de>
date
Sat, 03 Apr 2021 14:51:08 +0200
changeset 8195
db7f2badd374
parent 8194
b925628bf91f
child 8196
e3a5a84e409f

Code Style Checker
- completed the implementation of checkers for potential code simplifications

eric6/Plugins/CheckerPlugins/CodeStyleChecker/CodeStyleCheckerDialog.py file | annotate | diff | comparison | revisions
eric6/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/SimplifyChecker.py file | annotate | diff | comparison | revisions
eric6/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/SimplifyNodeVisitor.py file | annotate | diff | comparison | revisions
eric6/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/translations.py file | annotate | diff | comparison | revisions
--- a/eric6/Plugins/CheckerPlugins/CodeStyleChecker/CodeStyleCheckerDialog.py	Sat Apr 03 10:44:07 2021 +0200
+++ b/eric6/Plugins/CheckerPlugins/CodeStyleChecker/CodeStyleCheckerDialog.py	Sat Apr 03 14:51:08 2021 +0200
@@ -38,6 +38,7 @@
     basestring = str    # define for Python3
 
 
+# TODO: add a code filter to the results page
 class CodeStyleCheckerDialog(QDialog, Ui_CodeStyleCheckerDialog):
     """
     Class implementing a dialog to show the results of the code style check.
--- a/eric6/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/SimplifyChecker.py	Sat Apr 03 10:44:07 2021 +0200
+++ b/eric6/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/SimplifyChecker.py	Sat Apr 03 14:51:08 2021 +0200
@@ -24,7 +24,12 @@
         "Y117", "Y118", "Y119", "Y120",
         
         # Comparations
-        "Y201"
+        "Y201", "Y202", "Y203", "Y204", "Y205", "Y206", "Y207", "Y208",
+        "Y211", "Y212", "Y213",
+        "Y221", "Y222", "Y223", "Y224",
+        
+        # Opinionated
+        "Y301",
     ]
     
     def __init__(self, source, filename, selected, ignored, expected, repeat):
--- a/eric6/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/SimplifyNodeVisitor.py	Sat Apr 03 10:44:07 2021 +0200
+++ b/eric6/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/SimplifyNodeVisitor.py	Sat Apr 03 14:51:08 2021 +0200
@@ -9,6 +9,7 @@
 
 import ast
 import collections
+import copy
 import itertools
 
 try:
@@ -86,6 +87,10 @@
         """
         self.__check101(node)
         self.__check109(node)
+        self.__check221(node)
+        self.__check222(node)
+        self.__check223(node)
+        self.__check224(node)
         
         self.generic_visit(node)
     
@@ -105,6 +110,19 @@
         
         self.generic_visit(node)
     
+    def visit_IfExp(self, node):
+        """
+        Public method to process an IfExp node.
+        
+        @param node reference to the IfExp node
+        @type ast.IfExp
+        """
+        self.__check211(node)
+        self.__check212(node)
+        self.__check213(node)
+        
+        self.generic_visit(node)
+    
     def visit_For(self, node):
         """
         Public method to process a For node.
@@ -161,6 +179,7 @@
         @type ast.Compare
         """
         self.__check118(node)
+        self.__check301(node)
         
         self.generic_visit(node)
     
@@ -176,6 +195,24 @@
         
         self.generic_visit(node)
     
+    def visit_UnaryOp(self, node):
+        """
+        Public method to process a UnaryOp node.
+        
+        @param node reference to the UnaryOp node
+        @type ast.UnaryOp
+        """
+        self.__check201(node)
+        self.__check202(node)
+        self.__check203(node)
+        self.__check204(node)
+        self.__check205(node)
+        self.__check206(node)
+        self.__check207(node)
+        self.__check208(node)
+        
+        self.generic_visit(node)
+    
     #############################################################
     ## Helper methods for the various checkers below
     #############################################################
@@ -261,9 +298,26 @@
                 statementEqual = False
             if not statementEqual:
                 return False
+        
         return True
     
-    def __isStatementEqual(self, a: ast.stmt, b: ast.stmt) -> bool:
+    def __isSameExpression(self, a, b):
+        """
+        Private method to check, if two expressions are equal.
+        
+        @param a first expression to be checked
+        @type ast.expr
+        @param b second expression to be checked
+        @type ast.expr
+        @return flag indicating equal expressions
+        @rtype bool
+        """
+        if isinstance(a, ast.Name) and isinstance(b, ast.Name):
+            return a.id == b.id
+        else:
+            return False
+    
+    def __isStatementEqual(self, a, b):
         """
         Private method to check, if two statements are equal.
         
@@ -290,6 +344,53 @@
         else:
             return a == b
     
+    def __isExceptionCheck(self, node):
+        """
+        Private method to check, if the node is checking an exception.
+        
+        @param node reference to the node to be checked
+        @type ast.If
+        @return flag indicating an exception check
+        @rtype bool
+        """
+        return (
+            len(node.body) == 1 and isinstance(node.body[0], ast.Raise)
+        )
+    
+    def __negateTest(self, node):
+        """
+        Private method negate the given Compare node.
+        
+        @param node reference to the node to be negated
+        @type ast.Compare
+        @return node with negated logic
+        @rtype ast.Compare
+        """
+        newNode = copy.deepcopy(node)
+        op = newNode.ops[0]
+        if isinstance(op, ast.Eq):
+            op = ast.NotEq()
+        elif isinstance(op, ast.NotEq):
+            op = ast.Eq()
+        elif isinstance(op, ast.Lt):
+            op = ast.GtE()
+        elif isinstance(op, ast.LtE):
+            op = ast.Gt()
+        elif isinstance(op, ast.Gt):
+            op = ast.LtE()
+        elif isinstance(op, ast.GtE):
+            op = ast.Lt()
+        elif isinstance(op, ast.Is):
+            op = ast.IsNot()
+        elif isinstance(op, ast.IsNot):
+            op = ast.Is()
+        elif isinstance(op, ast.In):
+            op = ast.NotIn()
+        elif isinstance(op, ast.NotIn):
+            op = ast.In()
+        newNode.ops = [op]
+        return newNode
+    
     #############################################################
     ## Methods to check for possible code simplifications below
     #############################################################
@@ -955,6 +1056,374 @@
         ):
             self.__error(node.lineno - 1, node.col_offset, "Y120",
                          node.name)
+    
+    def __check201(self, node):
+        """
+        Private method to check for calls where an unary 'not' is used for
+        an unequality.
+        
+        @param node reference to the UnaryOp node
+        @type ast.UnaryOp
+        """
+        # not a == b
+        if not (
+            (
+                not isinstance(node.op, ast.Not) or
+                not isinstance(node.operand, ast.Compare) or
+                len(node.operand.ops) != 1 or
+                not isinstance(node.operand.ops[0], ast.Eq)
+            ) or
+            isinstance(node.parent, ast.If) and
+            self.__isExceptionCheck(node.parent)
+        ):
+            comparison = node.operand
+            left = unparse(comparison.left)
+            right = unparse(comparison.comparators[0])
+            self.__error(node.lineno - 1, node.col_offset, "Y201",
+                         left, right)
+    
+    def __check202(self, node):
+        """
+        Private method to check for calls where an unary 'not' is used for
+        an equality.
+        
+        @param node reference to the UnaryOp node
+        @type ast.UnaryOp
+        """
+        # not a != b
+        if not (
+            (
+                not isinstance(node.op, ast.Not) or
+                not isinstance(node.operand, ast.Compare) or
+                len(node.operand.ops) != 1 or
+                not isinstance(node.operand.ops[0], ast.NotEq)
+            ) or
+            isinstance(node.parent, ast.If) and
+            self.__isExceptionCheck(node.parent)
+        ):
+            comparison = node.operand
+            left = unparse(comparison.left)
+            right = unparse(comparison.comparators[0])
+            self.__error(node.lineno - 1, node.col_offset, "Y202",
+                         left, right)
+    
+    def __check203(self, node):
+        """
+        Private method to check for calls where an unary 'not' is used for
+        an in-check.
+        
+        @param node reference to the UnaryOp node
+        @type ast.UnaryOp
+        """
+        # not a in b
+        if not (
+            (
+                not isinstance(node.op, ast.Not) or
+                not isinstance(node.operand, ast.Compare) or
+                len(node.operand.ops) != 1 or
+                not isinstance(node.operand.ops[0], ast.In)
+            ) or
+            isinstance(node.parent, ast.If) and
+            self.__isExceptionCheck(node.parent)
+        ):
+            comparison = node.operand
+            left = unparse(comparison.left)
+            right = unparse(comparison.comparators[0])
+            self.__error(node.lineno - 1, node.col_offset, "Y203",
+                         left, right)
+    
+    def __check204(self, node):
+        """
+        Private method to check for calls of the type "not (a < b)".
+        
+        @param node reference to the UnaryOp node
+        @type ast.UnaryOp
+        """
+        # not a < b
+        if not (
+            (
+                not isinstance(node.op, ast.Not) or
+                not isinstance(node.operand, ast.Compare) or
+                len(node.operand.ops) != 1 or
+                not isinstance(node.operand.ops[0], ast.Lt)
+            ) or
+            isinstance(node.parent, ast.If) and
+            self.__isExceptionCheck(node.parent)
+        ):
+            comparison = node.operand
+            left = unparse(comparison.left)
+            right = unparse(comparison.comparators[0])
+            self.__error(node.lineno - 1, node.col_offset, "Y204",
+                         left, right)
+    
+    def __check205(self, node):
+        """
+        Private method to check for calls of the type "not (a <= b)".
+        
+        @param node reference to the UnaryOp node
+        @type ast.UnaryOp
+        """
+        # not a <= b
+        if not (
+            (
+                not isinstance(node.op, ast.Not) or
+                not isinstance(node.operand, ast.Compare) or
+                len(node.operand.ops) != 1 or
+                not isinstance(node.operand.ops[0], ast.LtE)
+            ) or
+            isinstance(node.parent, ast.If) and
+            self.__isExceptionCheck(node.parent)
+        ):
+            comparison = node.operand
+            left = unparse(comparison.left)
+            right = unparse(comparison.comparators[0])
+            self.__error(node.lineno - 1, node.col_offset, "Y205",
+                         left, right)
+    
+    def __check206(self, node):
+        """
+        Private method to check for calls of the type "not (a > b)".
+        
+        @param node reference to the UnaryOp node
+        @type ast.UnaryOp
+        """
+        # not a > b
+        if not (
+            (
+                not isinstance(node.op, ast.Not) or
+                not isinstance(node.operand, ast.Compare) or
+                len(node.operand.ops) != 1 or
+                not isinstance(node.operand.ops[0], ast.Gt)
+            ) or
+            isinstance(node.parent, ast.If) and
+            self.__isExceptionCheck(node.parent)
+        ):
+            comparison = node.operand
+            left = unparse(comparison.left)
+            right = unparse(comparison.comparators[0])
+            self.__error(node.lineno - 1, node.col_offset, "Y206",
+                         left, right)
+    
+    def __check207(self, node):
+        """
+        Private method to check for calls of the type "not (a >= b)".
+        
+        @param node reference to the UnaryOp node
+        @type ast.UnaryOp
+        """
+        # not a >= b
+        if not (
+            (
+                not isinstance(node.op, ast.Not) or
+                not isinstance(node.operand, ast.Compare) or
+                len(node.operand.ops) != 1 or
+                not isinstance(node.operand.ops[0], ast.GtE)
+            ) or
+            isinstance(node.parent, ast.If) and
+            self.__isExceptionCheck(node.parent)
+        ):
+            comparison = node.operand
+            left = unparse(comparison.left)
+            right = unparse(comparison.comparators[0])
+            self.__error(node.lineno - 1, node.col_offset, "Y207",
+                         left, right)
+    
+    def __check208(self, node):
+        """
+        Private method to check for calls of the type "not (not a)".
+        
+        @param node reference to the UnaryOp node
+        @type ast.UnaryOp
+        """
+        # not (not a)
+        if (
+            isinstance(node.op, ast.Not) and
+            isinstance(node.operand, ast.UnaryOp) and
+            isinstance(node.operand.op, ast.Not)
+        ):
+            var = unparse(node.operand.operand)
+            self.__error(node.lineno - 1, node.col_offset, "Y208", var)
+    
+    def __check211(self, node):
+        """
+        Private method to check for calls of the type "True if a else False".
+        
+        @param node reference to the AST node to be checked
+        @type ast.IfExp
+        """
+        # True if a else False
+        if (
+            isinstance(node.body, BOOL_CONST_TYPES) and
+            node.body.value is True and
+            isinstance(node.orelse, BOOL_CONST_TYPES) and
+            node.orelse.value is False
+        ):
+            cond = unparse(node.test)
+            if isinstance(node.test, ast.Name):
+                newCond = "bool({0})".format(cond)
+            else:
+                newCond = cond
+            self.__error(node.lineno - 1, node.col_offset, "Y211",
+                         cond, newCond)
+    
+    def __check212(self, node):
+        """
+        Private method to check for calls of the type "False if a else True".
+        
+        @param node reference to the AST node to be checked
+        @type ast.IfExp
+        """
+        # False if a else True
+        if (
+            isinstance(node.body, BOOL_CONST_TYPES) and
+            node.body.value is False and
+            isinstance(node.orelse, BOOL_CONST_TYPES) and
+            node.orelse.value is True
+        ):
+            cond = unparse(node.test)
+            if isinstance(node.test, ast.Name):
+                newCond = "not {0}".format(cond)
+            else:
+                if len(node.test.ops) == 1:
+                    newCond = unparse(self.__negateTest(node.test))
+                else:
+                    newCond = "not ({0})".format(cond)
+            self.__error(node.lineno - 1, node.col_offset, "Y212",
+                         cond, newCond)
+    
+    def __check213(self, node):
+        """
+        Private method to check for calls of the type "b if not a else a".
+        
+        @param node reference to the AST node to be checked
+        @type ast.IfExp
+        """
+        # b if not a else a
+        if (
+            isinstance(node.test, ast.UnaryOp) and
+            isinstance(node.test.op, ast.Not) and
+            self.__isSameExpression(node.test.operand, node.orelse)
+        ):
+            a = unparse(node.test.operand)
+            b = unparse(node.body)
+            self.__error(node.lineno - 1, node.col_offset, "Y213", a, b)
+    
+    def __check221(self, node):
+        """
+        Private method to check for calls of the type "a and not a".
+        
+        @param node reference to the AST node to be checked
+        @type ast.BoolOp
+        """
+        # a and not a
+        if (
+            isinstance(node.op, ast.And) and
+            len(node.values) >= 2
+        ):
+            # We have a boolean And. Let's make sure there is two times the
+            # same expression, but once with a "not"
+            negatedExpressions = []
+            nonNegatedExpressions = []
+            for exp in node.values:
+                if (
+                    isinstance(exp, ast.UnaryOp) and
+                    isinstance(exp.op, ast.Not)
+                ):
+                    negatedExpressions.append(exp.operand)
+                else:
+                    nonNegatedExpressions.append(exp)
+            for negatedExpression in negatedExpressions:
+                for nonNegatedExpression in nonNegatedExpressions:
+                    if self.__isSameExpression(
+                        negatedExpression, nonNegatedExpression
+                    ):
+                        negExp = unparse(negatedExpression)
+                        self.__error(node.lineno - 1, node.col_offset, "Y221",
+                                     negExp)
+    
+    def __check222(self, node):
+        """
+        Private method to check for calls of the type "a or not a".
+        
+        @param node reference to the AST node to be checked
+        @type ast.BoolOp
+        """
+        # a or not a
+        if (
+            isinstance(node.op, ast.Or) and
+            len(node.values) >= 2
+        ):
+            # We have a boolean And. Let's make sure there is two times the
+            # same expression, but once with a "not"
+            negatedExpressions = []
+            nonNegatedExpressions = []
+            for exp in node.values:
+                if (
+                    isinstance(exp, ast.UnaryOp) and
+                    isinstance(exp.op, ast.Not)
+                ):
+                    negatedExpressions.append(exp.operand)
+                else:
+                    nonNegatedExpressions.append(exp)
+            for negatedExpression in negatedExpressions:
+                for nonNegatedExpression in nonNegatedExpressions:
+                    if self.__isSameExpression(
+                        negatedExpression, nonNegatedExpression
+                    ):
+                        negExp = unparse(negatedExpression)
+                        self.__error(node.lineno - 1, node.col_offset, "Y222",
+                                     negExp)
+    
+    def __check223(self, node):
+        """
+        Private method to check for calls of the type "... or True".
+        
+        @param node reference to the AST node to be checked
+        @type ast.BoolOp
+        """
+        # a or True
+        if isinstance(node.op, ast.Or):
+            for exp in node.values:
+                if isinstance(exp, BOOL_CONST_TYPES) and exp.value is True:
+                    self.__error(node.lineno - 1, node.col_offset, "Y223")
+    
+    def __check224(self, node):
+        """
+        Private method to check for calls of the type "... and False".
+        
+        @param node reference to the AST node to be checked
+        @type ast.BoolOp
+        """
+        # a and False
+        if isinstance(node.op, ast.And):
+            for exp in node.values:
+                if isinstance(exp, BOOL_CONST_TYPES) and exp.value is False:
+                    self.__error(node.lineno - 1, node.col_offset, "Y224")
+    
+    def __check301(self, node):
+        """
+        Private method to check for Yoda conditions.
+        
+        @param node reference to the AST node to be checked
+        @type ast.Compare
+        """
+        # 42 == age
+        if (
+            isinstance(node.left, AST_CONST_TYPES) and
+            len(node.ops) == 1 and
+            isinstance(node.ops[0], ast.Eq)
+        ):
+            left = unparse(node.left)
+            isPy37Str = isinstance(node.left, ast.Str)
+            isPy38Str = (
+                isinstance(node.left, ast.Constant) and
+                isinstance(node.left.value, str)
+            )
+            if isPy37Str or isPy38Str:
+                left = f"'{left}'"
+            right = unparse(node.comparators[0])
+            self.__error(node.lineno - 1, node.col_offset, "Y301",
+                         left, right)
 
 #
 # eflag: noqa = M891
--- a/eric6/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/translations.py	Sat Apr 03 10:44:07 2021 +0200
+++ b/eric6/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/translations.py	Sat Apr 03 14:51:08 2021 +0200
@@ -12,6 +12,7 @@
 from PyQt5.QtCore import QCoreApplication
 
 _simplifyMessages = {
+    # Python-specifics
     "Y101": QCoreApplication.translate(
         "SimplifyChecker",
         '''Multiple "isinstance()" calls which can be merged into a single '''
@@ -75,9 +76,63 @@
     "Y120": QCoreApplication.translate(
         "SimplifyChecker",
         '''Use "class {0}:" instead of "class {0}(object):"'''),
+    
+    # Comparations
+    "Y201": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{0} != {1}" instead of "not {0} == {1}"'''),
+    "Y202": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{0} == {1}" instead of "not {0} != {1}"'''),
+    "Y203": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{0} not in {1}" instead of "not {0} in {1}"'''),
+    "Y204": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{0} >= {1}" instead of "not ({0} < {1})"'''),
+    "Y205": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{0} > {1}" instead of "not ({0} <= {1})"'''),
+    "Y206": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{0} <= {1}" instead of "not ({0} > {1})"'''),
+    "Y207": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{0} < {1}" instead of "not ({0} >= {1})"'''),
+    "Y208": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{0}" instead of "not (not {0})"'''),
+    
+    "Y211": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{1}" instead of "True if {0} else False"'''),
+    "Y212": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{1}" instead of "False if {0} else True"'''),
+    "Y213": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{0} if {0} else {1}" instead of "{1} if not {0} else {0}"'''),
+    
+    "Y221": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "False" instead of "{0} and not {0}"'''),
+    "Y222": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "True" instead of "{0} or not {0}"'''),
+    "Y223": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "True" instead of "... or True"'''),
+    "Y224": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "False" instead of "... and False"'''),
+    
+    "Y301": QCoreApplication.translate(
+        "SimplifyChecker",
+        '''Use "{1} == {0}" instead of "{0} == {1}" (Yoda-condition)'''),
 }
 
 _simplifyMessagesSampleArgs = {
+    # Python-specifics
     "Y101": ["foo"],
     "Y103": ["foo != bar"],
     "Y104": ["iterable"],
@@ -94,4 +149,23 @@
     "Y118": ["foo", "bar_dict"],
     "Y119": ["Foo"],
     "Y120": ["Foo"],
+    
+    # Comparations
+    "Y201": ["foo", "bar"],
+    "Y202": ["foo", "bar"],
+    "Y203": ["foo", "bar"],
+    "Y204": ["foo", "bar"],
+    "Y205": ["foo", "bar"],
+    "Y206": ["foo", "bar"],
+    "Y207": ["foo", "bar"],
+    "Y208": ["foo"],
+    
+    "Y211": ["foo", "bool(foo"],
+    "Y212": ["foo", "not foo"],
+    "Y213": ["foo", "bar"],
+    
+    "Y221": ["foo"],
+    "Y222": ["foo"],
+    
+    "Y301": ["42", "foo"],
 }

eric ide

mercurial