diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE807.py b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE807.py index 3de961c00b4f3..d4bb5ac089e03 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE807.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE807.py @@ -1,27 +1,37 @@ @dataclass class Foo: foo: List[str] = field(default_factory=lambda: []) # PIE807 + bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 class FooTable(BaseTable): - bar = fields.ListField(default=lambda: []) # PIE807 + foo = fields.ListField(default=lambda: []) # PIE807 + bar = fields.ListField(default=lambda: {}) # PIE807 class FooTable(BaseTable): - bar = fields.ListField(lambda: []) # PIE807 + foo = fields.ListField(lambda: []) # PIE807 + bar = fields.ListField(default=lambda: {}) # PIE807 @dataclass class Foo: foo: List[str] = field(default_factory=list) + bar: Dict[str, int] = field(default_factory=dict) class FooTable(BaseTable): - bar = fields.ListField(list) + foo = fields.ListField(list) + bar = fields.ListField(dict) lambda *args, **kwargs: [] +lambda *args, **kwargs: {} lambda *args: [] +lambda *args: {} lambda **kwargs: [] +lambda **kwargs: {} + +lambda: {**unwrap} diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_lambdas.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_lambdas.rs index e3dffc3ad3874..b3aa849f8feb0 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_lambdas.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_lambdas.rs @@ -18,8 +18,8 @@ pub(crate) fn deferred_lambdas(checker: &mut Checker) { if checker.enabled(Rule::UnnecessaryLambda) { pylint::rules::unnecessary_lambda(checker, lambda); } - if checker.enabled(Rule::ReimplementedListBuiltin) { - flake8_pie::rules::reimplemented_list_builtin(checker, lambda); + if checker.enabled(Rule::ReimplementedContainerBuiltin) { + flake8_pie::rules::reimplemented_container_builtin(checker, lambda); } } } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index d47bb85ae3fed..47b382eae4f76 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -773,7 +773,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pie, "796") => (RuleGroup::Stable, rules::flake8_pie::rules::NonUniqueEnums), (Flake8Pie, "800") => (RuleGroup::Stable, rules::flake8_pie::rules::UnnecessarySpread), (Flake8Pie, "804") => (RuleGroup::Stable, rules::flake8_pie::rules::UnnecessaryDictKwargs), - (Flake8Pie, "807") => (RuleGroup::Stable, rules::flake8_pie::rules::ReimplementedListBuiltin), + (Flake8Pie, "807") => (RuleGroup::Stable, rules::flake8_pie::rules::ReimplementedContainerBuiltin), (Flake8Pie, "808") => (RuleGroup::Stable, rules::flake8_pie::rules::UnnecessaryRangeStart), (Flake8Pie, "810") => (RuleGroup::Stable, rules::flake8_pie::rules::MultipleStartsEndsWith), diff --git a/crates/ruff_linter/src/rules/flake8_pie/mod.rs b/crates/ruff_linter/src/rules/flake8_pie/mod.rs index b631b66df66c6..be3a2ce2770d6 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/mod.rs @@ -9,6 +9,7 @@ mod tests { use test_case::test_case; use crate::registry::Rule; + use crate::settings::types::PreviewMode; use crate::test::test_path; use crate::{assert_messages, settings}; @@ -18,7 +19,7 @@ mod tests { #[test_case(Rule::UnnecessaryRangeStart, Path::new("PIE808.py"))] #[test_case(Rule::UnnecessaryPass, Path::new("PIE790.py"))] #[test_case(Rule::UnnecessarySpread, Path::new("PIE800.py"))] - #[test_case(Rule::ReimplementedListBuiltin, Path::new("PIE807.py"))] + #[test_case(Rule::ReimplementedContainerBuiltin, Path::new("PIE807.py"))] #[test_case(Rule::NonUniqueEnums, Path::new("PIE796.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); @@ -29,4 +30,22 @@ mod tests { assert_messages!(snapshot, diagnostics); Ok(()) } + + #[test_case(Rule::ReimplementedContainerBuiltin, Path::new("PIE807.py"))] + fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!( + "preview__{}_{}", + rule_code.noqa_code(), + path.to_string_lossy() + ); + let diagnostics = test_path( + Path::new("flake8_pie").join(path).as_path(), + &settings::LinterSettings { + preview: PreviewMode::Enabled, + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/mod.rs index aa030ca27519a..a02d706b64e50 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/mod.rs @@ -1,7 +1,7 @@ pub(crate) use duplicate_class_field_definition::*; pub(crate) use multiple_starts_ends_with::*; pub(crate) use non_unique_enums::*; -pub(crate) use reimplemented_list_builtin::*; +pub(crate) use reimplemented_container_builtin::*; pub(crate) use unnecessary_dict_kwargs::*; pub(crate) use unnecessary_pass::*; pub(crate) use unnecessary_range_start::*; @@ -10,7 +10,7 @@ pub(crate) use unnecessary_spread::*; mod duplicate_class_field_definition; mod multiple_starts_ends_with; mod non_unique_enums; -mod reimplemented_list_builtin; +mod reimplemented_container_builtin; mod unnecessary_dict_kwargs; mod unnecessary_pass; mod unnecessary_range_start; diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_container_builtin.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_container_builtin.rs new file mode 100644 index 0000000000000..fbc001d9b304a --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_container_builtin.rs @@ -0,0 +1,119 @@ +use ruff_python_ast::{self as ast, Expr, ExprLambda}; + +use ruff_diagnostics::{Diagnostic, Edit, Fix}; +use ruff_diagnostics::{FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for lambdas that can be replaced with the `list` builtin. +/// +/// In [preview], this rule will also flag lambdas that can be replaced with +/// the `dict` builtin. +/// +/// ## Why is this bad? +/// Using container builtins are more succinct and idiomatic than wrapping +/// the literal in a lambda. +/// +/// ## Example +/// ```python +/// from dataclasses import dataclass, field +/// +/// +/// @dataclass +/// class Foo: +/// bar: list[int] = field(default_factory=lambda: []) +/// ``` +/// +/// Use instead: +/// ```python +/// from dataclasses import dataclass, field +/// +/// +/// @dataclass +/// class Foo: +/// bar: list[int] = field(default_factory=list) +/// baz: dict[str, int] = field(default_factory=dict) +/// ``` +/// +/// ## References +/// - [Python documentation: `list`](https://docs.python.org/3/library/functions.html#func-list) +/// +/// [preview]: https://docs.astral.sh/ruff/preview/ +#[violation] +pub struct ReimplementedContainerBuiltin { + container: Container, +} + +impl Violation for ReimplementedContainerBuiltin { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + let Self { container } = self; + format!("Prefer `{container}` over useless lambda") + } + + fn fix_title(&self) -> Option { + let Self { container } = self; + Some(format!("Replace with `lambda` with `{container}`")) + } +} + +/// PIE807 +pub(crate) fn reimplemented_container_builtin(checker: &mut Checker, expr: &ExprLambda) { + let ExprLambda { + parameters, + body, + range: _, + } = expr; + + if parameters.is_none() { + let container = match body.as_ref() { + Expr::List(ast::ExprList { elts, .. }) if elts.is_empty() => Some(Container::List), + Expr::Dict(ast::ExprDict { values, .. }) + if values.is_empty() & checker.settings.preview.is_enabled() => + { + Some(Container::Dict) + } + _ => None, + }; + if let Some(container) = container { + let mut diagnostic = + Diagnostic::new(ReimplementedContainerBuiltin { container }, expr.range()); + if checker.semantic().is_builtin(container.as_str()) { + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + container.to_string(), + expr.range(), + ))); + } + checker.diagnostics.push(diagnostic); + } + } +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +enum Container { + List, + Dict, +} + +impl Container { + fn as_str(self) -> &'static str { + match self { + Container::List => "list", + Container::Dict => "dict", + } + } +} + +impl std::fmt::Display for Container { + fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + Container::List => fmt.write_str("list"), + Container::Dict => fmt.write_str("dict"), + } + } +} diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_list_builtin.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_list_builtin.rs deleted file mode 100644 index 6195d09357533..0000000000000 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_list_builtin.rs +++ /dev/null @@ -1,76 +0,0 @@ -use ruff_python_ast::{self as ast, Expr, ExprLambda}; - -use ruff_diagnostics::{Diagnostic, Edit, Fix}; -use ruff_diagnostics::{FixAvailability, Violation}; -use ruff_macros::{derive_message_formats, violation}; -use ruff_text_size::Ranged; - -use crate::checkers::ast::Checker; - -/// ## What it does -/// Checks for lambdas that can be replaced with the `list` builtin. -/// -/// ## Why is this bad? -/// Using `list` builtin is more readable. -/// -/// ## Example -/// ```python -/// from dataclasses import dataclass, field -/// -/// -/// @dataclass -/// class Foo: -/// bar: list[int] = field(default_factory=lambda: []) -/// ``` -/// -/// Use instead: -/// ```python -/// from dataclasses import dataclass, field -/// -/// -/// @dataclass -/// class Foo: -/// bar: list[int] = field(default_factory=list) -/// ``` -/// -/// ## References -/// - [Python documentation: `list`](https://docs.python.org/3/library/functions.html#func-list) -#[violation] -pub struct ReimplementedListBuiltin; - -impl Violation for ReimplementedListBuiltin { - const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; - - #[derive_message_formats] - fn message(&self) -> String { - format!("Prefer `list` over useless lambda") - } - - fn fix_title(&self) -> Option { - Some("Replace with `list`".to_string()) - } -} - -/// PIE807 -pub(crate) fn reimplemented_list_builtin(checker: &mut Checker, expr: &ExprLambda) { - let ExprLambda { - parameters, - body, - range: _, - } = expr; - - if parameters.is_none() { - if let Expr::List(ast::ExprList { elts, .. }) = body.as_ref() { - if elts.is_empty() { - let mut diagnostic = Diagnostic::new(ReimplementedListBuiltin, expr.range()); - if checker.semantic().is_builtin("list") { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - "list".to_string(), - expr.range(), - ))); - } - checker.diagnostics.push(diagnostic); - } - } - } -} diff --git a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE807_PIE807.py.snap b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE807_PIE807.py.snap index 97772c61d9b81..ad09dcb5de8ff 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE807_PIE807.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE807_PIE807.py.snap @@ -7,52 +7,55 @@ PIE807.py:3:44: PIE807 [*] Prefer `list` over useless lambda 2 | class Foo: 3 | foo: List[str] = field(default_factory=lambda: []) # PIE807 | ^^^^^^^^^^ PIE807 +4 | bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 | - = help: Replace with `list` + = help: Replace with `lambda` with `list` ℹ Safe fix 1 1 | @dataclass 2 2 | class Foo: 3 |- foo: List[str] = field(default_factory=lambda: []) # PIE807 3 |+ foo: List[str] = field(default_factory=list) # PIE807 -4 4 | +4 4 | bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 5 5 | -6 6 | class FooTable(BaseTable): +6 6 | -PIE807.py:7:36: PIE807 [*] Prefer `list` over useless lambda +PIE807.py:8:36: PIE807 [*] Prefer `list` over useless lambda | -6 | class FooTable(BaseTable): -7 | bar = fields.ListField(default=lambda: []) # PIE807 +7 | class FooTable(BaseTable): +8 | foo = fields.ListField(default=lambda: []) # PIE807 | ^^^^^^^^^^ PIE807 +9 | bar = fields.ListField(default=lambda: {}) # PIE807 | - = help: Replace with `list` + = help: Replace with `lambda` with `list` ℹ Safe fix -4 4 | 5 5 | -6 6 | class FooTable(BaseTable): -7 |- bar = fields.ListField(default=lambda: []) # PIE807 - 7 |+ bar = fields.ListField(default=list) # PIE807 -8 8 | -9 9 | -10 10 | class FooTable(BaseTable): +6 6 | +7 7 | class FooTable(BaseTable): +8 |- foo = fields.ListField(default=lambda: []) # PIE807 + 8 |+ foo = fields.ListField(default=list) # PIE807 +9 9 | bar = fields.ListField(default=lambda: {}) # PIE807 +10 10 | +11 11 | -PIE807.py:11:28: PIE807 [*] Prefer `list` over useless lambda +PIE807.py:13:28: PIE807 [*] Prefer `list` over useless lambda | -10 | class FooTable(BaseTable): -11 | bar = fields.ListField(lambda: []) # PIE807 +12 | class FooTable(BaseTable): +13 | foo = fields.ListField(lambda: []) # PIE807 | ^^^^^^^^^^ PIE807 +14 | bar = fields.ListField(default=lambda: {}) # PIE807 | - = help: Replace with `list` + = help: Replace with `lambda` with `list` ℹ Safe fix -8 8 | -9 9 | -10 10 | class FooTable(BaseTable): -11 |- bar = fields.ListField(lambda: []) # PIE807 - 11 |+ bar = fields.ListField(list) # PIE807 -12 12 | -13 13 | -14 14 | @dataclass +10 10 | +11 11 | +12 12 | class FooTable(BaseTable): +13 |- foo = fields.ListField(lambda: []) # PIE807 + 13 |+ foo = fields.ListField(list) # PIE807 +14 14 | bar = fields.ListField(default=lambda: {}) # PIE807 +15 15 | +16 16 | diff --git a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__preview__PIE807_PIE807.py.snap b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__preview__PIE807_PIE807.py.snap new file mode 100644 index 0000000000000..474c3456d53f9 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__preview__PIE807_PIE807.py.snap @@ -0,0 +1,118 @@ +--- +source: crates/ruff_linter/src/rules/flake8_pie/mod.rs +--- +PIE807.py:3:44: PIE807 [*] Prefer `list` over useless lambda + | +1 | @dataclass +2 | class Foo: +3 | foo: List[str] = field(default_factory=lambda: []) # PIE807 + | ^^^^^^^^^^ PIE807 +4 | bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 + | + = help: Replace with `lambda` with `list` + +ℹ Safe fix +1 1 | @dataclass +2 2 | class Foo: +3 |- foo: List[str] = field(default_factory=lambda: []) # PIE807 + 3 |+ foo: List[str] = field(default_factory=list) # PIE807 +4 4 | bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 +5 5 | +6 6 | + +PIE807.py:4:49: PIE807 [*] Prefer `dict` over useless lambda + | +2 | class Foo: +3 | foo: List[str] = field(default_factory=lambda: []) # PIE807 +4 | bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 + | ^^^^^^^^^^ PIE807 + | + = help: Replace with `lambda` with `dict` + +ℹ Safe fix +1 1 | @dataclass +2 2 | class Foo: +3 3 | foo: List[str] = field(default_factory=lambda: []) # PIE807 +4 |- bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 + 4 |+ bar: Dict[str, int] = field(default_factory=dict) # PIE807 +5 5 | +6 6 | +7 7 | class FooTable(BaseTable): + +PIE807.py:8:36: PIE807 [*] Prefer `list` over useless lambda + | +7 | class FooTable(BaseTable): +8 | foo = fields.ListField(default=lambda: []) # PIE807 + | ^^^^^^^^^^ PIE807 +9 | bar = fields.ListField(default=lambda: {}) # PIE807 + | + = help: Replace with `lambda` with `list` + +ℹ Safe fix +5 5 | +6 6 | +7 7 | class FooTable(BaseTable): +8 |- foo = fields.ListField(default=lambda: []) # PIE807 + 8 |+ foo = fields.ListField(default=list) # PIE807 +9 9 | bar = fields.ListField(default=lambda: {}) # PIE807 +10 10 | +11 11 | + +PIE807.py:9:36: PIE807 [*] Prefer `dict` over useless lambda + | +7 | class FooTable(BaseTable): +8 | foo = fields.ListField(default=lambda: []) # PIE807 +9 | bar = fields.ListField(default=lambda: {}) # PIE807 + | ^^^^^^^^^^ PIE807 + | + = help: Replace with `lambda` with `dict` + +ℹ Safe fix +6 6 | +7 7 | class FooTable(BaseTable): +8 8 | foo = fields.ListField(default=lambda: []) # PIE807 +9 |- bar = fields.ListField(default=lambda: {}) # PIE807 + 9 |+ bar = fields.ListField(default=dict) # PIE807 +10 10 | +11 11 | +12 12 | class FooTable(BaseTable): + +PIE807.py:13:28: PIE807 [*] Prefer `list` over useless lambda + | +12 | class FooTable(BaseTable): +13 | foo = fields.ListField(lambda: []) # PIE807 + | ^^^^^^^^^^ PIE807 +14 | bar = fields.ListField(default=lambda: {}) # PIE807 + | + = help: Replace with `lambda` with `list` + +ℹ Safe fix +10 10 | +11 11 | +12 12 | class FooTable(BaseTable): +13 |- foo = fields.ListField(lambda: []) # PIE807 + 13 |+ foo = fields.ListField(list) # PIE807 +14 14 | bar = fields.ListField(default=lambda: {}) # PIE807 +15 15 | +16 16 | + +PIE807.py:14:36: PIE807 [*] Prefer `dict` over useless lambda + | +12 | class FooTable(BaseTable): +13 | foo = fields.ListField(lambda: []) # PIE807 +14 | bar = fields.ListField(default=lambda: {}) # PIE807 + | ^^^^^^^^^^ PIE807 + | + = help: Replace with `lambda` with `dict` + +ℹ Safe fix +11 11 | +12 12 | class FooTable(BaseTable): +13 13 | foo = fields.ListField(lambda: []) # PIE807 +14 |- bar = fields.ListField(default=lambda: {}) # PIE807 + 14 |+ bar = fields.ListField(default=dict) # PIE807 +15 15 | +16 16 | +17 17 | @dataclass + +