Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor isort directive skips to use iterators #5623

Merged
merged 1 commit into from
Jul 8, 2023
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
6 changes: 6 additions & 0 deletions crates/ruff/resources/test/fixtures/isort/skip.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,9 @@ def f():
import os # isort:skip
import collections
import abc


def f():
import sys; import os # isort:skip
import sys; import os # isort:skip # isort:skip
import sys; import os
10 changes: 10 additions & 0 deletions crates/ruff/resources/test/fixtures/isort/split.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,13 @@

import D
import B


import e
import f

# isort: split
# isort: split

import d
import c
76 changes: 46 additions & 30 deletions crates/ruff/src/rules/isort/block.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use ruff_text_size::{TextRange, TextSize};
use rustpython_parser::ast::{self, ExceptHandler, MatchCase, Ranged, Stmt};
use std::iter::Peekable;
use std::slice;

use ruff_python_ast::source_code::Locator;
use ruff_python_ast::statement_visitor::StatementVisitor;
Expand Down Expand Up @@ -30,8 +32,8 @@ pub(crate) struct BlockBuilder<'a> {
locator: &'a Locator<'a>,
is_stub: bool,
blocks: Vec<Block<'a>>,
splits: &'a [TextSize],
cell_offsets: Option<&'a [TextSize]>,
splits: Peekable<slice::Iter<'a, TextSize>>,
cell_offsets: Option<Peekable<slice::Iter<'a, TextSize>>>,
exclusions: &'a [TextRange],
nested: bool,
}
Expand All @@ -47,12 +49,13 @@ impl<'a> BlockBuilder<'a> {
locator,
is_stub,
blocks: vec![Block::default()],
splits: &directives.splits,
splits: directives.splits.iter().peekable(),
exclusions: &directives.exclusions,
nested: false,
cell_offsets: source_kind
.and_then(SourceKind::notebook)
.map(Notebook::cell_offsets),
.map(Notebook::cell_offsets)
.map(|offsets| offsets.iter().peekable()),
}
}

Expand Down Expand Up @@ -126,45 +129,58 @@ where
'b: 'a,
{
fn visit_stmt(&mut self, stmt: &'b Stmt) {
// Track manual splits.
for (index, split) in self.splits.iter().enumerate() {
if stmt.start() >= *split {
self.finalize(self.trailer_for(stmt));
self.splits = &self.splits[index + 1..];
} else {
break;
// Track manual splits (e.g., `# isort: split`).
if self
.splits
.next_if(|split| stmt.start() >= **split)
.is_some()
{
// Skip any other splits that occur before the current statement, to support, e.g.:
// ```python
// # isort: split
// # isort: split
// import foo
// ```
while self
.splits
.peek()
.map_or(false, |split| stmt.start() >= **split)
{
self.splits.next();
}

self.finalize(self.trailer_for(stmt));
}

// Track Jupyter notebook cell offsets as splits. This will make sure
// that each cell is considered as an individual block to organize the
// imports in. Thus, not creating an edit which spans across multiple
// cells.
if let Some(cell_offsets) = self.cell_offsets {
for (index, split) in cell_offsets.iter().enumerate() {
if stmt.start() >= *split {
// We don't want any extra newlines between cells.
self.finalize(None);
self.cell_offsets = Some(&cell_offsets[index + 1..]);
} else {
break;
if let Some(cell_offsets) = self.cell_offsets.as_mut() {
if cell_offsets
.next_if(|cell_offset| stmt.start() >= **cell_offset)
.is_some()
{
// Skip any other cell offsets that occur before the current statement (e.g., in
// the case of multiple empty cells).
while cell_offsets
.peek()
.map_or(false, |split| stmt.start() >= **split)
{
cell_offsets.next();
}
}
}

// Test if the statement is in an excluded range
let mut is_excluded = false;
for (index, exclusion) in self.exclusions.iter().enumerate() {
if exclusion.end() < stmt.start() {
self.exclusions = &self.exclusions[index + 1..];
} else {
is_excluded = exclusion.contains(stmt.start());
break;
self.finalize(None);
}
}

// Track imports.
if matches!(stmt, Stmt::Import(_) | Stmt::ImportFrom(_)) && !is_excluded {
if matches!(stmt, Stmt::Import(_) | Stmt::ImportFrom(_))
&& !self
.exclusions
.iter()
.any(|exclusion| exclusion.contains(stmt.start()))
{
self.track_import(stmt);
} else {
self.finalize(self.trailer_for(stmt));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ skip.py:27:1: I001 [*] Import block is un-sorted or un-formatted
26 | import os # isort:skip
27 | / import collections
28 | | import abc
29 | |
| |_^ I001
30 |
31 | def f():
|
= help: Organize imports

Expand All @@ -41,5 +45,24 @@ skip.py:27:1: I001 [*] Import block is un-sorted or un-formatted
27 |+ import abc
27 28 | import collections
28 |- import abc
29 29 |
30 30 |
31 31 | def f():

skip.py:34:1: I001 [*] Import block is un-sorted or un-formatted
|
32 | import sys; import os # isort:skip
33 | import sys; import os # isort:skip # isort:skip
34 | / import sys; import os
|
= help: Organize imports

ℹ Fix
31 31 | def f():
32 32 | import sys; import os # isort:skip
33 33 | import sys; import os # isort:skip # isort:skip
34 |- import sys; import os
34 |+ import os
35 |+ import sys


Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ split.py:20:1: I001 [*] Import block is un-sorted or un-formatted
19 |
20 | / import D
21 | | import B
22 | |
| |_^ I001
23 |
24 | import e
|
= help: Organize imports

Expand All @@ -39,5 +43,25 @@ split.py:20:1: I001 [*] Import block is un-sorted or un-formatted
20 |+ import B
20 21 | import D
21 |- import B
22 22 |
23 23 |
24 24 | import e

split.py:30:1: I001 [*] Import block is un-sorted or un-formatted
|
28 | # isort: split
29 |
30 | / import d
31 | | import c
|
= help: Organize imports

ℹ Fix
27 27 | # isort: split
28 28 | # isort: split
29 29 |
30 |+import c
30 31 | import d
31 |-import c