fix(rome_analyze): fix the find_diff_range function (#2679)

crates/rome_analyze/src/signals.rs

@@ -6,7 +6,7 @@ use rome_diagnostics::{
     Applicability, CodeSuggestion, Diagnostic, SubDiagnostic, SuggestionChange, SuggestionStyle,
 };
 use rome_js_syntax::{JsAnyRoot, TextRange};
-use rome_rowan::{AstNode, Language, SyntaxNode};
+use rome_rowan::{AstNode, Direction, Language, SyntaxNode};
 
 use crate::{categories::ActionCategory, registry::Rule};
 
@@ -41,7 +41,7 @@ impl From<AnalyzerAction> for CodeSuggestion {
         // Only print the relevant subset of tokens
         let mut code = String::new();
 
-        for token in action.root.syntax().descendants_tokens() {
+        for token in action.root.syntax().descendants_tokens(Direction::Next) {
             let range = token.text_range();
             if range
                 .intersect(action.new_range)
@@ -140,11 +140,11 @@ fn find_diff_range<L>(prev: &SyntaxNode<L>, next: &SyntaxNode<L>) -> Option<(Tex
 where
     L: Language,
 {
-    let prev_tokens = prev.descendants_tokens();
-    let next_tokens = next.descendants_tokens();
+    let prev_tokens = prev.descendants_tokens(Direction::Next);
+    let next_tokens = next.descendants_tokens(Direction::Next);
     let mut tokens = prev_tokens.zip(next_tokens);
 
-    let range_start = (&mut tokens).find_map(|(prev_token, next_token)| {
+    let range_start = tokens.find_map(|(prev_token, next_token)| {
         debug_assert_eq!(
             prev_token.text_range().start(),
             next_token.text_range().start(),
@@ -157,12 +157,21 @@ where
         }
     });
 
+    let prev_tokens = prev.descendants_tokens(Direction::Prev);
+    let next_tokens = next.descendants_tokens(Direction::Prev);
+    let mut tokens = prev_tokens.zip(next_tokens);
+
     let range_end = tokens.find_map(|(prev_token, next_token)| {
-        if prev_token == next_token {
-            Some((
-                prev_token.text_range().start(),
-                next_token.text_range().start(),
-            ))
+        // This compares the texts instead of the tokens themselves, since the
+        // implementation of PartialEq for SyntaxToken compares the text offset
+        // of the tokens (which may be different since we're starting from the
+        // end of the file, after the edited section)
+        // It should still be rather efficient though as identical tokens will
+        // reuse the same underlying green node after an edit, so the equality
+        // check can stop at doing a pointer + length equality without having
+        // to actually check the content of the string
+        if prev_token.text() != next_token.text() {
+            Some((prev_token.text_range().end(), next_token.text_range().end()))
         } else {
             None
         }
@@ -187,3 +196,66 @@ where
         (None, Some(_)) => unreachable!(),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use rome_js_factory::make;
+    use rome_js_syntax::{JsAnyExpression, JsAnyStatement, TextRange, TextSize, T};
+    use rome_rowan::AstNode;
+
+    use super::find_diff_range;
+
+    #[test]
+    fn diff_range() {
+        let before = make::js_if_statement(
+            make::token(T![if]),
+            make::token(T!['(']),
+            JsAnyExpression::JsIdentifierExpression(make::js_identifier_expression(
+                make::js_reference_identifier(make::ident("test")),
+            )),
+            make::token(T![')']),
+            JsAnyStatement::JsExpressionStatement(
+                make::js_expression_statement(JsAnyExpression::JsIdentifierExpression(
+                    make::js_identifier_expression(make::js_reference_identifier(make::ident(
+                        "consequent",
+                    ))),
+                ))
+                .with_semicolon_token(make::token(T![;]))
+                .build(),
+            ),
+        )
+        .with_else_clause(make::js_else_clause(
+            make::token(T![else]),
+            JsAnyStatement::JsExpressionStatement(
+                make::js_expression_statement(JsAnyExpression::JsIdentifierExpression(
+                    make::js_identifier_expression(make::js_reference_identifier(make::ident(
+                        "alternate",
+                    ))),
+                ))
+                .with_semicolon_token(make::token(T![;]))
+                .build(),
+            ),
+        ))
+        .build();
+
+        let consequent = before.consequent().unwrap();
+        let else_clause = before.else_clause().unwrap();
+        let alternate = else_clause.alternate().unwrap();
+
+        let after = before
+            .clone()
+            .with_consequent(alternate)
+            .with_else_clause(Some(else_clause.with_alternate(consequent)));
+
+        let diff = find_diff_range(before.syntax(), after.syntax())
+            .expect("failed to calculate diff range");
+
+        let start = TextSize::of("if(test)");
+        let end = TextSize::of("if(test)consequent;elsealternate");
+
+        assert_eq!(
+            diff,
+            (TextRange::new(start, end), TextRange::new(start, end))
+        );
+    }
+}

crates/rome_formatter/src/printed_tokens.rs

@@ -1,4 +1,4 @@
-use rome_rowan::{Language, SyntaxNode, SyntaxToken, TextSize};
+use rome_rowan::{Direction, Language, SyntaxNode, SyntaxToken, TextSize};
 use std::collections::BTreeSet;
 
 /// Tracks the ranges of the formatted (including replaced or tokens formatted as verbatim) tokens.
@@ -29,7 +29,7 @@ impl PrintedTokens {
     /// ## Panics
     /// If any descendant token of `root` hasn't been tracked
     pub fn assert_all_tracked<L: Language>(&self, root: &SyntaxNode<L>) {
-        let mut descendants = root.descendants_tokens();
+        let mut descendants = root.descendants_tokens(Direction::Next);
         let mut offsets = self.offsets.iter();
 
         loop {

crates/rome_js_formatter/src/builders.rs

@@ -3,7 +3,7 @@ use crate::{AsFormat, TextRange};
 use rome_formatter::{format_args, write, Argument, Arguments, GroupId, PreambleBuffer, VecBuffer};
 use rome_js_syntax::{JsLanguage, JsSyntaxNode, JsSyntaxToken};
 use rome_rowan::syntax::SyntaxTriviaPiecesIterator;
-use rome_rowan::{AstNode, Language, SyntaxTriviaPiece};
+use rome_rowan::{AstNode, Direction, Language, SyntaxTriviaPiece};
 
 /// Formats a token without its leading or trailing trivia
 ///
@@ -405,7 +405,7 @@ pub struct FormatVerbatimNode<'node> {
 }
 impl Format<JsFormatContext> for FormatVerbatimNode<'_> {
     fn fmt(&self, f: &mut JsFormatter) -> FormatResult<()> {
-        for token in self.node.descendants_tokens() {
+        for token in self.node.descendants_tokens(Direction::Next) {
             f.state_mut().track_token(&token);
         }
 

crates/rome_js_formatter/src/utils/mod.rs

@@ -21,7 +21,7 @@ use rome_js_syntax::{
     JsPrivateClassMemberName, JsTemplateElement, Modifiers, TsTemplateElement, TsType,
 };
 use rome_js_syntax::{JsSyntaxKind, JsSyntaxNode, JsSyntaxToken};
-use rome_rowan::{AstNode, AstNodeList, SyntaxResult};
+use rome_rowan::{AstNode, AstNodeList, Direction, SyntaxResult};
 
 pub(crate) use simple::*;
 pub(crate) use string_utils::*;
@@ -100,7 +100,7 @@ impl Format<JsFormatContext> for FormatInterpreterToken<'_> {
 pub(crate) fn has_formatter_trivia(node: &JsSyntaxNode) -> bool {
     let mut line_count = 0;
 
-    for token in node.descendants_tokens() {
+    for token in node.descendants_tokens(Direction::Next) {
         for trivia in token.leading_trivia().pieces() {
             if trivia.is_comments() {
                 return true;

crates/rome_js_parser/src/parse.rs

@@ -133,7 +133,7 @@ pub fn parse_common(
 /// ```
 /// use rome_js_parser::parse_script;
 /// use rome_js_syntax::{JsSyntaxToken, SourceType, JsSyntaxList, JsComputedMemberExpression};
-/// use rome_rowan::AstNode;
+/// use rome_rowan::{AstNode, Direction};
 ///
 /// let parse = parse_script("foo.bar[2]", 0);
 /// // Parse returns a JS Root which contains two lists, the directives and the statements, let's get the statements
@@ -154,7 +154,7 @@ pub fn parse_common(
 /// assert_eq!(prop.syntax().text(), "2");
 ///
 /// // Util has a function for yielding all tokens of a node.
-/// let tokens = untyped_expr_node.descendants_tokens().map(|token| token.text_trimmed().to_string()).collect::<Vec<_>>();
+/// let tokens = untyped_expr_node.descendants_tokens(Direction::Next).map(|token| token.text_trimmed().to_string()).collect::<Vec<_>>();
 ///
 /// assert_eq!(&tokens, &vec!["foo", ".", "bar", "[", "2", "]"]);
 /// ```

crates/rome_js_parser/src/tests.rs

@@ -5,7 +5,7 @@ use rome_diagnostics::termcolor::Buffer;
 use rome_diagnostics::{file::SimpleFiles, Emitter};
 use rome_js_syntax::{JsAnyRoot, JsLanguage, JsSyntaxKind, SourceType};
 use rome_js_syntax::{JsCallArguments, JsLogicalExpression, JsSyntaxNode, JsSyntaxToken};
-use rome_rowan::{AstNode, SyntaxKind, TextSize};
+use rome_rowan::{AstNode, Direction, SyntaxKind, TextSize};
 use std::fmt::Debug;
 use std::panic::catch_unwind;
 use std::path::{Path, PathBuf};
@@ -194,7 +194,7 @@ where
 pub fn test_trivia_attached_to_tokens() {
     let text = "/**/let a = 1; // nice variable \n /*hey*/ let \t b = 2; // another nice variable";
     let m = parse_module(text, 0);
-    let mut tokens = m.syntax().descendants_tokens();
+    let mut tokens = m.syntax().descendants_tokens(Direction::Next);
 
     let is_let = |x: &JsSyntaxToken| x.text_trimmed() == "let";
     let first_let = tokens.find(is_let).unwrap();
@@ -256,7 +256,7 @@ pub fn jsroot_ranges() {
     assert_eq!(4usize, range.end().into());
 
     let eq = syntax
-        .descendants_tokens()
+        .descendants_tokens(Direction::Next)
         .find(|x| x.text_trimmed() == "=")
         .unwrap();
     let range = eq.text_range();

crates/rome_js_semantic/src/tests/events.rs

@@ -10,6 +10,7 @@ use rome_js_syntax::SourceType;
 use rome_js_syntax::TextRange;
 use rome_js_syntax::WalkEvent;
 use rome_markup::markup;
+use rome_rowan::Direction;
 use rome_rowan::NodeOrToken;
 
 #[test]
@@ -73,7 +74,7 @@ fn assert(code: &str) {
 
     let mut declarations_assertions = BTreeMap::new();
 
-    for node in r.syntax().preorder_with_tokens() {
+    for node in r.syntax().preorder_with_tokens(Direction::Next) {
         if let WalkEvent::Enter(NodeOrToken::Token(token)) = node {
             let trivia = token.trailing_trivia();
             let text = trivia.text();

crates/rome_js_semantic/src/tests/scopes.rs

@@ -4,7 +4,7 @@ use crate::{semantic_events, SemanticEvent};
 use rome_console::{markup, ConsoleExt, EnvConsole};
 use rome_diagnostics::{file::SimpleFile, Applicability, Diagnostic, Severity};
 use rome_js_syntax::{JsSyntaxToken, SourceType, TextRange, TextSize, WalkEvent};
-use rome_rowan::NodeOrToken;
+use rome_rowan::{Direction, NodeOrToken};
 
 #[test]
 pub fn ok_scope_blocks() {
@@ -86,7 +86,7 @@ fn assert(code: &str) {
     let mut scope_start_assertions = BTreeMap::new();
     let mut scope_end_assertions = BTreeMap::new();
 
-    for node in r.syntax().preorder_with_tokens() {
+    for node in r.syntax().preorder_with_tokens(Direction::Next) {
         if let WalkEvent::Enter(NodeOrToken::Token(token)) = node {
             let trivia = token.trailing_trivia();
             let text = trivia.text();

crates/rome_rowan/src/cursor/node.rs

@@ -238,11 +238,12 @@ impl SyntaxNode {
     }
 
     pub fn first_token(&self) -> Option<SyntaxToken> {
-        self.descendants_with_tokens().find_map(|x| x.into_token())
+        self.descendants_with_tokens(Direction::Next)
+            .find_map(|x| x.into_token())
     }
 
     pub fn last_token(&self) -> Option<SyntaxToken> {
-        PreorderWithTokens::reverse(self.clone())
+        PreorderWithTokens::new(self.clone(), Direction::Prev)
             .filter_map(|event| match event {
                 WalkEvent::Enter(it) => Some(it),
                 WalkEvent::Leave(_) => None,
@@ -279,11 +280,15 @@ impl SyntaxNode {
     }
 
     #[inline]
-    pub fn descendants_with_tokens(&self) -> impl Iterator<Item = SyntaxElement> {
-        self.preorder_with_tokens().filter_map(|event| match event {
-            WalkEvent::Enter(it) => Some(it),
-            WalkEvent::Leave(_) => None,
-        })
+    pub fn descendants_with_tokens(
+        &self,
+        direction: Direction,
+    ) -> impl Iterator<Item = SyntaxElement> {
+        self.preorder_with_tokens(direction)
+            .filter_map(|event| match event {
+                WalkEvent::Enter(it) => Some(it),
+                WalkEvent::Leave(_) => None,
+            })
     }
 
     #[inline]
@@ -292,8 +297,8 @@ impl SyntaxNode {
     }
 
     #[inline]
-    pub fn preorder_with_tokens(&self) -> PreorderWithTokens {
-        PreorderWithTokens::new(self.clone())
+    pub fn preorder_with_tokens(&self, direction: Direction) -> PreorderWithTokens {
+        PreorderWithTokens::new(self.clone(), direction)
     }
 
     pub(crate) fn preorder_slots(&self) -> SlotsPreorder {
@@ -427,7 +432,7 @@ impl fmt::Debug for SyntaxNode {
 
 impl fmt::Display for SyntaxNode {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.preorder_with_tokens()
+        self.preorder_with_tokens(Direction::Next)
             .filter_map(|event| match event {
                 WalkEvent::Enter(NodeOrToken::Token(token)) => Some(token),
                 _ => None,
@@ -557,22 +562,12 @@ pub(crate) struct PreorderWithTokens {
 }
 
 impl PreorderWithTokens {
-    fn new(start: SyntaxNode) -> PreorderWithTokens {
-        let next = Some(WalkEvent::Enter(start.clone().into()));
-        PreorderWithTokens {
-            start: start.into(),
-            next,
-            direction: Direction::Next,
-            skip_subtree: false,
-        }
-    }
-
-    fn reverse(start: SyntaxNode) -> PreorderWithTokens {
+    fn new(start: SyntaxNode, direction: Direction) -> PreorderWithTokens {
         let next = Some(WalkEvent::Enter(start.clone().into()));
         PreorderWithTokens {
             start: start.into(),
             next,
-            direction: Direction::Prev,
+            direction,
             skip_subtree: false,
         }
     }

crates/rome_rowan/src/syntax.rs

@@ -413,7 +413,7 @@ mod tests {
         // as NodeOrToken
 
         let eq_token = node
-            .descendants_with_tokens()
+            .descendants_with_tokens(Direction::Next)
             .find(|x| x.kind() == RawLanguageKind::EQUAL_TOKEN)
             .unwrap();
 

crates/rome_rowan/src/syntax/node.rs

@@ -328,13 +328,18 @@ impl<L: Language> SyntaxNode<L> {
         self.raw.descendants().map(SyntaxNode::from)
     }
 
-    pub fn descendants_tokens(&self) -> impl Iterator<Item = SyntaxToken<L>> {
-        self.descendants_with_tokens()
+    pub fn descendants_tokens(&self, direction: Direction) -> impl Iterator<Item = SyntaxToken<L>> {
+        self.descendants_with_tokens(direction)
             .filter_map(|x| x.as_token().cloned())
     }
 
-    pub fn descendants_with_tokens(&self) -> impl Iterator<Item = SyntaxElement<L>> {
-        self.raw.descendants_with_tokens().map(NodeOrToken::from)
+    pub fn descendants_with_tokens(
+        &self,
+        direction: Direction,
+    ) -> impl Iterator<Item = SyntaxElement<L>> {
+        self.raw
+            .descendants_with_tokens(direction)
+            .map(NodeOrToken::from)
     }
 
     /// Traverse the subtree rooted at the current node (including the current
@@ -348,9 +353,9 @@ impl<L: Language> SyntaxNode<L> {
 
     /// Traverse the subtree rooted at the current node (including the current
     /// node) in preorder, including tokens.
-    pub fn preorder_with_tokens(&self) -> PreorderWithTokens<L> {
+    pub fn preorder_with_tokens(&self, direction: Direction) -> PreorderWithTokens<L> {
         PreorderWithTokens {
-            raw: self.raw.preorder_with_tokens(),
+            raw: self.raw.preorder_with_tokens(direction),
             _p: PhantomData,
         }
     }
@@ -458,7 +463,7 @@ impl<L: Language> SyntaxNode<L> {
     /// Whether the node contains any comments. This function checks
     /// **all the descendants** of the current node.
     pub fn has_comments_descendants(&self) -> bool {
-        self.descendants_tokens()
+        self.descendants_tokens(Direction::Next)
             .any(|tok| tok.has_trailing_comments() || tok.has_leading_comments())
     }