Skip to content

Commit 7880636

Browse files
ntBreAlexWaygood
andauthored
Start detecting version-related syntax errors in the parser (#16090)
## 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]>
1 parent b39a4ad commit 7880636

File tree

14 files changed

+356
-37
lines changed

14 files changed

+356
-37
lines changed

crates/ruff/src/cache.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,14 +586,15 @@ mod tests {
586586
use anyhow::Result;
587587
use filetime::{set_file_mtime, FileTime};
588588
use itertools::Itertools;
589+
use ruff_linter::settings::LinterSettings;
589590
use test_case::test_case;
590591

591592
use ruff_cache::CACHE_DIR_NAME;
592593
use ruff_linter::message::Message;
593594
use ruff_linter::package::PackageRoot;
594595
use ruff_linter::settings::flags;
595596
use ruff_linter::settings::types::UnsafeFixes;
596-
use ruff_python_ast::PySourceType;
597+
use ruff_python_ast::{PySourceType, PythonVersion};
597598
use ruff_workspace::Settings;
598599

599600
use crate::cache::{self, FileCache, FileCacheData, FileCacheKey};
@@ -611,6 +612,10 @@ mod tests {
611612

612613
let settings = Settings {
613614
cache_dir,
615+
linter: LinterSettings {
616+
unresolved_target_version: PythonVersion::PY310,
617+
..Default::default()
618+
},
614619
..Settings::default()
615620
};
616621

crates/ruff/tests/lint.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2627,3 +2627,77 @@ class A(Generic[T]):
26272627
"
26282628
);
26292629
}
2630+
2631+
#[test]
2632+
fn match_before_py310() {
2633+
// ok on 3.10
2634+
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
2635+
.args(STDIN_BASE_OPTIONS)
2636+
.args(["--stdin-filename", "test.py"])
2637+
.arg("--target-version=py310")
2638+
.arg("-")
2639+
.pass_stdin(
2640+
r#"
2641+
match 2:
2642+
case 1:
2643+
print("it's one")
2644+
"#
2645+
),
2646+
@r"
2647+
success: true
2648+
exit_code: 0
2649+
----- stdout -----
2650+
All checks passed!
2651+
2652+
----- stderr -----
2653+
"
2654+
);
2655+
2656+
// ok on 3.9 without preview
2657+
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
2658+
.args(STDIN_BASE_OPTIONS)
2659+
.args(["--stdin-filename", "test.py"])
2660+
.arg("--target-version=py39")
2661+
.arg("-")
2662+
.pass_stdin(
2663+
r#"
2664+
match 2:
2665+
case 1:
2666+
print("it's one")
2667+
"#
2668+
),
2669+
@r"
2670+
success: true
2671+
exit_code: 0
2672+
----- stdout -----
2673+
All checks passed!
2674+
2675+
----- stderr -----
2676+
"
2677+
);
2678+
2679+
// syntax error on 3.9 with preview
2680+
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
2681+
.args(STDIN_BASE_OPTIONS)
2682+
.args(["--stdin-filename", "test.py"])
2683+
.arg("--target-version=py39")
2684+
.arg("--preview")
2685+
.arg("-")
2686+
.pass_stdin(
2687+
r#"
2688+
match 2:
2689+
case 1:
2690+
print("it's one")
2691+
"#
2692+
),
2693+
@r"
2694+
success: false
2695+
exit_code: 1
2696+
----- stdout -----
2697+
test.py:2:1: SyntaxError: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
2698+
Found 1 error.
2699+
2700+
----- stderr -----
2701+
"
2702+
);
2703+
}

crates/ruff_linter/src/linter.rs

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ use rustc_hash::FxHashMap;
1010

1111
use ruff_diagnostics::Diagnostic;
1212
use ruff_notebook::Notebook;
13-
use ruff_python_ast::{ModModule, PySourceType};
13+
use ruff_python_ast::{ModModule, PySourceType, PythonVersion};
1414
use ruff_python_codegen::Stylist;
1515
use ruff_python_index::Indexer;
16-
use ruff_python_parser::{ParseError, Parsed};
16+
use ruff_python_parser::{ParseError, ParseOptions, Parsed, UnsupportedSyntaxError};
1717
use ruff_source_file::SourceFileBuilder;
1818
use ruff_text_size::Ranged;
1919

@@ -71,6 +71,7 @@ pub fn check_path(
7171
source_kind: &SourceKind,
7272
source_type: PySourceType,
7373
parsed: &Parsed<ModModule>,
74+
target_version: PythonVersion,
7475
) -> Vec<Diagnostic> {
7576
// Aggregate all diagnostics.
7677
let mut diagnostics = vec![];
@@ -104,8 +105,6 @@ pub fn check_path(
104105
));
105106
}
106107

107-
let target_version = settings.resolve_target_version(path);
108-
109108
// Run the filesystem-based rules.
110109
if settings
111110
.rules
@@ -335,7 +334,8 @@ pub fn add_noqa_to_path(
335334
settings: &LinterSettings,
336335
) -> Result<usize> {
337336
// Parse once.
338-
let parsed = ruff_python_parser::parse_unchecked_source(source_kind.source_code(), source_type);
337+
let target_version = settings.resolve_target_version(path);
338+
let parsed = parse_unchecked_source(source_kind, source_type, target_version);
339339

340340
// Map row and column locations to byte slices (lazily).
341341
let locator = Locator::new(source_kind.source_code());
@@ -367,6 +367,7 @@ pub fn add_noqa_to_path(
367367
source_kind,
368368
source_type,
369369
&parsed,
370+
target_version,
370371
);
371372

372373
// Add any missing `# noqa` pragmas.
@@ -393,7 +394,8 @@ pub fn lint_only(
393394
source_type: PySourceType,
394395
source: ParseSource,
395396
) -> LinterResult {
396-
let parsed = source.into_parsed(source_kind, source_type);
397+
let target_version = settings.resolve_target_version(path);
398+
let parsed = source.into_parsed(source_kind, source_type, target_version);
397399

398400
// Map row and column locations to byte slices (lazily).
399401
let locator = Locator::new(source_kind.source_code());
@@ -425,12 +427,20 @@ pub fn lint_only(
425427
source_kind,
426428
source_type,
427429
&parsed,
430+
target_version,
428431
);
429432

433+
let syntax_errors = if settings.preview.is_enabled() {
434+
parsed.unsupported_syntax_errors()
435+
} else {
436+
&[]
437+
};
438+
430439
LinterResult {
431440
messages: diagnostics_to_messages(
432441
diagnostics,
433442
parsed.errors(),
443+
syntax_errors,
434444
path,
435445
&locator,
436446
&directives,
@@ -443,6 +453,7 @@ pub fn lint_only(
443453
fn diagnostics_to_messages(
444454
diagnostics: Vec<Diagnostic>,
445455
parse_errors: &[ParseError],
456+
unsupported_syntax_errors: &[UnsupportedSyntaxError],
446457
path: &Path,
447458
locator: &Locator,
448459
directives: &Directives,
@@ -461,6 +472,9 @@ fn diagnostics_to_messages(
461472
parse_errors
462473
.iter()
463474
.map(|parse_error| Message::from_parse_error(parse_error, locator, file.deref().clone()))
475+
.chain(unsupported_syntax_errors.iter().map(|syntax_error| {
476+
Message::from_unsupported_syntax_error(syntax_error, file.deref().clone())
477+
}))
464478
.chain(diagnostics.into_iter().map(|diagnostic| {
465479
let noqa_offset = directives.noqa_line_for.resolve(diagnostic.start());
466480
Message::from_diagnostic(diagnostic, file.deref().clone(), noqa_offset)
@@ -491,11 +505,12 @@ pub fn lint_fix<'a>(
491505
// Track whether the _initial_ source code is valid syntax.
492506
let mut is_valid_syntax = false;
493507

508+
let target_version = settings.resolve_target_version(path);
509+
494510
// Continuously fix until the source code stabilizes.
495511
loop {
496512
// Parse once.
497-
let parsed =
498-
ruff_python_parser::parse_unchecked_source(transformed.source_code(), source_type);
513+
let parsed = parse_unchecked_source(&transformed, source_type, target_version);
499514

500515
// Map row and column locations to byte slices (lazily).
501516
let locator = Locator::new(transformed.source_code());
@@ -527,6 +542,7 @@ pub fn lint_fix<'a>(
527542
&transformed,
528543
source_type,
529544
&parsed,
545+
target_version,
530546
);
531547

532548
if iterations == 0 {
@@ -573,11 +589,18 @@ pub fn lint_fix<'a>(
573589
report_failed_to_converge_error(path, transformed.source_code(), &diagnostics);
574590
}
575591

592+
let syntax_errors = if settings.preview.is_enabled() {
593+
parsed.unsupported_syntax_errors()
594+
} else {
595+
&[]
596+
};
597+
576598
return Ok(FixerResult {
577599
result: LinterResult {
578600
messages: diagnostics_to_messages(
579601
diagnostics,
580602
parsed.errors(),
603+
syntax_errors,
581604
path,
582605
&locator,
583606
&directives,
@@ -680,16 +703,35 @@ pub enum ParseSource {
680703
impl ParseSource {
681704
/// Consumes the [`ParseSource`] and returns the parsed [`Parsed`], parsing the source code if
682705
/// necessary.
683-
fn into_parsed(self, source_kind: &SourceKind, source_type: PySourceType) -> Parsed<ModModule> {
706+
fn into_parsed(
707+
self,
708+
source_kind: &SourceKind,
709+
source_type: PySourceType,
710+
target_version: PythonVersion,
711+
) -> Parsed<ModModule> {
684712
match self {
685-
ParseSource::None => {
686-
ruff_python_parser::parse_unchecked_source(source_kind.source_code(), source_type)
687-
}
713+
ParseSource::None => parse_unchecked_source(source_kind, source_type, target_version),
688714
ParseSource::Precomputed(parsed) => parsed,
689715
}
690716
}
691717
}
692718

719+
/// Like [`ruff_python_parser::parse_unchecked_source`] but with an additional [`PythonVersion`]
720+
/// argument.
721+
fn parse_unchecked_source(
722+
source_kind: &SourceKind,
723+
source_type: PySourceType,
724+
target_version: PythonVersion,
725+
) -> Parsed<ModModule> {
726+
let options = ParseOptions::from(source_type).with_target_version(target_version);
727+
// SAFETY: Safe because `PySourceType` always parses to a `ModModule`. See
728+
// `ruff_python_parser::parse_unchecked_source`. We use `parse_unchecked` (and thus
729+
// have to unwrap) in order to pass the `PythonVersion` via `ParseOptions`.
730+
ruff_python_parser::parse_unchecked(source_kind.source_code(), options)
731+
.try_into_module()
732+
.expect("PySourceType always parses into a module")
733+
}
734+
693735
#[cfg(test)]
694736
mod tests {
695737
use std::path::Path;

crates/ruff_linter/src/message/mod.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ pub use pylint::PylintEmitter;
1616
pub use rdjson::RdjsonEmitter;
1717
use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix};
1818
use ruff_notebook::NotebookIndex;
19-
use ruff_python_parser::ParseError;
19+
use ruff_python_parser::{ParseError, UnsupportedSyntaxError};
2020
use ruff_source_file::{SourceFile, SourceLocation};
2121
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
2222
pub use sarif::SarifEmitter;
@@ -121,6 +121,18 @@ impl Message {
121121
})
122122
}
123123

124+
/// Create a [`Message`] from the given [`UnsupportedSyntaxError`].
125+
pub fn from_unsupported_syntax_error(
126+
unsupported_syntax_error: &UnsupportedSyntaxError,
127+
file: SourceFile,
128+
) -> Message {
129+
Message::SyntaxError(SyntaxErrorMessage {
130+
message: format!("SyntaxError: {unsupported_syntax_error}"),
131+
range: unsupported_syntax_error.range,
132+
file,
133+
})
134+
}
135+
124136
pub const fn as_diagnostic_message(&self) -> Option<&DiagnosticMessage> {
125137
match self {
126138
Message::Diagnostic(m) => Some(m),

crates/ruff_linter/src/rules/pyflakes/mod.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ mod tests {
1111

1212
use anyhow::Result;
1313
use regex::Regex;
14+
use ruff_python_parser::ParseOptions;
1415
use rustc_hash::FxHashMap;
1516
use test_case::test_case;
1617

@@ -744,8 +745,11 @@ mod tests {
744745
let source_type = PySourceType::default();
745746
let source_kind = SourceKind::Python(contents.to_string());
746747
let settings = LinterSettings::for_rules(Linter::Pyflakes.rules());
747-
let parsed =
748-
ruff_python_parser::parse_unchecked_source(source_kind.source_code(), source_type);
748+
let options =
749+
ParseOptions::from(source_type).with_target_version(settings.unresolved_target_version);
750+
let parsed = ruff_python_parser::parse_unchecked(source_kind.source_code(), options)
751+
.try_into_module()
752+
.expect("PySourceType always parses into a module");
749753
let locator = Locator::new(&contents);
750754
let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents());
751755
let indexer = Indexer::from_tokens(parsed.tokens(), locator.contents());
@@ -767,6 +771,7 @@ mod tests {
767771
&source_kind,
768772
source_type,
769773
&parsed,
774+
settings.unresolved_target_version,
770775
);
771776
diagnostics.sort_by_key(Ranged::start);
772777
let actual = diagnostics

crates/ruff_linter/src/test.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use ruff_notebook::NotebookError;
1616
use ruff_python_ast::PySourceType;
1717
use ruff_python_codegen::Stylist;
1818
use ruff_python_index::Indexer;
19-
use ruff_python_parser::ParseError;
19+
use ruff_python_parser::{ParseError, ParseOptions};
2020
use ruff_python_trivia::textwrap::dedent;
2121
use ruff_source_file::SourceFileBuilder;
2222
use ruff_text_size::Ranged;
@@ -110,7 +110,11 @@ pub(crate) fn test_contents<'a>(
110110
settings: &LinterSettings,
111111
) -> (Vec<Message>, Cow<'a, SourceKind>) {
112112
let source_type = PySourceType::from(path);
113-
let parsed = ruff_python_parser::parse_unchecked_source(source_kind.source_code(), source_type);
113+
let target_version = settings.resolve_target_version(path);
114+
let options = ParseOptions::from(source_type).with_target_version(target_version);
115+
let parsed = ruff_python_parser::parse_unchecked(source_kind.source_code(), options.clone())
116+
.try_into_module()
117+
.expect("PySourceType always parses into a module");
114118
let locator = Locator::new(source_kind.source_code());
115119
let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents());
116120
let indexer = Indexer::from_tokens(parsed.tokens(), locator.contents());
@@ -134,6 +138,7 @@ pub(crate) fn test_contents<'a>(
134138
source_kind,
135139
source_type,
136140
&parsed,
141+
target_version,
137142
);
138143

139144
let source_has_errors = !parsed.is_valid();
@@ -174,7 +179,9 @@ pub(crate) fn test_contents<'a>(
174179
transformed = Cow::Owned(transformed.updated(fixed_contents, &source_map));
175180

176181
let parsed =
177-
ruff_python_parser::parse_unchecked_source(transformed.source_code(), source_type);
182+
ruff_python_parser::parse_unchecked(transformed.source_code(), options.clone())
183+
.try_into_module()
184+
.expect("PySourceType always parses into a module");
178185
let locator = Locator::new(transformed.source_code());
179186
let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents());
180187
let indexer = Indexer::from_tokens(parsed.tokens(), locator.contents());
@@ -197,6 +204,7 @@ pub(crate) fn test_contents<'a>(
197204
&transformed,
198205
source_type,
199206
&parsed,
207+
target_version,
200208
);
201209

202210
if !parsed.is_valid() && !source_has_errors {

0 commit comments

Comments
 (0)