[pylint] Implement consider-using-assignment-expr (R6103)

crates/ruff_diagnostics/src/edit.rs

@@ -39,7 +39,7 @@ impl Edit {
 
     /// Creates an edit that replaces the content in `range` with `content`.
     pub fn range_replacement(content: String, range: TextRange) -> Self {
-        debug_assert!(!content.is_empty(), "Prefer `Fix::deletion`");
+        debug_assert!(!content.is_empty(), "Prefer `Edit::deletion`");
 
         Self {
             content: Some(Box::from(content)),

crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_assignment.py

@@ -0,0 +1,70 @@
+bad1 = 'example'
+if bad1: # [consider-using-assignment-expr]
+    pass
+
+bad2 = 'example'
+if bad2 and True: # [consider-using-assignment-expr]
+    pass
+
+bad3 = 'example'
+if bad3 and bad3 == 'example': # [consider-using-assignment-expr]
+    pass
+
+
+def foo():
+    bad4 = 0
+    if bad4: # [consider-using-assignment-expr]
+        pass
+
+bad5 = (
+    'example',
+    'example',
+    'example',
+    'example',
+    'example',
+    'example',
+    'example',
+    'example',
+    'example',
+    'example',
+)
+if bad5: # [consider-using-assignment-expr]
+    pass
+
+bad6 = 'example'
+if bad6 is not None: # [consider-using-assignment-expr]
+    pass
+
+bad7 = 'example'
+if bad7 == 'something': # [consider-using-assignment-expr]
+    pass
+elif bad7 == 'something else':
+    pass
+
+
+good1_1 = 'example'
+good1_2 = 'example'
+if good1_1: # correct, assignment is not the previous statement
+    pass
+
+good2_1 = 'example'
+good2_2 = good2_1
+if good2_1: # correct, assignment is not the previous statement
+    pass
+
+if good3 := 'example': # correct, used like it is intented
+    pass
+
+def test(good4: str | None = None):
+    if good4 is None:
+        good4 = 'test'
+
+def bar():
+    good5_5 = 'example'
+    good5_2 = good5_5
+    if good5_5: # correct, assignment is not the previous statement
+        pass
+
+for good6 in [1, 2, 3]:
+    if good6: # correct, used like it is intented
+        pass

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

@@ -1131,6 +1131,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.enabled(Rule::TooManyNestedBlocks) {
                 pylint::rules::too_many_nested_blocks(checker, stmt);
             }
+            if checker.enabled(Rule::UnnecessaryAssignment) {
+                pylint::rules::unnecessary_assignment(checker, if_);
+            }
             if checker.enabled(Rule::EmptyTypeCheckingBlock) {
                 flake8_type_checking::rules::empty_type_checking_block(checker, if_);
             }

crates/ruff_linter/src/codes.rs

@@ -254,6 +254,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "R1704") => (RuleGroup::Stable, rules::pylint::rules::RedefinedArgumentFromLocal),
         (Pylint, "R1706") => (RuleGroup::Removed, rules::pylint::rules::AndOrTernary),
         (Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn),
+        (Pylint, "R6103") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryAssignment),
         (Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison),
         (Pylint, "R1722") => (RuleGroup::Stable, rules::pylint::rules::SysExitAlias),
         (Pylint, "R1730") => (RuleGroup::Stable, rules::pylint::rules::IfStmtMinMax),

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

@@ -40,6 +40,7 @@ mod tests {
     #[test_case(Rule::CompareToEmptyString, Path::new("compare_to_empty_string.py"))]
     #[test_case(Rule::ComparisonOfConstant, Path::new("comparison_of_constant.py"))]
     #[test_case(Rule::ComparisonWithItself, Path::new("comparison_with_itself.py"))]
+    #[test_case(Rule::UnnecessaryAssignment, Path::new("unnecessary_assignment.py"))]
     #[test_case(Rule::EqWithoutHash, Path::new("eq_without_hash.py"))]
     #[test_case(Rule::EmptyComment, Path::new("empty_comment.py"))]
     #[test_case(Rule::ManualFromImport, Path::new("import_aliasing.py"))]

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

@@ -87,6 +87,7 @@ pub(crate) use type_bivariance::*;
 pub(crate) use type_name_incorrect_variance::*;
 pub(crate) use type_param_name_mismatch::*;
 pub(crate) use unexpected_special_method_signature::*;
+pub(crate) use unnecessary_assignment::*;
 pub(crate) use unnecessary_dict_index_lookup::*;
 pub(crate) use unnecessary_direct_lambda_call::*;
 pub(crate) use unnecessary_dunder_call::*;
@@ -190,6 +191,7 @@ mod type_bivariance;
 mod type_name_incorrect_variance;
 mod type_param_name_mismatch;
 mod unexpected_special_method_signature;
+mod unnecessary_assignment;
 mod unnecessary_dict_index_lookup;
 mod unnecessary_direct_lambda_call;
 mod unnecessary_dunder_call;

crates/ruff_linter/src/rules/pylint/rules/unnecessary_assignment.rs

@@ -0,0 +1,199 @@
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{name::Name, Expr, ExprName, Stmt, StmtAssign, StmtIf};
+use ruff_python_codegen::Generator;
+use ruff_python_index::Indexer;
+use ruff_python_semantic::SemanticModel;
+use ruff_source_file::Locator;
+use ruff_text_size::Ranged;
+
+use crate::{checkers::ast::Checker, fix::edits::delete_stmt, settings::types::PythonVersion};
+
+/// ## What it does
+/// Check for cases where an variable assignment is directly followed by an if statement, these can be combined into a single statement using the `:=` operator.
+///
+/// ## Why is this bad?
+/// The code can written more concise, often improving readability.
+///
+/// ## Example
+///
+/// ```python
+/// test1 = "example"
+/// if test1:
+///     print(test1)
+/// ```
+///
+/// Use instead:
+///
+/// ```python
+/// if test1 := "example":
+///     print(test1)
+/// ```
+#[violation]
+pub struct UnnecessaryAssignment {
+    name: Name,
+    assignment: String,
+    parentheses: bool,
+}
+
+impl UnnecessaryAssignment {
+    fn get_fix(&self) -> String {
+        let UnnecessaryAssignment {
+            name,
+            assignment,
+            parentheses,
+        } = self;
+        if *parentheses {
+            format!("({name} := {assignment})")
+        } else {
+            format!("{name} := {assignment}")
+        }
+    }
+}
+
+impl Violation for UnnecessaryAssignment {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Use walrus operator `{}`.", self.get_fix())
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some(format!(
+            "Move variable assignment into if statement using walrus operator `{}`.",
+            self.get_fix()
+        ))
+    }
+}
+
+type AssignmentBeforeIfStmt<'a> = (Expr, ExprName, StmtAssign);
+
+/// PLR6103
+pub(crate) fn unnecessary_assignment(checker: &mut Checker, stmt: &StmtIf) {
+    if checker.settings.target_version < PythonVersion::Py38 {
+        return;
+    }
+
+    let if_test = *stmt.test.clone();
+    let semantic = checker.semantic();
+    let mut errors: Vec<AssignmentBeforeIfStmt> = Vec::new();
+
+    // case - if check (`if test1:`)
+    if let Some(unreferenced_binding) = find_assignment_before_if_stmt(semantic, &if_test, &if_test)
+    {
+        errors.push(unreferenced_binding);
+    };
+
+    // case - bool operations (`if test1 and test2:`)
+    if let Expr::BoolOp(expr) = if_test.clone() {
+        errors.extend(
+            expr.values
+                .iter()
+                .filter_map(|bool_test| {
+                    find_assignment_before_if_stmt(semantic, &if_test, bool_test)
+                })
+                .collect::<Vec<AssignmentBeforeIfStmt>>(),
+        );
+    }
+
+    // case - compare (`if test1 is not None:`)
+    if let Expr::Compare(compare) = if_test.clone() {
+        if let Some(error) = find_assignment_before_if_stmt(semantic, &if_test, &compare.left) {
+            errors.push(error);
+        };
+    }
+
+    // case - elif else clauses (`elif test1:`)
+    let elif_else_clauses = stmt.elif_else_clauses.clone();
+    errors.extend(
+        elif_else_clauses
+            .iter()
+            .filter(|elif_else_clause| elif_else_clause.test.is_some())
+            .filter_map(|elif_else_clause| {
+                let elif_check = elif_else_clause.test.clone().unwrap();
+                find_assignment_before_if_stmt(semantic, &elif_check, &elif_check)
+            })
+            .collect::<Vec<AssignmentBeforeIfStmt>>(),
+    );
+
+    // add found diagnostics
+    checker.diagnostics.extend(
+        errors
+            .into_iter()
+            .map(|error| {
+                create_diagnostic(
+                    checker.locator(),
+                    checker.indexer(),
+                    checker.generator(),
+                    error,
+                )
+            })
+            .collect::<Vec<Diagnostic>>(),
+    );
+}
+
+/// Find possible assignment before if statement
+///
+/// * `if_test` - the complete if test (`if test1 and test2`)
+/// * `if_test_part` - part of the if test (`test1`)
+fn find_assignment_before_if_stmt<'a>(
+    semantic: &'a SemanticModel,
+    if_test: &Expr,
+    if_test_part: &Expr,
+) -> Option<AssignmentBeforeIfStmt<'a>> {
+    // early exit when the test part is not a variable
+    let Expr::Name(test_variable) = if_test_part.clone() else {
+        return None;
+    };
+
+    let current_statement = semantic.current_statement();
+    let previous_statement = semantic
+        .previous_statement(current_statement)?
+        .as_assign_stmt()?;
+
+    // only care about single assignment target like `x = 'example'`
+    let [assigned_variable] = &previous_statement.targets[..] else {
+        return None;
+    };
+
+    // check whether the check variable is the assignment variable
+    if test_variable.id != assigned_variable.as_name_expr()?.id {
+        return None;
+    }
+
+    Some((if_test.clone(), test_variable, previous_statement.clone()))
+}
+
+fn create_diagnostic(
+    locator: &Locator,
+    indexer: &Indexer,
+    generator: Generator,
+    error: AssignmentBeforeIfStmt,
+) -> Diagnostic {
+    let (origin, expr_name, assignment) = error;
+    let assignment_expr = generator.expr(&assignment.value.clone());
+    let use_parentheses = origin.is_bool_op_expr() || !assignment.value.is_name_expr();
+
+    let mut diagnostic = Diagnostic::new(
+        UnnecessaryAssignment {
+            name: expr_name.clone().id,
+            assignment: assignment_expr.clone(),
+            parentheses: use_parentheses,
+        },
+        expr_name.clone().range(),
+    );
+
+    let format = if use_parentheses {
+        format!("({} := {})", expr_name.clone().id, assignment_expr.clone())
+    } else {
+        format!("{} := {}", expr_name.clone().id, assignment_expr.clone())
+    };
+
+    let delete_assignment_edit = delete_stmt(&Stmt::from(assignment), None, locator, indexer);
+    let use_walrus_edit = Edit::range_replacement(format, diagnostic.range());
+
+    diagnostic.set_fix(Fix::unsafe_edits(delete_assignment_edit, [use_walrus_edit]));
+
+    diagnostic
+}