Repository: psf/black
Pull request: Strip trailing commas in subscripts with -C
Fixes #2296, #3204

Problem statement:
black doesn't remove ugly trailing commas in subscripts
**Describe the bug**

We recently added `--skip-magic-trailing-comma` to black. When running with this option, we'd like it to always remove ugly unnecessary trailing commas that plagued some earlier versions of black, but it doesn't do this in the case of subscripts.

**To Reproduce**

```
(venv) ~/tmp/venv λ cat test.pyi
Tuple[A, B,]

def f(a, b,): ...
(venv) ~/tmp/venv λ black -l 130 test.pyi --skip-magic-trailing-comma --check --diff
--- test.pyi	2021-06-01 07:57:26.995533 +0000
+++ test.pyi	2021-06-01 07:58:31.094021 +0000
@@ -1,3 +1,3 @@
 Tuple[A, B,]
 
-def f(a, b,): ...
+def f(a, b): ...
would reformat test.pyi
Oh no! 💥 💔 💥
1 file would be reformatted.
```

**Expected behavior**

If the subscript expression has more than one element, we should remove the extra trailing comma.

Hints from issue discussion:

Commit message: srsly okay okay okay okay

Patch to review:
diff --git a/CHANGES.md b/CHANGES.md
index 5b29f20bfff..5e4286cf65e 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -24,6 +24,8 @@
   this is invalid. This was a bug introduced in version 22.6.0. (#3166)
 - `--skip-string-normalization` / `-S` now prevents docstring prefixes from being
   normalized as expected (#3168)
+- When using `--skip-magic-trailing-comma` or `-C`, trailing commas are stripped from
+  subscript expressions with more than 1 element (#3209)
 
 ### _Blackd_
 
diff --git a/src/black/lines.py b/src/black/lines.py
index 1ebc7808901..63914c9c785 100644
--- a/src/black/lines.py
+++ b/src/black/lines.py
@@ -273,6 +273,8 @@ def has_magic_trailing_comma(
         - it's not a single-element subscript
         Additionally, if ensure_removable:
         - it's not from square bracket indexing
+        (specifically, single-element square bracket indexing with
+        Preview.skip_magic_trailing_comma_in_subscript)
         """
         if not (
             closing.type in CLOSING_BRACKETS
@@ -301,8 +303,22 @@ def has_magic_trailing_comma(
 
             if not ensure_removable:
                 return True
+
             comma = self.leaves[-1]
-            return bool(comma.parent and comma.parent.type == syms.listmaker)
+            if comma.parent is None:
+                return False
+            if Preview.skip_magic_trailing_comma_in_subscript in self.mode:
+                return bool(
+                    comma.parent.type != syms.subscriptlist
+                    or closing.opening_bracket is None
+                    or not is_one_sequence_between(
+                        closing.opening_bracket,
+                        closing,
+                        self.leaves,
+                        brackets=(token.LSQB, token.RSQB),
+                    )
+                )
+            return comma.parent.type == syms.listmaker
 
         if self.is_import:
             return True
diff --git a/src/black/mode.py b/src/black/mode.py
index 32b65d16ca5..6e8f9f8b625 100644
--- a/src/black/mode.py
+++ b/src/black/mode.py
@@ -151,6 +151,7 @@ class Preview(Enum):
     remove_block_trailing_newline = auto()
     remove_redundant_parens = auto()
     string_processing = auto()
+    skip_magic_trailing_comma_in_subscript = auto()
 
 
 class Deprecated(UserWarning):
diff --git a/tests/data/preview/skip_magic_trailing_comma.py b/tests/data/preview/skip_magic_trailing_comma.py
new file mode 100644
index 00000000000..e98174af427
--- /dev/null
+++ b/tests/data/preview/skip_magic_trailing_comma.py
@@ -0,0 +1,34 @@
+# We should not remove the trailing comma in a single-element subscript.
+a: tuple[int,]
+b = tuple[int,]
+
+# But commas in multiple element subscripts should be removed.
+c: tuple[int, int,]
+d = tuple[int, int,]
+
+# Remove commas for non-subscripts.
+small_list = [1,]
+list_of_types = [tuple[int,],]
+small_set = {1,}
+set_of_types = {tuple[int,],}
+
+# Except single element tuples
+small_tuple = (1,)
+
+# output
+# We should not remove the trailing comma in a single-element subscript.
+a: tuple[int,]
+b = tuple[int,]
+
+# But commas in multiple element subscripts should be removed.
+c: tuple[int, int]
+d = tuple[int, int]
+
+# Remove commas for non-subscripts.
+small_list = [1]
+list_of_types = [tuple[int,]]
+small_set = {1}
+set_of_types = {tuple[int,]}
+
+# Except single element tuples
+small_tuple = (1,)
diff --git a/tests/test_format.py b/tests/test_format.py
index 3645934721f..01cd61eef63 100644
--- a/tests/test_format.py
+++ b/tests/test_format.py
@@ -36,7 +36,12 @@ def test_simple_format(filename: str) -> None:
 
 @pytest.mark.parametrize("filename", all_data_cases("preview"))
 def test_preview_format(filename: str) -> None:
-    check_file("preview", filename, black.Mode(preview=True))
+    magic_trailing_comma = filename != "skip_magic_trailing_comma"
+    check_file(
+        "preview",
+        filename,
+        black.Mode(preview=True, magic_trailing_comma=magic_trailing_comma),
+    )
 
 
 @pytest.mark.parametrize("filename", all_data_cases("preview_39"))


Code review comments:
diff_hunk: @@ -301,8 +303,22 @@ def has_magic_trailing_comma(
 
             if not ensure_removable:
                 return True
+
             comma = self.leaves[-1]
-            return bool(comma.parent and comma.parent.type == syms.listmaker)
+            if comma.parent is None:
+                return False
+            if Preview.skip_magic_trailing_comma_in_subscript in self.mode:
+                return bool(
path: src/black/lines.py
text: @user1:
```suggestion
                return (
```
Nit, but all the elements in the or are already bools.