Skip to content

[syntax-errors] Start detecting compile-time syntax errors #16106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 52 commits into from
Mar 21, 2025
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
c66f37a
try SourceOrderVisitor and hack into red-knot
ntBre Feb 11, 2025
825f971
ignore syntax diagnostics in red-knot tests
ntBre Feb 11, 2025
4e702dd
integrate with ruff, accept snap changes
ntBre Feb 11, 2025
877a108
implement F404 too
ntBre Feb 11, 2025
af9d9dd
just use SyntaxChecker::enter_stmt instead of SourceOrderVisitor
ntBre Feb 11, 2025
6f90709
update doc comment
ntBre Feb 11, 2025
ab2db92
tidy enter_stmt
ntBre Feb 11, 2025
c4a5cc7
move red_knot_python_semantic::PythonVersion to ruff_db
ntBre Feb 12, 2025
34b5829
add docs to VersionSyntaxError
ntBre Feb 12, 2025
6cc9198
revert red-knot integration
ntBre Mar 7, 2025
508d785
revert MatchBeforePy310 detection
ntBre Mar 7, 2025
9c38e2d
revert version-related diagnostic kind
ntBre Mar 7, 2025
da6d31d
add a failing test for comprehension rebinding
ntBre Mar 17, 2025
33b5381
start detecting list comprehensions but can't display messages yet
ntBre Mar 17, 2025
419b5cd
restore testing code, add a TestVisitor
ntBre Mar 17, 2025
662ffc3
factor out check_generator_expr, check other comprehensions
ntBre Mar 17, 2025
63276ef
delete linter test, at least for now
ntBre Mar 17, 2025
ca582a5
use a Visitor to traverse generator elts, add more test cases
ntBre Mar 18, 2025
f461718
emit new errors in ruff
ntBre Mar 19, 2025
67eb290
preview gate
ntBre Mar 19, 2025
38ee75d
add ruff test
ntBre Mar 19, 2025
dc1a94a
remove now-unused FUTURES_BOUNDARY flag and shift others down
ntBre Mar 19, 2025
edbc46c
fix crate version
ntBre Mar 19, 2025
6e131f6
update docs
ntBre Mar 19, 2025
659788a
remove unused as_str method
ntBre Mar 19, 2025
7d00d50
Merge branch 'main' into brent/syntax-error-source-order
ntBre Mar 20, 2025
c3417be
update LinterResult flag name and docs
ntBre Mar 20, 2025
d6d3437
prepend new types with Semantic
ntBre Mar 20, 2025
7144a82
make rebound_variable a Vec, add a test with multiple violations
ntBre Mar 20, 2025
ffdb7cb
rename enter_ methods to visit_
ntBre Mar 20, 2025
16479d4
don't build a HashSet
ntBre Mar 20, 2025
ef11801
target_version -> python_version
ntBre Mar 20, 2025
4cfaad0
report semantic errors even when AST-based rules are not selected
ntBre Mar 20, 2025
f1b2222
add context trait and pass to visit_stmt
ntBre Mar 20, 2025
ae17231
move python version into context trait too
ntBre Mar 20, 2025
a363c6b
move preview check to check_ast to avoid collecting at all
ntBre Mar 20, 2025
d48afb7
remove separate crate, put SemanticSyntaxChecker in the parser
ntBre Mar 20, 2025
7ffa680
expand inline tests to semantic_errors module
ntBre Mar 20, 2025
c482a02
test --statistics for all three kinds of errors
ntBre Mar 20, 2025
88cbb9b
fix outdated docs links
ntBre Mar 20, 2025
5f7de2a
move interior mutability to Checker
ntBre Mar 21, 2025
e8d44f1
update checker field name and add docs (also fix an old missing `.`)
ntBre Mar 21, 2025
7171e20
fix LinterResult docs
ntBre Mar 21, 2025
077bd37
reorder SemanticSyntaxChecker code to navigate more easily
ntBre Mar 21, 2025
f484a85
combine semantic error tests with existing inline tests
ntBre Mar 21, 2025
de0914b
move inline tests just above add_error call
ntBre Mar 21, 2025
3ccea07
delete getter/setter without refcell
ntBre Mar 21, 2025
0b62bde
restore docs from SemanticModel flag, delete todo
ntBre Mar 21, 2025
452283a
move semantic_errors to single-file module
ntBre Mar 21, 2025
b5f828b
delete outdated comment
ntBre Mar 21, 2025
ab89b04
update variable name to match LinterResult field
ntBre Mar 21, 2025
79ddb2f
delete now-unused inline/semantic directory
ntBre Mar 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions crates/ruff/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,52 @@ def mvce(keys, values):
"#);
}

#[test]
fn show_statistics_syntax_errors() {
let mut cmd = RuffCheck::default()
.args(["--statistics", "--target-version=py39", "--preview"])
.build();

// ParseError
assert_cmd_snapshot!(
cmd.pass_stdin("x ="),
@r"
success: false
exit_code: 1
----- stdout -----
1 syntax-error
Found 1 error.

----- stderr -----
");

// match before 3.10, UnsupportedSyntaxError
assert_cmd_snapshot!(
cmd.pass_stdin("match 2:\n case 1: ..."),
@r"
success: false
exit_code: 1
----- stdout -----
1 syntax-error
Found 1 error.

----- stderr -----
");

// rebound comprehension variable, SemanticSyntaxError
assert_cmd_snapshot!(
cmd.pass_stdin("[x := 1 for x in range(0)]"),
@r"
success: false
exit_code: 1
----- stdout -----
1 syntax-error
Found 1 error.

----- stderr -----
");
}

#[test]
fn preview_enabled_prefix() {
// All the RUF9XX test rules should be triggered
Expand Down
65 changes: 65 additions & 0 deletions crates/ruff/tests/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5491,3 +5491,68 @@ fn cookiecutter_globbing_no_project_root() -> Result<()> {

Ok(())
}

/// Test that semantic syntax errors (1) are emitted, (2) are not cached, (3) don't affect the
/// reporting of normal diagnostics, and (4) are not suppressed by `select = []` (or otherwise
/// disabling all AST-based rules).
#[test]
fn semantic_syntax_errors() -> Result<()> {
let tempdir = TempDir::new()?;
let contents = "[(x := 1) for x in foo]";
fs::write(tempdir.path().join("main.py"), contents)?;

let mut cmd = Command::new(get_cargo_bin(BIN_NAME));
// inline STDIN_BASE_OPTIONS to remove --no-cache
cmd.args(["check", "--output-format", "concise"])
.arg("--preview")
.arg("--quiet") // suppress `debug build without --no-cache` warnings
.current_dir(&tempdir);

assert_cmd_snapshot!(
cmd,
@r"
success: false
exit_code: 1
----- stdout -----
main.py:1:3: SyntaxError: assignment expression cannot rebind comprehension variable
main.py:1:20: F821 Undefined name `foo`

----- stderr -----
"
);

// this should *not* be cached, like normal parse errors
assert_cmd_snapshot!(
cmd,
@r"
success: false
exit_code: 1
----- stdout -----
main.py:1:3: SyntaxError: assignment expression cannot rebind comprehension variable
main.py:1:20: F821 Undefined name `foo`

----- stderr -----
"
);

// ensure semantic errors are caught even without AST-based rules selected
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
.args(["--config", "lint.select = []"])
.arg("--preview")
.arg("-")
.pass_stdin(contents),
@r"
success: false
exit_code: 1
----- stdout -----
-:1:3: SyntaxError: assignment expression cannot rebind comprehension variable
Found 1 error.

----- stderr -----
"
);

Ok(())
}
8 changes: 0 additions & 8 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -853,14 +853,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::FutureFeatureNotDefined) {
pyflakes::rules::future_feature_not_defined(checker, alias);
}
if checker.enabled(Rule::LateFutureImport) {
if checker.semantic.seen_futures_boundary() {
checker.report_diagnostic(Diagnostic::new(
pyflakes::rules::LateFutureImport,
stmt.range(),
));
}
}
} else if &alias.name == "*" {
if checker.enabled(Rule::UndefinedLocalWithNestedImportStarUsage) {
if !matches!(checker.semantic.current_scope().kind, ScopeKind::Module) {
Expand Down
59 changes: 53 additions & 6 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ use std::path::Path;

use itertools::Itertools;
use log::debug;
use ruff_python_parser::semantic_errors::{
SemanticSyntaxChecker, SemanticSyntaxContext, SemanticSyntaxError, SemanticSyntaxErrorKind,
};
use rustc_hash::{FxHashMap, FxHashSet};

use ruff_diagnostics::{Diagnostic, IsolationLevel};
Expand Down Expand Up @@ -63,6 +66,7 @@ use crate::importer::Importer;
use crate::noqa::NoqaMapping;
use crate::package::PackageRoot;
use crate::registry::Rule;
use crate::rules::pyflakes::rules::LateFutureImport;
use crate::rules::{flake8_pyi, flake8_type_checking, pyflakes, pyupgrade};
use crate::settings::{flags, LinterSettings};
use crate::{docstrings, noqa, Locator};
Expand Down Expand Up @@ -225,6 +229,9 @@ pub(crate) struct Checker<'a> {
docstring_state: DocstringState,
/// The target [`PythonVersion`] for version-dependent checks
target_version: PythonVersion,

#[allow(clippy::struct_field_names)]
syntax_checker: SemanticSyntaxChecker,
}

impl<'a> Checker<'a> {
Expand Down Expand Up @@ -272,6 +279,7 @@ impl<'a> Checker<'a> {
last_stmt_end: TextSize::default(),
docstring_state: DocstringState::default(),
target_version,
syntax_checker: SemanticSyntaxChecker::new(),
}
}
}
Expand Down Expand Up @@ -514,8 +522,20 @@ impl<'a> Checker<'a> {
}
}

impl SemanticSyntaxContext for Checker<'_> {
fn seen_docstring_boundary(&self) -> bool {
self.semantic.seen_module_docstring_boundary()
}

fn python_version(&self) -> PythonVersion {
self.target_version
}
}

impl<'a> Visitor<'a> for Checker<'a> {
fn visit_stmt(&mut self, stmt: &'a Stmt) {
self.syntax_checker.visit_stmt(stmt, self);

// Step 0: Pre-processing
self.semantic.push_node(stmt);

Expand Down Expand Up @@ -550,17 +570,13 @@ impl<'a> Visitor<'a> for Checker<'a> {
{
self.semantic.flags |= SemanticModelFlags::FUTURE_ANNOTATIONS;
}
} else {
self.semantic.flags |= SemanticModelFlags::FUTURES_BOUNDARY;
}
}
Stmt::Import(_) => {
self.semantic.flags |= SemanticModelFlags::MODULE_DOCSTRING_BOUNDARY;
self.semantic.flags |= SemanticModelFlags::FUTURES_BOUNDARY;
}
_ => {
self.semantic.flags |= SemanticModelFlags::MODULE_DOCSTRING_BOUNDARY;
self.semantic.flags |= SemanticModelFlags::FUTURES_BOUNDARY;
if !(self.semantic.seen_import_boundary()
|| stmt.is_ipy_escape_command_stmt()
|| helpers::is_assignment_to_a_dunder(stmt)
Expand Down Expand Up @@ -1131,6 +1147,8 @@ impl<'a> Visitor<'a> for Checker<'a> {
}

fn visit_expr(&mut self, expr: &'a Expr) {
self.syntax_checker.visit_expr(expr, self);

// Step 0: Pre-processing
if self.source_type.is_stub()
&& self.semantic.in_class_base()
Expand Down Expand Up @@ -2674,7 +2692,7 @@ pub(crate) fn check_ast(
cell_offsets: Option<&CellOffsets>,
notebook_index: Option<&NotebookIndex>,
target_version: PythonVersion,
) -> Vec<Diagnostic> {
) -> (Vec<Diagnostic>, Vec<SemanticSyntaxError>) {
let module_path = package
.map(PackageRoot::path)
.and_then(|package| to_module_path(package, path));
Expand Down Expand Up @@ -2739,5 +2757,34 @@ pub(crate) fn check_ast(
checker.analyze.scopes.push(ScopeId::global());
analyze::deferred_scopes(&checker);

checker.diagnostics.take()
let Checker {
diagnostics,
syntax_checker,
settings,
..
} = checker;

// partition the semantic syntax errors into Diagnostics and regular errors
let mut diagnostics = diagnostics.take();
let mut semantic_syntax_errors = Vec::new();
for semantic_syntax_error in syntax_checker.finish() {
match semantic_syntax_error.kind {
SemanticSyntaxErrorKind::LateFutureImport => {
if settings.rules.enabled(Rule::LateFutureImport) {
diagnostics.push(Diagnostic::new(
LateFutureImport,
semantic_syntax_error.range,
));
}
}
SemanticSyntaxErrorKind::ReboundComprehensionVariable
if settings.preview.is_enabled() =>
{
semantic_syntax_errors.push(semantic_syntax_error);
}
SemanticSyntaxErrorKind::ReboundComprehensionVariable => {}
}
}

(diagnostics, semantic_syntax_errors)
}
Loading
Loading