Skip to content

Commit

Permalink
Avoid inserting imports directly after continuation (#7553)
Browse files Browse the repository at this point in the history
## Summary

This is extremely rare in practice, but common in the fuzzer issues so
worth fixing quickly.

Closes #7199.
  • Loading branch information
charliermarsh committed Sep 20, 2023
1 parent ee9ee00 commit a0917ec
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import os \

exit(0)
24 changes: 20 additions & 4 deletions crates/ruff_linter/src/importer/insertion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<'a> Insertion<'a> {
let mut location = if let Some(location) = match_docstring_end(body) {
// If the first token after the docstring is a semicolon, insert after the semicolon as
// an inline statement.
if let Some(offset) = match_leading_semicolon(locator.after(location)) {
if let Some(offset) = match_semicolon(locator.after(location)) {
return Insertion::inline(" ", location.add(offset).add(TextSize::of(';')), ";");
}

Expand Down Expand Up @@ -109,10 +109,14 @@ impl<'a> Insertion<'a> {
stylist: &Stylist,
) -> Insertion<'static> {
let location = stmt.end();
if let Some(offset) = match_leading_semicolon(locator.after(location)) {
if let Some(offset) = match_semicolon(locator.after(location)) {
// If the first token after the statement is a semicolon, insert after the semicolon as
// an inline statement.
Insertion::inline(" ", location.add(offset).add(TextSize::of(';')), ";")
} else if match_continuation(locator.after(location)).is_some() {
// If the first token after the statement is a continuation, insert after the statement
// with a semicolon.
Insertion::inline("; ", location, "")
} else {
// Otherwise, insert on the next line.
Insertion::own_line(
Expand Down Expand Up @@ -289,8 +293,8 @@ fn match_docstring_end(body: &[Stmt]) -> Option<TextSize> {
Some(stmt.end())
}

/// If a line starts with a semicolon, return its offset.
fn match_leading_semicolon(s: &str) -> Option<TextSize> {
/// If the next token is a semicolon, return its offset.
fn match_semicolon(s: &str) -> Option<TextSize> {
for (offset, c) in s.char_indices() {
match c {
' ' | '\t' => continue,
Expand All @@ -301,6 +305,18 @@ fn match_leading_semicolon(s: &str) -> Option<TextSize> {
None
}

/// If the next token is a continuation (`\`), return its offset.
fn match_continuation(s: &str) -> Option<TextSize> {
for (offset, c) in s.char_indices() {
match c {
' ' | '\t' => continue,
'\\' => return Some(TextSize::try_from(offset).unwrap()),
_ => break,
}
}
None
}

#[cfg(test)]
mod tests {
use anyhow::Result;
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ mod tests {
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_9.py"))]
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_10.py"))]
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_11.py"))]
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_12.py"))]
#[test_case(Rule::ContinueInFinally, Path::new("continue_in_finally.py"))]
#[test_case(Rule::GlobalStatement, Path::new("global_statement.py"))]
#[test_case(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
sys_exit_alias_12.py:3:1: PLR1722 [*] Use `sys.exit()` instead of `exit`
|
1 | import os \
2 |
3 | exit(0)
| ^^^^ PLR1722
|
= help: Replace `exit` with `sys.exit()`

Suggested fix
1 |-import os \
1 |+import os; import sys \
2 2 |
3 |-exit(0)
3 |+sys.exit(0)


0 comments on commit a0917ec

Please sign in to comment.