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

Could a shorter alias be accepted in place of e.g., pyright: ignore[reportReallyLongProblemName]? #961

Open
finite-state-machine opened this issue Dec 19, 2024 · 5 comments
Labels
config issues relating to config (pyproject.toml, pyrightconfig.json, LSP config or vscode extension) type checking / linting issues relating to existing diagnostic rules or proposals for new diagnostic rules

Comments

@finite-state-machine
Copy link

finite-state-machine commented Dec 19, 2024

Description of problem

I've noticed a lot of [based]pyright issue names are lengthy. Since there's no way (AFAIK) to make a pyright: ignore[...] directive effective other than on the line it occurs, this often leads to lines of code much longer than any reasonable line length limit.

For example, ␣␣# pyright: ignore[reportUnknownVariableType] adds 46 columns to an existing line of code; for a typical method body (2 indentations, 4 spaces each) this leaves only 25 columns for the code itself before exceeding the 79-column limit given by PEP 8.

This isn't just a question of keeping a linter happy; I like to be able to fit many columns of code side-by-side on my display at once, so frequently needing very long lines does limit how many different code regions I can see at any given time.

Possible approaches

It would be great to make directive placement more flexible (much like pylint's disable-next=rulename, which applies to the next line rather than the current line as disable= does) but I can appreciate that introducing behaviours incompatible with vanilla pyright may be worth avoiding.

If there were a way to express these directives in a more concise way, that would be helpful.

Systematic abbreviation of rule names

[Edited to add:] I can appreciate that providing a second, shorter set of names covering every rule might be an ongoing maintenance burden which basedpyright's devs prefer to avoid.

As a very simple first step, could we perhaps treat pyright: ignore[SomeProblem] as an alias for pyright: ignore[reportSomeProblem]?

We might also consider allowing more general abbreviations based on simple, predictable rules. For example, we might accept ignore[UnkVarType] as an alias for ignore[reportUnknownVariableType] with rules such as:

  • the leading report must be omitted
  • each CamelCased word in the full name (Unknown, Variable, Type) must appear in the abbreviation:
    • the leading uppercase letter must appear verbatim
    • it must be followed by a (possibly empty) ordered subset of the lowercase letters belonging to the same word (e.g. Variable may be represented as Vrbl)
  • the result must unambiguously match exactly one known error name

I'm not married to these rules; they're just a possibility to get the discussion started.

@finite-state-machine finite-state-machine changed the title Could a shorter alias be accepted in place of e.g., pyright: ignore[reportReallyLongProblemNames]? Could a shorter alias be accepted in place of e.g., pyright: ignore[reportReallyLongProblemName]? Dec 19, 2024
@finite-state-machine
Copy link
Author

I'll add: users of other code quality tools may need to fit several directives on the same line. For example, if I want to keep my code friendly to both pyright and mypy, requiring two, separate directives without a line break does make it very difficult to keep code width anything like reasonable.

I wonder if we might also consider supporting a file-level # pyright: directive which declares that each occurence of type: ignore[mypyRuleName] should be interpreted by basedpyright as an alias for # pyright: ignore[reportPyrightsLongNameForTheSameIssue].

@DetachHead
Copy link
Owner

For example, ␣␣# pyright: ignore[reportUnknownVariableType] adds 46 columns to an existing line of code; for a typical method body (2 indentations, 4 spaces each) this leaves only 25 columns for the code itself before exceeding the 79-column limit given by PEP 8.

imo this is quite an outdated rule that only really made sense when everybody had tiny screens. both black and ruff default to slightly longer because they consider 79 to be too short - https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length

It would be great to make directive placement more flexible (much like pylint's disable-next=rulename, which applies to the next line rather than the current line as disable= does) but I can appreciate that introducing behaviours incompatible with vanilla pyright may be worth avoiding.

[Edited to add:] I can appreciate that providing a second, shorter set of names covering every rule might be an ongoing maintenance burden which basedpyright's devs prefer to avoid.

i'm not too concerned about breaking compatibility with pyright as long as users have the option to stick with the old behavior. though i am cautious of adding too many ways to accomplish the same thing, because it adds more room for bugs for little benefit.

As a very simple first step, could we perhaps treat pyright: ignore[SomeProblem] as an alias for pyright: ignore[reportSomeProblem]?

We might also consider allowing more general abbreviations based on simple, predictable rules. For example, we might accept ignore[UnkVarType] as an alias for ignore[reportUnknownVariableType] with rules such as:

i'm also wary of implementing things like this, because it's ambiguous what the valid abbreviations would be which adds more mental burden for the user to remember each one. tbh out of all linters/type checkers i've used, pyright has handled this the best imo. ruff, pylint, etc. require you to use these cryptic, number codes that are not descriptive of the error they're suppressing at all, and mypy uses different names for the ignore codes to the setting used to enable them.

i like how simple pyright's system is. a descriptive code name for the rule with a consistent way to suppress them. however i'm aware there is room for improvement. i would like to add support for disabling a rule for blocks of code like so:

# pyright: disable=reportUnknownVariableType
... # several lines of unknown variable types
# pyright: enable=reportUnknownVariableType

would this be a suitable solution for you?

@DetachHead DetachHead added type checking / linting issues relating to existing diagnostic rules or proposals for new diagnostic rules config issues relating to config (pyproject.toml, pyrightconfig.json, LSP config or vscode extension) labels Dec 20, 2024
@KotlinIsland
Copy link
Collaborator

KotlinIsland commented Dec 20, 2024

I've always thought the report part is redundant tbh

additionally, the ignore part is derived from type: ignore, which is an insane hack based in type comments, i.e. type: int

seeing as there aren't any other pyright directives, what about:

1 + ""  # pyright: OperatorIssue

@DetachHead
Copy link
Owner

i don't like that because it doesn't clarify that it's suppressing an error

@KotlinIsland
Copy link
Collaborator

eh, i think it's fairly straight forward: 'there is a pyright moment on this line and it is "operatorissue"'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config issues relating to config (pyproject.toml, pyrightconfig.json, LSP config or vscode extension) type checking / linting issues relating to existing diagnostic rules or proposals for new diagnostic rules
Projects
None yet
Development

No branches or pull requests

3 participants