From 02ff61e6a702b7608dfbba3e8cdc055dba1a116a Mon Sep 17 00:00:00 2001 From: Pedro Pessoa <54516747+yzPeedro@users.noreply.github.com> Date: Sun, 8 Nov 2026 19:46:56 -0300 Subject: [PATCH] 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 | 79 ++++++++++- crates/ide-host/tests/plugin_refactoring.rs | 149 ++++++++++++++++++++ docs/plugins/indexing.md | 6 + 3 files changed, 233 insertions(+), 1 deletion(-) diff --git a/crates/ide-host/src/ops/lsp.rs b/crates/ide-host/src/ops/lsp.rs index a77cbfcfe..fb7c7b4cc 100644 --- a/crates/ide-host/src/ops/lsp.rs +++ b/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>, plugin: Vec, @@ -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> { 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> { + 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 = "\n \n\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 }; diff --git a/crates/ide-host/tests/plugin_refactoring.rs b/crates/ide-host/tests/plugin_refactoring.rs index 2b9a678e0..5ce4b539a 100644 --- a/crates/ide-host/tests/plugin_refactoring.rs +++ b/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); + impl Handler for Native { + fn on_notification(&self, _: &str, _: Value) {} + fn on_request(&self, method: &str, params: Value) -> Result { + 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::::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"); +} diff --git a/docs/plugins/indexing.md b/docs/plugins/indexing.md index a9eb60676..9231d690d 100644 --- a/docs/plugins/indexing.md +++ b/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.