Merge pull request #1422 from Xanewok/reformat-range-fix

Always send correct end column when formatting

Author: Xanewok

rls/src/actions/format.rs

@@ -78,6 +78,9 @@ impl Rustfmt {
             NewlineStyle::Native => native,
         };
 
+        let lsp_line_length = |line: &str| line.chars().map(char::len_utf16).sum();
+        let line_cols: Vec<usize> = input.lines().map(lsp_line_length).collect();
+
         let output = self.format(input, cfg)?;
         let ModifiedLines { chunks } = output.parse().map_err(|_| Error::Failed)?;
 
@@ -86,15 +89,21 @@ impl Rustfmt {
             .map(|item| {
                 // Rustfmt's line indices are 1-based
                 let start_line = u64::from(item.line_number_orig) - 1;
-                // Could underflow if we don't remove lines and there's only one
-                let removed = u64::from(item.lines_removed).saturating_sub(1);
+                let end_line = {
+                    // Could underflow if we don't remove lines and there's only one
+                    let removed = u64::from(item.lines_removed).saturating_sub(1);
+                    start_line + removed
+                };
+                let end_col: Option<usize> = line_cols.get(end_line as usize).copied();
+                let end_col: u64 = end_col.map(|col| col as u64).unwrap_or_else(u64::max_value);
+
                 TextEdit {
                     range: Range {
                         start: Position::new(start_line, 0),
                         // We don't extend the range past the last line because
                         // sometimes it may not exist, skewing the diff and
                         // making us add an invalid additional trailing newline.
-                        end: Position::new(start_line + removed, u64::max_value()),
+                        end: Position::new(end_line, end_col),
                     },
                     new_text: item.lines.join(newline),
                 }
@@ -202,34 +211,34 @@ mod tests {
 
     #[test]
     fn calc_text_edits() {
-        let config = || FmtConfig::default().get_rustfmt_config().clone();
-        let format = |x: &str| Rustfmt::Internal.calc_text_edits(x.to_string(), config()).unwrap();
-        let line_range = |start, end| Range {
-            start: Position { line: start, character: 0 },
-            end: Position { line: end, character: u64::max_value() },
-        };
-        // Handle single-line text wrt. added/removed trailing newline
-        assert_eq!(
-            format("fn main() {} "),
-            vec![TextEdit { range: line_range(0, 0), new_text: "fn main() {}\n".to_owned() }]
-        );
-
-        assert_eq!(
-            format("fn main() {} \n"),
-            vec![TextEdit { range: line_range(0, 0), new_text: "fn main() {}".to_owned() }]
-        );
+        fn format(input: &str) -> Vec<TextEdit> {
+            let config = || FmtConfig::default().get_rustfmt_config().clone();
+            Rustfmt::Internal.calc_text_edits(input.to_string(), config()).unwrap()
+        }
 
-        assert_eq!(
-            format("\nfn main() {} \n"),
-            vec![TextEdit { range: line_range(0, 1), new_text: "fn main() {}".to_owned() }]
-        );
+        fn test_case(input: &str, output: Vec<(u64, u64, u64, u64, &str)>) {
+            assert_eq!(
+                format(input),
+                output
+                    .into_iter()
+                    .map(|(start_l, start_c, end_l, end_c, out)| TextEdit {
+                        range: Range {
+                            start: Position { line: start_l, character: start_c },
+                            end: Position { line: end_l, character: end_c },
+                        },
+                        new_text: out.to_owned(),
+                    })
+                    .collect::<Vec<_>>()
+            )
+        }
+        // Handle single-line text wrt. added/removed trailing newline
+        test_case("fn main() {} ", vec![(0, 0, 0, 13, "fn main() {}\n")]);
+        test_case("fn main() {} \n", vec![(0, 0, 0, 13, "fn main() {}")]);
+        test_case("\nfn main() {} \n", vec![(0, 0, 1, 13, "fn main() {}")]);
         // Check that we send two separate edits
-        assert_eq!(
-            format("  struct Upper ;\n\nstruct Lower ;"),
-            vec![
-                TextEdit { range: line_range(0, 0), new_text: "struct Upper;".to_owned() },
-                TextEdit { range: line_range(2, 2), new_text: "struct Lower;\n".to_owned() }
-            ]
+        test_case(
+            "  struct Upper ;\n\nstruct Lower ;",
+            vec![(0, 0, 0, 16, "struct Upper;"), (2, 0, 2, 14, "struct Lower;\n")],
         );
     }
 }

tests/client.rs

@@ -1763,7 +1763,7 @@ fn client_reformat() {
     assert_eq!(result.unwrap()[0], TextEdit {
         range: Range {
             start: Position { line: 0, character: 0 },
-            end: Position { line: 1, character: u64::max_value() },
+            end: Position { line: 1, character: 69 },
         },
         new_text: "pub mod foo;\npub fn main() {\n    let world = \"world\";\n    println!(\"Hello, {}!\", world);\n}".to_string(),
     });