diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 20ff72f14ea99..230cda7a84c11 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -7,7 +7,8 @@ use ruff_annotate_snippets::Level as AnnotateLevel; use ruff_text_size::{Ranged, TextRange, TextSize}; pub use self::render::{ - DisplayDiagnostic, DisplayDiagnostics, FileResolver, Input, ceil_char_boundary, + DisplayDiagnostic, DisplayDiagnostics, DummyFileResolver, FileResolver, Input, + ceil_char_boundary, github::{DisplayGithubDiagnostics, GithubRenderer}, }; use crate::{Db, files::File}; diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 96fbb565b4cc7..f7c618ae41464 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -1170,6 +1170,31 @@ pub fn ceil_char_boundary(text: &str, offset: TextSize) -> TextSize { .unwrap_or_else(|| TextSize::from(upper_bound)) } +/// A stub implementation of [`FileResolver`] intended for testing. +pub struct DummyFileResolver; + +impl FileResolver for DummyFileResolver { + fn path(&self, _file: File) -> &str { + unimplemented!() + } + + fn input(&self, _file: File) -> Input { + unimplemented!() + } + + fn notebook_index(&self, _file: &UnifiedFile) -> Option { + None + } + + fn is_notebook(&self, _file: &UnifiedFile) -> bool { + false + } + + fn current_directory(&self) -> &Path { + Path::new(".") + } +} + #[cfg(test)] mod tests { diff --git a/crates/ruff_linter/src/fs.rs b/crates/ruff_linter/src/fs.rs index ce1a1abe87ceb..5543d9a569c69 100644 --- a/crates/ruff_linter/src/fs.rs +++ b/crates/ruff_linter/src/fs.rs @@ -11,8 +11,7 @@ use crate::settings::types::CompiledPerFileIgnoreList; pub fn get_cwd() -> &'static Path { #[cfg(target_arch = "wasm32")] { - static CWD: std::sync::LazyLock = std::sync::LazyLock::new(|| PathBuf::from(".")); - &CWD + Path::new(".") } #[cfg(not(target_arch = "wasm32"))] path_absolutize::path_dedot::CWD.as_path() diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py index fd658cc5ce8ec..541af99baa571 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py @@ -706,6 +706,8 @@ f'{1: abcd "{1}" }' f'{1: abcd "{'aa'}" }' f'{1=: "abcd {'aa'}}' +# FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format +# spec, which is valid even before 3.12. f'{x:a{z:hy "user"}} \'\'\'' # Changing the outer quotes is fine because the format-spec is in a nested expression. diff --git a/crates/ruff_python_formatter/tests/fixtures.rs b/crates/ruff_python_formatter/tests/fixtures.rs index 49b207621f685..a5f25dfcd1984 100644 --- a/crates/ruff_python_formatter/tests/fixtures.rs +++ b/crates/ruff_python_formatter/tests/fixtures.rs @@ -1,10 +1,15 @@ use crate::normalizer::Normalizer; -use itertools::Itertools; +use ruff_db::diagnostic::{ + Annotation, Diagnostic, DiagnosticFormat, DiagnosticId, DisplayDiagnosticConfig, + DisplayDiagnostics, DummyFileResolver, Severity, Span, SubDiagnostic, SubDiagnosticSeverity, +}; use ruff_formatter::FormatOptions; +use ruff_python_ast::Mod; use ruff_python_ast::comparable::ComparableMod; +use ruff_python_ast::visitor::source_order::SourceOrderVisitor; use ruff_python_formatter::{PreviewMode, PyFormatOptions, format_module_source, format_range}; -use ruff_python_parser::{ParseOptions, UnsupportedSyntaxError, parse}; -use ruff_source_file::{LineIndex, OneIndexed}; +use ruff_python_parser::{ParseOptions, Parsed, UnsupportedSyntaxError, parse}; +use ruff_source_file::{LineIndex, OneIndexed, SourceFileBuilder}; use ruff_text_size::{Ranged, TextRange, TextSize}; use rustc_hash::FxHashMap; use similar::TextDiff; @@ -193,7 +198,8 @@ fn format() { writeln!(snapshot, "## Outputs").unwrap(); for (i, options) in options.into_iter().enumerate() { - let formatted_code = format_file(&content, &options, input_path); + let (formatted_code, unsupported_syntax_errors) = + format_file(&content, &options, input_path); writeln!( snapshot, @@ -210,7 +216,7 @@ fn format() { // We want to capture the differences in the preview style in our fixtures let options_preview = options.with_preview(PreviewMode::Enabled); - let formatted_preview = format_file(&content, &options_preview, input_path); + let (formatted_preview, _) = format_file(&content, &options_preview, input_path); if formatted_code != formatted_preview { // Having both snapshots makes it hard to see the difference, so we're keeping only @@ -227,14 +233,28 @@ fn format() { ) .unwrap(); } + + if !unsupported_syntax_errors.is_empty() { + writeln!( + snapshot, + "### Unsupported Syntax Errors\n{}", + DisplayDiagnostics::new( + &DummyFileResolver, + &DisplayDiagnosticConfig::default().format(DiagnosticFormat::Full), + &unsupported_syntax_errors + ) + ) + .unwrap(); + } } } else { // We want to capture the differences in the preview style in our fixtures let options = PyFormatOptions::from_extension(input_path); - let formatted_code = format_file(&content, &options, input_path); + let (formatted_code, unsupported_syntax_errors) = + format_file(&content, &options, input_path); let options_preview = options.with_preview(PreviewMode::Enabled); - let formatted_preview = format_file(&content, &options_preview, input_path); + let (formatted_preview, _) = format_file(&content, &options_preview, input_path); if formatted_code == formatted_preview { writeln!( @@ -259,6 +279,19 @@ fn format() { ) .unwrap(); } + + if !unsupported_syntax_errors.is_empty() { + writeln!( + snapshot, + "## Unsupported Syntax Errors\n{}", + DisplayDiagnostics::new( + &DummyFileResolver, + &DisplayDiagnosticConfig::default().format(DiagnosticFormat::Full), + &unsupported_syntax_errors + ) + ) + .unwrap(); + } } insta::with_settings!({ @@ -277,7 +310,11 @@ fn format() { ); } -fn format_file(source: &str, options: &PyFormatOptions, input_path: &Path) -> String { +fn format_file( + source: &str, + options: &PyFormatOptions, + input_path: &Path, +) -> (String, Vec) { let (unformatted, formatted_code) = if source.contains("") { let mut content = source.to_string(); let without_markers = content @@ -337,9 +374,10 @@ fn format_file(source: &str, options: &PyFormatOptions, input_path: &Path) -> St (Cow::Borrowed(source), formatted_code) }; - ensure_unchanged_ast(&unformatted, &formatted_code, options, input_path); + let unsupported_syntax_errors = + ensure_unchanged_ast(&unformatted, &formatted_code, options, input_path); - formatted_code + (formatted_code, unsupported_syntax_errors) } /// Format another time and make sure that there are no changes anymore @@ -391,12 +429,23 @@ Formatted twice: /// Like Black, there are a few exceptions to this "invariant" which are encoded in /// [`NormalizedMod`] and related structs. Namely, formatting can change indentation within strings, /// and can also flatten tuples within `del` statements. +/// +/// Returns any new [`UnsupportedSyntaxError`]s in the formatted code as [`Diagnostic`]s for +/// snapshotting. +/// +/// As noted in the sub-diagnostic message, new syntax errors should only be accepted when they are +/// the result of an existing syntax error in the input. For example, the formatter knows that +/// escapes in f-strings are only allowed after Python 3.12, so it can replace escaped quotes with +/// reused outer quote characters, which are also valid after 3.12, even if the configured Python +/// version is lower. Such cases disrupt the fingerprint filter because the syntax error, and thus +/// its fingerprint, is different from the input syntax error. More typical cases like using a +/// t-string before 3.14 will be filtered out and not included in snapshots. fn ensure_unchanged_ast( unformatted_code: &str, formatted_code: &str, options: &PyFormatOptions, input_path: &Path, -) { +) -> Vec { let source_type = options.source_type(); // Parse the unformatted code. @@ -407,7 +456,7 @@ fn ensure_unchanged_ast( .expect("Unformatted code to be valid syntax"); let unformatted_unsupported_syntax_errors = - collect_unsupported_syntax_errors(unformatted_parsed.unsupported_syntax_errors()); + collect_unsupported_syntax_errors(&unformatted_parsed); let mut unformatted_ast = unformatted_parsed.into_syntax(); Normalizer.visit_module(&mut unformatted_ast); @@ -422,29 +471,31 @@ fn ensure_unchanged_ast( // Assert that there are no new unsupported syntax errors let mut formatted_unsupported_syntax_errors = - collect_unsupported_syntax_errors(formatted_parsed.unsupported_syntax_errors()); + collect_unsupported_syntax_errors(&formatted_parsed); formatted_unsupported_syntax_errors .retain(|fingerprint, _| !unformatted_unsupported_syntax_errors.contains_key(fingerprint)); - if !formatted_unsupported_syntax_errors.is_empty() { - let index = LineIndex::from_source_text(formatted_code); - panic!( - "Formatted code `{}` introduced new unsupported syntax errors:\n---\n{}\n---", - input_path.display(), - formatted_unsupported_syntax_errors - .into_values() - .map(|error| { - let location = index.line_column(error.start(), formatted_code); - format!( - "{row}:{col} {error}", - row = location.line, - col = location.column - ) - }) - .join("\n") - ); - } + let file = SourceFileBuilder::new( + input_path.file_name().unwrap().to_string_lossy(), + formatted_code, + ) + .finish(); + let diagnostics = formatted_unsupported_syntax_errors + .values() + .map(|error| { + let mut diag = Diagnostic::new(DiagnosticId::InvalidSyntax, Severity::Error, error); + let span = Span::from(file.clone()).with_range(error.range()); + diag.annotate(Annotation::primary(span)); + let sub = SubDiagnostic::new( + SubDiagnosticSeverity::Warning, + "Only accept new syntax errors if they are also present in the input. \ + The formatter should not introduce syntax errors.", + ); + diag.sub(sub); + diag + }) + .collect::>(); let mut formatted_ast = formatted_parsed.into_syntax(); Normalizer.visit_module(&mut formatted_ast); @@ -466,6 +517,8 @@ fn ensure_unchanged_ast( input_path.display(), ); } + + diagnostics } struct Header<'a> { @@ -537,14 +590,56 @@ source_type = {source_type:?}"#, } } +/// A visitor to collect a sequence of node IDs for fingerprinting [`UnsupportedSyntaxError`]s. +/// +/// It visits each statement in the AST in source order and saves its range. The index of the node +/// enclosing a syntax error's range can then be retrieved with the `node_id` method. This `node_id` +/// should be stable across formatting runs since the formatter won't add or remove statements. +struct StmtVisitor { + nodes: Vec, +} + +impl StmtVisitor { + fn new(parsed: &Parsed) -> Self { + let mut visitor = Self { nodes: Vec::new() }; + visitor.visit_mod(parsed.syntax()); + visitor + } + + /// Return the index of the statement node that contains `range`. + fn node_id(&self, range: TextRange) -> usize { + self.nodes + .iter() + .enumerate() + .filter(|(_, node)| node.contains_range(range)) + .min_by_key(|(_, node)| node.len()) + .expect("Expected an enclosing node in the AST") + .0 + } +} + +impl<'a> SourceOrderVisitor<'a> for StmtVisitor { + fn visit_stmt(&mut self, stmt: &'a ruff_python_ast::Stmt) { + self.nodes.push(stmt.range()); + ruff_python_ast::visitor::source_order::walk_stmt(self, stmt); + } +} + /// Collects the unsupported syntax errors and assigns a unique hash to each error. fn collect_unsupported_syntax_errors( - errors: &[UnsupportedSyntaxError], + parsed: &Parsed, ) -> FxHashMap { let mut collected = FxHashMap::default(); - for error in errors { - let mut error_fingerprint = fingerprint_unsupported_syntax_error(error, 0); + if parsed.unsupported_syntax_errors().is_empty() { + return collected; + } + + let visitor = StmtVisitor::new(parsed); + + for error in parsed.unsupported_syntax_errors() { + let node_id = visitor.node_id(error.range); + let mut error_fingerprint = fingerprint_unsupported_syntax_error(error, node_id, 0); // Make sure that we do not get a fingerprint that is already in use // by adding in the previously generated one. @@ -552,7 +647,7 @@ fn collect_unsupported_syntax_errors( match collected.entry(error_fingerprint) { Entry::Occupied(_) => { error_fingerprint = - fingerprint_unsupported_syntax_error(error, error_fingerprint); + fingerprint_unsupported_syntax_error(error, node_id, error_fingerprint); } Entry::Vacant(entry) => { entry.insert(error.clone()); @@ -565,7 +660,11 @@ fn collect_unsupported_syntax_errors( collected } -fn fingerprint_unsupported_syntax_error(error: &UnsupportedSyntaxError, salt: u64) -> u64 { +fn fingerprint_unsupported_syntax_error( + error: &UnsupportedSyntaxError, + node_id: usize, + salt: u64, +) -> u64 { let mut hasher = DefaultHasher::new(); let UnsupportedSyntaxError { @@ -579,6 +678,7 @@ fn fingerprint_unsupported_syntax_error(error: &UnsupportedSyntaxError, salt: u6 salt.hash(&mut hasher); kind.hash(&mut hasher); target_version.hash(&mut hasher); + node_id.hash(&mut hasher); hasher.finish() } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap index 40ac13cb244ec..2eb1e09a08c6b 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap @@ -712,6 +712,8 @@ f'{1:hy "user"}' f'{1: abcd "{1}" }' f'{1: abcd "{'aa'}" }' f'{1=: "abcd {'aa'}}' +# FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format +# spec, which is valid even before 3.12. f'{x:a{z:hy "user"}} \'\'\'' # Changing the outer quotes is fine because the format-spec is in a nested expression. @@ -1534,6 +1536,8 @@ f'{1:hy "user"}' f'{1: abcd "{1}" }' f'{1: abcd "{"aa"}" }' f'{1=: "abcd {'aa'}}' +# FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format +# spec, which is valid even before 3.12. f"{x:a{z:hy \"user\"}} '''" # Changing the outer quotes is fine because the format-spec is in a nested expression. @@ -2361,6 +2365,8 @@ f'{1:hy "user"}' f'{1: abcd "{1}" }' f'{1: abcd "{"aa"}" }' f'{1=: "abcd {'aa'}}' +# FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format +# spec, which is valid even before 3.12. f"{x:a{z:hy \"user\"}} '''" # Changing the outer quotes is fine because the format-spec is in a nested expression. @@ -2409,3 +2415,64 @@ print( # Regression tests for https://github.com/astral-sh/ruff/issues/15536 print(f"{ {}, 1 }") ``` + + +### Unsupported Syntax Errors +error[invalid-syntax]: Cannot use an escape sequence (backslash) in f-strings on Python 3.10 (syntax was added in Python 3.12) + --> fstring.py:764:19 + | +762 | # FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format +763 | # spec, which is valid even before 3.12. +764 | f"{x:a{z:hy \"user\"}} '''" + | ^ +765 | +766 | # Changing the outer quotes is fine because the format-spec is in a nested expression. + | +warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. + +error[invalid-syntax]: Cannot use an escape sequence (backslash) in f-strings on Python 3.10 (syntax was added in Python 3.12) + --> fstring.py:764:13 + | +762 | # FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format +763 | # spec, which is valid even before 3.12. +764 | f"{x:a{z:hy \"user\"}} '''" + | ^ +765 | +766 | # Changing the outer quotes is fine because the format-spec is in a nested expression. + | +warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. + +error[invalid-syntax]: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12) + --> fstring.py:178:8 + | +176 | # Here, the formatter will remove the escapes which is correct because they aren't allowed +177 | # pre 3.12. This means we can assume that the f-string is used in the context of 3.12. +178 | f"foo {"'bar'"}" + | ^ +179 | f"foo {'"bar"'}" + | +warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. + +error[invalid-syntax]: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12) + --> fstring.py:773:14 + | +771 | f'{1=: "abcd \'\'}' # Don't change the outer quotes, or it results in a syntax error +772 | f"{1=: abcd \'\'}" # Changing the quotes here is fine because the inner quotes aren't the opposite quotes +773 | f"{1=: abcd \"\"}" # Changing the quotes here is fine because the inner quotes are escaped + | ^ +774 | # Don't change the quotes in the following cases: +775 | f'{x=:hy "user"} \'\'\'' + | +warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. + +error[invalid-syntax]: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12) + --> fstring.py:764:14 + | +762 | # FIXME(brent) This should not be a syntax error on output. The escaped quotes are in the format +763 | # spec, which is valid even before 3.12. +764 | f"{x:a{z:hy \"user\"}} '''" + | ^ +765 | +766 | # Changing the outer quotes is fine because the format-spec is in a nested expression. + | +warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap index bb04ae9560584..992cb23f5e9f9 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap @@ -807,6 +807,31 @@ with ( ``` +### Unsupported Syntax Errors +error[invalid-syntax]: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9) + --> with.py:333:10 + | +332 | if True: +333 | with ( + | ^ +334 | anyio.CancelScope(shield=True) +335 | if get_running_loop() + | +warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. + +error[invalid-syntax]: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9) + --> with.py:359:6 + | +357 | pass +358 | +359 | with ( + | ^ +360 | aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +361 | ): + | +warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors. + + ### Output 2 ``` indent-style = space