--- a/src/eric7/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/SimplifyNodeVisitor.py Thu Jul 28 14:19:57 2022 +0200
+++ b/src/eric7/Plugins/CheckerPlugins/CodeStyleChecker/Simplify/SimplifyNodeVisitor.py Thu Jul 28 19:44:54 2022 +0200
@@ -11,6 +11,7 @@
import collections
import copy
import itertools
+import json
try:
from ast import unparse
@@ -18,8 +19,8 @@
# Python < 3.9
from .ast_unparse import unparse
-######################################################################
-## The following code is derived from the flake8-simplify package.
+###############################################################################
+## The following code is derived from the flake8-simplify package (v0.19.2).
##
## Original License:
##
@@ -44,8 +45,7 @@
## LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
## FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
## IN THE SOFTWARE.
-######################################################################
-# TODO: update to bandit v0.19.2
+###############################################################################
BOOL_CONST_TYPES = (ast.Constant, ast.NameConstant)
AST_CONST_TYPES = (ast.Constant, ast.NameConstant, ast.Str, ast.Num)
@@ -90,6 +90,8 @@
@type ast.Assign
"""
self.__check181(node)
+ self.__check904(node)
+ self.__check909(node)
self.generic_visit(node)
@@ -123,6 +125,7 @@
self.__check114(node)
self.__check116(node)
self.__check122(node)
+ self.__check123(node)
self.generic_visit(node)
@@ -176,6 +179,9 @@
self.__check182(node)
self.__check401(node)
self.__check402(node)
+ self.__check901(node)
+ self.__check905(node)
+ self.__check906(node)
self.generic_visit(node)
@@ -236,6 +242,17 @@
self.__check208(node)
self.generic_visit(node)
+
+ def visit_Subscript(self, node):
+ """
+ Public method to process a Subscript node.
+
+ @param node reference to the Subscript node
+ @type ast.Subscript
+ """
+ self.__check907(node)
+
+ self.generic_visit(node)
#############################################################
## Helper methods for the various checkers below
@@ -266,22 +283,21 @@
return [name for name, count in counter.items() if count > 1]
- def __isConstantIncrease(self, expression):
+ def __isConstantIncrease(self, expr):
"""
- Private method check the given expression for being a constant
- increase.
-
- @param expression reference to the expression node
+ Private method to check an expression for being a constant increase.
+
+ @param expr reference to the node to be checked
@type ast.AugAssign
@return flag indicating a constant increase
@rtype bool
"""
- return isinstance(expression.op, ast.Add) and (
+ return isinstance(expr.op, ast.Add) and (
(
- isinstance(expression.value, ast.Constant)
- and isinstance(expression.value.value, int)
+ isinstance(expr.value, ast.Constant)
+ and expr.value.value == 1
)
- or isinstance(expression.value, ast.Num)
+ or (isinstance(expr.value, ast.Num) and expr.value.n == 1)
)
def __getIfBodyPairs(self, node):
@@ -415,6 +431,36 @@
newNode.ops = [op]
return newNode
+ def __expressionUsesVariable(self, expr, var):
+ """
+ Private method to check, if a variable is used by an expression.
+
+ @param expr expression node to be checked
+ @type ast.expr
+ @param var variable name to be checked for
+ @type str
+ @return flag indicating the expression uses the variable
+ @rtype bool
+ """
+ return var in unparse(expr)
+ # This is WAY too broad, but it's better to have false-negatives than
+ # false-positives.
+
+ def __bodyContainsContinue(self, stmts):
+ """
+ Private method to check, if a list of statements contain a 'continue' statement.
+
+ @param stmts list of statements
+ @type list of ast.stmt
+ @return flag indicating a continue statement
+ @rtype bool
+ """
+ return any(
+ isinstance(stmt, ast.Continue)
+ or (isinstance(stmt, ast.If) and self.__bodyContainsContinue(stmt.body))
+ for stmt in stmts
+ )
+
#############################################################
## Methods to check for possible code simplifications below
#############################################################
@@ -514,6 +560,7 @@
or not isinstance(node.body[0].value.value, ast.Name)
or node.target.id != node.body[0].value.value.id
or node.orelse != []
+ or isinstance(node.parent, ast.AsyncFunctionDef)
):
iterable = unparse(node.iter)
self.__error(node.lineno - 1, node.col_offset, "Y104", iterable)
@@ -609,7 +656,6 @@
if (
(tryHasReturn or exceptHasReturn)
and finallyHasReturn
- and finallyReturn is not None
):
self.__error(finallyReturn.lineno - 1, finallyReturn.col_offset, "Y107")
@@ -645,7 +691,19 @@
and node.body[0].targets[0].id == node.orelse[0].targets[0].id
and not isinstance(node.parent, ast.If)
):
- assign = unparse(node.body[0].targets[0])
+ targetVar = node.body[0].targets[0]
+ assign = unparse(targetVar)
+
+ # It's part of a bigger if-elseif block:
+ if isinstance(node.parent, ast.If):
+ for n in node.parent.body:
+ if (
+ isinstance(n, ast.Assign)
+ and isinstance(n.targets[0], ast.Name)
+ and n.targets[0].id == targetVar.id
+ ):
+ return
+
body = unparse(node.body[0].value)
cond = unparse(node.test)
orelse = unparse(node.orelse[0].value)
@@ -720,6 +778,7 @@
and isinstance(node.body[0].body[0], ast.Return)
and isinstance(node.body[0].body[0].value, BOOL_CONST_TYPES)
and hasattr(node.body[0].body[0].value, "value")
+ and isinstance(node.next_sibling, ast.Return)
):
check = unparse(node.body[0].test)
target = unparse(node.target)
@@ -729,9 +788,15 @@
node.lineno - 1, node.col_offset, "Y110", check, target, iterable
)
elif node.body[0].body[0].value.value is False:
- check = "not " + check
- if check.startswith("not not "):
- check = check[len("not not ") :]
+ isCompoundExpression = " and " in check or " or " in check
+
+ if isCompoundExpression:
+ check = f"not ({check})"
+ else:
+ if check.startswith("not "):
+ check = check[len("not "):]
+ else:
+ check = f"not {check}"
self.__error(
node.lineno - 1, node.col_offset, "Y111", check, target, iterable
)
@@ -826,19 +891,44 @@
# for el in iterable:
# ...
# idx += 1
- variableCandidates = []
- for expression in node.body:
- if (
- isinstance(expression, ast.AugAssign)
- and self.__isConstantIncrease(expression)
- and isinstance(expression.target, ast.Name)
- ):
- variableCandidates.append(expression.target)
+ if not self.__bodyContainsContinue(node.body):
+ # Find variables that might just count the iteration of the current loop
+ variableCandidates = []
+ for expression in node.body:
+ if (
+ isinstance(expression, ast.AugAssign)
+ and self.__isConstantIncrease(expression)
+ and isinstance(expression.target, ast.Name)
+ ):
+ variableCandidates.append(expression.target)
+ strCandidates = [unparse(x) for x in variableCandidates]
+
+ olderSiblings = []
+ for olderSibling in node.parent.body:
+ if olderSibling is node:
+ break
+ olderSiblings.append(olderSibling)
- for candidate in variableCandidates:
- self.__error(
- candidate.lineno - 1, candidate.col_offset, "Y113", unparse(candidate)
- )
+ matches = [
+ n.targets[0]
+ for n in olderSiblings
+ if isinstance(n, ast.Assign)
+ and len(n.targets) == 1
+ and isinstance(n.targets[0], ast.Name)
+ and unparse(n.targets[0]) in strCandidates
+ ]
+ if len(matches) == 0:
+ return
+
+ sibling = node.previous_sibling
+ while sibling is not None:
+ sibling = sibling.previous_sibling
+
+ for match in matches:
+ variable = unparse(match)
+ self.__error(
+ match.lineno - 1, match.col_offset, "Y113", variable
+ )
def __check114(self, node):
"""
@@ -854,7 +944,10 @@
# b
ifBodyPairs = self.__getIfBodyPairs(node)
errorPairs = []
- for ifbody1, ifbody2 in itertools.combinations(ifBodyPairs, 2):
+ for i in range(len(ifBodyPairs) - 1):
+ # It's not all combinations because of this:
+ ifbody1 = ifBodyPairs[i]
+ ifbody2 = ifBodyPairs[i + 1]
if self.__isSameBody(ifbody1[1], ifbody2[1]):
errorPairs.append((ifbody1, ifbody2))
for ifbody1, ifbody2 in errorPairs:
@@ -925,11 +1018,14 @@
else:
bodyValueStr = "None"
if isinstance(node.test.comparators[0], ast.Str):
- keyValuePairs = {node.test.comparators[0].s: bodyValueStr}
+ value = (
+ bodyValueStr
+ if bodyValueStr[0] == '"' and bodyValueStr[-1] == '"' else
+ bodyValueStr[1:-1]
+ )
+ keyValuePairs = {node.test.comparators[0].s: value}
elif isinstance(node.test.comparators[0], ast.Num):
- keyValuePairs = {
- node.test.comparators[0].n: bodyValueStr,
- }
+ keyValuePairs = {node.test.comparators[0].n: bodyValueStr}
else:
keyValuePairs = {node.test.comparators[0].value: bodyValueStr}
while child:
@@ -947,13 +1043,22 @@
):
return
+ returnCall = child.body[0]
+ if isinstance(returnCall.value, ast.Call):
+ return
+
if isinstance(child.test.comparators[0], ast.Str):
key = child.test.comparators[0].s
elif isinstance(child.test.comparators[0], ast.Num):
key = child.test.comparators[0].n
else:
key = child.test.comparators[0].value
- keyValuePairs[key] = unparse(child.body[0].value).strip("'")
+
+ value = unparse(child.body[0].value)
+ if value[0] == '"' and value[-1] == '"':
+ value = value[1:-1]
+ keyValuePairs[key] = value
+
if len(child.orelse) == 1:
if isinstance(child.orelse[0], ast.If):
child = child.orelse[0]
@@ -1125,6 +1230,69 @@
dictname = unparse(node.test.comparators[0])
self.__error(node.lineno - 1, node.col_offset, "Y122", dictname, key)
+ def __check123(self, node):
+ """
+ Private method to check for complicated dictionary access with default value.
+
+ @param node reference to the AST node to be checked
+ @type ast.If
+ """
+ isPattern1 = (
+ len(node.body) == 1
+ and isinstance(node.body[0], ast.Assign)
+ and len(node.body[0].targets) == 1
+ and isinstance(node.body[0].value, ast.Subscript)
+ and len(node.orelse) == 1
+ and isinstance(node.orelse[0], ast.Assign)
+ and len(node.orelse[0].targets) == 1
+ and isinstance(node.test, ast.Compare)
+ and len(node.test.ops) == 1
+ and isinstance(node.test.ops[0], ast.In)
+ )
+
+ # just like pattern_1, but using NotIn and reversing if/else
+ isPattern2 = (
+ len(node.body) == 1
+ and isinstance(node.body[0], ast.Assign)
+ and len(node.orelse) == 1
+ and isinstance(node.orelse[0], ast.Assign)
+ and isinstance(node.orelse[0].value, ast.Subscript)
+ and isinstance(node.test, ast.Compare)
+ and len(node.test.ops) == 1
+ and isinstance(node.test.ops[0], ast.NotIn)
+ )
+
+ if isPattern1:
+ key = node.test.left
+ if unparse(key) != unparse(node.body[0].value.slice):
+ return
+ assignToIfBody = node.body[0].targets[0]
+ assignToElse = node.orelse[0].targets[0]
+ if unparse(assignToIfBody) != unparse(assignToElse):
+ return
+ dictName = node.test.comparators[0]
+ defaultValue = node.orelse[0].value
+ valueNode = node.body[0].targets[0]
+ keyStr = unparse(key)
+ dictStr = unparse(dictName)
+ defaultStr = unparse(defaultValue)
+ valueStr = unparse(valueNode)
+ elif isPattern2:
+ key = node.test.left
+ if unparse(key) != unparse(node.orelse[0].value.slice):
+ return
+ dictName = node.test.comparators[0]
+ defaultValue = node.body[0].value
+ valueNode = node.body[0].targets[0]
+ keyStr = unparse(key)
+ dictStr = unparse(dictName)
+ defaultStr = unparse(defaultValue)
+ valueStr = unparse(valueNode)
+ else:
+ return
+ self.__error(node.lineno - 1, node.col_offset, "Y123", valueStr, dictStr,
+ keyStr, defaultStr)
+
def __check181(self, node):
"""
Private method to check for assignments that could be converted into
@@ -1550,6 +1718,171 @@
if hasBareNumeric and not isException:
self.__error(node.lineno - 1, node.col_offset, "Y402")
+ def __check901(self, node):
+ """
+ Private method to check for unnecessary bool conversion.
+
+ @param node reference to the AST node to be checked
+ @type ast.Call
+ """
+ if (
+ isinstance(node.func, ast.Name)
+ and node.func.id == "bool"
+ and len(node.args) == 1
+ and isinstance(node.args[0], ast.Compare)
+ ):
+ actual = unparse(node)
+ expected = unparse(node.args[0])
+ self.__error(node.lineno - 1, node.col_offset, "Y901", expected, actual)
+
+ def __check904(self, node):
+ """
+ Private method to check for dictionary initialization.
+
+ @param node reference to the AST node to be checked
+ @type ast.Assign
+ """
+ # a = {}; a['b'] = 'c'
+ n2 = node.next_sibling
+ if (
+ isinstance(node.value, ast.Dict)
+ and isinstance(n2, ast.Assign)
+ and len(n2.targets) == 1
+ and len(node.targets) == 1
+ and isinstance(n2.targets[0], ast.Subscript)
+ and isinstance(n2.targets[0].value, ast.Name)
+ and isinstance(node.targets[0], ast.Name)
+ and n2.targets[0].value.id == node.targets[0].id
+ ):
+ dictName = unparse(node.targets[0])
+ if not self.__expressionUsesVariable(n2.value, dictName):
+ self.__error(node.lineno - 1, node.col_offset, "Y904", dictName)
+
+ def __check905(self, node):
+ """
+ Private method to check for list initialization by splitting a string.
+
+ @param node reference to the AST node to be checked
+ @type ast.Call
+ """
+ if (
+ isinstance(node.func, ast.Attribute)
+ and node.func.attr == "split"
+ and isinstance(node.func.value, (ast.Str, ast.Constant))
+ ):
+ if isinstance(node.func.value, ast.Constant):
+ value = node.func.value.value
+ else:
+ value = node.func.value.s
+
+ expected = json.dumps(value.split())
+ actual = unparse(node.func.value) + ".split()"
+ self.__error(node.lineno - 1, node.col_offset, "Y905", expected, actual)
+
+ def __check906(self, node):
+ """
+ Private method to check for unnecessary nesting of os.path.join().
+
+ @param node reference to the AST node to be checked
+ @type ast.Call
+ """
+ # __IGNORE_WARNING_D234r__
+ def getOsPathJoinArgs(node):
+ names = []
+ for arg in node.args:
+ if (
+ isinstance(arg, ast.Call)
+ and isinstance(arg.func, ast.Attribute)
+ and isinstance(arg.func.value, ast.Attribute)
+ and isinstance(arg.func.value.value, ast.Name)
+ and arg.func.value.value.id == "os"
+ and arg.func.value.attr == "path"
+ and arg.func.attr == "join"
+ ):
+ names += getOsPathJoinArgs(arg)
+ elif isinstance(arg, ast.Name):
+ names.append(arg.id)
+ elif isinstance(arg, ast.Str):
+ names.append(f"'{arg.s}'")
+ return names
+
+ if (
+ isinstance(node.func, ast.Attribute)
+ and isinstance(node.func.value, ast.Attribute)
+ and isinstance(node.func.value.value, ast.Name)
+ and node.func.value.value.id == "os"
+ and node.func.value.attr == "path"
+ and node.func.attr == "join"
+ and len(node.args) == 2
+ and any(
+ (
+ isinstance(arg, ast.Call)
+ and isinstance(arg.func, ast.Attribute)
+ and isinstance(arg.func.value, ast.Attribute)
+ and isinstance(arg.func.value.value, ast.Name)
+ and arg.func.value.value.id == "os"
+ and arg.func.value.attr == "path"
+ and arg.func.attr == "join"
+ )
+ for arg in node.args
+ )
+ ):
+ names = getOsPathJoinArgs(node)
+
+ actual = unparse(node)
+ expected = "os.path.join({0})".format(', '.join(names))
+ self.__error(node.lineno - 1, node.col_offset, "Y906", expected, actual)
+
+ def __check907(self, node):
+ """
+ Private method to check for Union type annotation with None.
+
+ @param node reference to the AST node to be checked
+ @type ast.Subscript
+ """
+ if (isinstance(node.value, ast.Name) and node.value.id == "Union"):
+ if (
+ isinstance(node.slice, ast.Index)
+ and isinstance(node.slice.value, ast.Tuple)
+ ):
+ # Python 3.8
+ tupleVar = node.slice.value
+ elif isinstance(node.slice, ast.Tuple):
+ # Python 3.9+
+ tupleVar = node.slice
+ else:
+ return
+
+ hasNone = False
+ others = []
+ for elt in tupleVar.elts:
+ if isinstance(elt, BOOL_CONST_TYPES) and elt.value is None:
+ hasNone = True
+ else:
+ others.append(elt)
+
+ if len(others) == 1 and hasNone:
+ type_ = unparse(others[0])
+ self.__error(
+ node.lineno - 1, node.col_offset, "Y907", type_, unparse(node)
+ )
+
+ def __check909(self, node):
+ """
+ Private method to check for reflexive assignments.
+
+ @param node reference to the AST node to be checked
+ @type ast.Assign
+ """
+ names = []
+ if isinstance(node.value, (ast.Name, ast.Subscript, ast.Tuple)):
+ names.append(unparse(node.value))
+ for target in node.targets:
+ names.append(unparse(target))
+
+ if len(names) != len(set(names)) and not isinstance(node.parent, ast.ClassDef):
+ srccode = unparse(node)
+ self.__error(node.lineno - 1, node.col_offset, "Y909", srccode)
#
# eflag: noqa = M891
