✨ feat(lsp): add range formatting support and tests by harehare · Pull Request #1298 · harehare/mq

harehare · 2026-02-18T14:05:58Z

Implemented range_formatting in LanguageServer for mq-lsp
Added tests for single-line, multi-line, and error cases
Updated capabilities to advertise documentRangeFormattingProvider
Ensured formatting uses indent_width: 2
Improved diagnostics and test coverage

- Implemented range_formatting in LanguageServer for mq-lsp - Added tests for single-line, multi-line, and error cases - Updated capabilities to advertise documentRangeFormattingProvider - Ensured formatting uses indent_width: 2 - Improved diagnostics and test coverage Closes #1291

Copilot

Pull request overview

Adds textDocument/rangeFormatting support to mq-lsp to enable formatting a selected range (helping performance vs full-document formatting), along with capability advertisement and new tests.

Changes:

Implement LanguageServer::range_formatting to format only the selected Range.
Advertise document_range_formatting_provider in server capabilities.
Add tests for single-line, multi-line, and error cases for range formatting.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
`crates/mq-lsp/src/server.rs`	Adds `range_formatting` handler and unit tests for range formatting behavior.
`crates/mq-lsp/src/capabilities.rs`	Advertises `document_range_formatting_provider` capability.
`'`	New repo-root file that appears to duplicate `server.rs` contents (likely accidental).

Copilot · 2026-02-18T14:11:37Z

crates/mq-lsp/src/server.rs

+            let line = lines.get(start_line).unwrap_or(&"");
+            let start = start_char.min(line.len());
+            let end = end_char.min(line.len());
+            selected.push_str(&line[start..end]);


This slices strings using start_char/end_char as byte offsets (line[start..end]). LSP Position.character is UTF-16 based and may point into the middle of a UTF-8 codepoint; this can panic at runtime and/or format the wrong substring for non-ASCII text. Convert the LSP range to safe byte offsets (UTF-16 -> UTF-8) before slicing (or use a text/rope helper crate) and avoid direct byte slicing.

crates/mq-lsp/src/server.rs

'

…gress

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

Copilot · 2026-02-19T12:17:22Z

crates/mq-lsp/src/server.rs

+            None
+        };
+
        let text = Arc::clone(&self.text_map.get(&params.text_document.uri.to_string()).unwrap());


The formatting function uses .unwrap() which will panic if the URI is not found in text_map. This is inconsistent with the safer error handling in range_formatting (lines 237-241) which returns Ok(None) if the text is not found. The formatting function should be updated to handle missing text gracefully.

Suggested change

let text = Arc::clone(&self.text_map.get(&params.text_document.uri.to_string()).unwrap());

let text = if let Some(text) = self.text_map.get(&params.text_document.uri.to_string()) {

Arc::clone(&text)

} else {

// If the document text is not available, behave like range_formatting and return no edits.

return Ok(None);

};

Copilot · 2026-02-19T12:17:23Z

crates/mq-lsp/src/server.rs

+        // Extract the text in the specified range (line and character based).
+        let mut selected = String::new();
+        if start_line == end_line {
+            // Single line selection
+            let line = lines.get(start_line).unwrap_or(&"");
+            let start = start_char.min(line.len());
+            let end = end_char.min(line.len());
+            selected.push_str(&line[start..end]);
+        } else {
+            // First line: from start_char to end of line
+            let first_line = lines.get(start_line).unwrap_or(&"");
+            let start = start_char.min(first_line.len());
+            selected.push_str(&first_line[start..]);
+            selected.push('\n');
+            // Middle lines: whole lines
+            for l in (start_line + 1)..end_line {
+                if let Some(line) = lines.get(l) {
+                    selected.push_str(line);
+                    selected.push('\n');
+                }
+            }
+            // Last line: from 0 to end_char
+            if let Some(last_line) = lines.get(end_line) {
+                let end = end_char.min(last_line.len());
+                selected.push_str(&last_line[..end]);
+            }
+        }


The string slicing operations (&line[start..end], &first_line[start..], &last_line[..end]) may panic if the character positions don't align with UTF-8 character boundaries. While the code uses .min(line.len()) to prevent out-of-bounds access, this doesn't prevent panics when slicing occurs in the middle of a multi-byte UTF-8 character. The LSP specification defines character positions as UTF-16 code units, but this code treats them as byte offsets. Consider using methods like String::is_char_boundary() to validate slice positions, or use a library that handles UTF-16 to byte offset conversion properly.

Copilot · 2026-02-19T12:17:23Z

crates/mq-lsp/src/server.rs

+        let end_line = params.range.end.line as usize;
+        let end_char = params.range.end.character as usize;
+
+        if start_line >= lines.len() || end_line > lines.len() || start_line > end_line {


The boundary check end_line > lines.len() allows end_line == lines.len() which would cause an out-of-bounds access when calling lines.get(end_line) at line 289. While .get() returns None so it won't panic, this means that if the range ends exactly one line past the document, the last line won't be included in the selection. The check should be end_line >= lines.len() to be consistent with start_line >= lines.len().

Suggested change

if start_line >= lines.len() || end_line > lines.len() || start_line > end_line {

if start_line >= lines.len() || end_line >= lines.len() || start_line > end_line {

Copilot · 2026-02-19T12:17:23Z

crates/mq-lsp/src/server.rs

+        Ok(Some(vec![ls_types::TextEdit {
+            range: params.range,
+            new_text: formatted_range,
+        }]))


The formatted text is returned with the same range as the input (range: params.range), but formatting could potentially change the number of lines or line structure. If the formatter adds or removes newlines within the selected range, the TextEdit will replace the wrong range. This could cause corruption of the surrounding code. Consider either: 1) ensuring the formatter preserves line structure for range formatting, or 2) adjusting the range to account for structural changes in the formatted output.

Copilot · 2026-02-19T12:17:23Z

crates/mq-lsp/src/server.rs

+        let result = backend.range_formatting(params).await.unwrap();
+        assert!(result.is_some());
+        let edits = result.unwrap();
+        assert_eq!(edits.len(), 1);
+        assert_eq!(edits[0].range.start.line, 0);
+        assert_eq!(edits[0].range.end.line, 0);
+        assert!(edits[0].new_text.trim().starts_with("def"));
+    }


The test validation is weak - it only checks that the formatted text contains "def" and starts with "def". This doesn't verify that the formatting actually worked correctly or that the range was properly extracted. Consider adding assertions that compare the formatted output against expected formatted code, verify line counts, or check specific formatting improvements (e.g., proper spacing).

Copilot · 2026-02-19T12:17:23Z

crates/mq-lsp/src/server.rs

+        let result = backend.range_formatting(params).await.unwrap();
+        assert!(result.is_some());
+        let edits = result.unwrap();
+        assert_eq!(edits.len(), 1);
+        // The formatted text should be the formatted substring of the range.
+        assert!(edits[0].new_text.contains("def"));
+    }


The test assertions are minimal - only checking that the result contains "def". Similar to the single-line test, this doesn't validate that the multi-line range extraction worked correctly or that the formatting was applied properly. Add assertions to verify: 1) the exact lines that were formatted, 2) that the formatting matches expected output, 3) that boundaries were handled correctly (e.g., line 1 character 0 to line 2 character 10).

Copilot · 2026-02-19T12:17:24Z

crates/mq-lsp/src/server.rs


 use bimap::BiMap;
 use dashmap::DashMap;
+use tower_lsp_server::ls_types::DocumentRangeFormattingParams;


The import of DocumentRangeFormattingParams is redundant. Since ls_types is already imported as a module at line 13, the type can be referenced as ls_types::DocumentRangeFormattingParams without a separate import. This is consistent with how other types like DocumentFormattingParams are used (line 176). Consider removing this import and using the module prefix instead.

Suggested change

use tower_lsp_server::ls_types::DocumentRangeFormattingParams;

crates/mq-lsp/src/server.rs

Copilot AI review requested due to automatic review settings February 18, 2026 14:05

Copilot started reviewing on behalf of harehare February 18, 2026 14:07 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

✨ feat(lsp): enhance document and range formatting with work done pro…

4410bf9

…gress

Copilot AI review requested due to automatic review settings February 19, 2026 12:11

harehare force-pushed the feat/lsp-range-formatting branch from 7774f4e to 4410bf9 Compare February 19, 2026 12:11

Copilot started reviewing on behalf of harehare February 19, 2026 12:12 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

harehare merged commit 0d81b34 into main Feb 19, 2026
12 checks passed

harehare deleted the feat/lsp-range-formatting branch February 19, 2026 12:45

BrewTestBot mentioned this pull request Feb 19, 2026

mq 0.5.16 Homebrew/homebrew-core#268362

Merged

-        let text = Arc::clone(&self.text_map.get(&params.text_document.uri.to_string()).unwrap());
+        let text = if let Some(text) = self.text_map.get(&params.text_document.uri.to_string()) {
+            Arc::clone(&text)
+        } else {
+            // If the document text is not available, behave like range_formatting and return no edits.
+            return Ok(None);
+        };

	if start_line >= lines.len() \|\| end_line > lines.len() \|\| start_line > end_line {
	if start_line >= lines.len() \|\| end_line >= lines.len() \|\| start_line > end_line {

Conversation

harehare commented Feb 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants