Skip to content

Commit

Permalink
Do not autofix E731 in class bodies (#3050)
Browse files Browse the repository at this point in the history
  • Loading branch information
JoshKarpel committed Feb 20, 2023
1 parent 35606d7 commit 6f5a6b8
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 39 deletions.
3 changes: 3 additions & 0 deletions crates/ruff/resources/test/fixtures/pycodestyle/E731.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
88 changes: 54 additions & 34 deletions crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,64 +2,84 @@ 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<AutofixKind> = 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<fn(&Self) -> String> {
self.fixable
.then_some(|v| format!("Rewrite `{}` as a `def`", v.name))
}
}

/// E731
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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -22,7 +24,9 @@ expression: diagnostics
column: 19
parent: ~
- kind:
LambdaAssignment: f
LambdaAssignment:
name: f
fixable: true
location:
row: 4
column: 0
Expand All @@ -41,7 +45,9 @@ expression: diagnostics
column: 19
parent: ~
- kind:
LambdaAssignment: this
LambdaAssignment:
name: this
fixable: true
location:
row: 7
column: 4
Expand All @@ -60,7 +66,9 @@ expression: diagnostics
column: 29
parent: ~
- kind:
LambdaAssignment: f
LambdaAssignment:
name: f
fixable: true
location:
row: 9
column: 0
Expand All @@ -79,7 +87,9 @@ expression: diagnostics
column: 21
parent: ~
- kind:
LambdaAssignment: f
LambdaAssignment:
name: f
fixable: true
location:
row: 11
column: 0
Expand All @@ -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: ~

0 comments on commit 6f5a6b8

Please sign in to comment.