From 2a7757468b21499d8f8a8bab6e0ec833a9c8423b Mon Sep 17 00:00:00 2001 From: Vu Vo Date: Mon, 3 Aug 2026 12:58:47 +0700 Subject: [PATCH 1/5] refacor and split main/lib in lsp Signed-off-by: Vu Vo --- crates/lsp/src/file_db.rs | 5 +- crates/lsp/src/global_state.rs | 60 +++++----- crates/lsp/src/handler/completion.rs | 7 +- crates/lsp/src/handler/goto_definition.rs | 8 +- crates/lsp/src/handler/hover.rs | 39 +----- crates/lsp/src/handler/references.rs | 8 +- crates/lsp/src/handler/rename.rs | 56 ++------- crates/lsp/src/lib.rs | 138 ++++++++++++++++++++++ crates/lsp/src/main.rs | 132 +-------------------- crates/lsp/src/resolver.rs | 28 +++-- crates/lsp/src/test_util.rs | 57 +++++++++ 11 files changed, 265 insertions(+), 273 deletions(-) create mode 100644 crates/lsp/src/lib.rs create mode 100644 crates/lsp/src/test_util.rs diff --git a/crates/lsp/src/file_db.rs b/crates/lsp/src/file_db.rs index 45ae289..f8c5629 100644 --- a/crates/lsp/src/file_db.rs +++ b/crates/lsp/src/file_db.rs @@ -76,9 +76,10 @@ impl FileDB { /// UTF-16 units from the line start. pub fn position(&self, offset: TextSize) -> Position { let offset = u32::from(offset) as usize; + // `binary_search` returns the index both on hit (offset is exactly a `\n`) and on miss + // (insertion point) — for line numbering both yield the same line, so collapse the arms. let line = match self.newline_offsets.binary_search(&(offset as u32)) { - Ok(l) => l, - Err(l) => l, + Ok(line) | Err(line) => line, }; let line_start = self.line_start_byte(line as u32); // `line_start` and `offset` are both char boundaries (rowan token offsets always are, and diff --git a/crates/lsp/src/global_state.rs b/crates/lsp/src/global_state.rs index ac5bba9..650325d 100644 --- a/crates/lsp/src/global_state.rs +++ b/crates/lsp/src/global_state.rs @@ -5,7 +5,7 @@ use lsp_types::request::{ Completion, DocumentSymbolRequest, Formatting, GotoDefinition, HoverRequest, PrepareRenameRequest, References, Rename, Request as _, }; -use lsp_types::{DidChangeTextDocumentParams, DidOpenTextDocumentParams, Location, Url}; +use lsp_types::{DidChangeTextDocumentParams, DidOpenTextDocumentParams, Location, Range, Url}; use parser::token_kind::TokenKind; use rowan::ast::AstNode; use rowan::TextSize; @@ -141,18 +141,29 @@ impl GlobalState { } /// Goto-definition shaper: an include-path string routes to [`jump_to_lib`]; any other token - /// resolves via [`Self::resolve_use`] (possibly cross-file), file-tagged. + /// resolves via [`Self::resolve_token`], file-tagged. pub fn lookup_definition(&self, file_db: &FileDB, token: &SyntaxToken) -> Vec { if token.kind() == TokenKind::CircomString { return jump_to_lib(file_db, token, self.source_db.vfs()); } - // A component member-access field (`c.x` / `T()(...).x`) resolves via type inference - // (`resolve_member`), not the flat name resolver (which returns empty for fields by design). - let resolved = if resolver::component_field(token).is_some() { - self.resolve_member(file_db, token) + self.to_locations(self.resolve_token(file_db, token)) + } + + /// Resolve `token`: a component field (`c.x`) via [`Self::resolve_member`]; else [`Self::resolve_use`]. + pub(crate) fn resolve_token( + &self, + origin: &FileDB, + token: &SyntaxToken, + ) -> Vec<(FileId, ResolvedSymbol)> { + if resolver::component_field(token).is_some() { + self.resolve_member(origin, token) } else { - self.resolve_use(file_db, token) - }; + self.resolve_use(origin, token) + } + } + + /// File-tagged resolved declarations → file-tagged LSP [`Location`]s (at the name token). + fn to_locations(&self, resolved: Vec<(FileId, ResolvedSymbol)>) -> Vec { resolved .into_iter() .map(|(id, s)| Location::new(self.source_db.file_db(id).file_path.clone(), s.def_range)) @@ -204,15 +215,7 @@ impl GlobalState { for lib_id in self.loaded_includes(origin) { let lib_table = self.source_db.symbol_table(lib_id); for sym in lib_table.lookup_top_level(name) { - out.push(( - lib_id, - ResolvedSymbol { - kind: sym.kind, - name: sym.name.clone(), - def_range: sym.def_range, - decl_range: sym.decl_range, - }, - )); + out.push((lib_id, sym.into())); } } } @@ -261,17 +264,7 @@ impl GlobalState { .members_of(&template_name) .into_iter() .filter(|s| s.name == field_name) - .map(|s| { - ( - template_file, - ResolvedSymbol { - kind: s.kind, - name: s.name.clone(), - def_range: s.def_range, - decl_range: s.decl_range, - }, - ) - }) + .map(|s| (template_file, s.into())) .collect() } @@ -313,6 +306,17 @@ impl GlobalState { resolver::occurrences_in(ast.syntax(), &table, sym) } + /// Defining-file URL + every occurrence range of `target` (shared by references and rename). + pub(crate) fn occurrence_ranges(&self, target: &(FileId, ResolvedSymbol)) -> (Url, Vec) { + let def_file_db = self.source_db.file_db(target.0); + let ranges = self + .find_occurrences(target) + .into_iter() + .map(|t| def_file_db.token_range(&t)) + .collect(); + (def_file_db.file_path.clone(), ranges) + } + /// Register an updated document: set its text (dropping derived caches) and load each `include` /// once. No eager index — the symbol table builds lazily and invalidates on edit; a no-op /// (identical text) short-circuits; non-`file:` URIs and unreadable includes are skipped, not diff --git a/crates/lsp/src/handler/completion.rs b/crates/lsp/src/handler/completion.rs index 8aeb8cd..ac9c304 100644 --- a/crates/lsp/src/handler/completion.rs +++ b/crates/lsp/src/handler/completion.rs @@ -182,18 +182,13 @@ mod tests { use crate::file_db::{FileDB, FileId}; use crate::global_state::GlobalState; + use crate::test_util::state_with; use super::handle; use lsp_types::{ CompletionParams, CompletionResponse, TextDocumentIdentifier, TextDocumentPositionParams, }; - fn state_with(url: &Url, source: &str) -> GlobalState { - let mut state = GlobalState::new(Vec::new()); - state.source_db.set_document(url, source.to_string()); - state - } - /// Position at the byte offset just past the last occurrence of `needle`. fn position_after_last(source: &str, needle: &str) -> Position { let file = FileDB::new(FileId(0), source, Url::from_file_path("/tmp/x").unwrap()); diff --git a/crates/lsp/src/handler/goto_definition.rs b/crates/lsp/src/handler/goto_definition.rs index 5161df1..63124be 100644 --- a/crates/lsp/src/handler/goto_definition.rs +++ b/crates/lsp/src/handler/goto_definition.rs @@ -86,6 +86,7 @@ mod tests { use crate::file_db::FileDB; use crate::global_state::GlobalState; use crate::source_db::SourceDatabase; + use crate::test_util::state_with; use parser::token_kind::TokenKind; use super::token_at_offset; @@ -148,13 +149,6 @@ mod tests { assert_eq!("/hello", parent); } - /// A `GlobalState` seeded with one open document (no workspace roots — in-file only). - fn state_with(url: &Url, source: &str) -> GlobalState { - let mut state = GlobalState::new(Vec::new()); - state.source_db.set_document(url, source.to_string()); - state - } - /// `lookup_definition` for the `occurrence`-th `Identifier` token named `name`, using the db's /// own `FileDB` (so `origin.file_id` matches the interned id `resolve_use` queries). fn jump( diff --git a/crates/lsp/src/handler/hover.rs b/crates/lsp/src/handler/hover.rs index 104d884..09c3a5f 100644 --- a/crates/lsp/src/handler/hover.rs +++ b/crates/lsp/src/handler/hover.rs @@ -10,7 +10,7 @@ use anyhow::Result; use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind}; use crate::global_state::GlobalState; -use crate::resolver::{component_field, identifier_at, SymbolKind}; +use crate::resolver::{identifier_at, SymbolKind}; use crate::source_db::SourceDatabase; /// Entry point for the `textDocument/hover` request. Returns `None` (no hover) when the cursor @@ -26,12 +26,8 @@ pub fn handle(state: &GlobalState, params: HoverParams) -> Result> return Ok(None); }; // A component member-access field (`c.x` / `T()(...).x`) resolves via type inference; everything - // else uses the flat name resolver. - let resolved = if component_field(&token).is_some() { - state.resolve_member(&ctx.file_db, &token) - } else { - state.resolve_use(&ctx.file_db, &token) - }; + // else uses the flat name resolver. `resolve_token` picks the path. + let resolved = state.resolve_token(&ctx.file_db, &token); let Some((def_id, sym)) = resolved.into_iter().next() else { return Ok(None); }; @@ -81,41 +77,14 @@ fn kind_label(kind: SymbolKind) -> &'static str { mod tests { use lsp_types::{Position, Url}; - use crate::file_db::{FileDB, FileId}; use crate::global_state::GlobalState; - use parser::token_kind::TokenKind; - use syntax::tree::syntax_tree; + use crate::test_util::{position_of, state_with}; use super::handle; use lsp_types::{ HoverContents, HoverParams, MarkupKind, TextDocumentIdentifier, TextDocumentPositionParams, }; - fn state_with(url: &Url, source: &str) -> GlobalState { - let mut state = GlobalState::new(Vec::new()); - state.source_db.set_document(url, source.to_string()); - state - } - - /// LSP `Position` of the `occurrence`-th (0-indexed) `Identifier` token whose text is `name`. - fn position_of(source: &str, name: &str, occurrence: usize) -> Position { - let file = FileDB::new(FileId(0), source, Url::from_file_path("/tmp/x").unwrap()); - let node = syntax_tree(source); - let mut count = 0; - for t in node - .descendants_with_tokens() - .filter_map(|e| e.into_token()) - { - if t.kind() == TokenKind::Identifier && t.text() == name { - if count == occurrence { - return file.position(t.text_range().start()); - } - count += 1; - } - } - panic!("token {name}#{occurrence} not found"); - } - /// The hover markdown value for the cursor's position, or `None`. fn hover_value(state: &GlobalState, url: &Url, position: Position) -> Option { handle( diff --git a/crates/lsp/src/handler/references.rs b/crates/lsp/src/handler/references.rs index 02713db..4a6636b 100644 --- a/crates/lsp/src/handler/references.rs +++ b/crates/lsp/src/handler/references.rs @@ -9,7 +9,6 @@ use lsp_types::{Location, ReferenceParams}; use crate::global_state::GlobalState; use crate::resolver::identifier_at; -use crate::source_db::SourceDatabase; /// Entry point for the `textDocument/references` request. Returns the declaration plus every /// in-scope use as `Location`s (file-tagged), or `None` if the cursor isn't on a referenceable @@ -30,11 +29,10 @@ pub fn handle(state: &GlobalState, params: ReferenceParams) -> Result = state - .find_occurrences(&target) + let (file_uri, ranges) = state.occurrence_ranges(&target); + let locations: Vec = ranges .into_iter() - .map(|t| Location::new(def_file_db.file_path.clone(), def_file_db.token_range(&t))) + .map(|r| Location::new(file_uri.clone(), r)) .collect(); Ok(Some(locations)) } diff --git a/crates/lsp/src/handler/rename.rs b/crates/lsp/src/handler/rename.rs index 2171bf9..91a1b2e 100644 --- a/crates/lsp/src/handler/rename.rs +++ b/crates/lsp/src/handler/rename.rs @@ -17,7 +17,6 @@ use parser::token_kind::TokenKind; use crate::file_db::FileId; use crate::global_state::{CursorContext, GlobalState}; use crate::resolver::{identifier_at, ResolvedSymbol}; -use crate::source_db::SourceDatabase; use syntax::node::SyntaxToken; /// Entry point for `textDocument/rename`. Returns `None` (no edits) when the cursor isn't on a @@ -37,18 +36,17 @@ pub fn handle(state: &GlobalState, params: RenameParams) -> Result = state - .find_occurrences(&target) + // follow-up). One edit per occurrence range. + let (file_uri, ranges) = state.occurrence_ranges(&target); + let edits: Vec = ranges .into_iter() - .map(|t| TextEdit { - range: def_file_db.token_range(&t), + .map(|range| TextEdit { + range, new_text: new_name.clone(), }) .collect(); - let changes = HashMap::from([(def_file_db.file_path.clone(), edits)]); + let changes = HashMap::from([(file_uri, edits)]); Ok(Some(WorkspaceEdit { changes: Some(changes), document_changes: None, @@ -113,34 +111,10 @@ mod tests { use crate::file_db::{FileDB, FileId}; use crate::global_state::GlobalState; + use crate::test_util::{position_of, position_of_token, state_with}; use super::handle; - fn state_with(url: &Url, source: &str) -> GlobalState { - let mut state = GlobalState::new(Vec::new()); - state.source_db.set_document(url, source.to_string()); - state - } - - /// LSP `Position` of the `occurrence`-th (0-indexed) `Identifier` token whose text is `name`. - fn position_of(source: &str, name: &str, occurrence: usize) -> Position { - let file = FileDB::new(FileId(0), source, Url::from_file_path("/tmp/x").unwrap()); - let node = syntax_tree(source); - let mut count = 0; - for t in node - .descendants_with_tokens() - .filter_map(|e| e.into_token()) - { - if t.kind() == TokenKind::Identifier && t.text() == name { - if count == occurrence { - return file.position(t.text_range().start()); - } - count += 1; - } - } - panic!("token {name}#{occurrence} not found"); - } - fn rename( state: &GlobalState, url: &Url, @@ -257,22 +231,6 @@ mod tests { } } - /// LSP `Position` of the first token of *any* kind whose text equals `text` (for keyword / - /// include-string cursors, which `position_of` skips because it only matches identifiers). - fn position_of_token(source: &str, text: &str) -> Position { - let file = FileDB::new(FileId(0), source, Url::from_file_path("/tmp/x").unwrap()); - let node = syntax_tree(source); - for t in node - .descendants_with_tokens() - .filter_map(|e| e.into_token()) - { - if t.text() == text { - return file.position(t.text_range().start()); - } - } - panic!("token {text:?} not found"); - } - /// A cursor on a keyword or an include-path string yields no edits (only identifiers rename). #[test] fn rename_refuses_non_identifier_cursor_test() { diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs new file mode 100644 index 0000000..d3dbed5 --- /dev/null +++ b/crates/lsp/src/lib.rs @@ -0,0 +1,138 @@ +//! ccls — a Circom language server (library core). The `main.rs` binary is a thin [`run`] wrapper. + +pub mod file_db; +pub mod global_state; +pub mod handler; +pub mod resolver; +pub mod source_db; +pub mod symbol_table; + +#[cfg(test)] +mod test_util; + +use std::error::Error; +use std::path::PathBuf; + +use lsp_server::{Connection, Message}; +use lsp_types::{ + CompletionOptions, HoverProviderCapability, InitializeParams, OneOf, ServerCapabilities, + TextDocumentSyncCapability, TextDocumentSyncKind, +}; + +use crate::global_state::GlobalState; + +/// Run the language server over stdio (logging to stderr; stdout is the LSP channel). +pub fn run() -> Result<(), Box> { + eprintln!("starting ccls (circom language server)"); + + let (connection, io_threads) = Connection::stdio(); + + let server_capabilities = + serde_json::to_value(server_capabilities()).expect("ServerCapabilities is serializable"); + let initialization_params = match connection.initialize(server_capabilities) { + Ok(it) => it, + Err(e) => { + if e.channel_is_disconnected() { + io_threads.join()?; + } + return Err(e.into()); + } + }; + main_loop(connection, initialization_params)?; + io_threads.join()?; + + eprintln!("shutting down server"); + Ok(()) +} + +/// Advertise the LSP features this server handles. +/// +/// `definition` is fully implemented; `hover`/`completion`/`references`/`documentSymbol`/ +/// `formatting` are registered as placeholders — the client routes them to the server, which +/// currently returns an empty result until each is implemented in `handler::*`. `rename` is fully +/// implemented and advertises `prepareSupport` so the client consults the server (not its own +/// textual word check) before opening the rename box — keywords/strings never become renamable. +fn server_capabilities() -> ServerCapabilities { + ServerCapabilities { + text_document_sync: Some(TextDocumentSyncCapability::Kind(TextDocumentSyncKind::FULL)), + definition_provider: Some(OneOf::Left(true)), + hover_provider: Some(HoverProviderCapability::Simple(true)), + completion_provider: Some(CompletionOptions { + trigger_characters: Some(vec![".".to_string()]), + ..Default::default() + }), + references_provider: Some(OneOf::Left(true)), + document_symbol_provider: Some(OneOf::Left(true)), + document_formatting_provider: Some(OneOf::Left(true)), + rename_provider: Some(OneOf::Right(lsp_types::RenameOptions { + prepare_provider: Some(true), + work_done_progress_options: Default::default(), + })), + ..Default::default() + } +} + +/// Receive messages over the LSP transport and route them to `GlobalState`. Requests are answered +/// with a `Response`; notifications mutate state; responses from the client are ignored. +fn main_loop( + connection: Connection, + params: serde_json::Value, +) -> Result<(), Box> { + let params: InitializeParams = serde_json::from_value(params)?; + + // Capture workspace roots so `include` resolution can be confined to them (path-traversal + // defense). Without roots the server refuses to load any include rather than read arbitrarily. + let mut state = GlobalState::new(workspace_roots(¶ms)); + + for msg in &connection.receiver { + match msg { + Message::Request(req) => { + // The `shutdown` request is handled by the transport itself. + if connection.handle_shutdown(&req)? { + return Ok(()); + } + if let Some(resp) = state.handle_request(req)? { + connection.sender.send(Message::Response(resp))?; + } + } + Message::Response(_) => {} + Message::Notification(not) => { + state.handle_notification(not)?; + } + } + } + Ok(()) +} + +/// Workspace root folders from the `initialize` handshake, in priority order: modern clients send +/// `workspace_folders`; older clients send a single `root_uri`. Each is converted from its `file:` +/// URI to a path and canonicalized (so the Vfs's pure containment check compares canonical vs +/// canonical). Non-`file:` roots and roots that can't be canonicalized are dropped. +fn workspace_roots(params: &InitializeParams) -> Vec { + let raw: Vec = if let Some(folders) = ¶ms.workspace_folders { + let roots: Vec = folders + .iter() + .filter_map(|f| f.uri.to_file_path().ok()) + .collect(); + if !roots.is_empty() { + roots + } else { + params + .root_uri + .as_ref() + .and_then(|uri| uri.to_file_path().ok()) + .map(|root| vec![root]) + .unwrap_or_default() + } + } else { + params + .root_uri + .as_ref() + .and_then(|uri| uri.to_file_path().ok()) + .map(|root| vec![root]) + .unwrap_or_default() + }; + raw.into_iter() + .filter_map(|r| r.canonicalize().ok()) + .collect() +} diff --git a/crates/lsp/src/main.rs b/crates/lsp/src/main.rs index 88671e2..471e5eb 100644 --- a/crates/lsp/src/main.rs +++ b/crates/lsp/src/main.rs @@ -1,133 +1,7 @@ -use std::error::Error; -use std::path::PathBuf; - -use lsp_server::{Connection, Message}; -use lsp_types::{ - CompletionOptions, HoverProviderCapability, InitializeParams, OneOf, ServerCapabilities, - TextDocumentSyncCapability, TextDocumentSyncKind, -}; +//! ccls binary entry point — delegates to the `ccls` library. -use crate::global_state::GlobalState; - -pub mod file_db; -pub mod global_state; -pub mod handler; -pub mod resolver; -pub mod source_db; -pub mod symbol_table; +use std::error::Error; fn main() -> Result<(), Box> { - // All logging must go to stderr — stdout is the LSP message channel. - eprintln!("starting ccls (circom language server)"); - - let (connection, io_threads) = Connection::stdio(); - - let server_capabilities = - serde_json::to_value(server_capabilities()).expect("ServerCapabilities is serializable"); - let initialization_params = match connection.initialize(server_capabilities) { - Ok(it) => it, - Err(e) => { - if e.channel_is_disconnected() { - io_threads.join()?; - } - return Err(e.into()); - } - }; - main_loop(connection, initialization_params)?; - io_threads.join()?; - - eprintln!("shutting down server"); - Ok(()) -} - -/// Advertise the LSP features this server handles. -/// -/// `definition` is fully implemented; `hover`/`completion`/`references`/`documentSymbol`/ -/// `formatting` are registered as placeholders — the client routes them to the server, which -/// currently returns an empty result until each is implemented in `handler::*`. `rename` is fully -/// implemented and advertises `prepareSupport` so the client consults the server (not its own -/// textual word check) before opening the rename box — keywords/strings never become renamable. -fn server_capabilities() -> ServerCapabilities { - ServerCapabilities { - text_document_sync: Some(TextDocumentSyncCapability::Kind(TextDocumentSyncKind::FULL)), - definition_provider: Some(OneOf::Left(true)), - hover_provider: Some(HoverProviderCapability::Simple(true)), - completion_provider: Some(CompletionOptions { - trigger_characters: Some(vec![".".to_string()]), - ..Default::default() - }), - references_provider: Some(OneOf::Left(true)), - document_symbol_provider: Some(OneOf::Left(true)), - document_formatting_provider: Some(OneOf::Left(true)), - rename_provider: Some(OneOf::Right(lsp_types::RenameOptions { - prepare_provider: Some(true), - work_done_progress_options: Default::default(), - })), - ..Default::default() - } -} - -/// Receive messages over the LSP transport and route them to `GlobalState`. Requests are answered -/// with a `Response`; notifications mutate state; responses from the client are ignored. -fn main_loop( - connection: Connection, - params: serde_json::Value, -) -> Result<(), Box> { - let params: InitializeParams = serde_json::from_value(params)?; - - // Capture workspace roots so `include` resolution can be confined to them (path-traversal - // defense). Without roots the server refuses to load any include rather than read arbitrarily. - let mut state = GlobalState::new(workspace_roots(¶ms)); - - for msg in &connection.receiver { - match msg { - Message::Request(req) => { - // The `shutdown` request is handled by the transport itself. - if connection.handle_shutdown(&req)? { - return Ok(()); - } - if let Some(resp) = state.handle_request(req)? { - connection.sender.send(Message::Response(resp))?; - } - } - Message::Response(_) => {} - Message::Notification(not) => { - state.handle_notification(not)?; - } - } - } - Ok(()) -} - -/// Workspace root folders from the `initialize` handshake, in priority order: modern clients send -/// `workspace_folders`; older clients send a single `root_uri`. Each is converted from its `file:` -/// URI to a path and canonicalized (so the Vfs's pure containment check compares canonical vs -/// canonical). Non-`file:` roots and roots that can't be canonicalized are dropped. -fn workspace_roots(params: &InitializeParams) -> Vec { - let raw: Vec = if let Some(folders) = ¶ms.workspace_folders { - let roots: Vec = folders - .iter() - .filter_map(|f| f.uri.to_file_path().ok()) - .collect(); - if !roots.is_empty() { - roots - } else { - params - .root_uri - .as_ref() - .and_then(|uri| uri.to_file_path().ok()) - .map(|root| vec![root]) - .unwrap_or_default() - } - } else { - params - .root_uri - .as_ref() - .and_then(|uri| uri.to_file_path().ok()) - .map(|root| vec![root]) - .unwrap_or_default() - }; - raw.into_iter() - .filter_map(|r| r.canonicalize().ok()) - .collect() + ccls::run() } diff --git a/crates/lsp/src/resolver.rs b/crates/lsp/src/resolver.rs index b1f3165..6e6e752 100644 --- a/crates/lsp/src/resolver.rs +++ b/crates/lsp/src/resolver.rs @@ -38,13 +38,10 @@ pub fn token_at_offset(ast: &AstCircomProgram, offset: TextSize) -> Option impl Iterator { - token - .parent() - .into_iter() - .flat_map(|p| p.ancestors().collect::>()) + std::iter::successors(token.parent(), |node| node.parent()) } /// The `Identifier` token covering `offset`, or `None` (`token_at_offset` narrowed). Shared by @@ -105,6 +102,18 @@ pub struct ResolvedSymbol { pub decl_range: Range, } +/// Lift a [`Symbol`] into a [`ResolvedSymbol`] (drops `type_name`). +impl From<&Symbol> for ResolvedSymbol { + fn from(sym: &Symbol) -> Self { + Self { + kind: sym.kind, + name: sym.name.clone(), + def_range: sym.def_range, + decl_range: sym.decl_range, + } + } +} + /// Symbols named `name` visible at `offset`: the body scope containing it, then the file top-level. /// Shared by [`resolve`] and [`resolves_to`] so the lookup source set lives in one place. fn lookup_all<'a>( @@ -125,12 +134,7 @@ pub fn resolve(table: &SymbolTable, token: &SyntaxToken) -> Vec let offset: TextSize = token.text_range().start(); lookup_all(table, offset, name) - .map(|sym| ResolvedSymbol { - kind: sym.kind, - name: sym.name.clone(), - def_range: sym.def_range, - decl_range: sym.decl_range, - }) + .map(ResolvedSymbol::from) .collect() } diff --git a/crates/lsp/src/test_util.rs b/crates/lsp/src/test_util.rs new file mode 100644 index 0000000..818d28d --- /dev/null +++ b/crates/lsp/src/test_util.rs @@ -0,0 +1,57 @@ +//! Shared helpers for `ccls` unit-test modules (`#[cfg(test)]`-only). + +#![cfg(test)] + +use lsp_types::{Position, Url}; +use parser::token_kind::TokenKind; +use syntax::node::SyntaxToken; +use syntax::tree::syntax_tree; + +use crate::file_db::{FileDB, FileId}; +use crate::global_state::GlobalState; + +/// A `GlobalState` with one open document and no workspace roots (in-file only). +pub(crate) fn state_with(url: &Url, source: &str) -> GlobalState { + let mut state = GlobalState::new(Vec::new()); + state.source_db.set_document(url, source.to_string()); + state +} + +/// `Position` of the `occurrence`-th `Identifier` token named `name` (document order). +pub(crate) fn position_of(source: &str, name: &str, occurrence: usize) -> Position { + token_position( + source, + |t| t.kind() == TokenKind::Identifier && t.text() == name, + occurrence, + ) + .unwrap_or_else(|| panic!("identifier token {name}#{occurrence} not found")) +} + +/// `Position` of the first token of any kind whose text equals `text` (keywords / include-strings). +pub(crate) fn position_of_token(source: &str, text: &str) -> Position { + token_position(source, |t: &SyntaxToken| t.text() == text, 0) + .unwrap_or_else(|| panic!("token {text:?} not found")) +} + +/// `Position` of the `occurrence`-th token matching `predicate`, or `None`. +fn token_position( + source: &str, + predicate: impl Fn(&SyntaxToken) -> bool, + occurrence: usize, +) -> Option { + let file = FileDB::new(FileId(0), source, Url::from_file_path("/tmp/x").unwrap()); + let node = syntax_tree(source); + let mut count = 0; + for t in node + .descendants_with_tokens() + .filter_map(|e| e.into_token()) + { + if predicate(&t) { + if count == occurrence { + return Some(file.position(t.text_range().start())); + } + count += 1; + } + } + None +} From 930a943389d72ddf8554f1a4ccb92776eff3e68f Mon Sep 17 00:00:00 2001 From: Vu Vo Date: Sat, 8 Aug 2026 22:37:01 +0700 Subject: [PATCH 2/5] better workspace manage Signed-off-by: Vu Vo --- Cargo.toml | 1 + crates/lsp/Cargo.toml | 1 + crates/lsp/src/global_state.rs | 391 +++++++++++++++++- crates/lsp/src/handler/goto_definition.rs | 89 +++- crates/lsp/src/lib.rs | 72 +++- crates/lsp/src/project_index.rs | 120 ++++++ crates/lsp/src/source_db.rs | 80 +++- .../circuits/lib.circom | 7 + .../with_include_nonsibling/main.circom | 10 + crates/vfs/src/lib.rs | 226 ++++++++++ 10 files changed, 955 insertions(+), 42 deletions(-) create mode 100644 crates/lsp/src/project_index.rs create mode 100644 crates/lsp/src/test_files/handler/with_include_nonsibling/circuits/lib.circom create mode 100644 crates/lsp/src/test_files/handler/with_include_nonsibling/main.circom diff --git a/Cargo.toml b/Cargo.toml index c16a5b3..67d04e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ serde_json = "1.0.78" anyhow = "1.0.79" path-absolutize = "3.1.1" +walkdir = "2.5" # For testing insta = { version = "1.41.1" } diff --git a/crates/lsp/Cargo.toml b/crates/lsp/Cargo.toml index e75f872..f40e5d7 100644 --- a/crates/lsp/Cargo.toml +++ b/crates/lsp/Cargo.toml @@ -19,6 +19,7 @@ serde = { workspace = true, features = ["derive"] } anyhow = { workspace = true } path-absolutize = { workspace = true } +walkdir = { workspace = true } [dev-dependencies] insta = { workspace = true, features = ["yaml"] } diff --git a/crates/lsp/src/global_state.rs b/crates/lsp/src/global_state.rs index 650325d..e8c0af9 100644 --- a/crates/lsp/src/global_state.rs +++ b/crates/lsp/src/global_state.rs @@ -1,11 +1,16 @@ use anyhow::Result; use lsp_server::{Notification, Request, RequestId, Response}; -use lsp_types::notification::{DidChangeTextDocument, DidOpenTextDocument, Notification as _}; +use lsp_types::notification::{ + DidChangeTextDocument, DidChangeWatchedFiles, DidChangeWorkspaceFolders, DidCloseTextDocument, + DidOpenTextDocument, Notification as _, +}; use lsp_types::request::{ Completion, DocumentSymbolRequest, Formatting, GotoDefinition, HoverRequest, PrepareRenameRequest, References, Rename, Request as _, }; -use lsp_types::{DidChangeTextDocumentParams, DidOpenTextDocumentParams, Location, Range, Url}; +use lsp_types::{ + DidChangeTextDocumentParams, DidOpenTextDocumentParams, FileChangeType, Location, Range, Url, +}; use parser::token_kind::TokenKind; use rowan::ast::AstNode; use rowan::TextSize; @@ -14,7 +19,10 @@ use syntax::abstract_syntax_tree::{ }; use syntax::node::SyntaxToken; +use std::cell::RefCell; +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; +use std::sync::Arc; use crate::file_db::{FileDB, FileId}; use crate::handler; @@ -60,6 +68,25 @@ impl From for TextDocument { /// cache. pub struct GlobalState { pub source_db: ContentCacheDb, + /// URIs of documents the client has open (didOpen, not yet didClose). The file-watcher's + /// `CHANGED` arm skips these so an external disk touch never clobbers a document's unsaved + /// (dirty) text — for an open doc, `didChange` is authoritative. + open_documents: HashSet, + /// Memoized result of [`Self::loaded_includes`] per origin [`FileId`]. Interior-mutable so the + /// `&self` read path can fill it; cleared by [`Self::drop_include_cache`] on any text/index + /// mutation (an edit, a watched create/delete, a workspace walk result) so it can't go stale. + loaded_includes_cache: RefCell>>, +} + +/// Internal (non-LSP) notification method the backgrounded workspace walker uses to hand its +/// collected paths back to the main loop, so the eager index build never blocks `initialize`. +pub(crate) const INDEX_WORKSPACE_RESULT_METHOD: &str = "ccls/indexWorkspaceResult"; + +/// Payload of [`INDEX_WORKSPACE_RESULT_METHOD`]: the canonical `.circom` paths the background +/// walker collected. Deserialized on the main thread and fed to `register_indexed_paths`. +#[derive(serde::Deserialize)] +struct IndexWorkspaceResult { + paths: Vec, } /// A resolved cursor location (id, parse, file DB, byte offset) — the shared prologue of every @@ -83,17 +110,36 @@ impl GlobalState { pub fn new(roots: Vec) -> Self { let mut source_db = ContentCacheDb::new(); source_db.set_workspace_roots(roots); - Self { source_db } + Self { + source_db, + open_documents: HashSet::new(), + loaded_includes_cache: RefCell::new(HashMap::new()), + } + } + + /// Drop the memoized [`Self::loaded_includes`] entries — call after any text or basename-index + /// mutation (an edit, a watched create/delete/change, an index walk result, a workspace-folder + /// change) so a stale include-id list can't be served. Coarse but safe: edits are infrequent + /// relative to reads. + pub(crate) fn drop_include_cache(&mut self) { + self.loaded_includes_cache.borrow_mut().clear(); } - /// Resolve `(uri, position)` to a [`CursorContext`], or `None` if the file is unknown or fails - /// to parse. + /// Resolve `(uri, position)` to a [`CursorContext`], or `None` if the file is unknown, has no + /// loaded text, or fails to parse. The no-text guard is load-bearing: the eager workspace walk + /// interns every `.circom` path with **no text**, and a watcher `DELETED` can null an open + /// doc's text — without this check such an id would reach `file_text().expect()` and crash the + /// single-threaded server. pub(crate) fn cursor_context( &self, uri: &Url, position: lsp_types::Position, ) -> Option { let id = self.source_db.id_for_url(uri)?; + // No-text guard: the eager walk interns paths with no text and a watcher DELETED can null an + // open doc's text — without this an id here would reach `file_text().expect()` and crash the + // single-threaded server. `?` returns None for an absent-text id. + self.source_db.vfs().file_text(id)?; let ast = self.source_db.ast(id)?; let file_db = self.source_db.file_db(id); let offset = file_db.offset(position); @@ -124,7 +170,9 @@ impl GlobalState { } } - /// Dispatch an LSP notification; only document open/change are handled. + /// Dispatch an LSP notification. Document open/change load includes; didClose drops open-doc + /// tracking; watched-file changes keep the project basename index fresh (created/deleted + /// `.circom` files); workspace-folder changes walk the newly-added roots. pub fn handle_notification(&mut self, not: Notification) -> Result<()> { match not.method.as_str() { DidOpenTextDocument::METHOD => { @@ -135,11 +183,143 @@ impl GlobalState { let params: DidChangeTextDocumentParams = serde_json::from_value(not.params)?; self.handle_update(TextDocument::from(params))?; } + DidCloseTextDocument::METHOD => { + // No params needed beyond the uri; just stop tracking it as open so a later disk + // change can reload it. Deserialize to validate shape; ignore parse errors softly. + if let Ok(params) = + serde_json::from_value::(not.params) + { + self.open_documents.remove(¶ms.text_document.uri); + } + } + DidChangeWatchedFiles::METHOD => { + let params: lsp_types::DidChangeWatchedFilesParams = + serde_json::from_value(not.params)?; + for change in params.changes { + self.handle_watched_file_change(change); + } + } + DidChangeWorkspaceFolders::METHOD => { + let params: lsp_types::DidChangeWorkspaceFoldersParams = + serde_json::from_value(not.params)?; + self.handle_workspace_folders_change(params); + } + INDEX_WORKSPACE_RESULT_METHOD => { + // Background walker (spawned at initialize) hands back the collected paths; + // register them on the main thread (Vfs is single-threaded). + if let Ok(result) = serde_json::from_value::(not.params) { + self.register_indexed_paths(result.paths); + } + } _ => {} } Ok(()) } + /// Apply one watched-file change to the project basename index. Only `*.circom` files matter: + /// Created → `register_path` (intern path-only); Deleted → `unregister_path` (drop text + + /// caches); Changed → re-read **iff** the file was already loaded AND is not currently open + /// (an open doc's unsaved buffer stays authoritative — `didChange` re-asserts it). A never-loaded + /// file stays path-only until an include resolves to it. + fn handle_watched_file_change(&mut self, change: lsp_types::FileEvent) { + let Ok(path) = change.uri.to_file_path() else { + return; + }; + if path.extension().and_then(|e| e.to_str()) != Some("circom") { + return; + } + match change.typ { + FileChangeType::CREATED => { + if let Ok(canon) = path.canonicalize() { + if let Some(vpath) = vfs::VfsPath::from_abs_path(&canon) { + self.source_db.vfs_mut().register_path(vpath); + self.source_db.invalidate_changed(); + self.drop_include_cache(); + } + } + } + FileChangeType::DELETED => { + // The file is gone, so `canonicalize` fails; absolutize the reported path and look + // it up by identity. Best-effort: a symlinked path that doesn't match the interned + // canonical VfsPath simply isn't found, leaving a stale entry until the next re-walk. + if let Some(vpath) = vfs::VfsPath::from_abs_path(&path) { + self.source_db.vfs_mut().unregister_path(&vpath); + self.source_db.invalidate_changed(); + self.drop_include_cache(); + } + } + FileChangeType::CHANGED => { + // Skip open documents: their authoritative text arrives via didChange, so a disk + // touch (formatter/git) must not clobber the unsaved buffer. + if self.open_documents.contains(&change.uri) { + return; + } + // Re-read only if already loaded (text present); a path-only file stays lazy. + if let Some(vpath) = vfs::VfsPath::from_abs_path(&path) { + let needs_reload = self + .source_db + .vfs() + .file_id(&vpath) + .is_some_and(|id| self.source_db.vfs().file_text(id).is_some()); + if needs_reload { + if let Ok(src) = std::fs::read_to_string(&path) { + self.source_db + .vfs_mut() + .set_file_contents(vpath, Some(Arc::from(src))); + self.source_db.invalidate_changed(); + self.drop_include_cache(); + } + } + } + } + _ => {} + } + } + + /// Merge added/removed workspace folders into the roots and walk **only the newly-added** roots + /// (a full re-walk on every folder change would re-traverse all of `node_modules`). Removed + /// folders' already-indexed files linger but become unconfined once the root is dropped, so no + /// include will load them. + fn handle_workspace_folders_change( + &mut self, + params: lsp_types::DidChangeWorkspaceFoldersParams, + ) { + let mut roots: Vec = self.source_db.vfs().workspace_roots().to_vec(); + let mut added_canon: Vec = Vec::new(); + for added in ¶ms.event.added { + if let Some(canon) = added + .uri + .to_file_path() + .ok() + .and_then(|p| p.canonicalize().ok()) + { + if !roots.contains(&canon) { + roots.push(canon.clone()); + added_canon.push(canon); + } + } + } + let removed = !params.event.removed.is_empty(); + for removed_folder in ¶ms.event.removed { + if let Some(canon) = removed_folder + .uri + .to_file_path() + .ok() + .and_then(|p| p.canonicalize().ok()) + { + roots.retain(|r| r != &canon); + } + } + self.source_db.set_workspace_roots(roots); + // Walk only the added roots; a removed root can't add files. Drop the include cache either + // way (confinement/roots changed, so prior resolved includes may no longer apply). + if !added_canon.is_empty() { + self.register_indexed_paths(crate::project_index::collect_circom_files(&added_canon)); + } else if removed { + self.drop_include_cache(); + } + } + /// Goto-definition shaper: an include-path string routes to [`jump_to_lib`]; any other token /// resolves via [`Self::resolve_token`], file-tagged. pub fn lookup_definition(&self, file_db: &FileDB, token: &SyntaxToken) -> Vec { @@ -171,8 +351,24 @@ impl GlobalState { } /// The [`FileId`]s of every include loaded for `origin`. Shared by [`Self::resolve_use`] and - /// [`Self::resolve_template_file`] so the include walk lives in one place. + /// [`Self::resolve_template_file`] so the include walk lives in one place. Only text-bearing + /// ids are surfaced: `id_for_include`'s basename fallback can return a path-only id (interned + /// by the workspace walk but not yet read), and `symbol_table` below parses its result — a + /// None-text id would panic in `file_text`, so it's filtered out here. Memoized per origin and + /// cleared by [`Self::drop_include_cache`] on any mutation. fn loaded_includes(&self, origin: &FileDB) -> Vec { + if let Some(cached) = self.loaded_includes_cache.borrow().get(&origin.file_id) { + return cached.clone(); + } + let result = self.compute_loaded_includes(origin); + self.loaded_includes_cache + .borrow_mut() + .insert(origin.file_id, result.clone()); + result + } + + /// The uncached resolution behind [`Self::loaded_includes`]. + fn compute_loaded_includes(&self, origin: &FileDB) -> Vec { let Some(ast) = self.source_db.ast(origin.file_id) else { return Vec::new(); }; @@ -180,8 +376,11 @@ impl GlobalState { .into_iter() .filter_map(|inc| inc.lib()) .filter_map(|path| { - self.source_db - .id_for_include(&origin.file_path, &path.value()) + let id = self + .source_db + .id_for_include(&origin.file_path, &path.value())?; + let has_text = self.source_db.vfs().file_text(id).is_some(); + has_text.then_some(id) }) .collect() } @@ -323,6 +522,10 @@ impl GlobalState { /// crashed. Takes the document by value so `text` moves (not clones) — drops one `String` clone /// per keystroke. pub fn handle_update(&mut self, text_document: TextDocument) -> Result<()> { + // Track open documents so the file-watcher never clobbers a dirty buffer (didChange is + // authoritative for open docs). + self.open_documents.insert(text_document.uri.clone()); + let Some((id, changed)) = self .source_db .set_document(&text_document.uri, text_document.text) @@ -336,6 +539,9 @@ impl GlobalState { return Ok(()); } + // An edit can change which includes are loaded → drop the resolved-include memo. + self.drop_include_cache(); + // Includes load from disk once then cache; symbol tables build lazily on first query. if let Some(ast) = self.source_db.ast(id) { for include in ast.libs() { @@ -454,4 +660,171 @@ mod tests { .unwrap(); assert!(state.source_db.id_for_url(&untitled).is_none()); } + + /// Regression: a same-dir `include "lib.circom"` resolves **without** `index_workspace` — the + /// basename index is an addition, not a gate, so the pre-index behavior is unchanged. + #[test] + fn sibling_include_resolves_without_index_test() { + let main_uri = fixture_uri("with_include/main.circom"); + let lib_uri = fixture_uri("with_include/lib.circom"); + let src = std::fs::read_to_string(main_uri.to_file_path().unwrap()).unwrap(); + let crate_path = std::env::var("CARGO_MANIFEST_DIR").unwrap(); + let root = Path::new(&crate_path) + .join("src/test_files/handler/with_include") + .canonicalize() + .unwrap(); + let mut state = GlobalState::new(vec![root]); + // NOTE: no index_workspace() — same-dir resolution must work exactly as before. + state.handle_update(doc(&main_uri, src)).unwrap(); + assert!( + state.source_db.id_for_url(&lib_uri).is_some(), + "sibling include loads without the basename index" + ); + } + + /// A non-sibling `include "lib.circom"` (lib under `circuits/`) resolves via the basename index + /// after `index_workspace`: the same-dir lookup misses, `find_include` picks the indexed + /// `circuits/lib.circom`, and `load_include` reads it. `id_for_include` then surfaces it for + /// cross-file resolve. + #[test] + fn nonsibling_include_resolves_via_index_test() { + let main_uri = fixture_uri("with_include_nonsibling/main.circom"); + let src = std::fs::read_to_string(main_uri.to_file_path().unwrap()).unwrap(); + let crate_path = std::env::var("CARGO_MANIFEST_DIR").unwrap(); + let root = Path::new(&crate_path) + .join("src/test_files/handler/with_include_nonsibling") + .canonicalize() + .unwrap(); + let mut state = GlobalState::new(vec![root]); + state.index_workspace(); + state.handle_update(doc(&main_uri, src)).unwrap(); + + // The lib lives under circuits/ — a same-dir lookup would miss, so loading proves the + // basename fallback fired. + let lib_id = state + .source_db + .load_include(&main_uri, "lib.circom") + .expect("non-sibling include must resolve via the basename index"); + assert!( + state.source_db.vfs().file_text(lib_id).is_some(), + "the resolved lib has loaded text" + ); + + // `id_for_include` (the pure path used by cross-file resolve) also finds it. + assert_eq!( + state.source_db.id_for_include(&main_uri, "lib.circom"), + Some(lib_id), + "id_for_include surfaces the same winner as load_include" + ); + } + + /// With two files sharing the basename `lib.circom`, an `include "circuits/lib.circom"` resolves + /// to the suffix-matching candidate (`circuits/lib.circom`), not the decoy (`other/lib.circom`). + #[test] + fn duplicate_basename_picks_suffix_match_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_dup_{}", std::process::id())); + let ws = base.join("ws"); + let circuits = ws.join("circuits"); + let other = ws.join("other"); + fs::create_dir_all(&circuits).unwrap(); + fs::create_dir_all(&other).unwrap(); + let lib_body = "pragma circom 2.0.0;\ntemplate Lib() { signal output o; o <== 0; }\n"; + fs::write(circuits.join("lib.circom"), lib_body).unwrap(); + fs::write(other.join("lib.circom"), lib_body).unwrap(); + let main_src = "pragma circom 2.0.0;\ninclude \"circuits/lib.circom\";\n"; + fs::write(ws.join("main.circom"), main_src).unwrap(); + + let main_url = Url::from_file_path(ws.join("main.circom")).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + + let id = state + .source_db + .load_include(&main_url, "circuits/lib.circom") + .expect("include resolves among duplicate basenames"); + let resolved = state + .source_db + .vfs() + .path(id) + .expect("winner is interned") + .as_path() + .to_path_buf(); + assert!( + resolved.ends_with("circuits/lib.circom"), + "suffix match wins over the decoy: {resolved:?}" + ); + + let _ = fs::remove_dir_all(&base); + } + + /// Regression (CRITICAL fix): a workspace `.circom` file that is indexed (path-only, no text) + /// but never opened must NOT crash `cursor_context`/`file_text().expect()`. `id_for_url` returns + /// `Some` for the indexed id; the query path must treat it as unreadable and return `None`. + #[test] + fn cursor_context_no_panic_on_indexed_unloaded_file_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_panic_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + fs::write(ws.join("main.circom"), "pragma circom 2.0.0;\n").unwrap(); + + let url = Url::from_file_path(ws.join("main.circom")).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); // interns main.circom with NO text + + // Indexed → id resolves; text absent → query must not panic, just decline. + assert!( + state.source_db.id_for_url(&url).is_some(), + "file is indexed" + ); + assert!( + state + .cursor_context(&url, lsp_types::Position::new(0, 0)) + .is_none(), + "None-text indexed file must not crash cursor_context" + ); + + let _ = fs::remove_dir_all(&base); + } + + /// Regression (dirty-buffer fix): a `workspace/didChangeWatchedFiles` CHANGED event for a file + /// the client has open must NOT overwrite its in-memory (dirty) text from disk — `didChange` is + /// authoritative for open docs. + #[test] + fn watched_changed_skips_open_document_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_open_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let lib_path = ws.join("lib.circom"); + fs::write(&lib_path, "pragma circom 2.0.0;\n").unwrap(); + + let lib_url = Url::from_file_path(&lib_path).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + // Open with dirty text not yet on disk. + state + .handle_update(doc( + &lib_url, + "pragma circom 2.0.0;\ntemplate Dirty() {}\n".to_string(), + )) + .unwrap(); + + // Disk content changes underneath; watcher fires CHANGED. + fs::write(&lib_path, "pragma circom 2.0.0;\n").unwrap(); + state.handle_watched_file_change(lsp_types::FileEvent { + uri: lib_url.clone(), + typ: lsp_types::FileChangeType::CHANGED, + }); + + // The open doc's dirty text is preserved (reload was skipped). + let id = state.source_db.id_for_url(&lib_url).unwrap(); + let text = state.source_db.file_text(id); + assert!( + text.contains("Dirty"), + "open-doc buffer must be preserved across a watcher CHANGED: {text}" + ); + + let _ = fs::remove_dir_all(&base); + } } diff --git a/crates/lsp/src/handler/goto_definition.rs b/crates/lsp/src/handler/goto_definition.rs index 63124be..736d983 100644 --- a/crates/lsp/src/handler/goto_definition.rs +++ b/crates/lsp/src/handler/goto_definition.rs @@ -37,7 +37,9 @@ pub fn handle( // If `token` is an include path (`include "lib.circom";`), jump to that library file's URL. // Routed here (never the resolver) because the resolver only handles `Identifier` tokens — a -// `CircomString` carries a path, not a symbol name. +// `CircomString` carries a path, not a symbol name. Resolution order matches `load_include` so the +// jump target and the actual load always agree: same-dir path first, then a project-wide basename +// fallback (`vfs.find_include`) for a non-sibling include. pub fn jump_to_lib(file_db: &FileDB, token: &SyntaxToken, vfs: &Vfs) -> Vec { let Some(include_stmt) = token_ancestors(token).find_map(AstInclude::cast) else { return Vec::new(); @@ -45,28 +47,46 @@ pub fn jump_to_lib(file_db: &FileDB, token: &SyntaxToken, vfs: &Vfs) -> Vec {} - _ => return Vec::new(), + if let Ok(canon) = lib_path.canonicalize() { + if vfs.is_confined(&canon) { + if let Some(vpath) = vfs::VfsPath::from_abs_path(&lib_path) { + if let Ok(lib_url) = Url::from_file_path(vpath.as_path()) { + return vec![Location::new(lib_url, Range::default())]; + } + } + } } - // Absolutize so the returned location URL is canonical and matches the FileId the source db - // interns for the same include (otherwise a `"../lib.circom"` include resolves to a - // non-canonical URL like `/a/b/../lib.circom` that won't match the interned `/a/lib.circom`). - let Some(vpath) = vfs::VfsPath::from_abs_path(&lib_path) else { + // Same-dir miss → project-wide basename fallback. The winner was interned (path-only is fine — + // a jump only needs the URL, not the text) with an already-canonical path collected from a + // workspace root, so it's confined by construction; the explicit `is_confined` check is kept as + // defense-in-depth. `parent` is the includer **file** (matching `load_include`/`id_for_include`) + // so `find_include`'s nearest tiebreak ranks identically and the jump target always equals the + // loaded target. + let Some(parent_vpath) = vfs::VfsPath::from_abs_path(&path) else { + return Vec::new(); + }; + let Some(winner) = vfs.find_include(&parent_vpath, &rel) else { + return Vec::new(); + }; + let Some(winner_path) = vfs.path(winner) else { return Vec::new(); }; - let Ok(lib_url) = Url::from_file_path(vpath.as_path()) else { + if !vfs.is_confined(winner_path.as_path()) { + return Vec::new(); + } + let Ok(lib_url) = Url::from_file_path(winner_path.as_path()) else { return Vec::new(); }; vec![Location::new(lib_url, Range::default())] @@ -368,4 +388,53 @@ mod tests { let _ = fs::remove_dir_all(&base); } + + /// `jump_to_lib` on a **non-sibling** include (`include "lib.circom"` where lib lives under + /// `circuits/`) resolves to the indexed lib's URL via the basename fallback after + /// `index_workspace`. The same-dir lookup misses (no sibling), so the project index must drive + /// the jump — and the jump target must agree with `load_include`'s resolution. + #[test] + fn jump_to_lib_nonsibling_via_index_test() { + use std::fs; + + let base = std::env::temp_dir().join(format!("ccls_jump_ns_{}", std::process::id())); + let ws = base.join("ws"); + let circuits = ws.join("circuits"); + fs::create_dir_all(&circuits).unwrap(); + fs::write(circuits.join("lib.circom"), "pragma circom 2.0.0;\n").unwrap(); + let main_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\n"; + let main_path = ws.join("main.circom"); + fs::write(&main_path, main_src).unwrap(); + + let main_url = Url::from_file_path(&main_path).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + state + .source_db + .set_document(&main_url, main_src.to_string()); + + let id = state.source_db.id_for_url(&main_url).unwrap(); + let file_db = state.source_db.file_db(id); + let ast = AstCircomProgram::cast(syntax_tree(main_src)).expect("parses to a program"); + let token = ast + .syntax() + .descendants_with_tokens() + .filter_map(|e| e.into_token()) + .find(|t| t.kind() == TokenKind::CircomString) + .expect("include path string present"); + + let locs = super::jump_to_lib(&file_db, &token, state.source_db.vfs()); + assert_eq!( + locs.len(), + 1, + "non-sibling include jumps to exactly one target" + ); + let resolved = locs[0].uri.to_file_path().unwrap(); + assert!( + resolved.ends_with("circuits/lib.circom"), + "jumps to the indexed non-sibling lib: {resolved:?}" + ); + + let _ = fs::remove_dir_all(&base); + } } diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs index d3dbed5..209e8dd 100644 --- a/crates/lsp/src/lib.rs +++ b/crates/lsp/src/lib.rs @@ -3,6 +3,7 @@ pub mod file_db; pub mod global_state; pub mod handler; +pub mod project_index; pub mod resolver; pub mod source_db; pub mod symbol_table; @@ -13,7 +14,7 @@ mod test_util; use std::error::Error; use std::path::PathBuf; -use lsp_server::{Connection, Message}; +use lsp_server::{Connection, Message, Request, RequestId}; use lsp_types::{ CompletionOptions, HoverProviderCapability, InitializeParams, OneOf, ServerCapabilities, TextDocumentSyncCapability, TextDocumentSyncKind, @@ -82,9 +83,36 @@ fn main_loop( // Capture workspace roots so `include` resolution can be confined to them (path-traversal // defense). Without roots the server refuses to load any include rather than read arbitrarily. - let mut state = GlobalState::new(workspace_roots(¶ms)); + let roots = workspace_roots(¶ms); + let mut state = GlobalState::new(roots.clone()); + + // Background the workspace walk so `initialize` never blocks on I/O (a large `node_modules` can + // take seconds). The thread only does the I/O-heavy `collect_circom_files`; it hands the + // canonical paths back on a side channel, and the main loop registers them between messages. + // The server is responsive immediately; non-sibling includes resolve once the index lands, and + // same-dir includes work from the start (never gated on the index). + let (index_tx, index_rx) = std::sync::mpsc::channel::>(); + if !roots.is_empty() { + let walk_roots = roots.clone(); + std::thread::spawn(move || { + let paths = crate::project_index::collect_circom_files(&walk_roots); + // The only error is the receiver being gone (server shutting down); nothing to do then. + let _ = index_tx.send(paths); + }); + } + + // If the client supports it, register a `**/*.circom` watcher per root so created/deleted + // files keep the index fresh without a full re-walk. Best-effort: a missing reply or + // unsupported client just leaves the index stale at the file level (refreshed on workspace + // folder changes); the `Message::Response(_)` no-op arm below absorbs the registration reply. + register_watched_files_capability(&connection, ¶ms, &roots)?; for msg in &connection.receiver { + // Apply any completed background-walk results without blocking, before handling this + // message. `try_recv` returns immediately when nothing is ready yet. + while let Ok(paths) = index_rx.try_recv() { + state.register_indexed_paths(paths); + } match msg { Message::Request(req) => { // The `shutdown` request is handled by the transport itself. @@ -136,3 +164,43 @@ fn workspace_roots(params: &InitializeParams) -> Vec { .filter_map(|r| r.canonicalize().ok()) .collect() } + +/// If the client advertised `workspace.didChangeWatchedFiles` dynamic-registration support, +/// register one `**/*.circom` file watcher per workspace root via a `client/registerCapability` +/// request. Best-effort: no-op if unsupported or if there are no roots. The request is sent on the +/// `connection.sender`; its (empty) reply hits the `Message::Response(_)` no-op arm in +/// [`main_loop`]. We build the params as JSON to stay independent of the proposed +/// `GlobPattern`/`RelativePattern` shape across `lsp_types` versions — the wire format is stable. +fn register_watched_files_capability( + connection: &Connection, + params: &InitializeParams, + roots: &[PathBuf], +) -> Result<(), Box> { + let supported = params + .capabilities + .workspace + .as_ref() + .and_then(|w| w.did_change_watched_files.as_ref()) + .and_then(|d| d.dynamic_registration) + .unwrap_or(false); + if !supported || roots.is_empty() { + return Ok(()); + } + + let watchers: Vec = roots + .iter() + .map(|root| serde_json::json!({ "globPattern": format!("{}/**/*.circom", root.display()) })) + .collect(); + let registrations = vec![serde_json::json!({ + "id": "ccls-watched-files", + "method": "workspace/didChangeWatchedFiles", + "registerOptions": { "watchers": watchers } + })]; + let req = Request { + id: RequestId::from(String::from("ccls-register-watched-files")), + method: String::from("client/registerCapability"), + params: serde_json::json!({ "registrations": registrations }), + }; + connection.sender.send(Message::Request(req))?; + Ok(()) +} diff --git a/crates/lsp/src/project_index.rs b/crates/lsp/src/project_index.rs new file mode 100644 index 0000000..2291f7c --- /dev/null +++ b/crates/lsp/src/project_index.rs @@ -0,0 +1,120 @@ +//! Project-wide `.circom` file discovery + indexing (the only place in the server that walks the +//! filesystem). +//! +//! The pure basename index lives in [`vfs`]; this module is the I/O boundary that feeds it. At +//! `initialize` (and on workspace-folder changes) [`collect_circom_files`] walks each root once and +//! [`GlobalState::index_workspace`] interns every found path with **no text** (cheap — no file +//! reads). Text loads lazily, on demand, the first time an `include` resolves to that path. + +use std::path::PathBuf; + +use vfs::VfsPath; + +use crate::global_state::GlobalState; + +/// Recursively collect every `*.circom` file under `roots`, returning each as a **canonical** +/// absolute path. Descends into `node_modules` (circomlib lives there), so this deliberately does +/// NOT use an ignore-respecting walker. Symlinks are not followed (`walkdir` default), which breaks +/// symlink loops; entries that fail to canonicalize (raced-away, permission-denied) are skipped. +pub(crate) fn collect_circom_files(roots: &[PathBuf]) -> Vec { + let mut out = Vec::new(); + for root in roots { + for entry in walkdir::WalkDir::new(root) + .follow_links(false) + .into_iter() + .filter_map(|e| e.ok()) + { + let path = entry.path(); + // Use the type walkdir already determined (often via `d_type`, no extra syscall) instead + // of `path.is_file()` which issues a fresh `stat` per entry. `file_type()` also skips + // symlinks-to-files, consistent with `follow_links(false)` above. + if !entry.file_type().is_file() { + continue; + } + if path.extension().and_then(|e| e.to_str()) != Some("circom") { + continue; + } + // Canonicalize BEFORE interning: `VfsPath::from_abs_path` only *absolutizes* (lexical, + // no symlink/`..` resolution), so confinement's `starts_with` against canonical roots + // needs a real canonical path here. Skip entries that vanish mid-walk. + if let Ok(canon) = path.canonicalize() { + out.push(canon); + } + } + } + out +} + +impl GlobalState { + /// Intern a pre-collected list of **canonical** `.circom` paths (path-only, no text) into the + /// basename index. The I/O-heavy collection ([`collect_circom_files`]) can run on a background + /// thread and hand its result here; this step is pure interning + cache flush. Idempotent — + /// re-registering existing paths never clobbers text a `load_include`/`didOpen` loaded. + pub(crate) fn register_indexed_paths(&mut self, paths: Vec) { + if paths.is_empty() { + return; + } + for canon in paths { + if let Some(vpath) = VfsPath::from_abs_path(&canon) { + self.source_db.vfs_mut().register_path(vpath); + } + } + // register_path records no change-log entry, so this flush is usually a no-op; new files in + // the index can newly satisfy an include, so the resolved-include cache is dropped too. + self.source_db.invalidate_changed(); + self.drop_include_cache(); + } + + /// Eagerly intern every `.circom` file under the **current** workspace roots (path-only, no + /// text). Used by tests and as the synchronous fallback; the live `main_loop` backgrounds the + /// walk and calls [`Self::register_indexed_paths`] instead so init never blocks on I/O. + pub fn index_workspace(&mut self) { + let roots: Vec = self.source_db.vfs().workspace_roots().to_vec(); + self.register_indexed_paths(collect_circom_files(&roots)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn collect_circom_files_walks_and_canonicalizes_test() { + let base = std::env::temp_dir().join(format!("ccls_walk_{}", std::process::id())); + let ws = base.join("ws"); + let node_modules = ws.join("node_modules").join("circomlib").join("circuits"); + std::fs::create_dir_all(&node_modules).unwrap(); + std::fs::write(ws.join("main.circom"), "pragma circom 2.0.0;").unwrap(); + std::fs::write( + node_modules.join("comparators.circom"), + "pragma circom 2.0.0;", + ) + .unwrap(); + // Non-circom files are ignored. + std::fs::write(ws.join("readme.md"), "# nope").unwrap(); + + let found = collect_circom_files(std::slice::from_ref(&ws)); + let names: Vec = found + .iter() + .filter_map(|p| p.file_name().and_then(|n| n.to_str()).map(String::from)) + .collect(); + assert!( + names.contains(&"main.circom".to_string()), + "walks top-level: {names:?}" + ); + assert!( + names.contains(&"comparators.circom".to_string()), + "descends into node_modules: {names:?}" + ); + assert!( + !names.contains(&"readme.md".to_string()), + "ignores non-.circom files: {names:?}" + ); + // Every result is canonical (absolute, no `..`). + for p in &found { + assert_eq!(p, &p.canonicalize().unwrap(), "result is canonical"); + } + + let _ = std::fs::remove_dir_all(&base); + } +} diff --git a/crates/lsp/src/source_db.rs b/crates/lsp/src/source_db.rs index 2471d56..016c836 100644 --- a/crates/lsp/src/source_db.rs +++ b/crates/lsp/src/source_db.rs @@ -89,6 +89,12 @@ impl ContentCacheDb { &self.vfs } + /// Mutable [`Vfs`] handle for the workspace walker (registers/unregisters project paths in the + /// basename index) and the file-watcher refresh. + pub(crate) fn vfs_mut(&mut self) -> &mut Vfs { + &mut self.vfs + } + /// Convert a `file:` URL to its absolutized [`VfsPath`], or `None` for non-`file:` schemes or /// non-absolutizable paths. Single source for the URI→path step so interning and lookup can't /// disagree (a split would intern under one key and look up another, silently breaking @@ -115,10 +121,18 @@ impl ContentCacheDb { } /// The already-interned `FileId` for an include, without reading disk. `None` if unresolvable - /// or not yet loaded. Used by cross-file goto-def (the include loads once in `handle_update`). + /// or not yet loaded. Resolution order: the **same-dir** [`VfsPath`] lookup first (unchanged — a + /// sibling include resolves without the index), then the **project-wide basename fallback** + /// ([`Vfs::find_include`]) so a non-sibling `include "X.circom"` still resolves when the eager + /// walk indexed `X.circom` elsewhere in the project. Pure — no disk I/O. pub fn id_for_include(&self, parent_url: &Url, rel: &str) -> Option { - let vpath = Self::resolve_include(parent_url, rel)?; - self.vfs.file_id(&vpath) + if let Some(vpath) = Self::resolve_include(parent_url, rel) { + if let Some(id) = self.vfs.file_id(&vpath) { + return Some(id); + } + } + let parent_vpath = Self::url_to_vpath(parent_url)?; + self.vfs.find_include(&parent_vpath, rel) } /// Register/update a document's text. Returns `(FileId, changed)`; `changed` is `false` when @@ -132,36 +146,60 @@ impl ContentCacheDb { } /// Load a relative include from disk **once**, then serve the interned `FileId` from cache — a - /// keystroke in the main file never re-reads its includes. `None` (skipped) for non-`file:` - /// schemes, missing parent dir, unreadable file, or non-absolutizable path. + /// keystroke in the main file never re-reads its includes. Resolution order (so a non-sibling + /// include resolves without regressing the sibling case): + /// 1. **Same-dir disk path** (unchanged): join `rel` onto the includer's dir, then + /// canonicalize + confine + read. + /// 2. On a miss, **project-wide basename fallback**: rank indexed files via + /// [`Vfs::find_include`], take the winner, canonicalize + confine + read it. + /// + /// `None` (skipped) for non-`file:` schemes, a missing/unreadable file, a non-absolutizable + /// path, or an include that escapes the workspace roots. pub fn load_include(&mut self, parent_url: &Url, rel: &str) -> Option { - let vpath = Self::resolve_include(parent_url, rel)?; - - // Security: confine the resolved include to a workspace root. `canonicalize` (the one disk - // stat — kept in this LSP layer so Vfs stays I/O-free) resolves `..`/`.`/symlinks to a real - // absolute path; `is_confined` then checks it's inside a root. Escapes (`include - // "/etc/passwd"`, `../../.ssh/id_rsa`, or a root symlink pointing out) are refused. This is - // the single read boundary; lookup/jump paths only surface includes loaded (and thus - // confined) here. + // 1. Same-dir path first. + if let Some(vpath) = Self::resolve_include(parent_url, rel) { + if let Some(id) = self.load_from_disk(&vpath) { + return Some(id); + } + } + // 2. Basename fallback against the project index. + let parent_vpath = Self::url_to_vpath(parent_url)?; + let winner = self.vfs.find_include(&parent_vpath, rel)?; + let winner_path = self.vfs.path(winner)?.clone(); + self.load_from_disk(&winner_path) + } + + /// Canonicalize + confine + (read-if-needed) for one resolved include path — the single disk + /// boundary shared by the same-dir path and the basename-fallback winner. Returns the loaded + /// `FileId` (cached if already text-bearing), or `None` if the path can't be canonicalized, + /// escapes the workspace, or is unreadable. Security: `canonicalize` resolves `..`/`.`/symlinks; + /// `is_confined` is the pure prefix check. Escapes (`/etc/passwd`, `../../.ssh/id_rsa`, a root + /// symlink pointing out) are refused here so lookup/jump paths only ever surface confined files. + fn load_from_disk(&mut self, vpath: &VfsPath) -> Option { let canonical = vpath.as_path().canonicalize().ok()?; if !self.vfs.is_confined(&canonical) { return None; } - - // Already loaded — serve the cached id (never re-read). - if let Some(id) = self.vfs.file_id(&vpath) { - return Some(id); + // Already interned with text → serve the cached id (never re-read). A path interned with no + // text (by the workspace walk's `register_path`) falls through to the read. + if let Some(id) = self.vfs.file_id(vpath) { + if self.vfs.file_text(id).is_some() { + return Some(id); + } } - let src = std::fs::read_to_string(vpath.as_path()).ok()?; - let id = self.vfs.set_file_contents(vpath, Some(Arc::from(src))); + let id = self + .vfs + .set_file_contents(vpath.clone(), Some(Arc::from(src))); self.invalidate_changed(); Some(id) } /// Drain the VFS change log and drop every changed id's derived caches (others untouched). - /// `parse_count` is intentionally preserved — it tracks total parses, not cache state. - fn invalidate_changed(&mut self) -> Vec { + /// `pub(crate)` so the workspace walker / file-watcher refresh can flush a `Delete` (from + /// `unregister_path`) after mutating the index. `parse_count` is intentionally preserved — it + /// tracks total parses, not cache state. + pub(crate) fn invalidate_changed(&mut self) -> Vec { let changes = self.vfs.take_changes(); if !changes.is_empty() { let mut caches = self.caches.borrow_mut(); diff --git a/crates/lsp/src/test_files/handler/with_include_nonsibling/circuits/lib.circom b/crates/lsp/src/test_files/handler/with_include_nonsibling/circuits/lib.circom new file mode 100644 index 0000000..24d893d --- /dev/null +++ b/crates/lsp/src/test_files/handler/with_include_nonsibling/circuits/lib.circom @@ -0,0 +1,7 @@ +pragma circom 2.0.0; + +template Lib() { + signal input in; + signal output out; + out <== in; +} diff --git a/crates/lsp/src/test_files/handler/with_include_nonsibling/main.circom b/crates/lsp/src/test_files/handler/with_include_nonsibling/main.circom new file mode 100644 index 0000000..ddec99f --- /dev/null +++ b/crates/lsp/src/test_files/handler/with_include_nonsibling/main.circom @@ -0,0 +1,10 @@ +pragma circom 2.0.0; + +include "lib.circom"; + +template Main() { + signal input a; + signal output c; + component m = Lib(); + c <== m.out; +} diff --git a/crates/vfs/src/lib.rs b/crates/vfs/src/lib.rs index 50c5311..2f4d866 100644 --- a/crates/vfs/src/lib.rs +++ b/crates/vfs/src/lib.rs @@ -12,6 +12,7 @@ //! feeds text in via [`Vfs::set_file_contents`]. Keeping vfs I/O-free makes it unit-testable //! without touching the filesystem. +use std::cmp::Reverse; use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -78,6 +79,12 @@ pub struct Vfs { /// [`Vfs::is_confined`] stays pure (no disk I/O) — the `canonicalize` stat is the LSP layer's /// job, keeping this crate I/O-free and unit-testable. workspace_roots: Vec, + /// Project-wide basename index: `file_name` (e.g. `lib.circom`) → every interned [`FileId`] + /// with that basename. The search surface for include resolution when the same-dir lookup + /// misses (an include whose target lives elsewhere in the project). Built by the LSP layer's + /// workspace walk via [`Vfs::register_path`]; kept I/O-free here — only path identity, never a + /// disk read. Ranking in [`Vfs::find_include`] is deterministic and pure. + index: HashMap>, } impl Default for Vfs { @@ -93,6 +100,7 @@ impl Vfs { files: Vec::new(), changes: Vec::new(), workspace_roots: Vec::new(), + index: HashMap::new(), } } @@ -102,6 +110,12 @@ impl Vfs { self.workspace_roots = roots; } + /// The workspace roots confining [`Self::is_confined`] (already canonicalized by the caller). + /// Read accessor so the LSP layer's workspace walker can re-walk the same roots it set. + pub fn workspace_roots(&self) -> &[PathBuf] { + &self.workspace_roots + } + /// Pure containment check: is `canonical` (an already-canonicalized absolute path) inside one of /// the workspace roots? **Fail-closed** — with no roots configured nothing is confined, so the /// LSP refuses to load any include rather than risk an arbitrary read. No disk I/O: the caller @@ -115,6 +129,7 @@ impl Vfs { } /// The id for `path` if it has been interned, else `None`. + #[inline] pub fn file_id(&self, path: &VfsPath) -> Option { self.path_to_id.get(path).copied() } @@ -125,6 +140,7 @@ impl Vfs { } /// The current text of `id`, or `None` if the file is deleted/absent/unknown. + #[inline] pub fn file_text(&self, id: FileId) -> Option> { self.files.get(id.0 as usize).and_then(|s| s.text.clone()) } @@ -197,6 +213,114 @@ impl Vfs { pub fn has_changes(&self) -> bool { !self.changes.is_empty() } + + // --- project-wide basename index (pure, no I/O) --------------------------- + + /// Intern `path` with **no text** and add it to the basename index, so a later + /// [`Self::find_include`] can locate it. Idempotent: a path already interned (whether by an + /// earlier `register_path` or by a text-bearing `set_file_contents`) keeps its existing + /// [`FileId`] and is added to the index at most once. **Does not clobber loaded text** — if the + /// path was loaded (e.g. by `load_include`), its text is preserved; only an unknown path is + /// interned, and interning with `None` records no change-log entry (so no spurious cache drop). + /// The caller canonicalizes the path *before* calling; this method stays pure. + #[inline] + pub fn register_path(&mut self, path: VfsPath) -> FileId { + let id = if let Some(&id) = self.path_to_id.get(&path) { + id + } else { + // Unknown path: intern with no text. This is the no-change-log branch of + // `set_file_contents` (Create-with-None records nothing), so the path becomes known + // without a disk read or a cache invalidation. + self.set_file_contents(path.clone(), None) + }; + if let Some(key) = file_name_of(path.as_path()) { + push_dedup(self.index.entry(key).or_default(), id); + } + id + } + + /// Remove `path`'s [`FileId`] from the basename index and drop any loaded text. Records a + /// [`ChangeKind::Delete`] change **iff** the file had text, so the source db drops its + /// parse/symbol-table caches — a delete must not leave stale caches behind. No-op (and no + /// change recorded) if `path` was never interned, or was interned with no text. + pub fn unregister_path(&mut self, path: &VfsPath) { + let Some(&id) = self.path_to_id.get(path) else { + return; + }; + if let Some(key) = file_name_of(path.as_path()) { + if let Some(vec) = self.index.get_mut(&key) { + vec.retain(|f| *f != id); + if vec.is_empty() { + self.index.remove(&key); + } + } + } + // Drop any loaded text. None→None is a no-op (no change); Some→None records a Delete. + self.set_file_contents(path.clone(), None); + } + + /// Pure ranked search for the best [`FileId`] matching include `rel` (e.g. `lib.circom` or + /// `circuits/x.circom`) from the includer `parent` (any interned path, typically the includer + /// file's own [`VfsPath`]). Candidates are every indexed file sharing `rel`'s basename. Ranking + /// is deterministic: + /// 1. **Suffix match** (desc): a candidate whose path *ends with* the full `rel` (so + /// `include "circuits/x.circom"` prefers `…/circuits/x.circom` over a bare `…/x.circom`). + /// 2. **Nearest** (desc): longest shared path-component prefix with `parent`. + /// 3. **Shortest path** (asc). + /// 4. **Alphabetical** (asc) — the final tiebreak for full determinism. + /// + /// Returns `None` if the basename isn't indexed. + #[inline] + pub fn find_include(&self, parent: &VfsPath, rel: &str) -> Option { + let rel_path = Path::new(rel); + let key = file_name_of(rel_path)?; + let candidates = self.index.get(&key)?; + if candidates.is_empty() { + return None; + } + let parent_path = parent.as_path(); + candidates.iter().copied().min_by_key(|&id| { + let cpath = self + .path(id) + .map(|p| p.as_path()) + .unwrap_or_else(|| Path::new("")); + // min_by_key picks the smallest key, so the "want-highest" fields are inverted. The + // final tiebreak compares the path's `OsStr` directly (`OsStr: Ord`, allocation-free, + // and MSRV-safe — unlike `as_encoded_bytes` which needs Rust 1.74). + let osname = cpath.as_os_str(); + ( + !cpath.ends_with(rel_path), + Reverse(shared_component_prefix(parent_path, cpath)), + osname.len(), + osname, + ) + }) + } +} + +/// The `file_name` of `path` as a UTF-8 `String` (the index key), or `None` if it has none or is +/// non-UTF-8. Both registration and lookup derive the key the same way, so a `&str` include and an +/// interned path agree on their basename bucket. +fn file_name_of(path: &Path) -> Option { + path.file_name()?.to_str().map(|s| s.to_string()) +} + +/// Push `id` into `vec` only if absent (keeps the index dedup'd under repeated registration). +fn push_dedup(vec: &mut Vec, id: FileId) { + if !vec.contains(&id) { + vec.push(id); + } +} + +/// Count of leading path components `candidate` shares with `parent` (component-wise, not +/// byte-wise). Used by [`Vfs::find_include`] to prefer the include target nearest the includer. +/// Zips the two paths' component iterators directly — no intermediate `Vec` allocation. +fn shared_component_prefix(parent: &Path, candidate: &Path) -> usize { + parent + .components() + .zip(candidate.components()) + .take_while(|(a, b)| a == b) + .count() } #[cfg(test)] @@ -278,4 +402,106 @@ mod tests { assert_eq!(changes[0].change_kind, ChangeKind::Delete); assert_eq!(vfs.file_text(id), None, "deleted file has no text"); } + + // --- basename index tests (pure, no disk) --------------------------------- + + #[test] + fn register_then_find_include_test() { + let mut vfs = Vfs::new(); + let a = vfs.register_path(vp("/proj/a/lib.circom")); + let b = vfs.register_path(vp("/proj/b/lib.circom")); + let parent = vp("/proj/main.circom"); + // Same suffix (no), equal shared prefix (/proj), equal length → alphabetical: a < b. + assert_eq!(vfs.find_include(&parent, "lib.circom"), Some(a)); + let _ = b; + } + + #[test] + fn find_include_suffix_match_preferred_test() { + let mut vfs = Vfs::new(); + let bare = vfs.register_path(vp("/proj/x.circom")); + let nested = vfs.register_path(vp("/proj/circuits/x.circom")); + let parent = vp("/proj/main.circom"); + // `circuits/x.circom` matches the nested candidate's suffix; bare does not. + assert_eq!(vfs.find_include(&parent, "circuits/x.circom"), Some(nested)); + // `x.circom` suffix-matches both; tie on shared prefix; shortest path wins → bare. + assert_eq!(vfs.find_include(&parent, "x.circom"), Some(bare)); + } + + #[test] + fn find_include_nearest_wins_test() { + let mut vfs = Vfs::new(); + let near = vfs.register_path(vp("/proj/sub/lib.circom")); + let _far = vfs.register_path(vp("/other/lib.circom")); + let parent = vp("/proj/sub/main.circom"); + // `near` shares `/proj/sub` with the includer; `far` shares only `/`. + assert_eq!(vfs.find_include(&parent, "lib.circom"), Some(near)); + } + + #[test] + fn register_path_idempotent_test() { + let mut vfs = Vfs::new(); + let p = vp("/proj/lib.circom"); + let id1 = vfs.register_path(p.clone()); + let id2 = vfs.register_path(p.clone()); + assert_eq!(id1, id2, "re-registering the same path returns one id"); + assert_eq!( + vfs.find_include(&vp("/proj/m.circom"), "lib.circom"), + Some(id1) + ); + } + + #[test] + fn register_path_preserves_loaded_text_test() { + let mut vfs = Vfs::new(); + let p = vp("/proj/lib.circom"); + let id = vfs.set_file_contents(p.clone(), Some(Arc::from("loaded"))); + let _ = vfs.take_changes(); + // Registering an already-loaded path must NOT delete its text or record a change. + let id2 = vfs.register_path(p.clone()); + assert_eq!(id, id2); + assert_eq!(vfs.file_text(id).as_deref(), Some("loaded")); + assert!( + !vfs.has_changes(), + "registering a loaded path records no change" + ); + } + + #[test] + fn unregister_drops_candidate_test() { + let mut vfs = Vfs::new(); + let p = vp("/proj/lib.circom"); + let id = vfs.register_path(p.clone()); + assert!(vfs + .find_include(&vp("/proj/m.circom"), "lib.circom") + .is_some()); + // A path-only file (never loaded) has no text to delete → no change recorded. + vfs.unregister_path(&p); + assert!( + !vfs.has_changes(), + "deleting a never-loaded path records no change" + ); + assert!(vfs + .find_include(&vp("/proj/m.circom"), "lib.circom") + .is_none()); + // Idempotent: unregistering again is a no-op. + vfs.unregister_path(&p); + let _ = id; + } + + #[test] + fn unregister_loaded_records_delete_test() { + let mut vfs = Vfs::new(); + let p = vp("/proj/lib.circom"); + let id = vfs.set_file_contents(p.clone(), Some(Arc::from("loaded"))); + vfs.register_path(p.clone()); + let _ = vfs.take_changes(); + // Deleting a loaded file must record a Delete so caches drop. + vfs.unregister_path(&p); + let changes = vfs.take_changes(); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].change_kind, ChangeKind::Delete); + assert_eq!(changes[0].file_id, id); + assert!(vfs.file_text(id).is_none()); + } } From 646132a160412b0225a90d0a0ddfdae001bbb551 Mon Sep 17 00:00:00 2001 From: Vu Vo Date: Sun, 9 Aug 2026 00:10:13 +0700 Subject: [PATCH 3/5] add workspace Signed-off-by: Vu Vo --- crates/lsp/src/file_db.rs | 32 +- crates/lsp/src/global_state.rs | 466 +++++++++++++++------ crates/lsp/src/handler.rs | 1 + crates/lsp/src/handler/goto_definition.rs | 176 +++++++- crates/lsp/src/handler/hover.rs | 9 +- crates/lsp/src/handler/references.rs | 381 ++++++++++++++++- crates/lsp/src/handler/rename.rs | 82 +++- crates/lsp/src/handler/workspace_symbol.rs | 126 ++++++ crates/lsp/src/lib.rs | 52 ++- crates/lsp/src/project_index.rs | 133 ++++-- crates/lsp/src/resolver.rs | 17 +- crates/lsp/src/source_db.rs | 98 +++-- crates/vfs/src/lib.rs | 111 +++-- 13 files changed, 1356 insertions(+), 328 deletions(-) create mode 100644 crates/lsp/src/handler/workspace_symbol.rs diff --git a/crates/lsp/src/file_db.rs b/crates/lsp/src/file_db.rs index f8c5629..3da10f2 100644 --- a/crates/lsp/src/file_db.rs +++ b/crates/lsp/src/file_db.rs @@ -25,7 +25,10 @@ pub struct FileDB { impl FileDB { pub(crate) fn new(file_id: FileId, content: &str, file_path: Url) -> Self { let mut newline_offsets = Vec::new(); - for (offset, c) in content.chars().enumerate() { + // `char_indices` yields BYTE offsets; `chars().enumerate()` would yield char indices, which + // diverge from byte offsets once a multi-byte char appears and later panic in the + // `[line_start..]` slice (`line_start_byte` uses these as byte offsets). + for (offset, c) in content.char_indices() { if c == '\n' { newline_offsets.push(offset as u32); } @@ -196,4 +199,31 @@ mod tests { assert_eq!(file_db.position(6.into()), Position::new(0, 3)); assert_eq!(file_db.position(2.into()), Position::new(0, 1)); } + + /// Regression for the exit-101 panic: a multi-byte char **before a newline** must not make + /// `newline_offsets` (and thus `line_start_byte`'s `[line_start..]` slice) land mid-character. + /// The box-drawing `─` in circomlib comments (3 bytes) crashed `position()` because the offsets + /// were stored as char indices, not byte offsets. + #[test] + fn multibyte_before_newline_no_panic_test() { + // Line 0 ends with `─` (U+2500, 3 bytes); line 1 is `ab`. Bytes: ─=0..3, \n=3, a=4, b=5. + let source = "─\nab"; + let file_db = FileDB::new( + FileId(1), + source, + Url::from_file_path(Path::new("/tmp.txt")).unwrap(), + ); + // The newline is at byte 3, not char index 1. + assert_eq!(file_db.newline_offsets, vec![3]); + + // `a` is at byte 4 → line 1, character 0 (first char on line 1). + assert_eq!(file_db.position(4.into()), Position::new(1, 0)); + // `b` is at byte 5 → line 1, character 1. + assert_eq!(file_db.position(5.into()), Position::new(1, 1)); + // `─` occupies line 0 characters 0..1 (1 UTF-16 unit). + assert_eq!(file_db.position(0.into()), Position::new(0, 0)); + + // Round trip: Position(1,1) → byte 5. + assert_eq!(file_db.offset(Position::new(1, 1)), 5.into()); + } } diff --git a/crates/lsp/src/global_state.rs b/crates/lsp/src/global_state.rs index e8c0af9..b62aa08 100644 --- a/crates/lsp/src/global_state.rs +++ b/crates/lsp/src/global_state.rs @@ -26,9 +26,10 @@ use std::sync::Arc; use crate::file_db::{FileDB, FileId}; use crate::handler; -use crate::handler::goto_definition::jump_to_lib; +use crate::handler::goto_definition::include_target_location; use crate::resolver::{self, token_ancestors, ResolvedSymbol}; use crate::source_db::{ContentCacheDb, SourceDatabase}; +use crate::symbol_table::{Symbol, SymbolKind}; /// A didOpen/didChange notification normalized to its full text + URI. #[derive(Debug)] @@ -68,25 +69,14 @@ impl From for TextDocument { /// cache. pub struct GlobalState { pub source_db: ContentCacheDb, - /// URIs of documents the client has open (didOpen, not yet didClose). The file-watcher's - /// `CHANGED` arm skips these so an external disk touch never clobbers a document's unsaved - /// (dirty) text — for an open doc, `didChange` is authoritative. - open_documents: HashSet, - /// Memoized result of [`Self::loaded_includes`] per origin [`FileId`]. Interior-mutable so the - /// `&self` read path can fill it; cleared by [`Self::drop_include_cache`] on any text/index - /// mutation (an edit, a watched create/delete, a workspace walk result) so it can't go stale. + /// URIs the client has open (didOpen, not yet didClose). The watcher's Deleted/Changed arms + /// skip these so an external disk touch can't clobber an open doc's unsaved buffer. + pub(crate) open_documents: HashSet, + /// Memoized [`Self::loaded_includes`] per origin; interior-mutable for the `&self` read path. + /// Cleared by [`Self::drop_include_cache`] on any text/index mutation. loaded_includes_cache: RefCell>>, -} - -/// Internal (non-LSP) notification method the backgrounded workspace walker uses to hand its -/// collected paths back to the main loop, so the eager index build never blocks `initialize`. -pub(crate) const INDEX_WORKSPACE_RESULT_METHOD: &str = "ccls/indexWorkspaceResult"; - -/// Payload of [`INDEX_WORKSPACE_RESULT_METHOD`]: the canonical `.circom` paths the background -/// walker collected. Deserialized on the main thread and fed to `register_indexed_paths`. -#[derive(serde::Deserialize)] -struct IndexWorkspaceResult { - paths: Vec, + /// Eagerly loaded workspace `.circom` files — the scan set for workspace occurrences/symbol. + pub(crate) workspace_files: HashSet, } /// A resolved cursor location (id, parse, file DB, byte offset) — the shared prologue of every @@ -114,31 +104,33 @@ impl GlobalState { source_db, open_documents: HashSet::new(), loaded_includes_cache: RefCell::new(HashMap::new()), + workspace_files: HashSet::new(), } } - /// Drop the memoized [`Self::loaded_includes`] entries — call after any text or basename-index - /// mutation (an edit, a watched create/delete/change, an index walk result, a workspace-folder - /// change) so a stale include-id list can't be served. Coarse but safe: edits are infrequent - /// relative to reads. + /// Drop the memoized [`Self::loaded_includes`] — call after any text/index mutation so a stale + /// include-id list can't be served. pub(crate) fn drop_include_cache(&mut self) { self.loaded_includes_cache.borrow_mut().clear(); } - /// Resolve `(uri, position)` to a [`CursorContext`], or `None` if the file is unknown, has no - /// loaded text, or fails to parse. The no-text guard is load-bearing: the eager workspace walk - /// interns every `.circom` path with **no text**, and a watcher `DELETED` can null an open - /// doc's text — without this check such an id would reach `file_text().expect()` and crash the - /// single-threaded server. + /// Flush derived caches after a VFS mutation that may record a change-log entry (text + /// delete/modify). Register-only mutations (no change recorded) call `drop_include_cache` only. + fn after_vfs_mutation(&mut self) { + self.source_db.invalidate_changed(); + self.drop_include_cache(); + } + + /// Resolve `(uri, position)` to a [`CursorContext`], or `None` if unknown, text-less, or + /// unparseable. The no-text guard is load-bearing: the workspace walk interns paths with no text + /// and a watcher `DELETED` can null an open doc's text — without it such an id would reach + /// `file_text().expect()` and crash the single-threaded server. pub(crate) fn cursor_context( &self, uri: &Url, position: lsp_types::Position, ) -> Option { let id = self.source_db.id_for_url(uri)?; - // No-text guard: the eager walk interns paths with no text and a watcher DELETED can null an - // open doc's text — without this an id here would reach `file_text().expect()` and crash the - // single-threaded server. `?` returns None for an absent-text id. self.source_db.vfs().file_text(id)?; let ast = self.source_db.ast(id)?; let file_db = self.source_db.file_db(id); @@ -166,6 +158,7 @@ impl GlobalState { Formatting::METHOD => dispatch(self, id, req, handler::formatting::handle), Rename::METHOD => dispatch(self, id, req, handler::rename::handle), PrepareRenameRequest::METHOD => dispatch(self, id, req, handler::rename::prepare), + "workspace/symbol" => dispatch(self, id, req, handler::workspace_symbol::handle), _ => Ok(None), } } @@ -184,8 +177,6 @@ impl GlobalState { self.handle_update(TextDocument::from(params))?; } DidCloseTextDocument::METHOD => { - // No params needed beyond the uri; just stop tracking it as open so a later disk - // change can reload it. Deserialize to validate shape; ignore parse errors softly. if let Ok(params) = serde_json::from_value::(not.params) { @@ -204,23 +195,15 @@ impl GlobalState { serde_json::from_value(not.params)?; self.handle_workspace_folders_change(params); } - INDEX_WORKSPACE_RESULT_METHOD => { - // Background walker (spawned at initialize) hands back the collected paths; - // register them on the main thread (Vfs is single-threaded). - if let Ok(result) = serde_json::from_value::(not.params) { - self.register_indexed_paths(result.paths); - } - } _ => {} } Ok(()) } - /// Apply one watched-file change to the project basename index. Only `*.circom` files matter: - /// Created → `register_path` (intern path-only); Deleted → `unregister_path` (drop text + - /// caches); Changed → re-read **iff** the file was already loaded AND is not currently open - /// (an open doc's unsaved buffer stays authoritative — `didChange` re-asserts it). A never-loaded - /// file stays path-only until an include resolves to it. + /// Apply one watched-file change to the index. `*.circom` only. Created → intern; Deleted → + /// unregister; Changed → re-read iff loaded. Deleted/Changed skip open docs (a build tool + /// deleting/recreating/touching the file must not clobber the open doc's unsaved buffer — + /// `didChange` is authoritative for it). fn handle_watched_file_change(&mut self, change: lsp_types::FileEvent) { let Ok(path) = change.uri.to_file_path() else { return; @@ -232,29 +215,38 @@ impl GlobalState { FileChangeType::CREATED => { if let Ok(canon) = path.canonicalize() { if let Some(vpath) = vfs::VfsPath::from_abs_path(&canon) { - self.source_db.vfs_mut().register_path(vpath); - self.source_db.invalidate_changed(); - self.drop_include_cache(); + let id = self.source_db.vfs_mut().register_path(vpath.clone()); + self.workspace_files.insert(id); + if let Ok(src) = std::fs::read_to_string(&path) { + self.source_db + .vfs_mut() + .set_file_contents(vpath, Some(Arc::from(src))); + self.after_vfs_mutation(); + } else { + self.drop_include_cache(); + } } } } FileChangeType::DELETED => { - // The file is gone, so `canonicalize` fails; absolutize the reported path and look - // it up by identity. Best-effort: a symlinked path that doesn't match the interned - // canonical VfsPath simply isn't found, leaving a stale entry until the next re-walk. + if self.open_documents.contains(&change.uri) { + return; + } + // The file is gone, so `canonicalize` fails; absolutize and look up by identity. + // Best-effort: a symlinked path that doesn't match the interned canonical VfsPath + // isn't found, leaving a stale entry until the next re-walk. if let Some(vpath) = vfs::VfsPath::from_abs_path(&path) { + if let Some(id) = self.source_db.vfs().file_id(&vpath) { + self.workspace_files.remove(&id); + } self.source_db.vfs_mut().unregister_path(&vpath); - self.source_db.invalidate_changed(); - self.drop_include_cache(); + self.after_vfs_mutation(); } } FileChangeType::CHANGED => { - // Skip open documents: their authoritative text arrives via didChange, so a disk - // touch (formatter/git) must not clobber the unsaved buffer. if self.open_documents.contains(&change.uri) { return; } - // Re-read only if already loaded (text present); a path-only file stays lazy. if let Some(vpath) = vfs::VfsPath::from_abs_path(&path) { let needs_reload = self .source_db @@ -266,8 +258,7 @@ impl GlobalState { self.source_db .vfs_mut() .set_file_contents(vpath, Some(Arc::from(src))); - self.source_db.invalidate_changed(); - self.drop_include_cache(); + self.after_vfs_mutation(); } } } @@ -276,10 +267,8 @@ impl GlobalState { } } - /// Merge added/removed workspace folders into the roots and walk **only the newly-added** roots - /// (a full re-walk on every folder change would re-traverse all of `node_modules`). Removed - /// folders' already-indexed files linger but become unconfined once the root is dropped, so no - /// include will load them. + /// Merge added/removed folders into the roots. Added → walk only that root; removed → unregister + /// its files so they stop resolving (the pure read path can't confine them). fn handle_workspace_folders_change( &mut self, params: lsp_types::DidChangeWorkspaceFoldersParams, @@ -287,44 +276,42 @@ impl GlobalState { let mut roots: Vec = self.source_db.vfs().workspace_roots().to_vec(); let mut added_canon: Vec = Vec::new(); for added in ¶ms.event.added { - if let Some(canon) = added - .uri - .to_file_path() - .ok() - .and_then(|p| p.canonicalize().ok()) - { + if let Some(canon) = canon_folder(&added.uri) { if !roots.contains(&canon) { roots.push(canon.clone()); added_canon.push(canon); } } } - let removed = !params.event.removed.is_empty(); + let mut removed_canon: Vec = Vec::new(); for removed_folder in ¶ms.event.removed { - if let Some(canon) = removed_folder - .uri - .to_file_path() - .ok() - .and_then(|p| p.canonicalize().ok()) - { + if let Some(canon) = canon_folder(&removed_folder.uri) { roots.retain(|r| r != &canon); + removed_canon.push(canon); } } self.source_db.set_workspace_roots(roots); - // Walk only the added roots; a removed root can't add files. Drop the include cache either - // way (confinement/roots changed, so prior resolved includes may no longer apply). + if !removed_canon.is_empty() { + for canon in &removed_canon { + for id in self.source_db.vfs().ids_under(canon) { + self.workspace_files.remove(&id); + } + self.source_db.vfs_mut().unregister_under(canon); + } + self.after_vfs_mutation(); + } if !added_canon.is_empty() { - self.register_indexed_paths(crate::project_index::collect_circom_files(&added_canon)); - } else if removed { - self.drop_include_cache(); + self.register_indexed_paths(crate::project_index::collect_circom_files_with_content( + &added_canon, + )); } } - /// Goto-definition shaper: an include-path string routes to [`jump_to_lib`]; any other token + /// Goto-definition shaper: an include-path string routes to [`include_target_location`]; any other token /// resolves via [`Self::resolve_token`], file-tagged. pub fn lookup_definition(&self, file_db: &FileDB, token: &SyntaxToken) -> Vec { if token.kind() == TokenKind::CircomString { - return jump_to_lib(file_db, token, self.source_db.vfs()); + return include_target_location(file_db, token, self.source_db.vfs()); } self.to_locations(self.resolve_token(file_db, token)) } @@ -350,12 +337,9 @@ impl GlobalState { .collect() } - /// The [`FileId`]s of every include loaded for `origin`. Shared by [`Self::resolve_use`] and - /// [`Self::resolve_template_file`] so the include walk lives in one place. Only text-bearing - /// ids are surfaced: `id_for_include`'s basename fallback can return a path-only id (interned - /// by the workspace walk but not yet read), and `symbol_table` below parses its result — a - /// None-text id would panic in `file_text`, so it's filtered out here. Memoized per origin and - /// cleared by [`Self::drop_include_cache`] on any mutation. + /// The [`FileId`]s of every text-bearing include loaded for `origin`. Path-only ids (interned + /// by the walk but unread) are filtered: `symbol_table` parses the result and would panic on a + /// None-text id. Memoized per origin; cleared by [`Self::drop_include_cache`]. fn loaded_includes(&self, origin: &FileDB) -> Vec { if let Some(cached) = self.loaded_includes_cache.borrow().get(&origin.file_id) { return cached.clone(); @@ -367,7 +351,6 @@ impl GlobalState { result } - /// The uncached resolution behind [`Self::loaded_includes`]. fn compute_loaded_includes(&self, origin: &FileDB) -> Vec { let Some(ast) = self.source_db.ast(origin.file_id) else { return Vec::new(); @@ -421,6 +404,37 @@ impl GlobalState { out } + /// Resolve `token` with workspace visibility — [`Self::resolve_use`] **without** the + /// component-use gate. In-file `resolve` first (precedence/shadowing): when non-empty it wins + /// outright, so an in-file def shadows an include's same-named def. Only when in-file + /// resolution is empty does it search each **direct** include's top-level by name (circom's + /// non-transitive include visibility). References/rename need this ungated path to find usages + /// of *any* included top-level symbol (a template/function referenced by name inside a body), + /// not just component instantiations. + pub(crate) fn resolve_visible( + &self, + origin: &FileDB, + token: &SyntaxToken, + ) -> Vec<(FileId, ResolvedSymbol)> { + let table = self.source_db.symbol_table(origin.file_id); + let in_file: Vec<(FileId, ResolvedSymbol)> = resolver::resolve(&table, token) + .into_iter() + .map(|s| (origin.file_id, s)) + .collect(); + if !in_file.is_empty() { + return in_file; + } + let name = token.text(); + let mut out = Vec::new(); + for lib_id in self.loaded_includes(origin) { + let lib_table = self.source_db.symbol_table(lib_id); + for sym in lib_table.lookup_top_level(name) { + out.push((lib_id, ResolvedSymbol::from(sym))); + } + } + out + } + /// Resolve a component member-access **field** token (`c.x` / `T()(...).x`) to its signal /// declaration in the receiver's template — the type-inference path the flat [`resolve`] /// deliberately omits. Returns the file-tagged signal declaration(s), or empty if the receiver @@ -488,42 +502,77 @@ impl GlobalState { }) } - /// Occurrences of `target` in its defining file, as tokens. In-file by design: each - /// `SymbolTable` indexes only its own file, so a token resolves unambiguously. Cross-file rename - /// is deferred — it needs a workspace symbol graph, not name/`def_range` matching across files - /// (that both misses real cross-file usages and can collide when two files define a same-named - /// symbol at the same line:column). - pub(crate) fn find_occurrences( + /// Every occurrence of `target` across the workspace, as `(FileId, Range)`. Scans + /// [`Self::workspace_files`] plus the cursor's `origin` file and the target's defining file + /// (the latter two cover the no-index / in-file case). For each candidate file, finds + /// `Identifier` tokens named `target.name` and keeps those that [`Self::resolve_visible`] + /// resolves to `(target FileId, kind, def_range)`. Resolution-based, so shadowing and same-name + /// collisions across files are sound: a token matches only if it genuinely refers to `target`. + /// Files with no loaded text or no parse are skipped (the `cursor_context` None-text guard, + /// workspace-wide). + pub(crate) fn workspace_occurrences( &self, target: &(FileId, ResolvedSymbol), - ) -> Vec { + origin: FileId, + ) -> Vec<(FileId, Range)> { let (def_file, sym) = target; - let Some(ast) = self.source_db.ast(*def_file) else { - return Vec::new(); - }; - let table = self.source_db.symbol_table(*def_file); - resolver::occurrences_in(ast.syntax(), &table, sym) + let name = sym.name.as_str(); + + let mut scan = self.workspace_files.clone(); + scan.insert(origin); + scan.insert(*def_file); + + let mut out = Vec::new(); + for f in scan { + if self.source_db.vfs().file_text(f).is_none() { + continue; + } + let Some(ast) = self.source_db.ast(f) else { + continue; + }; + let file_db = self.source_db.file_db(f); + for tok in resolver::identifiers_named(ast.syntax(), name) { + let matched = self + .resolve_visible(&file_db, &tok) + .into_iter() + .any(|(fid, r)| { + fid == *def_file && r.kind == sym.kind && r.def_range == sym.def_range + }); + if matched { + out.push((f, file_db.token_range(&tok))); + } + } + } + out } - /// Defining-file URL + every occurrence range of `target` (shared by references and rename). - pub(crate) fn occurrence_ranges(&self, target: &(FileId, ResolvedSymbol)) -> (Url, Vec) { - let def_file_db = self.source_db.file_db(target.0); - let ranges = self - .find_occurrences(target) - .into_iter() - .map(|t| def_file_db.token_range(&t)) - .collect(); - (def_file_db.file_path.clone(), ranges) + /// Every top-level template/function/bus in the workspace whose name matches `query` (empty + /// query ⇒ all), as owned `(FileId, Symbol)` (each file's `SymbolTable` is cached behind a + /// short-lived borrow). Powers `workspace/symbol`. + pub(crate) fn workspace_symbols(&self, query: &str) -> Vec<(FileId, Symbol)> { + let q = query.trim(); + let mut out = Vec::new(); + for f in &self.workspace_files { + if self.source_db.vfs().file_text(*f).is_none() { + continue; + } + if self.source_db.ast(*f).is_none() { + continue; + } + let table = self.source_db.symbol_table(*f); + for sym in table.top_level_symbols() { + if is_workspace_symbol_kind(sym.kind) && matches_query(q, &sym.name) { + out.push((*f, sym.clone())); + } + } + } + out } - /// Register an updated document: set its text (dropping derived caches) and load each `include` - /// once. No eager index — the symbol table builds lazily and invalidates on edit; a no-op - /// (identical text) short-circuits; non-`file:` URIs and unreadable includes are skipped, not - /// crashed. Takes the document by value so `text` moves (not clones) — drops one `String` clone - /// per keystroke. + /// Register an updated document: set text (dropping derived caches) and load each include once. + /// Identical text short-circuits; non-`file:` URIs and unreadable includes are skipped, not + /// crashed. Takes the doc by value so `text` moves. pub fn handle_update(&mut self, text_document: TextDocument) -> Result<()> { - // Track open documents so the file-watcher never clobbers a dirty buffer (didChange is - // authoritative for open docs). self.open_documents.insert(text_document.uri.clone()); let Some((id, changed)) = self @@ -558,6 +607,25 @@ impl GlobalState { } } +/// Canonical absolute path of a workspace-folder URI (`None` for non-`file:` or un-canonicalizable). +fn canon_folder(uri: &Url) -> Option { + uri.to_file_path().ok().and_then(|p| p.canonicalize().ok()) +} + +/// `true` for top-level kinds surfaced by `workspace/symbol` (templates/functions/buses). +fn is_workspace_symbol_kind(kind: SymbolKind) -> bool { + matches!( + kind, + SymbolKind::Template | SymbolKind::Function | SymbolKind::Bus + ) +} + +/// Case-insensitive substring match; an empty query matches everything (`workspace/symbol` lists +/// all symbols when the query is blank). +fn matches_query(query: &str, name: &str) -> bool { + query.is_empty() || name.to_lowercase().contains(&query.to_lowercase()) +} + /// Deserialize params, run the handler, wrap the result in a success `Response`. Generic so /// [`GlobalState::handle_request`] stays a flat one-line-per-feature table. fn dispatch( @@ -758,31 +826,36 @@ mod tests { let _ = fs::remove_dir_all(&base); } - /// Regression (CRITICAL fix): a workspace `.circom` file that is indexed (path-only, no text) - /// but never opened must NOT crash `cursor_context`/`file_text().expect()`. `id_for_url` returns - /// `Some` for the indexed id; the query path must treat it as unreadable and return `None`. + /// Regression (CRITICAL fix): an interned file with **no text** must NOT crash + /// `cursor_context`/`file_text().expect()`. `id_for_url` returns `Some` for the interned id; + /// the query path must treat it as unreadable and return `None`. (Eager loading means + /// `index_workspace` no longer produces text-less ids, so this interns a path-only id directly + /// to exercise the guard.) #[test] - fn cursor_context_no_panic_on_indexed_unloaded_file_test() { + fn cursor_context_no_panic_on_textless_file_test() { use std::fs; let base = std::env::temp_dir().join(format!("ccls_panic_{}", std::process::id())); let ws = base.join("ws"); fs::create_dir_all(&ws).unwrap(); fs::write(ws.join("main.circom"), "pragma circom 2.0.0;\n").unwrap(); + let canon = ws.join("main.circom").canonicalize().unwrap(); - let url = Url::from_file_path(ws.join("main.circom")).unwrap(); + let url = Url::from_file_path(&canon).unwrap(); let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); - state.index_workspace(); // interns main.circom with NO text + // Intern the path with NO text (path-only), simulating a not-yet-loaded/leaked id. + if let Some(vpath) = vfs::VfsPath::from_abs_path(&canon) { + state.source_db.vfs_mut().register_path(vpath); + } - // Indexed → id resolves; text absent → query must not panic, just decline. assert!( state.source_db.id_for_url(&url).is_some(), - "file is indexed" + "file is interned" ); assert!( state .cursor_context(&url, lsp_types::Position::new(0, 0)) .is_none(), - "None-text indexed file must not crash cursor_context" + "None-text file must not crash cursor_context" ); let _ = fs::remove_dir_all(&base); @@ -827,4 +900,145 @@ mod tests { let _ = fs::remove_dir_all(&base); } + + /// Regression (DELETED fix): a `workspace/didChangeWatchedFiles` DELETED event for a file the + /// client has open must NOT null its in-memory text (a build tool deleting+recreating the file + /// must not make the open doc unresolvable). Mirrors the CHANGED open-doc guard. + #[test] + fn watched_deleted_skips_open_document_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_del_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let lib_path = ws.join("lib.circom"); + fs::write(&lib_path, "pragma circom 2.0.0;\n").unwrap(); + let lib_url = Url::from_file_path(&lib_path).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state + .handle_update(doc( + &lib_url, + "pragma circom 2.0.0;\ntemplate Dirty() {}\n".to_string(), + )) + .unwrap(); + + // File deleted on disk while the doc is open; watcher fires DELETED. + let _ = fs::remove_file(&lib_path); + state.handle_watched_file_change(lsp_types::FileEvent { + uri: lib_url.clone(), + typ: lsp_types::FileChangeType::DELETED, + }); + + // The open doc's text is preserved (the unregister was skipped). + let id = state.source_db.id_for_url(&lib_url).unwrap(); + let text = state.source_db.file_text(id); + assert!( + text.contains("Dirty"), + "open-doc text must be preserved across a watcher DELETED: {text}" + ); + + let _ = fs::remove_dir_all(&base); + } + + /// Regression (removed-folder fix): after a workspace folder is removed, an already-loaded + /// include that lived under it must stop resolving (the read path now checks confinement). + #[test] + fn removed_folder_include_stops_resolving_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_rm_{}", std::process::id())); + let ws1 = base.join("ws1"); + let ws2 = base.join("ws2"); + fs::create_dir_all(&ws1).unwrap(); + fs::create_dir_all(&ws2).unwrap(); + let main_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\n"; + fs::write(ws1.join("main.circom"), main_src).unwrap(); + fs::write( + ws2.join("lib.circom"), + "pragma circom 2.0.0;\ntemplate Lib() { signal output o; o <== 0; }\n", + ) + .unwrap(); + let main_url = Url::from_file_path(ws1.join("main.circom")).unwrap(); + let mut state = GlobalState::new(vec![ + ws1.canonicalize().unwrap(), + ws2.canonicalize().unwrap(), + ]); + state.index_workspace(); + state + .handle_update(doc(&main_url, main_src.to_string())) + .unwrap(); + // The non-sibling lib (under ws2) resolves via the basename fallback. + assert!( + state + .source_db + .id_for_include(&main_url, "lib.circom") + .is_some(), + "lib resolves while its root is present" + ); + + // Remove ws2 from the workspace. + state.handle_workspace_folders_change(lsp_types::DidChangeWorkspaceFoldersParams { + event: lsp_types::WorkspaceFoldersChangeEvent { + added: Vec::new(), + removed: vec![lsp_types::WorkspaceFolder { + uri: Url::from_file_path(&ws2).unwrap(), + name: "ws2".to_string(), + }], + }, + }); + + // The lib is no longer confined → must stop resolving. + assert!( + state + .source_db + .id_for_include(&main_url, "lib.circom") + .is_none(), + "removed-folder include must stop resolving" + ); + + let _ = fs::remove_dir_all(&base); + } + + /// Regression (walk-landing reload fix): a doc with a non-sibling include opened BEFORE the + /// index is populated has its include loaded when the walk lands — without needing another edit + /// — so cross-file symbol features activate immediately. + #[test] + fn index_landing_reloads_open_doc_non_sibling_include_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_land_{}", std::process::id())); + let ws = base.join("ws"); + let circuits = ws.join("circuits"); + fs::create_dir_all(&circuits).unwrap(); + fs::write( + circuits.join("lib.circom"), + "pragma circom 2.0.0;\ntemplate Lib() { signal output o; o <== 0; }\n", + ) + .unwrap(); + let main_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\ncomponent main = Lib();\n"; + fs::write(ws.join("main.circom"), main_src).unwrap(); + let main_url = Url::from_file_path(ws.join("main.circom")).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + + // Open main BEFORE the index is built (simulates didOpen beating the background walk). + state + .handle_update(doc(&main_url, main_src.to_string())) + .unwrap(); + assert!( + state + .source_db + .id_for_include(&main_url, "lib.circom") + .is_none(), + "pre-index: non-sibling include is unresolved" + ); + + // The walk lands → open docs' includes are re-loaded. + state.index_workspace(); + assert!( + state + .source_db + .id_for_include(&main_url, "lib.circom") + .is_some(), + "post-index: include resolves after the walk reloads open docs" + ); + + let _ = fs::remove_dir_all(&base); + } } diff --git a/crates/lsp/src/handler.rs b/crates/lsp/src/handler.rs index dc31ed8..fb7b583 100644 --- a/crates/lsp/src/handler.rs +++ b/crates/lsp/src/handler.rs @@ -12,3 +12,4 @@ pub mod goto_definition; pub mod hover; pub mod references; pub mod rename; +pub mod workspace_symbol; diff --git a/crates/lsp/src/handler/goto_definition.rs b/crates/lsp/src/handler/goto_definition.rs index 736d983..95b52dd 100644 --- a/crates/lsp/src/handler/goto_definition.rs +++ b/crates/lsp/src/handler/goto_definition.rs @@ -35,12 +35,10 @@ pub fn handle( Ok(Some(GotoDefinitionResponse::Array(locations))) } -// If `token` is an include path (`include "lib.circom";`), jump to that library file's URL. -// Routed here (never the resolver) because the resolver only handles `Identifier` tokens — a -// `CircomString` carries a path, not a symbol name. Resolution order matches `load_include` so the -// jump target and the actual load always agree: same-dir path first, then a project-wide basename -// fallback (`vfs.find_include`) for a non-sibling include. -pub fn jump_to_lib(file_db: &FileDB, token: &SyntaxToken, vfs: &Vfs) -> Vec { +// If `token` is an include path (`include "lib.circom";`), jump to that file's URL. Routed here +// (not the resolver) because a `CircomString` carries a path, not a symbol name. Same resolution +// order as `load_include` so the jump target and the load always agree. +pub fn include_target_location(file_db: &FileDB, token: &SyntaxToken, vfs: &Vfs) -> Vec { let Some(include_stmt) = token_ancestors(token).find_map(AstInclude::cast) else { return Vec::new(); }; @@ -54,10 +52,8 @@ pub fn jump_to_lib(file_db: &FileDB, token: &SyntaxToken, vfs: &Vfs) -> Vec Vec Vec Vec { + let id = state + .source_db + .id_for_url(url) + .expect("document registered"); + let file_db = state.source_db.file_db(id); + let ast = AstCircomProgram::cast(syntax_tree(source)).expect("parses to a program"); + let token = ast + .syntax() + .descendants_with_tokens() + .filter_map(|e| e.into_token()) + .filter(|t| t.kind() == TokenKind::Identifier && t.text() == name) + .nth(occurrence) + .unwrap_or_else(|| panic!("token {name}#{occurrence} not found")); + let Some(ctx) = state.cursor_context(url, file_db.position(token.text_range().start())) + else { + return Vec::new(); + }; + let Some(tok) = token_at_offset(&ctx.ast, ctx.offset) else { + return Vec::new(); + }; + state.lookup_definition(&ctx.file_db, &tok) + } + /// Goto-definition from the template reference inside `component main = X()` resolves to `X`'s /// definition **in the same file** (the in-file `lookup_top_level` path; unaffected by the gate). #[test] @@ -389,12 +418,12 @@ mod tests { let _ = fs::remove_dir_all(&base); } - /// `jump_to_lib` on a **non-sibling** include (`include "lib.circom"` where lib lives under + /// `include_target_location` on a **non-sibling** include (`include "lib.circom"` where lib lives under /// `circuits/`) resolves to the indexed lib's URL via the basename fallback after /// `index_workspace`. The same-dir lookup misses (no sibling), so the project index must drive /// the jump — and the jump target must agree with `load_include`'s resolution. #[test] - fn jump_to_lib_nonsibling_via_index_test() { + fn include_target_location_nonsibling_via_index_test() { use std::fs; let base = std::env::temp_dir().join(format!("ccls_jump_ns_{}", std::process::id())); @@ -423,7 +452,7 @@ mod tests { .find(|t| t.kind() == TokenKind::CircomString) .expect("include path string present"); - let locs = super::jump_to_lib(&file_db, &token, state.source_db.vfs()); + let locs = super::include_target_location(&file_db, &token, state.source_db.vfs()); assert_eq!( locs.len(), 1, @@ -437,4 +466,111 @@ mod tests { let _ = fs::remove_dir_all(&base); } + + /// Repro: open A, jump to its include B, then goto-def a component usage *inside* B. B defines + /// the template it uses (in-file resolve). Drives the real `cursor_context` gate. The component + /// usage lives inside a template body (valid circom — top-level instantiation is `main` only). + #[test] + fn goto_def_inside_included_file_infile_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_repro_infile_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let b_src = "pragma circom 2.0.0;\ntemplate Comp() {\n signal output o;\n o <== 0;\n}\ntemplate Foo() {\n component c = Comp();\n}\n"; + fs::write(ws.join("B.circom"), b_src).unwrap(); + let a_src = "pragma circom 2.0.0;\ninclude \"B.circom\";\n"; + let a_path = ws.join("A.circom"); + fs::write(&a_path, a_src).unwrap(); + let a_url = Url::from_file_path(&a_path).unwrap(); + let b_url = Url::from_file_path(ws.join("B.circom")).unwrap(); + + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + state.source_db.set_document(&a_url, a_src.to_string()); + state.source_db.load_include(&a_url, "B.circom"); + state.source_db.set_document(&b_url, b_src.to_string()); + + // `Comp` occurrences in B: [0]=decl, [1]=the `component c = Comp()` usage inside `Foo`. + let locs = goto_at(&state, &b_url, b_src, "Comp", 1); + assert_eq!( + locs.len(), + 1, + "in-file goto-def inside the included B: {locs:?}" + ); + assert_eq!(locs[0].uri, b_url, "resolves within B"); + + let _ = fs::remove_dir_all(&base); + } + + /// Repro (transitive): A includes B; B includes C; C defines `Comp`; a template in B uses it. + /// After opening A then B, goto-def `Comp` inside B must resolve across B's include to C. + #[test] + fn goto_def_inside_included_file_transitive_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_repro_xfile_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let c_src = + "pragma circom 2.0.0;\ntemplate Comp() {\n signal output o;\n o <== 0;\n}\n"; + fs::write(ws.join("C.circom"), c_src).unwrap(); + let b_src = "pragma circom 2.0.0;\ninclude \"C.circom\";\ntemplate Foo() {\n component c = Comp();\n}\n"; + fs::write(ws.join("B.circom"), b_src).unwrap(); + let a_src = "pragma circom 2.0.0;\ninclude \"B.circom\";\n"; + let a_path = ws.join("A.circom"); + fs::write(&a_path, a_src).unwrap(); + let a_url = Url::from_file_path(&a_path).unwrap(); + let b_url = Url::from_file_path(ws.join("B.circom")).unwrap(); + let c_url = Url::from_file_path(ws.join("C.circom")).unwrap(); + + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + state.source_db.set_document(&a_url, a_src.to_string()); + state.source_db.load_include(&a_url, "B.circom"); + state.source_db.set_document(&b_url, b_src.to_string()); + state.source_db.load_include(&b_url, "C.circom"); + + // Only one `Comp` in B — the usage inside `Foo` (occurrence 0). + let locs = goto_at(&state, &b_url, b_src, "Comp", 0); + assert_eq!(locs.len(), 1, "transitive goto-def inside B: {locs:?}"); + assert_eq!(locs[0].uri, c_url, "resolves across B's include to C"); + + let _ = fs::remove_dir_all(&base); + } + + /// Repro (transitive, B viewed but NOT didOpen'd): A includes B; B includes C; C defines `Comp`; + /// a template in B uses it. Open A only (loads B as its include, but NOT B's include C). Goto-def + /// `Comp` inside B should still resolve — B's includes must load on demand. + #[test] + fn goto_def_inside_included_file_transitive_lazy_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_repro_lazy_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let c_src = + "pragma circom 2.0.0;\ntemplate Comp() {\n signal output o;\n o <== 0;\n}\n"; + fs::write(ws.join("C.circom"), c_src).unwrap(); + let b_src = "pragma circom 2.0.0;\ninclude \"C.circom\";\ntemplate Foo() {\n component c = Comp();\n}\n"; + fs::write(ws.join("B.circom"), b_src).unwrap(); + let a_src = "pragma circom 2.0.0;\ninclude \"B.circom\";\n"; + let a_path = ws.join("A.circom"); + fs::write(&a_path, a_src).unwrap(); + let a_url = Url::from_file_path(&a_path).unwrap(); + let b_url = Url::from_file_path(ws.join("B.circom")).unwrap(); + let c_url = Url::from_file_path(ws.join("C.circom")).unwrap(); + + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + // Open A only — B is loaded (as A's include) but its include C is NOT loaded. + state.source_db.set_document(&a_url, a_src.to_string()); + state.source_db.load_include(&a_url, "B.circom"); + + let locs = goto_at(&state, &b_url, b_src, "Comp", 0); + assert_eq!(locs.len(), 1, "lazy transitive goto-def inside B: {locs:?}"); + assert_eq!( + locs[0].uri, c_url, + "B's includes load on demand → resolves to C" + ); + + let _ = fs::remove_dir_all(&base); + } } diff --git a/crates/lsp/src/handler/hover.rs b/crates/lsp/src/handler/hover.rs index 09c3a5f..27a0fa7 100644 --- a/crates/lsp/src/handler/hover.rs +++ b/crates/lsp/src/handler/hover.rs @@ -1,10 +1,11 @@ //! Hover: show the declaration of the symbol under the cursor. //! //! Rides the shared resolution core ([`GlobalState::cursor_context`] + -//! [`GlobalState::resolve_use`]). For a resolved identifier (a declared name or a usage of one) it -//! returns the symbol's kind and its declaration signature (the source text of the defining node, -//! trimmed to the header for block-bodied defs). Member-access fields (`c.x`) aren't resolved by the -//! flat resolver and yield `None` (consistent with rename/references). +//! [`GlobalState::resolve_token`]). For a resolved identifier (a declared name or a usage of one) +//! it returns the symbol's kind and its declaration signature (the source text of the defining node, +//! trimmed to the header for block-bodied defs). Member-access fields (`c.x`) ARE resolved here +//! (via `resolve_token` → `resolve_member`), unlike references/rename, which intentionally ride +//! `resolve_use` and do not handle fields. use anyhow::Result; use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind}; diff --git a/crates/lsp/src/handler/references.rs b/crates/lsp/src/handler/references.rs index 4a6636b..ad164c6 100644 --- a/crates/lsp/src/handler/references.rs +++ b/crates/lsp/src/handler/references.rs @@ -1,21 +1,24 @@ //! "Find references": every occurrence of the symbol under the cursor. //! -//! Rides the same shared core as rename ([`GlobalState::resolve_use`] + -//! [`GlobalState::find_occurrences`]). In-file by design (the symbol's defining file); cross-file -//! references are a follow-up. +//! Rides [`GlobalState::resolve_visible`] + [`GlobalState::workspace_occurrences`]. Workspace-wide +//! and resolution-based: a token is a reference only if it resolves to the target symbol, so +//! shadowing and same-name collisions across files are handled. use anyhow::Result; use lsp_types::{Location, ReferenceParams}; use crate::global_state::GlobalState; use crate::resolver::identifier_at; +use crate::source_db::SourceDatabase; -/// Entry point for the `textDocument/references` request. Returns the declaration plus every -/// in-scope use as `Location`s (file-tagged), or `None` if the cursor isn't on a referenceable -/// identifier or the file is unknown. Never errors. +/// Entry point for the `textDocument/references` request. Returns the declaration (unless +/// `context.include_declaration` is false) plus every reference across the workspace as +/// `Location`s, or `None` if the cursor isn't on a referenceable identifier or the file is +/// unknown. Never errors. pub fn handle(state: &GlobalState, params: ReferenceParams) -> Result>> { let uri = params.text_document_position.text_document.uri; let position = params.text_document_position.position; + let include_declaration = params.context.include_declaration; let Some(ctx) = state.cursor_context(&uri, position) else { return Ok(None); @@ -24,16 +27,26 @@ pub fn handle(state: &GlobalState, params: ReferenceParams) -> Result = ranges + let (def_file, def_range) = (target.0, target.1.def_range); + let mut locations: Vec = state + .workspace_occurrences(&target, ctx.id) .into_iter() - .map(|r| Location::new(file_uri.clone(), r)) + .filter(|(f, r)| include_declaration || !(*f == def_file && *r == def_range)) + .map(|(f, r)| Location::new(state.source_db.file_db(f).file_path.clone(), r)) .collect(); + locations.sort_by(|a, b| { + a.uri + .cmp(&b.uri) + .then_with(|| a.range.start.cmp(&b.range.start)) + }); Ok(Some(locations)) } @@ -107,4 +120,350 @@ mod tests { occurrences.len() ); } + + use lsp_types::{ + Location, Position, ReferenceContext, ReferenceParams, TextDocumentIdentifier, + TextDocumentPositionParams, + }; + + use crate::global_state::GlobalState; + use crate::test_util::position_of; + + /// Drive the real `textDocument/references` handler (cursor → resolve → workspace scan). + fn refs( + state: &GlobalState, + url: &Url, + position: Position, + include_declaration: bool, + ) -> Vec { + super::handle( + state, + ReferenceParams { + text_document_position: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: url.clone() }, + position, + }, + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + context: ReferenceContext { + include_declaration, + }, + }, + ) + .unwrap() + .unwrap_or_default() + } + + fn basenames(locs: &[Location]) -> Vec { + locs.iter() + .filter_map(|l| { + l.uri + .to_file_path() + .ok() + .and_then(|p| p.file_name().and_then(|n| n.to_str()).map(String::from)) + }) + .collect() + } + + /// Cross-file references: `Lib` is defined in `lib.circom`, used in `main.circom` and + /// `other.circom`. References from a usage in main find the def in lib plus both usages — the + /// core workspace-graph win (previously in-file only, missing both other files). + #[test] + fn references_span_workspace_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_refs_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let lib_src = + "pragma circom 2.0.0;\ntemplate Lib() {\n signal output o;\n o <== 0;\n}\n"; + let main_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\ntemplate Main() {\n component c = Lib();\n}\n"; + let other_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\ntemplate Other() {\n component d = Lib();\n}\n"; + fs::write(ws.join("lib.circom"), lib_src).unwrap(); + fs::write(ws.join("main.circom"), main_src).unwrap(); + fs::write(ws.join("other.circom"), other_src).unwrap(); + + let main_url = Url::from_file_path(ws.join("main.circom").canonicalize().unwrap()).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + state + .source_db + .set_document(&main_url, main_src.to_string()); + + // Cursor on the `Lib` usage in main (`component c = Lib()`). + let locs = refs(&state, &main_url, position_of(main_src, "Lib", 0), true); + assert_eq!( + locs.len(), + 3, + "def in lib + usage in main + usage in other: {locs:?}" + ); + let names = basenames(&locs); + assert!(names.contains(&"lib.circom".to_string()), "{names:?}"); + assert!(names.contains(&"main.circom".to_string()), "{names:?}"); + assert!(names.contains(&"other.circom".to_string()), "{names:?}"); + + let _ = fs::remove_dir_all(&base); + } + + /// Same-name symbols in two files don't collide: `template Foo` exists independently in + /// `a.circom` and `b.circom`, neither including the other. References of `Foo` in A find only + /// A's occurrences; B's `Foo` is a distinct symbol (different `FileId`). + #[test] + fn references_same_name_distinct_files_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_refs2_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let src = "pragma circom 2.0.0;\ntemplate Foo() {\n signal output o;\n o <== 0;\n}\n"; + let a_path = ws.join("a.circom"); + let b_path = ws.join("b.circom"); + fs::write(&a_path, src).unwrap(); + fs::write(&b_path, src).unwrap(); + + let a_url = Url::from_file_path(a_path.canonicalize().unwrap()).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + state.source_db.set_document(&a_url, src.to_string()); + + // Cursor on `Foo`'s definition name in A. + let locs = refs(&state, &a_url, position_of(src, "Foo", 0), true); + let in_a = locs.iter().filter(|l| l.uri == a_url).count(); + let names = basenames(&locs); + assert_eq!(locs.len(), 1, "only A's `Foo` name token: {locs:?}"); + assert_eq!(in_a, 1, "the single occurrence is in A"); + assert!( + !names.contains(&"b.circom".to_string()), + "B's same-named Foo must not be referenced: {names:?}" + ); + + let _ = fs::remove_dir_all(&base); + } + + /// Shadowing: when `main.circom` defines `template Foo` *and* its include `lib.circom` also + /// defines `template Foo`, references of `Foo` from main resolve only to main's def — the + /// include's same-named def is shadowed and never reported. + #[test] + fn references_shadow_include_same_name_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_refs3_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let lib_src = + "pragma circom 2.0.0;\ntemplate Foo() {\n signal output o;\n o <== 0;\n}\n"; + let main_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\ntemplate Foo() {\n signal output out;\n out <== 0;\n}\n"; + fs::write(ws.join("lib.circom"), lib_src).unwrap(); + fs::write(ws.join("main.circom"), main_src).unwrap(); + + let main_url = Url::from_file_path(ws.join("main.circom").canonicalize().unwrap()).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + state + .source_db + .set_document(&main_url, main_src.to_string()); + + // Cursor on main's `Foo` definition name (occurrence 0). + let locs = refs(&state, &main_url, position_of(main_src, "Foo", 0), true); + let names = basenames(&locs); + assert!( + names.iter().all(|n| n == "main.circom"), + "shadowed include's Foo must not appear: {names:?}" + ); + assert!( + !names.contains(&"lib.circom".to_string()), + "lib's Foo is shadowed by main's: {names:?}" + ); + + let _ = fs::remove_dir_all(&base); + } + + /// `context.include_declaration = false` omits the declaration range (the def in lib), keeping + /// only the usages. + #[test] + fn references_exclude_declaration_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_refs4_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let lib_src = + "pragma circom 2.0.0;\ntemplate Lib() {\n signal output o;\n o <== 0;\n}\n"; + let main_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\ntemplate Main() {\n component c = Lib();\n}\n"; + fs::write(ws.join("lib.circom"), lib_src).unwrap(); + fs::write(ws.join("main.circom"), main_src).unwrap(); + + let main_url = Url::from_file_path(ws.join("main.circom").canonicalize().unwrap()).unwrap(); + let lib_url = Url::from_file_path(ws.join("lib.circom").canonicalize().unwrap()).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + state + .source_db + .set_document(&main_url, main_src.to_string()); + + let with_decl = refs(&state, &main_url, position_of(main_src, "Lib", 0), true); + let no_decl = refs(&state, &main_url, position_of(main_src, "Lib", 0), false); + assert_eq!(with_decl.len(), 2, "decl in lib + usage in main"); + assert_eq!(no_decl.len(), 1, "declaration excluded"); + assert!( + no_decl.iter().all(|l| l.uri != lib_url), + "the lib declaration is excluded when include_declaration=false" + ); + + let _ = fs::remove_dir_all(&base); + } + + /// A **function** (non-component top-level symbol) defined in an include and called in the + /// includer's body. This is the exact gap [`GlobalState::resolve_visible`] closes: the old gated + /// `resolve_use` only crossed files for component decls/calls, so a `helper(a)` call inside a + /// template body never resolved across the include. References from the call find the def in lib + /// plus the call in main. + #[test] + fn references_cross_file_function_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_refs_fn_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let lib_src = "pragma circom 2.0.0;\nfunction helper(x) {\n return x;\n}\n"; + let main_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\ntemplate Main() {\n signal input a;\n signal output c;\n c <== helper(a);\n}\n"; + fs::write(ws.join("lib.circom"), lib_src).unwrap(); + fs::write(ws.join("main.circom"), main_src).unwrap(); + + let lib_url = Url::from_file_path(ws.join("lib.circom").canonicalize().unwrap()).unwrap(); + let main_url = Url::from_file_path(ws.join("main.circom").canonicalize().unwrap()).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + state + .source_db + .set_document(&main_url, main_src.to_string()); + + // Cursor on `helper(` call in main. + let locs = refs(&state, &main_url, position_of(main_src, "helper", 0), true); + assert_eq!(locs.len(), 2, "def in lib + call in main: {locs:?}"); + let names = basenames(&locs); + assert!(names.contains(&"lib.circom".to_string())); + assert!(names.contains(&"main.circom".to_string())); + let _ = (lib_url, fs::remove_dir_all(&base)); + } + + /// Real circomlib scenario: references for `Num2Bits` in `bitify.circom` (which includes + /// `comparators.circom` + `aliascheck.circom`). `Num2Bits` is defined and instantiated in the + /// same file → 2 in-file occurrences. Also covers the exit-101 panic fix: `comparators.circom` + /// carries a multi-byte box-drawing char, and an `IsZero` reference (defined in comparators, + /// used in bitify) computes a range on that multi-byte file via `position()`. + #[test] + fn bitify_num2bits_references_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_bitify_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + + // `─` (U+2500) in the header → multi-byte content that panicked `position()` before the fix. + let comparators_src = "/* ─── comparators.circom ─── */\npragma circom 2.0.0;\n\ntemplate IsZero() {\n signal input in;\n signal output out;\n out <== 0;\n}\n"; + let aliascheck_src = + "pragma circom 2.0.0;\n\ntemplate AliasCheck() {\n signal input in[254];\n}\n"; + let bitify_src = r#"pragma circom 2.0.0; + +include "comparators.circom"; +include "aliascheck.circom"; + + +template Num2Bits(n) { + signal input in; + signal output out[n]; + var lc1=0; + + var e2=1; + for (var i = 0; i> i) & 1; + out[i] * (out[i] -1 ) === 0; + lc1 += out[i] * e2; + e2 = e2+e2; + } + + lc1 === in; +} + +template Num2Bits_strict() { + signal input in; + signal output out[254]; + + component aliasCheck = AliasCheck(); + component n2b = Num2Bits(254); + in ==> n2b.in; + + for (var i=0; i<254; i++) { + n2b.out[i] ==> out[i]; + n2b.out[i] ==> aliasCheck.in[i]; + } +} + +template Num2BitsNeg(n) { + signal input in; + signal output out[n]; + var lc1=0; + + component isZero; + + isZero = IsZero(); + + var neg = n == 0 ? 0 : 2**n - in; + + for (var i = 0; i> i) & 1; + out[i] * (out[i] -1 ) === 0; + lc1 += out[i] * 2**i; + } + + in ==> isZero.in; + + + + lc1 + isZero.out * 2**n === 2**n - in; +} +"#; + fs::write(ws.join("comparators.circom"), comparators_src).unwrap(); + fs::write(ws.join("aliascheck.circom"), aliascheck_src).unwrap(); + fs::write(ws.join("bitify.circom"), bitify_src).unwrap(); + + let bitify_url = + Url::from_file_path(ws.join("bitify.circom").canonicalize().unwrap()).unwrap(); + let comparators_url = + Url::from_file_path(ws.join("comparators.circom").canonicalize().unwrap()).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + state + .source_db + .set_document(&bitify_url, bitify_src.to_string()); + + // References of `Num2Bits` from its definition: def name + the `Num2Bits(254)` instantiation. + let num2bits = refs( + &state, + &bitify_url, + position_of(bitify_src, "Num2Bits", 0), + true, + ); + assert_eq!(num2bits.len(), 2, "def + instantiation: {num2bits:?}"); + assert!( + num2bits.iter().all(|l| l.uri == bitify_url), + "both occurrences are in bitify.circom" + ); + + // References of `IsZero` from its usage in bitify: def in comparators + usage in bitify. + // Computing comparators' range exercises `position()` on the multi-byte file (panic fix). + let iszero = refs( + &state, + &bitify_url, + position_of(bitify_src, "IsZero", 0), + true, + ); + assert_eq!( + iszero.len(), + 2, + "def in comparators + usage in bitify: {iszero:?}" + ); + let names = basenames(&iszero); + assert!( + names.contains(&"comparators.circom".to_string()), + "{names:?}" + ); + assert!(names.contains(&"bitify.circom".to_string()), "{names:?}"); + let _ = (comparators_url, fs::remove_dir_all(&base)); + } } diff --git a/crates/lsp/src/handler/rename.rs b/crates/lsp/src/handler/rename.rs index 91a1b2e..827abb4 100644 --- a/crates/lsp/src/handler/rename.rs +++ b/crates/lsp/src/handler/rename.rs @@ -1,8 +1,8 @@ //! Symbol rename: rewrite every occurrence of the symbol under the cursor to `new_name`. //! -//! Rides [`GlobalState::resolve_use`] + [`GlobalState::find_occurrences`] — occurrences are found -//! by *resolving* each candidate (not text-matching), so shadowing is correct. In-file (the -//! symbol's defining file); cross-file rename is a follow-up. +//! Rides [`GlobalState::resolve_visible`] + [`GlobalState::workspace_occurrences`] — occurrences are +//! found by *resolving* each candidate (not text-matching), so shadowing is correct, and span the +//! whole workspace (cross-file rename groups edits by file URI into a single `WorkspaceEdit`). use std::collections::HashMap; @@ -17,10 +17,12 @@ use parser::token_kind::TokenKind; use crate::file_db::FileId; use crate::global_state::{CursorContext, GlobalState}; use crate::resolver::{identifier_at, ResolvedSymbol}; +use crate::source_db::SourceDatabase; use syntax::node::SyntaxToken; /// Entry point for `textDocument/rename`. Returns `None` (no edits) when the cursor isn't on a /// renamable `Identifier`, `new_name` isn't a legal circom identifier, or the file is unknown. +/// Otherwise returns a `WorkspaceEdit` whose `changes` span every file referencing the symbol. pub fn handle(state: &GlobalState, params: RenameParams) -> Result> { let uri = params.text_document_position.text_document.uri; let position = params.text_document_position.position; @@ -30,23 +32,19 @@ pub fn handle(state: &GlobalState, params: RenameParams) -> Result = ranges - .into_iter() - .map(|range| TextEdit { + let mut changes: HashMap> = HashMap::new(); + for (f, range) in state.workspace_occurrences(&target, ctx.id) { + let file_uri = state.source_db.file_db(f).file_path.clone(); + changes.entry(file_uri).or_default().push(TextEdit { range, new_text: new_name.clone(), - }) - .collect(); + }); + } - let changes = HashMap::from([(file_uri, edits)]); Ok(Some(WorkspaceEdit { changes: Some(changes), document_changes: None, @@ -65,7 +63,10 @@ fn renamable_cursor( ) -> Option<(CursorContext, SyntaxToken, (FileId, ResolvedSymbol))> { let ctx = state.cursor_context(uri, position)?; let token = identifier_at(&ctx.ast, ctx.offset)?; - let target = state.resolve_use(&ctx.file_db, &token).into_iter().next()?; + let target = state + .resolve_visible(&ctx.file_db, &token) + .into_iter() + .next()?; Some((ctx, token, target)) } @@ -272,9 +273,9 @@ mod tests { ); } - /// Renaming is in-file: two documents each defining `template T()` at the same line:column must - /// not collide — renaming in file A edits only file A, never file B. Pins the fix for the - /// cross-file `def_range` collision the all-files search used to have. + /// Renaming a symbol in one file must not touch a same-named symbol in a sibling file. + /// Resolution-based: B's `template T()` resolves to its own `(b, T)`, not A's, so it is never + /// matched. Pins the cross-file correctness that a naive name search would break. #[test] fn rename_does_not_touch_other_files_test() { let src = "pragma circom 2.0.0;\ntemplate T() { signal output o; o <== 0; }\n"; @@ -298,6 +299,51 @@ mod tests { ); } + /// Cross-file rename: `Lib` is defined in `lib.circom` and instantiated in `main.circom`. + /// Renaming from the usage in main edits **both** files — the def in lib and the usage in main + /// — grouped by URI into one `WorkspaceEdit`. + #[test] + fn rename_spans_workspace_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_rename_x_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let lib_src = + "pragma circom 2.0.0;\ntemplate Lib() {\n signal output o;\n o <== 0;\n}\n"; + let main_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\ntemplate Main() {\n component c = Lib();\n}\n"; + fs::write(ws.join("lib.circom"), lib_src).unwrap(); + fs::write(ws.join("main.circom"), main_src).unwrap(); + + let lib_url = Url::from_file_path(ws.join("lib.circom").canonicalize().unwrap()).unwrap(); + let main_url = Url::from_file_path(ws.join("main.circom").canonicalize().unwrap()).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + state + .source_db + .set_document(&main_url, main_src.to_string()); + + // Cursor on the `Lib` usage in main. + let changes = rename(&state, &main_url, position_of(main_src, "Lib", 0), "NewLib") + .expect("rename produces an edit") + .changes + .unwrap(); + + assert_eq!(changes.len(), 2, "edits span lib + main: {changes:?}"); + let lib_edits = changes.get(&lib_url).expect("lib edited (its def)"); + let main_edits = changes.get(&main_url).expect("main edited (its usage)"); + assert_eq!(lib_edits.len(), 1, "lib's `Lib` def name"); + assert_eq!(main_edits.len(), 1, "main's `Lib` usage"); + assert!( + lib_edits + .iter() + .chain(main_edits.iter()) + .all(|e| e.new_text == "NewLib"), + "all edits use the new name" + ); + + let _ = fs::remove_dir_all(&base); + } + /// Renaming a loop variable (`var i` in `for (var i = …; i < N; i++)`) finds all occurrences: /// the declaration in the for-init, the condition `i < N`, the increment `i++`, and usages in /// the loop body. Regression for the `find_children` → `descendants` fix (loop vars were diff --git a/crates/lsp/src/handler/workspace_symbol.rs b/crates/lsp/src/handler/workspace_symbol.rs new file mode 100644 index 0000000..c962efc --- /dev/null +++ b/crates/lsp/src/handler/workspace_symbol.rs @@ -0,0 +1,126 @@ +//! Workspace-wide symbol search (`workspace/symbol`): every template/function/bus across the +//! workspace whose name matches the query. Cheap now that the workspace is eagerly indexed — each +//! file's `SymbolTable` is pre-warmed. + +use anyhow::Result; +use lsp_types::{ + SymbolInformation, SymbolKind as LspSymbolKind, WorkspaceSymbolParams, WorkspaceSymbolResponse, +}; + +use crate::global_state::GlobalState; +use crate::source_db::SourceDatabase; +use crate::symbol_table::SymbolKind; + +/// Entry point for `workspace/symbol`. Returns matching top-level symbols as flat +/// `SymbolInformation`s (location = the symbol's name token in its file), or `None` for an empty +/// result. Never errors. +#[allow(deprecated)] // `SymbolInformation::deprecated` is required by lsp-types 0.94 (use `tags`). +pub fn handle( + state: &GlobalState, + params: WorkspaceSymbolParams, +) -> Result> { + let symbols = state.workspace_symbols(¶ms.query); + let infos: Vec = symbols + .into_iter() + .map(|(id, sym)| SymbolInformation { + name: sym.name, + kind: to_lsp_kind(sym.kind), + tags: None, + deprecated: None, + location: lsp_types::Location { + uri: state.source_db.file_db(id).file_path.clone(), + range: sym.def_range, + }, + container_name: None, + }) + .collect(); + Ok(if infos.is_empty() { + None + } else { + Some(WorkspaceSymbolResponse::Flat(infos)) + }) +} + +/// Map a circom [`SymbolKind`] to the closest LSP [`LspSymbolKind`]. +fn to_lsp_kind(kind: SymbolKind) -> LspSymbolKind { + match kind { + SymbolKind::Template => LspSymbolKind::CLASS, + SymbolKind::Function => LspSymbolKind::FUNCTION, + SymbolKind::Bus => LspSymbolKind::STRUCT, + _ => LspSymbolKind::VARIABLE, + } +} + +#[cfg(test)] +mod tests { + use std::fs; + + use lsp_types::{WorkspaceSymbolParams, WorkspaceSymbolResponse}; + + use crate::global_state::GlobalState; + + use super::handle; + + fn names(state: &GlobalState, query: &str) -> Vec { + let resp = handle( + state, + WorkspaceSymbolParams { + query: query.to_string(), + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + }, + ) + .unwrap(); + match resp { + Some(WorkspaceSymbolResponse::Flat(infos)) => { + infos.into_iter().map(|i| i.name).collect() + } + _ => Vec::new(), + } + } + + /// `workspace/symbol` lists every top-level template/function across the workspace when the + /// query is blank, and filters case-insensitively otherwise. Bodies (signals/vars) are excluded. + #[test] + fn workspace_symbol_lists_and_filters_test() { + let base = std::env::temp_dir().join(format!("ccls_ws_sym_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let lib_src = "pragma circom 2.0.0;\ntemplate Lib() {\n signal output o;\n o <== 0;\n}\nfunction helper(x) {\n return x;\n}\n"; + let main_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\ntemplate Main() {\n signal output out;\n out <== 0;\n}\n"; + fs::write(ws.join("lib.circom"), lib_src).unwrap(); + fs::write(ws.join("main.circom"), main_src).unwrap(); + + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + state.index_workspace(); + + let all = names(&state, ""); + assert!(all.contains(&"Lib".to_string()), "Lib listed: {all:?}"); + assert!(all.contains(&"Main".to_string()), "Main listed: {all:?}"); + assert!( + all.contains(&"helper".to_string()), + "function helper listed: {all:?}" + ); + // Body-only symbols (signals) are not top-level, so never listed. + assert!( + !all.iter().any(|n| n == "o" || n == "out"), + "signals are not workspace symbols: {all:?}" + ); + + // Case-insensitive substring filter. + let only_lib = names(&state, "lib"); + assert_eq!(only_lib, vec!["Lib".to_string()], "filter to Lib"); + + let only_main = names(&state, "MA"); + assert_eq!( + only_main, + vec!["Main".to_string()], + "case-insensitive match" + ); + + // No match ⇒ empty result (`None` from the handler). + assert!(names(&state, "zzz").is_empty(), "no match yields empty"); + + let _ = fs::remove_dir_all(&base); + } +} diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs index 209e8dd..cf9564a 100644 --- a/crates/lsp/src/lib.rs +++ b/crates/lsp/src/lib.rs @@ -15,6 +15,7 @@ use std::error::Error; use std::path::PathBuf; use lsp_server::{Connection, Message, Request, RequestId}; +use lsp_types::notification::Notification; use lsp_types::{ CompletionOptions, HoverProviderCapability, InitializeParams, OneOf, ServerCapabilities, TextDocumentSyncCapability, TextDocumentSyncKind, @@ -69,6 +70,7 @@ fn server_capabilities() -> ServerCapabilities { prepare_provider: Some(true), work_done_progress_options: Default::default(), })), + workspace_symbol_provider: Some(OneOf::Left(true)), ..Default::default() } } @@ -81,41 +83,33 @@ fn main_loop( ) -> Result<(), Box> { let params: InitializeParams = serde_json::from_value(params)?; - // Capture workspace roots so `include` resolution can be confined to them (path-traversal - // defense). Without roots the server refuses to load any include rather than read arbitrarily. + // Roots confine include resolution (path-traversal defense); empty roots ⇒ no include loads. let roots = workspace_roots(¶ms); let mut state = GlobalState::new(roots.clone()); - // Background the workspace walk so `initialize` never blocks on I/O (a large `node_modules` can - // take seconds). The thread only does the I/O-heavy `collect_circom_files`; it hands the - // canonical paths back on a side channel, and the main loop registers them between messages. - // The server is responsive immediately; non-sibling includes resolve once the index lands, and - // same-dir includes work from the start (never gated on the index). - let (index_tx, index_rx) = std::sync::mpsc::channel::>(); + // Background the walk so `initialize` never blocks on I/O; the thread collects canonical paths + // **and reads file content** (pure I/O — text interning stays on the main thread), and the main + // loop interns it between messages. Same-dir includes work immediately; non-sibling ones resolve + // once the index lands. Eager-loading all text lets workspace references/rename/symbol scan + // every file. + let (index_tx, index_rx) = std::sync::mpsc::channel::>(); if !roots.is_empty() { let walk_roots = roots.clone(); std::thread::spawn(move || { - let paths = crate::project_index::collect_circom_files(&walk_roots); - // The only error is the receiver being gone (server shutting down); nothing to do then. - let _ = index_tx.send(paths); + let entries = crate::project_index::collect_circom_files_with_content(&walk_roots); + let _ = index_tx.send(entries); // error ⇒ receiver gone (shutdown); ignore }); } - // If the client supports it, register a `**/*.circom` watcher per root so created/deleted - // files keep the index fresh without a full re-walk. Best-effort: a missing reply or - // unsupported client just leaves the index stale at the file level (refreshed on workspace - // folder changes); the `Message::Response(_)` no-op arm below absorbs the registration reply. - register_watched_files_capability(&connection, ¶ms, &roots)?; + let mut watcher_registered = false; for msg in &connection.receiver { - // Apply any completed background-walk results without blocking, before handling this - // message. `try_recv` returns immediately when nothing is ready yet. - while let Ok(paths) = index_rx.try_recv() { - state.register_indexed_paths(paths); + // Apply any completed walk results without blocking before handling this message. + while let Ok(entries) = index_rx.try_recv() { + state.register_indexed_paths(entries); } match msg { Message::Request(req) => { - // The `shutdown` request is handled by the transport itself. if connection.handle_shutdown(&req)? { return Ok(()); } @@ -125,6 +119,13 @@ fn main_loop( } Message::Response(_) => {} Message::Notification(not) => { + // Register the watcher after `initialized` (LSP servers must not send requests + // before it; a strict client would silently drop a pre-init registration). + if !watcher_registered && not.method == lsp_types::notification::Initialized::METHOD + { + watcher_registered = true; + register_watched_files_capability(&connection, ¶ms, &roots)?; + } state.handle_notification(not)?; } } @@ -165,12 +166,9 @@ fn workspace_roots(params: &InitializeParams) -> Vec { .collect() } -/// If the client advertised `workspace.didChangeWatchedFiles` dynamic-registration support, -/// register one `**/*.circom` file watcher per workspace root via a `client/registerCapability` -/// request. Best-effort: no-op if unsupported or if there are no roots. The request is sent on the -/// `connection.sender`; its (empty) reply hits the `Message::Response(_)` no-op arm in -/// [`main_loop`]. We build the params as JSON to stay independent of the proposed -/// `GlobPattern`/`RelativePattern` shape across `lsp_types` versions — the wire format is stable. +/// Register a `**/*.circom` watcher per root via `client/registerCapability`, if the client +/// supports dynamic registration. No-op if unsupported or no roots. Params are built as JSON to +/// avoid the proposed `GlobPattern` shape across `lsp_types` versions. fn register_watched_files_capability( connection: &Connection, params: &InitializeParams, diff --git a/crates/lsp/src/project_index.rs b/crates/lsp/src/project_index.rs index 2291f7c..ecf46e2 100644 --- a/crates/lsp/src/project_index.rs +++ b/crates/lsp/src/project_index.rs @@ -1,42 +1,41 @@ -//! Project-wide `.circom` file discovery + indexing (the only place in the server that walks the -//! filesystem). -//! -//! The pure basename index lives in [`vfs`]; this module is the I/O boundary that feeds it. At -//! `initialize` (and on workspace-folder changes) [`collect_circom_files`] walks each root once and -//! [`GlobalState::index_workspace`] interns every found path with **no text** (cheap — no file -//! reads). Text loads lazily, on demand, the first time an `include` resolves to that path. +//! Project-wide `.circom` file discovery + indexing — the only place that walks the filesystem. +//! `collect_circom_files` runs (backgrounded) at `initialize` and on workspace-folder changes; it +//! interns paths with no text, and text loads lazily on first `include` resolution. use std::path::PathBuf; +use std::sync::Arc; use vfs::VfsPath; use crate::global_state::GlobalState; +use crate::source_db::SourceDatabase; -/// Recursively collect every `*.circom` file under `roots`, returning each as a **canonical** -/// absolute path. Descends into `node_modules` (circomlib lives there), so this deliberately does -/// NOT use an ignore-respecting walker. Symlinks are not followed (`walkdir` default), which breaks -/// symlink loops; entries that fail to canonicalize (raced-away, permission-denied) are skipped. +/// `.git`/`target` never hold circom sources; `node_modules` is kept (circomlib lives there). +fn is_pruned_dir(file_name: &std::ffi::OsStr) -> bool { + matches!(file_name.to_str(), Some(".git") | Some("target")) +} + +/// Recursively collect every `*.circom` file under `roots` as canonical absolute paths. Descends +/// into `node_modules`, prunes `.git`/`target`, and doesn't follow symlinks. pub(crate) fn collect_circom_files(roots: &[PathBuf]) -> Vec { let mut out = Vec::new(); for root in roots { for entry in walkdir::WalkDir::new(root) .follow_links(false) .into_iter() + .filter_entry(|e| !is_pruned_dir(e.file_name())) .filter_map(|e| e.ok()) { let path = entry.path(); - // Use the type walkdir already determined (often via `d_type`, no extra syscall) instead - // of `path.is_file()` which issues a fresh `stat` per entry. `file_type()` also skips - // symlinks-to-files, consistent with `follow_links(false)` above. + // `file_type()` reuses walkdir's cached type (no extra `stat`) and skips symlinks. if !entry.file_type().is_file() { continue; } if path.extension().and_then(|e| e.to_str()) != Some("circom") { continue; } - // Canonicalize BEFORE interning: `VfsPath::from_abs_path` only *absolutizes* (lexical, - // no symlink/`..` resolution), so confinement's `starts_with` against canonical roots - // needs a real canonical path here. Skip entries that vanish mid-walk. + // Canonicalize here: `VfsPath::from_abs_path` only absolutizes, but confinement needs a + // real canonical path. Skip entries that vanish mid-walk. if let Ok(canon) = path.canonicalize() { out.push(canon); } @@ -45,32 +44,72 @@ pub(crate) fn collect_circom_files(roots: &[PathBuf]) -> Vec { out } +/// Walk `roots` and return each `*.circom` file as a canonical path **plus** its content. Pure I/O — +/// safe to run on the background thread (text interning stays on the main thread). Files that +/// vanish between the walk and the read are skipped. +pub(crate) fn collect_circom_files_with_content(roots: &[PathBuf]) -> Vec<(PathBuf, String)> { + let mut out = Vec::new(); + for canon in collect_circom_files(roots) { + if let Ok(content) = std::fs::read_to_string(&canon) { + out.push((canon, content)); + } + } + out +} + impl GlobalState { - /// Intern a pre-collected list of **canonical** `.circom` paths (path-only, no text) into the - /// basename index. The I/O-heavy collection ([`collect_circom_files`]) can run on a background - /// thread and hand its result here; this step is pure interning + cache flush. Idempotent — - /// re-registering existing paths never clobbers text a `load_include`/`didOpen` loaded. - pub(crate) fn register_indexed_paths(&mut self, paths: Vec) { - if paths.is_empty() { + /// Intern workspace `.circom` paths **with their text** (eager load so workspace occurrences/ + /// symbol can scan every file), record their ids, and re-load each open doc's includes — a + /// non-sibling include that missed at open time (walk hadn't landed) becomes resolvable now. + /// Idempotent; never clobbers loaded text. + pub(crate) fn register_indexed_paths(&mut self, entries: Vec<(PathBuf, String)>) { + if entries.is_empty() { return; } - for canon in paths { - if let Some(vpath) = VfsPath::from_abs_path(&canon) { - self.source_db.vfs_mut().register_path(vpath); - } + for (canon, content) in entries { + let Some(vpath) = VfsPath::from_abs_path(&canon) else { + continue; + }; + let id = self.source_db.vfs_mut().register_path(vpath.clone()); + self.workspace_files.insert(id); + self.source_db + .vfs_mut() + .set_file_contents(vpath, Some(Arc::from(content))); } - // register_path records no change-log entry, so this flush is usually a no-op; new files in - // the index can newly satisfy an include, so the resolved-include cache is dropped too. self.source_db.invalidate_changed(); + self.reload_open_doc_includes(); self.drop_include_cache(); } - /// Eagerly intern every `.circom` file under the **current** workspace roots (path-only, no - /// text). Used by tests and as the synchronous fallback; the live `main_loop` backgrounds the - /// walk and calls [`Self::register_indexed_paths`] instead so init never blocks on I/O. + /// Re-load includes for every open document. Includes are collected first so the AST borrow is + /// dropped before the mutable `load_include`. + fn reload_open_doc_includes(&mut self) { + let open: Vec = self.open_documents.iter().cloned().collect(); + for uri in open { + let Some(id) = self.source_db.id_for_url(&uri) else { + continue; + }; + let libs: Vec = self + .source_db + .ast(id) + .map(|a| { + a.libs() + .into_iter() + .filter_map(|i| i.lib().map(|l| l.value())) + .collect() + }) + .unwrap_or_default(); + for rel in libs { + let _ = self.source_db.load_include(&uri, &rel); + } + } + } + + /// Synchronous full re-walk of the current roots (tests / fallback). The live `main_loop` + /// backgrounds the walk and calls [`Self::register_indexed_paths`] instead. pub fn index_workspace(&mut self) { let roots: Vec = self.source_db.vfs().workspace_roots().to_vec(); - self.register_indexed_paths(collect_circom_files(&roots)); + self.register_indexed_paths(collect_circom_files_with_content(&roots)); } } @@ -78,6 +117,34 @@ impl GlobalState { mod tests { use super::*; + #[test] + fn pruned_dirs_are_skipped_test() { + let base = std::env::temp_dir().join(format!("ccls_prune_{}", std::process::id())); + let ws = base.join("ws"); + std::fs::create_dir_all(ws.join("target")).unwrap(); + std::fs::create_dir_all(ws.join(".git")).unwrap(); + std::fs::write(ws.join("main.circom"), "pragma circom 2.0.0;").unwrap(); + std::fs::write(ws.join("target/out.circom"), "pragma circom 2.0.0;").unwrap(); + std::fs::write(ws.join(".git/config.circom"), "pragma circom 2.0.0;").unwrap(); + + let found = collect_circom_files(std::slice::from_ref(&ws)); + let names: Vec = found + .iter() + .filter_map(|p| p.file_name().and_then(|n| n.to_str()).map(String::from)) + .collect(); + assert!( + names.contains(&"main.circom".to_string()), + "top-level kept: {names:?}" + ); + assert!( + !names + .iter() + .any(|n| n.starts_with("out") || n.starts_with("config")), + "target/.git pruned: {names:?}" + ); + let _ = std::fs::remove_dir_all(&base); + } + #[test] fn collect_circom_files_walks_and_canonicalizes_test() { let base = std::env::temp_dir().join(format!("ccls_walk_{}", std::process::id())); diff --git a/crates/lsp/src/resolver.rs b/crates/lsp/src/resolver.rs index 6e6e752..f020050 100644 --- a/crates/lsp/src/resolver.rs +++ b/crates/lsp/src/resolver.rs @@ -128,7 +128,7 @@ fn lookup_all<'a>( } /// Resolve `token` to its declaration(s) via [`lookup_all`]. Only `Identifier` tokens are resolved -/// here — `CircomString` include-paths are routed to `jump_to_lib` by the handler. +/// here — `CircomString` include-paths are routed to `include_target_location` by the handler. pub fn resolve(table: &SymbolTable, token: &SyntaxToken) -> Vec { let name = token.text(); let offset: TextSize = token.text_range().start(); @@ -148,6 +148,15 @@ fn resolves_to(table: &SymbolTable, token: &SyntaxToken, target: &ResolvedSymbol lookup_all(table, offset, name).any(hit) } +/// Every `Identifier` token in `root` whose text equals `name`, in document order. The candidate +/// set for [`occurrences_in`] and the workspace occurrence scan. +pub fn identifiers_named(root: &SyntaxNode, name: &str) -> Vec { + root.descendants_with_tokens() + .filter_map(|e| e.into_token()) + .filter(|t| t.kind() == TokenKind::Identifier && t.text() == name) + .collect() +} + /// Every `Identifier` token in `root` that resolves (against `table`) to `target` — the /// declaration plus all its in-scope usages, excluding shadowed same-named tokens. Document order. /// @@ -159,10 +168,8 @@ pub fn occurrences_in( table: &SymbolTable, target: &ResolvedSymbol, ) -> Vec { - let name = target.name.as_str(); - root.descendants_with_tokens() - .filter_map(|e| e.into_token()) - .filter(|t| t.kind() == TokenKind::Identifier && t.text() == name) + identifiers_named(root, &target.name) + .into_iter() .filter(|t| resolves_to(table, t, target)) .collect() } diff --git a/crates/lsp/src/source_db.rs b/crates/lsp/src/source_db.rs index 016c836..95ade56 100644 --- a/crates/lsp/src/source_db.rs +++ b/crates/lsp/src/source_db.rs @@ -7,7 +7,7 @@ //! resending unchanged text records no change (no reparse). use std::cell::RefCell; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use std::sync::Arc; @@ -83,30 +83,25 @@ impl ContentCacheDb { self.vfs.set_workspace_roots(roots); } - /// Read-only [`Vfs`] handle (e.g. so `jump_to_lib` applies the same containment check as + /// Read-only [`Vfs`] handle (e.g. so `include_target_location` applies the same containment check as /// `load_include`). pub(crate) fn vfs(&self) -> &Vfs { &self.vfs } - /// Mutable [`Vfs`] handle for the workspace walker (registers/unregisters project paths in the - /// basename index) and the file-watcher refresh. + /// Mutable [`Vfs`] handle for the workspace walker and file-watcher refresh. pub(crate) fn vfs_mut(&mut self) -> &mut Vfs { &mut self.vfs } - /// Convert a `file:` URL to its absolutized [`VfsPath`], or `None` for non-`file:` schemes or - /// non-absolutizable paths. Single source for the URI→path step so interning and lookup can't - /// disagree (a split would intern under one key and look up another, silently breaking - /// resolution). + /// `file:` URL → absolutized [`VfsPath`] (`None` for other schemes). Single source so interning + /// and lookup agree on the path key. fn url_to_vpath(url: &Url) -> Option { let path = url.to_file_path().ok()?; VfsPath::from_abs_path(&path) } /// Resolve a relative include `rel` against `parent_url`'s dir to an absolutized [`VfsPath`]. - /// Shared by [`Self::load_include`], [`Self::id_for_include`], and goto-def so all three agree - /// on the resolved path. fn resolve_include(parent_url: &Url, rel: &str) -> Option { let parent_path = parent_url.to_file_path().ok()?; let parent_dir = parent_path.parent()?; @@ -114,17 +109,15 @@ impl ContentCacheDb { VfsPath::from_abs_path(&lib_path) } - /// The `FileId` for `url`, from its absolutized path (so aliased paths collapse to one id). - /// `None` for non-`file:` schemes or non-absolutizable paths — callers skip indexing those. + /// The `FileId` for `url` (`None` for non-`file:` URIs). pub fn id_for_url(&self, url: &Url) -> Option { self.vfs.file_id(&Self::url_to_vpath(url)?) } - /// The already-interned `FileId` for an include, without reading disk. `None` if unresolvable - /// or not yet loaded. Resolution order: the **same-dir** [`VfsPath`] lookup first (unchanged — a - /// sibling include resolves without the index), then the **project-wide basename fallback** - /// ([`Vfs::find_include`]) so a non-sibling `include "X.circom"` still resolves when the eager - /// walk indexed `X.circom` elsewhere in the project. Pure — no disk I/O. + /// The already-interned `FileId` for an include, without reading disk. Same-dir lookup first, + /// then the project-wide basename fallback ([`Vfs::find_include`]). Pure — it does NOT confine + /// (no `canonicalize`); stale removed-folder includes are dropped at removal time instead + /// (`Vfs::unregister_under`). pub fn id_for_include(&self, parent_url: &Url, rel: &str) -> Option { if let Some(vpath) = Self::resolve_include(parent_url, rel) { if let Some(id) = self.vfs.file_id(&vpath) { @@ -145,43 +138,68 @@ impl ContentCacheDb { Some((id, changed)) } - /// Load a relative include from disk **once**, then serve the interned `FileId` from cache — a - /// keystroke in the main file never re-reads its includes. Resolution order (so a non-sibling - /// include resolves without regressing the sibling case): - /// 1. **Same-dir disk path** (unchanged): join `rel` onto the includer's dir, then - /// canonicalize + confine + read. - /// 2. On a miss, **project-wide basename fallback**: rank indexed files via - /// [`Vfs::find_include`], take the winner, canonicalize + confine + read it. - /// - /// `None` (skipped) for non-`file:` schemes, a missing/unreadable file, a non-absolutizable - /// path, or an include that escapes the workspace roots. + /// Load a relative include from disk once (then serve the cached id), and load its includes + /// transitively — so goto-def/hover *inside* an include (e.g. one opened via peek/jump without a + /// full didOpen) can still resolve across that include's own includes. Same-dir path first, then + /// the project-wide basename fallback. `None` for non-`file:` URIs, missing/unreadable files, or + /// includes escaping the workspace roots. pub fn load_include(&mut self, parent_url: &Url, rel: &str) -> Option { - // 1. Same-dir path first. + let id = self.load_one_include(parent_url, rel)?; + let mut visited = HashSet::new(); + visited.insert(id); + self.load_transitive_includes(id, &mut visited); + Some(id) + } + + /// Load a single include (no transitive closure): same-dir path first, then basename fallback. + fn load_one_include(&mut self, parent_url: &Url, rel: &str) -> Option { if let Some(vpath) = Self::resolve_include(parent_url, rel) { if let Some(id) = self.load_from_disk(&vpath) { return Some(id); } } - // 2. Basename fallback against the project index. let parent_vpath = Self::url_to_vpath(parent_url)?; let winner = self.vfs.find_include(&parent_vpath, rel)?; let winner_path = self.vfs.path(winner)?.clone(); self.load_from_disk(&winner_path) } - /// Canonicalize + confine + (read-if-needed) for one resolved include path — the single disk - /// boundary shared by the same-dir path and the basename-fallback winner. Returns the loaded - /// `FileId` (cached if already text-bearing), or `None` if the path can't be canonicalized, - /// escapes the workspace, or is unreadable. Security: `canonicalize` resolves `..`/`.`/symlinks; - /// `is_confined` is the pure prefix check. Escapes (`/etc/passwd`, `../../.ssh/id_rsa`, a root - /// symlink pointing out) are refused here so lookup/jump paths only ever surface confined files. + /// Recursively load `id`'s own includes (its include-closure) so resolution works from within + /// `id`. `visited` breaks include cycles; already-loaded files are served from cache (no re-read). + fn load_transitive_includes(&mut self, id: FileId, visited: &mut HashSet) { + let Some(path) = self.vfs.path(id) else { + return; + }; + let Ok(parent_url) = Url::from_file_path(path.as_path()) else { + return; + }; + let includes: Vec = self + .ast(id) + .map(|a| { + a.libs() + .into_iter() + .filter_map(|i| i.lib().map(|l| l.value())) + .collect() + }) + .unwrap_or_default(); + for rel in includes { + if let Some(child) = self.load_one_include(&parent_url, &rel) { + if visited.insert(child) { + self.load_transitive_includes(child, visited); + } + } + } + } + + /// Canonicalize + confine + read one include path — the single disk boundary. `canonicalize` + /// resolves `..`/symlinks; `is_confined` is the pure prefix check. Path-traversal escapes are + /// refused here, so the pure lookup/jump paths only ever surface already-confined files. fn load_from_disk(&mut self, vpath: &VfsPath) -> Option { let canonical = vpath.as_path().canonicalize().ok()?; if !self.vfs.is_confined(&canonical) { return None; } - // Already interned with text → serve the cached id (never re-read). A path interned with no - // text (by the workspace walk's `register_path`) falls through to the read. + // Serve the cached id if text is already loaded; a path-only id (from the walk) reads below. if let Some(id) = self.vfs.file_id(vpath) { if self.vfs.file_text(id).is_some() { return Some(id); @@ -195,10 +213,8 @@ impl ContentCacheDb { Some(id) } - /// Drain the VFS change log and drop every changed id's derived caches (others untouched). - /// `pub(crate)` so the workspace walker / file-watcher refresh can flush a `Delete` (from - /// `unregister_path`) after mutating the index. `parse_count` is intentionally preserved — it - /// tracks total parses, not cache state. + /// Drain the VFS change log and drop changed ids' derived caches (`pub(crate)` so the watcher + /// can flush a `Delete`). `parse_count` is preserved — it tracks total parses, not cache state. pub(crate) fn invalidate_changed(&mut self) -> Vec { let changes = self.vfs.take_changes(); if !changes.is_empty() { diff --git a/crates/vfs/src/lib.rs b/crates/vfs/src/lib.rs index 2f4d866..dbafbac 100644 --- a/crates/vfs/src/lib.rs +++ b/crates/vfs/src/lib.rs @@ -216,65 +216,73 @@ impl Vfs { // --- project-wide basename index (pure, no I/O) --------------------------- - /// Intern `path` with **no text** and add it to the basename index, so a later - /// [`Self::find_include`] can locate it. Idempotent: a path already interned (whether by an - /// earlier `register_path` or by a text-bearing `set_file_contents`) keeps its existing - /// [`FileId`] and is added to the index at most once. **Does not clobber loaded text** — if the - /// path was loaded (e.g. by `load_include`), its text is preserved; only an unknown path is - /// interned, and interning with `None` records no change-log entry (so no spurious cache drop). - /// The caller canonicalizes the path *before* calling; this method stays pure. + /// Intern `path` with no text into the basename index, so [`Self::find_include`] can locate it. + /// Idempotent and never clobbers loaded text; interning an unknown path records no change. The + /// caller canonicalizes `path` first — this method stays pure. #[inline] pub fn register_path(&mut self, path: VfsPath) -> FileId { let id = if let Some(&id) = self.path_to_id.get(&path) { id } else { - // Unknown path: intern with no text. This is the no-change-log branch of - // `set_file_contents` (Create-with-None records nothing), so the path becomes known - // without a disk read or a cache invalidation. self.set_file_contents(path.clone(), None) }; if let Some(key) = file_name_of(path.as_path()) { - push_dedup(self.index.entry(key).or_default(), id); + push_dedup(self.index.entry(key.to_string()).or_default(), id); } id } - /// Remove `path`'s [`FileId`] from the basename index and drop any loaded text. Records a - /// [`ChangeKind::Delete`] change **iff** the file had text, so the source db drops its - /// parse/symbol-table caches — a delete must not leave stale caches behind. No-op (and no - /// change recorded) if `path` was never interned, or was interned with no text. + /// Remove `path` from the index and drop its text. Records a [`ChangeKind::Delete`] iff it had + /// text (so caches drop); no-op if unknown or already text-less. pub fn unregister_path(&mut self, path: &VfsPath) { let Some(&id) = self.path_to_id.get(path) else { return; }; if let Some(key) = file_name_of(path.as_path()) { - if let Some(vec) = self.index.get_mut(&key) { + if let Some(vec) = self.index.get_mut(key) { vec.retain(|f| *f != id); if vec.is_empty() { - self.index.remove(&key); + self.index.remove(key); } } } - // Drop any loaded text. None→None is a no-op (no change); Some→None records a Delete. self.set_file_contents(path.clone(), None); } - /// Pure ranked search for the best [`FileId`] matching include `rel` (e.g. `lib.circom` or - /// `circuits/x.circom`) from the includer `parent` (any interned path, typically the includer - /// file's own [`VfsPath`]). Candidates are every indexed file sharing `rel`'s basename. Ranking - /// is deterministic: - /// 1. **Suffix match** (desc): a candidate whose path *ends with* the full `rel` (so - /// `include "circuits/x.circom"` prefers `…/circuits/x.circom` over a bare `…/x.circom`). - /// 2. **Nearest** (desc): longest shared path-component prefix with `parent`. - /// 3. **Shortest path** (asc). - /// 4. **Alphabetical** (asc) — the final tiebreak for full determinism. - /// - /// Returns `None` if the basename isn't indexed. + /// Unregister every interned path at or under `prefix` (component-wise `starts_with`). Used when + /// a workspace folder is removed. Collects matches first: the scan borrows `self.files` + /// immutably while removal needs `&mut self`. + pub fn unregister_under(&mut self, prefix: &Path) { + let to_remove: Vec = self + .files + .iter() + .filter(|s| s.path.as_path().starts_with(prefix)) + .map(|s| s.path.clone()) + .collect(); + for p in to_remove { + self.unregister_path(&p); + } + } + + /// Every interned [`FileId`] whose path is at or under `prefix` (component-wise `starts_with`). + /// Relies on the invariant `FileId == index in `files` (ids are stable; slots aren't compacted). + pub fn ids_under(&self, prefix: &Path) -> Vec { + self.files + .iter() + .enumerate() + .filter(|(_, s)| s.path.as_path().starts_with(prefix)) + .map(|(i, _)| FileId(i as u32)) + .collect() + } + + /// Pure ranked search for the best [`FileId`] for include `rel` from the includer `parent`. + /// Ranking (deterministic): suffix-match desc, nearest (shared component prefix) desc, shortest + /// path asc, then alphabetical asc. #[inline] pub fn find_include(&self, parent: &VfsPath, rel: &str) -> Option { let rel_path = Path::new(rel); let key = file_name_of(rel_path)?; - let candidates = self.index.get(&key)?; + let candidates = self.index.get(key)?; if candidates.is_empty() { return None; } @@ -284,10 +292,9 @@ impl Vfs { .path(id) .map(|p| p.as_path()) .unwrap_or_else(|| Path::new("")); - // min_by_key picks the smallest key, so the "want-highest" fields are inverted. The - // final tiebreak compares the path's `OsStr` directly (`OsStr: Ord`, allocation-free, - // and MSRV-safe — unlike `as_encoded_bytes` which needs Rust 1.74). let osname = cpath.as_os_str(); + // `OsStr: Ord` gives an allocation-free, MSRV-safe tiebreak (unlike `as_encoded_bytes`, + // which needs Rust 1.74). `min_by_key` picks the smallest key, so the desc fields invert. ( !cpath.ends_with(rel_path), Reverse(shared_component_prefix(parent_path, cpath)), @@ -298,23 +305,19 @@ impl Vfs { } } -/// The `file_name` of `path` as a UTF-8 `String` (the index key), or `None` if it has none or is -/// non-UTF-8. Both registration and lookup derive the key the same way, so a `&str` include and an -/// interned path agree on their basename bucket. -fn file_name_of(path: &Path) -> Option { - path.file_name()?.to_str().map(|s| s.to_string()) +/// `path`'s file name as a borrowed `&str` (the index lookup key), or `None` if absent/non-UTF-8. +fn file_name_of(path: &Path) -> Option<&str> { + path.file_name()?.to_str() } -/// Push `id` into `vec` only if absent (keeps the index dedup'd under repeated registration). +/// Push `id` only if absent. fn push_dedup(vec: &mut Vec, id: FileId) { if !vec.contains(&id) { vec.push(id); } } -/// Count of leading path components `candidate` shares with `parent` (component-wise, not -/// byte-wise). Used by [`Vfs::find_include`] to prefer the include target nearest the includer. -/// Zips the two paths' component iterators directly — no intermediate `Vec` allocation. +/// Count of leading path components `candidate` shares with `parent` (component-wise). fn shared_component_prefix(parent: &Path, candidate: &Path) -> usize { parent .components() @@ -504,4 +507,28 @@ mod tests { assert_eq!(changes[0].file_id, id); assert!(vfs.file_text(id).is_none()); } + + #[test] + fn unregister_under_drops_whole_subtree_test() { + let mut vfs = Vfs::new(); + let keep = vp("/proj/keep.circom"); + let gone1 = vp("/proj/removed/a.circom"); + let gone2 = vp("/proj/removed/sub/b.circom"); + vfs.register_path(keep.clone()); + vfs.register_path(gone1); + vfs.register_path(gone2); + // Remove everything under /proj/removed (component-wise prefix). + vfs.unregister_under(Path::new("/proj/removed")); + // The kept file survives; the removed subtree is gone from the index. + assert!(vfs + .find_include(&vp("/proj/m.circom"), "keep.circom") + .is_some()); + assert!(vfs + .find_include(&vp("/proj/m.circom"), "a.circom") + .is_none()); + assert!(vfs + .find_include(&vp("/proj/m.circom"), "b.circom") + .is_none()); + let _ = keep; + } } From bc30b73099ac0cc0f81513c3833ec2e2d0cab7c6 Mon Sep 17 00:00:00 2001 From: Vu Vo Date: Sun, 9 Aug 2026 00:26:12 +0700 Subject: [PATCH 4/5] update Signed-off-by: Vu Vo --- crates/lsp/src/handler/goto_definition.rs | 148 +++++++++++++++++++--- crates/lsp/src/source_db.rs | 73 ++++++----- crates/vfs/src/lib.rs | 33 ++--- 3 files changed, 182 insertions(+), 72 deletions(-) diff --git a/crates/lsp/src/handler/goto_definition.rs b/crates/lsp/src/handler/goto_definition.rs index 95b52dd..57f0577 100644 --- a/crates/lsp/src/handler/goto_definition.rs +++ b/crates/lsp/src/handler/goto_definition.rs @@ -37,7 +37,9 @@ pub fn handle( // If `token` is an include path (`include "lib.circom";`), jump to that file's URL. Routed here // (not the resolver) because a `CircomString` carries a path, not a symbol name. Same resolution -// order as `load_include` so the jump target and the load always agree. +// order as `load_include` so the jump target and the load always agree. A relative include jumps +// to its file (resolved like circom, relative to the source — independent of the workspace root); +// the basename fallback jumps to a workspace-indexed file. pub fn include_target_location(file_db: &FileDB, token: &SyntaxToken, vfs: &Vfs) -> Vec { let Some(include_stmt) = token_ancestors(token).find_map(AstInclude::cast) else { return Vec::new(); @@ -52,21 +54,24 @@ pub fn include_target_location(file_db: &FileDB, token: &SyntaxToken, vfs: &Vfs) }; let lib_path = parent_dir.join(&rel); - // Same-dir: canonicalize + confine (defense-in-depth — `load_include` refuses escapes; mirror it - // so goto-def never surfaces an unreachable path). - if let Ok(canon) = lib_path.canonicalize() { - if vfs.is_confined(&canon) { - if let Some(vpath) = vfs::VfsPath::from_abs_path(&lib_path) { - if let Ok(lib_url) = Url::from_file_path(vpath.as_path()) { - return vec![Location::new(lib_url, Range::default())]; - } + // Relative include: jump to it iff it's a real file on disk (circom resolves includes relative + // to the source file, regardless of the editor's workspace root). Refuse absolute `rel` + // (`PathBuf::join` would replace the base → arbitrary path) and non-files, matching what + // `load_include` would actually load; on a miss we fall through to the workspace-indexed + // basename fallback below. + let rel_is_absolute = std::path::Path::new(&rel).is_absolute(); + if !rel_is_absolute && lib_path.is_file() { + if let Some(vpath) = vfs::VfsPath::from_abs_path(&lib_path) { + if let Ok(lib_url) = Url::from_file_path(vpath.as_path()) { + return vec![Location::new(lib_url, Range::default())]; } } } - // Same-dir miss → basename fallback. `parent` is the includer file (matching `load_include`) so - // `find_include` ranks identically. Canonicalize + confine the winner too (it may have been - // deleted between the walk and this jump). + // Same-dir miss → basename fallback over the workspace index. `parent` is the includer file + // (matching `load_include`) so `find_include` ranks identically. Re-check the winner exists on + // disk so the jump never targets a file the walk indexed but that has since been deleted (and + // that `load_include` would therefore refuse to load). let Some(parent_vpath) = vfs::VfsPath::from_abs_path(&path) else { return Vec::new(); }; @@ -76,10 +81,7 @@ pub fn include_target_location(file_db: &FileDB, token: &SyntaxToken, vfs: &Vfs) let Some(winner_path) = vfs.path(winner) else { return Vec::new(); }; - let Ok(canon) = winner_path.as_path().canonicalize() else { - return Vec::new(); - }; - if !vfs.is_confined(&canon) { + if !winner_path.as_path().is_file() { return Vec::new(); } let Ok(lib_url) = Url::from_file_path(winner_path.as_path()) else { @@ -573,4 +575,118 @@ mod tests { let _ = fs::remove_dir_all(&base); } + + // --- real-client-flow helpers (didOpen via handle_update, not set_document) ---------------- + + use lsp_types::{ + DidOpenTextDocumentParams, GotoDefinitionParams, TextDocumentIdentifier, TextDocumentItem, + TextDocumentPositionParams, + }; + + use crate::global_state::TextDocument; + + /// didOpen a document through the real notification path (`handle_update`). + fn open_doc(state: &mut GlobalState, url: &Url, src: &str) { + state + .handle_update(TextDocument::from(DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: url.clone(), + language_id: "circom".to_string(), + version: 0, + text: src.to_string(), + }, + })) + .unwrap(); + } + + /// Drive the real `textDocument/definition` handler at the `occurrence`-th token whose + /// kind+text match (covers `Identifier` symbols and `CircomString` include paths). + fn goto_def( + state: &GlobalState, + url: &Url, + source: &str, + kind: parser::token_kind::TokenKind, + text: &str, + occurrence: usize, + ) -> Vec { + let id = state.source_db.id_for_url(url).expect("doc registered"); + let file_db = state.source_db.file_db(id); + let ast = AstCircomProgram::cast(syntax_tree(source)).expect("program"); + let token = ast + .syntax() + .descendants_with_tokens() + .filter_map(|e| e.into_token()) + .filter(|t| t.kind() == kind && t.text() == text) + .nth(occurrence) + .unwrap_or_else(|| panic!("token {text:?}#{occurrence} not found")); + let pos = file_db.position(token.text_range().start()); + super::handle( + state, + GotoDefinitionParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: url.clone() }, + position: pos, + }, + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + }, + ) + .unwrap() + .map(|r| match r { + lsp_types::GotoDefinitionResponse::Array(v) => v, + _ => Vec::new(), + }) + .unwrap_or_default() + } + + /// Reproduction of the reported bug: a circom project whose files live **outside** the + /// configured workspace root (the editor pointed ccls at the wrong/incomplete folder). Every + /// relative `include` must still resolve the way circom resolves it — relative to the source + /// file — so goto-lib and goto-def into the lib work. Confining includes to the (wrong) + /// workspace root refused them, breaking all cross-file navigation. + #[test] + fn goto_works_when_project_outside_workspace_root_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_outside_{}", std::process::id())); + let proj = base.join("proj"); // the real circom project + let wrong_root = base.join("wrong_root"); // the (wrong) workspace root + fs::create_dir_all(&proj).unwrap(); + fs::create_dir_all(&wrong_root).unwrap(); + fs::write( + proj.join("lib.circom"), + "pragma circom 2.0.0;\ntemplate Lib() {\n signal output o;\n o <== 0;\n}\n", + ) + .unwrap(); + let main_src = "pragma circom 2.0.0;\ninclude \"lib.circom\";\ncomponent main = Lib();\n"; + fs::write(proj.join("main.circom"), main_src).unwrap(); + + let main_url = Url::from_file_path(proj.join("main.circom")).unwrap(); + let lib_url = Url::from_file_path(proj.join("lib.circom")).unwrap(); + // Workspace root is the WRONG folder — `proj` is not under it. + let mut state = GlobalState::new(vec![wrong_root.canonicalize().unwrap()]); + open_doc(&mut state, &main_url, main_src); + + // 1. Goto-def on the `include "lib.circom"` string → jumps to lib.circom. + let inc = goto_def( + &state, + &main_url, + main_src, + TokenKind::CircomString, + "\"lib.circom\"", + 0, + ); + assert_eq!( + inc.len(), + 1, + "goto-lib jumps to the include target: {inc:?}" + ); + assert_eq!(inc[0].uri, lib_url, "lands on lib.circom"); + + // 2. Goto-def on `Lib` in `component main = Lib()` → jumps into lib.circom. + let def = goto_def(&state, &main_url, main_src, TokenKind::Identifier, "Lib", 0); + assert_eq!(def.len(), 1, "cross-file goto-def into the lib: {def:?}"); + assert_eq!(def[0].uri, lib_url, "jumps into lib.circom"); + + let _ = fs::remove_dir_all(&base); + } } diff --git a/crates/lsp/src/source_db.rs b/crates/lsp/src/source_db.rs index 95ade56..0b1e2f7 100644 --- a/crates/lsp/src/source_db.rs +++ b/crates/lsp/src/source_db.rs @@ -102,7 +102,15 @@ impl ContentCacheDb { } /// Resolve a relative include `rel` against `parent_url`'s dir to an absolutized [`VfsPath`]. + /// An **absolute** `rel` is refused: `PathBuf::join` would *replace* the base (`include + /// "/etc/passwd"`), enabling an arbitrary local-file read. circom's legitimate includes are + /// relative (`..` allowed) and resolve against the project tree; absolute include strings are + /// not a real circom idiom. (The basename fallback still fields any `rel` but only over the + /// workspace-indexed set, so it stays safe.) fn resolve_include(parent_url: &Url, rel: &str) -> Option { + if std::path::Path::new(rel).is_absolute() { + return None; + } let parent_path = parent_url.to_file_path().ok()?; let parent_dir = parent_path.parent()?; let lib_path = parent_dir.join(rel); @@ -191,14 +199,13 @@ impl ContentCacheDb { } } - /// Canonicalize + confine + read one include path — the single disk boundary. `canonicalize` - /// resolves `..`/symlinks; `is_confined` is the pure prefix check. Path-traversal escapes are - /// refused here, so the pure lookup/jump paths only ever surface already-confined files. + /// Read one include path from disk — the single disk boundary. A relative include resolves the + /// way circom resolves it: relative to the **including source file**, so it loads regardless of + /// the editor's (possibly missing/wrong) workspace root. Both callers produce trusted paths: + /// the relative path is built from an opened doc's location, and the basename fallback + /// ([`Vfs::find_include`]) only returns files the workspace walk already indexed. Serves a + /// cached id when the text is already loaded. fn load_from_disk(&mut self, vpath: &VfsPath) -> Option { - let canonical = vpath.as_path().canonicalize().ok()?; - if !self.vfs.is_confined(&canonical) { - return None; - } // Serve the cached id if text is already loaded; a path-only id (from the walk) reads below. if let Some(id) = self.vfs.file_id(vpath) { if self.vfs.file_text(id).is_some() { @@ -446,55 +453,53 @@ template Multiplier2() { ); } - /// Path-traversal confinement: an `include` that resolves outside the workspace root is refused - /// (never read), whether via `..`, an absolute path, or a symlink. Set up a workspace dir, a - /// main file inside it, and a secret file one level above; both escape forms must return `None`. + /// Includes resolve relative to the **source file** the way circom resolves them — independent + /// of the editor's workspace root. So a relative include loads even with no roots configured, + /// and even when it points outside the (possibly wrong) root. (The basename fallback is still + /// workspace-index-scoped.) Sanity: a same-directory include loads, a missing one does not. #[test] - fn include_confined_to_workspace_root_test() { + fn relative_include_resolves_without_workspace_root_test() { use std::fs; - use std::path::PathBuf; let base = std::env::temp_dir().join(format!("ccls_confine_{}", std::process::id())); let ws = base.join("ws"); - let secret = base.join("secret.circom"); let main_path = ws.join("main.circom"); fs::create_dir_all(&ws).unwrap(); - fs::write(&secret, "pragma circom 2.0.0;").unwrap(); fs::write(&main_path, "pragma circom 2.0.0;").unwrap(); + fs::write(ws.join("lib.circom"), "pragma circom 2.0.0;").unwrap(); + fs::write(base.join("sibling.circom"), "pragma circom 2.0.0;").unwrap(); let main_url = Url::from_file_path(&main_path).unwrap(); - let mut db = ContentCacheDb::new(); - // Root is the workspace dir only — `secret.circom` lives above it. - db.set_workspace_roots(vec![ws.canonicalize().unwrap()]); - // No roots configured yet in the empty case: every include is refused (fail closed). - let mut empty = ContentCacheDb::new(); + // No roots configured: a same-directory include still resolves (circom semantics). + let mut no_roots = ContentCacheDb::new(); assert!( - empty.load_include(&main_url, "lib.circom").is_none(), - "with no workspace roots, no include may be loaded (fail closed)" + no_roots.load_include(&main_url, "lib.circom").is_some(), + "a relative include resolves even with no workspace roots" ); - // `..` escape to a sibling file outside the root is refused. + // A `..` include to a sibling outside the root resolves too (circom allows `..`). + let mut no_roots2 = ContentCacheDb::new(); assert!( - db.load_include(&main_url, "../secret.circom").is_none(), - "include escaping the workspace via `..` must be refused" + no_roots2 + .load_include(&main_url, "../sibling.circom") + .is_some(), + "a `..` include resolves relative to the source (circom allows it)" ); - // Absolute path outside the root is refused (`PathBuf::join` would otherwise replace base). + + // A non-existent include resolves to None (file simply isn't there). + let mut db = ContentCacheDb::new(); + db.set_workspace_roots(vec![ws.canonicalize().unwrap()]); assert!( - db.load_include(&main_url, secret.to_str().unwrap()) - .is_none(), - "an absolute include outside the workspace must be refused" + db.load_include(&main_url, "missing.circom").is_none(), + "a missing include resolves to None" ); - - // Sanity: a same-directory include inside the root loads (file must exist). - let in_root = ws.join("lib.circom"); - fs::write(&in_root, "pragma circom 2.0.0;").unwrap(); + // And a present same-directory include loads. assert!( db.load_include(&main_url, "lib.circom").is_some(), - "an include inside the workspace root must load" + "a same-directory include loads" ); - let _ = PathBuf::from(&base); let _ = fs::remove_dir_all(&base); } } diff --git a/crates/vfs/src/lib.rs b/crates/vfs/src/lib.rs index dbafbac..2205128 100644 --- a/crates/vfs/src/lib.rs +++ b/crates/vfs/src/lib.rs @@ -72,12 +72,12 @@ pub struct Vfs { path_to_id: HashMap, files: Vec, changes: Vec, - /// Canonicalized workspace root folders. The containment primitive for confining untrusted - /// `include "…"` resolution (path-traversal defense): an include may only resolve to a file - /// inside one of these. Set once via [`Vfs::set_workspace_roots`]; empty (fail-closed) until - /// then. The roots themselves are stored already-canonicalized by the caller, so - /// [`Vfs::is_confined`] stays pure (no disk I/O) — the `canonicalize` stat is the LSP layer's - /// job, keeping this crate I/O-free and unit-testable. + /// Canonicalized workspace root folders. The scope of the project-wide `.circom` walk + /// (`collect_circom_files`) that feeds the basename index — the search surface for include + /// resolution when a same-dir lookup misses. Includes themselves resolve relative to the + /// source file (circom semantics), so these roots are an *indexing* scope, not a confinement + /// gate. Set via [`Vfs::set_workspace_roots`]; stored already-canonicalized by the caller so + /// this crate stays I/O-free and unit-testable. workspace_roots: Vec, /// Project-wide basename index: `file_name` (e.g. `lib.circom`) → every interned [`FileId`] /// with that basename. The search surface for include resolution when the same-dir lookup @@ -104,30 +104,19 @@ impl Vfs { } } - /// Set the workspace roots used by [`Self::is_confined`]. Callers (the LSP `initialize` - /// handshake) pass already-canonicalized absolute paths; no disk I/O happens here. + /// Set the workspace roots scoped by the project `.circom` walk (the basename-index source). + /// Callers (the LSP `initialize` handshake) pass already-canonicalized absolute paths; no disk + /// I/O happens here. pub fn set_workspace_roots(&mut self, roots: Vec) { self.workspace_roots = roots; } - /// The workspace roots confining [`Self::is_confined`] (already canonicalized by the caller). - /// Read accessor so the LSP layer's workspace walker can re-walk the same roots it set. + /// The workspace roots (already canonicalized by the caller) that scope the project walk. A + /// read accessor so the LSP layer's walker can re-walk the same roots it set. pub fn workspace_roots(&self) -> &[PathBuf] { &self.workspace_roots } - /// Pure containment check: is `canonical` (an already-canonicalized absolute path) inside one of - /// the workspace roots? **Fail-closed** — with no roots configured nothing is confined, so the - /// LSP refuses to load any include rather than risk an arbitrary read. No disk I/O: the caller - /// canonicalizes the candidate path (resolving `..`/symlinks) before calling. - pub fn is_confined(&self, canonical: &Path) -> bool { - !self.workspace_roots.is_empty() - && self - .workspace_roots - .iter() - .any(|root| canonical.starts_with(root)) - } - /// The id for `path` if it has been interned, else `None`. #[inline] pub fn file_id(&self, path: &VfsPath) -> Option { From 7af83726d312df5f014e73d3c3d34e5f3f5e10e6 Mon Sep 17 00:00:00 2001 From: Vu Vo Date: Sun, 9 Aug 2026 00:43:39 +0700 Subject: [PATCH 5/5] update Signed-off-by: Vu Vo --- crates/lsp/src/global_state.rs | 240 +++++++++++++++++----- crates/lsp/src/handler/goto_definition.rs | 60 ++---- crates/lsp/src/lib.rs | 3 +- crates/lsp/src/source_db.rs | 22 +- 4 files changed, 220 insertions(+), 105 deletions(-) diff --git a/crates/lsp/src/global_state.rs b/crates/lsp/src/global_state.rs index b62aa08..3903dc7 100644 --- a/crates/lsp/src/global_state.rs +++ b/crates/lsp/src/global_state.rs @@ -95,8 +95,10 @@ impl Default for GlobalState { } impl GlobalState { - /// Construct with workspace `roots` confining `include` resolution. Empty roots ⇒ no include - /// ever loads (fail-closed against path traversal). + /// Construct with workspace `roots`. Roots scope the project `.circom` walk that feeds the + /// basename index; `include` resolution itself is circom-style (relative to the source file), + /// not confined to the roots. Absolute include paths are rejected; relative/`..` includes + /// resolve (and may reach files outside the roots, matching the circom compiler). pub fn new(roots: Vec) -> Self { let mut source_db = ContentCacheDb::new(); source_db.set_workspace_roots(roots); @@ -213,6 +215,11 @@ impl GlobalState { } match change.typ { FileChangeType::CREATED => { + // Skip open docs: a build tool deleting+recreating a file must not clobber an open + // doc's dirty buffer (`didChange` is authoritative for it) — mirrors DELETED/CHANGED. + if self.open_documents.contains(&change.uri) { + return; + } if let Ok(canon) = path.canonicalize() { if let Some(vpath) = vfs::VfsPath::from_abs_path(&canon) { let id = self.source_db.vfs_mut().register_path(vpath.clone()); @@ -290,15 +297,34 @@ impl GlobalState { removed_canon.push(canon); } } - self.source_db.set_workspace_roots(roots); + self.source_db.set_workspace_roots(roots.clone()); if !removed_canon.is_empty() { + // Drop only files no longer under ANY remaining root — a removed folder may be nested + // inside one, in which case its files must stay resolvable. `unregister_under` is too + // blunt (it would null their text), so unregister the uncovered paths individually. + let still_under = |id: FileId| -> bool { + self.source_db + .vfs() + .path(id) + .is_some_and(|p| roots.iter().any(|r| p.as_path().starts_with(r))) + }; + let mut to_drop: Vec = Vec::new(); for canon in &removed_canon { for id in self.source_db.vfs().ids_under(canon) { - self.workspace_files.remove(&id); + if !still_under(id) { + to_drop.push(id); + } + } + } + for id in &to_drop { + self.workspace_files.remove(id); + if let Some(p) = self.source_db.vfs().path(*id).cloned() { + self.source_db.vfs_mut().unregister_path(&p); } - self.source_db.vfs_mut().unregister_under(canon); } - self.after_vfs_mutation(); + if !to_drop.is_empty() { + self.after_vfs_mutation(); + } } if !added_canon.is_empty() { self.register_indexed_paths(crate::project_index::collect_circom_files_with_content( @@ -311,7 +337,7 @@ impl GlobalState { /// resolves via [`Self::resolve_token`], file-tagged. pub fn lookup_definition(&self, file_db: &FileDB, token: &SyntaxToken) -> Vec { if token.kind() == TokenKind::CircomString { - return include_target_location(file_db, token, self.source_db.vfs()); + return include_target_location(file_db, token, &self.source_db); } self.to_locations(self.resolve_token(file_db, token)) } @@ -368,71 +394,79 @@ impl GlobalState { .collect() } - /// Resolve `token` to its file-tagged declaration(s): in-file first; then, for a component - /// decl/call — or the top-level `component main = X()` instantiation — each loaded include's - /// top-level by name. Cross-file is file-scope only (template/function names) — the shared - /// core for goto-def, rename, references. - pub(crate) fn resolve_use( + /// In-file resolution of `token` against `origin`'s own `SymbolTable` (file-tagged). + fn resolve_in_file( &self, origin: &FileDB, token: &SyntaxToken, ) -> Vec<(FileId, ResolvedSymbol)> { let table = self.source_db.symbol_table(origin.file_id); - let mut out: Vec<(FileId, ResolvedSymbol)> = resolver::resolve(&table, token) + resolver::resolve(&table, token) .into_iter() .map(|s| (origin.file_id, s)) - .collect(); + .collect() + } + + /// Each **direct** include's top-level declarations named like `token` (circom's non-transitive + /// include visibility), file-tagged with the include's `FileId`. + fn resolve_in_includes( + &self, + origin: &FileDB, + token: &SyntaxToken, + ) -> Vec<(FileId, ResolvedSymbol)> { + let name = token.text(); + let mut out = Vec::new(); + for lib_id in self.loaded_includes(origin) { + let lib_table = self.source_db.symbol_table(lib_id); + for sym in lib_table.lookup_top_level(name) { + out.push((lib_id, ResolvedSymbol::from(sym))); + } + } + out + } - // A component declaration/call — or the top-level `component main = X()` instantiation — - // also resolves to template/function defs in loaded includes. `MainComponent` is a distinct - // node kind from `ComponentDecl`/`ComponentCall`, so it is listed explicitly (without it, - // `component main = Lib()` where `Lib` is in an include would never resolve). + /// Resolve `token` to its file-tagged declaration(s) for goto-def/hover: in-file first + /// (shadowing); if empty, for a component decl/call — or the top-level `component main = X()` + /// instantiation — each loaded include's top-level by name. `MainComponent` is a distinct node + /// kind from `ComponentDecl`/`ComponentCall`, so it is listed explicitly (without it, + /// `component main = Lib()` where `Lib` is in an include would never resolve). + pub(crate) fn resolve_use( + &self, + origin: &FileDB, + token: &SyntaxToken, + ) -> Vec<(FileId, ResolvedSymbol)> { + let in_file = self.resolve_in_file(origin, token); + if !in_file.is_empty() { + return in_file; + } let is_component_use = token_ancestors(token).any(|n| { AstComponentDecl::can_cast(n.kind()) || AstComponentCall::can_cast(n.kind()) || AstMainComponent::can_cast(n.kind()) }); if is_component_use { - let name = token.text(); - for lib_id in self.loaded_includes(origin) { - let lib_table = self.source_db.symbol_table(lib_id); - for sym in lib_table.lookup_top_level(name) { - out.push((lib_id, sym.into())); - } - } + self.resolve_in_includes(origin, token) + } else { + Vec::new() } - out } /// Resolve `token` with workspace visibility — [`Self::resolve_use`] **without** the - /// component-use gate. In-file `resolve` first (precedence/shadowing): when non-empty it wins - /// outright, so an in-file def shadows an include's same-named def. Only when in-file - /// resolution is empty does it search each **direct** include's top-level by name (circom's - /// non-transitive include visibility). References/rename need this ungated path to find usages - /// of *any* included top-level symbol (a template/function referenced by name inside a body), - /// not just component instantiations. + /// component-use gate: in-file first (precedence/shadowing); when non-empty it wins outright, so + /// an in-file def shadows an include's same-named def; otherwise every direct include's + /// top-level by name. References/rename need this ungated path to find usages of *any* included + /// top-level symbol (a template/function referenced by name inside a body), not just component + /// instantiations. pub(crate) fn resolve_visible( &self, origin: &FileDB, token: &SyntaxToken, ) -> Vec<(FileId, ResolvedSymbol)> { - let table = self.source_db.symbol_table(origin.file_id); - let in_file: Vec<(FileId, ResolvedSymbol)> = resolver::resolve(&table, token) - .into_iter() - .map(|s| (origin.file_id, s)) - .collect(); + let in_file = self.resolve_in_file(origin, token); if !in_file.is_empty() { return in_file; } - let name = token.text(); - let mut out = Vec::new(); - for lib_id in self.loaded_includes(origin) { - let lib_table = self.source_db.symbol_table(lib_id); - for sym in lib_table.lookup_top_level(name) { - out.push((lib_id, ResolvedSymbol::from(sym))); - } - } - out + self.resolve_in_includes(origin, token) } /// Resolve a component member-access **field** token (`c.x` / `T()(...).x`) to its signal @@ -527,10 +561,19 @@ impl GlobalState { if self.source_db.vfs().file_text(f).is_none() { continue; } + let file_db = self.source_db.file_db(f); + // A token in `f` can only resolve to `target` if `f` IS the defining file, is the + // cursor's file, or directly includes the defining file (circom's non-transitive + // visibility). Skip the rest — they can't reference `target`, so walking their tokens + // (an O(file_text) walk each) is wasted work on large workspaces. + let visible = + f == *def_file || f == origin || self.loaded_includes(&file_db).contains(def_file); + if !visible { + continue; + } let Some(ast) = self.source_db.ast(f) else { continue; }; - let file_db = self.source_db.file_db(f); for tok in resolver::identifiers_named(ast.syntax(), name) { let matched = self .resolve_visible(&file_db, &tok) @@ -550,7 +593,7 @@ impl GlobalState { /// query ⇒ all), as owned `(FileId, Symbol)` (each file's `SymbolTable` is cached behind a /// short-lived borrow). Powers `workspace/symbol`. pub(crate) fn workspace_symbols(&self, query: &str) -> Vec<(FileId, Symbol)> { - let q = query.trim(); + let q = query.trim().to_lowercase(); let mut out = Vec::new(); for f in &self.workspace_files { if self.source_db.vfs().file_text(*f).is_none() { @@ -561,7 +604,7 @@ impl GlobalState { } let table = self.source_db.symbol_table(*f); for sym in table.top_level_symbols() { - if is_workspace_symbol_kind(sym.kind) && matches_query(q, &sym.name) { + if is_workspace_symbol_kind(sym.kind) && matches_query(&q, &sym.name) { out.push((*f, sym.clone())); } } @@ -620,10 +663,10 @@ fn is_workspace_symbol_kind(kind: SymbolKind) -> bool { ) } -/// Case-insensitive substring match; an empty query matches everything (`workspace/symbol` lists -/// all symbols when the query is blank). -fn matches_query(query: &str, name: &str) -> bool { - query.is_empty() || name.to_lowercase().contains(&query.to_lowercase()) +/// Case-insensitive substring match against an already-lowercased `query` (callers hoist the +/// `to_lowercase` once per request); an empty query matches everything. +fn matches_query(query_lower: &str, name: &str) -> bool { + query_lower.is_empty() || name.to_lowercase().contains(query_lower) } /// Deserialize params, run the handler, wrap the result in a success `Response`. Generic so @@ -901,6 +944,44 @@ mod tests { let _ = fs::remove_dir_all(&base); } + /// Regression (CREATED fix): a `workspace/didChangeWatchedFiles` CREATED event for a file the + /// client has open must NOT overwrite its dirty buffer. A build tool that deletes+recreates a + /// file while it's open fires CREATED; the open doc's unsaved edits must survive (mirrors the + /// CHANGED/DELETED open-doc guards). + #[test] + fn watched_created_skips_open_document_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_created_{}", std::process::id())); + let ws = base.join("ws"); + fs::create_dir_all(&ws).unwrap(); + let lib_path = ws.join("lib.circom"); + let lib_url = Url::from_file_path(&lib_path).unwrap(); + let mut state = GlobalState::new(vec![ws.canonicalize().unwrap()]); + // Open with dirty text; the file does not yet exist on disk. + state + .handle_update(doc( + &lib_url, + "pragma circom 2.0.0;\ntemplate Dirty() {}\n".to_string(), + )) + .unwrap(); + + // The file is (re)created on disk underneath; watcher fires CREATED. + fs::write(&lib_path, "pragma circom 2.0.0;\n").unwrap(); + state.handle_watched_file_change(lsp_types::FileEvent { + uri: lib_url.clone(), + typ: lsp_types::FileChangeType::CREATED, + }); + + let id = state.source_db.id_for_url(&lib_url).unwrap(); + let text = state.source_db.file_text(id); + assert!( + text.contains("Dirty"), + "open-doc buffer must be preserved across a watcher CREATED: {text}" + ); + + let _ = fs::remove_dir_all(&base); + } + /// Regression (DELETED fix): a `workspace/didChangeWatchedFiles` DELETED event for a file the /// client has open must NOT null its in-memory text (a build tool deleting+recreating the file /// must not make the open doc unresolvable). Mirrors the CHANGED open-doc guard. @@ -997,6 +1078,59 @@ mod tests { let _ = fs::remove_dir_all(&base); } + /// Regression (nested-folder fix): removing a folder nested inside a *remaining* root must NOT + /// drop files that are still covered by the outer root. Previously `unregister_under` nulled + /// their text, silently breaking resolution. + #[test] + fn removed_nested_folder_keeps_covered_files_test() { + use std::fs; + let base = std::env::temp_dir().join(format!("ccls_nested_{}", std::process::id())); + let outer = base.join("proj"); + let inner = outer.join("sub"); + fs::create_dir_all(&inner).unwrap(); + fs::write( + inner.join("lib.circom"), + "pragma circom 2.0.0;\ntemplate Lib() { signal output o; o <== 0; }\n", + ) + .unwrap(); + let lib_url = Url::from_file_path(inner.join("lib.circom")).unwrap(); + + // Both the outer project and its nested sub-folder are roots. + let mut state = GlobalState::new(vec![ + outer.canonicalize().unwrap(), + inner.canonicalize().unwrap(), + ]); + state.index_workspace(); + let id = state.source_db.id_for_url(&lib_url).expect("lib indexed"); + assert!( + state.source_db.vfs().file_text(id).is_some(), + "lib has loaded text" + ); + + // Remove only the nested folder — `outer` still covers `lib.circom`. + state.handle_workspace_folders_change(lsp_types::DidChangeWorkspaceFoldersParams { + event: lsp_types::WorkspaceFoldersChangeEvent { + added: Vec::new(), + removed: vec![lsp_types::WorkspaceFolder { + uri: Url::from_file_path(&inner).unwrap(), + name: "sub".to_string(), + }], + }, + }); + + // The file is still under the outer root → it must keep its text + stay in the workspace. + assert!( + state.source_db.vfs().file_text(id).is_some(), + "a file still under a remaining root must keep its text" + ); + assert!( + state.workspace_files.contains(&id), + "a file still under a remaining root stays in workspace_files" + ); + + let _ = fs::remove_dir_all(&base); + } + /// Regression (walk-landing reload fix): a doc with a non-sibling include opened BEFORE the /// index is populated has its include loaded when the walk lands — without needing another edit /// — so cross-file symbol features activate immediately. diff --git a/crates/lsp/src/handler/goto_definition.rs b/crates/lsp/src/handler/goto_definition.rs index 57f0577..1eb3f4c 100644 --- a/crates/lsp/src/handler/goto_definition.rs +++ b/crates/lsp/src/handler/goto_definition.rs @@ -3,11 +3,11 @@ use rowan::ast::AstNode; use syntax::abstract_syntax_tree::AstInclude; use syntax::node::SyntaxToken; -use vfs::Vfs; use crate::file_db::FileDB; use crate::global_state::GlobalState; use crate::resolver::{token_ancestors, token_at_offset}; +use crate::source_db::ContentCacheDb; use anyhow::Result; use lsp_types::{GotoDefinitionParams, GotoDefinitionResponse}; @@ -36,58 +36,28 @@ pub fn handle( } // If `token` is an include path (`include "lib.circom";`), jump to that file's URL. Routed here -// (not the resolver) because a `CircomString` carries a path, not a symbol name. Same resolution -// order as `load_include` so the jump target and the load always agree. A relative include jumps -// to its file (resolved like circom, relative to the source — independent of the workspace root); -// the basename fallback jumps to a workspace-indexed file. -pub fn include_target_location(file_db: &FileDB, token: &SyntaxToken, vfs: &Vfs) -> Vec { +// (not the resolver) because a `CircomString` carries a path, not a symbol name. Delegates to the +// same resolver `load_include` uses (`resolve_include_vpath`), so the jump target and the loaded +// file always agree — one resolution order (relative same-dir then workspace basename fallback), +// no drift. Refuses absolute include paths. +pub fn include_target_location( + file_db: &FileDB, + token: &SyntaxToken, + db: &ContentCacheDb, +) -> Vec { let Some(include_stmt) = token_ancestors(token).find_map(AstInclude::cast) else { return Vec::new(); }; let Some(include_path) = include_stmt.lib() else { return Vec::new(); }; - let rel = include_path.value(); - let path = file_db.get_path(); - let Some(parent_dir) = path.parent() else { + let Some(vpath) = db.resolve_include_vpath(&file_db.file_path, &include_path.value()) else { return Vec::new(); }; - let lib_path = parent_dir.join(&rel); - - // Relative include: jump to it iff it's a real file on disk (circom resolves includes relative - // to the source file, regardless of the editor's workspace root). Refuse absolute `rel` - // (`PathBuf::join` would replace the base → arbitrary path) and non-files, matching what - // `load_include` would actually load; on a miss we fall through to the workspace-indexed - // basename fallback below. - let rel_is_absolute = std::path::Path::new(&rel).is_absolute(); - if !rel_is_absolute && lib_path.is_file() { - if let Some(vpath) = vfs::VfsPath::from_abs_path(&lib_path) { - if let Ok(lib_url) = Url::from_file_path(vpath.as_path()) { - return vec![Location::new(lib_url, Range::default())]; - } - } + match Url::from_file_path(vpath.as_path()) { + Ok(lib_url) => vec![Location::new(lib_url, Range::default())], + Err(_) => Vec::new(), } - - // Same-dir miss → basename fallback over the workspace index. `parent` is the includer file - // (matching `load_include`) so `find_include` ranks identically. Re-check the winner exists on - // disk so the jump never targets a file the walk indexed but that has since been deleted (and - // that `load_include` would therefore refuse to load). - let Some(parent_vpath) = vfs::VfsPath::from_abs_path(&path) else { - return Vec::new(); - }; - let Some(winner) = vfs.find_include(&parent_vpath, &rel) else { - return Vec::new(); - }; - let Some(winner_path) = vfs.path(winner) else { - return Vec::new(); - }; - if !winner_path.as_path().is_file() { - return Vec::new(); - } - let Ok(lib_url) = Url::from_file_path(winner_path.as_path()) else { - return Vec::new(); - }; - vec![Location::new(lib_url, Range::default())] } #[cfg(test)] @@ -454,7 +424,7 @@ mod tests { .find(|t| t.kind() == TokenKind::CircomString) .expect("include path string present"); - let locs = super::include_target_location(&file_db, &token, state.source_db.vfs()); + let locs = super::include_target_location(&file_db, &token, &state.source_db); assert_eq!( locs.len(), 1, diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs index cf9564a..65718da 100644 --- a/crates/lsp/src/lib.rs +++ b/crates/lsp/src/lib.rs @@ -83,7 +83,8 @@ fn main_loop( ) -> Result<(), Box> { let params: InitializeParams = serde_json::from_value(params)?; - // Roots confine include resolution (path-traversal defense); empty roots ⇒ no include loads. + // Roots scope the project `.circom` walk (the basename-index source). Includes resolve + // circom-style relative to each source file, so navigation works even with wrong/empty roots. let roots = workspace_roots(¶ms); let mut state = GlobalState::new(roots.clone()); diff --git a/crates/lsp/src/source_db.rs b/crates/lsp/src/source_db.rs index 0b1e2f7..28011ed 100644 --- a/crates/lsp/src/source_db.rs +++ b/crates/lsp/src/source_db.rs @@ -159,17 +159,27 @@ impl ContentCacheDb { Some(id) } - /// Load a single include (no transitive closure): same-dir path first, then basename fallback. - fn load_one_include(&mut self, parent_url: &Url, rel: &str) -> Option { + /// The on-disk target of include `rel` from `parent_url`, mirroring [`Self::load_one_include`]'s + /// path selection: the same-dir relative target if it is a real file, else the workspace-indexed + /// basename winner (also confirmed to still exist). Refuses absolute `rel`. **Pure** (no read) — + /// shared by goto-lib (build a URL) and the loader (read + intern), so the jump target and the + /// loaded file always agree and there is one resolution order to maintain. + pub(crate) fn resolve_include_vpath(&self, parent_url: &Url, rel: &str) -> Option { if let Some(vpath) = Self::resolve_include(parent_url, rel) { - if let Some(id) = self.load_from_disk(&vpath) { - return Some(id); + if vpath.as_path().is_file() { + return Some(vpath); } } let parent_vpath = Self::url_to_vpath(parent_url)?; let winner = self.vfs.find_include(&parent_vpath, rel)?; - let winner_path = self.vfs.path(winner)?.clone(); - self.load_from_disk(&winner_path) + let winner_path = self.vfs.path(winner)?; + winner_path.as_path().is_file().then(|| winner_path.clone()) + } + + /// Load a single include (no transitive closure) via [`Self::resolve_include_vpath`]. + fn load_one_include(&mut self, parent_url: &Url, rel: &str) -> Option { + let vpath = self.resolve_include_vpath(parent_url, rel)?; + self.load_from_disk(&vpath) } /// Recursively load `id`'s own includes (its include-closure) so resolution works from within