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

[pycodestyle] Add fix for multiple-imports-on-one-line (E401) #9518

Merged
merged 8 commits into from
Jan 21, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E40.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
#: E401
import os, sys
import re as regex, string # also with a comment!
import re as regex, string; x = 1

x = 1; import re as regex, string


def blah():
import datetime as dt, copy
def nested_and_tested():
import builtins, textwrap as tw

#: Okay
import os
import sys
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pycodestyle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ mod tests {
}

#[test_case(Rule::IsLiteral, Path::new("constant_literals.py"))]
#[test_case(Rule::MultipleImportsOnOneLine, Path::new("E40.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_0.py"))]
#[test_case(Rule::TypeComparison, Path::new("E721.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
Expand Down
46 changes: 42 additions & 4 deletions crates/ruff_linter/src/rules/pycodestyle/rules/imports.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Alias, PySourceType, Stmt};
use ruff_python_trivia::{indentation_at_offset, PythonWhitespace};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand All @@ -27,10 +28,16 @@ use crate::checkers::ast::Checker;
pub struct MultipleImportsOnOneLine;

impl Violation for MultipleImportsOnOneLine {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
format!("Multiple imports on one line")
}

fn fix_title(&self) -> Option<String> {
Some(format!("Split imports onto multiple lines"))
}
}

/// ## What it does
Expand Down Expand Up @@ -85,9 +92,40 @@ impl Violation for ModuleImportNotAtTopOfFile {
/// E401
pub(crate) fn multiple_imports_on_one_line(checker: &mut Checker, stmt: &Stmt, names: &[Alias]) {
if names.len() > 1 {
checker
.diagnostics
.push(Diagnostic::new(MultipleImportsOnOneLine, stmt.range()));
let mut diagnostic = Diagnostic::new(MultipleImportsOnOneLine, stmt.range());

if checker.settings.preview.is_enabled() {
let indentation = indentation_at_offset(stmt.start(), checker.locator()).unwrap_or("");

let mut replacement = String::new();

for item in names {
let Alias {
range: _,
name,
asname,
} = item;

if let Some(asname) = asname {
replacement = format!("{replacement}{indentation}import {name} as {asname}\n");
} else {
replacement = format!("{replacement}{indentation}import {name}\n");
}
}

// remove leading whitespace because we start at the import keyword
replacement = replacement.trim_whitespace_start().to_string();

// remove trailing newline
replacement = replacement.trim_end_matches('\n').to_string();

diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
replacement,
stmt.range(),
)));
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you add test cases for, e.g.:

x = 1; import re as regex, string

import re as regex, string; x = 1


checker.diagnostics.push(diagnostic);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,60 @@ E40.py:2:1: E401 Multiple imports on one line
1 | #: E401
2 | import os, sys
| ^^^^^^^^^^^^^^ E401
3 | #: Okay
4 | import os
3 | import re as regex, string # also with a comment!
4 | import re as regex, string; x = 1
|
= help: Split imports onto multiple lines

E40.py:3:1: E401 Multiple imports on one line
|
1 | #: E401
2 | import os, sys
3 | import re as regex, string # also with a comment!
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ E401
4 | import re as regex, string; x = 1
|
= help: Split imports onto multiple lines

E40.py:4:1: E401 Multiple imports on one line
|
2 | import os, sys
3 | import re as regex, string # also with a comment!
4 | import re as regex, string; x = 1
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ E401
5 |
6 | x = 1; import re as regex, string
|
= help: Split imports onto multiple lines

E40.py:6:8: E401 Multiple imports on one line
|
4 | import re as regex, string; x = 1
5 |
6 | x = 1; import re as regex, string
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ E401
|
= help: Split imports onto multiple lines

E40.py:10:5: E401 Multiple imports on one line
|
9 | def blah():
10 | import datetime as dt, copy
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ E401
11 | def nested_and_tested():
12 | import builtins, textwrap as tw
|
= help: Split imports onto multiple lines

E40.py:12:9: E401 Multiple imports on one line
|
10 | import datetime as dt, copy
11 | def nested_and_tested():
12 | import builtins, textwrap as tw
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E401
13 |
14 | #: Okay
|
= help: Split imports onto multiple lines


Original file line number Diff line number Diff line change
@@ -1,31 +1,164 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E40.py:55:1: E402 Module level import not at top of file
E40.py:6:8: E402 Module level import not at top of file
|
4 | import re as regex, string; x = 1
5 |
6 | x = 1; import re as regex, string
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ E402
|

E40.py:15:1: E402 Module level import not at top of file
|
14 | #: Okay
15 | import os
| ^^^^^^^^^ E402
16 | import sys
|

E40.py:16:1: E402 Module level import not at top of file
|
14 | #: Okay
15 | import os
16 | import sys
| ^^^^^^^^^^ E402
17 |
18 | from subprocess import Popen, PIPE
|

E40.py:18:1: E402 Module level import not at top of file
|
16 | import sys
17 |
18 | from subprocess import Popen, PIPE
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E402
19 |
20 | from myclass import MyClass
|

E40.py:20:1: E402 Module level import not at top of file
|
18 | from subprocess import Popen, PIPE
19 |
20 | from myclass import MyClass
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ E402
21 | from foo.bar.yourclass import YourClass
|

E40.py:21:1: E402 Module level import not at top of file
|
20 | from myclass import MyClass
21 | from foo.bar.yourclass import YourClass
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E402
22 |
23 | import myclass
|

E40.py:23:1: E402 Module level import not at top of file
|
21 | from foo.bar.yourclass import YourClass
22 |
23 | import myclass
| ^^^^^^^^^^^^^^ E402
24 | import foo.bar.yourclass
25 | #: Okay
|

E40.py:24:1: E402 Module level import not at top of file
|
23 | import myclass
24 | import foo.bar.yourclass
| ^^^^^^^^^^^^^^^^^^^^^^^^ E402
25 | #: Okay
26 | __all__ = ['abc']
|

E40.py:28:1: E402 Module level import not at top of file
|
26 | __all__ = ['abc']
27 |
28 | import foo
| ^^^^^^^^^^ E402
29 | #: Okay
30 | __version__ = "42"
|

E40.py:32:1: E402 Module level import not at top of file
|
30 | __version__ = "42"
31 |
32 | import foo
| ^^^^^^^^^^ E402
33 | #: Okay
34 | __author__ = "Simon Gomizelj"
|

E40.py:36:1: E402 Module level import not at top of file
|
34 | __author__ = "Simon Gomizelj"
35 |
36 | import foo
| ^^^^^^^^^^ E402
37 | #: Okay
38 | try:
|

E40.py:47:1: E402 Module level import not at top of file
|
45 | print('made attempt to import foo')
46 |
47 | import bar
| ^^^^^^^^^^ E402
48 | #: Okay
49 | with warnings.catch_warnings():
|

E40.py:53:1: E402 Module level import not at top of file
|
51 | import foo
52 |
53 | import bar
| ^^^^^^^^^^ E402
54 | #: Okay
55 | if False:
|

E40.py:62:1: E402 Module level import not at top of file
|
60 | import mwahaha
61 |
62 | import bar
| ^^^^^^^^^^ E402
63 | #: E402
64 | VERSION = '1.2.3'
|

E40.py:66:1: E402 Module level import not at top of file
|
53 | VERSION = '1.2.3'
54 |
55 | import foo
64 | VERSION = '1.2.3'
65 |
66 | import foo
| ^^^^^^^^^^ E402
56 | #: E402
57 | import foo
67 | #: E402
68 | import foo
|

E40.py:57:1: E402 Module level import not at top of file
E40.py:68:1: E402 Module level import not at top of file
|
55 | import foo
56 | #: E402
57 | import foo
66 | import foo
67 | #: E402
68 | import foo
| ^^^^^^^^^^ E402
58 |
59 | a = 1
69 |
70 | a = 1
|

E40.py:61:1: E402 Module level import not at top of file
E40.py:72:1: E402 Module level import not at top of file
|
59 | a = 1
60 |
61 | import bar
70 | a = 1
71 |
72 | import bar
| ^^^^^^^^^^ E402
|

Expand Down