[`I001`] fix isort check for files with tabs and no indented blocks (#2374)

This is a followup to #2361. The isort check still had an issue in a rather specific case: files with a multiline import, indented with tabs, and not containing any indented blocks.

The root cause is this: [`Stylist`'s indentation detection](ad8693e3de/src/source_code/stylist.rs (L163-L172)) works by finding `Indent` tokens to determine the type of indentation used by a file. This works for indented code blocks (loops/classes/functions/etc) but does not work for multiline values, so falls back to 4 spaces if the file doesn't contain code blocks.

I considered a few possible solutions:

1. Fix `detect_indentation` to avoid tokenizing and instead use some other heuristic to determine indentation. This would have the benefit of working in other places where this is potentially an issue, but would still fail if the file doesn't contain any indentation at all, and would need to fall back to option 2 anyways.
2. Add an option for specifying the default indentation in Ruff's config. I think this would confusing, since it wouldn't affect the detection behavior and only operate as a fallback, has no other current application and would probably end up being overloaded for other things.
3. Relax the isort check by comparing the expected and actual code's lexed tokens. This would require an additional lexing step.
4. Relax the isort check by comparing the expected and actual code modulo whitespace at the start of lines.

This PR does approach 4, which in addition to being the simplest option, has the (expected, although I didn't benchmark) added benefit of improved performance, since the check no longer needs to do two allocations for the two `dedent` calls. I also believe that the check is still correct enough for all practical purposes.
This commit is contained in:
Samuel Cormier-Iijima 2023-01-31 07:18:54 -05:00 committed by GitHub
parent adc134ced0
commit 09d593b124
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 35 additions and 3 deletions

View File

@ -0,0 +1,13 @@
from numpy import (
cos,
int8,
int16,
int32,
int64,
sin,
tan,
uint8,
uint16,
uint32,
uint64,
)

View File

@ -715,6 +715,7 @@ mod tests {
#[test_case(Path::new("preserve_import_star.py"))]
#[test_case(Path::new("preserve_indentation.py"))]
#[test_case(Path::new("preserve_tabs.py"))]
#[test_case(Path::new("preserve_tabs_2.py"))]
#[test_case(Path::new("relative_imports_order.py"))]
#[test_case(Path::new("reorder_within_section.py"))]
#[test_case(Path::new("separate_first_party_imports.py"))]

View File

@ -1,8 +1,9 @@
use std::path::Path;
use itertools::{EitherOrBoth, Itertools};
use ruff_macros::derive_message_formats;
use rustpython_ast::{Location, Stmt};
use textwrap::{dedent, indent};
use textwrap::indent;
use super::super::track::Block;
use super::super::{comments, format_imports};
@ -43,6 +44,17 @@ fn extract_indentation_range(body: &[&Stmt]) -> Range {
Range::new(Location::new(location.row(), 0), location)
}
/// Compares two strings, returning true if they are equal modulo whitespace
/// at the start of each line.
fn matches_ignoring_indentation(val1: &str, val2: &str) -> bool {
val1.lines()
.zip_longest(val2.lines())
.all(|pair| match pair {
EitherOrBoth::Both(line1, line2) => line1.trim_start() == line2.trim_start(),
_ => false,
})
}
/// I001
pub fn organize_imports(
block: &Block,
@ -112,8 +124,8 @@ pub fn organize_imports(
Location::new(range.location.row(), 0),
Location::new(range.end_location.row() + 1 + num_trailing_lines, 0),
);
let actual = dedent(locator.slice_source_code_range(&range));
if actual == dedent(&expected) {
let actual = locator.slice_source_code_range(&range);
if matches_ignoring_indentation(actual, &expected) {
None
} else {
let mut diagnostic = Diagnostic::new(UnsortedImports, range);

View File

@ -0,0 +1,6 @@
---
source: src/rules/isort/mod.rs
expression: diagnostics
---
[]