Handle parenthesized arguments in remove_argument (#18805)

Co-authored-by: Alex Waygood <alex.waygood@gmail.com>

crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE808.py

@@ -14,3 +14,6 @@ range(0, 10, 1)
 range(0, 10, step=1)
 range(start=0, stop=10)
 range(0, stop=10)
+
+# regression test for https://github.com/astral-sh/ruff/pull/18805
+range((0), 42)

crates/ruff_linter/resources/test/fixtures/ruff/RUF056.py

@@ -185,3 +185,7 @@ not my_dict.get(
 # TypeError is raised at runtime before and after the fix, but we still bail
 # out for having an unrecognized number of arguments
 not my_dict.get("key", False, foo=...)
+
+# https://github.com/astral-sh/ruff/issues/18798
+d = {}
+not d.get("key", (False))

crates/ruff_linter/src/fix/edits.rs

@@ -209,6 +209,7 @@ pub(crate) fn remove_argument<T: Ranged>(
     arguments: &Arguments,
     parentheses: Parentheses,
     source: &str,
+    comment_ranges: &CommentRanges,
 ) -> Result<Edit> {
     // Partition into arguments before and after the argument to remove.
     let (before, after): (Vec<_>, Vec<_>) = arguments
@@ -217,6 +218,15 @@ pub(crate) fn remove_argument<T: Ranged>(
         .filter(|range| argument.range() != *range)
         .partition(|range| range.start() < argument.start());
 
+    let arg = arguments
+        .arguments_source_order()
+        .find(|arg| arg.range() == argument.range())
+        .context("Unable to find argument")?;
+
+    let parenthesized_range =
+        parenthesized_range(arg.value().into(), arguments.into(), comment_ranges, source)
+            .unwrap_or(arg.range());
+
     if !after.is_empty() {
         // Case 1: argument or keyword is _not_ the last node, so delete from the start of the
         // argument to the end of the subsequent comma.
@@ -234,7 +244,7 @@ pub(crate) fn remove_argument<T: Ranged>(
             })
             .context("Unable to find next token")?;
 
-        Ok(Edit::deletion(argument.start(), next.start()))
+        Ok(Edit::deletion(parenthesized_range.start(), next.start()))
     } else if let Some(previous) = before.iter().map(Ranged::end).max() {
         // Case 2: argument or keyword is the last node, so delete from the start of the
         // previous comma to the end of the argument.
@@ -245,7 +255,7 @@ pub(crate) fn remove_argument<T: Ranged>(
             .find(|token| token.kind == SimpleTokenKind::Comma)
             .context("Unable to find trailing comma")?;
 
-        Ok(Edit::deletion(comma.start(), argument.end()))
+        Ok(Edit::deletion(comma.start(), parenthesized_range.end()))
     } else {
         // Case 3: argument or keyword is the only node, so delete the arguments (but preserve
         // parentheses, if needed).

crates/ruff_linter/src/rules/fastapi/rules/fastapi_redundant_response_model.rs

@@ -91,6 +91,7 @@ pub(crate) fn fastapi_redundant_response_model(checker: &Checker, function_def:
                 &call.arguments,
                 Parentheses::Preserve,
                 checker.locator().contents(),
+                checker.comment_ranges(),
             )
             .map(Fix::unsafe_edit)
         });

crates/ruff_linter/src/rules/flake8_logging/rules/exc_info_outside_except_handler.rs

@@ -114,7 +114,13 @@ pub(crate) fn exc_info_outside_except_handler(checker: &Checker, call: &ExprCall
     let mut diagnostic = checker.report_diagnostic(ExcInfoOutsideExceptHandler, exc_info.range);
 
     diagnostic.try_set_fix(|| {
-        let edit = remove_argument(exc_info, arguments, Parentheses::Preserve, source)?;
+        let edit = remove_argument(
+            exc_info,
+            arguments,
+            Parentheses::Preserve,
+            source,
+            checker.comment_ranges(),
+        )?;
         Ok(Fix::unsafe_edit(edit))
     });
 }

crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs

@@ -129,6 +129,7 @@ pub(crate) fn unnecessary_dict_kwargs(checker: &Checker, call: &ast::ExprCall) {
                     &call.arguments,
                     Parentheses::Preserve,
                     checker.locator().contents(),
+                    checker.comment_ranges(),
                 )
                 .map(Fix::safe_edit)
             });

crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_range_start.rs

@@ -76,6 +76,7 @@ pub(crate) fn unnecessary_range_start(checker: &Checker, call: &ast::ExprCall) {
             &call.arguments,
             Parentheses::Preserve,
             checker.locator().contents(),
+            checker.comment_ranges(),
         )
         .map(Fix::safe_edit)
     });

crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE808_PIE808.py.snap

@@ -38,3 +38,18 @@ PIE808.py:5:16: PIE808 [*] Unnecessary `start` argument in `range`
 6 6 | 
 7 7 | # OK
 8 8 | range(x, 10)
+
+PIE808.py:19:8: PIE808 [*] Unnecessary `start` argument in `range`
+   |
+18 | # regression test for https://github.com/astral-sh/ruff/pull/18805
+19 | range((0), 42)
+   |        ^ PIE808
+   |
+   = help: Remove `start` argument
+
+ℹ Safe fix
+16 16 | range(0, stop=10)
+17 17 | 
+18 18 | # regression test for https://github.com/astral-sh/ruff/pull/18805
+19    |-range((0), 42)
+   19 |+range(42)

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

@@ -140,7 +140,13 @@ fn generate_fix(
     let locator = checker.locator();
     let source = locator.contents();
 
-    let deletion = remove_argument(generic_base, arguments, Parentheses::Preserve, source)?;
+    let deletion = remove_argument(
+        generic_base,
+        arguments,
+        Parentheses::Preserve,
+        source,
+        checker.comment_ranges(),
+    )?;
     let insertion = add_argument(
         locator.slice(generic_base),
         arguments,

crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs

@@ -761,6 +761,7 @@ fn check_fixture_decorator(checker: &Checker, func_name: &str, decorator: &Decor
                                 arguments,
                                 edits::Parentheses::Preserve,
                                 checker.locator().contents(),
+                                checker.comment_ranges(),
                             )
                             .map(Fix::unsafe_edit)
                         });

crates/ruff_linter/src/rules/flake8_use_pathlib/rules/path_constructor_current_directory.rs

@@ -107,7 +107,13 @@ pub(crate) fn path_constructor_current_directory(checker: &Checker, call: &ExprC
             diagnostic.set_fix(Fix::applicable_edit(edit, applicability(parent_range)));
         }
         None => diagnostic.try_set_fix(|| {
-            let edit = remove_argument(arg, arguments, Parentheses::Preserve, checker.source())?;
+            let edit = remove_argument(
+                arg,
+                arguments,
+                Parentheses::Preserve,
+                checker.source(),
+                checker.comment_ranges(),
+            )?;
             Ok(Fix::applicable_edit(edit, applicability(call.range())))
         }),
     }

crates/ruff_linter/src/rules/pandas_vet/rules/inplace_argument.rs

@@ -137,6 +137,7 @@ fn convert_inplace_argument_to_assignment(
         &call.arguments,
         Parentheses::Preserve,
         locator.contents(),
+        comment_ranges,
     )
     .ok()?;
 

crates/ruff_linter/src/rules/pylint/rules/duplicate_bases.rs

@@ -80,6 +80,7 @@ pub(crate) fn duplicate_bases(checker: &Checker, name: &str, arguments: Option<&
                         arguments,
                         Parentheses::Remove,
                         checker.locator().contents(),
+                        checker.comment_ranges(),
                     )
                     .map(Fix::safe_edit)
                 });

crates/ruff_linter/src/rules/pyupgrade/rules/pep695/non_pep695_generic_class.rs

@@ -203,6 +203,7 @@ pub(crate) fn non_pep695_generic_class(checker: &Checker, class_def: &StmtClassD
                 arguments,
                 Parentheses::Remove,
                 checker.source(),
+                checker.comment_ranges(),
             )?;
             Ok(Fix::unsafe_edits(
                 Edit::insertion(type_params.to_string(), name.end()),

crates/ruff_linter/src/rules/pyupgrade/rules/replace_stdout_stderr.rs

@@ -3,6 +3,7 @@ use anyhow::Result;
 use ruff_macros::{ViolationMetadata, derive_message_formats};
 use ruff_python_ast::{self as ast, Keyword};
 use ruff_python_semantic::Modules;
+use ruff_python_trivia::CommentRanges;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
@@ -96,8 +97,15 @@ pub(crate) fn replace_stdout_stderr(checker: &Checker, call: &ast::ExprCall) {
 
         let mut diagnostic = checker.report_diagnostic(ReplaceStdoutStderr, call.range());
         if call.arguments.find_keyword("capture_output").is_none() {
-            diagnostic
+            diagnostic.try_set_fix(|| {
-                .try_set_fix(|| generate_fix(stdout, stderr, call, checker.locator().contents()));
+                generate_fix(
+                    stdout,
+                    stderr,
+                    call,
+                    checker.locator().contents(),
+                    checker.comment_ranges(),
+                )
+            });
         }
     }
 }
@@ -108,6 +116,7 @@ fn generate_fix(
     stderr: &Keyword,
     call: &ast::ExprCall,
     source: &str,
+    comment_ranges: &CommentRanges,
 ) -> Result<Fix> {
     let (first, second) = if stdout.start() < stderr.start() {
         (stdout, stderr)
@@ -122,6 +131,7 @@ fn generate_fix(
             &call.arguments,
             Parentheses::Preserve,
             source,
+            comment_ranges,
         )?],
     ))
 }

crates/ruff_linter/src/rules/pyupgrade/rules/replace_universal_newlines.rs

@@ -76,6 +76,7 @@ pub(crate) fn replace_universal_newlines(checker: &Checker, call: &ast::ExprCall
                     &call.arguments,
                     Parentheses::Preserve,
                     checker.locator().contents(),
+                    checker.comment_ranges(),
                 )
                 .map(Fix::safe_edit)
             });

crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_encode_utf8.rs

@@ -187,6 +187,7 @@ pub(crate) fn unnecessary_encode_utf8(checker: &Checker, call: &ast::ExprCall) {
                             &call.arguments,
                             Parentheses::Preserve,
                             checker.locator().contents(),
+                            checker.comment_ranges(),
                         )
                         .map(Fix::safe_edit)
                     });
@@ -204,6 +205,7 @@ pub(crate) fn unnecessary_encode_utf8(checker: &Checker, call: &ast::ExprCall) {
                             &call.arguments,
                             Parentheses::Preserve,
                             checker.locator().contents(),
+                            checker.comment_ranges(),
                         )
                         .map(Fix::safe_edit)
                     });
@@ -228,6 +230,7 @@ pub(crate) fn unnecessary_encode_utf8(checker: &Checker, call: &ast::ExprCall) {
                             &call.arguments,
                             Parentheses::Preserve,
                             checker.locator().contents(),
+                            checker.comment_ranges(),
                         )
                         .map(Fix::safe_edit)
                     });
@@ -245,6 +248,7 @@ pub(crate) fn unnecessary_encode_utf8(checker: &Checker, call: &ast::ExprCall) {
                             &call.arguments,
                             Parentheses::Preserve,
                             checker.locator().contents(),
+                            checker.comment_ranges(),
                         )
                         .map(Fix::safe_edit)
                     });

crates/ruff_linter/src/rules/pyupgrade/rules/useless_class_metaclass_type.rs

@@ -69,6 +69,7 @@ pub(crate) fn useless_class_metaclass_type(checker: &Checker, class_def: &StmtCl
                         arguments,
                         Parentheses::Remove,
                         checker.locator().contents(),
+                        checker.comment_ranges(),
                     )?;
 
                     let range = edit.range();

crates/ruff_linter/src/rules/pyupgrade/rules/useless_object_inheritance.rs

@@ -69,6 +69,7 @@ pub(crate) fn useless_object_inheritance(checker: &Checker, class_def: &ast::Stm
                 arguments,
                 Parentheses::Remove,
                 checker.locator().contents(),
+                checker.comment_ranges(),
             )?;
 
             let range = edit.range();

crates/ruff_linter/src/rules/ruff/rules/class_with_mixed_type_vars.rs

@@ -157,8 +157,13 @@ fn convert_type_vars(
         source,
     };
 
-    let remove_generic_base =
+    let remove_generic_base = remove_argument(
-        remove_argument(generic_base, class_arguments, Parentheses::Remove, source)?;
+        generic_base,
+        class_arguments,
+        Parentheses::Remove,
+        source,
+        checker.comment_ranges(),
+    )?;
     let replace_type_params =
         Edit::range_replacement(new_type_params.to_string(), type_params.range);
 

crates/ruff_linter/src/rules/ruff/rules/default_factory_kwarg.rs

@@ -4,6 +4,7 @@ use ast::Keyword;
 use ruff_macros::{ViolationMetadata, derive_message_formats};
 use ruff_python_ast::helpers::is_constant;
 use ruff_python_ast::{self as ast, Expr};
+use ruff_python_trivia::CommentRanges;
 use ruff_text_size::Ranged;
 
 use crate::Locator;
@@ -106,7 +107,9 @@ pub(crate) fn default_factory_kwarg(checker: &Checker, call: &ast::ExprCall) {
         },
         call.range(),
     );
-    diagnostic.try_set_fix(|| convert_to_positional(call, keyword, checker.locator()));
+    diagnostic.try_set_fix(|| {
+        convert_to_positional(call, keyword, checker.locator(), checker.comment_ranges())
+    });
 }
 
 /// Returns `true` if a value is definitively not callable (e.g., `1` or `[]`).
@@ -131,6 +134,7 @@ fn convert_to_positional(
     call: &ast::ExprCall,
     default_factory: &Keyword,
     locator: &Locator,
+    comment_ranges: &CommentRanges,
 ) -> Result<Fix> {
     if call.arguments.len() == 1 {
         // Ex) `defaultdict(default_factory=list)`
@@ -147,6 +151,7 @@ fn convert_to_positional(
             &call.arguments,
             Parentheses::Preserve,
             locator.contents(),
+            comment_ranges,
         )?;
 
         // Second, insert the value as the first positional argument.

crates/ruff_linter/src/rules/ruff/rules/falsy_dict_get_fallback.rs

@@ -127,6 +127,7 @@ pub(crate) fn falsy_dict_get_fallback(checker: &Checker, expr: &Expr) {
             &call.arguments,
             Parentheses::Preserve,
             checker.locator().contents(),
+            checker.comment_ranges(),
         )
         .map(|edit| Fix::applicable_edit(edit, applicability))
     });

crates/ruff_linter/src/rules/ruff/rules/unnecessary_literal_within_deque_call.rs

@@ -157,6 +157,7 @@ fn fix_unnecessary_literal_in_deque(
             &deque.arguments,
             Parentheses::Preserve,
             checker.source(),
+            checker.comment_ranges(),
         )?
     };
     let has_comments = checker.comment_ranges().intersects(edit.range());

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF056_RUF056.py.snap

@@ -469,5 +469,23 @@ RUF056.py:187:24: RUF056 Avoid providing a falsy fallback to `dict.get()` in boo
 186 | # out for having an unrecognized number of arguments
 187 | not my_dict.get("key", False, foo=...)
     |                        ^^^^^ RUF056
+188 |
+189 | # https://github.com/astral-sh/ruff/issues/18798
     |
     = help: Remove falsy fallback from `dict.get()`
+
+RUF056.py:191:19: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy.
+    |
+189 | # https://github.com/astral-sh/ruff/issues/18798
+190 | d = {}
+191 | not d.get("key", (False))
+    |                   ^^^^^ RUF056
+    |
+    = help: Remove falsy fallback from `dict.get()`
+
+ℹ Safe fix
+188 188 | 
+189 189 | # https://github.com/astral-sh/ruff/issues/18798
+190 190 | d = {}
+191     |-not d.get("key", (False))
+    191 |+not d.get("key")