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

Formatter: Bug with Pragma Comment for pyright being moved to invalid position #8481

Open
roshanjrajan-zip opened this issue Nov 3, 2023 · 8 comments
Labels
bug Something isn't working formatter Related to the formatter

Comments

@roshanjrajan-zip
Copy link

roshanjrajan-zip commented Nov 3, 2023

Trying to enable ruff formatter on my repo and am getting an error when using in conjunction with a #pyright pragma comment. Its a bit convoluted of an example but this does error out. The # pyright: ignore[reportUnknownVariableType] should apply to the for a in line however keeps getting moved to where the parentheses is. If I try and move the parentheses or comment to the same line, it moves it back to what it looks like below. Interesting enough, in VsCode it moves the parentheses on the first save and then moves parentheses and comment later again on subsequent saves/running from terminal.

Code example:

def test_function(
    very_long_dictionary: dict,  # pyright:ignore
):
    something = [
        a["foo"]
        for a in
        (  # pyright: ignore[reportUnknownVariableType]
            very_long_dictionary.get(
                "longer_key_name"  # pyright: ignore
            )
            or []
        )
    ]

Running on ruff v0.1.3. Edit: Error still shows up on v0.1.4.

No specific rules for ruff format and config looks like this

select = [
  "F",      
  "I",     
  "RUF100", 
  "UP004",  
  "UP005",  
  "UP006", 
  "UP007",  
  "UP010", 
  "UP011", 
  "UP012", 
  "UP013", 
  "UP015", 
  "UP017",
  "UP018",
  "UP024", 
  "UP027", 
  "UP034", 
  "UP037", 

]
exclude = [".git", "__pycache__"]
extend-safe-fixes = ["UP006", "UP007"]
line-length = 88
target-version = "py311"

[isort]
required-imports = ["from __future__ import annotations"]
@charliermarsh charliermarsh added the formatter Related to the formatter label Nov 4, 2023
@roshanjrajan-zip
Copy link
Author

roshanjrajan-zip commented Nov 7, 2023

Any help would be greatly appreciated! I hoped it would be fixed here - #8431? But it does seem to be along the same avenue.

@dhruvmanila
Copy link
Member

Can you provide the original source code? The way you're describing the comment place, is it like this:

		# pyright: ignore[reportUnknownVariableType]
        for a in
        (

Or,

        for a in  # pyright: ignore[reportUnknownVariableType]
        (

@roshanjrajan-zip
Copy link
Author

It should be the second one. The code that I shared should show the same result that I am describing. Are you not able to reproduce it?

@roshanjrajan-zip
Copy link
Author

roshanjrajan-zip commented Nov 11, 2023

Unfortunately I can't even use # fmt:skip on the same line as the #pyright pragma comment to skip this change. The only way to get past this is to add the # fmt:skip on the line with just the parentheses.

def test_function(
    very_long_dictionary: dict,  # pyright:ignore
):
    something = [
        a["foo"]
        for a in  # pyright: ignore[reportUnknownVariableType]
        (  # fmt:skip <- Needed to add this here
            very_long_dictionary.get(
                "longer_key_name"  # pyright: ignore
            )
            or []
        )
    ]

@roshanjrajan-zip
Copy link
Author

roshanjrajan-zip commented Nov 11, 2023

Okay I got a way to reproduce this on terminal as it happens consistently in VSCode but not in terminal...
I suspect I see this in VSCode consistently because its probably run without cache across runs since it only formats the file on save.

Context: The most important line is where the # pyright: ignore[reportUnknownVariableType] is placed. It is supposed to be applied to the variable a and putting it on a different line invalidates its application when using pyright.

Code is in file test.py
Step 1. Copy code snippet to test.py

# test.py
def test_function(
    very_long_dictionary: dict,  # pyright:ignore
):
    something = [
        a["foo"]
        for a in  # pyright: ignore[reportUnknownVariableType]
        ( 
            very_long_dictionary.get(
                "longer_key_name"  # pyright: ignore
            )
            or []
        )
    ]

Step 2. Run ruff format --no-cache test.py.
It should look like this. This is a valid case and is a perfectly valid solution.

# test.py
def test_function(
    very_long_dictionary: dict,  # pyright:ignore
):
    something = [
        a["foo"]
        for a in (  # pyright: ignore[reportUnknownVariableType]
            very_long_dictionary.get(
                "longer_key_name"  # pyright: ignore
            )
            or []
        )
    ]

Step 3. Run ruff format --no-cache test.py.
It should now look like this which is incorrect. The comment isn't on the correct line and any further ruff format --no-cache test.py will keep the code as is.

# test.py
def test_function(
    very_long_dictionary: dict,  # pyright:ignore
):
    something = [
        a["foo"]
        for a in 
        (  # pyright: ignore[reportUnknownVariableType]
            very_long_dictionary.get(
                "longer_key_name"  # pyright: ignore
            )
            or []
        )
    ]

@roshanjrajan-zip
Copy link
Author

roshanjrajan-zip commented Nov 13, 2023

I think this maybe be related to this other issue that I am creating to take about differences in no_cache.
Uncertain whether it is the no_cache causing the problem or the way that the pragma comment is formatted.

@roshanjrajan-zip
Copy link
Author

@MichaReiser Do you have any code pointers of why there might be instability here? I'm trying to look at #8431 to see if there is anything there but uncertain.

@zanieb zanieb added the bug Something isn't working label Nov 16, 2023
@MichaReiser
Copy link
Member

MichaReiser commented Nov 27, 2023

This isn't related to caching but is an instability bug where the formatter requires two passes to reach the ultimate formatting (unfortunately, the worse). You can see this in the Playground where I copied the formatted code of the input as the second example.

The issue comes from that the comment is a dangling comment of the comprehension when formatting the input source, but becomes a leading comment of b.get when formatting it the second time.

A workaround to unblock you is to suppress formatting of the entire statement:

something = [
    a
    for a in # pyright: ignore[reportUnknownVariableType]
    (  
        b.get(
            "longer_key_name",
        )
    )
] # fmt: skip

@MichaReiser MichaReiser added this to the Formatter: Stable milestone Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working formatter Related to the formatter
Projects
None yet
Development

No branches or pull requests

5 participants