mirror of
https://github.com/astral-sh/ruff.git
synced 2026-10-28 10:45:37 +08:00
[ty] Preserve redundant-condition diagnostics with unreachable operands (#28374)
This commit is contained in:
1 parent
a063c73bf7
commit
4cdd004a71
3 files changed
+62
-1
No files matched your search
@@ -2426,6 +2426,55 @@ def short_circuit(value: object):
|
||||
pass
|
||||
```
|
||||
|
||||
We normally suppress `redundant-condition-strict` on a complete condition if any subexpression would
|
||||
trigger `redundant-condition`, to avoid reporting the same mistake twice. An unreachable
|
||||
subexpression is different: we would suppress its diagnostic because Python never evaluates it.
|
||||
Suppressing the outer diagnostic as well would leave no warning at all, so we still report the
|
||||
complete condition in these cases.
|
||||
|
||||
When Python checks `falsy and flag` in the below example, it checks `falsy` first. Since that is
|
||||
false, the whole condition is false, and Python skips checking `flag`. Replacing `flag` with
|
||||
`"yes"`, as in the second `if` test, does not change this: the `and` condition can still be
|
||||
determined to always be false without checking the truthiness of the string.
|
||||
|
||||
The `or` condition works the other way around. Since `truthy` is true, `truthy or ""` can be
|
||||
determined to be always true without checking the truthiness of the empty string.
|
||||
|
||||
The strings `"yes"` and `""` would normally trigger `redundant-condition`, since one always counts
|
||||
as true and the other always counts as false. Because neither is reached here, we report only
|
||||
`redundant-condition-strict` on each complete condition:
|
||||
|
||||
```py
|
||||
from typing import Literal
|
||||
|
||||
def unreachable_operands(falsy: Literal[False], truthy: Literal[True], flag: bool):
|
||||
if falsy and flag: # error: [redundant-condition-strict] "Condition `falsy and flag` is always false"
|
||||
pass
|
||||
if falsy and "yes": # error: [redundant-condition-strict] "Condition `falsy and "yes"` is always false"
|
||||
pass
|
||||
if truthy or "": # error: [redundant-condition-strict] "Condition `truthy or ""` is always true"
|
||||
pass
|
||||
```
|
||||
|
||||
The same applies to the strings in the inline `if` expressions below. Python's `a if test else b`
|
||||
syntax chooses between two values: it evaluates `a` if `test` is true, and `b` otherwise. The other
|
||||
expression is not evaluated. The chosen value is then checked by the outer `if` statement to decide
|
||||
whether to run its body.
|
||||
|
||||
In `False if True else "yes"`, the middle `True` selects `False`, so the first `if` body never runs.
|
||||
In `"yes" if False else True`, the middle `False` selects the `True` after `else`, so the second
|
||||
`if` body always runs. We report both complete conditions under `redundant-condition-strict`. As
|
||||
with the `and` and `or` examples above, neither string is evaluated, so we do not report a warning
|
||||
on the string or let its presence hide the warning about the complete condition:
|
||||
|
||||
```py
|
||||
def unreachable_branches():
|
||||
if False if True else "yes": # error: [redundant-condition-strict] "Condition `False if True else "yes"` is always false"
|
||||
pass
|
||||
if "yes" if False else True: # error: [redundant-condition-strict] "Condition `"yes" if False else True` is always true"
|
||||
pass
|
||||
```
|
||||
|
||||
## Boolean tests inside value expressions
|
||||
|
||||
A call's arguments compute values, but can contain their own boolean tests. Those tests are checked
|
||||
|
||||
@@ -348,7 +348,7 @@ impl<'db, 'ast> InferContext<'db, 'ast> {
|
||||
///
|
||||
/// This checks both whether the scope itself is reachable and whether the
|
||||
/// specific statement or expression containing this range is reachable.
|
||||
fn is_range_reachable(&self, range: TextRange) -> bool {
|
||||
pub(super) fn is_range_reachable(&self, range: TextRange) -> bool {
|
||||
let db = self.db;
|
||||
let index = semantic_index(self.db(), self.program_file);
|
||||
let scope_id = self.scope.file_scope_id(self.db());
|
||||
|
||||
@@ -765,6 +765,18 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
|
||||
return ConditionCheckResult::CheckEnclosingCondition;
|
||||
}
|
||||
|
||||
// An unreachable operand can retain a literal type, as in `False and "yes"`.
|
||||
// Its diagnostic would be discarded, so it must not suppress the enclosing condition.
|
||||
// Only scan reachability here if an enclosing condition could be reported. Otherwise,
|
||||
// diagnostic emission checks reachability after checking whether the rule is enabled.
|
||||
if preference == BooleanDiagnosticPreference::EnclosingCondition
|
||||
&& !self
|
||||
.context
|
||||
.is_range_reachable(condition.expression.range())
|
||||
{
|
||||
return ConditionCheckResult::CheckEnclosingCondition;
|
||||
}
|
||||
|
||||
let rule = condition.kind.rule();
|
||||
|
||||
if self.context.is_lint_enabled(rule) && !condition_context.exempts(self, &condition) {
|
||||
|
||||
Reference in new issue
Block a user