Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unstable formatting with "# type: ignore" comment #1061

Closed
The-Compiler opened this issue Oct 13, 2019 · 6 comments · Fixed by #1113
Closed

Unstable formatting with "# type: ignore" comment #1061

The-Compiler opened this issue Oct 13, 2019 · 6 comments · Fixed by #1113

Comments

@The-Compiler
Copy link

The-Compiler commented Oct 13, 2019

Operating system: Arch Linux
Python version: 3.7
Black version: 19.3b0, 19.3b1.dev96+g57ab909
Does also happen on master: yes

I think I'm seeing a "INTERNAL ERROR: Black produced different code on the second pass of the formatter." which is different from others I find here (#954, #1042, #1044).

With this code:

class Test:
    def _init_host(self, parsed) -> None:
        if (parsed.hostname is None or  # type: ignore
                not parsed.hostname.strip()):
            pass

I get this diff:

--- source
+++ first pass
@@ -1,6 +1,8 @@
 class Test:
     def _init_host(self, parsed) -> None:
-        if (parsed.hostname is None or  # type: ignore
-                not parsed.hostname.strip()):
+        if (
+            parsed.hostname is None
+            or not parsed.hostname.strip()  # type: ignore
+        ):
             pass
 
--- first pass
+++ second pass
@@ -1,8 +1,7 @@
 class Test:
     def _init_host(self, parsed) -> None:
         if (
-            parsed.hostname is None
-            or not parsed.hostname.strip()  # type: ignore
+            parsed.hostname is None or not parsed.hostname.strip()  # type: ignore
         ):
             pass
 
@JelleZijlstra
Copy link
Collaborator

@msullivan is this related to any of the changes you made recently?

@The-Compiler
Copy link
Author

I was able to bisect this to ca9ad69.

@msullivan
Copy link
Contributor

msullivan commented Oct 14, 2019

Not related to any of the changes I made recently but I looked into it.

The issue here is that the # type: ignore attached to the end of the original line prevents everything from getting collapsed onto one line (since it is a type comment in the "middle" of a potential line), but it is still attached to the or and so it gets moved down to the second line.

But then on the second run, it's not in the middle anymore, so black thinks it is safe to collapse everything onto one line.

One fix to this would be to not have internal type: ignore comments prevent merging lines. This fixes the issue here, but I'm not totally confident it fixes the issue always. It also changes some behavior and will make it harder to ignore a specific argument to a function without ignoring the whole call in some cases.

Another possibility could be to go the other direction and consider a type comment (just an ignore?) at the end uncollapsable also, so we stick with the version split on two lines?

@IIBenII
Copy link

IIBenII commented Oct 21, 2019

I open an issue on mypy python/mypy#7762 because when I use black + mypy and # type: ignore, black uncollapse the line and mypy ignore the comment, so it raise an error. It's possible to not uncollapse line when they are # type: ignore?

@JelleZijlstra
Copy link
Collaborator

@IIBenII the next release of Black will be better at keeping # type: ignore lines in the right place. I'm not sure whether it will cover your exact use case though.

@IIBenII
Copy link

IIBenII commented Oct 21, 2019

@JelleZijlstra Thanks, I will wait :)

msullivan added a commit to msullivan/black that referenced this issue Oct 29, 2019
`type: ignore` shouldn't block collapsing a line, since it will still
apply fine to the merged line. This prevents an issue where a reformat
causes it to shift lines and then be merged on a subsequent pass.

There is a downside to this, which is that it can cause a `type:
ignore` to apply to more code than was originally intended. There
might be a way to apply this in a more limited situation, but I'm not
sure what it is.

Fixes psf#1061.
JelleZijlstra pushed a commit that referenced this issue Nov 25, 2019
`type: ignore` shouldn't block collapsing a line, since it will still
apply fine to the merged line. This prevents an issue where a reformat
causes it to shift lines and then be merged on a subsequent pass.

There is a downside to this, which is that it can cause a `type:
ignore` to apply to more code than was originally intended. There
might be a way to apply this in a more limited situation, but I'm not
sure what it is.

Fixes #1061.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants