fix(plugins): refuse indexed-only renames of code identifiers the language server can't rename (#2205)

When no language server answers a rename (none running, or no rename
support) and the indexed anchor under the caret is a code identifier,
prepare and rename now refuse instead of renaming only the plugin's
anchors, which left the declaration, imports and file name behind.
Anchors in strings, comments, attribute values or files without a
grammar keep their indexed rename.

Closes #2204

crates/ide-host/src/ops/lsp.rs

@@ -271,6 +271,12 @@ fn string_at(structure: &ide_syntax::StructureService, snap: &ide_syntax::Snapsh
     stringy(&at.node.kind) || (part(&at.node.kind) && at.ancestors.first().is_some_and(|parent| stringy(&parent.kind)))
 }
 
+fn identifier_at(structure: &ide_syntax::StructureService, snap: &ide_syntax::Snapshot, range: Range) -> bool {
+    let cancel = ide_core::CancellationToken::new();
+    let Ok(Some(at)) = structure.node_at(snap, range, &cancel) else { return false };
+    at.node.kind.contains("identifier") || matches!(at.node.kind.as_str(), "name" | "constant")
+}
+
 fn definition_targets(
     server: IdeResult<Vec<Location>>,
     plugin: Vec<Location>,
@@ -788,6 +794,12 @@ async fn is_client_follow_up(
 pub async fn lsp_prepare_rename(host: &Host, window: &str, doc: DocId, pos: Pos) -> IdeResult<Option<Range>> {
     if let Some(symbol) = indexed_symbol(host, window, doc, pos).await? {
         let project = host.project(window)?;
+        if let Some(refusal) = indexed_code_identifier(host, window, doc, symbol.range).await?
+            && let Err(e) = project.lsp.prepare_rename(doc, pos).await
+            && indexed_lsp_error(e)?.is_none()
+        {
+            return Err(refusal);
+        }
         return blocking(move || {
             validate_indexed_symbol(&project, &symbol, None)?;
             Ok(Some(symbol.range))
@@ -883,7 +895,14 @@ pub async fn lsp_rename(
         let mut renamed = match result {
             Ok(renamed) => renamed,
             Err(e) if symbol.is_some() && !native_owned => {
-                Renamed { incomplete: indexed_lsp_error(e)?, ..Default::default() }
+                let incomplete = indexed_lsp_error(e)?;
+                if incomplete.is_none()
+                    && let Some(symbol) = &symbol
+                    && let Some(refusal) = indexed_code_identifier(host, window, doc, symbol.range).await?
+                {
+                    return Err(refusal);
+                }
+                Renamed { incomplete, ..Default::default() }
             }
             Err(e) => return Err(e),
         };
@@ -921,6 +940,31 @@ pub async fn lsp_rename(
     result
 }
 
+async fn indexed_code_identifier(
+    host: &Host,
+    window: &str,
+    doc: DocId,
+    range: Range,
+) -> IdeResult<Option<ide_core::IdeError>> {
+    let project = host.project(window)?;
+    blocking(move || {
+        let snap = project.documents.text_snapshot(doc)?;
+        if snap.large {
+            return Ok(None);
+        }
+        let s = ide_syntax::Snapshot::doc(doc, snap.version, &snap.language, &snap.text);
+        if !identifier_at(&project.structure, &s, range) {
+            return Ok(None);
+        }
+        let language = project.languages.get(&snap.language).map_or_else(|| snap.language.to_string(), |l| l.name);
+        Ok(Some(ide_core::IdeError::invalid(format!(
+            "The {language} language server can't rename this symbol; \
+             renaming only the plugin's indexed names would leave it half renamed"
+        ))))
+    })
+    .await
+}
+
 fn merge_indexed_rename(
     renamed: &mut Renamed,
     symbol: &crate::plugins::navigation::IndexedSymbol,
@@ -1607,6 +1651,39 @@ mod tests {
         assert!(string_at(&structure, &snap, Pos::new(1, 53)), "inside \"users\"");
     }
 
+    #[test]
+    fn indexed_names_on_code_identifiers_are_told_apart_from_strings_and_comments() {
+        let syntax = std::sync::Arc::new(ide_language::Syntax::new());
+        let structure = ide_syntax::StructureService::new(std::sync::Arc::new(ide_language::SyntaxCache::new(syntax)));
+        let anchor = |text: &str, line: u32, name: &str| {
+            let col = text.lines().nth(line as usize).unwrap().find(name).unwrap() as u32;
+            Range::new(Pos::new(line, col), Pos::new(line, col + name.len() as u32))
+        };
+        let php = ide_core::LanguageId::new("php");
+        let text = "<?php\nuse App\\Models\\Order;\nclass Order {}\n// Order is a model\n\
+                    return route('users.show', Order::query());\n";
+        let snap = ide_syntax::Snapshot::text(&php, text);
+        assert!(identifier_at(&structure, &snap, anchor(text, 1, "Order")), "an imported class");
+        assert!(identifier_at(&structure, &snap, anchor(text, 2, "Order")), "a class declaration");
+        assert!(identifier_at(&structure, &snap, anchor(text, 4, "Order")), "a static call's class");
+        assert!(!identifier_at(&structure, &snap, anchor(text, 4, "users.show")), "a route name");
+        assert!(!identifier_at(&structure, &snap, anchor(text, 3, "Order")), "a comment");
+        let ruby = ide_core::LanguageId::new("ruby");
+        let text = "class Order\nend\nOrder.where(status: \"paid\")\n";
+        let snap = ide_syntax::Snapshot::text(&ruby, text);
+        assert!(identifier_at(&structure, &snap, anchor(text, 0, "Order")), "a Ruby class name");
+        assert!(identifier_at(&structure, &snap, anchor(text, 2, "Order")), "a Ruby constant reference");
+        assert!(!identifier_at(&structure, &snap, anchor(text, 2, "paid")), "a Ruby string");
+        let xml = ide_core::LanguageId::new("xml");
+        let text = "<mapper>\n  <select id=\"selectById\">select 1</select>\n</mapper>\n";
+        let snap = ide_syntax::Snapshot::text(&xml, text);
+        assert!(!identifier_at(&structure, &snap, anchor(text, 1, "selectById")), "a MyBatis statement id");
+        let idx = ide_core::LanguageId::new("idx");
+        let text = "def route\n";
+        let snap = ide_syntax::Snapshot::text(&idx, text);
+        assert!(!identifier_at(&structure, &snap, anchor(text, 0, "route")), "a language without a grammar");
+    }
+
     #[test]
     fn generated_ide_helpers_give_way_to_real_declarations() {
         let at = |path: &str| Location { path: path.into(), range: Range::default(), uri: None };

crates/ide-host/tests/plugin_refactoring.rs

@@ -213,6 +213,7 @@ async fn indexed_usages_rename_preview_staleness_and_grouped_undo() {
             .unwrap();
     }
     native_rename_refusal(&host, &project, doc.id, &first, source).await;
+    code_identifier_rename(&host, &plugins, &root, &fixture).await;
     plugins.set_enabled("test.indexer", false).unwrap();
     plugins.set_enabled("test.duplicate", false).unwrap();
     assert!(plugins.indexed_symbol_at(&project.root, &first, Pos::new(0, 4), Some("main")).is_none());
@@ -392,3 +393,151 @@ async fn native_rename_refusal(
         peer.kill();
     }
 }
+
+async fn code_identifier_rename(host: &Host, plugins: &PluginHost, root: &Path, fixture: &Path) {
+    use ide_core::{FileEdit, Range};
+    use ide_jsonrpc::{Handler, Peer};
+    use serde_json::{Value, json};
+    use std::sync::atomic::{AtomicBool, Ordering};
+    let plugin = root.join("ruby-indexer");
+    std::fs::create_dir_all(plugin.join("dist")).unwrap();
+    std::fs::copy(fixture.join("dist/plugin.wasm"), plugin.join("dist/plugin.wasm")).unwrap();
+    std::fs::write(
+        plugin.join("plugin.toml"),
+        std::fs::read_to_string(fixture.join("plugin.toml"))
+            .unwrap()
+            .replace("test.indexer", "test.ruby")
+            .replace(r#"extensions = ["idx"]"#, r#"extensions = ["rb"]"#),
+    )
+    .unwrap();
+    plugins.install_from_dir(&plugin, &Consent::all()).unwrap();
+    let app = root.join("app");
+    std::fs::create_dir_all(&app).unwrap();
+    let source = "def order\nend\nref order\nref \"orders.index\"\n";
+    let used = "ref order\nref \"orders.index\"\n";
+    let model = app.join("order.rb");
+    let usage = app.join("orders_controller.rb");
+    std::fs::write(&model, source).unwrap();
+    std::fs::write(&usage, used).unwrap();
+    let project = host.open_light("ruby", &model).unwrap();
+    let doc = project.documents.open(dunce::simplified(&model)).unwrap();
+    for (path, text, version) in [(&model, source, Some(doc.version)), (&usage, used, None)] {
+        plugins
+            .index_navigation(
+                &project.root,
+                Snapshot { path, text, version, symbols: &[], scope: "app" },
+                Invocation {
+                    client: Some("ruby".into()),
+                    workspace_root: Some(project.root.clone()),
+                    ..Default::default()
+                },
+            )
+            .unwrap();
+    }
+    let edits = |files: &[FileEdit]| {
+        let mut out: Vec<(String, u32, u32, String)> = files
+            .iter()
+            .flat_map(|file| match file {
+                FileEdit::Edit { path, edits, .. } => {
+                    let name = path.file_name().unwrap().to_string_lossy().into_owned();
+                    edits
+                        .iter()
+                        .map(|e| (name.clone(), e.range.start.line, e.range.start.col, e.new_text.clone()))
+                        .collect()
+                }
+                _ => vec![],
+            })
+            .collect();
+        out.sort();
+        out
+    };
+    let refusal = "The Ruby language server can't rename this symbol; \
+                   renaming only the plugin's indexed names would leave it half renamed";
+    let ruby = ide_core::LanguageId::new("ruby");
+    project.lsp.set_servers_hook(Box::new(|_, _| vec![]));
+    project.lsp.did_open(doc.id, &model, &ruby, 0, source);
+    for pos in [Pos::new(0, 5), Pos::new(2, 5)] {
+        let refused = ops::lsp::lsp_prepare_rename(host, "ruby", doc.id, pos).await.unwrap_err();
+        assert_eq!((refused.kind, refused.message.as_str()), (ErrorKind::InvalidInput, refusal));
+        let refused =
+            ops::lsp::lsp_rename(host, "ruby", RequestId(120), doc.id, pos, "purchase".into()).await.unwrap_err();
+        assert_eq!((refused.kind, refused.message.as_str()), (ErrorKind::InvalidInput, refusal));
+    }
+    assert_eq!(
+        ops::lsp::lsp_prepare_rename(host, "ruby", doc.id, Pos::new(3, 6)).await.unwrap(),
+        Some(Range::new(Pos::new(3, 4), Pos::new(3, 18))),
+        "a name inside a string stays the plugin's"
+    );
+    let list = "\"orders.list\"".to_owned();
+    let renamed =
+        ops::lsp::lsp_rename(host, "ruby", RequestId(121), doc.id, Pos::new(3, 6), list.clone()).await.unwrap();
+    assert!(renamed.preview);
+    assert_eq!(
+        edits(&renamed.edit.files),
+        [("order.rb".to_owned(), 3, 4, list.clone()), ("orders_controller.rb".to_owned(), 1, 4, list)]
+    );
+    project.lsp.did_close(doc.id);
+    struct Native(Arc<AtomicBool>);
+    impl Handler for Native {
+        fn on_notification(&self, _: &str, _: Value) {}
+        fn on_request(&self, method: &str, params: Value) -> Result<Value, (i64, String)> {
+            let owned = self.0.load(Ordering::SeqCst);
+            let name = json!({"start": {"line": 2, "character": 4}, "end": {"line": 2, "character": 9}});
+            Ok(match method {
+                "initialize" => {
+                    json!({"capabilities": {"textDocumentSync": 1, "renameProvider": {"prepareProvider": true}}})
+                }
+                "textDocument/prepareRename" if owned => name,
+                "textDocument/rename" if owned => {
+                    let uri = params["textDocument"]["uri"].as_str().unwrap().to_owned();
+                    json!({"changes": {uri: [{"range": name, "newText": params["newName"]}]}})
+                }
+                _ => Value::Null,
+            })
+        }
+    }
+    let owned = Arc::new(AtomicBool::new(true));
+    let listener = tokio::net::TcpListener::bind(("127.0.0.1", 0)).await.unwrap();
+    let port = listener.local_addr().unwrap().port();
+    let peers = Arc::new(Mutex::new(Vec::<Peer>::new()));
+    let accepted = peers.clone();
+    let state = owned.clone();
+    let task = tokio::spawn(async move {
+        while let Ok((stream, _)) = listener.accept().await {
+            let (reader, writer) = stream.into_split();
+            accepted.lock().push(Peer::connect(reader, writer, Arc::new(Native(state.clone()))));
+        }
+    });
+    let config: ide_language::LanguageServerConfig = toml::from_str(&format!(
+        "name = 'ruby-renamer'\ncommand = 'no-process-required'\nconnect = '127.0.0.1:{port}'"
+    ))
+    .unwrap();
+    project.lsp.set_servers_hook(Box::new(move |_, _| vec![config.clone()]));
+    project.lsp.did_open(doc.id, &model, &ruby, 0, source);
+    let everywhere: Vec<_> = [("order.rb", 0), ("order.rb", 2), ("orders_controller.rb", 0)]
+        .map(|(file, line)| (file.to_owned(), line, 4, "purchase".to_owned()))
+        .into();
+    assert_eq!(
+        ops::lsp::lsp_prepare_rename(host, "ruby", doc.id, Pos::new(2, 5)).await.unwrap(),
+        Some(Range::new(Pos::new(2, 4), Pos::new(2, 9)))
+    );
+    let renamed =
+        ops::lsp::lsp_rename(host, "ruby", RequestId(122), doc.id, Pos::new(2, 5), "purchase".into()).await.unwrap();
+    assert!(renamed.preview);
+    assert_eq!(edits(&renamed.edit.files), everywhere, "the server's rename and the indexed names merge");
+    owned.store(false, Ordering::SeqCst);
+    let renamed =
+        ops::lsp::lsp_rename(host, "ruby", RequestId(123), doc.id, Pos::new(2, 5), "purchase".into()).await.unwrap();
+    assert_eq!(edits(&renamed.edit.files), everywhere, "a server that declines the name leaves it to the plugin");
+    assert_eq!(std::fs::read_to_string(&model).unwrap(), source);
+    assert_eq!(std::fs::read_to_string(&usage).unwrap(), used);
+    project.lsp.did_close(doc.id);
+    project.lsp.shutdown_all().await;
+    project.lsp.set_servers_hook(Box::new(|_, _| vec![]));
+    task.abort();
+    let _ = task.await;
+    for peer in peers.lock().drain(..) {
+        peer.kill();
+    }
+    host.close_project("ruby");
+}

docs/plugins/indexing.md

@@ -84,6 +84,12 @@ its indexed anchors from being renamed. Failed reference or name-preparation
 queries show an incomplete-search warning. A server that owns the same name
 keeps its rename validation; a rejected native rename changes nothing.
 
+When no language server can rename there (none is running, or it has no rename
+support), an anchor on a code identifier, per the file's syntax tree, is not
+renamed: the anchors would miss the declaration, imports and file name its
+language owns. Anchors in strings, comments or attribute values, and in files
+without a syntax grammar, are still renamed from the index alone.
+
 These features consume the existing graph; no references or rename export is
 needed. Unindexed names continue through the normal language-server path.