Start improving detection of syntax errors #16034
Conversation
Co-authored-by: Alex Waygood <[email protected]>
it turned out not to be enough to convert to a `Diagnostic` because ruff needs to be able to map the name of the `DiagnosticKind` to an error code. As far as I can tell so far, that requires defining normal `Violation` types
There was a problem hiding this comment.
I initially tried just implementing From<SyntaxError> for ruff_diagnostics::Diagnostic like in red-knot, but this caused problems in code_to_rule, which maps code names like match-before-python-310 back to rule codes. As it stands, every new SyntaxErrorKind will also need an entry in the syntax_errors macro and an arm in the match here.
This macro should pass along the documentation, though, so I think this will integrate well with the way other lint rules are documented, at least.
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| SYN001 | 6 | 6 | 0 | 0 | 0 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+6 -0 violations, +0 -0 fixes in 1 projects; 54 projects unchanged)
facebookresearch/chameleon (+6 -0 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview
+ chameleon/viewer/backend/models/chameleon_distributed.py:405:13: SYN001 Cannot use `match` statement on Python 3.9 (syntax was new in Python 3.10) + chameleon/viewer/backend/models/chameleon_distributed.py:818:17: SYN001 Cannot use `match` statement on Python 3.9 (syntax was new in Python 3.10) + chameleon/viewer/backend/models/chameleon_local.py:633:13: SYN001 Cannot use `match` statement on Python 3.9 (syntax was new in Python 3.10) + chameleon/viewer/backend/models/service.py:131:21: SYN001 Cannot use `match` statement on Python 3.9 (syntax was new in Python 3.10) + chameleon/viewer/backend/models/service.py:154:17: SYN001 Cannot use `match` statement on Python 3.9 (syntax was new in Python 3.10) + chameleon/viewer/backend/models/service.py:226:25: SYN001 Cannot use `match` statement on Python 3.9 (syntax was new in Python 3.10)
Changes by rule (1 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| SYN001 | 6 | 6 | 0 | 0 | 0 |
Formatter (stable)
✅ ecosystem check detected no format changes.
Formatter (preview)
✅ ecosystem check detected no format changes.
|
It would be great to have a design discussion first on how we plan on integrating this into red know and ruff from a ux perspective because this approach already implies some important decisions |
Brent's been writing up a design doc and he's shared an early draft with me. He will be sharing it with the rest of the team as well. I believe the idea of this (draft) PR is for it to be a prototype to share alongside the design doc. |
|
Yes, sorry about that. The intention is definitely to have a design discussion first. |
Summary
This is a first prototype of syntax error detection meant to accompany an internal design document (coming soon!).
This PR introduces the
ruff_python_syntax_errorscrate, which contains acheck_syntaxfunction for traversing the parsed AST and detectingSyntaxErrors, which can be converted intoDiagnostics in both ruff and red-knot.Only one new syntax error is currently detected,
MatchBeforePy310, the use ofmatchstatements in Python 3.9 or earlier. However,check_syntaxis wired into ruff and red-knot, so all future syntax errors should only require additions toruff_python_syntax_errors, with a caveat I'll note as a review comment below.Test Plan
There are two new, very simple test cases in
ruff_python_syntax_errorsitself, as well as a couple of new diagnostics in existing ruff tests. I think it would be nice to add at least one red-knot test that triggers the new diagnostic before merging.