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.