From e3125e10dcf79306c37856739fc6f89befbce71b Mon Sep 17 00:00:00 2001 From: Hong Jiarong Date: Sun, 16 Nov 2025 13:41:06 +0800 Subject: [PATCH] refactor: streamline dead code analysis by removing unused functions and optimizing import usage computation --- crates/tinymist-lint/src/dead_code.rs | 93 ++----------------- .../src/analysis/code_action.rs | 86 ++++++++++++----- 2 files changed, 73 insertions(+), 106 deletions(-) diff --git a/crates/tinymist-lint/src/dead_code.rs b/crates/tinymist-lint/src/dead_code.rs index 39e36691..f12cbe82 100644 --- a/crates/tinymist-lint/src/dead_code.rs +++ b/crates/tinymist-lint/src/dead_code.rs @@ -4,14 +4,13 @@ mod collector; mod diagnostic; use std::collections::{HashMap, HashSet}; -use std::ops::Range; use tinymist_analysis::{ adt::interner::Interned, syntax::{Decl, Expr, ExprInfo}, }; -use tinymist_project::{LspWorld, SourceWorld}; -use typst::{ecow::EcoVec, syntax::FileId}; +use tinymist_project::LspWorld; +use typst::ecow::EcoVec; use crate::DiagnosticVec; use collector::{DefInfo, DefScope, collect_definitions}; @@ -52,8 +51,7 @@ pub fn check_dead_code( return diagnostics; } - let (import_usage, shadowed_imports, module_children) = - compute_import_usage(world, &definitions, ei); + let (import_usage, shadowed_imports, module_children) = compute_import_usage(&definitions, ei); let mut seen_module_aliases = HashSet::new(); @@ -92,7 +90,6 @@ pub fn check_dead_code( } fn compute_import_usage( - world: &LspWorld, definitions: &[DefInfo], ei: &ExprInfo, ) -> ( @@ -100,29 +97,16 @@ fn compute_import_usage( HashSet>, HashMap, HashSet>>, ) { - struct ModuleSpan { - decl: Interned, - fid: FileId, - range: Range, - } - - let text = ei.source.text(); - let module_spans: Vec = definitions - .iter() - .filter_map(|def| match def.decl.as_ref() { - Decl::ModuleImport(_) => world.source_range(def.span).map(|range| ModuleSpan { - decl: def.decl.clone(), - fid: def.fid, - range, - }), - _ => None, - }) - .collect(); - let mut alias_links: HashMap, Interned> = HashMap::new(); let mut shadowed = HashSet::new(); let mut module_children: HashMap, HashSet>> = HashMap::new(); - let mut alias_item_ranges: Vec<(Range, Interned)> = Vec::new(); + + for (child, layout) in ei.module_items.iter() { + module_children + .entry(layout.parent.clone()) + .or_default() + .insert(child.clone()); + } for def in definitions { if matches!(def.decl.as_ref(), Decl::ImportAlias(_)) { @@ -133,39 +117,6 @@ fn compute_import_usage( } } } - if matches!(def.decl.as_ref(), Decl::ModuleAlias(_)) { - if let Some(alias_range) = world.source_range(def.span) { - if let Some(items_range) = alias_items_range(text, &alias_range) { - alias_item_ranges.push((items_range, def.decl.clone())); - } - } - } - } - - for def in definitions { - if matches!(def.decl.as_ref(), Decl::Import(_) | Decl::ImportAlias(_)) { - if let Some(child_range) = world.source_range(def.span) { - if let Some(module) = module_spans - .iter() - .find(|span| span.fid == def.fid && contains_range(&span.range, &child_range)) - { - module_children - .entry(module.decl.clone()) - .or_default() - .insert(def.decl.clone()); - } - - if let Some((_, alias_decl)) = alias_item_ranges - .iter() - .find(|(range, _)| range.contains(&child_range.start)) - { - module_children - .entry(alias_decl.clone()) - .or_default() - .insert(def.decl.clone()); - } - } - } } let mut used: HashSet> = HashSet::new(); @@ -192,30 +143,6 @@ fn compute_import_usage( (used, shadowed, module_children) } -fn contains_range(outer: &Range, inner: &Range) -> bool { - outer.start <= inner.start && outer.end >= inner.end -} - -fn alias_items_range(text: &str, alias_range: &Range) -> Option> { - let bytes = text.as_bytes(); - let mut idx = alias_range.end; - while idx < bytes.len() && matches!(bytes[idx], b' ' | b'\t') { - idx += 1; - } - if idx >= bytes.len() || bytes[idx] != b':' { - return None; - } - idx += 1; - while idx < bytes.len() && matches!(bytes[idx], b' ' | b'\t') { - idx += 1; - } - let mut end = idx; - while end < bytes.len() && bytes[end] != b'\n' && bytes[end] != b'\r' { - end += 1; - } - Some(idx..end) -} - fn should_skip_definition(def_info: &DefInfo, config: &DeadCodeConfig) -> bool { if matches!(def_info.scope, DefScope::Exported) && !config.check_exported { return true; diff --git a/crates/tinymist-query/src/analysis/code_action.rs b/crates/tinymist-query/src/analysis/code_action.rs index 2d0c7528..1b9970dd 100644 --- a/crates/tinymist-query/src/analysis/code_action.rs +++ b/crates/tinymist-query/src/analysis/code_action.rs @@ -4,7 +4,8 @@ use ecow::{EcoString, eco_format}; use lsp_types::{ChangeAnnotation, CreateFile, CreateFileOptions}; use regex::Regex; use tinymist_analysis::syntax::{ - PreviousItem, SyntaxClass, adjust_expr, node_ancestors, previous_items, + ExprInfo, ModuleItemLayout, PreviousItem, SyntaxClass, adjust_expr, node_ancestors, + previous_items, }; use tinymist_std::path::{diff, unix_slash}; use typst::syntax::Side; @@ -24,6 +25,8 @@ pub struct CodeActionWorker<'a> { pub actions: Vec, /// The lazily calculated local URL to [`Self::source`]. local_url: OnceLock>, + /// Cached expression information for the current source file. + expr_info: Option, } impl<'a> CodeActionWorker<'a> { @@ -34,6 +37,7 @@ impl<'a> CodeActionWorker<'a> { source, actions: Vec::new(), local_url: OnceLock::new(), + expr_info: None, } } @@ -446,29 +450,14 @@ impl<'a> CodeActionWorker<'a> { ) -> Option<()> { // Calculate the range to remove, expand to cover the whole import item // (e.g. `foo as bar`) and include trailing comma if present. - let mut remove_range = self - .module_alias_remove_range(root, name_range) - .or_else(|| self.find_import_item_range(root, name_range)) - .unwrap_or_else(|| name_range.clone()); - let bytes = self.source.text().as_bytes(); - - // Check for trailing comma after the item - let mut end = remove_range.end; - while end < bytes.len() && matches!(bytes[end], b' ' | b'\t') { - end += 1; - } - if end < bytes.len() && bytes[end] == b',' { - remove_range.end = end + 1; + let mut remove_range = if let Some(layout) = self.module_item_layout_for_range(name_range) { + layout.item_range.clone() } else { - // Check for comma before the item - let mut start = remove_range.start; - while start > 0 && matches!(bytes[start - 1], b' ' | b'\t') { - start -= 1; - } - if start > 0 && bytes[start - 1] == b',' { - remove_range.start = start - 1; - } - } + self.module_alias_remove_range(root, name_range) + .or_else(|| self.find_import_item_range(root, name_range)) + .unwrap_or_else(|| name_range.clone()) + }; + remove_range = self.expand_import_item_range(remove_range); let lsp_range = self.ctx.to_lsp_range(remove_range.clone(), &self.source); let edit = self.local_edit(EcoSnippetTextEdit::new_plain(lsp_range, EcoString::new()))?; @@ -483,6 +472,57 @@ impl<'a> CodeActionWorker<'a> { Some(()) } + fn module_item_layout_for_range( + &mut self, + binding_range: &Range, + ) -> Option { + let info = self.expr_info()?; + info.module_items + .values() + .find(|layout| layout.binding_range == *binding_range) + .cloned() + } + + fn expand_import_item_range(&self, mut range: Range) -> Range { + let bytes = self.source.text().as_bytes(); + let len = bytes.len(); + + let mut idx = range.end; + while idx < len && matches!(bytes[idx], b' ' | b'\t' | b'\r' | b'\n') { + idx += 1; + } + if idx < len && bytes[idx] == b',' { + range.end = idx + 1; + let mut tail = range.end; + while tail < len && matches!(bytes[tail], b' ' | b'\t') { + tail += 1; + } + range.end = tail; + return range; + } + + let mut idx = range.start; + while idx > 0 && matches!(bytes[idx - 1], b' ' | b'\t' | b'\r' | b'\n') { + idx -= 1; + } + if idx > 0 && bytes[idx - 1] == b',' { + range.start = idx - 1; + while range.start > 0 && matches!(bytes[range.start - 1], b' ' | b'\t') { + range.start -= 1; + } + } + + range + } + + fn expr_info(&mut self) -> Option { + if self.expr_info.is_none() { + let info = self.ctx.expr_stage(&self.source); + self.expr_info = Some(info); + } + self.expr_info.clone() + } + fn find_import_item_range( &self, root: &LinkedNode<'_>,