Skip to content

Commit 2baaedd

Browse files
ntBreMichaReiser
andauthored
[syntax-errors] Start detecting compile-time syntax errors (#16106)
## Summary This PR implements the "greeter" approach for checking the AST for syntax errors emitted by the CPython compiler. It introduces two main infrastructural changes to support all of the compile-time errors: 1. Adds a new `semantic_errors` module to the parser crate with public `SemanticSyntaxChecker` and `SemanticSyntaxError` types 2. Embeds a `SemanticSyntaxChecker` in the `ruff_linter::Checker` for checking these errors in ruff As a proof of concept, it also implements detection of two syntax errors: 1. A reimplementation of [`late-future-import`](https://docs.astral.sh/ruff/rules/late-future-import/) (`F404`) 2. Detection of rebound comprehension iteration variables (#14395) ## Test plan Existing F404 tests, new inline tests in the `ruff_python_parser` crate, and a linter CLI test showing an example of the `Message` output. I also tested in VS Code, where `preview = false` and turning off syntax errors both disable the new errors: ![image](https://github.com/user-attachments/assets/cf453d95-04f7-484b-8440-cb812f29d45e) And on the playground, where `preview = false` also disables the errors: ![image](https://github.com/user-attachments/assets/a97570c4-1efa-439f-9d99-a54487dd6064) Fixes #14395 --------- Co-authored-by: Micha Reiser <[email protected]>
1 parent b1deab8 commit 2baaedd

File tree

17 files changed

+1601
-93
lines changed

17 files changed

+1601
-93
lines changed

crates/ruff/tests/integration_test.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,52 @@ def mvce(keys, values):
10541054
"#);
10551055
}
10561056

1057+
#[test]
1058+
fn show_statistics_syntax_errors() {
1059+
let mut cmd = RuffCheck::default()
1060+
.args(["--statistics", "--target-version=py39", "--preview"])
1061+
.build();
1062+
1063+
// ParseError
1064+
assert_cmd_snapshot!(
1065+
cmd.pass_stdin("x ="),
1066+
@r"
1067+
success: false
1068+
exit_code: 1
1069+
----- stdout -----
1070+
1 syntax-error
1071+
Found 1 error.
1072+
1073+
----- stderr -----
1074+
");
1075+
1076+
// match before 3.10, UnsupportedSyntaxError
1077+
assert_cmd_snapshot!(
1078+
cmd.pass_stdin("match 2:\n case 1: ..."),
1079+
@r"
1080+
success: false
1081+
exit_code: 1
1082+
----- stdout -----
1083+
1 syntax-error
1084+
Found 1 error.
1085+
1086+
----- stderr -----
1087+
");
1088+
1089+
// rebound comprehension variable, SemanticSyntaxError
1090+
assert_cmd_snapshot!(
1091+
cmd.pass_stdin("[x := 1 for x in range(0)]"),
1092+
@r"
1093+
success: false
1094+
exit_code: 1
1095+
----- stdout -----
1096+
1 syntax-error
1097+
Found 1 error.
1098+
1099+
----- stderr -----
1100+
");
1101+
}
1102+
10571103
#[test]
10581104
fn preview_enabled_prefix() {
10591105
// All the RUF9XX test rules should be triggered

crates/ruff/tests/lint.rs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5491,3 +5491,68 @@ fn cookiecutter_globbing_no_project_root() -> Result<()> {
54915491

54925492
Ok(())
54935493
}
5494+
5495+
/// Test that semantic syntax errors (1) are emitted, (2) are not cached, (3) don't affect the
5496+
/// reporting of normal diagnostics, and (4) are not suppressed by `select = []` (or otherwise
5497+
/// disabling all AST-based rules).
5498+
#[test]
5499+
fn semantic_syntax_errors() -> Result<()> {
5500+
let tempdir = TempDir::new()?;
5501+
let contents = "[(x := 1) for x in foo]";
5502+
fs::write(tempdir.path().join("main.py"), contents)?;
5503+
5504+
let mut cmd = Command::new(get_cargo_bin(BIN_NAME));
5505+
// inline STDIN_BASE_OPTIONS to remove --no-cache
5506+
cmd.args(["check", "--output-format", "concise"])
5507+
.arg("--preview")
5508+
.arg("--quiet") // suppress `debug build without --no-cache` warnings
5509+
.current_dir(&tempdir);
5510+
5511+
assert_cmd_snapshot!(
5512+
cmd,
5513+
@r"
5514+
success: false
5515+
exit_code: 1
5516+
----- stdout -----
5517+
main.py:1:3: SyntaxError: assignment expression cannot rebind comprehension variable
5518+
main.py:1:20: F821 Undefined name `foo`
5519+
5520+
----- stderr -----
5521+
"
5522+
);
5523+
5524+
// this should *not* be cached, like normal parse errors
5525+
assert_cmd_snapshot!(
5526+
cmd,
5527+
@r"
5528+
success: false
5529+
exit_code: 1
5530+
----- stdout -----
5531+
main.py:1:3: SyntaxError: assignment expression cannot rebind comprehension variable
5532+
main.py:1:20: F821 Undefined name `foo`
5533+
5534+
----- stderr -----
5535+
"
5536+
);
5537+
5538+
// ensure semantic errors are caught even without AST-based rules selected
5539+
assert_cmd_snapshot!(
5540+
Command::new(get_cargo_bin(BIN_NAME))
5541+
.args(STDIN_BASE_OPTIONS)
5542+
.args(["--config", "lint.select = []"])
5543+
.arg("--preview")
5544+
.arg("-")
5545+
.pass_stdin(contents),
5546+
@r"
5547+
success: false
5548+
exit_code: 1
5549+
----- stdout -----
5550+
-:1:3: SyntaxError: assignment expression cannot rebind comprehension variable
5551+
Found 1 error.
5552+
5553+
----- stderr -----
5554+
"
5555+
);
5556+
5557+
Ok(())
5558+
}

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -853,14 +853,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
853853
if checker.enabled(Rule::FutureFeatureNotDefined) {
854854
pyflakes::rules::future_feature_not_defined(checker, alias);
855855
}
856-
if checker.enabled(Rule::LateFutureImport) {
857-
if checker.semantic.seen_futures_boundary() {
858-
checker.report_diagnostic(Diagnostic::new(
859-
pyflakes::rules::LateFutureImport,
860-
stmt.range(),
861-
));
862-
}
863-
}
864856
} else if &alias.name == "*" {
865857
if checker.enabled(Rule::UndefinedLocalWithNestedImportStarUsage) {
866858
if !matches!(checker.semantic.current_scope().kind, ScopeKind::Module) {

crates/ruff_linter/src/checkers/ast/mod.rs

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ use std::path::Path;
2626

2727
use itertools::Itertools;
2828
use log::debug;
29+
use ruff_python_parser::semantic_errors::{
30+
SemanticSyntaxChecker, SemanticSyntaxContext, SemanticSyntaxError, SemanticSyntaxErrorKind,
31+
};
2932
use rustc_hash::{FxHashMap, FxHashSet};
3033

3134
use ruff_diagnostics::{Diagnostic, IsolationLevel};
@@ -63,6 +66,7 @@ use crate::importer::Importer;
6366
use crate::noqa::NoqaMapping;
6467
use crate::package::PackageRoot;
6568
use crate::registry::Rule;
69+
use crate::rules::pyflakes::rules::LateFutureImport;
6670
use crate::rules::{flake8_pyi, flake8_type_checking, pyflakes, pyupgrade};
6771
use crate::settings::{flags, LinterSettings};
6872
use crate::{docstrings, noqa, Locator};
@@ -223,8 +227,13 @@ pub(crate) struct Checker<'a> {
223227
last_stmt_end: TextSize,
224228
/// A state describing if a docstring is expected or not.
225229
docstring_state: DocstringState,
226-
/// The target [`PythonVersion`] for version-dependent checks
230+
/// The target [`PythonVersion`] for version-dependent checks.
227231
target_version: PythonVersion,
232+
/// Helper visitor for detecting semantic syntax errors.
233+
#[allow(clippy::struct_field_names)]
234+
semantic_checker: SemanticSyntaxChecker,
235+
/// Errors collected by the `semantic_checker`.
236+
semantic_errors: RefCell<Vec<SemanticSyntaxError>>,
228237
}
229238

230239
impl<'a> Checker<'a> {
@@ -272,6 +281,8 @@ impl<'a> Checker<'a> {
272281
last_stmt_end: TextSize::default(),
273282
docstring_state: DocstringState::default(),
274283
target_version,
284+
semantic_checker: SemanticSyntaxChecker::new(),
285+
semantic_errors: RefCell::default(),
275286
}
276287
}
277288
}
@@ -512,10 +523,44 @@ impl<'a> Checker<'a> {
512523
pub(crate) const fn target_version(&self) -> PythonVersion {
513524
self.target_version
514525
}
526+
527+
fn with_semantic_checker(&mut self, f: impl FnOnce(&mut SemanticSyntaxChecker, &Checker)) {
528+
let mut checker = std::mem::take(&mut self.semantic_checker);
529+
f(&mut checker, self);
530+
self.semantic_checker = checker;
531+
}
532+
}
533+
534+
impl SemanticSyntaxContext for Checker<'_> {
535+
fn seen_docstring_boundary(&self) -> bool {
536+
self.semantic.seen_module_docstring_boundary()
537+
}
538+
539+
fn python_version(&self) -> PythonVersion {
540+
self.target_version
541+
}
542+
543+
fn report_semantic_error(&self, error: SemanticSyntaxError) {
544+
match error.kind {
545+
SemanticSyntaxErrorKind::LateFutureImport => {
546+
if self.settings.rules.enabled(Rule::LateFutureImport) {
547+
self.report_diagnostic(Diagnostic::new(LateFutureImport, error.range));
548+
}
549+
}
550+
SemanticSyntaxErrorKind::ReboundComprehensionVariable
551+
if self.settings.preview.is_enabled() =>
552+
{
553+
self.semantic_errors.borrow_mut().push(error);
554+
}
555+
SemanticSyntaxErrorKind::ReboundComprehensionVariable => {}
556+
}
557+
}
515558
}
516559

517560
impl<'a> Visitor<'a> for Checker<'a> {
518561
fn visit_stmt(&mut self, stmt: &'a Stmt) {
562+
self.with_semantic_checker(|semantic, context| semantic.visit_stmt(stmt, context));
563+
519564
// Step 0: Pre-processing
520565
self.semantic.push_node(stmt);
521566

@@ -550,17 +595,13 @@ impl<'a> Visitor<'a> for Checker<'a> {
550595
{
551596
self.semantic.flags |= SemanticModelFlags::FUTURE_ANNOTATIONS;
552597
}
553-
} else {
554-
self.semantic.flags |= SemanticModelFlags::FUTURES_BOUNDARY;
555598
}
556599
}
557600
Stmt::Import(_) => {
558601
self.semantic.flags |= SemanticModelFlags::MODULE_DOCSTRING_BOUNDARY;
559-
self.semantic.flags |= SemanticModelFlags::FUTURES_BOUNDARY;
560602
}
561603
_ => {
562604
self.semantic.flags |= SemanticModelFlags::MODULE_DOCSTRING_BOUNDARY;
563-
self.semantic.flags |= SemanticModelFlags::FUTURES_BOUNDARY;
564605
if !(self.semantic.seen_import_boundary()
565606
|| stmt.is_ipy_escape_command_stmt()
566607
|| helpers::is_assignment_to_a_dunder(stmt)
@@ -1131,6 +1172,8 @@ impl<'a> Visitor<'a> for Checker<'a> {
11311172
}
11321173

11331174
fn visit_expr(&mut self, expr: &'a Expr) {
1175+
self.with_semantic_checker(|semantic, context| semantic.visit_expr(expr, context));
1176+
11341177
// Step 0: Pre-processing
11351178
if self.source_type.is_stub()
11361179
&& self.semantic.in_class_base()
@@ -2674,7 +2717,7 @@ pub(crate) fn check_ast(
26742717
cell_offsets: Option<&CellOffsets>,
26752718
notebook_index: Option<&NotebookIndex>,
26762719
target_version: PythonVersion,
2677-
) -> Vec<Diagnostic> {
2720+
) -> (Vec<Diagnostic>, Vec<SemanticSyntaxError>) {
26782721
let module_path = package
26792722
.map(PackageRoot::path)
26802723
.and_then(|package| to_module_path(package, path));
@@ -2739,5 +2782,11 @@ pub(crate) fn check_ast(
27392782
checker.analyze.scopes.push(ScopeId::global());
27402783
analyze::deferred_scopes(&checker);
27412784

2742-
checker.diagnostics.take()
2785+
let Checker {
2786+
diagnostics,
2787+
semantic_errors,
2788+
..
2789+
} = checker;
2790+
2791+
(diagnostics.into_inner(), semantic_errors.into_inner())
27432792
}

0 commit comments

Comments
 (0)