Fix "# pyright: ignore" comment mobility

[etripier]

Description

Closes #3946

Hi! It looks like you have some special handling for # type: ignore comments to avoid moving said comments around due to positional meaning. I think this same special handling should also apply to # pyright: ignore comments.

The use case for # pyright: ignore comments is being able to ignore Pyright-specific diagnostic rules that might not make sense in the context of more generic # type: ignore comments.

Checklist - did you ...

• [x] Add an entry in CHANGES.md if necessary?
• [x] Add / update tests if necessary?
• [ ] Add new / update outdated documentation?

[JelleZijlstra]

Should we add this to black.comments.contains_pragma_comment instead? I think these pyright comments are more like noqa than like # type:, which is treated specially by the Python parser.

[etripier]

Ah, my understanding is that the pragma stuff only applies to string concatenation and splitting. I'm really hoping to fix the annoyance of having

blah_blah_blah = [ # pyright: ignore
    ...
]

shift into

blah_blah_blah = [
    ...
] # pyright: ignore

The behavior in https://github.com/psf/black/blob/de65741b8d49d78fa2675ef79b799cd35e92e7c1/src/black/lines.py#L263 is what I'd love to have, ideally.

[roshanjrajan-zip]

Hi! I wanted to see if there any concerns about gettin this merged other than fixing the conflicts?

If it isn't clear, we get scenarios where black transform code from this

some_long_variable_name = some_long_dict_name[0] # pyright: ignore[reportGeneralTypeIssues]

to this

some_long_variable_name = some_long_dict_name[
	0
] # pyright: ignore[reportGeneralTypeIssues]

where after the transformation, the ignore is no longer being applied to the actual line causing pyright to error. This looks similar to the # type: ignore fix and this would helpful to have! Let me know if you have any other questions!

Our current solution is use cast or do a rename that looks like this

a = some_long_dict_name[0] # pyright: ignore[reportGeneralTypeIssues]
some_long_variable_name = a  # pyright: ignore[reportGeneralTypeIssues]

which are both not ideal scenarios and is very tedious and adds unnecessary bloat to the codebase.

Also using # fmt: skip doesn't work because sometimes it is ignored and moves the comment anyways.

[roshanjrajan-zip]

@JelleZijlstra In this case, is it better to do change it in the contains_pragma_comment to get the intended behavior or does it make sense to keep it in is_type_comment?

[JelleZijlstra]

I need to look more into this to figure out exactly how we use these, but a few thoughts:

• I think we should treat the pyright comments more like the pylint comments than like the # type: ignore comments. # type: comments are extra special because they are treated specially by the Python AST, so it makes sense to be even more conservative with them.
• I'd prefer to find a solution that avoids Black having too much knowledge of magic comments used by specific tools, since that's bad for long-term maintainability.

[etripier]

Would it make more sense to add a Black configuration option opting certain types of patterned comments into the unsplittable behavior, then?

[vors]

Would love to see this feature, it's much needed.

[vors]

type: comments are extra special because they are treated specially by the Python AST, so it makes sense to be even more conservative with them

For my education, can you please elaborate how they are treated differently?

[vors]

Looks like the best path forward for our company at the moment is to fork black and apply this patch. We really need this functionality soon. This is very unfortunate, I hope there is a path forward in upstream to resolve this.

[vors]

Actually just tried applying the patch on 23.3.0 with python 3.9 and it doesn't seem to change anything... Maybe that's what you referred to @JelleZijlstra ?