Skip to content

Commit

Permalink
[pylint] - add fix for PLR5501
Browse files Browse the repository at this point in the history
  • Loading branch information
diceroll123 committed Jan 20, 2024
1 parent 866bea6 commit b312212
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 27 deletions.
12 changes: 2 additions & 10 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1069,13 +1069,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
ruff::rules::sort_dunder_all_aug_assign(checker, aug_assign);
}
}
Stmt::If(
if_ @ ast::StmtIf {
test,
elif_else_clauses,
..
},
) => {
Stmt::If(if_ @ ast::StmtIf { test, .. }) => {
if checker.enabled(Rule::EmptyTypeCheckingBlock) {
if typing::is_type_checking_block(if_, &checker.semantic) {
flake8_type_checking::rules::empty_type_checking_block(checker, if_);
Expand Down Expand Up @@ -1117,9 +1111,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pyupgrade::rules::outdated_version_block(checker, if_);
}
if checker.enabled(Rule::CollapsibleElseIf) {
if let Some(diagnostic) = pylint::rules::collapsible_else_if(elif_else_clauses) {
checker.diagnostics.push(diagnostic);
}
pylint::rules::collapsible_else_if(checker, stmt);
}
if checker.enabled(Rule::CheckAndRemoveFromSet) {
refurb::rules::check_and_remove_from_set(checker, if_);
Expand Down
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::CollapsibleElseIf, Path::new("collapsible_else_if.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
82 changes: 67 additions & 15 deletions crates/ruff_linter/src/rules/pylint/rules/collapsible_else_if.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
use ruff_python_ast::{ElifElseClause, Stmt};
use ast::whitespace::indentation;
use ruff_python_ast::{self as ast, ElifElseClause, Stmt};
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{Ranged, TextRange};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;
use crate::rules::pyupgrade::fixes::adjust_indentation;

/// ## What it does
/// Checks for `else` blocks that consist of a single `if` statement.
///
Expand Down Expand Up @@ -40,27 +45,74 @@ use ruff_macros::{derive_message_formats, violation};
pub struct CollapsibleElseIf;

impl Violation for CollapsibleElseIf {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats]
fn message(&self) -> String {
format!("Use `elif` instead of `else` then `if`, to reduce indentation")
}

fn fix_title(&self) -> Option<String> {
Some("Use `elif` instead of `else` then `if`, to reduce indentation".to_string())
}
}

/// PLR5501
pub(crate) fn collapsible_else_if(elif_else_clauses: &[ElifElseClause]) -> Option<Diagnostic> {
let Some(ElifElseClause {
body,
test: None,
range,
}) = elif_else_clauses.last()
pub(crate) fn collapsible_else_if(checker: &mut Checker, stmt: &Stmt) {
let Stmt::If(ast::StmtIf {
elif_else_clauses, ..
}) = stmt
else {
return;
};

let Some(
else_clause @ ElifElseClause {
body,
test: None,
range,
},
) = elif_else_clauses.last()
else {
return None;
return;
};
if let [first @ Stmt::If(_)] = body.as_slice() {
return Some(Diagnostic::new(
CollapsibleElseIf,
TextRange::new(range.start(), first.start()),
));
let [first @ Stmt::If(ast::StmtIf { .. })] = body.as_slice() else {
return;
};

let mut diagnostic = Diagnostic::new(
CollapsibleElseIf,
TextRange::new(range.start(), first.start()),
);

if checker.settings.preview.is_enabled() {
let inner_if_line_start = checker.locator().line_start(first.start());

let desired_indentation = indentation(checker.locator(), else_clause).unwrap_or("");

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

let fixed_indented = format!("el{}", indented.strip_prefix(desired_indentation).unwrap());

let else_colon =
SimpleTokenizer::starts_at(else_clause.start(), checker.locator().contents())
.find(|token| token.kind() == SimpleTokenKind::Colon)
.unwrap();

diagnostic.set_fix(Fix::applicable_edits(
Edit::deletion(inner_if_line_start, first.end()),
[Edit::range_replacement(
fixed_indented,
TextRange::new(else_clause.start(), else_colon.end()),
)],
Applicability::Safe,
))
}
None

checker.diagnostics.push(diagnostic);
}
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

0 comments on commit b312212

Please sign in to comment.