From 14c7a92944bee853a84bd0c09e70acc130ff4c1d Mon Sep 17 00:00:00 2001 From: Myriad-Dreamin <35292584+Myriad-Dreamin@users.noreply.github.com> Date: Sat, 9 Aug 2025 19:54:22 +0800 Subject: [PATCH] feat: rename labels (#1858) closes #549 --- .../tinymist-analysis/src/syntax/matcher.rs | 10 +++ .../src/fixtures/rename/label.typ | 7 ++ .../src/fixtures/rename/label_indir.typ | 5 ++ .../src/fixtures/rename/label_indir2.typ | 9 +++ .../rename/snaps/prepare@label.typ.snap | 9 +++ .../rename/snaps/prepare@label_indir.typ.snap | 9 +++ .../snaps/prepare@label_indir2.typ.snap | 9 +++ .../fixtures/rename/snaps/test@label.typ.snap | 29 ++++++++ .../rename/snaps/test@label_indir.typ.snap | 29 ++++++++ .../rename/snaps/test@label_indir2.typ.snap | 29 ++++++++ crates/tinymist-query/src/prepare_rename.rs | 56 ++++++++------ crates/tinymist-query/src/rename.rs | 73 ++++++++++++++++--- .../tinymist-query/src/will_rename_files.rs | 22 ++---- 13 files changed, 250 insertions(+), 46 deletions(-) create mode 100644 crates/tinymist-query/src/fixtures/rename/label.typ create mode 100644 crates/tinymist-query/src/fixtures/rename/label_indir.typ create mode 100644 crates/tinymist-query/src/fixtures/rename/label_indir2.typ create mode 100644 crates/tinymist-query/src/fixtures/rename/snaps/prepare@label.typ.snap create mode 100644 crates/tinymist-query/src/fixtures/rename/snaps/prepare@label_indir.typ.snap create mode 100644 crates/tinymist-query/src/fixtures/rename/snaps/prepare@label_indir2.typ.snap create mode 100644 crates/tinymist-query/src/fixtures/rename/snaps/test@label.typ.snap create mode 100644 crates/tinymist-query/src/fixtures/rename/snaps/test@label_indir.typ.snap create mode 100644 crates/tinymist-query/src/fixtures/rename/snaps/test@label_indir2.typ.snap diff --git a/crates/tinymist-analysis/src/syntax/matcher.rs b/crates/tinymist-analysis/src/syntax/matcher.rs index cf2b2852..3fc0ddb5 100644 --- a/crates/tinymist-analysis/src/syntax/matcher.rs +++ b/crates/tinymist-analysis/src/syntax/matcher.rs @@ -701,6 +701,16 @@ impl<'a> SyntaxClass<'a> { _ => None, } } + + pub fn contains_error(&self) -> bool { + use SyntaxClass::*; + match self { + Label { is_error, .. } => *is_error, + Normal(kind, _) => *kind == SyntaxKind::Error, + Callee(..) | VarAccess(..) => self.node().kind() == SyntaxKind::Error, + Ref { .. } | ImportPath(..) | IncludePath(..) => false, + } + } } /// Classifies node's syntax (inner syntax) that can be operated on by IDE diff --git a/crates/tinymist-query/src/fixtures/rename/label.typ b/crates/tinymist-query/src/fixtures/rename/label.typ new file mode 100644 index 00000000..06d9b777 --- /dev/null +++ b/crates/tinymist-query/src/fixtures/rename/label.typ @@ -0,0 +1,7 @@ +/// compile: true + +#set heading(numbering: "1.") + += Labeled + +/* position after */ @title_label diff --git a/crates/tinymist-query/src/fixtures/rename/label_indir.typ b/crates/tinymist-query/src/fixtures/rename/label_indir.typ new file mode 100644 index 00000000..a1cee56e --- /dev/null +++ b/crates/tinymist-query/src/fixtures/rename/label_indir.typ @@ -0,0 +1,5 @@ +/// compile: true + +#let test1(body) = figure(body) +#test1([Test1]) +/* position after */ @fig:test1 diff --git a/crates/tinymist-query/src/fixtures/rename/label_indir2.typ b/crates/tinymist-query/src/fixtures/rename/label_indir2.typ new file mode 100644 index 00000000..a24bc1fc --- /dev/null +++ b/crates/tinymist-query/src/fixtures/rename/label_indir2.typ @@ -0,0 +1,9 @@ +/// compile: true + +#let test1(body) = figure(body) +#test1([Test1]) +@fig:test1 + +#let test2(body) = test1(body) +#test2([Test2]) +/* position after */ @fig:test2 diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/prepare@label.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/prepare@label.typ.snap new file mode 100644 index 00000000..8ec35549 --- /dev/null +++ b/crates/tinymist-query/src/fixtures/rename/snaps/prepare@label.typ.snap @@ -0,0 +1,9 @@ +--- +source: crates/tinymist-query/src/prepare_rename.rs +expression: "JsonRepr::new_redacted(result, &REDACT_LOC)" +input_file: crates/tinymist-query/src/fixtures/rename/label.typ +--- +{ + "placeholder": "title_label", + "range": "6:21:6:33" +} diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/prepare@label_indir.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/prepare@label_indir.typ.snap new file mode 100644 index 00000000..b136f34d --- /dev/null +++ b/crates/tinymist-query/src/fixtures/rename/snaps/prepare@label_indir.typ.snap @@ -0,0 +1,9 @@ +--- +source: crates/tinymist-query/src/prepare_rename.rs +expression: "JsonRepr::new_redacted(result, &REDACT_LOC)" +input_file: crates/tinymist-query/src/fixtures/rename/label_indir.typ +--- +{ + "placeholder": "fig:test1", + "range": "4:21:4:31" +} diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/prepare@label_indir2.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/prepare@label_indir2.typ.snap new file mode 100644 index 00000000..3d9d0467 --- /dev/null +++ b/crates/tinymist-query/src/fixtures/rename/snaps/prepare@label_indir2.typ.snap @@ -0,0 +1,9 @@ +--- +source: crates/tinymist-query/src/prepare_rename.rs +expression: "JsonRepr::new_redacted(result, &REDACT_LOC)" +input_file: crates/tinymist-query/src/fixtures/rename/label_indir2.typ +--- +{ + "placeholder": "fig:test2", + "range": "8:21:8:31" +} diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/test@label.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/test@label.typ.snap new file mode 100644 index 00000000..a1e6fdbc --- /dev/null +++ b/crates/tinymist-query/src/fixtures/rename/snaps/test@label.typ.snap @@ -0,0 +1,29 @@ +--- +source: crates/tinymist-query/src/rename.rs +expression: "JsonRepr::new_redacted(result, &REDACT_LOC)" +input_file: crates/tinymist-query/src/fixtures/rename/label.typ +--- +{ + "changeAnnotations": { + "Typst Rename Labels": { + "description": "The language server searched the labels ambiguously", + "label": "Typst Rename Labels", + "needsConfirmation": true + } + }, + "documentChanges": [ + { + "edits": [ + { + "annotationId": "Typst Rename Labels", + "newText": "new_name", + "range": "6:21:6:33" + } + ], + "textDocument": { + "uri": "s0.typ", + "version": null + } + } + ] +} diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/test@label_indir.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/test@label_indir.typ.snap new file mode 100644 index 00000000..2e7fbac4 --- /dev/null +++ b/crates/tinymist-query/src/fixtures/rename/snaps/test@label_indir.typ.snap @@ -0,0 +1,29 @@ +--- +source: crates/tinymist-query/src/rename.rs +expression: "JsonRepr::new_redacted(result, &REDACT_LOC)" +input_file: crates/tinymist-query/src/fixtures/rename/label_indir.typ +--- +{ + "changeAnnotations": { + "Typst Rename Labels": { + "description": "The language server searched the labels ambiguously", + "label": "Typst Rename Labels", + "needsConfirmation": true + } + }, + "documentChanges": [ + { + "edits": [ + { + "annotationId": "Typst Rename Labels", + "newText": "new_name", + "range": "4:21:4:31" + } + ], + "textDocument": { + "uri": "s0.typ", + "version": null + } + } + ] +} diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/test@label_indir2.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/test@label_indir2.typ.snap new file mode 100644 index 00000000..ab5d21db --- /dev/null +++ b/crates/tinymist-query/src/fixtures/rename/snaps/test@label_indir2.typ.snap @@ -0,0 +1,29 @@ +--- +source: crates/tinymist-query/src/rename.rs +expression: "JsonRepr::new_redacted(result, &REDACT_LOC)" +input_file: crates/tinymist-query/src/fixtures/rename/label_indir2.typ +--- +{ + "changeAnnotations": { + "Typst Rename Labels": { + "description": "The language server searched the labels ambiguously", + "label": "Typst Rename Labels", + "needsConfirmation": true + } + }, + "documentChanges": [ + { + "edits": [ + { + "annotationId": "Typst Rename Labels", + "newText": "new_name", + "range": "8:21:8:31" + } + ], + "textDocument": { + "uri": "s0.typ", + "version": null + } + } + ] +} diff --git a/crates/tinymist-query/src/prepare_rename.rs b/crates/tinymist-query/src/prepare_rename.rs index e32b1c93..0fd483bc 100644 --- a/crates/tinymist-query/src/prepare_rename.rs +++ b/crates/tinymist-query/src/prepare_rename.rs @@ -38,42 +38,53 @@ impl StatefulRequest for PrepareRenameRequest { let doc = graph.snap.success_doc.as_ref(); let source = ctx.source_by_path(&self.path).ok()?; let syntax = ctx.classify_for_decl(&source, self.position)?; - if matches!(syntax.node().kind(), SyntaxKind::FieldAccess) { - // todo: rename field access - log::info!("prepare_rename: field access is not a definition site"); + if bad_syntax(&syntax) { return None; } let origin_selection_range = ctx.to_lsp_range(syntax.node().range(), &source); let def = ctx.def_of_syntax(&source, doc, syntax.clone())?; - let (name, range) = prepare_renaming(&syntax, &def)?; + let name = prepare_renaming(&syntax, &def)?; Some(PrepareRenameResponse::RangeWithPlaceholder { - range: range.unwrap_or(origin_selection_range), + range: origin_selection_range, placeholder: name, }) } } -pub(crate) fn prepare_renaming( - deref_target: &SyntaxClass, - def: &Definition, -) -> Option<(String, Option)> { - let name = def.name().clone(); +fn bad_syntax(syntax: &SyntaxClass) -> bool { + if matches!(syntax.node().kind(), SyntaxKind::FieldAccess) { + // todo: rename field access + log::info!("prepare_rename: field access is not a definition site"); + return true; + } + + if syntax.contains_error() { + return true; + } + + false +} + +pub(crate) fn prepare_renaming(syntax: &SyntaxClass, def: &Definition) -> Option { + if bad_syntax(syntax) { + return None; + } + let def_fid = def.file_id()?; if WorkspaceResolver::is_package_file(def_fid) { crate::log_debug_ct!( - "prepare_rename: {name} is in a package {pkg:?}", + "prepare_rename: is in a package {pkg:?}, def: {def:?}", pkg = def_fid.package(), ); return None; } - let var_rename = || Some((name.to_string(), None)); + let decl_name = || def.name().clone().to_string(); - crate::log_debug_ct!("prepare_rename: {name}"); use Decl::*; match def.decl.as_ref() { // Cannot rename headings or blocks @@ -82,17 +93,17 @@ pub(crate) fn prepare_renaming( // LexicalKind::Mod(Star) => None, // Cannot rename expression import // LexicalKind::Mod(Module(ModSrc::Expr(..))) => None, - Var(..) => var_rename(), - Func(..) | Closure(..) => validate_fn_renaming(def).map(|_| (name.to_string(), None)), + Var(..) | Label(..) | ContentRef(..) => Some(decl_name()), + Func(..) | Closure(..) => validate_fn_renaming(def).map(|_| decl_name()), Module(..) | ModuleAlias(..) | PathStem(..) | ImportPath(..) | IncludePath(..) | ModuleImport(..) => { - let node = deref_target.node().get().clone(); + let node = syntax.node().get().clone(); let path = node.cast::()?; let name = path.get().to_string(); - Some((name, None)) + Some(name) } - // todo: label renaming, bibkey renaming - BibEntry(..) | Label(..) | ContentRef(..) => None, + // todo: bibkey renaming + BibEntry(..) => None, ImportAlias(..) | Constant(..) | IdentRef(..) | Import(..) | StrName(..) | Spread(..) => { None } @@ -135,13 +146,16 @@ mod tests { snapshot_testing("rename", &|ctx, path| { let source = ctx.source_by_path(&path).unwrap(); + let docs = find_module_level_docs(&source).unwrap_or_default(); + let properties = get_test_properties(&docs); + let graph = compile_doc_for_test(ctx, &properties); + let request = PrepareRenameRequest { path: path.clone(), position: find_test_position(&source), }; - let snap = WorldComputeGraph::from_world(ctx.world.clone()); - let result = request.request(ctx, snap); + let result = request.request(ctx, graph); assert_snapshot!(JsonRepr::new_redacted(result, &REDACT_LOC)); }); } diff --git a/crates/tinymist-query/src/rename.rs b/crates/tinymist-query/src/rename.rs index a7352c0b..6b45f503 100644 --- a/crates/tinymist-query/src/rename.rs +++ b/crates/tinymist-query/src/rename.rs @@ -1,6 +1,6 @@ use lsp_types::{ - DocumentChangeOperation, DocumentChanges, OneOf, OptionalVersionedTextDocumentIdentifier, - RenameFile, TextDocumentEdit, + AnnotatedTextEdit, ChangeAnnotation, DocumentChangeOperation, DocumentChanges, OneOf, + OptionalVersionedTextDocumentIdentifier, RenameFile, TextDocumentEdit, }; use rustc_hash::FxHashSet; use tinymist_std::path::{unix_slash, PathClean}; @@ -76,7 +76,7 @@ impl StatefulRequest for RenameRequest { let mut edits: HashMap> = HashMap::new(); do_rename_file(ctx, def_fid, diff, &mut edits); - let mut document_changes = edits_to_document_changes(edits); + let mut document_changes = edits_to_document_changes(edits, None); document_changes.push(lsp_types::DocumentChangeOperation::Op( lsp_types::ResourceOp::Rename(RenameFile { @@ -94,6 +94,8 @@ impl StatefulRequest for RenameRequest { }) } _ => { + let is_label = matches!(def.decl.kind(), DefKind::Reference); + let references = find_references(ctx, &source, doc, syntax)?; let mut edits = HashMap::new(); @@ -108,12 +110,30 @@ impl StatefulRequest for RenameRequest { }); } - log::info!("rename edits: {edits:?}"); + crate::log_debug_ct!("rename edits: {edits:?}"); - Some(WorkspaceEdit { - changes: Some(edits), - ..Default::default() - }) + if !is_label { + Some(WorkspaceEdit { + changes: Some(edits), + ..Default::default() + }) + } else { + let change_id = "Typst Rename Labels"; + + let document_changes = edits_to_document_changes(edits, Some(change_id)); + + let change_annotations = Some(create_change_annotation( + change_id, + true, + Some("The language server searched the labels ambiguously".to_string()), + )); + + Some(WorkspaceEdit { + document_changes: Some(DocumentChanges::Operations(document_changes)), + change_annotations, + ..Default::default() + }) + } } } } @@ -300,19 +320,47 @@ impl RenameFileWorker<'_> { pub(crate) fn edits_to_document_changes( edits: HashMap>, + change_id: Option<&str>, ) -> Vec { let mut document_changes = vec![]; for (uri, edits) in edits { document_changes.push(lsp_types::DocumentChangeOperation::Edit(TextDocumentEdit { text_document: OptionalVersionedTextDocumentIdentifier { uri, version: None }, - edits: edits.into_iter().map(OneOf::Left).collect(), + edits: edits + .into_iter() + .map(|edit| match change_id { + Some(change_id) => OneOf::Right(AnnotatedTextEdit { + text_edit: edit, + annotation_id: change_id.to_owned(), + }), + None => OneOf::Left(edit), + }) + .collect(), })); } document_changes } +pub(crate) fn create_change_annotation( + label: &str, + needs_confirmation: bool, + description: Option, +) -> HashMap { + let mut change_annotations = HashMap::new(); + change_annotations.insert( + label.to_owned(), + ChangeAnnotation { + label: label.to_owned(), + needs_confirmation: Some(needs_confirmation), + description, + }, + ); + + change_annotations +} + #[cfg(test)] mod tests { use super::*; @@ -323,14 +371,17 @@ mod tests { snapshot_testing("rename", &|ctx, path| { let source = ctx.source_by_path(&path).unwrap(); + let docs = find_module_level_docs(&source).unwrap_or_default(); + let properties = get_test_properties(&docs); + let graph = compile_doc_for_test(ctx, &properties); + let request = RenameRequest { path: path.clone(), position: find_test_position(&source), new_name: "new_name".to_string(), }; - let snap = WorldComputeGraph::from_world(ctx.world.clone()); - let mut result = request.request(ctx, snap); + let mut result = request.request(ctx, graph); // sort the edits to make the snapshot stable if let Some(r) = result.as_mut().and_then(|r| r.changes.as_mut()) { for edits in r.values_mut() { diff --git a/crates/tinymist-query/src/will_rename_files.rs b/crates/tinymist-query/src/will_rename_files.rs index 365cd057..ba692d42 100644 --- a/crates/tinymist-query/src/will_rename_files.rs +++ b/crates/tinymist-query/src/will_rename_files.rs @@ -1,6 +1,4 @@ -use lsp_types::ChangeAnnotation; - -use crate::{do_rename_file, edits_to_document_changes, prelude::*}; +use crate::{create_change_annotation, do_rename_file, edits_to_document_changes, prelude::*}; /// Handle [`workspace/willRenameFiles`] request is sent from the client to the /// server. @@ -37,25 +35,21 @@ impl StatefulRequest for WillRenameFilesRequest { }) .collect::>>()?; log::info!("did rename edits: {edits:?}"); - let document_changes = edits_to_document_changes(edits); + let document_changes = edits_to_document_changes(edits, None); if document_changes.is_empty() { return None; } - let mut change_annotations = HashMap::new(); - change_annotations.insert( - "Typst Rename Files".to_string(), - ChangeAnnotation { - label: "Typst Rename Files".to_string(), - needs_confirmation: Some(true), - description: Some("Rename files should update imports".to_string()), - }, - ); + let change_annotations = Some(create_change_annotation( + "Typst Rename Files", + true, + Some("Renaming files should update imports".to_string()), + )); Some(WorkspaceEdit { changes: None, document_changes: Some(lsp_types::DocumentChanges::Operations(document_changes)), - change_annotations: Some(change_annotations), + change_annotations, }) } }