diff --git a/doc/whatsnew/fragments/7626.false_positive b/doc/whatsnew/fragments/7626.false_positive new file mode 100644 index 0000000000..bfa56c1072 --- /dev/null +++ b/doc/whatsnew/fragments/7626.false_positive @@ -0,0 +1,4 @@ +Fix a false positive for ``simplify-boolean-expression`` when multiple values +are inferred for a constant. + +Closes #7626 diff --git a/doc/whatsnew/fragments/7626.other b/doc/whatsnew/fragments/7626.other new file mode 100644 index 0000000000..8020fc83f3 --- /dev/null +++ b/doc/whatsnew/fragments/7626.other @@ -0,0 +1,3 @@ +Add a keyword-only ``compare_constants`` argument to ``safe_infer``. + +Refs #7626 diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index ed1b7a9380..60f7f397b4 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -21,7 +21,7 @@ from pylint import checkers from pylint.checkers import utils from pylint.checkers.utils import node_frame_class -from pylint.interfaces import HIGH +from pylint.interfaces import HIGH, INFERENCE if TYPE_CHECKING: from pylint.lint import PyLinter @@ -1495,11 +1495,10 @@ def visit_return(self, node: nodes.Return | nodes.Assign) -> None: ): return - inferred_truth_value = utils.safe_infer(truth_value) + inferred_truth_value = utils.safe_infer(truth_value, compare_constants=True) if inferred_truth_value is None or inferred_truth_value == astroid.Uninferable: - truth_boolean_value = True - else: - truth_boolean_value = inferred_truth_value.bool_value() + return + truth_boolean_value = inferred_truth_value.bool_value() if truth_boolean_value is False: message = "simplify-boolean-expression" @@ -1507,7 +1506,7 @@ def visit_return(self, node: nodes.Return | nodes.Assign) -> None: else: message = "consider-using-ternary" suggestion = f"{truth_value.as_string()} if {cond.as_string()} else {false_value.as_string()}" - self.add_message(message, node=node, args=(suggestion,)) + self.add_message(message, node=node, args=(suggestion,), confidence=INFERENCE) def _append_context_managers_to_stack(self, node: nodes.Assign) -> None: if _is_inside_context_manager(node): diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 65ecf57b66..cfcdccd066 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -1343,12 +1343,18 @@ def _get_python_type_of_node(node: nodes.NodeNG) -> str | None: @lru_cache(maxsize=1024) def safe_infer( - node: nodes.NodeNG, context: InferenceContext | None = None + node: nodes.NodeNG, + context: InferenceContext | None = None, + *, + compare_constants: bool = False, ) -> InferenceResult | None: """Return the inferred value for the given node. Return None if inference failed or if there is some ambiguity (more than one node has been inferred of different types). + + If compare_constants is True and if multiple constants are inferred, + unequal inferred values are also considered ambiguous and return None. """ inferred_types: set[str | None] = set() try: @@ -1367,6 +1373,13 @@ def safe_infer( inferred_type = _get_python_type_of_node(inferred) if inferred_type not in inferred_types: return None # If there is ambiguity on the inferred node. + if ( + compare_constants + and isinstance(inferred, nodes.Const) + and isinstance(value, nodes.Const) + and inferred.value != value.value + ): + return None if ( isinstance(inferred, nodes.FunctionDef) and inferred.args.args is not None diff --git a/tests/functional/t/ternary.py b/tests/functional/t/ternary.py index 58171942fb..48f97ffd97 100644 --- a/tests/functional/t/ternary.py +++ b/tests/functional/t/ternary.py @@ -1,18 +1,23 @@ """Test for old ternary constructs""" -from UNINFERABLE import condition, true_value, false_value, some_callable # pylint: disable=import-error +from UNINFERABLE import condition, some_callable, maybe_true, maybe_false # pylint: disable=import-error -SOME_VALUE1 = true_value if condition else false_value -SOME_VALUE2 = condition and true_value or false_value # [consider-using-ternary] +TRUE_VALUE = True +FALSE_VALUE = False + +SOME_VALUE1 = TRUE_VALUE if condition else FALSE_VALUE +SOME_VALUE2 = condition and TRUE_VALUE or FALSE_VALUE # [consider-using-ternary] +NOT_SIMPLIFIABLE_1 = maybe_true if condition else maybe_false +NOT_SIMPLIFIABLE_2 = condition and maybe_true or maybe_false SOME_VALUE3 = condition def func1(): """Ternary return value correct""" - return true_value if condition else false_value + return TRUE_VALUE if condition else FALSE_VALUE def func2(): """Ternary return value incorrect""" - return condition and true_value or false_value # [consider-using-ternary] + return condition and TRUE_VALUE or FALSE_VALUE # [consider-using-ternary] SOME_VALUE4 = some_callable(condition) and 'ERROR' or 'SUCCESS' # [consider-using-ternary] @@ -30,10 +35,23 @@ def func2(): def func4(): """"Using a Name as a condition but still emits""" truth_value = 42 - return condition and truth_value or false_value # [consider-using-ternary] + return condition and truth_value or FALSE_VALUE # [consider-using-ternary] def func5(): """"Using a Name that infers to False as a condition does not emit""" falsy_value = False - return condition and falsy_value or false_value # [simplify-boolean-expression] + return condition and falsy_value or FALSE_VALUE # [simplify-boolean-expression] + + +def func_control_flow(): + """Redefining variables should invalidate simplify-boolean-expression.""" + flag_a = False + flag_b = False + for num in range(2): + if num == 1: + flag_a = True + else: + flag_b = True + multiple = (flag_a and flag_b) or func5() + return multiple diff --git a/tests/functional/t/ternary.txt b/tests/functional/t/ternary.txt index bdec7bcc43..ca93acd2fa 100644 --- a/tests/functional/t/ternary.txt +++ b/tests/functional/t/ternary.txt @@ -1,8 +1,8 @@ -consider-using-ternary:5:0:5:53::Consider using ternary (true_value if condition else false_value):UNDEFINED -consider-using-ternary:15:4:15:50:func2:Consider using ternary (true_value if condition else false_value):UNDEFINED -consider-using-ternary:18:0:18:63::Consider using ternary ('ERROR' if some_callable(condition) else 'SUCCESS'):UNDEFINED -consider-using-ternary:19:0:19:60::Consider using ternary ('greater' if SOME_VALUE1 > 3 else 'not greater'):UNDEFINED -consider-using-ternary:20:0:20:67::Consider using ternary ('both' if SOME_VALUE2 > 4 and SOME_VALUE3 else 'not'):UNDEFINED -simplify-boolean-expression:23:0:23:50::Boolean expression may be simplified to SOME_VALUE2:UNDEFINED -consider-using-ternary:33:4:33:51:func4:Consider using ternary (truth_value if condition else false_value):UNDEFINED -simplify-boolean-expression:39:4:39:51:func5:Boolean expression may be simplified to false_value:UNDEFINED +consider-using-ternary:8:0:8:53::Consider using ternary (TRUE_VALUE if condition else FALSE_VALUE):INFERENCE +consider-using-ternary:20:4:20:50:func2:Consider using ternary (TRUE_VALUE if condition else FALSE_VALUE):INFERENCE +consider-using-ternary:23:0:23:63::Consider using ternary ('ERROR' if some_callable(condition) else 'SUCCESS'):INFERENCE +consider-using-ternary:24:0:24:60::Consider using ternary ('greater' if SOME_VALUE1 > 3 else 'not greater'):INFERENCE +consider-using-ternary:25:0:25:67::Consider using ternary ('both' if SOME_VALUE2 > 4 and SOME_VALUE3 else 'not'):INFERENCE +simplify-boolean-expression:28:0:28:50::Boolean expression may be simplified to SOME_VALUE2:INFERENCE +consider-using-ternary:38:4:38:51:func4:Consider using ternary (truth_value if condition else FALSE_VALUE):INFERENCE +simplify-boolean-expression:44:4:44:51:func5:Boolean expression may be simplified to FALSE_VALUE:INFERENCE