Skip to content

Commit

Permalink
Avoid collapsing elif and else branches during import sorting (#5964
Browse files Browse the repository at this point in the history
)

## Summary

I ran into this in the wild. It looks like Ruff will collapse the `else`
and `elif` branches here (i.e., it doesn't recognize that they're too
independent import blocks):

```python
if "sdist" in cmds:
    _sdist = cmds["sdist"]
elif "setuptools" in sys.modules:
    from setuptools.command.sdist import sdist as _sdist
else:
    from setuptools.command.sdist import sdist as _sdist
    from distutils.command.sdist import sdist as _sdist
```

Likely fallout from the `elif_else_branches` refactor.
  • Loading branch information
charliermarsh authored Jul 22, 2023
1 parent aaf7f36 commit 94a004e
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 2 deletions.
7 changes: 7 additions & 0 deletions crates/ruff/resources/test/fixtures/isort/if_elif_else.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
if "sdist" in cmds:
_sdist = cmds["sdist"]
elif "setuptools" in sys.modules:
from setuptools.command.sdist import sdist as _sdist
else:
from setuptools.command.sdist import sdist as _sdist
from distutils.command.sdist import sdist as _sdist
7 changes: 7 additions & 0 deletions crates/ruff/resources/test/fixtures/isort/match_case.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
match 1:
case 1:
import sys
import os
case 2:
import collections
import abc
10 changes: 8 additions & 2 deletions crates/ruff/src/rules/isort/block.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use ruff_text_size::{TextRange, TextSize};
use rustpython_parser::ast::{self, ExceptHandler, MatchCase, Ranged, Stmt};
use rustpython_parser::ast::{self, ElifElseClause, ExceptHandler, MatchCase, Ranged, Stmt};
use std::iter::Peekable;
use std::slice;

Expand Down Expand Up @@ -254,7 +254,6 @@ where
for clause in elif_else_clauses {
self.visit_elif_else_clause(clause);
}
self.finalize(None);
}
Stmt::With(ast::StmtWith { body, .. }) => {
for stmt in body {
Expand Down Expand Up @@ -331,4 +330,11 @@ where
}
self.finalize(None);
}

fn visit_elif_else_clause(&mut self, elif_else_clause: &'b ElifElseClause) {
for stmt in &elif_else_clause.body {
self.visit_stmt(stmt);
}
self.finalize(None);
}
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/isort/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,15 @@ mod tests {
#[test_case(Path::new("force_sort_within_sections.py"))]
#[test_case(Path::new("force_to_top.py"))]
#[test_case(Path::new("force_wrap_aliases.py"))]
#[test_case(Path::new("if_elif_else.py"))]
#[test_case(Path::new("import_from_after_import.py"))]
#[test_case(Path::new("inline_comments.py"))]
#[test_case(Path::new("insert_empty_lines.py"))]
#[test_case(Path::new("insert_empty_lines.pyi"))]
#[test_case(Path::new("isort_skip_file.py"))]
#[test_case(Path::new("leading_prefix.py"))]
#[test_case(Path::new("magic_trailing_comma.py"))]
#[test_case(Path::new("match_case.py"))]
#[test_case(Path::new("natural_order.py"))]
#[test_case(Path::new("no_lines_before.py"))]
#[test_case(Path::new("no_reorder_within_section.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: crates/ruff/src/rules/isort/mod.rs
---
if_elif_else.py:6:1: I001 [*] Import block is un-sorted or un-formatted
|
4 | from setuptools.command.sdist import sdist as _sdist
5 | else:
6 | / from setuptools.command.sdist import sdist as _sdist
7 | | from distutils.command.sdist import sdist as _sdist
|
= help: Organize imports

Fix
3 3 | elif "setuptools" in sys.modules:
4 4 | from setuptools.command.sdist import sdist as _sdist
5 5 | else:
6 |- from setuptools.command.sdist import sdist as _sdist
7 6 | from distutils.command.sdist import sdist as _sdist
7 |+
8 |+ from setuptools.command.sdist import sdist as _sdist


Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
source: crates/ruff/src/rules/isort/mod.rs
---
match_case.py:3:1: I001 [*] Import block is un-sorted or un-formatted
|
1 | match 1:
2 | case 1:
3 | / import sys
4 | | import os
5 | | case 2:
| |_^ I001
6 | import collections
7 | import abc
|
= help: Organize imports

Fix
1 1 | match 1:
2 2 | case 1:
3 |+ import os
3 4 | import sys
4 |- import os
5 5 | case 2:
6 6 | import collections
7 7 | import abc

match_case.py:6:1: I001 [*] Import block is un-sorted or un-formatted
|
4 | import os
5 | case 2:
6 | / import collections
7 | | import abc
|
= help: Organize imports

Fix
3 3 | import sys
4 4 | import os
5 5 | case 2:
6 |+ import abc
6 7 | import collections
7 |- import abc


0 comments on commit 94a004e

Please sign in to comment.