Allow passing `ParseOptions` to inline tests #16357

ntBre · 2025-02-24T19:50:13Z

Summary

The changes here are based on the similar behavior in biome's collect_tests function, which allows a syntax for inline test headers like

label language name [options]

Before this PR, we only allowed headers of the form label name, where label is either test_err or test_ok and name is the name of the test. This PR adds support for an optional, trailing options field, corresponding to JSON-serialized ParseOptions. These get written to a *.options.json file alongside the inline test script and read when that test is run.

This is currently stacked on #16090 so that I had something to test.

I wanted to add serde as a dev dependency only, but since these are run as integration tests, I had to add it as a full dependency. For this exact use case, we could just parse a trailing version string like "3.10" but I thought we might want the full ParseOptions for the future.

Test Plan

Existing inline tests, plus two new inline tests for match-before-py310.

I initially tried passing the target version to all of the parser functions, but this required a huge number of changes. The downside of the current approach is that we're likely to accumulate quite a large Vec<SyntaxError> in the parser, only to filter it down later. The upside is that we don't have to fix every single call site of every parser function right now

could be Copy as well, currently because Mode and PythonVersion are

This reverts commit 3f43c82. This didn't work because the fuzzing only runs with cfg(test) not cfg(fuzzing)

codspeed-hq · 2025-02-25T18:08:57Z

CodSpeed Performance Report

Merging #16357 will not alter performance

_{Comparing brent/parser-tests (62b43a7) with main (dd6f623)}

Summary

✅ 32 untouched benchmarks

ntBre · 2025-02-25T18:09:00Z

I rebased on #16090 to get the new SyntaxError name and switched to the pragma implementation. It seems quite nice and ended up being less code too. The only piece I might be missing is including the code file contents in the snapshot, but I wasn't quite sure if that was still needed. It sounds like that could be a follow-up either way.

I made serde an optional dependency gated behind the serde feature in the parser. You have to pass --features serde to run the parser tests with something like cargo test -p ruff_python_parser --features serde, but I think CI runs everything with --all-features. This feels a bit hacky but maybe it's okay? I left that commit last so it would be easy to revert.

ntBre · 2025-02-25T18:24:58Z

My other branch must be missing Doug's alist changes, at least I assume that's where the codspeed failure is coming from. I expect that to go away when I change the base branch to main.

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.

ntBre · 2025-02-26T00:10:59Z

While trying to use this branch as a base for detecting some syntax errors, I realized that I forgot to actually check for the new errors in the valid syntax case. It's difficult to test this case because it requires a version-related syntax error in a test_ok inline test. I guess a #[should_panic] test calling test_valid_syntax directly could work, but I can also confirm that these changes now detect the false-positive I was debugging on the other branch.

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 builds on the changes in #16220 to pass a target Python version to the parser. It also adds the `Parser::unsupported_syntax_errors` field, which collects version-related syntax errors while parsing. These syntax errors are then turned into `Message`s in ruff (in preview mode). This PR only detects one syntax error (`match` statement before Python 3.10), but it has been pretty quick to extend to several other simple errors (see #16308 for example). ## Test Plan The current tests are CLI tests in the linter crate, but these could be supplemented with inline parser tests after #16357. I also tested the display of these syntax errors in VS Code: ![image](https://github.com/user-attachments/assets/062b4441-740e-46c3-887c-a954049ef26e) ![image](https://github.com/user-attachments/assets/101f55b8-146c-4d59-b6b0-922f19bcd0fa) --------- Co-authored-by: Alex Waygood <[email protected]>

crates/ruff_python_parser/src/lib.rs

crates/ruff_server/src/lint.rs

crates/ruff_python_parser/src/lib.rs

crates/ruff_server/src/lint.rs

crates/ruff_python_parser/src/parser/options.rs

ntBre · 2025-02-26T15:27:00Z

Based on a discussion with @MichaReiser on Discord, I backed out the serde feature changes and added JsonParseOptions and JsonMode types in fixtures.rs to keep serde as a dev-dependency.

This avoids needing to run the parser integration tests with the serde feature enabled or requiring serde as a full dependency.

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.

ntBre and others added 30 commits February 24, 2025 08:48

add unused Parser::syntax_errors field

e871194

add hard-coded python version and detect match

591063f

make PythonVersion more public, convert SyntaxError in red-knot

e628e66

process SyntaxErrors in ruff

36ee9cd

pass tests

bfb3bb5

add ruff test case

a6b8e34

detect late future imports in the parser

288821d

pass f404 tests

ed67b01

check if rules are enabled before converting to diagnostics

fbf8765

add a todo about duplicate diagnostics

5768f71

clippy

2ecf3f2

update to use ast::PythonVersion and Span

ef858bf

remove LateFutureImport detection from the parser

d817289

tidy up

6e4b956

clean up after rebase

1805a65

add ParseOptions::target_version

cf77087

filter syntax errors when parsing, store version on SyntaxError

3c67b03

make ParseOptions Clone

09ccf65

could be Copy as well, currently because Mode and PythonVersion are

pass ParseOptions::target_version in the linter

a6bc0db

pass ParseOptions::target_version in red-knot

49a48f2

ignore version-related syntax errors when fuzzing

79e6322

Red-knot no longer panics!

12d04b0

Fix fuzzing script and fix Rust formatting

efb9c2a

Revert "ignore version-related syntax errors when fuzzing"

ee30486

This reverts commit 3f43c82. This didn't work because the fuzzing only runs with cfg(test) not cfg(fuzzing)

pass check_file_skips_type_checking_when by initializing settings

c8a22c5

fix clippy lint about &impl ToString

380aaff

pass version to new parsed_module call

80be7f7

use LinterSettings::resolve_target_version

6b69bb5

add ok test for match

cd02f61

ntBre added 6 commits February 25, 2025 12:38

clippy

a0cb2e8

use an options pragma

5f935e3

revert test installation changes

4fe6788

don't print empty Errors section, update snap

e3b770b

update snap after rebase

72d9ea1

activate serde with a feature

ef8c37e

ntBre force-pushed the brent/parser-tests branch from d2950d6 to ef8c37e Compare February 25, 2025 18:03

ntBre mentioned this pull request Feb 25, 2025

Detect assignment expressions before Python 3.8 #16383

Draft

ntBre added 4 commits February 25, 2025 18:00

fix is_valid check for test_valid_syntax

ae800ec

fix error printing for test_valid_syntax

d76ce1a

default to latest python to avoid breaking existing tests

8304f75

use latest instead of 3.13

45101ce

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

ntBre requested a review from AlexWaygood as a code owner February 26, 2025 04:03

Merge branch 'main' into brent/parser-tests

e2db37d

MichaReiser approved these changes Feb 26, 2025

View reviewed changes

crates/ruff_python_parser/src/lib.rs Outdated Show resolved Hide resolved

crates/ruff_server/src/lint.rs Outdated Show resolved Hide resolved

dhruvmanila approved these changes Feb 26, 2025

View reviewed changes

crates/ruff_python_parser/src/lib.rs Outdated Show resolved Hide resolved

crates/ruff_server/src/lint.rs Outdated Show resolved Hide resolved

crates/ruff_python_parser/src/parser/options.rs Outdated Show resolved Hide resolved

ntBre added 4 commits February 26, 2025 08:39

delete duplicate diagnostics.chain

e82b83a

use kebab-case

f854172

delete outdated comment

7329128

move serde to dev-dependency, work around with Json* types

62b43a7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow passing `ParseOptions` to inline tests #16357

Allow passing `ParseOptions` to inline tests #16357

ntBre commented Feb 24, 2025

codspeed-hq bot commented Feb 25, 2025 •

edited

Loading

ntBre commented Feb 25, 2025

ntBre commented Feb 25, 2025

ntBre commented Feb 26, 2025

ntBre commented Feb 26, 2025

Allow passing ParseOptions to inline tests #16357

Are you sure you want to change the base?

Allow passing ParseOptions to inline tests #16357

Conversation

ntBre commented Feb 24, 2025

Summary

Test Plan

codspeed-hq bot commented Feb 25, 2025 • edited Loading

CodSpeed Performance Report

Merging #16357 will not alter performance

Summary

ntBre commented Feb 25, 2025

ntBre commented Feb 25, 2025

ntBre commented Feb 26, 2025

ntBre commented Feb 26, 2025

Allow passing `ParseOptions` to inline tests #16357

Allow passing `ParseOptions` to inline tests #16357

codspeed-hq bot commented Feb 25, 2025 •

edited

Loading