From a3638b3adc42ca5b50fbe0dbff07ef23fe9e670d Mon Sep 17 00:00:00 2001 From: Robsdedude Date: Mon, 30 Jun 2025 13:42:05 +0000 Subject: [PATCH] [`pyupgrade`] Mark `UP008` fix safe if no comments in range (#18683) ## Summary Mark `UP008`'s fix safe if it won't delete comments. ## Relevant Issues Fixes: https://github.com/astral-sh/ruff/issues/18533 --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- .../test/fixtures/pyupgrade/UP008.py | 20 ++ crates/ruff_linter/src/preview.rs | 7 + crates/ruff_linter/src/rules/pyupgrade/mod.rs | 16 +- .../rules/super_call_with_parameters.rs | 25 +- ...er__rules__pyupgrade__tests__UP008.py.snap | 88 ++++++ ...__pyupgrade__tests__UP008.py__preview.snap | 251 ++++++++++++++++++ 6 files changed, 402 insertions(+), 5 deletions(-) create mode 100644 crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP008.py__preview.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP008.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP008.py index 5945913299..174906993d 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP008.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP008.py @@ -105,3 +105,23 @@ import builtins class C: def f(self): builtins.super(C, self) + + +# see: https://github.com/astral-sh/ruff/issues/18533 +class ClassForCommentEnthusiasts(BaseClass): + def with_comments(self): + super( + # super helpful comment + ClassForCommentEnthusiasts, + self + ).f() + super( + ClassForCommentEnthusiasts, + # even more helpful comment + self + ).f() + super( + ClassForCommentEnthusiasts, + self + # also a comment + ).f() diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index 3f826aee02..68d6e45b3f 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -104,3 +104,10 @@ pub(crate) const fn is_invalid_async_mock_access_check_enabled(settings: &Linter pub(crate) const fn is_raise_exception_byte_string_enabled(settings: &LinterSettings) -> bool { settings.preview.is_enabled() } + +// https://github.com/astral-sh/ruff/pull/18683 +pub(crate) const fn is_safe_super_call_with_parameters_fix_enabled( + settings: &LinterSettings, +) -> bool { + settings.preview.is_enabled() +} diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index e7864a85a1..76ff4b0983 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -15,7 +15,7 @@ mod tests { use crate::registry::Rule; use crate::rules::pyupgrade; - + use crate::settings::types::PreviewMode; use crate::test::test_path; use crate::{assert_diagnostics, settings}; @@ -122,6 +122,20 @@ mod tests { Ok(()) } + #[test_case(Rule::SuperCallWithParameters, Path::new("UP008.py"))] + fn rules_preview(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("{}__preview", path.to_string_lossy()); + let diagnostics = test_path( + Path::new("pyupgrade").join(path).as_path(), + &settings::LinterSettings { + preview: PreviewMode::Enabled, + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_diagnostics!(snapshot, diagnostics); + Ok(()) + } + #[test] fn async_timeout_error_alias_not_applied_py310() -> Result<()> { let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/super_call_with_parameters.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/super_call_with_parameters.rs index 6ea5ae5659..d75602914e 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/super_call_with_parameters.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/super_call_with_parameters.rs @@ -1,8 +1,10 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::{self as ast, Expr, Stmt}; use ruff_text_size::{Ranged, TextSize}; use crate::checkers::ast::Checker; +use crate::preview::is_safe_super_call_with_parameters_fix_enabled; use crate::{AlwaysFixableViolation, Edit, Fix}; /// ## What it does @@ -45,6 +47,10 @@ use crate::{AlwaysFixableViolation, Edit, Fix}; /// This rule's fix is marked as unsafe because removing the arguments from a call /// may delete comments that are attached to the arguments. /// +/// In [preview], the fix is marked safe if no comments are present. +/// +/// [preview]: https://docs.astral.sh/ruff/preview/ +/// /// ## References /// - [Python documentation: `super`](https://docs.python.org/3/library/functions.html#super) /// - [super/MRO, Python's most misunderstood feature.](https://www.youtube.com/watch?v=X1PQ7zzltz4) @@ -159,11 +165,22 @@ pub(crate) fn super_call_with_parameters(checker: &Checker, call: &ast::ExprCall return; } + let applicability = if !checker.comment_ranges().intersects(call.arguments.range()) + && is_safe_super_call_with_parameters_fix_enabled(checker.settings()) + { + Applicability::Safe + } else { + Applicability::Unsafe + }; + let mut diagnostic = checker.report_diagnostic(SuperCallWithParameters, call.arguments.range()); - diagnostic.set_fix(Fix::unsafe_edit(Edit::deletion( - call.arguments.start() + TextSize::new(1), - call.arguments.end() - TextSize::new(1), - ))); + diagnostic.set_fix(Fix::applicable_edit( + Edit::deletion( + call.arguments.start() + TextSize::new(1), + call.arguments.end() - TextSize::new(1), + ), + applicability, + )); } /// Returns `true` if a call is an argumented `super` invocation. diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP008.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP008.py.snap index aac7259e8a..0e742cce05 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP008.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP008.py.snap @@ -161,3 +161,91 @@ UP008.py:107:23: UP008 [*] Use `super()` instead of `super(__class__, self)` 106 106 | def f(self): 107 |- builtins.super(C, self) 107 |+ builtins.super() +108 108 | +109 109 | +110 110 | # see: https://github.com/astral-sh/ruff/issues/18533 + +UP008.py:113:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +111 | class ClassForCommentEnthusiasts(BaseClass): +112 | def with_comments(self): +113 | super( + | ______________^ +114 | | # super helpful comment +115 | | ClassForCommentEnthusiasts, +116 | | self +117 | | ).f() + | |_________^ UP008 +118 | super( +119 | ClassForCommentEnthusiasts, + | + = help: Remove `super()` parameters + +ℹ Unsafe fix +110 110 | # see: https://github.com/astral-sh/ruff/issues/18533 +111 111 | class ClassForCommentEnthusiasts(BaseClass): +112 112 | def with_comments(self): +113 |- super( +114 |- # super helpful comment +115 |- ClassForCommentEnthusiasts, +116 |- self +117 |- ).f() + 113 |+ super().f() +118 114 | super( +119 115 | ClassForCommentEnthusiasts, +120 116 | # even more helpful comment + +UP008.py:118:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +116 | self +117 | ).f() +118 | super( + | ______________^ +119 | | ClassForCommentEnthusiasts, +120 | | # even more helpful comment +121 | | self +122 | | ).f() + | |_________^ UP008 +123 | super( +124 | ClassForCommentEnthusiasts, + | + = help: Remove `super()` parameters + +ℹ Unsafe fix +115 115 | ClassForCommentEnthusiasts, +116 116 | self +117 117 | ).f() +118 |- super( +119 |- ClassForCommentEnthusiasts, +120 |- # even more helpful comment +121 |- self +122 |- ).f() + 118 |+ super().f() +123 119 | super( +124 120 | ClassForCommentEnthusiasts, +125 121 | self + +UP008.py:123:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +121 | self +122 | ).f() +123 | super( + | ______________^ +124 | | ClassForCommentEnthusiasts, +125 | | self +126 | | # also a comment +127 | | ).f() + | |_________^ UP008 + | + = help: Remove `super()` parameters + +ℹ Unsafe fix +120 120 | # even more helpful comment +121 121 | self +122 122 | ).f() +123 |- super( +124 |- ClassForCommentEnthusiasts, +125 |- self +126 |- # also a comment +127 |- ).f() + 123 |+ super().f() diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP008.py__preview.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP008.py__preview.snap new file mode 100644 index 0000000000..2f52d1313a --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP008.py__preview.snap @@ -0,0 +1,251 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- +UP008.py:17:23: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +16 | def wrong(self): +17 | parent = super(Child, self) # wrong + | ^^^^^^^^^^^^^ UP008 +18 | super(Child, self).method # wrong +19 | super( + | + = help: Remove `super()` parameters + +ℹ Safe fix +14 14 | Parent.super(1, 2) # ok +15 15 | +16 16 | def wrong(self): +17 |- parent = super(Child, self) # wrong + 17 |+ parent = super() # wrong +18 18 | super(Child, self).method # wrong +19 19 | super( +20 20 | Child, + +UP008.py:18:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +16 | def wrong(self): +17 | parent = super(Child, self) # wrong +18 | super(Child, self).method # wrong + | ^^^^^^^^^^^^^ UP008 +19 | super( +20 | Child, + | + = help: Remove `super()` parameters + +ℹ Safe fix +15 15 | +16 16 | def wrong(self): +17 17 | parent = super(Child, self) # wrong +18 |- super(Child, self).method # wrong + 18 |+ super().method # wrong +19 19 | super( +20 20 | Child, +21 21 | self, + +UP008.py:19:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +17 | parent = super(Child, self) # wrong +18 | super(Child, self).method # wrong +19 | super( + | ______________^ +20 | | Child, +21 | | self, +22 | | ).method() # wrong + | |_________^ UP008 + | + = help: Remove `super()` parameters + +ℹ Safe fix +16 16 | def wrong(self): +17 17 | parent = super(Child, self) # wrong +18 18 | super(Child, self).method # wrong +19 |- super( +20 |- Child, +21 |- self, +22 |- ).method() # wrong + 19 |+ super().method() # wrong +23 20 | +24 21 | +25 22 | class BaseClass: + +UP008.py:36:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +34 | class MyClass(BaseClass): +35 | def normal(self): +36 | super(MyClass, self).f() # can use super() + | ^^^^^^^^^^^^^^^ UP008 +37 | super().f() + | + = help: Remove `super()` parameters + +ℹ Safe fix +33 33 | +34 34 | class MyClass(BaseClass): +35 35 | def normal(self): +36 |- super(MyClass, self).f() # can use super() + 36 |+ super().f() # can use super() +37 37 | super().f() +38 38 | +39 39 | def different_argument(self, other): + +UP008.py:50:18: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +49 | def inner_argument(self): +50 | super(MyClass, self).f() # can use super() + | ^^^^^^^^^^^^^^^ UP008 +51 | super().f() + | + = help: Remove `super()` parameters + +ℹ Safe fix +47 47 | super(MyClass, self).f() # CANNOT use super() +48 48 | +49 49 | def inner_argument(self): +50 |- super(MyClass, self).f() # can use super() + 50 |+ super().f() # can use super() +51 51 | super().f() +52 52 | +53 53 | outer_argument() + +UP008.py:74:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +72 | class DataClass: +73 | def normal(self): +74 | super(DataClass, self).f() # Error + | ^^^^^^^^^^^^^^^^^ UP008 +75 | super().f() # OK + | + = help: Remove `super()` parameters + +ℹ Safe fix +71 71 | @dataclass +72 72 | class DataClass: +73 73 | def normal(self): +74 |- super(DataClass, self).f() # Error + 74 |+ super().f() # Error +75 75 | super().f() # OK +76 76 | +77 77 | + +UP008.py:92:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +90 | class B(A): +91 | def bar(self): +92 | super(__class__, self).foo() + | ^^^^^^^^^^^^^^^^^ UP008 + | + = help: Remove `super()` parameters + +ℹ Safe fix +89 89 | +90 90 | class B(A): +91 91 | def bar(self): +92 |- super(__class__, self).foo() + 92 |+ super().foo() +93 93 | +94 94 | +95 95 | # see: https://github.com/astral-sh/ruff/issues/18684 + +UP008.py:107:23: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +105 | class C: +106 | def f(self): +107 | builtins.super(C, self) + | ^^^^^^^^^ UP008 + | + = help: Remove `super()` parameters + +ℹ Safe fix +104 104 | +105 105 | class C: +106 106 | def f(self): +107 |- builtins.super(C, self) + 107 |+ builtins.super() +108 108 | +109 109 | +110 110 | # see: https://github.com/astral-sh/ruff/issues/18533 + +UP008.py:113:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +111 | class ClassForCommentEnthusiasts(BaseClass): +112 | def with_comments(self): +113 | super( + | ______________^ +114 | | # super helpful comment +115 | | ClassForCommentEnthusiasts, +116 | | self +117 | | ).f() + | |_________^ UP008 +118 | super( +119 | ClassForCommentEnthusiasts, + | + = help: Remove `super()` parameters + +ℹ Unsafe fix +110 110 | # see: https://github.com/astral-sh/ruff/issues/18533 +111 111 | class ClassForCommentEnthusiasts(BaseClass): +112 112 | def with_comments(self): +113 |- super( +114 |- # super helpful comment +115 |- ClassForCommentEnthusiasts, +116 |- self +117 |- ).f() + 113 |+ super().f() +118 114 | super( +119 115 | ClassForCommentEnthusiasts, +120 116 | # even more helpful comment + +UP008.py:118:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +116 | self +117 | ).f() +118 | super( + | ______________^ +119 | | ClassForCommentEnthusiasts, +120 | | # even more helpful comment +121 | | self +122 | | ).f() + | |_________^ UP008 +123 | super( +124 | ClassForCommentEnthusiasts, + | + = help: Remove `super()` parameters + +ℹ Unsafe fix +115 115 | ClassForCommentEnthusiasts, +116 116 | self +117 117 | ).f() +118 |- super( +119 |- ClassForCommentEnthusiasts, +120 |- # even more helpful comment +121 |- self +122 |- ).f() + 118 |+ super().f() +123 119 | super( +124 120 | ClassForCommentEnthusiasts, +125 121 | self + +UP008.py:123:14: UP008 [*] Use `super()` instead of `super(__class__, self)` + | +121 | self +122 | ).f() +123 | super( + | ______________^ +124 | | ClassForCommentEnthusiasts, +125 | | self +126 | | # also a comment +127 | | ).f() + | |_________^ UP008 + | + = help: Remove `super()` parameters + +ℹ Unsafe fix +120 120 | # even more helpful comment +121 121 | self +122 122 | ).f() +123 |- super( +124 |- ClassForCommentEnthusiasts, +125 |- self +126 |- # also a comment +127 |- ).f() + 123 |+ super().f()