From 245c0eaadd905464adf6d99c176a25dea644540a Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 19 Apr 2022 12:25:10 +0200 Subject: [PATCH] Code review feedback --- crates/rome_formatter/src/format_element.rs | 20 ++++++++++---------- crates/rome_rowan/src/syntax/token.rs | 1 + 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/rome_formatter/src/format_element.rs b/crates/rome_formatter/src/format_element.rs index 19850747a43..98de6d31b75 100644 --- a/crates/rome_formatter/src/format_element.rs +++ b/crates/rome_formatter/src/format_element.rs @@ -1127,7 +1127,8 @@ pub enum Token { // The position of the dynamic token in the unformatted source code source_position: TextSize, }, - // A token that is taken 1:1 from the source code + /// A token for a text that is taken as is from the source code (input text and formatted representation are identical). + /// Implementing by taking a slice from a `SyntaxToken` to avoid allocating a new string. SyntaxTokenSlice { /// The start position of the token in the unformatted source code source_position: TextSize, @@ -1160,7 +1161,8 @@ impl Token { /// Create a token from a dynamic string and a range of the input source pub fn new_dynamic(text: String, position: TextSize) -> Self { - Self::assert_no_newlines(&text); + debug_assert_no_newlines(&text); + Self::Dynamic { text: text.into_boxed_str(), source_position: position, @@ -1179,8 +1181,6 @@ impl Token { token: &SyntaxToken, start: TextSize, ) -> Self { - Self::assert_no_newlines(&text); - match text { Cow::Owned(text) => Self::new_dynamic(text, start), Cow::Borrowed(text) => { @@ -1188,7 +1188,7 @@ impl Token { debug_assert_eq!( text, &token.text()[range - token.text_range().start()], - "The borrowed string doesn't match the specified token substring" + "The borrowed string doesn't match the specified token substring. Does the borrowed string belong to this token and range?" ); Token::new_syntax_token_slice(token, range) } @@ -1200,7 +1200,7 @@ impl Token { let relative_range = range - token.text_range().start(); let slice = token.token_text().slice(relative_range); - Self::assert_no_newlines(&slice); + debug_assert_no_newlines(&slice); Self::SyntaxTokenSlice { slice, @@ -1208,10 +1208,6 @@ impl Token { } } - fn assert_no_newlines(text: &str) { - debug_assert!(!text.contains('\r'), "The content '{}' contains an unsupported '\\r' line terminator character but string tokens must only use line feeds '\\n' as line separator. Use '\\n' instead of '\\r' and '\\r\\n' to insert a line break in strings.", text); - } - /// Get the range of the input source covered by this token, /// or None if the token was synthesized by the formatter pub fn source_position(&self) -> Option<&TextSize> { @@ -1227,6 +1223,10 @@ impl Token { } } +fn debug_assert_no_newlines(text: &str) { + debug_assert!(!text.contains('\r'), "The content '{}' contains an unsupported '\\r' line terminator character but string tokens must only use line feeds '\\n' as line separator. Use '\\n' instead of '\\r' and '\\r\\n' to insert a line break in strings.", text); +} + // Token equality only compares the text content impl PartialEq for Token { fn eq(&self, other: &Self) -> bool { diff --git a/crates/rome_rowan/src/syntax/token.rs b/crates/rome_rowan/src/syntax/token.rs index 5d0c5ce020a..eba0d838752 100644 --- a/crates/rome_rowan/src/syntax/token.rs +++ b/crates/rome_rowan/src/syntax/token.rs @@ -47,6 +47,7 @@ impl SyntaxToken { self.raw.text() } + /// Returns the text of a token, including all trivia as an owned value. pub fn token_text(&self) -> SyntaxTokenText { self.raw.token_text() }