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

[pylint] Add fix for useless-else-on-loop (PLW0120) #9590

Merged
merged 7 commits into from
Jan 21, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
21 changes: 20 additions & 1 deletion crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@ mod tests {
use rustc_hash::FxHashSet;
use test_case::test_case;

use crate::assert_messages;
use crate::registry::Rule;
use crate::rules::pylint;
use crate::settings::types::PreviewMode;
use crate::settings::types::PythonVersion;
use crate::settings::LinterSettings;
use crate::test::test_path;
use crate::{assert_messages, settings};

#[test_case(Rule::AndOrTernary, Path::new("and_or_ternary.py"))]
#[test_case(Rule::AssertOnStringLiteral, Path::new("assert_on_string_literal.py"))]
Expand Down Expand Up @@ -190,6 +191,24 @@ mod tests {
Ok(())
}

#[test_case(Rule::UselessElseOnLoop, Path::new("useless_else_on_loop.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("pylint").join(path).as_path(),
&settings::LinterSettings {
preview: PreviewMode::Enabled,
..settings::LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test]
fn repeated_isinstance_calls() -> Result<()> {
let diagnostics = test_path(
Expand Down
68 changes: 62 additions & 6 deletions crates/ruff_linter/src/rules/pylint/rules/useless_else_on_loop.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
use ast::whitespace::indentation;
use ruff_python_ast::{self as ast, ExceptHandler, MatchCase, Stmt};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::identifier;
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{Ranged, TextRange};

use crate::rules::pyupgrade::fixes::adjust_indentation;

use crate::checkers::ast::Checker;

Expand Down Expand Up @@ -42,13 +47,18 @@ use crate::checkers::ast::Checker;
pub struct UselessElseOnLoop;

impl Violation for UselessElseOnLoop {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats]
fn message(&self) -> String {
format!(
"`else` clause on loop without a `break` statement; remove the `else` and de-indent all the \
code inside it"
)
}

fn fix_title(&self) -> Option<String> {
Some("Remove redundant `else` clause".to_string())
}
}

fn loop_exits_early(body: &[Stmt]) -> bool {
Expand Down Expand Up @@ -98,10 +108,56 @@ pub(crate) fn useless_else_on_loop(
body: &[Stmt],
orelse: &[Stmt],
) {
if !orelse.is_empty() && !loop_exits_early(body) {
checker.diagnostics.push(Diagnostic::new(
UselessElseOnLoop,
identifier::else_(stmt, checker.locator().contents()).unwrap(),
));
if orelse.is_empty() || loop_exits_early(body) {
return;
}

let else_range = identifier::else_(stmt, checker.locator().contents()).unwrap();

let mut diagnostic = Diagnostic::new(UselessElseOnLoop, else_range);

if checker.settings.preview.is_enabled() {
let start = orelse.first().unwrap();
let end = orelse.last().unwrap();
let start_indentation = indentation(checker.locator(), start);
if start_indentation.is_none() {
// Inline `else` block (e.g., `else: x = 1`).
diagnostic.set_fix(Fix::applicable_edit(
Edit::range_replacement(
String::new(),
TextRange::new(else_range.start(), start.start()),
),
Applicability::Safe,
));
} else {
let desired_indentation = indentation(checker.locator(), stmt).unwrap_or("");
let else_line_range = checker.locator().full_line_range(else_range.start());
let else_colon =
SimpleTokenizer::starts_at(else_range.start(), checker.locator().contents())
.find(|token| token.kind == SimpleTokenKind::Colon)
.unwrap();

let indented = adjust_indentation(
TextRange::new(else_line_range.end(), end.end()),
desired_indentation,
checker.locator(),
checker.stylist(),
)
.unwrap();

diagnostic.set_fix(Fix::applicable_edits(
Edit::deletion(
else_line_range.end(),
checker.locator().full_line_end(end.end()),
),
[Edit::range_replacement(
indented,
TextRange::new(else_line_range.start(), else_colon.end()),
)],
Applicability::Safe,
));
}
}

checker.diagnostics.push(diagnostic);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ useless_else_on_loop.py:9:5: PLW0120 `else` clause on loop without a `break` sta
10 | print("math is broken")
11 | return None
|
= help: Remove redundant `else` clause

useless_else_on_loop.py:18:5: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
Expand All @@ -20,6 +21,7 @@ useless_else_on_loop.py:18:5: PLW0120 `else` clause on loop without a `break` st
19 | print("math is broken")
20 | return None
|
= help: Remove redundant `else` clause

useless_else_on_loop.py:30:1: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
Expand All @@ -29,6 +31,7 @@ useless_else_on_loop.py:30:1: PLW0120 `else` clause on loop without a `break` st
| ^^^^ PLW0120
31 | print("or else!")
|
= help: Remove redundant `else` clause

useless_else_on_loop.py:37:1: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
Expand All @@ -38,6 +41,7 @@ useless_else_on_loop.py:37:1: PLW0120 `else` clause on loop without a `break` st
| ^^^^ PLW0120
38 | print("or else!")
|
= help: Remove redundant `else` clause

useless_else_on_loop.py:42:1: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
Expand All @@ -48,6 +52,7 @@ useless_else_on_loop.py:42:1: PLW0120 `else` clause on loop without a `break` st
43 | print("fat chance")
44 | for j in range(10):
|
= help: Remove redundant `else` clause

useless_else_on_loop.py:88:5: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
Expand All @@ -58,6 +63,7 @@ useless_else_on_loop.py:88:5: PLW0120 `else` clause on loop without a `break` st
89 | return True
90 | return False
|
= help: Remove redundant `else` clause

useless_else_on_loop.py:98:9: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
Expand All @@ -68,5 +74,6 @@ useless_else_on_loop.py:98:9: PLW0120 `else` clause on loop without a `break` st
99 | if 1 < 2: # pylint: disable=comparison-of-constants
100 | break
|
= help: Remove redundant `else` clause


Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
useless_else_on_loop.py:9:5: PLW0120 [*] `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
7 | if i % 2:
8 | return i
9 | else: # [useless-else-on-loop]
| ^^^^ PLW0120
10 | print("math is broken")
11 | return None
|
= help: Remove redundant `else` clause

ℹ Safe fix
6 6 | for i in range(10):
7 7 | if i % 2:
8 8 | return i
9 |- else: # [useless-else-on-loop]
10 |- print("math is broken")
9 |+ print("math is broken") # [useless-else-on-loop]
11 10 | return None
12 11 |
13 12 |

useless_else_on_loop.py:18:5: PLW0120 [*] `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
16 | while True:
17 | return 1
18 | else: # [useless-else-on-loop]
| ^^^^ PLW0120
19 | print("math is broken")
20 | return None
|
= help: Remove redundant `else` clause

ℹ Safe fix
15 15 | """else + return is not acceptable."""
16 16 | while True:
17 17 | return 1
18 |- else: # [useless-else-on-loop]
19 |- print("math is broken")
18 |+ print("math is broken") # [useless-else-on-loop]
20 19 | return None
21 20 |
22 21 |

useless_else_on_loop.py:30:1: PLW0120 [*] `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
28 | break
29 |
30 | else: # [useless-else-on-loop]
| ^^^^ PLW0120
31 | print("or else!")
|
= help: Remove redundant `else` clause

ℹ Safe fix
27 27 | for _ in range(10):
28 28 | break
29 29 |
30 |-else: # [useless-else-on-loop]
31 |- print("or else!")
30 |+print("or else!") # [useless-else-on-loop]
32 31 |
33 32 |
34 33 | while True:

useless_else_on_loop.py:37:1: PLW0120 [*] `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
35 | while False:
36 | break
37 | else: # [useless-else-on-loop]
| ^^^^ PLW0120
38 | print("or else!")
|
= help: Remove redundant `else` clause

ℹ Safe fix
34 34 | while True:
35 35 | while False:
36 36 | break
37 |-else: # [useless-else-on-loop]
38 |- print("or else!")
37 |+print("or else!") # [useless-else-on-loop]
39 38 |
40 39 | for j in range(10):
41 40 | pass

useless_else_on_loop.py:42:1: PLW0120 [*] `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
40 | for j in range(10):
41 | pass
42 | else: # [useless-else-on-loop]
| ^^^^ PLW0120
43 | print("fat chance")
44 | for j in range(10):
|
= help: Remove redundant `else` clause

ℹ Safe fix
39 39 |
40 40 | for j in range(10):
41 41 | pass
42 |-else: # [useless-else-on-loop]
43 |- print("fat chance")
44 |- for j in range(10):
45 |- break
42 |+print("fat chance")
43 |+for j in range(10):
44 |+ break # [useless-else-on-loop]
46 45 |
47 46 |
48 47 | def test_return_for2():

useless_else_on_loop.py:88:5: PLW0120 [*] `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
86 | else:
87 | print("all right")
88 | else: # [useless-else-on-loop]
| ^^^^ PLW0120
89 | return True
90 | return False
|
= help: Remove redundant `else` clause

ℹ Safe fix
85 85 | break
86 86 | else:
87 87 | print("all right")
88 |- else: # [useless-else-on-loop]
89 |- return True
88 |+ return True # [useless-else-on-loop]
90 89 | return False
91 90 |
92 91 |

useless_else_on_loop.py:98:9: PLW0120 [*] `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
|
96 | for _ in range(3):
97 | pass
98 | else:
| ^^^^ PLW0120
99 | if 1 < 2: # pylint: disable=comparison-of-constants
100 | break
|
= help: Remove redundant `else` clause

ℹ Safe fix
95 95 | for _ in range(10):
96 96 | for _ in range(3):
97 97 | pass
98 |- else:
99 |- if 1 < 2: # pylint: disable=comparison-of-constants
100 |- break
98 |+ if 1 < 2: # pylint: disable=comparison-of-constants
99 |+ break
101 100 | else:
102 101 | return True
103 102 | return False


2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pyupgrade/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Rules from [pyupgrade](https://pypi.org/project/pyupgrade/).
mod fixes;
pub(crate) mod fixes;
mod helpers;
pub(crate) mod rules;
pub mod settings;
Expand Down