Skip to content

Commit

Permalink
Ignore annotated lambdas in class scopes (#10720)
Browse files Browse the repository at this point in the history
## Summary

An annotated lambda assignment within a class scope is often
intentional. For example, within a dataclass or Pydantic model, these
are treated as fields rather than methods (and so can be passed values
in constructors).

I originally wrote this to special-case dataclasses and Pydantic
models... But was left feeling like we'd see more false positives here
for little gain (an annotated lambda within a `class` is likely
intentional?). Open to opinions, though.

Closes #10718.
  • Loading branch information
charliermarsh committed Apr 1, 2024
1 parent 67f0f61 commit d36f609
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 32 deletions.
11 changes: 10 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/pycodestyle/E731.py
Expand Up @@ -60,7 +60,7 @@ class Scope:
class Scope:
from typing import Callable

# E731
# OK
f: Callable[[int], int] = lambda x: 2 * x


Expand Down Expand Up @@ -147,3 +147,12 @@ def scope():
f = lambda: (
i := 1,
)


from dataclasses import dataclass
from typing import Callable

@dataclass
class FilterDataclass:
# OK
filter: Callable[[str], bool] = lambda _: True
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pycodestyle/helpers.rs
@@ -1,3 +1,4 @@
/// Returns `true` if the name should be considered "ambiguous".
pub(super) fn is_ambiguous_name(name: &str) -> bool {
name == "l" || name == "I" || name == "O"
}
25 changes: 15 additions & 10 deletions crates/ruff_linter/src/rules/pycodestyle/rules/lambda_assignment.rs
@@ -1,14 +1,13 @@
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{
self as ast, Expr, Identifier, Parameter, ParameterWithDefault, Parameters, Stmt,
};
use ruff_text_size::{Ranged, TextRange};

use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_codegen::Generator;
use ruff_python_semantic::SemanticModel;
use ruff_python_trivia::{has_leading_content, has_trailing_content, leading_indentation};
use ruff_source_file::UniversalNewlines;
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;

Expand Down Expand Up @@ -106,12 +105,17 @@ pub(crate) fn lambda_assignment(
}
}

// 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. Even if it _doesn't_ have an annotation, rewriting safely would require
// making this a static method.
// See: https://github.com/astral-sh/ruff/issues/3046
// If the assignment is a class attribute (with an annotation), ignore it.
//
// This is most common for, e.g., dataclasses and Pydantic models. Those libraries will
// treat the lambda as an assignable field, and the use of a lambda is almost certainly
// intentional.
if annotation.is_some() && checker.semantic().current_scope().kind.is_class() {
return;
}

// Otherwise, if the assignment is in a class body, flag it, but use a display-only fix.
// Rewriting safely would require making this a static method.
//
// Similarly, if the lambda is shadowing a variable in the current scope,
// rewriting it as a function declaration may break type-checking.
Expand Down Expand Up @@ -179,6 +183,7 @@ fn extract_types(annotation: &Expr, semantic: &SemanticModel) -> Option<(Vec<Exp
Some((params, return_type))
}

/// Generate a function definition from a `lambda` expression.
fn function(
name: &str,
parameters: Option<&Parameters>,
Expand Down
Expand Up @@ -120,25 +120,6 @@ E731.py:57:5: E731 Do not assign a `lambda` expression, use a `def`
59 60 |
60 61 | class Scope:

E731.py:64:5: E731 Do not assign a `lambda` expression, use a `def`
|
63 | # E731
64 | f: Callable[[int], int] = lambda x: 2 * x
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E731
|
= help: Rewrite `f` as a `def`

Display-only fix
61 61 | from typing import Callable
62 62 |
63 63 | # E731
64 |- f: Callable[[int], int] = lambda x: 2 * x
64 |+ def f(x: int) -> int:
65 |+ return 2 * x
65 66 |
66 67 |
67 68 | def scope():

E731.py:73:9: E731 Do not assign a `lambda` expression, use a `def`
|
71 | x: Callable[[int], int]
Expand Down Expand Up @@ -383,5 +364,6 @@ E731.py:147:5: E731 [*] Do not assign a `lambda` expression, use a `def`
149 |- )
147 |+ def f():
148 |+ return (i := 1),


150 149 |
151 150 |
152 151 | from dataclasses import dataclass

0 comments on commit d36f609

Please sign in to comment.