Repository: facebookexperimental/usort (python)

Issue:
compatibility: handle isort:skip on the _first_ line of a wrapped statement
Something like

```
from x import (  # isort:skip
  y,
  z,
)
```

Currently is considered sortable (because it looks for the directive at the end of the _statement_, which here is after the closing paren.  The fix here is probably intertwined with wrapping in #16, and some other libcst or fixit efforts that make edits to change imports.

Pull request description:
Improved skip directive support
* Allow `#usort:skip` on the first line of multi-line imports
* Support `#usort:skip` after other comments/directives
* Document supported directive locations/positions
* Add test cases that fail on main but pass with PR
* Better error reporting from functional tests when exceptions occur during sorting

Fixes #27 

Docs preview: https://usort--108.org.readthedocs.build/en/108/guide.html#comments

Patch:
diff --git a/docs/guide.rst b/docs/guide.rst
index 4b6159a..7810f95 100644
--- a/docs/guide.rst
+++ b/docs/guide.rst
@@ -186,7 +186,19 @@ including moving any other statements across the skipped statement::
 
     import difflib
 
-See `Import Blocks`_ for details on how this affects sorting behavior.
+Comment directives must be on the first or last line of multi-line imports::
+
+    from side_effect import (  # usort:skip  # here
+        thing_one,
+        thing_two,
+    )  # usort:skip  # or here
+
+Directives are also allowed anywhere in a comment, but must include another ``#``
+character if they are not the first element::
+
+    import side_effect  # noqa: F401  # usort:skip
+
+See `Import Blocks`_ for details on how skip directives affect sorting behavior.
 
 .. note:: 
     For compatibility with existing codebases previously using isort, the
@@ -313,6 +325,8 @@ Both ``#usort:skip`` and ``#isort:skip`` (with any amount of whitespace),
 will trigger this behavior, so existing comments intended for isort will still
 work with µsort.
 
+See `directives`_ for details on supported comment directives.
+
 Statements
 ^^^^^^^^^^
 
diff --git a/usort/sorting.py b/usort/sorting.py
index 8a44652..16d9107 100644
--- a/usort/sorting.py
+++ b/usort/sorting.py
@@ -27,6 +27,18 @@ class ImportSorter:
         self.wrapper = cst.MetadataWrapper(module)
         self.transformer = ImportSortingTransformer(config, module, self)
 
+    def has_skip_comment(self, comment: Optional[cst.Comment]) -> bool:
+        if comment is None or not comment.value:
+            return False
+
+        directives = comment.value.split("#")
+        for directive in directives:
+            directive = directive.replace(" ", "")
+            if directive.startswith("usort:skip") or directive.startswith("isort:skip"):
+                return True
+
+        return False
+
     def is_sortable_import(self, stmt: cst.CSTNode) -> bool:
         """
         Determine if any individual statement is sortable or should be a barrier.
@@ -34,13 +46,12 @@ class ImportSorter:
         Handles skip directives, configured side effect modules, and star imports.
         """
         if isinstance(stmt, cst.SimpleStatementLine):
-            com = stmt.trailing_whitespace.comment
-            if com:
-                com_str = com.value.replace(" ", "")
-                if com_str.startswith("#usort:skip") or com_str.startswith(
-                    "#isort:skip"
-                ):
-                    return False
+            # from foo import (
+            #     bar,
+            # )  # usort:skip
+            comment = stmt.trailing_whitespace.comment
+            if self.has_skip_comment(comment):
+                return False
 
             # N.b. `body` is a list, because the SimpleStatementLine might have
             # semicolons.  We only look at the first, which is probably the most
@@ -51,6 +62,15 @@ class ImportSorter:
             # like noqa.  TODO do that before calling is_sortable_import, and assert
             # that it's not a compound statement line.
             if isinstance(stmt.body[0], cst.ImportFrom):
+                # from foo import (  # usort:skip
+                #     bar,
+                # )
+                if isinstance(stmt.body[0].lpar, cst.LeftParen) and isinstance(
+                    stmt.body[0].lpar.whitespace_after, cst.ParenthesizedWhitespace
+                ):
+                    comment = stmt.body[0].lpar.whitespace_after.first_line.comment
+                    if self.has_skip_comment(comment):
+                        return False
                 # `from x import *` is a barrier
                 if isinstance(stmt.body[0].names, cst.ImportStar):
                     return False


Test patch:
diff --git a/usort/tests/functional.py b/usort/tests/functional.py
index 17043f7..3a94188 100644
--- a/usort/tests/functional.py
+++ b/usort/tests/functional.py
@@ -50,6 +50,10 @@ class UsortStringFunctionalTest(unittest.TestCase):
         config = config or DEFAULT_CONFIG
         result1 = usort(before.encode(), config)  # first pass
         result2 = usort(result1.output, config)  # enforce stable sorting on second pass
+        if result1.error:
+            raise result1.error
+        if result2.error:
+            raise result2.error
         if result2.output != result1.output:
             self.fail(
                 "µsort result was not stable on second pass:\n\n"
@@ -796,6 +800,65 @@ numpy = ["numpy", "pandas"]
             """,
         )
 
+    def test_skip_directives(self) -> None:
+        """Test both usort:skip and isort:skip on single line imports"""
+        self.assertUsortResult(
+            """
+                from os import path
+                import functools  # usort: skip
+                import asyncio
+                from collections import defaultdict  # isort:skip
+                from asyncio import gather
+            """,
+            """
+                from os import path
+                import functools  # usort: skip
+                import asyncio
+                from collections import defaultdict  # isort:skip
+                from asyncio import gather
+            """,
+        )
+
+    def test_skip_directives_after_noqa(self) -> None:
+        """Test that skips are obeyed, even if they aren't the first directive"""
+        self.assertUsortResult(
+            """
+                from os import path
+                import functools  # noqa  # usort: skip
+                import asyncio
+            """,
+            """
+                from os import path
+                import functools  # noqa  # usort: skip
+                import asyncio
+            """,
+        )
+
+    def test_skip_directives_multiline(self) -> None:
+        """Validate that skip work on both first and last line of multiline imports"""
+        self.assertUsortResult(
+            """
+                from unittest.mock import (
+                    Mock, MagicMock, call, patch, sentinel, ANY,
+                )  # usort:skip
+                from functools import wraps
+                from asyncio import (  # usort:skip
+                    gather, wait,
+                )
+                from collections import defaultdict
+            """,
+            """
+                from unittest.mock import (
+                    Mock, MagicMock, call, patch, sentinel, ANY,
+                )  # usort:skip
+                from functools import wraps
+                from asyncio import (  # usort:skip
+                    gather, wait,
+                )
+                from collections import defaultdict
+            """,
+        )
+
 
 if __name__ == "__main__":
     unittest.main()