astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
31.55k stars 1.06k forks source link

Emit diagnostics for new syntax as per the target Python version #6591

Open dhruvmanila opened 1 year ago

dhruvmanila commented 1 year ago

Our parser doesn't take into account the Python version aka target-version setting while parsing the source code. This means that we would allow having a match statement when the target Python version is 3.9 or lower. We want to signal this to the user.

One solution is to provide a diagnostic after the parsing is done. This diagnostic would indicate to the user that there's a syntax usage which isn't valid for the current target version. This is what's being done in Rome and Pyright[^1]. We would still continue linting the file and emit other diagnostics as well.

Following is a non-exhaustive list of syntax changes with the minimum required Python version:

3.13

3.12

3.11

3.10

3.9

3.8

[^1]: Pyright: Type alias statement requires Python 3.12 or newer

dhruvmanila commented 1 year ago

The Match and Type alias statement can be implemented in the AST Statement analyzer within the match arms of the respective statement and checking the target-version setting.

I would suggest to add this as a new rule under the RUF category and all of the above would be in the same rule code.

td-anne commented 10 months ago

There might be other new syntax worth addressing:

Some of these might be closer to functional changes than syntax and hard to detect. But perhaps the easy ones could be folded in? There are advantages to python 3.12 that might make people want to develop with it even though their code is meant to run on python 3.9 (say).

s-cork commented 5 months ago
  • [ ] 3.8 Walrus operator (if there's interest in supporting pre-3.8)

Just to say we have a use case for targeting 3.7. Thanks for the great work on this!

achimnol commented 5 months ago

So far, most new syntaxes were not "auto-applied" during formatting, but PEP-701 f-string placeholder updates are auto-applied to all codes. This prevents backporting those codes to older Python versions seamlessly as Ruff does not report syntax errors in older Python versions. Looking forward to see this issue to be resolved. :eyes:

zanieb commented 4 months ago

We'd appreciate contributions here if someone is interested in working on the project. We can do this relatively incrementally, but there is some initial complexity to determine how diagnostics are propagated.

@dhruvmanila / @charliermarsh will provide some additional details.

dhruvmanila commented 4 months ago

There might be other new syntax worth addressing:

@td-anne For visibility, I've updated the PR description based on your list but this issue is mainly focused on the syntax itself and not the semantic meaning. For example, dictionary union ({'a': 1} | {'b': 2}) is valid syntactically but raises a TypeError at runtime. So, the PR description is updated with the following:

  • [ ] 3.11 except *
  • [ ] 3.8 Walrus operator (if there's interest in supporting pre-3.8)
  • [ ] 3.10 Parenthesized with statements (with (a_gen() as a, b_gen() as b):
  • [ ] 3.12 type parameter syntax (def max[T](list[T]) -> T:)
  • [ ] 3.12-only f-strings (?)

But, the following can possibly be done as a separate rule and is not part of what we're proposing here.

  • [ ] 3.11 Variadic generics
  • [ ] 3.10 Type unions with | (?)
  • [ ] 3.9 dict unions with | (?)
  • [ ] 3.9 lower-case list[]/dict[] in type annotations

Can you expand on the following? I don't see any new syntax related to PEP 617.

  • [ ] 3.9 PEP 617 extended generator syntax
AlexWaygood commented 4 months ago

[ ] 3.11 Variadic generics

These did involve the introduction of some new syntax: unpacking using * at the outermost level of an expression in a type annotation was invalid syntax on earlier versions of Python. See https://peps.python.org/pep-0646/#grammar-changes

dhruvmanila commented 4 months ago

I see. Thanks for pointing that out. I'll update the PR description.

dhruvmanila commented 4 months ago

The goal here is to update Ruff to detect syntax which is invalid as per the target-version. There are two ways to achieve this:

  1. Update the parser to detect this, add new parse error types
  2. Add a new single rule

For (1),

For (2),

I'm more leaning towards (2). An open question here is to whether implement this as a new rule or add it to E999. The new rule can be added under the ruff group with the name as unsupported-syntax.

AlexWaygood commented 4 months ago

I support detecting these syntax errors via the linter (Option (2)) wherever possible. I think it'll be great to have these errors be # noqa-able and to be able to plug into the diagnostic system directly. However, I don't necessarily see a need to be dogmatic about it -- if there are specific syntax errors (such as parenthesized with items) that would be much easier to detect in the parser, then possibly we could detect some in the parser.

I'd much prefer to have these syntax errors be detected as part of E999 rather than adding a new lint rule. Although we'll be using a different part of ruff to detect them, from the perspective of users a syntax error is a syntax error. I think it will be pretty confusing for them if some syntax errors are detected by one rule and other syntax errors are detected by another rule. The effect of a syntax error for a user is always the same -- your code is never going to be compiled by Python, let alone run!

MichaReiser commented 4 months ago

I'd much prefer to have these syntax errors be detected as part of E999 rather than adding a new lint rule. Although we'll be using a different part of ruff to detect them, from the perspective of users a syntax error is a syntax error. I think it will be pretty confusing for them if some syntax errors are detected by one rule and other syntax errors are detected by another rule. The effect of a syntax error for a user is always the same -- your code is never going to be compiled by Python, let alone run!

I think one key difference between the two is that you should never suppress a syntax error that is an error regardless of the python version (I don't even know if we support suppressing them). I don't know if there are good reasons for suppressing python-version specific syntax errors but I could see an argument for that. I don't think these should be suppressed by inline noqa comments but a user might decide to suppress them with per-file-ignores (although a new pyrpoject.toml with the right requires-python version would be better)

AlexWaygood commented 4 months ago

The only reasons I can see for wanting to suppress a syntax error diagnostic are:

If you have the right target version in your config file, I can't personally see any reason why you'd want to be able to suppress Python-version-specific syntax error diagnostics, or treat them any differently from syntax that's invalid on all Python versions.

achimnol commented 2 months ago

Any updates on this? We are still holding back the ruff version due to the f-string backporting issue.