[red-knot] Detect version-related syntax errors by ntBre · Pull Request #16379 · astral-sh/ruff

ntBre · 2025-02-25T18:19:19Z

Summary

This PR extends version-related syntax error detection to red-knot. The main changes here are:

Passing ParseOptions specifying a PythonVersion to parser calls
Adding a python_version method to the Db trait to make this possible
Converting UnsupportedSyntaxErrors to Diagnostics
Updating existing mdtests to avoid unrelated syntax errors

My initial draft of (1) and (2) in #16090 instead tried passing a PythonVersion down to every parser call, but @MichaReiser suggested the Db approach instead here, and I think it turned out much nicer.

All of the new python_version methods look like this:

fn python_version(&self) -> ruff_python_ast::PythonVersion {
    Program::get(self).python_version(self)
}

with the exception of the TestDb in ruff_db, which hard-codes PythonVersion::latest().

Test Plan

Existing mdtests, plus a new mdtest to see at least one of the new diagnostics.

codspeed-hq · 2025-02-25T19:06:25Z

CodSpeed Performance Report

Merging #16379 will not alter performance

_{Comparing brent/syntax-errors-red-knot (2ede1e0) with main (d2ebfd6)}

Summary

✅ 33 untouched benchmarks

github-actions · 2025-02-25T19:10:22Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

This PR is the first in a series derived from #16308, each of which add support for detecting one version-related syntax error from #6591. This one should be the largest because it also includes a couple of additional changes: 1. the `syntax_errors!` macro, which makes adding more variants a bit easier 2. the `Parser::add_unsupported_syntax_error` method Otherwise I think the general structure will be the same for each syntax error: * Detecting the error in the parser * Inline parser tests for the new error * New ruff CLI tests for the new error Because of the second point here, this PR is currently stacked on #16357. As noted above, there are new inline parser tests, as well as new ruff CLI tests. Once #16379 is resolved, there should also be new mdtests for red-knot, but this PR does not currently include those.

## Summary This PR is the first in a series derived from #16308, each of which add support for detecting one version-related syntax error from #6591. This one should be the largest because it also includes the addition of the `Parser::add_unsupported_syntax_error` method Otherwise I think the general structure will be the same for each syntax error: * Detecting the error in the parser * Inline parser tests for the new error * New ruff CLI tests for the new error ## Test Plan As noted above, there are new inline parser tests, as well as new ruff CLI tests. Once #16379 is resolved, there should also be new mdtests for red-knot, but this PR does not currently include those.

This PR is the first in a series derived from #16308, each of which add support for detecting one version-related syntax error from #6591. This one should be the largest because it also includes a couple of additional changes: 1. the `syntax_errors!` macro, which makes adding more variants a bit easier 2. the `Parser::add_unsupported_syntax_error` method Otherwise I think the general structure will be the same for each syntax error: * Detecting the error in the parser * Inline parser tests for the new error * New ruff CLI tests for the new error Because of the second point here, this PR is currently stacked on #16357. As noted above, there are new inline parser tests, as well as new ruff CLI tests. Once #16379 is resolved, there should also be new mdtests for red-knot, but this PR does not currently include those.

github-actions · 2025-04-15T21:28:43Z

`mypy_primer` results

Changes were detected when running on open source projects

psycopg (https://github.com/psycopg/psycopg)
+ error[invalid-syntax] /tmp/mypy_primer/projects/psycopg/tools/async_to_sync.py:243:9: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
+ error[invalid-syntax] /tmp/mypy_primer/projects/psycopg/tools/async_to_sync.py:345:13: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
+ error[invalid-syntax] /tmp/mypy_primer/projects/psycopg/tools/async_to_sync.py:355:9: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
+ error[invalid-syntax] /tmp/mypy_primer/projects/psycopg/tools/async_to_sync.py:371:9: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
+ error[invalid-syntax] /tmp/mypy_primer/projects/psycopg/tools/async_to_sync.py:379:13: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
+ error[invalid-syntax] /tmp/mypy_primer/projects/psycopg/tools/async_to_sync.py:390:9: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
+ error[invalid-syntax] /tmp/mypy_primer/projects/psycopg/tools/async_to_sync.py:416:13: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
+ error[invalid-syntax] /tmp/mypy_primer/projects/psycopg/tools/async_to_sync.py:459:9: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
+ error[invalid-syntax] /tmp/mypy_primer/projects/psycopg/tools/bump_version.py:103:9: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
- Found 1589 diagnostics
+ Found 1598 diagnostics

sharkdp

This looks great — thank you @ntBre!

As you suggested in the PR description, I think we should try to be a bit more fine grained with increasing Python versions in the tests. Python typing has evolved a lot between 3.9 and 3.13, and we want to make sure that a large part of our tests still work for 3.9.

crates/red_knot_python_semantic/resources/mdtest/annotations/callable.md

crates/red_knot_python_semantic/resources/mdtest/assignment/annotations.md

crates/red_knot_python_semantic/resources/mdtest/call/methods.md

crates/red_knot_python_semantic/resources/mdtest/dataclasses.md

crates/red_knot_python_semantic/resources/mdtest/function/parameters.md

crates/red_knot_python_semantic/resources/mdtest/import/star.md

crates/red_knot_python_semantic/resources/mdtest/metaclass.md

crates/red_knot_python_semantic/resources/mdtest/narrow/type.md

crates/red_knot_python_semantic/resources/mdtest/stubs/class.md

crates/red_knot_python_semantic/resources/mdtest/type_properties/is_disjoint_from.md

ntBre · 2025-04-17T14:02:22Z

Thanks @sharkdp! Sorry about the mdtests, I didn't mean to make you go through them all, but I appreciate it. I'll update those now.

sharkdp · 2025-04-17T17:41:15Z

Thanks @sharkdp! Sorry about the mdtests, I didn't mean to make you go through them all, but I appreciate it. I'll update those now.

No worries! I was interested in how it affected particular parts of our test suite and looked at it anyway.

Interestingly, we now see some ecosystem changes (I merged a PR which added some new projects this morning). And they are correct, because https://github.com/psycopg/psycopg does not specify a python-version. I would like to make some improvements to how the ecosystem checks work (how exactly we run red knot on projects), but for now, I think we can just ignore it. Might even be interesting to have a few true positives in there.

ntBre · 2025-04-17T17:53:44Z

Interestingly, we now see some ecosystem changes (I merged a PR which added some new projects this morning). And they are correct, because https://github.com/psycopg/psycopg does not specify a python-version. I would like to make some improvements to how the ecosystem checks work (how exactly we run red knot on projects), but for now, I think we can just ignore it. Might even be interesting to have a few true positives in there.

Oh good catch, I hadn't scrolled back up to check these after merging main.

So go ahead and merge? I can also leave it open a bit longer in case anyone else wants to review it.

(I see some emotes from @AlexWaygood, so I can consider that two reviews 😆)

sharkdp · 2025-04-17T17:58:34Z

I say, go ahead an merge it! 😄 Others can still do post-merge reviews, if they want.

* main: [red-knot] Detect version-related syntax errors (#16379) [`pyflakes`] Add fix safety section (`F841`) (#17410) [red-knot] Add `KnownFunction` variants for `is_protocol`, `get_protocol_members` and `runtime_checkable` (#17450) Bump 0.11.6 (#17449) Auto generate `visit_source_order` (#17180) [red-knot] Initial tests for protocols (#17436) [red-knot] Dataclasses: synthesize `__init__` with proper signature (#17428) [red-knot] Dataclasses: support `order=True` (#17406)

dhruvmanila

Looks good!

* main: (123 commits) [red-knot] Handle explicit class specialization in type expressions (#17434) [red-knot] allow assignment expression in call compare narrowing (#17461) [red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy (#17451) [red-knot] Type narrowing for assertions (take 2) (#17345) [red-knot] class bases are not affected by __future__.annotations (#17456) [red-knot] Add support for overloaded functions (#17366) [`pyupgrade`] Add fix safety section to docs (`UP036`) (#17444) [red-knot] more type-narrowing in match statements (#17302) [red-knot] Add some narrowing for assignment expressions (#17448) [red-knot] Understand `typing.Protocol` and `typing_extensions.Protocol` as equivalent (#17446) Server: Use `min` instead of `max` to limit the number of threads (#17421) [red-knot] Detect version-related syntax errors (#16379) [`pyflakes`] Add fix safety section (`F841`) (#17410) [red-knot] Add `KnownFunction` variants for `is_protocol`, `get_protocol_members` and `runtime_checkable` (#17450) Bump 0.11.6 (#17449) Auto generate `visit_source_order` (#17180) [red-knot] Initial tests for protocols (#17436) [red-knot] Dataclasses: synthesize `__init__` with proper signature (#17428) [red-knot] Dataclasses: support `order=True` (#17406) [red-knot] Super-basic generic inference at call sites (#17301) ...

ntBre added the ty Multi-file analysis & type inference label Feb 25, 2025

ntBre changed the title ~~[red-knot] Detect version-related syntax errors~~ [red-knot] Detect version-related syntax errors Feb 25, 2025

ntBre force-pushed the brent/syntax-errors-red-knot branch 3 times, most recently from 8014499 to f0005f7 Compare February 25, 2025 19:01

ntBre mentioned this pull request Feb 25, 2025

Detect assignment expressions before Python 3.8 #16383

Merged

Base automatically changed from brent/syntax-errors-parser to main February 26, 2025 04:03

ntBre force-pushed the brent/syntax-errors-red-knot branch from f0005f7 to bc274bd Compare February 26, 2025 04:15

ntBre mentioned this pull request Mar 4, 2025

[red-knot] Several failing tests for generics #16509

Merged

ntBre force-pushed the brent/syntax-errors-red-knot branch from bc274bd to 09bd070 Compare April 15, 2025 21:26

ntBre force-pushed the brent/syntax-errors-red-knot branch 2 times, most recently from 203cc85 to 6653aad Compare April 16, 2025 16:32

ntBre added 6 commits April 16, 2025 12:48

restore red-knot changes (compiling but many test failures)

304174f

add Db::python_version and update all tests to pass

0719de2

add title to avoid mdformat

cb09899

fix fuzz build

f237617

factor out create_unsupported_syntax_diagnostic

40f1fcf

clean up comments and narrow one config

b0ec1b4

ntBre force-pushed the brent/syntax-errors-red-knot branch from 6653aad to b0ec1b4 Compare April 16, 2025 16:57

ntBre marked this pull request as ready for review April 16, 2025 17:13

ntBre requested review from AlexWaygood, MichaReiser, carljm, dcreager and sharkdp as code owners April 16, 2025 17:13

sharkdp approved these changes Apr 17, 2025

View reviewed changes

ntBre added 5 commits April 17, 2025 10:17

narrow python-version configuration

9839031

revert heading addition now that it's not needed

500537c

more narrowing

808d170

Merge branch 'main' into brent/syntax-errors-red-knot

9700240

add python-version to new mdtests

2ede1e0

ntBre merged commit 9c47b6d into main Apr 17, 2025
23 checks passed

ntBre deleted the brent/syntax-errors-red-knot branch April 17, 2025 18:00

dhruvmanila reviewed Apr 17, 2025

View reviewed changes

dcreager mentioned this pull request Apr 18, 2025

[red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy #17451

Merged

Comments

Conversation

ntBre commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

codspeed-hq bot commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #16379 will not alter performance

Summary

Uh oh!

github-actions bot commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

github-actions bot commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

sharkdp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ntBre commented Apr 17, 2025

Uh oh!

sharkdp commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ntBre commented Apr 17, 2025

Uh oh!

sharkdp commented Apr 17, 2025

Uh oh!

Uh oh!

dhruvmanila left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ntBre commented Feb 25, 2025 •

edited

Loading

codspeed-hq bot commented Feb 25, 2025 •

edited

Loading

github-actions bot commented Feb 25, 2025 •

edited

Loading

`ruff-ecosystem` results

github-actions bot commented Apr 15, 2025 •

edited

Loading

`mypy_primer` results

sharkdp commented Apr 17, 2025 •

edited

Loading