Implement pylint's else-if-used rule (PLR5501) (#3231)

Attempt to implement else-if-used
https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/else-if-used.html

Issue #970

crates/ruff/resources/test/fixtures/pylint/collapsible_else_if.py

@@ -0,0 +1,49 @@
+"""
+Test for else-if-used
+"""
+
+
+def ok0():
+    """Should not trigger on elif"""
+    if 1:
+        pass
+    elif 2:
+        pass
+
+
+def ok1():
+    """If the orelse has more than 1 item in it, shouldn't trigger"""
+    if 1:
+        pass
+    else:
+        print()
+        if 1:
+            pass
+
+
+def ok2():
+    """If the orelse has more than 1 item in it, shouldn't trigger"""
+    if 1:
+        pass
+    else:
+        if 1:
+            pass
+        print()
+
+
+def not_ok0():
+    if 1:
+        pass
+    else:
+        if 2:
+            pass
+
+
+def not_ok1():
+    if 1:
+        pass
+    else:
+        if 2:
+            pass
+        else:
+            pass

crates/ruff/src/checkers/ast.rs

@@ -1633,6 +1633,13 @@ where
                 if self.settings.rules.enabled(&Rule::OutdatedVersionBlock) {
                     pyupgrade::rules::outdated_version_block(self, stmt, test, body, orelse);
                 }
+                if self.settings.rules.enabled(&Rule::CollapsibleElseIf) {
+                    if let Some(diagnostic) =
+                        pylint::rules::collapsible_else_if(orelse, self.locator)
+                    {
+                        self.diagnostics.push(diagnostic);
+                    }
+                }
             }
             StmtKind::Assert { test, msg } => {
                 if self.settings.rules.enabled(&Rule::AssertTuple) {

crates/ruff/src/codes.rs

@@ -145,6 +145,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Pylint, "R0133") => Rule::ComparisonOfConstant,
         (Pylint, "R1701") => Rule::ConsiderMergingIsinstance,
         (Pylint, "R1722") => Rule::ConsiderUsingSysExit,
+        (Pylint, "R5501") => Rule::CollapsibleElseIf,
         (Pylint, "R2004") => Rule::MagicValueComparison,
         (Pylint, "W0120") => Rule::UselessElseOnLoop,
         (Pylint, "W0602") => Rule::GlobalVariableNotAssigned,

crates/ruff/src/registry.rs

@@ -135,6 +135,7 @@ ruff_macros::register_rules!(
     rules::pylint::rules::BadStringFormatType,
     rules::pylint::rules::BidirectionalUnicode,
     rules::pylint::rules::BadStrStripCall,
+    rules::pylint::rules::CollapsibleElseIf,
     rules::pylint::rules::UselessImportAlias,
     rules::pylint::rules::UnnecessaryDirectLambdaCall,
     rules::pylint::rules::NonlocalWithoutBinding,

crates/ruff/src/rules/pylint/mod.rs

@@ -45,6 +45,7 @@ mod tests {
     #[test_case(Rule::TooManyArguments, Path::new("too_many_arguments.py"); "PLR0913")]
     #[test_case(Rule::TooManyBranches, Path::new("too_many_branches.py"); "PLR0912")]
     #[test_case(Rule::TooManyStatements, Path::new("too_many_statements.py"); "PLR0915")]
+    #[test_case(Rule::CollapsibleElseIf, Path::new("collapsible_else_if.py"); "PLR5501")]
     #[test_case(Rule::BadStringFormatType, Path::new("bad_string_format_type.py"); "PLE1307")]
     #[test_case(Rule::BidirectionalUnicode, Path::new("bidirectional_unicode.py"); "PLE2502")]
     #[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"); "PLE01310")]

crates/ruff/src/rules/pylint/rules/collapsible_else_if.rs

@@ -0,0 +1,42 @@
+use rustpython_parser::ast::{Stmt, StmtKind};
+
+use ruff_macros::{define_violation, derive_message_formats};
+
+use crate::ast::types::Range;
+use crate::registry::Diagnostic;
+use crate::source_code::Locator;
+use crate::violation::Violation;
+
+define_violation!(
+    pub struct CollapsibleElseIf;
+);
+
+impl Violation for CollapsibleElseIf {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Consider using `elif` instead of `else` then `if` to remove one indentation level")
+    }
+}
+
+/// PLR5501
+pub fn collapsible_else_if(orelse: &[Stmt], locator: &Locator) -> Option<Diagnostic> {
+    if orelse.len() == 1 {
+        let first = &orelse[0];
+        if matches!(first.node, StmtKind::If { .. }) {
+            // Determine whether this is an `elif`, or an `if` in an `else` block.
+            if locator
+                .slice(&Range {
+                    location: first.location,
+                    end_location: first.end_location.unwrap(),
+                })
+                .starts_with("if")
+            {
+                return Some(Diagnostic::new(
+                    CollapsibleElseIf,
+                    Range::from_located(first),
+                ));
+            }
+        }
+    }
+    None
+}

crates/ruff/src/rules/pylint/rules/mod.rs

@@ -2,6 +2,7 @@ pub use await_outside_async::{await_outside_async, AwaitOutsideAsync};
 pub use bad_str_strip_call::{bad_str_strip_call, BadStrStripCall};
 pub use bad_string_format_type::{bad_string_format_type, BadStringFormatType};
 pub use bidirectional_unicode::{bidirectional_unicode, BidirectionalUnicode};
+pub use collapsible_else_if::{collapsible_else_if, CollapsibleElseIf};
 pub use comparison_of_constant::{comparison_of_constant, ComparisonOfConstant};
 pub use consider_using_sys_exit::{consider_using_sys_exit, ConsiderUsingSysExit};
 pub use global_variable_not_assigned::GlobalVariableNotAssigned;
@@ -33,6 +34,7 @@ mod await_outside_async;
 mod bad_str_strip_call;
 mod bad_string_format_type;
 mod bidirectional_unicode;
+mod collapsible_else_if;
 mod comparison_of_constant;
 mod consider_using_sys_exit;
 mod global_variable_not_assigned;

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap

@@ -0,0 +1,25 @@
+---
+source: crates/ruff/src/rules/pylint/mod.rs
+expression: diagnostics
+---
+- kind:
+    CollapsibleElseIf: ~
+  location:
+    row: 38
+    column: 8
+  end_location:
+    row: 39
+    column: 16
+  fix: ~
+  parent: ~
+- kind:
+    CollapsibleElseIf: ~
+  location:
+    row: 46
+    column: 8
+  end_location:
+    row: 49
+    column: 16
+  fix: ~
+  parent: ~
+