diff --git a/crates/ruff/src/diagnostics.rs b/crates/ruff/src/diagnostics.rs index 3376133ed7..ef7623145a 100644 --- a/crates/ruff/src/diagnostics.rs +++ b/crates/ruff/src/diagnostics.rs @@ -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; @@ -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(), ) } diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 230cda7a84..9d9ff2dbc3 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -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, 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 } diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 56e180bfab..1386704920 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -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; @@ -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() { diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index f552d0df51..2525322fd9 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -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; @@ -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, - 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( @@ -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 { let source = r"from os import @@ -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() } diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 01eb02705e..67a6728404 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -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; @@ -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(); @@ -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(); diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index 45d7304be5..e911e300c0 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -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 --> /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(()) } @@ -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(()) } diff --git a/crates/ty/tests/cli/python_environment.rs b/crates/ty/tests/cli/python_environment.rs index 4168da8e4c..4f3e442473 100644 --- a/crates/ty/tests/cli/python_environment.rs +++ b/crates/ty/tests/cli/python_environment.rs @@ -655,15 +655,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 | @@ -678,17 +678,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 | @@ -697,7 +697,7 @@ fn config_file_annotation_showing_where_python_version_set_syntax_error() -> any Found 1 diagnostic ----- stderr ----- - "###); + "); Ok(()) } diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/semantic_syntax_erro…_-_Semantic_syntax_erro…_-_`async`_comprehensio…_-_Python_3.10_(96aa8ec77d46553d).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/semantic_syntax_erro…_-_Semantic_syntax_erro…_-_`async`_comprehensio…_-_Python_3.10_(96aa8ec77d46553d).snap index f25e7b1bac..975be8520f 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/semantic_syntax_erro…_-_Semantic_syntax_erro…_-_`async`_comprehensio…_-_Python_3.10_(96aa8ec77d46553d).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/semantic_syntax_erro…_-_Semantic_syntax_erro…_-_`async`_comprehensio…_-_Python_3.10_(96aa8ec77d46553d).snap @@ -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" | diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/version_related_synt…_-_Version-related_synt…_-_`match`_statement_-_Before_3.10_(2545eaa83b635b8b).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/version_related_synt…_-_Version-related_synt…_-_`match`_statement_-_Before_3.10_(2545eaa83b635b8b).snap index 2500cc4544..a2cea52ff9 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/version_related_synt…_-_Version-related_synt…_-_`match`_statement_-_Before_3.10_(2545eaa83b635b8b).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/version_related_synt…_-_Version-related_synt…_-_`match`_statement_-_Before_3.10_(2545eaa83b635b8b).snap @@ -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") |