Repository: rust-lang/rust-analyzer
Pull request #19124 (2025-02-09 19:52:07)

Fix off-by-one error in RangeFormatting
fixes https://github.com/rust-lang/rust-analyzer/issues/19123.

the test doesn't actually verify that rust-analyzer applies the range *returned* from rustfmt correctly, but i tested locally that it does.

jyn514: > the test doesn't actually verify that rust-analyzer applies the range _returned_ from rustfmt correctly, but i tested locally that it does.

oh, i was confused. RA never does IO. the editor takes the returned JSON and applies it to the file. so this is testing everything is should, just a little indirectly.

Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index ec33009239..81b55712d7 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -64,7 +64,11 @@ jobs:
         run: |
           rustup update --no-self-update ${{ env.RUST_CHANNEL }}
           rustup default ${{ env.RUST_CHANNEL }}
-          rustup component add --toolchain ${{ env.RUST_CHANNEL }} rustfmt rust-src
+          rustup component add --toolchain ${{ env.RUST_CHANNEL }} rust-src
+          # We always use a nightly rustfmt, regardless of channel, because we need
+          # --file-lines.
+          rustup toolchain add nightly --profile minimal
+          rustup component add --toolchain nightly rustfmt
       # https://github.com/actions-rust-lang/setup-rust-toolchain/blob/main/rust.json
       - name: Install Rust Problem Matcher
         if: matrix.os == 'ubuntu-latest'
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index e915e55722..2e9bb8c08e 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -3798,8 +3798,10 @@ mod tests {
         (config, _, _) = config.apply_change(change);
 
         assert_eq!(config.cargo_targetDir(None), &Some(TargetDirectory::UseSubdirectory(true)));
+        let target =
+            Utf8PathBuf::from(std::env::var("CARGO_TARGET_DIR").unwrap_or("target".to_owned()));
         assert!(
-            matches!(config.flycheck(None), FlycheckConfig::CargoCommand { options, .. } if options.target_dir == Some(Utf8PathBuf::from("target/rust-analyzer")))
+            matches!(config.flycheck(None), FlycheckConfig::CargoCommand { options, .. } if options.target_dir == Some(target.join("rust-analyzer")))
         );
     }
 
diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs
index ed028f1d37..37e4a4940b 100644
--- a/crates/rust-analyzer/src/handlers/request.rs
+++ b/crates/rust-analyzer/src/handlers/request.rs
@@ -2284,7 +2284,8 @@ fn run_rustfmt(
                 cmd.arg(
                     json!([{
                         "file": "stdin",
-                        "range": [start_line, end_line]
+                        // LineCol is 0-based, but rustfmt is 1-based.
+                        "range": [start_line + 1, end_line + 1]
                     }])
                     .to_string(),
                 );


Test patch:
diff --git a/crates/rust-analyzer/tests/slow-tests/main.rs b/crates/rust-analyzer/tests/slow-tests/main.rs
index 2b3c0a47a2..7f95641746 100644
--- a/crates/rust-analyzer/tests/slow-tests/main.rs
+++ b/crates/rust-analyzer/tests/slow-tests/main.rs
@@ -21,12 +21,14 @@ use lsp_types::{
     notification::DidOpenTextDocument,
     request::{
         CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest,
-        InlayHintRequest, InlayHintResolveRequest, WillRenameFiles, WorkspaceSymbolRequest,
+        InlayHintRequest, InlayHintResolveRequest, RangeFormatting, WillRenameFiles,
+        WorkspaceSymbolRequest,
     },
     CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams,
-    DocumentFormattingParams, FileRename, FormattingOptions, GotoDefinitionParams, HoverParams,
-    InlayHint, InlayHintLabel, InlayHintParams, PartialResultParams, Position, Range,
-    RenameFilesParams, TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams,
+    DocumentFormattingParams, DocumentRangeFormattingParams, FileRename, FormattingOptions,
+    GotoDefinitionParams, HoverParams, InlayHint, InlayHintLabel, InlayHintParams,
+    PartialResultParams, Position, Range, RenameFilesParams, TextDocumentItem,
+    TextDocumentPositionParams, WorkDoneProgressParams,
 };
 use rust_analyzer::lsp::ext::{OnEnter, Runnables, RunnablesParams};
 use serde_json::json;
@@ -660,6 +662,70 @@ fn main() {}
     );
 }
 
+#[test]
+fn test_format_document_range() {
+    if skip_slow_tests() {
+        return;
+    }
+
+    let server = Project::with_fixture(
+        r#"
+//- /Cargo.toml
+[package]
+name = "foo"
+version = "0.0.0"
+
+//- /src/lib.rs
+fn main() {
+    let unit_offsets_cache = collect(dwarf.units  ())  ?;
+}
+"#,
+    )
+    .with_config(serde_json::json!({
+        "rustfmt": {
+            "overrideCommand": [ "rustfmt", "+nightly", ],
+            "rangeFormatting": { "enable": true }
+        },
+    }))
+    .server()
+    .wait_until_workspace_is_loaded();
+
+    server.request::<RangeFormatting>(
+        DocumentRangeFormattingParams {
+            range: Range {
+                end: Position { line: 1, character: 0 },
+                start: Position { line: 1, character: 0 },
+            },
+            text_document: server.doc_id("src/lib.rs"),
+            options: FormattingOptions {
+                tab_size: 4,
+                insert_spaces: false,
+                insert_final_newline: None,
+                trim_final_newlines: None,
+                trim_trailing_whitespace: None,
+                properties: HashMap::new(),
+            },
+            work_done_progress_params: WorkDoneProgressParams::default(),
+        },
+        json!([
+            {
+                "newText": "",
+                "range": {
+                    "start": { "character": 48, "line": 1 },
+                    "end": { "character": 50, "line": 1 },
+                },
+            },
+            {
+                "newText": "",
+                "range": {
+                    "start": { "character": 53, "line": 1 },
+                    "end": { "character": 55, "line": 1 },
+                },
+            }
+        ]),
+    );
+}
+
 #[test]
 fn test_missing_module_code_action() {
     if skip_slow_tests() {
diff --git a/crates/test-utils/src/lib.rs b/crates/test-utils/src/lib.rs
index 36be9937d3..e7279fa1f6 100644
--- a/crates/test-utils/src/lib.rs
+++ b/crates/test-utils/src/lib.rs
@@ -396,12 +396,19 @@ pub fn skip_slow_tests() -> bool {
     if should_skip {
         eprintln!("ignoring slow test");
     } else {
-        let path = project_root().join("./target/.slow_tests_cookie");
+        let path = target_dir().join(".slow_tests_cookie");
         fs::write(path, ".").unwrap();
     }
     should_skip
 }
 
+pub fn target_dir() -> Utf8PathBuf {
+    match std::env::var("CARGO_TARGET_DIR") {
+        Ok(target) => Utf8PathBuf::from(target),
+        Err(_) => project_root().join("target"),
+    }
+}
+
 /// Returns the path to the root directory of `rust-analyzer` project.
 pub fn project_root() -> Utf8PathBuf {
     let dir = env!("CARGO_MANIFEST_DIR");