Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 124 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF050.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
### Errors (condition removed entirely)

# Simple if with pass
if True:
pass

# Simple if with ellipsis
if True:
...

# Side-effect-free condition (comparison)
import sys
if sys.version_info >= (3, 11):
pass

# Side-effect-free condition (boolean operator)
if x and y:
pass

# Nested in function
def nested():
if a:
pass

# Single-line form (pass)
if True: pass

# Single-line form (ellipsis)
if True: ...

# Multiple pass statements
if True:
pass
pass

# Mixed pass and ellipsis
if True:
pass
...

# Only statement in a with block
with pytest.raises(ValueError, match=msg):
if obj1:
pass


### Errors (condition preserved as expression statement)

# Function call
if foo():
pass

# Method call
if bar.baz():
pass

# Nested call in boolean operator
if x and foo():
pass

# Walrus operator with call
if (x := foo()):
pass

# Walrus operator without call
if (x := y):
pass

# Only statement in a suite
class Foo:
if foo():
pass


### No errors

# Non-empty body
if True:
bar()

# Body with non-stub statement
if True:
pass
foo()

# Has elif clause (handled by RUF047)
if True:
pass
elif True:
pass

# Has else clause (handled by RUF047)
if True:
pass
else:
pass

# TYPE_CHECKING block (handled by TC005)
from typing import TYPE_CHECKING
if TYPE_CHECKING:
pass

# Comment in body
if True:
# comment
pass

# Inline comment after pass
if True:
pass # comment

# Inline comment on if line
if True: # comment
pass

# Trailing comment at body indentation
if True:
pass
# trailing comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an example where the if is the only statement within a suite like what we see in https://github.com/pandas-dev/pandas/blob/389051d090579550f1a1259855f486d7f7e2159e/pandas/tests/generic/test_generic.py#L148

with pytest.raises(ValueError, match=msg):
    if obj1:
        pass

Copy link
Copy Markdown
Contributor Author

@seroperson seroperson Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added, but this intentionally kind of "wrongly" (specifically for this case) results in:

with pytest.raises(ValueError, match=msg):
    pass

Python's if statement calls obj1.__bool__() to evaluate the condition, and pandas overrides __bool__ on DataFrame to raise ValueError. That's what this check asserts (that you can't do if some_data_frame: checks in your code).

With RUF050 auto-fix this test will fail and I'm not sure what we can do about it (I've mentioned this in PR's description initially).

Shortly, in current implementation the situation is so:

  • We strip if Var: pass entirely.
  • Side-effects like if foo(): pass result in foo().
  • But technically if Var: pass is actually if Var.__bool__(): pass, which has side-effect and logically must result in Var.__bool__(). But that's weird.

So we either should consider this limitation intended and expect users to set # noqa: RUF050 where they expect some side-effect from __bool__() (which is quite very edge case to be honest), or maybe make the whole rule unsafe.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's mention this caveat in the documentation. This should be very uncommon and we can always change it if many users are running into this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know when you updated the documentation so that we can merge this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


# Trailing comment deeper than body indentation
if True:
pass
# deeper trailing comment
18 changes: 18 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF050_F401.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Reproduces the scenario from issue #9472:
# F401 removes unused imports leaving empty `if` blocks,
# RUF050 removes those blocks, then F401 cleans up the
# now-unused guard imports on subsequent fix iterations.

import os
import sys

# F401 removes the unused `ExceptionGroup` import, leaving `pass`.
# Then RUF050 removes the empty `if`, and F401 removes `sys`.
if sys.version_info < (3, 11):
from exceptiongroup import ExceptionGroup

# Already-empty block handled in a single pass by RUF050
if sys.version_info < (3, 11):
pass

print(os.getcwd())
31 changes: 31 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF050_RUF047.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
### Errors (both RUF047 and RUF050 converge)

# RUF047 removes the else, then RUF050 removes the remaining if.
if sys.version_info >= (3, 11):
pass
else:
pass

# Same with elif.
if sys.version_info >= (3, 11):
pass
elif sys.version_info >= (3, 10):
pass
else:
pass

# Side-effect in condition: RUF047 removes the else, then RUF050
# replaces the remaining `if` with the condition expression
if foo():
pass
else:
pass


### No errors

# Non-empty if body: neither rule fires.
if sys.version_info >= (3, 11):
bar()
else:
baz()
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.is_rule_enabled(Rule::NeedlessElse) {
ruff::rules::needless_else(checker, if_.into());
}
if checker.is_rule_enabled(Rule::UnnecessaryIf) {
ruff::rules::unnecessary_if(checker, if_);
}
}
Stmt::Assert(
assert_stmt @ ast::StmtAssert {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "047") => rules::ruff::rules::NeedlessElse,
(Ruff, "048") => rules::ruff::rules::MapIntVersionParsing,
(Ruff, "049") => rules::ruff::rules::DataclassEnum,
(Ruff, "050") => rules::ruff::rules::UnnecessaryIf,
(Ruff, "051") => rules::ruff::rules::IfKeyInDictDel,
(Ruff, "052") => rules::ruff::rules::UsedDummyVariable,
(Ruff, "053") => rules::ruff::rules::ClassWithMixedTypeVars,
Expand Down
42 changes: 42 additions & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ mod tests {
#[test_case(Rule::MapIntVersionParsing, Path::new("RUF048.py"))]
#[test_case(Rule::MapIntVersionParsing, Path::new("RUF048_1.py"))]
#[test_case(Rule::DataclassEnum, Path::new("RUF049.py"))]
#[test_case(Rule::UnnecessaryIf, Path::new("RUF050.py"))]
#[test_case(Rule::IfKeyInDictDel, Path::new("RUF051.py"))]
#[test_case(Rule::UsedDummyVariable, Path::new("RUF052_0.py"))]
#[test_case(Rule::UsedDummyVariable, Path::new("RUF052_1.py"))]
Expand Down Expand Up @@ -199,6 +200,47 @@ mod tests {
Ok(())
}

/// Test that RUF047 (needless-else) and RUF050 (unnecessary-if) converge
/// when both are enabled: RUF047 removes the empty `else` first, then
/// RUF050 removes the remaining empty `if` on the next fix iteration.
#[test]
fn unnecessary_if_and_needless_else() -> Result<()> {
use ruff_python_ast::{PySourceType, SourceType};

let path = test_resource_path("fixtures").join("ruff/RUF050_RUF047.py");
let source_type = SourceType::Python(PySourceType::from(&path));
let source_kind = SourceKind::from_path(&path, source_type)?.expect("valid source");
let settings =
settings::LinterSettings::for_rules(vec![Rule::NeedlessElse, Rule::UnnecessaryIf]);

let (diagnostics, transformed) = test_contents(&source_kind, &path, &settings);
assert_diagnostics!(diagnostics);

insta::assert_snapshot!(transformed.source_code());
Ok(())
}

/// Reproduces issue #9472: F401 removes unused imports leaving empty `if`
/// blocks, then RUF050 removes those, then F401 cleans up the now-unused
/// guard imports. Verifies the full chain converges and produces the
/// expected output.
#[test]
fn unnecessary_if_and_unused_import() -> Result<()> {
use ruff_python_ast::{PySourceType, SourceType};

let path = test_resource_path("fixtures").join("ruff/RUF050_F401.py");
let source_type = SourceType::Python(PySourceType::from(&path));
let source_kind = SourceKind::from_path(&path, source_type)?.expect("valid source");
let settings =
settings::LinterSettings::for_rules(vec![Rule::UnusedImport, Rule::UnnecessaryIf]);

let (diagnostics, transformed) = test_contents(&source_kind, &path, &settings);
assert_diagnostics!(diagnostics);

insta::assert_snapshot!(transformed.source_code());
Ok(())
}

#[test]
fn missing_fstring_syntax_backslash_py311() -> Result<()> {
assert_diagnostics_diff!(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ pub(crate) use test_rules::*;
pub(crate) use unmatched_suppression_comment::*;
pub(crate) use unnecessary_assign_before_yield::*;
pub(crate) use unnecessary_cast_to_int::*;
pub(crate) use unnecessary_if::*;
pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
pub(crate) use unnecessary_key_check::*;
pub(crate) use unnecessary_literal_within_deque_call::*;
Expand Down Expand Up @@ -129,6 +130,7 @@ pub(crate) mod test_rules;
mod unmatched_suppression_comment;
mod unnecessary_assign_before_yield;
mod unnecessary_cast_to_int;
mod unnecessary_if;
mod unnecessary_iterable_allocation_for_first_element;
mod unnecessary_key_check;
mod unnecessary_literal_within_deque_call;
Expand Down
Loading
Loading