From 793f552f5154aaba1015be21f69e07ee8e62ed4d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 9 Jun 2025 22:49:44 -0400 Subject: [PATCH 1/9] add failing tests --- .../resources/test/fixtures/flake8_pyi/PYI059.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI059.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI059.py index 0e4d877d4b485d..cfe42a12d38367 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI059.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI059.py @@ -52,3 +52,15 @@ def __init__(self) -> None: class SomeGeneric(Generic[T]): # Only one generic pass + + +# syntax errors with starred and keyword arguments from +# https://github.com/astral-sh/ruff/issues/18602 +class C1(Generic[T], str, **{"metaclass": type}): # PYI059 + ... + +class C2(Generic[T], str, metaclass=type): # PYI059 + ... + +class C3(Generic[T], metaclass=type, *[str]): # PYI059 but no fix + ... From ee7d834f12914afcf41a8134b90b37b050f57415 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 10 Jun 2025 09:09:25 -0400 Subject: [PATCH 2/9] avoid a fix for starred arguments --- .../rules/generic_not_last_base_class.rs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs index b7ab81f3d315b0..9d435eebd1c7d5 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs @@ -1,4 +1,5 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ArgOrKeyword; use ruff_python_ast::{self as ast, helpers::map_subscript}; use ruff_text_size::Ranged; @@ -94,6 +95,25 @@ pub(crate) fn generic_not_last_base_class(checker: &Checker, class_def: &ast::St let mut diagnostic = checker.report_diagnostic(GenericNotLastBaseClass, bases.range()); + // Avoid suggesting a fix if any of the arguments is starred. This avoids tricky syntax errors + // in cases like + // + // ```python + // class C3(Generic[T], metaclass=type, *[str]): ... + // ``` + // + // where we would naively try to put `Generic[T]` after `*[str]`, which is also after a keyword + // argument, causing the error. + // + // This also filters out keyword unpacking like `**{"metaclass": type}`, identified by a keyword + // with no name. + if bases.arguments_source_order().any(|arg| { + arg.value().is_starred_expr() + || matches!(arg, ArgOrKeyword::Keyword(ast::Keyword { arg: None, .. })) + }) { + return; + } + // No fix if multiple `Generic[]`s are seen in the class bases. if generic_base_iter.next().is_none() { diagnostic.try_set_fix(|| generate_fix(generic_base, bases, checker)); From 18bc625e7d399c7e42f034f209c3d5f700a4ee32 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 10 Jun 2025 09:25:11 -0400 Subject: [PATCH 3/9] handle insertions before the last keyword argument --- .../rules/generic_not_last_base_class.rs | 22 ++++++---- ...__flake8_pyi__tests__PYI059_PYI059.py.snap | 40 +++++++++++++++++++ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs index 9d435eebd1c7d5..6a0a90d777b4b7 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs @@ -1,5 +1,6 @@ +use ruff_diagnostics::Edit; use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ArgOrKeyword; +use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{self as ast, helpers::map_subscript}; use ruff_text_size::Ranged; @@ -129,12 +130,19 @@ fn generate_fix( let source = locator.contents(); let deletion = remove_argument(generic_base, arguments, Parentheses::Preserve, source)?; - let insertion = add_argument( - locator.slice(generic_base), - arguments, - checker.comment_ranges(), - source, - ); + + let argument = locator.slice(generic_base); + let comment_ranges = checker.comment_ranges(); + + // adapted from `add_argument`, which doesn't automatically handle inserting before the first + // keyword argument. + let insertion = if let Some(ast::Keyword { range, value, .. }) = arguments.keywords.first() { + let keyword = parenthesized_range(value.into(), arguments.into(), comment_ranges, source) + .unwrap_or(*range); + Edit::insertion(format!("{argument}, "), keyword.start()) + } else { + add_argument(argument, arguments, comment_ranges, source) + }; Ok(Fix::safe_edits(deletion, [insertion])) } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap index bc91fb867cef4d..b00b69948271c1 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap @@ -111,3 +111,43 @@ PYI059.py:45:8: PYI059 `Generic[]` should always be the last base class | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI059 | = help: Move `Generic[]` to the end + +PYI059.py:59:9: PYI059 `Generic[]` should always be the last base class + | +57 | # syntax errors with starred and keyword arguments from +58 | # https://github.com/astral-sh/ruff/issues/18602 +59 | class C1(Generic[T], str, **{"metaclass": type}): # PYI059 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI059 +60 | ... + | + = help: Move `Generic[]` to the end + +PYI059.py:62:9: PYI059 [*] `Generic[]` should always be the last base class + | +60 | ... +61 | +62 | class C2(Generic[T], str, metaclass=type): # PYI059 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI059 +63 | ... + | + = help: Move `Generic[]` to the end + +ℹ Safe fix +59 59 | class C1(Generic[T], str, **{"metaclass": type}): # PYI059 +60 60 | ... +61 61 | +62 |-class C2(Generic[T], str, metaclass=type): # PYI059 + 62 |+class C2(str, Generic[T], metaclass=type): # PYI059 +63 63 | ... +64 64 | +65 65 | class C3(Generic[T], metaclass=type, *[str]): # PYI059 but no fix + +PYI059.py:65:9: PYI059 `Generic[]` should always be the last base class + | +63 | ... +64 | +65 | class C3(Generic[T], metaclass=type, *[str]): # PYI059 but no fix + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI059 +66 | ... + | + = help: Move `Generic[]` to the end From c240c13d5e287daa5ffc3c80583520ad29b40212 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 10 Jun 2025 09:25:47 -0400 Subject: [PATCH 4/9] remove special **kwarg handling, falls out of other fix --- .../flake8_pyi/rules/generic_not_last_base_class.rs | 11 ++++------- ...__rules__flake8_pyi__tests__PYI059_PYI059.py.snap | 12 +++++++++++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs index 6a0a90d777b4b7..6044defd54a5ed 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs @@ -105,13 +105,10 @@ pub(crate) fn generic_not_last_base_class(checker: &Checker, class_def: &ast::St // // where we would naively try to put `Generic[T]` after `*[str]`, which is also after a keyword // argument, causing the error. - // - // This also filters out keyword unpacking like `**{"metaclass": type}`, identified by a keyword - // with no name. - if bases.arguments_source_order().any(|arg| { - arg.value().is_starred_expr() - || matches!(arg, ArgOrKeyword::Keyword(ast::Keyword { arg: None, .. })) - }) { + if bases + .arguments_source_order() + .any(|arg| arg.value().is_starred_expr()) + { return; } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap index b00b69948271c1..2dddc007073072 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap @@ -112,7 +112,7 @@ PYI059.py:45:8: PYI059 `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -PYI059.py:59:9: PYI059 `Generic[]` should always be the last base class +PYI059.py:59:9: PYI059 [*] `Generic[]` should always be the last base class | 57 | # syntax errors with starred and keyword arguments from 58 | # https://github.com/astral-sh/ruff/issues/18602 @@ -122,6 +122,16 @@ PYI059.py:59:9: PYI059 `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end +ℹ Safe fix +56 56 | +57 57 | # syntax errors with starred and keyword arguments from +58 58 | # https://github.com/astral-sh/ruff/issues/18602 +59 |-class C1(Generic[T], str, **{"metaclass": type}): # PYI059 + 59 |+class C1(str, Generic[T], **{"metaclass": type}): # PYI059 +60 60 | ... +61 61 | +62 62 | class C2(Generic[T], str, metaclass=type): # PYI059 + PYI059.py:62:9: PYI059 [*] `Generic[]` should always be the last base class | 60 | ... From 41a65ec06748a3e4070538b6ea2c37cc6f34e06b Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 10 Jun 2025 09:31:02 -0400 Subject: [PATCH 5/9] add a brief Fix availability section --- .../rules/flake8_pyi/rules/generic_not_last_base_class.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs index 6044defd54a5ed..2116a897972e0c 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs @@ -45,6 +45,11 @@ use crate::{Fix, FixAvailability, Violation}; /// ): /// ... /// ``` +/// +/// ## Fix availability +/// +/// This rule's fix is only available when there are no `*args` present in the base class list. +/// /// ## References /// - [`typing.Generic` documentation](https://docs.python.org/3/library/typing.html#typing.Generic) /// From 6f5fad9dbc2736169627656f3f649df922eaee37 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 10 Jun 2025 11:49:27 -0400 Subject: [PATCH 6/9] copy over docs improvements from #18601 Co-authored-by: Alex Waygood --- .../rules/flake8_pyi/rules/generic_not_last_base_class.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs index 2116a897972e0c..97f78bf623bfa9 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs @@ -15,8 +15,11 @@ use crate::{Fix, FixAvailability, Violation}; /// ## Why is this bad? /// If `Generic[]` is not the final class in the bases tuple, unexpected /// behaviour can occur at runtime (See [this CPython issue][1] for an example). -/// The rule is also applied to stub files, but, unlike at runtime, -/// in stubs it is purely enforced for stylistic consistency. +/// +/// The rule is also applied to stub files, where it won't cause issues at +/// runtime. This is because type checkers may not be able to infer an +/// accurate [MRO] for the class, which could lead to unexpected or +/// inaccurate results when they analyze your code. /// /// For example: /// ```python @@ -54,6 +57,7 @@ use crate::{Fix, FixAvailability, Violation}; /// - [`typing.Generic` documentation](https://docs.python.org/3/library/typing.html#typing.Generic) /// /// [1]: https://github.com/python/cpython/issues/106102 +/// [MRO]: https://docs.python.org/3/glossary.html#term-method-resolution-order #[derive(ViolationMetadata)] pub(crate) struct GenericNotLastBaseClass; From 258257cdff0dfb812fffad6a77e99012f6cb2d03 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 10 Jun 2025 11:53:51 -0400 Subject: [PATCH 7/9] make the fix always unsafe and add fix safety docs --- .../flake8_pyi/rules/generic_not_last_base_class.rs | 9 ++++++++- ...__rules__flake8_pyi__tests__PYI059_PYI059.py.snap | 12 ++++++------ ..._rules__flake8_pyi__tests__PYI059_PYI059.pyi.snap | 8 ++++---- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs index 97f78bf623bfa9..88f5bb9d3a0249 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs @@ -49,6 +49,13 @@ use crate::{Fix, FixAvailability, Violation}; /// ... /// ``` /// +/// ## Fix safety +/// +/// This rule's fix is always unsafe because reordering base classes can change +/// the behavior of the code by modifying the class's MRO. The fix will also +/// delete trailing comments after the `Generic` base class in multi-line base +/// class lists, if any are present. +/// /// ## Fix availability /// /// This rule's fix is only available when there are no `*args` present in the base class list. @@ -150,5 +157,5 @@ fn generate_fix( add_argument(argument, arguments, comment_ranges, source) }; - Ok(Fix::safe_edits(deletion, [insertion])) + Ok(Fix::unsafe_edits(deletion, [insertion])) } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap index 2dddc007073072..d5f7860983c0cf 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap @@ -12,7 +12,7 @@ PYI059.py:8:17: PYI059 [*] `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -ℹ Safe fix +ℹ Unsafe fix 5 5 | K = TypeVar('K') 6 6 | V = TypeVar('V') 7 7 | @@ -37,7 +37,7 @@ PYI059.py:15:16: PYI059 [*] `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -ℹ Safe fix +ℹ Unsafe fix 13 13 | self._items.append(item) 14 14 | 15 15 | class MyMapping( # PYI059 @@ -59,7 +59,7 @@ PYI059.py:26:10: PYI059 [*] `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -ℹ Safe fix +ℹ Unsafe fix 23 23 | # Inheriting from just `Generic` is a TypeError, but it's probably fine 24 24 | # to flag this issue in this case as well, since after fixing the error 25 25 | # the Generic's position issue persists. @@ -88,7 +88,7 @@ PYI059.py:30:10: PYI059 [*] `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -ℹ Safe fix +ℹ Unsafe fix 30 30 | class Foo( # comment about the bracket 31 31 | # Part 1 of multiline comment 1 32 32 | # Part 2 of multiline comment 1 @@ -122,7 +122,7 @@ PYI059.py:59:9: PYI059 [*] `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -ℹ Safe fix +ℹ Unsafe fix 56 56 | 57 57 | # syntax errors with starred and keyword arguments from 58 58 | # https://github.com/astral-sh/ruff/issues/18602 @@ -142,7 +142,7 @@ PYI059.py:62:9: PYI059 [*] `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -ℹ Safe fix +ℹ Unsafe fix 59 59 | class C1(Generic[T], str, **{"metaclass": type}): # PYI059 60 60 | ... 61 61 | diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.pyi.snap index 275dd20dbaa087..16c5c72d087055 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.pyi.snap @@ -12,7 +12,7 @@ PYI059.pyi:8:17: PYI059 [*] `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -ℹ Safe fix +ℹ Unsafe fix 5 5 | K = TypeVar('K') 6 6 | V = TypeVar('V') 7 7 | @@ -37,7 +37,7 @@ PYI059.pyi:12:16: PYI059 [*] `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -ℹ Safe fix +ℹ Unsafe fix 10 10 | def push(self, item: T) -> None: ... 11 11 | 12 12 | class MyMapping( # PYI059 @@ -58,7 +58,7 @@ PYI059.pyi:22:10: PYI059 [*] `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -ℹ Safe fix +ℹ Unsafe fix 19 19 | # Inheriting from just `Generic` is a TypeError, but it's probably fine 20 20 | # to flag this issue in this case as well, since after fixing the error 21 21 | # the Generic's position issue persists. @@ -87,7 +87,7 @@ PYI059.pyi:25:10: PYI059 [*] `Generic[]` should always be the last base class | = help: Move `Generic[]` to the end -ℹ Safe fix +ℹ Unsafe fix 25 25 | class Foo( # comment about the bracket 26 26 | # Part 1 of multiline comment 1 27 27 | # Part 2 of multiline comment 1 From 6815c68e63210a73958247775f5164746807f081 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 10 Jun 2025 12:01:10 -0400 Subject: [PATCH 8/9] update `add_argument` to handle kwargs always --- crates/ruff_linter/src/fix/edits.rs | 16 ++++++++++------ ...les__flake8_bugbear__tests__B028_B028.py.snap | 9 ++++----- .../rules/generic_not_last_base_class.rs | 10 +--------- ..._PLW1510_subprocess_run_without_check.py.snap | 6 +++--- 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/crates/ruff_linter/src/fix/edits.rs b/crates/ruff_linter/src/fix/edits.rs index 7d324380229850..2ed6f4c9cf7e3f 100644 --- a/crates/ruff_linter/src/fix/edits.rs +++ b/crates/ruff_linter/src/fix/edits.rs @@ -2,9 +2,9 @@ use anyhow::{Context, Result}; +use ruff_python_ast::AnyNodeRef; use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Expr, ExprList, Parameters, Stmt}; -use ruff_python_ast::{AnyNodeRef, ArgOrKeyword}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; use ruff_python_trivia::textwrap::dedent_to; @@ -257,19 +257,23 @@ pub(crate) fn remove_argument( } /// Generic function to add arguments or keyword arguments to function calls. +/// +/// The new argument will be inserted before the first existing keyword argument in `arguments`, if +/// there are any present. Otherwise, the new argument is added to the end of the argument list. pub(crate) fn add_argument( argument: &str, arguments: &Arguments, comment_ranges: &CommentRanges, source: &str, ) -> Edit { - if let Some(last) = arguments.arguments_source_order().last() { + if let Some(ast::Keyword { range, value, .. }) = arguments.keywords.first() { + let keyword = parenthesized_range(value.into(), arguments.into(), comment_ranges, source) + .unwrap_or(*range); + Edit::insertion(format!("{argument}, "), keyword.start()) + } else if let Some(last) = arguments.arguments_source_order().last() { // Case 1: existing arguments, so append after the last argument. let last = parenthesized_range( - match last { - ArgOrKeyword::Arg(arg) => arg.into(), - ArgOrKeyword::Keyword(keyword) => (&keyword.value).into(), - }, + last.value().into(), arguments.into(), comment_ranges, source, diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B028_B028.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B028_B028.py.snap index db323efe66afbd..ddaf48098f7574 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B028_B028.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B028_B028.py.snap @@ -37,11 +37,10 @@ B028.py:9:1: B028 [*] No explicit `stacklevel` keyword argument found 7 7 | 8 8 | warnings.warn("test", DeprecationWarning) 9 |-warnings.warn("test", DeprecationWarning, source=None) -10 9 | warnings.warn("test", DeprecationWarning, source=None, stacklevel=2) - 10 |+warnings.warn("test", DeprecationWarning, source=None, stacklevel=2) + 9 |+warnings.warn("test", DeprecationWarning, stacklevel=2, source=None) +10 10 | warnings.warn("test", DeprecationWarning, source=None, stacklevel=2) 11 11 | warnings.warn("test", DeprecationWarning, stacklevel=1) 12 12 | warnings.warn("test", DeprecationWarning, 1) -13 13 | warnings.warn("test", category=DeprecationWarning, stacklevel=1) B028.py:22:1: B028 [*] No explicit `stacklevel` keyword argument found | @@ -59,7 +58,7 @@ B028.py:22:1: B028 [*] No explicit `stacklevel` keyword argument found 24 24 | DeprecationWarning, 25 25 | # some comments here 26 |- source = None # no trailing comma - 26 |+ source = None, stacklevel=2 # no trailing comma + 26 |+ stacklevel=2, source = None # no trailing comma 27 27 | ) 28 28 | 29 29 | # https://github.com/astral-sh/ruff/issues/18011 @@ -80,7 +79,7 @@ B028.py:32:1: B028 [*] No explicit `stacklevel` keyword argument found 30 30 | warnings.warn("test", skip_file_prefixes=(os.path.dirname(__file__),)) 31 31 | # trigger diagnostic if `skip_file_prefixes` is present and set to the default value 32 |-warnings.warn("test", skip_file_prefixes=()) - 32 |+warnings.warn("test", skip_file_prefixes=(), stacklevel=2) + 32 |+warnings.warn("test", stacklevel=2, skip_file_prefixes=()) 33 33 | 34 34 | _my_prefixes = ("this","that") 35 35 | warnings.warn("test", skip_file_prefixes = _my_prefixes) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs index 88f5bb9d3a0249..c82132c4027d0a 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs @@ -1,6 +1,4 @@ -use ruff_diagnostics::Edit; use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{self as ast, helpers::map_subscript}; use ruff_text_size::Ranged; @@ -149,13 +147,7 @@ fn generate_fix( // adapted from `add_argument`, which doesn't automatically handle inserting before the first // keyword argument. - let insertion = if let Some(ast::Keyword { range, value, .. }) = arguments.keywords.first() { - let keyword = parenthesized_range(value.into(), arguments.into(), comment_ranges, source) - .unwrap_or(*range); - Edit::insertion(format!("{argument}, "), keyword.start()) - } else { - add_argument(argument, arguments, comment_ranges, source) - }; + let insertion = add_argument(argument, arguments, comment_ranges, source); Ok(Fix::unsafe_edits(deletion, [insertion])) } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1510_subprocess_run_without_check.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1510_subprocess_run_without_check.py.snap index 7e6539ec04c0b1..b0d1645a5d07fd 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1510_subprocess_run_without_check.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1510_subprocess_run_without_check.py.snap @@ -37,7 +37,7 @@ subprocess_run_without_check.py:5:1: PLW1510 [*] `subprocess.run` without explic 3 3 | # Errors. 4 4 | subprocess.run("ls") 5 |-subprocess.run("ls", shell=True) - 5 |+subprocess.run("ls", shell=True, check=False) + 5 |+subprocess.run("ls", check=False, shell=True) 6 6 | subprocess.run( 7 7 | ["ls"], 8 8 | shell=False, @@ -58,7 +58,7 @@ subprocess_run_without_check.py:6:1: PLW1510 [*] `subprocess.run` without explic 6 6 | subprocess.run( 7 7 | ["ls"], 8 |- shell=False, - 8 |+ shell=False, check=False, + 8 |+ check=False, shell=False, 9 9 | ) 10 10 | subprocess.run(["ls"], **kwargs) 11 11 | @@ -79,7 +79,7 @@ subprocess_run_without_check.py:10:1: PLW1510 [*] `subprocess.run` without expli 8 8 | shell=False, 9 9 | ) 10 |-subprocess.run(["ls"], **kwargs) - 10 |+subprocess.run(["ls"], **kwargs, check=False) + 10 |+subprocess.run(["ls"], check=False, **kwargs) 11 11 | 12 12 | # Non-errors. 13 13 | subprocess.run("ls", check=True) From aa9803bcbb0c4bd779733c6b1b65c847fba94aa2 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 10 Jun 2025 12:05:05 -0400 Subject: [PATCH 9/9] restore original generate_fix implementation --- .../flake8_pyi/rules/generic_not_last_base_class.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs index c82132c4027d0a..4d40bfafd94d07 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs @@ -141,13 +141,12 @@ fn generate_fix( let source = locator.contents(); let deletion = remove_argument(generic_base, arguments, Parentheses::Preserve, source)?; - - let argument = locator.slice(generic_base); - let comment_ranges = checker.comment_ranges(); - - // adapted from `add_argument`, which doesn't automatically handle inserting before the first - // keyword argument. - let insertion = add_argument(argument, arguments, comment_ranges, source); + let insertion = add_argument( + locator.slice(generic_base), + arguments, + checker.comment_ranges(), + source, + ); Ok(Fix::unsafe_edits(deletion, [insertion])) }