From 6f5a6b8c8b009683bb4317cc7ae6a4687348ea67 Mon Sep 17 00:00:00 2001 From: Josh Karpel Date: Mon, 20 Feb 2023 11:38:42 -0600 Subject: [PATCH] Do not autofix `E731` in class bodies (#3050) --- .../test/fixtures/pycodestyle/E731.py | 3 + .../pycodestyle/rules/lambda_assignment.rs | 88 ++++++++++++------- ...les__pycodestyle__tests__E731_E731.py.snap | 32 +++++-- 3 files changed, 84 insertions(+), 39 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pycodestyle/E731.py b/crates/ruff/resources/test/fixtures/pycodestyle/E731.py index 28dbe78b652e4..3abdd9e4bca6e 100644 --- a/crates/ruff/resources/test/fixtures/pycodestyle/E731.py +++ b/crates/ruff/resources/test/fixtures/pycodestyle/E731.py @@ -9,6 +9,9 @@ f = lambda: (yield 1) #: E731 f = lambda: (yield from g()) +#: E731 +class F: + f = lambda x: 2 * x f = object() f.method = lambda: "Method" diff --git a/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs b/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs index 3ac15d9fc045f..ec4899635d3f4 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs @@ -2,26 +2,31 @@ use ruff_macros::{define_violation, derive_message_formats}; use rustpython_parser::ast::{Arguments, Expr, ExprKind, Location, Stmt, StmtKind}; use crate::ast::helpers::{match_leading_content, match_trailing_content, unparse_stmt}; -use crate::ast::types::Range; +use crate::ast::types::{Range, ScopeKind}; use crate::ast::whitespace::leading_space; use crate::checkers::ast::Checker; use crate::fix::Fix; use crate::registry::Diagnostic; use crate::source_code::Stylist; -use crate::violation::AlwaysAutofixableViolation; +use crate::violation::{AutofixKind, Availability, Violation}; define_violation!( - pub struct LambdaAssignment(pub String); + pub struct LambdaAssignment { + pub name: String, + pub fixable: bool, + } ); -impl AlwaysAutofixableViolation for LambdaAssignment { +impl Violation for LambdaAssignment { + const AUTOFIX: Option = Some(AutofixKind::new(Availability::Sometimes)); + #[derive_message_formats] fn message(&self) -> String { format!("Do not assign a `lambda` expression, use a `def`") } - fn autofix_title(&self) -> String { - let LambdaAssignment(name) = self; - format!("Rewrite `{name}` as a `def`") + fn autofix_title_formatter(&self) -> Option String> { + self.fixable + .then_some(|v| format!("Rewrite `{}` as a `def`", v.name)) } } @@ -29,37 +34,52 @@ impl AlwaysAutofixableViolation for LambdaAssignment { pub fn lambda_assignment(checker: &mut Checker, target: &Expr, value: &Expr, stmt: &Stmt) { if let ExprKind::Name { id, .. } = &target.node { if let ExprKind::Lambda { args, body } = &value.node { - let mut diagnostic = - Diagnostic::new(LambdaAssignment(id.to_string()), Range::from_located(stmt)); - if checker.patch(diagnostic.kind.rule()) { - if !match_leading_content(stmt, checker.locator) - && !match_trailing_content(stmt, checker.locator) + // If the assignment is in a class body, it might not be safe + // to replace it because the assignment might be + // carrying a type annotation that will be used by some + // package like dataclasses, which wouldn't consider the + // rewritten function definition to be equivalent. + // See https://github.com/charliermarsh/ruff/issues/3046 + let fixable = !matches!(checker.current_scope().kind, ScopeKind::Class(_)); + + let mut diagnostic = Diagnostic::new( + LambdaAssignment { + name: id.to_string(), + fixable, + }, + Range::from_located(stmt), + ); + + if checker.patch(diagnostic.kind.rule()) + && fixable + && !match_leading_content(stmt, checker.locator) + && !match_trailing_content(stmt, checker.locator) + { + let first_line = checker.locator.slice(&Range::new( + Location::new(stmt.location.row(), 0), + Location::new(stmt.location.row() + 1, 0), + )); + let indentation = &leading_space(first_line); + let mut indented = String::new(); + for (idx, line) in function(id, args, body, checker.stylist) + .lines() + .enumerate() { - let first_line = checker.locator.slice(&Range::new( - Location::new(stmt.location.row(), 0), - Location::new(stmt.location.row() + 1, 0), - )); - let indentation = &leading_space(first_line); - let mut indented = String::new(); - for (idx, line) in function(id, args, body, checker.stylist) - .lines() - .enumerate() - { - if idx == 0 { - indented.push_str(line); - } else { - indented.push_str(checker.stylist.line_ending().as_str()); - indented.push_str(indentation); - indented.push_str(line); - } + if idx == 0 { + indented.push_str(line); + } else { + indented.push_str(checker.stylist.line_ending().as_str()); + indented.push_str(indentation); + indented.push_str(line); } - diagnostic.amend(Fix::replacement( - indented, - stmt.location, - stmt.end_location.unwrap(), - )); } + diagnostic.amend(Fix::replacement( + indented, + stmt.location, + stmt.end_location.unwrap(), + )); } + checker.diagnostics.push(diagnostic); } } diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E731_E731.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E731_E731.py.snap index 42b5af753a028..98459243c5ab1 100644 --- a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E731_E731.py.snap +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E731_E731.py.snap @@ -3,7 +3,9 @@ source: crates/ruff/src/rules/pycodestyle/mod.rs expression: diagnostics --- - kind: - LambdaAssignment: f + LambdaAssignment: + name: f + fixable: true location: row: 2 column: 0 @@ -22,7 +24,9 @@ expression: diagnostics column: 19 parent: ~ - kind: - LambdaAssignment: f + LambdaAssignment: + name: f + fixable: true location: row: 4 column: 0 @@ -41,7 +45,9 @@ expression: diagnostics column: 19 parent: ~ - kind: - LambdaAssignment: this + LambdaAssignment: + name: this + fixable: true location: row: 7 column: 4 @@ -60,7 +66,9 @@ expression: diagnostics column: 29 parent: ~ - kind: - LambdaAssignment: f + LambdaAssignment: + name: f + fixable: true location: row: 9 column: 0 @@ -79,7 +87,9 @@ expression: diagnostics column: 21 parent: ~ - kind: - LambdaAssignment: f + LambdaAssignment: + name: f + fixable: true location: row: 11 column: 0 @@ -97,4 +107,16 @@ expression: diagnostics row: 11 column: 28 parent: ~ +- kind: + LambdaAssignment: + name: f + fixable: false + location: + row: 14 + column: 4 + end_location: + row: 14 + column: 23 + fix: ~ + parent: ~