Fix multi-parentheses

astral-sh · Dec 1, 2023 · 591313e · 591313e
1 parent 70bef5c
commit 591313e
Show file tree

Hide file tree

Showing 5 changed files with 215 additions and 106 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/unspecified_encoding.py b/crates/ruff_linter/resources/test/fixtures/pylint/unspecified_encoding.py
@@ -53,11 +53,19 @@ def func(*args, **kwargs):
     # comment
 )
 open(("test.txt"),)
-open()
 open(
     ("test.txt"),  # comment
 )
 open(
     ("test.txt"),
     # comment
 )
+
+open((("test.txt")),)
+open(
+    (("test.txt")),  # comment
+)
+open(
+    (("test.txt")),
+    # comment
+)
diff --git a/crates/ruff_linter/src/fix/edits.rs b/crates/ruff_linter/src/fix/edits.rs
@@ -5,12 +5,14 @@ use std::ops::Sub;
 use anyhow::{Context, Result};
 
 use ruff_diagnostics::Edit;
-use ruff_python_ast::AnyNodeRef;
+use ruff_python_ast::parenthesize::parenthesized_range;
 use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Stmt};
+use ruff_python_ast::{AnyNodeRef, ArgOrKeyword};
 use ruff_python_codegen::Stylist;
 use ruff_python_index::Indexer;
 use ruff_python_trivia::{
-    has_leading_content, is_python_whitespace, PythonWhitespace, SimpleTokenKind, SimpleTokenizer,
+    has_leading_content, is_python_whitespace, CommentRanges, PythonWhitespace, SimpleTokenKind,
+    SimpleTokenizer,
 };
 use ruff_source_file::{Locator, NewlineWithTrailingNewline, UniversalNewlines};
 use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
@@ -141,26 +143,25 @@ pub(crate) fn remove_argument<T: Ranged>(
 }
 
 /// Generic function to add arguments or keyword arguments to function calls.
-pub(crate) fn add_argument(argument: &str, arguments: &Arguments, source: &str) -> Edit {
+pub(crate) fn add_argument(
+    argument: &str,
+    arguments: &Arguments,
+    comment_ranges: &CommentRanges,
+    source: &str,
+) -> Edit {
     if let Some(last) = arguments.arguments_source_order().last() {
         // Case 1: existing arguments, so append after the last argument.
-        let tokenizer = SimpleTokenizer::new(
+        let last = parenthesized_range(
+            match last {
+                ArgOrKeyword::Arg(arg) => arg.into(),
+                ArgOrKeyword::Keyword(keyword) => (&keyword.value).into(),
+            },
+            arguments.into(),
+            comment_ranges,
             source,
-            TextRange::new(last.end(), arguments.end().sub(TextSize::from(1))),
-        );
-
-        // Skip any parentheses.
-        if let Some(token) = tokenizer
-            .skip_while(|token| token.kind.is_trivia())
-            .next()
-            .filter(|token| token.kind == SimpleTokenKind::RParen)
-        {
-            // Ex) Insert after `func(x=(1))`.
-            Edit::insertion(format!(", {argument}"), token.end())
-        } else {
-            // Ex) Insert after `func(x=1)`.
-            Edit::insertion(format!(", {argument}"), last.end())
-        }
+        )
+        .unwrap_or(last.range());
+        Edit::insertion(format!(", {argument}"), last.end())
     } else {
         // Case 2: no arguments. Add argument, without any trailing comma.
         Edit::insertion(argument.to_string(), arguments.start() + TextSize::from(1))

diff --git a/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs b/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs
@@ -118,6 +118,7 @@ fn generate_keyword_fix(checker: &Checker, call: &ast::ExprCall) -> Fix {
                 }))
         ),
         &call.arguments,
+        checker.indexer().comment_ranges(),
         checker.locator().contents(),
     ))
 }
@@ -132,6 +133,7 @@ fn generate_import_fix(checker: &Checker, call: &ast::ExprCall) -> Result<Fix> {
     let argument_edit = add_argument(
         &format!("encoding={binding}(False)"),
         &call.arguments,
+        checker.indexer().comment_ranges(),
         checker.locator().contents(),
     );
     Ok(Fix::unsafe_edits(import_edit, [argument_edit]))

diff --git a/.../pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1514_unspecified_encoding.py.snap b/.../pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1514_unspecified_encoding.py.snap
@@ -235,8 +235,8 @@ unspecified_encoding.py:55:1: PLW1514 [*] `open` in text mode without explicit `
 54 | )
 55 | open(("test.txt"),)
    | ^^^^ PLW1514
-56 | open()
-57 | open(
+56 | open(
+57 |     ("test.txt"),  # comment
    |
    = help: Add explicit `encoding` argument
 
@@ -246,70 +246,111 @@ unspecified_encoding.py:55:1: PLW1514 [*] `open` in text mode without explicit `
 54 54 | )
 55    |-open(("test.txt"),)
    55 |+open(("test.txt"), encoding="locale",)
-56 56 | open()
-57 57 | open(
-58 58 |     ("test.txt"),  # comment
+56 56 | open(
+57 57 |     ("test.txt"),  # comment
+58 58 | )
 
 unspecified_encoding.py:56:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument
    |
 54 | )
 55 | open(("test.txt"),)
-56 | open()
+56 | open(
    | ^^^^ PLW1514
-57 | open(
-58 |     ("test.txt"),  # comment
+57 |     ("test.txt"),  # comment
+58 | )
    |
    = help: Add explicit `encoding` argument
 
 ℹ Unsafe fix
-53 53 |     # comment
 54 54 | )
 55 55 | open(("test.txt"),)
-56    |-open()
-   56 |+open(encoding="locale")
-57 57 | open(
-58 58 |     ("test.txt"),  # comment
-59 59 | )
+56 56 | open(
+57    |-    ("test.txt"),  # comment
+   57 |+    ("test.txt"), encoding="locale",  # comment
+58 58 | )
+59 59 | open(
+60 60 |     ("test.txt"),
 
-unspecified_encoding.py:57:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument
+unspecified_encoding.py:59:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument
    |
-55 | open(("test.txt"),)
-56 | open()
-57 | open(
+57 |     ("test.txt"),  # comment
+58 | )
+59 | open(
    | ^^^^ PLW1514
-58 |     ("test.txt"),  # comment
-59 | )
+60 |     ("test.txt"),
+61 |     # comment
    |
    = help: Add explicit `encoding` argument
 
 ℹ Unsafe fix
-55 55 | open(("test.txt"),)
-56 56 | open()
-57 57 | open(
-58    |-    ("test.txt"),  # comment
-   58 |+    ("test.txt"), encoding="locale",  # comment
-59 59 | )
-60 60 | open(
-61 61 |     ("test.txt"),
-
-unspecified_encoding.py:60:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument
-   |
-58 |     ("test.txt"),  # comment
-59 | )
-60 | open(
+57 57 |     ("test.txt"),  # comment
+58 58 | )
+59 59 | open(
+60    |-    ("test.txt"),
+   60 |+    ("test.txt"), encoding="locale",
+61 61 |     # comment
+62 62 | )
+63 63 | 
+
+unspecified_encoding.py:64:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument
+   |
+62 | )
+63 | 
+64 | open((("test.txt")),)
+   | ^^^^ PLW1514
+65 | open(
+66 |     (("test.txt")),  # comment
+   |
+   = help: Add explicit `encoding` argument
+
+ℹ Unsafe fix
+61 61 |     # comment
+62 62 | )
+63 63 | 
+64    |-open((("test.txt")),)
+   64 |+open((("test.txt")), encoding="locale",)
+65 65 | open(
+66 66 |     (("test.txt")),  # comment
+67 67 | )
+
+unspecified_encoding.py:65:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument
+   |
+64 | open((("test.txt")),)
+65 | open(
+   | ^^^^ PLW1514
+66 |     (("test.txt")),  # comment
+67 | )
+   |
+   = help: Add explicit `encoding` argument
+
+ℹ Unsafe fix
+63 63 | 
+64 64 | open((("test.txt")),)
+65 65 | open(
+66    |-    (("test.txt")),  # comment
+   66 |+    (("test.txt")), encoding="locale",  # comment
+67 67 | )
+68 68 | open(
+69 69 |     (("test.txt")),
+
+unspecified_encoding.py:68:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument
+   |
+66 |     (("test.txt")),  # comment
+67 | )
+68 | open(
    | ^^^^ PLW1514
-61 |     ("test.txt"),
-62 |     # comment
+69 |     (("test.txt")),
+70 |     # comment
    |
    = help: Add explicit `encoding` argument
 
 ℹ Unsafe fix
-58 58 |     ("test.txt"),  # comment
-59 59 | )
-60 60 | open(
-61    |-    ("test.txt"),
-   61 |+    ("test.txt"), encoding="locale",
-62 62 |     # comment
-63 63 | )
+66 66 |     (("test.txt")),  # comment
+67 67 | )
+68 68 | open(
+69    |-    (("test.txt")),
+   69 |+    (("test.txt")), encoding="locale",
+70 70 |     # comment
+71 71 | )