Skip to content

Commit 19ba736

Browse files
committed
More refactoring
1 parent edda6f8 commit 19ba736

147 files changed

Lines changed: 2868 additions & 2715 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

docs/todo/refactor.md

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -334,39 +334,6 @@ unrelated scheduling and filtering code.
334334

335335
---
336336

337-
## `util.rs` mixes eleven unrelated concerns
338-
339-
**What to do.** Break `src/util.rs` (2,613 lines, 87 functions) into
340-
cohesive modules, and move single-consumer helpers to their only
341-
consumer:
342-
343-
- `text_position.rs` — the most reused cluster: `LineIndex`,
344-
`offset_to_position`, `position_to_byte_offset`,
345-
`byte_range_to_lsp_range`, UTF-16 column conversion.
346-
- `php_text.rs` — string scanning (`unquote_php_string`,
347-
`find_matching_forward`/`_backward`, `find_semicolon_balanced`,
348-
`collapse_continuation_lines`).
349-
- `class_lookup.rs``find_class_by_name`, `find_class_at_offset`,
350-
`is_subtype_of*`, `is_self_or_static`, `resolve_class_keyword`
351-
(~360 lines).
352-
- `process.rs``CommandOutput`, `run_command_with_timeout`.
353-
- Move to their sole consumers: `log` + `progress_*` (only
354-
`server.rs`), `has_unclosed_delimiters` (docblock area),
355-
`find_identical_occurrences` (`code_actions`),
356-
`contains_php_attribute` + `find_brace_match_line`
357-
(`code_actions/phpstan`), `collect_php_files_gitignore` (workspace
358-
indexing).
359-
- The `impl Backend` file-content/context accessors (`get_file_content`,
360-
`file_context*`, `namespace_at_offset`, `clear_file_maps`, …) are
361-
Backend behaviour, not utilities — move next to `lib.rs` (e.g.
362-
`backend/file_access.rs`).
363-
364-
**Why it matters.** "Put it in util" is how grab-bags grow; the
365-
position-conversion cluster in particular is used by every feature and
366-
deserves a findable home.
367-
368-
---
369-
370337
## `server.rs` carries a ~950-line workspace-init block
371338

372339
**What to do.** `src/server.rs` (2,821 lines) contains an
@@ -487,4 +454,23 @@ misses this code action, and its answers can contradict hover.
487454

488455
---
489456

457+
## Duplicate forward-delimiter scanners in `text_scan.rs`
458+
459+
**What to do.** `src/text_scan.rs` now hosts two near-identical
460+
forward delimiter-matching scanners side by side:
461+
`find_matching_delimiter_forward` (skips string literals only; single
462+
consumer, `completion/source/throws_analysis/scanning.rs`) and
463+
`find_matching_forward` (skips string literals *and* both PHP comment
464+
styles; many consumers). Determine whether the
465+
`throws_analysis/scanning.rs` call site can safely move to the
466+
comment-aware `find_matching_forward` (check whether it is a
467+
performance-sensitive hot path first, since the comment-unaware
468+
version is cheaper), and delete whichever scanner becomes redundant.
469+
470+
**Why it matters.** Two scanners answering "find the matching closing
471+
delimiter" invite a fix (e.g. a heredoc-awareness bug) to land in one
472+
copy and not the other.
473+
474+
---
475+
490476

src/backend.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
//! `Backend` behaviour that doesn't belong in the LSP-dispatch layer
2+
//! (`server.rs`) or in a specific feature module.
3+
4+
pub(crate) mod file_access;

src/backend/file_access.rs

Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
//! Per-file content and context accessors on [`Backend`].
2+
//!
3+
//! These read (and, for `clear_file_maps`, clear) the per-URI maps that
4+
//! back every feature's "what do we know about this file?" query:
5+
//! open-file content, the class index, the import table, and the
6+
//! namespace map. Centralising them here removes the repeated
7+
//! lock-and-unwrap boilerplate that used to be duplicated across the
8+
//! completion handler, definition resolver, and other consumers.
9+
10+
use std::sync::Arc;
11+
12+
use tower_lsp::lsp_types::Url;
13+
14+
use crate::Backend;
15+
use crate::types::{ClassInfo, FileContext};
16+
17+
impl Backend {
18+
/// Look up a class by its (possibly namespace-qualified) name in the
19+
/// in-memory `uri_classes_index`, without triggering any disk I/O.
20+
///
21+
/// The `class_name` can be:
22+
/// - A simple name like `"Customer"`
23+
/// - A namespace-qualified name like `"Klarna\\Customer"`
24+
/// - A fully-qualified name like `"\\Klarna\\Customer"` (leading `\` is stripped)
25+
///
26+
/// When a namespace prefix is present, the file's namespace (from
27+
/// `namespace_map`) must match for the class to be returned. This
28+
/// prevents `"Demo\\PDO"` from matching the global `PDO` stub.
29+
///
30+
/// Returns a shared `Arc<ClassInfo>` if found, or `None`.
31+
pub(crate) fn find_class_in_uri_classes_index(
32+
&self,
33+
class_name: &str,
34+
) -> Option<Arc<ClassInfo>> {
35+
// ── Fast path: O(1) lookup via fqn_index ──
36+
// For namespace-qualified names the FQN is the normalized name
37+
// itself. For bare names (no backslash) the FQN equals the
38+
// short name, which is also stored in the index.
39+
if let Some(cls) = self.fqn_class_index.read().get(class_name) {
40+
return Some(Arc::clone(cls));
41+
}
42+
43+
// The fqn_class_index is always populated before (or at the
44+
// same time as) uri_classes_index, so if the O(1) lookup above
45+
// missed, a linear scan would not find it either.
46+
None
47+
}
48+
49+
/// Get the content of a file by URI, trying open files first then disk.
50+
///
51+
/// This replaces the repeated pattern of locking `open_files`, looking
52+
/// up the URI, and falling back to reading from disk via
53+
/// `Url::to_file_path` + `std::fs::read_to_string`. Three call sites
54+
/// in the definition modules used this exact sequence.
55+
pub(crate) fn get_file_content(&self, uri: &str) -> Option<String> {
56+
if let Some(content) = self.open_files.read().get(uri) {
57+
return Some(String::clone(content));
58+
}
59+
60+
// Embedded class stubs live under synthetic `phpantom-stub://`
61+
// URIs and have no on-disk file. Retrieve the raw source from
62+
// the stub_index keyed by the class short name (the URI path).
63+
if let Some(class_name) = uri.strip_prefix("phpantom-stub://") {
64+
let stub_idx = self.stub_index.read();
65+
return stub_idx.get(class_name).map(|s| s.to_string());
66+
}
67+
68+
// Embedded function stubs use `phpantom-stub-fn://` URIs.
69+
// The path component is the function name used as key in
70+
// stub_function_index.
71+
if let Some(func_name) = uri.strip_prefix("phpantom-stub-fn://") {
72+
let stub_fn_idx = self.stub_function_index.read();
73+
return stub_fn_idx.get(func_name).map(|s| s.to_string());
74+
}
75+
76+
let path = Url::parse(uri).ok()?.to_file_path().ok()?;
77+
std::fs::read_to_string(path).ok()
78+
}
79+
80+
/// Retrieve file content as a cheap `Arc<String>` reference when the
81+
/// file is in `open_files`. Falls back to reading from disk (which
82+
/// wraps the result in a new `Arc`).
83+
///
84+
/// Prefer this over [`get_file_content`] in hot paths where the
85+
/// content will be shared across tasks or stored for the duration
86+
/// of a request, since it avoids deep-cloning the file string.
87+
pub(crate) fn get_file_content_arc(&self, uri: &str) -> Option<Arc<String>> {
88+
if let Some(content) = self.open_files.read().get(uri) {
89+
return Some(Arc::clone(content));
90+
}
91+
92+
// Embedded class stubs live under synthetic `phpantom-stub://`
93+
// URIs and have no on-disk file.
94+
if let Some(class_name) = uri.strip_prefix("phpantom-stub://") {
95+
let stub_idx = self.stub_index.read();
96+
return stub_idx.get(class_name).map(|s| Arc::new(s.to_string()));
97+
}
98+
99+
// Embedded function stubs use `phpantom-stub-fn://` URIs.
100+
if let Some(func_name) = uri.strip_prefix("phpantom-stub-fn://") {
101+
let stub_fn_idx = self.stub_function_index.read();
102+
return stub_fn_idx.get(func_name).map(|s| Arc::new(s.to_string()));
103+
}
104+
105+
let path = Url::parse(uri).ok()?.to_file_path().ok()?;
106+
std::fs::read_to_string(path).ok().map(Arc::new)
107+
}
108+
109+
/// Public helper for tests: get the uri_classes_index entry for a given URI.
110+
pub fn get_classes_for_uri(&self, uri: &str) -> Option<Vec<ClassInfo>> {
111+
self.uri_classes_index
112+
.read()
113+
.get(uri)
114+
.map(|classes| classes.iter().map(|c| ClassInfo::clone(c)).collect())
115+
}
116+
117+
/// Gather the per-file context (classes, use-map, namespace) in one call.
118+
///
119+
/// This replaces the repeated lock-and-unwrap boilerplate that was
120+
/// duplicated across the completion handler, definition resolver,
121+
/// implementation resolver, and variable definition modules. Each of
122+
/// those sites used to have three nearly-identical blocks acquiring
123+
/// `uri_classes_index`, `use_map`, and `namespace_map` locks and
124+
/// extracting the entry for a given URI.
125+
pub(crate) fn file_context(&self, uri: &str) -> FileContext {
126+
let classes = self
127+
.uri_classes_index
128+
.read()
129+
.get(uri)
130+
.cloned()
131+
.unwrap_or_default();
132+
133+
// The legacy use_map (short name → FQN from `use` statements)
134+
// remains the canonical import table. `resolved_names` is a
135+
// supplementary data source for consumers that can query by
136+
// byte offset — it must NOT replace the use_map because
137+
// `to_use_map()` only contains names that are actually
138+
// *referenced* in the code, not all *declared* imports.
139+
// The unused-imports diagnostic relies on seeing declared-but-
140+
// unreferenced imports.
141+
let use_map = self
142+
.file_imports
143+
.read()
144+
.get(uri)
145+
.cloned()
146+
.unwrap_or_default();
147+
148+
let namespace = self
149+
.file_namespaces
150+
.read()
151+
.get(uri)
152+
.and_then(|spans| spans.first())
153+
.and_then(|s| s.namespace.clone());
154+
155+
let resolved_names = self.resolved_names.read().get(uri).cloned();
156+
157+
FileContext {
158+
classes,
159+
use_map,
160+
namespace,
161+
resolved_names,
162+
}
163+
}
164+
165+
/// Like [`file_context`](Self::file_context) but resolves the namespace
166+
/// for the namespace block that contains `byte_offset`.
167+
///
168+
/// In single-namespace files this returns the same result as
169+
/// `file_context`. In multi-namespace files it picks the correct
170+
/// namespace block for the cursor position.
171+
pub(crate) fn file_context_at(&self, uri: &str, byte_offset: u32) -> FileContext {
172+
let classes = self
173+
.uri_classes_index
174+
.read()
175+
.get(uri)
176+
.cloned()
177+
.unwrap_or_default();
178+
let use_map = self
179+
.file_imports
180+
.read()
181+
.get(uri)
182+
.cloned()
183+
.unwrap_or_default();
184+
let namespace = self.namespace_at_offset(uri, byte_offset);
185+
let resolved_names = self.resolved_names.read().get(uri).cloned();
186+
187+
FileContext {
188+
classes,
189+
use_map,
190+
namespace,
191+
resolved_names,
192+
}
193+
}
194+
195+
/// Return the namespace that contains the given byte offset in a file.
196+
///
197+
/// For single-namespace files (the common case) this returns the file's
198+
/// only namespace. For multi-namespace files it finds the namespace
199+
/// block whose byte range contains `byte_offset`. Returns `None` when
200+
/// the offset is in the global namespace or the file has no namespace.
201+
pub(crate) fn namespace_at_offset(&self, uri: &str, byte_offset: u32) -> Option<String> {
202+
let nmap = self.file_namespaces.read();
203+
let spans = nmap.get(uri)?;
204+
// Try to find the namespace block containing the offset.
205+
for span in spans {
206+
if byte_offset >= span.start && byte_offset <= span.end {
207+
return span.namespace.clone();
208+
}
209+
}
210+
// Fallback: if the offset is past all namespace blocks (e.g.
211+
// code after the last closing brace), return the last namespace.
212+
spans.last().and_then(|s| s.namespace.clone())
213+
}
214+
215+
/// Return the first namespace declared in a file.
216+
///
217+
/// For single-namespace files this is the file's namespace. For
218+
/// multi-namespace files this returns the first block's namespace,
219+
/// which may not be correct for all positions in the file. Prefer
220+
/// [`namespace_at_offset`](Self::namespace_at_offset) when a cursor
221+
/// position is available.
222+
pub(crate) fn first_file_namespace(&self, uri: &str) -> Option<String> {
223+
self.file_namespaces
224+
.read()
225+
.get(uri)
226+
.and_then(|spans| spans.first())
227+
.and_then(|s| s.namespace.clone())
228+
}
229+
230+
/// Return the import table (short name → FQN) for a file.
231+
///
232+
/// Returns the legacy `use_map` which contains all *declared*
233+
/// imports from `use` statements, regardless of whether they are
234+
/// actually referenced in the code. This is the correct source
235+
/// for consumers that need the full import table (unused-import
236+
/// detection, import-class code actions, name resolution helpers).
237+
///
238+
/// For consumers that can resolve names by byte offset, prefer
239+
/// querying `resolved_names` directly via [`file_context`] instead.
240+
pub(crate) fn file_use_map(&self, uri: &str) -> std::collections::HashMap<String, String> {
241+
self.file_imports
242+
.read()
243+
.get(uri)
244+
.cloned()
245+
.unwrap_or_default()
246+
}
247+
248+
/// Remove a file's entries from `uri_classes_index`, `use_map`, and `namespace_map`.
249+
///
250+
/// This is the mirror of [`file_context`](Self::file_context): where that
251+
/// method *reads* the three maps, this method *clears* them for a given URI.
252+
/// Called from `did_close` to clean up state when a file is closed.
253+
pub(crate) fn clear_file_maps(&self, uri: &str) {
254+
// Drop per-file maps that are only needed while the file is
255+
// open. uri_classes_index is redundant with fqn_class_index once indexing is
256+
// complete — GTD falls back to fqn_uri_index + parse_and_cache_file
257+
// when the uri_classes_index entry is missing.
258+
self.uri_classes_index.write().remove(uri);
259+
self.symbol_maps.write().remove(uri);
260+
self.evict_reference_index_uri(uri);
261+
self.file_imports.write().remove(uri);
262+
self.resolved_names.write().remove(uri);
263+
self.file_namespaces.write().remove(uri);
264+
// Parse errors are stored per file during update_ast and consumed
265+
// by the syntax-error diagnostic. Without this removal the last
266+
// parse-error vector for every file ever opened (or deleted from
267+
// disk) stays resident for the whole session.
268+
self.parse_errors.write().remove(uri);
269+
// NOTE: We intentionally keep fqn_uri_index and fqn_class_index intact.
270+
// fqn_uri_index maps FQN → URI so GTD can locate the file, and
271+
// fqn_class_index keeps the full ClassInfo for cross-file resolution.
272+
// The file will be re-parsed from disk on next access via
273+
// parse_and_cache_file when needed (issue #99).
274+
}
275+
}

0 commit comments

Comments
 (0)