From a1b1e5a79eacbaed24ce4e207617a8e66a04536d Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 19 Jul 2023 15:52:43 +0200 Subject: [PATCH 1/2] Add message to formatter SyntaxError **Summary** Add a static string error message to the formatter syntax error so we can disambiguate where the syntax error came from **Test Plan** No fixed tests, we don't expect this to occur, but it helped with transformers syntax error debugging: ``` Error: Failed to format node Caused by: syntax error: slice first colon token was not a colon ``` --- crates/ruff_formatter/src/diagnostics.rs | 6 ++++-- crates/ruff_python_formatter/src/comments/format.rs | 4 +++- .../src/expression/expr_slice.rs | 12 +++++++++--- .../ruff_python_formatter/src/expression/string.rs | 8 ++++++-- .../src/statement/stmt_assign.rs | 4 +++- .../ruff_python_formatter/src/statement/stmt_with.rs | 8 ++++++-- 6 files changed, 31 insertions(+), 11 deletions(-) diff --git a/crates/ruff_formatter/src/diagnostics.rs b/crates/ruff_formatter/src/diagnostics.rs index a5fd6f9632a4d..0a79a4ede3dcf 100644 --- a/crates/ruff_formatter/src/diagnostics.rs +++ b/crates/ruff_formatter/src/diagnostics.rs @@ -8,7 +8,7 @@ use std::error::Error; pub enum FormatError { /// In case a node can't be formatted because it either misses a require child element or /// a child is present that should not (e.g. a trailing comma after a rest element). - SyntaxError, + SyntaxError { message: &'static str }, /// In case range formatting failed because the provided range was larger /// than the formatted syntax tree RangeError { input: TextRange, tree: TextRange }, @@ -28,7 +28,9 @@ pub enum FormatError { impl std::fmt::Display for FormatError { fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - FormatError::SyntaxError => fmt.write_str("syntax error"), + FormatError::SyntaxError {message} => { + std::write!(fmt, "syntax error: {message}") + }, FormatError::RangeError { input, tree } => std::write!( fmt, "formatting range {input:?} is larger than syntax tree {tree:?}" diff --git a/crates/ruff_python_formatter/src/comments/format.rs b/crates/ruff_python_formatter/src/comments/format.rs index aa7d7296f1059..4a63caa02e0e9 100644 --- a/crates/ruff_python_formatter/src/comments/format.rs +++ b/crates/ruff_python_formatter/src/comments/format.rs @@ -251,7 +251,9 @@ impl Format> for FormatComment<'_> { let trailing_whitespace_len = comment_text.text_len() - trimmed.text_len(); let Some(content) = trimmed.strip_prefix('#') else { - return Err(FormatError::SyntaxError); + return Err(FormatError::SyntaxError { + message: "Didn't find expected comment token `#`", + }); }; // Fast path for correctly formatted comments: diff --git a/crates/ruff_python_formatter/src/expression/expr_slice.rs b/crates/ruff_python_formatter/src/expression/expr_slice.rs index 93434b87777fd..6a4921d371fb8 100644 --- a/crates/ruff_python_formatter/src/expression/expr_slice.rs +++ b/crates/ruff_python_formatter/src/expression/expr_slice.rs @@ -163,9 +163,13 @@ pub(crate) fn find_colons( .as_ref() .map_or(range.start(), |lower| lower.range().end()); let first_colon = - first_non_trivia_token(after_lower, contents).ok_or(FormatError::SyntaxError)?; + first_non_trivia_token(after_lower, contents).ok_or(FormatError::SyntaxError { + message: "Din't find any token for slice first colon", + })?; if first_colon.kind != TokenKind::Colon { - return Err(FormatError::SyntaxError); + return Err(FormatError::SyntaxError { + message: "slice first colon token was not a colon", + }); } let after_upper = upper @@ -173,7 +177,9 @@ pub(crate) fn find_colons( .map_or(first_colon.end(), |upper| upper.range().end()); // At least the closing bracket must exist, so there must be a token there let next_token = - first_non_trivia_token(after_upper, contents).ok_or(FormatError::SyntaxError)?; + first_non_trivia_token(after_upper, contents).ok_or(FormatError::SyntaxError { + message: "Din't find any token for slice second colon", + })?; let second_colon = if next_token.kind == TokenKind::Colon { debug_assert!( next_token.range.start() < range.end(), diff --git a/crates/ruff_python_formatter/src/expression/string.rs b/crates/ruff_python_formatter/src/expression/string.rs index a162a9a76a938..9af4928dd2cd8 100644 --- a/crates/ruff_python_formatter/src/expression/string.rs +++ b/crates/ruff_python_formatter/src/expression/string.rs @@ -91,7 +91,9 @@ impl Format> for FormatStringContinuation<'_> { continue; } Err(_) => { - return Err(FormatError::SyntaxError); + return Err(FormatError::SyntaxError { + message: "Unexpected lexer error in string formatting", + }); } }; @@ -167,7 +169,9 @@ impl Format> for FormatStringPart { let prefix = StringPrefix::parse(string_content); let after_prefix = &string_content[usize::from(prefix.text_len())..]; - let quotes = StringQuotes::parse(after_prefix).ok_or(FormatError::SyntaxError)?; + let quotes = StringQuotes::parse(after_prefix).ok_or(FormatError::SyntaxError { + message: "Didn't find string quotes after prefix", + })?; let relative_raw_content_range = TextRange::new( prefix.text_len() + quotes.text_len(), string_content.text_len() - quotes.text_len(), diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index 55ff07d9495eb..13d2a5e468252 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -20,7 +20,9 @@ impl FormatNodeRule for FormatStmtAssign { type_comment: _, } = item; - let (first, rest) = targets.split_first().ok_or(FormatError::SyntaxError)?; + let (first, rest) = targets.split_first().ok_or(FormatError::SyntaxError { + message: "Expected at least on assignment target", + })?; write!( f, diff --git a/crates/ruff_python_formatter/src/statement/stmt_with.rs b/crates/ruff_python_formatter/src/statement/stmt_with.rs index 2c610f029d588..887a99c07e50a 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_with.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_with.rs @@ -117,14 +117,18 @@ fn are_with_items_parenthesized( with: &AnyStatementWith, context: &PyFormatContext, ) -> FormatResult { - let first_with_item = with.items().first().ok_or(FormatError::SyntaxError)?; + let first_with_item = with.items().first().ok_or(FormatError::SyntaxError { + message: "Expected at least one with item", + })?; let before_first_with_item = TextRange::new(with.start(), first_with_item.start()); let mut tokenizer = SimpleTokenizer::new(context.source(), before_first_with_item) .skip_trivia() .skip_while(|t| t.kind() == TokenKind::Async); - let with_keyword = tokenizer.next().ok_or(FormatError::SyntaxError)?; + let with_keyword = tokenizer.next().ok_or(FormatError::SyntaxError { + message: "Expected a with keyword, didn't find any token", + })?; debug_assert_eq!( with_keyword.kind(), From 64efcab472794ede3179c83412c877dfb51ad392 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 19 Jul 2023 16:55:40 +0200 Subject: [PATCH 2/2] Add helper method --- crates/ruff_formatter/src/diagnostics.rs | 6 ++++++ .../src/comments/format.rs | 6 +++--- .../src/expression/expr_slice.rs | 20 +++++++++---------- .../src/expression/string.rs | 12 +++++------ .../src/statement/stmt_assign.rs | 6 +++--- .../src/statement/stmt_with.rs | 13 ++++++------ 6 files changed, 34 insertions(+), 29 deletions(-) diff --git a/crates/ruff_formatter/src/diagnostics.rs b/crates/ruff_formatter/src/diagnostics.rs index 0a79a4ede3dcf..9228975a7e64e 100644 --- a/crates/ruff_formatter/src/diagnostics.rs +++ b/crates/ruff_formatter/src/diagnostics.rs @@ -59,6 +59,12 @@ impl From<&PrintError> for FormatError { } } +impl FormatError { + pub fn syntax_error(message: &'static str) -> Self { + Self::SyntaxError { message } + } +} + #[derive(Debug, Copy, Clone, Eq, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum InvalidDocumentError { diff --git a/crates/ruff_python_formatter/src/comments/format.rs b/crates/ruff_python_formatter/src/comments/format.rs index 4a63caa02e0e9..7376f9288b2db 100644 --- a/crates/ruff_python_formatter/src/comments/format.rs +++ b/crates/ruff_python_formatter/src/comments/format.rs @@ -251,9 +251,9 @@ impl Format> for FormatComment<'_> { let trailing_whitespace_len = comment_text.text_len() - trimmed.text_len(); let Some(content) = trimmed.strip_prefix('#') else { - return Err(FormatError::SyntaxError { - message: "Didn't find expected comment token `#`", - }); + return Err(FormatError::syntax_error( + "Didn't find expected comment token `#`", + )); }; // Fast path for correctly formatted comments: diff --git a/crates/ruff_python_formatter/src/expression/expr_slice.rs b/crates/ruff_python_formatter/src/expression/expr_slice.rs index 6a4921d371fb8..be5f8396b724b 100644 --- a/crates/ruff_python_formatter/src/expression/expr_slice.rs +++ b/crates/ruff_python_formatter/src/expression/expr_slice.rs @@ -162,24 +162,22 @@ pub(crate) fn find_colons( let after_lower = lower .as_ref() .map_or(range.start(), |lower| lower.range().end()); - let first_colon = - first_non_trivia_token(after_lower, contents).ok_or(FormatError::SyntaxError { - message: "Din't find any token for slice first colon", - })?; + let first_colon = first_non_trivia_token(after_lower, contents).ok_or( + FormatError::syntax_error("Din't find any token for slice first colon"), + )?; if first_colon.kind != TokenKind::Colon { - return Err(FormatError::SyntaxError { - message: "slice first colon token was not a colon", - }); + return Err(FormatError::syntax_error( + "slice first colon token was not a colon", + )); } let after_upper = upper .as_ref() .map_or(first_colon.end(), |upper| upper.range().end()); // At least the closing bracket must exist, so there must be a token there - let next_token = - first_non_trivia_token(after_upper, contents).ok_or(FormatError::SyntaxError { - message: "Din't find any token for slice second colon", - })?; + let next_token = first_non_trivia_token(after_upper, contents).ok_or( + FormatError::syntax_error("Din't find any token for slice second colon"), + )?; let second_colon = if next_token.kind == TokenKind::Colon { debug_assert!( next_token.range.start() < range.end(), diff --git a/crates/ruff_python_formatter/src/expression/string.rs b/crates/ruff_python_formatter/src/expression/string.rs index 9af4928dd2cd8..1970b7216398c 100644 --- a/crates/ruff_python_formatter/src/expression/string.rs +++ b/crates/ruff_python_formatter/src/expression/string.rs @@ -91,9 +91,9 @@ impl Format> for FormatStringContinuation<'_> { continue; } Err(_) => { - return Err(FormatError::SyntaxError { - message: "Unexpected lexer error in string formatting", - }); + return Err(FormatError::syntax_error( + "Unexpected lexer error in string formatting", + )); } }; @@ -169,9 +169,9 @@ impl Format> for FormatStringPart { let prefix = StringPrefix::parse(string_content); let after_prefix = &string_content[usize::from(prefix.text_len())..]; - let quotes = StringQuotes::parse(after_prefix).ok_or(FormatError::SyntaxError { - message: "Didn't find string quotes after prefix", - })?; + let quotes = StringQuotes::parse(after_prefix).ok_or(FormatError::syntax_error( + "Didn't find string quotes after prefix", + ))?; let relative_raw_content_range = TextRange::new( prefix.text_len() + quotes.text_len(), string_content.text_len() - quotes.text_len(), diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index 13d2a5e468252..4a2d6538e4d79 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -20,9 +20,9 @@ impl FormatNodeRule for FormatStmtAssign { type_comment: _, } = item; - let (first, rest) = targets.split_first().ok_or(FormatError::SyntaxError { - message: "Expected at least on assignment target", - })?; + let (first, rest) = targets.split_first().ok_or(FormatError::syntax_error( + "Expected at least on assignment target", + ))?; write!( f, diff --git a/crates/ruff_python_formatter/src/statement/stmt_with.rs b/crates/ruff_python_formatter/src/statement/stmt_with.rs index 887a99c07e50a..95465e9ec5e8f 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_with.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_with.rs @@ -117,18 +117,19 @@ fn are_with_items_parenthesized( with: &AnyStatementWith, context: &PyFormatContext, ) -> FormatResult { - let first_with_item = with.items().first().ok_or(FormatError::SyntaxError { - message: "Expected at least one with item", - })?; + let first_with_item = with + .items() + .first() + .ok_or(FormatError::syntax_error("Expected at least one with item"))?; let before_first_with_item = TextRange::new(with.start(), first_with_item.start()); let mut tokenizer = SimpleTokenizer::new(context.source(), before_first_with_item) .skip_trivia() .skip_while(|t| t.kind() == TokenKind::Async); - let with_keyword = tokenizer.next().ok_or(FormatError::SyntaxError { - message: "Expected a with keyword, didn't find any token", - })?; + let with_keyword = tokenizer.next().ok_or(FormatError::syntax_error( + "Expected a with keyword, didn't find any token", + ))?; debug_assert_eq!( with_keyword.kind(),