[flake8-pyi] Avoid syntax error in the case of starred and keyword arguments (PYI059)

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
+    ...

crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs

@@ -1,4 +1,6 @@
+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;
 
@@ -43,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)
 ///
@@ -94,6 +101,22 @@ 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.
+    if bases
+        .arguments_source_order()
+        .any(|arg| arg.value().is_starred_expr())
+    {
+        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));
@@ -109,12 +132,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]))
 }

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap

@@ -111,3 +111,53 @@ 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
+
+ℹ 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 |     ...
+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