Regression with master

[hugovk]

Operating system: macOS Mojave 10.14.6
Python version: 3.7.4
Black version: master

Possibly duplicates of #1041 and #1042.

Running Black master on the pip master codebase, all reformat except one:

error: cannot format /Users/hugo/github/pip/tests/unit/test_collector.py: INTERNAL ERROR: Black produced different code on the second pass of the formatter.  Please report a bug on https://github.com/psf/black/issues.  This diff might be helpful: /var/folders/kt/j77sf4_n6fnbx6pg199rbx700000gn/T/blk_2wyp3a1o.log

The source file:
https://github.com/pypa/pip/blob/e6f69fadd8c84526d933b2650141193595059e72/tests/unit/test_collector.py

The Black log file:

--- source
+++ first pass
@@ -27,15 +27,11 @@
 from pip._internal.network.session import PipSession
 from tests.lib import make_test_link_collector
 
 
 @pytest.mark.parametrize(
-    "url",
-    [
-        "ftp://python.org/python-3.7.1.zip",
-        "file:///opt/data/pip-18.0.tar.gz",
-    ],
+    "url", ["ftp://python.org/python-3.7.1.zip", "file:///opt/data/pip-18.0.tar.gz"]
 )
 def test_get_html_response_archive_to_naive_scheme(url):
     """
     `_get_html_response()` should error on an archive-like URL if the scheme
     does not allow "poking" without getting data.
@@ -55,52 +51,44 @@
     """
     `_get_html_response()` should send a HEAD request on an archive-like URL
     if the scheme supports it, and raise `_NotHTML` if the response isn't HTML.
     """
     session = mock.Mock(PipSession)
-    session.head.return_value = mock.Mock(**{
-        "request.method": "HEAD",
-        "headers": {"Content-Type": content_type},
-    })
+    session.head.return_value = mock.Mock(
+        **{"request.method": "HEAD", "headers": {"Content-Type": content_type}}
+    )
 
     with pytest.raises(_NotHTML) as ctx:
         _get_html_response(url, session=session)
 
-    session.assert_has_calls([
-        mock.call.head(url, allow_redirects=True),
-    ])
+    session.assert_has_calls([mock.call.head(url, allow_redirects=True)])
     assert ctx.value.args == (content_type, "HEAD")
 
 
 @pytest.mark.parametrize(
-    "url",
-    [
-        "http://python.org/python-3.7.1.zip",
-        "https://pypi.org/pip-18.0.tar.gz",
-    ],
+    "url", ["http://python.org/python-3.7.1.zip", "https://pypi.org/pip-18.0.tar.gz"]
 )
 def test_get_html_response_archive_to_http_scheme_is_html(url):
     """
     `_get_html_response()` should work with archive-like URLs if the HEAD
     request is responded with text/html.
     """
     session = mock.Mock(PipSession)
-    session.head.return_value = mock.Mock(**{
-        "request.method": "HEAD",
-        "headers": {"Content-Type": "text/html"},
-    })
+    session.head.return_value = mock.Mock(
+        **{"request.method": "HEAD", "headers": {"Content-Type": "text/html"}}
+    )
     session.get.return_value = mock.Mock(headers={"Content-Type": "text/html"})
 
     resp = _get_html_response(url, session=session)
 
     assert resp is not None
     assert session.mock_calls == [
         mock.call.head(url, allow_redirects=True),
         mock.call.head().raise_for_status(),
-        mock.call.get(url, headers={
-            "Accept": "text/html", "Cache-Control": "max-age=0",
-        }),
+        mock.call.get(
+            url, headers={"Accept": "text/html", "Cache-Control": "max-age=0"}
+        ),
         mock.call.get().raise_for_status(),
     ]
 
 
 @pytest.mark.parametrize(
@@ -117,22 +105,20 @@
     look like an archive, only the GET request that retrieves data.
     """
     session = mock.Mock(PipSession)
 
     # Mock the headers dict to ensure it is accessed.
-    session.get.return_value = mock.Mock(headers=mock.Mock(**{
-        "get.return_value": "text/html",
-    }))
+    session.get.return_value = mock.Mock(
+        headers=mock.Mock(**{"get.return_value": "text/html"})
+    )
 
     resp = _get_html_response(url, session=session)
 
     assert resp is not None
     assert session.head.call_count == 0
     assert session.get.mock_calls == [
-        mock.call(url, headers={
-            "Accept": "text/html", "Cache-Control": "max-age=0",
-        }),
+        mock.call(url, headers={"Accept": "text/html", "Cache-Control": "max-age=0"}),
         mock.call().raise_for_status(),
         mock.call().headers.get("Content-Type", ""),
     ]
 
 
@@ -142,13 +128,13 @@
     in its DEBUG log message.
     """
     session = mock.Mock(PipSession)
 
     # Mock the headers dict to ensure it is accessed.
-    session.get.return_value = mock.Mock(headers=mock.Mock(**{
-        "get.return_value": "text/html",
-    }))
+    session.get.return_value = mock.Mock(
+        headers=mock.Mock(**{"get.return_value": "text/html"})
+    )
 
     caplog.set_level(logging.DEBUG)
 
     resp = _get_html_response(
         "https://user:[email protected]/simple/", session=session
@@ -156,73 +142,87 @@
 
     assert resp is not None
 
     assert len(caplog.records) == 1
     record = caplog.records[0]
-    assert record.levelname == 'DEBUG'
+    assert record.levelname == "DEBUG"
     assert record.message.splitlines() == [
-        "Getting page https://user:****@example.com/simple/",
+        "Getting page https://user:****@example.com/simple/"
     ]
 
 
 @pytest.mark.parametrize(
     ("html", "url", "expected"),
     [
         (b"<html></html>", "https://example.com/", "https://example.com/"),
         (
+            b"<html><head>" b'<base href="https://foo.example.com/">' b"</head></html>",
+            "https://example.com/",
+            "https://foo.example.com/",
+        ),
+        (
             b"<html><head>"
-            b"<base href=\"https://foo.example.com/\">"
+            b'<base><base href="https://foo.example.com/">'
             b"</head></html>",
             "https://example.com/",
             "https://foo.example.com/",
         ),
-        (
-            b"<html><head>"
-            b"<base><base href=\"https://foo.example.com/\">"
-            b"</head></html>",
-            "https://example.com/",
-            "https://foo.example.com/",
-        ),
     ],
 )
 def test_determine_base_url(html, url, expected):
     document = html5lib.parse(
-        html, transport_encoding=None, namespaceHTMLElements=False,
+        html, transport_encoding=None, namespaceHTMLElements=False
     )
     assert _determine_base_url(document, url) == expected
 
 
 @pytest.mark.parametrize(
     ("url", "clean_url"),
     [
         # URL with hostname and port. Port separator should not be quoted.
-        ("https://localhost.localdomain:8181/path/with space/",
-         "https://localhost.localdomain:8181/path/with%20space/"),
+        (
+            "https://localhost.localdomain:8181/path/with space/",
+            "https://localhost.localdomain:8181/path/with%20space/",
+        ),
         # URL that is already properly quoted. The quoting `%`
         # characters should not be quoted again.
-        ("https://localhost.localdomain:8181/path/with%20quoted%20space/",
-         "https://localhost.localdomain:8181/path/with%20quoted%20space/"),
+        (
+            "https://localhost.localdomain:8181/path/with%20quoted%20space/",
+            "https://localhost.localdomain:8181/path/with%20quoted%20space/",
+        ),
         # URL with IPv4 address and port.
-        ("https://127.0.0.1:8181/path/with space/",
-         "https://127.0.0.1:8181/path/with%20space/"),
+        (
+            "https://127.0.0.1:8181/path/with space/",
+            "https://127.0.0.1:8181/path/with%20space/",
+        ),
         # URL with IPv6 address and port. The `[]` brackets around the
         # IPv6 address should not be quoted.
-        ("https://[fd00:0:0:236::100]:8181/path/with space/",
-         "https://[fd00:0:0:236::100]:8181/path/with%20space/"),
+        (
+            "https://[fd00:0:0:236::100]:8181/path/with space/",
+            "https://[fd00:0:0:236::100]:8181/path/with%20space/",
+        ),
         # URL with query. The leading `?` should not be quoted.
-        ("https://localhost.localdomain:8181/path/with/query?request=test",
-         "https://localhost.localdomain:8181/path/with/query?request=test"),
+        (
+            "https://localhost.localdomain:8181/path/with/query?request=test",
+            "https://localhost.localdomain:8181/path/with/query?request=test",
+        ),
         # URL with colon in the path portion.
-        ("https://localhost.localdomain:8181/path:/with:/colon",
-         "https://localhost.localdomain:8181/path%3A/with%3A/colon"),
+        (
+            "https://localhost.localdomain:8181/path:/with:/colon",
+            "https://localhost.localdomain:8181/path%3A/with%3A/colon",
+        ),
         # URL with something that looks like a drive letter, but is
         # not. The `:` should be quoted.
-        ("https://localhost.localdomain/T:/path/",
-         "https://localhost.localdomain/T%3A/path/"),
+        (
+            "https://localhost.localdomain/T:/path/",
+            "https://localhost.localdomain/T%3A/path/",
+        ),
         # VCS URL containing revision string.
-        ("git+ssh://example.com/path to/[email protected]#egg=my-package-1.0",
-         "git+ssh://example.com/path%20to/[email protected]#egg=my-package-1.0"),
+        (
+            "git+ssh://example.com/path to/[email protected]#egg=my-package-1.0",
+            "git+ssh://example.com/path%20to/[email protected]#egg=my-package-1.0",
+        ),
         # URL with Windows drive letter. The `:` after the drive
         # letter should not be quoted. The trailing `/` should be
         # removed.
         pytest.param(
             "file:///T:/path/with spaces/",
@@ -234,88 +234,80 @@
         pytest.param(
             "file:///T:/path/with spaces/",
             "file:///T%3A/path/with%20spaces/",
             marks=pytest.mark.skipif("sys.platform == 'win32'"),
         ),
-    ]
+    ],
 )
 def test_clean_link(url, clean_url):
-    assert(_clean_link(url) == clean_url)
-
-
-@pytest.mark.parametrize('anchor_html, expected', [
-    # Test not present.
-    ('<a href="/pkg1-1.0.tar.gz"></a>', None),
-    # Test present with no value.
-    ('<a href="/pkg2-1.0.tar.gz" data-yanked></a>', ''),
-    # Test the empty string.
-    ('<a href="/pkg3-1.0.tar.gz" data-yanked=""></a>', ''),
-    # Test a non-empty string.
-    ('<a href="/pkg4-1.0.tar.gz" data-yanked="error"></a>', 'error'),
-    # Test a value with an escaped character.
-    ('<a href="/pkg4-1.0.tar.gz" data-yanked="version &lt 1"></a>',
-        'version < 1'),
-    # Test a yanked reason with a non-ascii character.
-    (u'<a href="/pkg-1.0.tar.gz" data-yanked="curlyquote \u2018"></a>',
-        u'curlyquote \u2018'),
-])
+    assert _clean_link(url) == clean_url
+
+
+@pytest.mark.parametrize(
+    "anchor_html, expected",
+    [
+        # Test not present.
+        ('<a href="/pkg1-1.0.tar.gz"></a>', None),
+        # Test present with no value.
+        ('<a href="/pkg2-1.0.tar.gz" data-yanked></a>', ""),
+        # Test the empty string.
+        ('<a href="/pkg3-1.0.tar.gz" data-yanked=""></a>', ""),
+        # Test a non-empty string.
+        ('<a href="/pkg4-1.0.tar.gz" data-yanked="error"></a>', "error"),
+        # Test a value with an escaped character.
+        ('<a href="/pkg4-1.0.tar.gz" data-yanked="version &lt 1"></a>', "version < 1"),
+        # Test a yanked reason with a non-ascii character.
+        (
+            u'<a href="/pkg-1.0.tar.gz" data-yanked="curlyquote \u2018"></a>',
+            u"curlyquote \u2018",
+        ),
+    ],
+)
 def test_parse_links__yanked_reason(anchor_html, expected):
     html = (
         # Mark this as a unicode string for Python 2 since anchor_html
         # can contain non-ascii.
         u'<html><head><meta charset="utf-8"><head>'
-        '<body>{}</body></html>'
+        "<body>{}</body></html>"
     ).format(anchor_html)
-    html_bytes = html.encode('utf-8')
-    page = HTMLPage(
-        html_bytes,
-        encoding=None,
-        url='https://example.com/simple/',
-    )
+    html_bytes = html.encode("utf-8")
+    page = HTMLPage(html_bytes, encoding=None, url="https://example.com/simple/")
     links = list(parse_links(page))
-    link, = links
+    (link,) = links
     actual = link.yanked_reason
     assert actual == expected
 
 
 def test_request_http_error(caplog):
     caplog.set_level(logging.DEBUG)
-    link = Link('http://localhost')
+    link = Link("http://localhost")
     session = Mock(PipSession)
     session.get.return_value = resp = Mock()
-    resp.raise_for_status.side_effect = requests.HTTPError('Http error')
+    resp.raise_for_status.side_effect = requests.HTTPError("Http error")
     assert _get_html_page(link, session=session) is None
-    assert (
-        'Could not fetch URL http://localhost: Http error - skipping'
-        in caplog.text
-    )
+    assert "Could not fetch URL http://localhost: Http error - skipping" in caplog.text
 
 
 def test_request_retries(caplog):
     caplog.set_level(logging.DEBUG)
-    link = Link('http://localhost')
+    link = Link("http://localhost")
     session = Mock(PipSession)
-    session.get.side_effect = requests.exceptions.RetryError('Retry error')
+    session.get.side_effect = requests.exceptions.RetryError("Retry error")
     assert _get_html_page(link, session=session) is None
-    assert (
-        'Could not fetch URL http://localhost: Retry error - skipping'
-        in caplog.text
-    )
+    assert "Could not fetch URL http://localhost: Retry error - skipping" in caplog.text
 
 
 def test_make_html_page():
-    headers = {'Content-Type': 'text/html; charset=UTF-8'}
+    headers = {"Content-Type": "text/html; charset=UTF-8"}
     response = pretend.stub(
-        content=b'<content>',
-        url='https://example.com/index.html',
-        headers=headers,
+        content=b"<content>", url="https://example.com/index.html", headers=headers
     )
 
     actual = _make_html_page(response)
-    assert actual.content == b'<content>'
-    assert actual.encoding == 'UTF-8'
-    assert actual.url == 'https://example.com/index.html'
+    assert actual.content == b"<content>"
+    assert actual.encoding == "UTF-8"
+    assert actual.url == "https://example.com/index.html"
 
 
 @pytest.mark.parametrize(
     "url, vcs_scheme",
     [
@@ -335,73 +327,70 @@
     assert caplog.record_tuples == [
         (
             "pip._internal.collector",
             logging.DEBUG,
             "Cannot look at {} URL {}".format(vcs_scheme, url),
-        ),
+        )
     ]
 
 
 def make_fake_html_response(url):
     """
     Create a fake requests.Response object.
     """
-    html = dedent(u"""\
+    html = dedent(
+        u"""\
     <html><head><meta name="api-version" value="2" /></head>
     <body>
     <a href="/abc-1.0.tar.gz#md5=000000000">abc-1.0.tar.gz</a>
     </body></html>
-    """)
-    content = html.encode('utf-8')
+    """
+    )
+    content = html.encode("utf-8")
     return pretend.stub(content=content, url=url, headers={})
 
 
 def test_get_html_page_directory_append_index(tmpdir):
     """`_get_html_page()` should append "index.html" to a directory URL.
     """
     dirpath = tmpdir.mkdir("something")
-    dir_url = "file:///{}".format(
-        urllib_request.pathname2url(dirpath).lstrip("/"),
-    )
+    dir_url = "file:///{}".format(urllib_request.pathname2url(dirpath).lstrip("/"))
     expected_url = "{}/index.html".format(dir_url.rstrip("/"))
 
     session = mock.Mock(PipSession)
     fake_response = make_fake_html_response(expected_url)
     with mock.patch("pip._internal.collector._get_html_response") as mock_func:
         mock_func.return_value = fake_response
         actual = _get_html_page(Link(dir_url), session=session)
         assert mock_func.mock_calls == [
-            mock.call(expected_url, session=session),
-        ], 'actual calls: {}'.format(mock_func.mock_calls)
+            mock.call(expected_url, session=session)
+        ], "actual calls: {}".format(mock_func.mock_calls)
 
         assert actual.content == fake_response.content
         assert actual.encoding is None
         assert actual.url == expected_url
 
 
 def test_remove_duplicate_links():
     links = [
         # We choose Links that will test that ordering is preserved.
-        Link('https://example.com/2'),
-        Link('https://example.com/1'),
-        Link('https://example.com/2'),
+        Link("https://example.com/2"),
+        Link("https://example.com/1"),
+        Link("https://example.com/2"),
     ]
     actual = _remove_duplicate_links(links)
-    assert actual == [
-        Link('https://example.com/2'),
-        Link('https://example.com/1'),
-    ]
+    assert actual == [Link("https://example.com/2"), Link("https://example.com/1")]
 
 
 def test_group_locations__file_expand_dir(data):
     """
     Test that a file:// dir gets listdir run with expand_dir
     """
     files, urls = group_locations([data.find_links], expand_dir=True)
     assert files and not urls, (
-        "files and not urls should have been found at find-links url: %s" %
-        data.find_links
+        "files and not urls should have been found at find-links url: %s"
+        % data.find_links
     )
 
 
 def test_group_locations__file_not_find_link(data):
     """
@@ -414,65 +403,66 @@
 
 def test_group_locations__non_existing_path():
     """
     Test that a non-existing path is ignored.
     """
-    files, urls = group_locations([os.path.join('this', 'doesnt', 'exist')])
+    files, urls = group_locations([os.path.join("this", "doesnt", "exist")])
     assert not urls and not files, "nothing should have been found"
 
 
 def check_links_include(links, names):
     """
     Assert that the given list of Link objects includes, for each of the
     given names, a link whose URL has a base name matching that name.
     """
     for name in names:
-        assert any(link.url.endswith(name) for link in links), (
-            'name {!r} not among links: {}'.format(name, links)
-        )
+        assert any(
+            link.url.endswith(name) for link in links
+        ), "name {!r} not among links: {}".format(name, links)
 
 
 class TestLinkCollector(object):
-
-    @patch('pip._internal.collector._get_html_response')
+    @patch("pip._internal.collector._get_html_response")
     def test_collect_links(self, mock_get_html_response, caplog, data):
         caplog.set_level(logging.DEBUG)
 
-        expected_url = 'https://pypi.org/simple/twine/'
+        expected_url = "https://pypi.org/simple/twine/"
 
         fake_page = make_fake_html_response(expected_url)
         mock_get_html_response.return_value = fake_page
 
         link_collector = make_test_link_collector(
             find_links=[data.find_links],
             # Include two copies of the URL to check that the second one
             # is skipped.
             index_urls=[PyPI.simple_url, PyPI.simple_url],
         )
-        actual = link_collector.collect_links('twine')
+        actual = link_collector.collect_links("twine")
 
         mock_get_html_response.assert_called_once_with(
-            expected_url, session=link_collector.session,
+            expected_url, session=link_collector.session
         )
 
         # Spot-check the CollectedLinks return value.
         assert len(actual.files) > 20
-        check_links_include(actual.files, names=['simple-1.0.tar.gz'])
+        check_links_include(actual.files, names=["simple-1.0.tar.gz"])
 
         assert len(actual.find_links) == 1
-        check_links_include(actual.find_links, names=['packages'])
+        check_links_include(actual.find_links, names=["packages"])
 
         actual_pages = actual.pages
         assert list(actual_pages) == [expected_url]
         actual_page_links = actual_pages[expected_url]
         assert len(actual_page_links) == 1
-        assert actual_page_links[0].url == (
-            'https://pypi.org/abc-1.0.tar.gz#md5=000000000'
+        assert actual_page_links[
+            0
+        ].url == "https://pypi.org/abc-1.0.tar.gz#md5=000000000"
+
+        expected_message = dedent(
+            """\
+        1 location(s) to search for versions of twine:
+        * https://pypi.org/simple/twine/"""
         )
-
-        expected_message = dedent("""\
-        1 location(s) to search for versions of twine:
-        * https://pypi.org/simple/twine/""")
         assert caplog.record_tuples == [
-            ('pip._internal.collector', logging.DEBUG, expected_message),
+            ("pip._internal.collector", logging.DEBUG, expected_message)
         ]
 
--- first pass
+++ second pass
@@ -451,13 +451,13 @@
 
         actual_pages = actual.pages
         assert list(actual_pages) == [expected_url]
         actual_page_links = actual_pages[expected_url]
         assert len(actual_page_links) == 1
-        assert actual_page_links[
-            0
-        ].url == "https://pypi.org/abc-1.0.tar.gz#md5=000000000"
+        assert (
+            actual_page_links[0].url == "https://pypi.org/abc-1.0.tar.gz#md5=000000000"
+        )
 
         expected_message = dedent(
             """\
         1 location(s) to search for versions of twine:
         * https://pypi.org/simple/twine/"""

[hugovk]

Bisecting points to 957ba24 from PR #850 to fix #548:

957ba24bb6cdd32e4dd14ff2808dcf4d58851844 is the first bad commit
commit 957ba24bb6cdd32e4dd14ff2808dcf4d58851844
Date:   Wed May 15 21:11:04 2019 -0700

    remove obviously unnecessary parentheses (#850)

    Fixes #548

:100644 100644 17aea7af264adafc4f5ee18cb9389cdfcdce8e17 694c1e961aa41bef489ea08145f1ee9faed7c69b M	black.py
:040000 040000 8da77aae59466f50234b0c16138cfcc5e88ebc17 3d7f6604495d23cbe68f30c434cf0d7a8ad1779d M	tests
bisect run success

[hugovk]

And here's one problematic snippet from the source file:

class TestLinkCollector(object):
    def test_collect_links(self):
        assert actual_page_links[0].url == (
            "https://pypi.org/abc-1.0.tar.gz#md5=000000000"
        )

And the log:

--- source
+++ first pass
@@ -1,7 +1,6 @@
 class TestLinkCollector(object):
     def test_collect_links(self):
-        assert actual_page_links[0].url == (
-            "https://pypi.org/abc-1.0.tar.gz#md5=000000000"
-        )
+        assert actual_page_links[
+            0
+        ].url == "https://pypi.org/abc-1.0.tar.gz#md5=000000000"
 
-
--- first pass
+++ second pass
@@ -1,6 +1,6 @@
 class TestLinkCollector(object):
     def test_collect_links(self):
-        assert actual_page_links[
-            0
-        ].url == "https://pypi.org/abc-1.0.tar.gz#md5=000000000"
+        assert (
+            actual_page_links[0].url == "https://pypi.org/abc-1.0.tar.gz#md5=000000000"
+        )
 

In this case, the second pass would be stable and Black won't change it again.

[hugovk]

And that snippet is the only bit in the input file that Black tripped over: manually fixing that bit meant the rest of the file (and codebase) could be formatted fine.

[JelleZijlstra]

This does look like a duplicate of #1042. Could you verify whether the change from #1079 fixes your code?

[hugovk]

It works now with latest master on 14b28c8:

$ black --version
black, version 19.3b1.dev114+g14b28c8

The whole codebase formats with no errors.

Thanks!

[JelleZijlstra]

Thanks for checking!