From 56eb170f9ae38ad9e27d42961b6a8c5b0a7daca8 Mon Sep 17 00:00:00 2001 From: Alan Du Date: Fri, 10 Nov 2023 14:17:29 -0500 Subject: [PATCH 1/6] Generalize PIE807 to include dict literals --- .../checkers/ast/analyze/deferred_lambdas.rs | 4 +-- crates/ruff_linter/src/codes.rs | 2 +- .../ruff_linter/src/rules/flake8_pie/mod.rs | 2 +- .../src/rules/flake8_pie/rules/mod.rs | 4 +-- ....rs => reimplemented_container_builtin.rs} | 35 +++++++++++-------- 5 files changed, 26 insertions(+), 21 deletions(-) rename crates/ruff_linter/src/rules/flake8_pie/rules/{reimplemented_list_builtin.rs => reimplemented_container_builtin.rs} (54%) 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 7b307a7da68ac..ab82f87ffa4bd 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -772,7 +772,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..bd68bcdb0b8c8 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/mod.rs @@ -18,7 +18,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()); 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_list_builtin.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_container_builtin.rs similarity index 54% rename from crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_list_builtin.rs rename to crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_container_builtin.rs index 6195d09357533..dcefb05d55645 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_list_builtin.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_container_builtin.rs @@ -21,6 +21,7 @@ use crate::checkers::ast::Checker; /// @dataclass /// class Foo: /// bar: list[int] = field(default_factory=lambda: []) +/// baz: dict[str, int] = field(default_factory=lambda: {}) /// ``` /// /// Use instead: @@ -31,28 +32,29 @@ use crate::checkers::ast::Checker; /// @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) #[violation] -pub struct ReimplementedListBuiltin; +pub struct ReimplementedContainerBuiltin; -impl Violation for ReimplementedListBuiltin { +impl Violation for ReimplementedContainerBuiltin { const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; #[derive_message_formats] fn message(&self) -> String { - format!("Prefer `list` over useless lambda") + format!("Prefer `list` or `dict` over useless lambda") } fn fix_title(&self) -> Option { - Some("Replace with `list`".to_string()) + Some("Replace with `list` or `dict`".to_string()) } } /// PIE807 -pub(crate) fn reimplemented_list_builtin(checker: &mut Checker, expr: &ExprLambda) { +pub(crate) fn reimplemented_container_builtin(checker: &mut Checker, expr: &ExprLambda) { let ExprLambda { parameters, body, @@ -60,17 +62,20 @@ pub(crate) fn reimplemented_list_builtin(checker: &mut Checker, expr: &ExprLambd } = 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); + let builtin = match body.as_ref() { + Expr::List(ast::ExprList { elts, .. }) if elts.is_empty() => Some("list"), + Expr::Dict(ast::ExprDict { values, .. }) if values.is_empty() => Some("dict"), + _ => None, + }; + if let Some(builtin) = builtin { + let mut diagnostic = Diagnostic::new(ReimplementedContainerBuiltin, expr.range()); + if checker.semantic().is_builtin(builtin) { + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + builtin.to_string(), + expr.range(), + ))); } + checker.diagnostics.push(diagnostic); } } } From d4af4f5a5f70550efb473d3c9baa7eba4079cfcd Mon Sep 17 00:00:00 2001 From: Alan Du Date: Fri, 10 Nov 2023 14:30:48 -0500 Subject: [PATCH 2/6] Update snpashot tests --- .../test/fixtures/flake8_pie/PIE807.py | 16 ++- ...__flake8_pie__tests__PIE807_PIE807.py.snap | 116 +++++++++++++----- 2 files changed, 101 insertions(+), 31 deletions(-) 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/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..ba7481e9c374e 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 @@ -1,58 +1,118 @@ --- source: crates/ruff_linter/src/rules/flake8_pie/mod.rs --- -PIE807.py:3:44: PIE807 [*] Prefer `list` over useless lambda +PIE807.py:3:44: PIE807 [*] Prefer `list` or `dict` 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 `list` + = help: Replace with `list` or `dict` ℹ 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:4:49: PIE807 [*] Prefer `list` or `dict` over useless lambda | -6 | class FooTable(BaseTable): -7 | bar = fields.ListField(default=lambda: []) # PIE807 +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 `list` or `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` or `dict` 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 `list` + = help: Replace with `list` or `dict` ℹ 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): - -PIE807.py:11:28: PIE807 [*] Prefer `list` over useless lambda +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 `list` or `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 `list` or `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` or `dict` 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` or `dict` + +ℹ 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 `list` or `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 `list` + = help: Replace with `list` or `dict` ℹ 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 +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 From b630ddece716aaa887442c3b103c9f1a9ab10fde Mon Sep 17 00:00:00 2001 From: Alan Du Date: Mon, 13 Nov 2023 10:32:49 -0500 Subject: [PATCH 3/6] Make PIE807 dict fix preview only --- .../ruff_linter/src/rules/flake8_pie/mod.rs | 19 +++ .../rules/reimplemented_container_builtin.rs | 24 ++-- ...__flake8_pie__tests__PIE807_PIE807.py.snap | 69 +--------- ...pie__tests__preview__PIE807_PIE807.py.snap | 118 ++++++++++++++++++ 4 files changed, 160 insertions(+), 70 deletions(-) create mode 100644 crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__preview__PIE807_PIE807.py.snap diff --git a/crates/ruff_linter/src/rules/flake8_pie/mod.rs b/crates/ruff_linter/src/rules/flake8_pie/mod.rs index bd68bcdb0b8c8..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}; @@ -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/reimplemented_container_builtin.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_container_builtin.rs index dcefb05d55645..bacd3c8c1e517 100644 --- 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 @@ -9,9 +9,11 @@ use crate::checkers::ast::Checker; /// ## What it does /// Checks for lambdas that can be replaced with the `list` builtin. +/// If [preview] mode is enabled, then we will also look for lambdas +/// that can be replaced with the `dict` builtin. /// /// ## Why is this bad? -/// Using `list` builtin is more readable. +/// Using container builtins is more readable. /// /// ## Example /// ```python @@ -21,7 +23,6 @@ use crate::checkers::ast::Checker; /// @dataclass /// class Foo: /// bar: list[int] = field(default_factory=lambda: []) -/// baz: dict[str, int] = field(default_factory=lambda: {}) /// ``` /// /// Use instead: @@ -35,21 +36,25 @@ use crate::checkers::ast::Checker; /// baz: dict[str, int] = field(default_factory=dict) /// ``` /// +/// +/// If [preview] /// ## 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; +pub struct ReimplementedContainerBuiltin(&'static str); impl Violation for ReimplementedContainerBuiltin { const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; #[derive_message_formats] fn message(&self) -> String { - format!("Prefer `list` or `dict` over useless lambda") + format!("Prefer `{}` over useless lambda", self.0) } fn fix_title(&self) -> Option { - Some("Replace with `list` or `dict`".to_string()) + Some(format!("Replace with lambda with `{}`", self.0)) } } @@ -64,11 +69,16 @@ pub(crate) fn reimplemented_container_builtin(checker: &mut Checker, expr: &Expr if parameters.is_none() { let builtin = match body.as_ref() { Expr::List(ast::ExprList { elts, .. }) if elts.is_empty() => Some("list"), - Expr::Dict(ast::ExprDict { values, .. }) if values.is_empty() => Some("dict"), + Expr::Dict(ast::ExprDict { values, .. }) + if values.is_empty() & checker.settings.preview.is_enabled() => + { + Some("dict") + } _ => None, }; if let Some(builtin) = builtin { - let mut diagnostic = Diagnostic::new(ReimplementedContainerBuiltin, expr.range()); + let mut diagnostic = + Diagnostic::new(ReimplementedContainerBuiltin(builtin), expr.range()); if checker.semantic().is_builtin(builtin) { diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( builtin.to_string(), 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 ba7481e9c374e..99f88bfe1b7ac 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 @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_pie/mod.rs --- -PIE807.py:3:44: PIE807 [*] Prefer `list` or `dict` over useless lambda +PIE807.py:3:44: PIE807 [*] Prefer `list` over useless lambda | 1 | @dataclass 2 | class Foo: @@ -9,7 +9,7 @@ PIE807.py:3:44: PIE807 [*] Prefer `list` or `dict` over useless lambda | ^^^^^^^^^^ PIE807 4 | bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 | - = help: Replace with `list` or `dict` + = help: Replace with lambda with `list` ℹ Safe fix 1 1 | @dataclass @@ -20,33 +20,14 @@ PIE807.py:3:44: PIE807 [*] Prefer `list` or `dict` over useless lambda 5 5 | 6 6 | -PIE807.py:4:49: PIE807 [*] Prefer `list` or `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 `list` or `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` or `dict` over useless lambda +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 `list` or `dict` + = help: Replace with lambda with `list` ℹ Safe fix 5 5 | @@ -58,33 +39,14 @@ PIE807.py:8:36: PIE807 [*] Prefer `list` or `dict` over useless lambda 10 10 | 11 11 | -PIE807.py:9:36: PIE807 [*] Prefer `list` or `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 `list` or `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` or `dict` over useless lambda +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 `list` or `dict` + = help: Replace with lambda with `list` ℹ Safe fix 10 10 | @@ -96,23 +58,4 @@ PIE807.py:13:28: PIE807 [*] Prefer `list` or `dict` over useless lambda 15 15 | 16 16 | -PIE807.py:14:36: PIE807 [*] Prefer `list` or `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 `list` or `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 - 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..1cb01931102e7 --- /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 + + From 1154cbf38c810b0d800c2f5aae37a31e3bc3eedd Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 13 Nov 2023 12:45:52 -0500 Subject: [PATCH 4/6] Use an enum --- .../rules/reimplemented_container_builtin.rs | 54 ++++++++++++++----- ...__flake8_pie__tests__PIE807_PIE807.py.snap | 6 +-- ...pie__tests__preview__PIE807_PIE807.py.snap | 12 ++--- 3 files changed, 51 insertions(+), 21 deletions(-) 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 index bacd3c8c1e517..ffc06a6fdda6a 100644 --- 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 @@ -1,4 +1,5 @@ use ruff_python_ast::{self as ast, Expr, ExprLambda}; +use std::fmt::Write; use ruff_diagnostics::{Diagnostic, Edit, Fix}; use ruff_diagnostics::{FixAvailability, Violation}; @@ -9,8 +10,9 @@ use crate::checkers::ast::Checker; /// ## What it does /// Checks for lambdas that can be replaced with the `list` builtin. -/// If [preview] mode is enabled, then we will also look for lambdas -/// that can be replaced with the `dict` builtin. +/// +/// In [preview], this rule will also flag lambdas that can be replaced with +/// the `dict` builtin. /// /// ## Why is this bad? /// Using container builtins is more readable. @@ -43,18 +45,22 @@ use crate::checkers::ast::Checker; /// /// [preview]: https://docs.astral.sh/ruff/preview/ #[violation] -pub struct ReimplementedContainerBuiltin(&'static str); +pub struct ReimplementedContainerBuiltin { + container: Container, +} impl Violation for ReimplementedContainerBuiltin { const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; #[derive_message_formats] fn message(&self) -> String { - format!("Prefer `{}` over useless lambda", self.0) + let Self { container } = self; + format!("Prefer `{container}` over useless lambda") } fn fix_title(&self) -> Option { - Some(format!("Replace with lambda with `{}`", self.0)) + let Self { container } = self; + Some(format!("Replace with `lambda` with `{container}`")) } } @@ -67,21 +73,21 @@ pub(crate) fn reimplemented_container_builtin(checker: &mut Checker, expr: &Expr } = expr; if parameters.is_none() { - let builtin = match body.as_ref() { - Expr::List(ast::ExprList { elts, .. }) if elts.is_empty() => Some("list"), + 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("dict") + Some(Container::Dict) } _ => None, }; - if let Some(builtin) = builtin { + if let Some(container) = container { let mut diagnostic = - Diagnostic::new(ReimplementedContainerBuiltin(builtin), expr.range()); - if checker.semantic().is_builtin(builtin) { + Diagnostic::new(ReimplementedContainerBuiltin { container }, expr.range()); + if checker.semantic().is_builtin(container.as_str()) { diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - builtin.to_string(), + container.to_string(), expr.range(), ))); } @@ -89,3 +95,27 @@ pub(crate) fn reimplemented_container_builtin(checker: &mut Checker, expr: &Expr } } } + +#[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/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 99f88bfe1b7ac..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 @@ -9,7 +9,7 @@ PIE807.py:3:44: PIE807 [*] Prefer `list` over useless lambda | ^^^^^^^^^^ PIE807 4 | bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 | - = help: Replace with lambda with `list` + = help: Replace with `lambda` with `list` ℹ Safe fix 1 1 | @dataclass @@ -27,7 +27,7 @@ PIE807.py:8:36: PIE807 [*] Prefer `list` over useless lambda | ^^^^^^^^^^ PIE807 9 | bar = fields.ListField(default=lambda: {}) # PIE807 | - = help: Replace with lambda with `list` + = help: Replace with `lambda` with `list` ℹ Safe fix 5 5 | @@ -46,7 +46,7 @@ PIE807.py:13:28: PIE807 [*] Prefer `list` over useless lambda | ^^^^^^^^^^ PIE807 14 | bar = fields.ListField(default=lambda: {}) # PIE807 | - = help: Replace with lambda with `list` + = help: Replace with `lambda` with `list` ℹ Safe fix 10 10 | 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 index 1cb01931102e7..474c3456d53f9 100644 --- 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 @@ -9,7 +9,7 @@ PIE807.py:3:44: PIE807 [*] Prefer `list` over useless lambda | ^^^^^^^^^^ PIE807 4 | bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 | - = help: Replace with lambda with `list` + = help: Replace with `lambda` with `list` ℹ Safe fix 1 1 | @dataclass @@ -27,7 +27,7 @@ PIE807.py:4:49: PIE807 [*] Prefer `dict` over useless lambda 4 | bar: Dict[str, int] = field(default_factory=lambda: {}) # PIE807 | ^^^^^^^^^^ PIE807 | - = help: Replace with lambda with `dict` + = help: Replace with `lambda` with `dict` ℹ Safe fix 1 1 | @dataclass @@ -46,7 +46,7 @@ PIE807.py:8:36: PIE807 [*] Prefer `list` over useless lambda | ^^^^^^^^^^ PIE807 9 | bar = fields.ListField(default=lambda: {}) # PIE807 | - = help: Replace with lambda with `list` + = help: Replace with `lambda` with `list` ℹ Safe fix 5 5 | @@ -65,7 +65,7 @@ PIE807.py:9:36: PIE807 [*] Prefer `dict` over useless lambda 9 | bar = fields.ListField(default=lambda: {}) # PIE807 | ^^^^^^^^^^ PIE807 | - = help: Replace with lambda with `dict` + = help: Replace with `lambda` with `dict` ℹ Safe fix 6 6 | @@ -84,7 +84,7 @@ PIE807.py:13:28: PIE807 [*] Prefer `list` over useless lambda | ^^^^^^^^^^ PIE807 14 | bar = fields.ListField(default=lambda: {}) # PIE807 | - = help: Replace with lambda with `list` + = help: Replace with `lambda` with `list` ℹ Safe fix 10 10 | @@ -103,7 +103,7 @@ PIE807.py:14:36: PIE807 [*] Prefer `dict` over useless lambda 14 | bar = fields.ListField(default=lambda: {}) # PIE807 | ^^^^^^^^^^ PIE807 | - = help: Replace with lambda with `dict` + = help: Replace with `lambda` with `dict` ℹ Safe fix 11 11 | From 0ef31d4482268869824cea1343c973fb2fd65790 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 13 Nov 2023 12:46:27 -0500 Subject: [PATCH 5/6] Tweak docs --- .../flake8_pie/rules/reimplemented_container_builtin.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 index ffc06a6fdda6a..6f4a635998b00 100644 --- 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 @@ -15,7 +15,8 @@ use crate::checkers::ast::Checker; /// the `dict` builtin. /// /// ## Why is this bad? -/// Using container builtins is more readable. +/// Using container builtins are more succinct and idiomatic than wrapping +/// the literal in a lambda. /// /// ## Example /// ```python @@ -38,8 +39,6 @@ use crate::checkers::ast::Checker; /// baz: dict[str, int] = field(default_factory=dict) /// ``` /// -/// -/// If [preview] /// ## References /// - [Python documentation: `list`](https://docs.python.org/3/library/functions.html#func-list) /// From e14024e0bbc1a8f4cb71647c25002a070483778b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 13 Nov 2023 12:50:15 -0500 Subject: [PATCH 6/6] Clippy --- .../rules/flake8_pie/rules/reimplemented_container_builtin.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 index 6f4a635998b00..fbc001d9b304a 100644 --- 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 @@ -1,5 +1,4 @@ use ruff_python_ast::{self as ast, Expr, ExprLambda}; -use std::fmt::Write; use ruff_diagnostics::{Diagnostic, Edit, Fix}; use ruff_diagnostics::{FixAvailability, Violation}; @@ -102,7 +101,7 @@ enum Container { } impl Container { - fn as_str(&self) -> &'static str { + fn as_str(self) -> &'static str { match self { Container::List => "list", Container::Dict => "dict",