Skip to content

Standardize syntax error construction #20903

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.

Sign up for GitHub

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 1 commit into from
Oct 16, 2025
Merged

Standardize syntax error construction #20903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 1 addition & 6 deletions crates/ruff/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use log::{debug, warn};
use ruff_db::diagnostic::Diagnostic;
use ruff_linter::codes::Rule;
use ruff_linter::linter::{FixTable, FixerResult, LinterResult, ParseSource, lint_fix, lint_only};
use ruff_linter::message::create_syntax_error_diagnostic;
use ruff_linter::package::PackageRoot;
use ruff_linter::pyproject_toml::lint_pyproject_toml;
use ruff_linter::settings::types::UnsafeFixes;
Expand Down Expand Up @@ -103,11 +102,7 @@ impl Diagnostics {
let name = path.map_or_else(|| "-".into(), Path::to_string_lossy);
let dummy = SourceFileBuilder::new(name, "").finish();
Self::new(
vec![create_syntax_error_diagnostic(
dummy,
err,
TextRange::default(),
)],
vec![Diagnostic::invalid_syntax(dummy, err, TextRange::default())],
FxHashMap::default(),
)
}
Expand Down
7 changes: 2 additions & 5 deletions crates/ruff_db/src/diagnostic/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,14 @@ impl Diagnostic {
/// at time of writing, `ruff_db` depends on `ruff_python_parser` instead of
/// the other way around. And since we want to do this conversion in a couple
/// places, it makes sense to centralize it _somewhere_. So it's here for now.
///
/// Note that `message` is stored in the primary annotation, _not_ in the primary diagnostic
/// message.
pub fn invalid_syntax(
span: impl Into<Span>,
message: impl IntoDiagnosticMessage,
range: impl Ranged,
) -> Diagnostic {
let mut diag = Diagnostic::new(DiagnosticId::InvalidSyntax, Severity::Error, "");
let mut diag = Diagnostic::new(DiagnosticId::InvalidSyntax, Severity::Error, message);
let span = span.into().with_range(range.range());
diag.annotate(Annotation::primary(span).message(message));
diag.annotate(Annotation::primary(span));
diag
}

Expand Down
7 changes: 3 additions & 4 deletions crates/ruff_linter/src/linter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use crate::checkers::tokens::check_tokens;
use crate::directives::Directives;
use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens};
use crate::fix::{FixResult, fix_file};
use crate::message::create_syntax_error_diagnostic;
use crate::noqa::add_noqa;
use crate::package::PackageRoot;
use crate::registry::Rule;
Expand Down Expand Up @@ -496,15 +495,15 @@ fn diagnostics_to_messages(
parse_errors
.iter()
.map(|parse_error| {
create_syntax_error_diagnostic(source_file.clone(), &parse_error.error, parse_error)
Diagnostic::invalid_syntax(source_file.clone(), &parse_error.error, parse_error)
})
.chain(unsupported_syntax_errors.iter().map(|syntax_error| {
create_syntax_error_diagnostic(source_file.clone(), syntax_error, syntax_error)
Diagnostic::invalid_syntax(source_file.clone(), syntax_error, syntax_error)
}))
.chain(
semantic_syntax_errors
.iter()
.map(|error| create_syntax_error_diagnostic(source_file.clone(), error, error)),
.map(|error| Diagnostic::invalid_syntax(source_file.clone(), error, error)),
)
.chain(diagnostics.into_iter().map(|mut diagnostic| {
if let Some(range) = diagnostic.range() {
Expand Down
24 changes: 2 additions & 22 deletions crates/ruff_linter/src/message/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use ruff_db::files::File;
pub use grouped::GroupedEmitter;
use ruff_notebook::NotebookIndex;
use ruff_source_file::{SourceFile, SourceFileBuilder};
use ruff_text_size::{Ranged, TextRange, TextSize};
use ruff_text_size::{TextRange, TextSize};
pub use sarif::SarifEmitter;

use crate::Fix;
Expand All @@ -26,24 +26,6 @@ use crate::settings::types::{OutputFormat, RuffOutputFormat};
mod grouped;
mod sarif;

/// Creates a `Diagnostic` from a syntax error, with the format expected by Ruff.
///
/// This is almost identical to `ruff_db::diagnostic::create_syntax_error_diagnostic`, except the
/// `message` is stored as the primary diagnostic message instead of on the primary annotation.
///
/// TODO(brent) These should be unified at some point, but we keep them separate for now to avoid a
/// ton of snapshot changes while combining ruff's diagnostic type with `Diagnostic`.
pub fn create_syntax_error_diagnostic(
span: impl Into<Span>,
message: impl std::fmt::Display,
range: impl Ranged,
) -> Diagnostic {
let mut diag = Diagnostic::new(DiagnosticId::InvalidSyntax, Severity::Error, message);
let span = span.into().with_range(range.range());
diag.annotate(Annotation::primary(span));
diag
}

/// Create a `Diagnostic` from a panic.
pub fn create_panic_diagnostic(error: &PanicError, path: Option<&Path>) -> Diagnostic {
let mut diagnostic = Diagnostic::new(
Expand Down Expand Up @@ -260,8 +242,6 @@ mod tests {
use crate::message::{Emitter, EmitterContext, create_lint_diagnostic};
use crate::{Edit, Fix};

use super::create_syntax_error_diagnostic;

pub(super) fn create_syntax_error_diagnostics() -> Vec<Diagnostic> {
let source = r"from os import

Expand All @@ -274,7 +254,7 @@ if call(foo
.errors()
.iter()
.map(|parse_error| {
create_syntax_error_diagnostic(source_file.clone(), &parse_error.error, parse_error)
Diagnostic::invalid_syntax(source_file.clone(), &parse_error.error, parse_error)
})
.collect()
}
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use ruff_source_file::SourceFileBuilder;
use crate::codes::Rule;
use crate::fix::{FixResult, fix_file};
use crate::linter::check_path;
use crate::message::{EmitterContext, create_syntax_error_diagnostic};
use crate::message::EmitterContext;
use crate::package::PackageRoot;
use crate::packaging::detect_package_root;
use crate::settings::types::UnsafeFixes;
Expand Down Expand Up @@ -405,7 +405,7 @@ Either ensure you always emit a fix or change `Violation::FIX_AVAILABILITY` to e
diagnostic
})
.chain(parsed.errors().iter().map(|parse_error| {
create_syntax_error_diagnostic(source_code.clone(), &parse_error.error, parse_error)
Diagnostic::invalid_syntax(source_code.clone(), &parse_error.error, parse_error)
}))
.sorted_by(Diagnostic::ruff_start_ordering)
.collect();
Expand All @@ -419,7 +419,7 @@ fn print_syntax_errors(errors: &[ParseError], path: &Path, source: &SourceKind)
let messages: Vec<_> = errors
.iter()
.map(|parse_error| {
create_syntax_error_diagnostic(source_file.clone(), &parse_error.error, parse_error)
Diagnostic::invalid_syntax(source_file.clone(), &parse_error.error, parse_error)
})
.collect();

Expand Down
40 changes: 20 additions & 20 deletions crates/ty/tests/cli/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,42 +92,42 @@ fn test_quiet_output() -> anyhow::Result<()> {
#[test]
fn test_run_in_sub_directory() -> anyhow::Result<()> {
let case = CliTest::with_files([("test.py", "~"), ("subdir/nothing", "")])?;
assert_cmd_snapshot!(case.command().current_dir(case.root().join("subdir")).arg(".."), @r###"
assert_cmd_snapshot!(case.command().current_dir(case.root().join("subdir")).arg(".."), @r"
success: false
exit_code: 1
----- stdout -----
error[invalid-syntax]
error[invalid-syntax]: Expected an expression
--> <temp_dir>/test.py:1:2
|
1 | ~
| ^ Expected an expression
| ^
|

Found 1 diagnostic

----- stderr -----
"###);
");
Ok(())
}

#[test]
fn test_include_hidden_files_by_default() -> anyhow::Result<()> {
let case = CliTest::with_files([(".test.py", "~")])?;
assert_cmd_snapshot!(case.command(), @r###"
assert_cmd_snapshot!(case.command(), @r"
success: false
exit_code: 1
----- stdout -----
error[invalid-syntax]
error[invalid-syntax]: Expected an expression
--> .test.py:1:2
|
1 | ~
| ^ Expected an expression
| ^
|

Found 1 diagnostic

----- stderr -----
"###);
");
Ok(())
}

Expand All @@ -146,57 +146,57 @@ fn test_respect_ignore_files() -> anyhow::Result<()> {
"###);

// Test that we can set to false via CLI
assert_cmd_snapshot!(case.command().arg("--no-respect-ignore-files"), @r###"
assert_cmd_snapshot!(case.command().arg("--no-respect-ignore-files"), @r"
success: false
exit_code: 1
----- stdout -----
error[invalid-syntax]
error[invalid-syntax]: Expected an expression
--> test.py:1:2
|
1 | ~
| ^ Expected an expression
| ^
|

Found 1 diagnostic

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

// Test that we can set to false via config file
case.write_file("ty.toml", "src.respect-ignore-files = false")?;
assert_cmd_snapshot!(case.command(), @r###"
assert_cmd_snapshot!(case.command(), @r"
success: false
exit_code: 1
----- stdout -----
error[invalid-syntax]
error[invalid-syntax]: Expected an expression
--> test.py:1:2
|
1 | ~
| ^ Expected an expression
| ^
|

Found 1 diagnostic

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

// Ensure CLI takes precedence
case.write_file("ty.toml", "src.respect-ignore-files = true")?;
assert_cmd_snapshot!(case.command().arg("--no-respect-ignore-files"), @r###"
assert_cmd_snapshot!(case.command().arg("--no-respect-ignore-files"), @r"
success: false
exit_code: 1
----- stdout -----
error[invalid-syntax]
error[invalid-syntax]: Expected an expression
--> test.py:1:2
|
1 | ~
| ^ Expected an expression
| ^
|

Found 1 diagnostic

----- stderr -----
"###);
");
Ok(())
}

Expand Down
16 changes: 8 additions & 8 deletions crates/ty/tests/cli/python_environment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,15 +560,15 @@ fn config_file_annotation_showing_where_python_version_set_syntax_error() -> any
),
])?;

assert_cmd_snapshot!(case.command(), @r###"
assert_cmd_snapshot!(case.command(), @r#"
success: false
exit_code: 1
----- stdout -----
error[invalid-syntax]
error[invalid-syntax]: Cannot use `match` statement on Python 3.8 (syntax was added in Python 3.10)
--> test.py:2:1
|
2 | match object():
| ^^^^^ Cannot use `match` statement on Python 3.8 (syntax was added in Python 3.10)
| ^^^^^
3 | case int():
4 | pass
|
Expand All @@ -583,17 +583,17 @@ fn config_file_annotation_showing_where_python_version_set_syntax_error() -> any
Found 1 diagnostic

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

assert_cmd_snapshot!(case.command().arg("--python-version=3.9"), @r###"
assert_cmd_snapshot!(case.command().arg("--python-version=3.9"), @r"
success: false
exit_code: 1
----- stdout -----
error[invalid-syntax]
error[invalid-syntax]: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
--> test.py:2:1
|
2 | match object():
| ^^^^^ Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
| ^^^^^
3 | case int():
4 | pass
|
Expand All @@ -602,7 +602,7 @@ fn config_file_annotation_showing_where_python_version_set_syntax_error() -> any
Found 1 diagnostic

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

Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions ...ro…_-_Semantic_syntax_erro…_-_`async`_comprehensio…_-_Python_3.10_(96aa8ec77d46553d).snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syn
# Diagnostics

```
error[invalid-syntax]
error[invalid-syntax]: cannot use an asynchronous comprehension inside of a synchronous comprehension on Python 3.10 (syntax was added in 3.11)
--> src/mdtest_snippet.py:6:19
|
4 | async def f():
5 | # error: 19 [invalid-syntax] "cannot use an asynchronous comprehension inside of a synchronous comprehension on Python 3.10 (syntax…
6 | return {n: [x async for x in elements(n)] for n in range(3)}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot use an asynchronous comprehension inside of a synchronous comprehension on Python 3.10 (syntax was added in 3.11)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
7 | async def test():
8 | # error: [not-iterable] "Object of type `range` is not async-iterable"
Comment on lines +36 to 44
Copy link
Member

@AlexWaygood AlexWaygood Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this might be nice here (but not blocking):

error[invalid-syntax]: cannot use an asynchronous comprehension inside of a synchronous comprehension on Python 3.10
 --> src/mdtest_snippet.py:6:19
  |
4 | async def f():
5 |     # error: 19 [invalid-syntax] "cannot use an asynchronous comprehension inside of a synchronous comprehension on Python 3.10 (syntax…
6 |     return {n: [x async for x in elements(n)] for n in range(3)}
  |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^ Syntax was added in 3.11
7 | async def test():
8 |     # error: [not-iterable] "Object of type `range` is not async-iterable"

Copy link
Contributor Author

@ntBre ntBre Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about that too, or an info sub-diagnostic. I think either option will need something like #20901, though.

Copy link
Contributor Author

@ntBre ntBre Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(or just a separate constructor from the very generic invalid_syntax for unsupported syntax errors)

|
Expand Down
4 changes: 2 additions & 2 deletions ...d_synt…_-_Version-related_synt…_-_`match`_statement_-_Before_3.10_(2545eaa83b635b8b).snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/version_rela
# Diagnostics

```
error[invalid-syntax]
error[invalid-syntax]: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
--> src/mdtest_snippet.py:1:1
|
1 | match 2: # error: 1 [invalid-syntax] "Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)"
| ^^^^^ Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
| ^^^^^
2 | case 1:
3 | print("it's one")
|
Expand Down