diff --git a/doc/whatsnew/fragments/9800.false_negative b/doc/whatsnew/fragments/9800.false_negative new file mode 100644 index 0000000000..d7d09caf8f --- /dev/null +++ b/doc/whatsnew/fragments/9800.false_negative @@ -0,0 +1,3 @@ +Fix a false positive for `consider-using-min-max-builtin` when the assignment target is an attribute. + +Refs #9800 diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 3377bf1183..9e4de94cdd 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -919,11 +919,9 @@ def get_node_name(node: nodes.NodeNG) -> str: """Obtain simplest representation of a node as a string.""" if isinstance(node, nodes.Name): return node.name # type: ignore[no-any-return] - if isinstance(node, nodes.Attribute): - return node.attrname # type: ignore[no-any-return] if isinstance(node, nodes.Const): return str(node.value) - # this is a catch-all for nodes that are not of type Name or Attribute + # this is a catch-all for nodes that are not of type Name or Const # extremely helpful for Call or BinOp return node.as_string() # type: ignore[no-any-return] @@ -944,13 +942,11 @@ def get_node_name(node: nodes.NodeNG) -> str: # is of type name or attribute. Attribute referring to NamedTuple.x perse. # So we have to check that target is of these types - if hasattr(target, "name"): - target_assignation = target.name - elif hasattr(target, "attrname"): - target_assignation = target.attrname - else: + if not (hasattr(target, "name") or hasattr(target, "attrname")): return + target_assignation = get_node_name(target) + if len(node.test.ops) > 1: return operator, right_statement = node.test.ops[0] diff --git a/tests/functional/c/consider/consider_using_min_max_builtin.txt b/tests/functional/c/consider/consider_using_min_max_builtin.txt index ce1e98bd09..231d166f01 100644 --- a/tests/functional/c/consider/consider_using_min_max_builtin.txt +++ b/tests/functional/c/consider/consider_using_min_max_builtin.txt @@ -8,7 +8,7 @@ consider-using-max-builtin:26:0:27:19::Consider using 'value3 = max(value3, valu consider-using-min-builtin:29:0:30:18::Consider using 'value2 = min(value2, value)' instead of unnecessary if block:UNDEFINED consider-using-min-builtin:32:0:33:25::Consider using 'value = min(value, float(value3))' instead of unnecessary if block:UNDEFINED consider-using-min-builtin:36:0:37:27::Consider using 'value2 = min(value2, offset + value)' instead of unnecessary if block:UNDEFINED -consider-using-min-builtin:45:0:46:17::Consider using 'value = min(value, 10)' instead of unnecessary if block:UNDEFINED +consider-using-min-builtin:45:0:46:17::Consider using 'A1.value = min(A1.value, 10)' instead of unnecessary if block:UNDEFINED consider-using-min-builtin:69:0:70:11::Consider using 'A1 = min(A1, A2)' instead of unnecessary if block:UNDEFINED consider-using-max-builtin:72:0:73:11::Consider using 'A2 = max(A2, A1)' instead of unnecessary if block:UNDEFINED consider-using-min-builtin:75:0:76:11::Consider using 'A1 = min(A1, A2)' instead of unnecessary if block:UNDEFINED