diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 682e14bf3..c8e49d2d5 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -748,6 +748,8 @@ The merged pipeline works in three steps: (1) load `autoload_classmap.php` into When self-scanning with a `composer.json` present, the scanner reads `autoload.psr-4`, `autoload-dev.psr-4`, `autoload.classmap`, and `autoload-dev.classmap` to determine which directories to walk. PSR-4 directories are filtered: only classes whose FQN matches the namespace prefix plus the relative file path are included. Vendor packages are discovered from `vendor/composer/installed.json` (both Composer 1 and 2 formats); the JSON packages array is borrowed rather than cloned to avoid allocating a copy of the entire vendor manifest. All directory walkers (full-scan, PSR-4 scanner, vendor package scanner, and go-to-implementation file collector) use the `ignore` crate for gitignore-aware traversal. Hidden directories are skipped automatically, and `.gitignore` rules are respected at every level. When no `composer.json` exists at all, the scanner falls back to walking all `.php` files under the workspace root. +**Directory symlinks.** The walkers descend into a symlinked directory, so a project that keeps its framework or a shared library outside the repository and links it into the tree gets the linked code indexed with the rest. Every path keeps the symlink spelling rather than the target's, which is what makes a file reached through the link the same file the editor opened. `LinkClaims` gives each walk one visit per target directory: the walk's own roots and its skipped trees are claimed up front, and each link claims its target the first time it is descended, so two links to one tree, a link pointing back at something the walk already covers, and a chain of directories holding several links apiece all cost one pass rather than one per route. `orchestra/testbench-core` ships `laravel/vendor -> /vendor`, which is why the skipped trees are claimed and not merely pruned by path. Each link the index reached through also gets a watcher based at it (see `build_watched_file_registration`), since a workspace-relative watcher pattern never covers a path outside the workspace folders. + The scan results are converted to URI strings and inserted into `fqn_uri_index`. Everything downstream (resolution, diagnostics, go-to-definition) uses the unified index. **Redundant I/O elimination:** `init_single_project` parses `composer.json` once and passes the pre-parsed `serde_json::Value` to `build_self_scan_composer`. Previously each function re-read and re-parsed the file independently. diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a9c9ef0e5..9b1cdb786 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Directories reached through a symlink are indexed with the rest of the project.** A project that keeps its framework or a shared library outside the repository and links it into the tree (`kdhelp -> ../kdhelp`, say) now resolves the symbols there like any other project code. Indexed paths keep the symlink spelling, so a file reached through the link is the same file the editor opened, and a tree is indexed once however many links lead to it. Changes another tool writes inside a linked directory are picked up as well on editors that can watch a path outside the project; where they cannot, such a change needs a window reload, while a file open in the editor always re-parses as it is edited. Contributed by @liudashuang. Closes #383. - **Formatting from the command line.** `phpantom_lsp format` formats every PHP file and Blade template in a project with the same formatter the editor runs on save, and `phpantom_lsp format --check` reports the files that are not formatted and exits non-zero without writing anything, so a CI job can require that a pull request ran the formatter. A run honours whatever the project already formats with, a Laravel Pint, php-cs-fixer, or PHP_CodeSniffer it depends on, and the built-in formatter otherwise, exactly as the editor resolves it, and opens with a line naming what it resolved so a CI log records which formatter enforced the result. Templates whose indentation is output rather than layout are left alone and never fail a check, and formatting turned off in `.phpantom.toml` is reported as such rather than passing as a project where every file happens to be formatted. Paths can be named to restrict the run, `--format github` annotates the pull request diff, and `--format json` is shaped like the object `analyze` and `fix` emit. - **Storage disk names are navigable wherever Laravel accepts one.** `Storage::disk()`, `fake()`, `persistentFake()`, `forgetDisk()`, and the `#[Storage]` container attribute now complete from `config/filesystems.php`; hover shows the config key, Ctrl+Click opens its declaration, and find-references links every use. Calls that require a configured disk report misspellings, while test fakes and disk eviction keep accepting the ad-hoc names Laravel permits at runtime. Contributed by @shuvroroy. - **Qualified names can be converted to imports in one action.** Invoke the refactoring on an absolute or relative qualified class, function, or constant to add the matching `use`, `use function`, or `use const` declaration and shorten every equivalent usage in the file. When the natural short name is already imported from elsewhere, the new import receives a namespace-derived alias instead. A companion action on the same cursor position does the whole namespace at once, importing every qualified class, function, and constant it contains and aliasing the ones whose short names collide. Contributed by @calebdw. diff --git a/src/analyse/run.rs b/src/analyse/run.rs index 48700131e..3ee017b4b 100644 --- a/src/analyse/run.rs +++ b/src/analyse/run.rs @@ -250,6 +250,10 @@ fn collect_php_files( let skip_vendor = skip_vendor.to_vec(); let filter_excludes = std::sync::Arc::clone(filters); + // Same one-visit-per-target rule the shared workspace walker applies, + // so `analyze` cannot report the same file once per spelling a chain + // of links gives it. + let claims = crate::classmap_scanner::LinkClaims::new([dir.to_path_buf()], None); let walker = WalkBuilder::new(dir) .git_ignore(true) .git_global(true) @@ -257,14 +261,19 @@ fn collect_php_files( .hidden(true) .parents(true) .ignore(true) + .follow_links(true) .filter_entry(move |entry| { let is_dir = entry.file_type().is_some_and(|ft| ft.is_dir()); - if is_dir - && !skip_vendor.is_empty() - && let Ok(canonical) = entry.path().canonicalize() - && skip_vendor.iter().any(|v| canonical.starts_with(v)) - { - return false; + if is_dir { + if !skip_vendor.is_empty() + && let Ok(canonical) = entry.path().canonicalize() + && skip_vendor.iter().any(|v| canonical.starts_with(v)) + { + return false; + } + if entry.depth() > 0 && entry.path_is_symlink() && !claims.claim(entry.path()) { + return false; + } } !filter_excludes.is_excluded_entry(entry.path(), is_dir) }) @@ -488,4 +497,61 @@ mod tests { assert_eq!(files, vec![root.join("src/A.php"), root.join("src/B.php")]); } + + #[test] + fn discover_user_files_follows_interior_symlink_when_enabled() { + // CLI analyse's user-file walker keeps the same symlink + // contract as the workspace walkers (issue #383). + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + let real = dir.path().join("real"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&real).unwrap(); + std::fs::write(real.join("Hidden.php"), " usize { pub fn scan_directories( dirs: &[PathBuf], vendor_dir_paths: &[PathBuf], + followed: Option<&FollowedLinks>, ) -> HashMap { let skip_paths = HashSet::new(); let opts = WalkOptions::new( vendor_dir_paths.to_vec(), &skip_paths, IndexFilters::empty(), + followed, ); let paths: Vec = walk_roots(dirs, &opts).into_iter().flatten().collect(); scan_files_parallel_classes(&paths, None) @@ -101,6 +103,7 @@ pub fn scan_psr4_directories( psr4: &[(String, PathBuf)], classmap_dirs: &[PathBuf], vendor_dir_paths: &[PathBuf], + followed: Option<&FollowedLinks>, ) -> HashMap { scan_psr4_directories_with_skip( psr4, @@ -109,6 +112,7 @@ pub fn scan_psr4_directories( &HashSet::new(), &IndexFilters::empty(), None, + followed, ) } @@ -124,12 +128,14 @@ pub fn scan_psr4_directories_with_skip( skip_paths: &HashSet, filters: &std::sync::Arc, progress: Option<&ScanProgress>, + followed: Option<&FollowedLinks>, ) -> HashMap { // ── Walk the PSR-4 and classmap roots in one parallel pass ────── let opts = WalkOptions::new( vendor_dir_paths.to_vec(), skip_paths, std::sync::Arc::clone(filters), + followed, ); let mut roots: Vec = psr4.iter().map(|(_, dir)| dir.clone()).collect(); roots.extend(classmap_dirs.iter().cloned()); @@ -173,6 +179,7 @@ pub fn scan_vendor_packages(workspace_root: &Path, vendor_dir: &str) -> Workspac &HashSet::new(), &IndexFilters::empty(), None, + None, ) } @@ -396,6 +403,7 @@ pub fn scan_vendor_packages_with_skip( explicit_deps: &HashSet, filters: &std::sync::Arc, progress: Option<&ScanProgress>, + followed: Option<&FollowedLinks>, ) -> WorkspaceScanResult { let vendor_path = workspace_root.join(vendor_dir); @@ -476,6 +484,7 @@ pub fn scan_vendor_packages_with_skip( vec![vendor_path.clone()], skip_paths, std::sync::Arc::clone(filters), + followed, ); let mut roots: Vec = Vec::new(); for (_, sources) in &collected { @@ -539,8 +548,9 @@ pub fn scan_vendor_packages_with_skip( pub fn scan_workspace_fallback( workspace_root: &Path, vendor_dir_paths: &[PathBuf], + followed: Option<&FollowedLinks>, ) -> HashMap { - scan_directories(&[workspace_root.to_path_buf()], vendor_dir_paths) + scan_directories(&[workspace_root.to_path_buf()], vendor_dir_paths, followed) } /// Scan `files` in parallel, calling `emit` on each to append the @@ -820,6 +830,7 @@ pub fn scan_workspace_fallback_full( skip_dirs: &HashSet, filters: &std::sync::Arc, progress: Option<&ScanProgress>, + followed: Option<&FollowedLinks>, ) -> WorkspaceScanResult { // Phase 1: collect file paths let skip_paths = HashSet::new(); @@ -827,6 +838,7 @@ pub fn scan_workspace_fallback_full( skip_dirs.iter().cloned().collect(), &skip_paths, std::sync::Arc::clone(filters), + followed, ); let php_files: Vec<(PathBuf, crate::ClassCompletionOrigin)> = walk_roots(&[workspace_root.to_path_buf()], &opts) @@ -948,6 +960,11 @@ struct WalkOptions<'a> { skip_paths: &'a HashSet, /// Compiled `[indexing]` exclude globs and extra PHP extensions. filters: std::sync::Arc, + /// Where to report the directory symlinks this walk descends + /// through, so the client can be asked to watch the trees behind + /// them. `None` for a walk whose caller has no watchers to register + /// (the `analyze`, `fix`, and `format` pipelines). + followed: Option<&'a FollowedLinks>, } impl<'a> WalkOptions<'a> { @@ -955,11 +972,13 @@ impl<'a> WalkOptions<'a> { skip_dirs: Vec, skip_paths: &'a HashSet, filters: std::sync::Arc, + followed: Option<&'a FollowedLinks>, ) -> Self { Self { skip_dirs: std::sync::Arc::new(skip_dirs), skip_paths, filters, + followed, } } } @@ -1012,11 +1031,17 @@ fn walk_roots(roots: &[PathBuf], opts: &WalkOptions) -> Vec> { return out; }; + // Every root is claimed up front, so a link inside one root pointing + // into another is skipped in favour of that root's own walk. let mut builder = super::workspace_walk_builder( first_root, std::sync::Arc::clone(&opts.skip_dirs), std::sync::Arc::clone(&opts.filters), false, + super::LinkClaims::new( + distinct_roots.iter().map(|r| r.to_path_buf()), + opts.followed, + ), ); for dir in other_roots { builder.add(dir); @@ -1038,9 +1063,11 @@ fn walk_roots(roots: &[PathBuf], opts: &WalkOptions) -> Vec> { if file_type.is_some_and(|ft| ft.is_dir()) || !filters.is_php_file(path) || skip_paths.contains(path) - // `ignore` reports a symlink's own type, so confirm the - // target is a regular file before indexing it. The tests - // above keep this stat off the common path. + // `ignore` reports the target's type for a symlink it + // followed, and the symlink's own for one it did not + // (a claimed target, a broken link), so confirm the + // target is a regular file before indexing it. The + // tests above keep this stat off the common path. || !(file_type.is_some_and(|ft| ft.is_file()) || path.is_file()) { return WalkState::Continue; diff --git a/src/classmap_scanner/discovery_tests.rs b/src/classmap_scanner/discovery_tests.rs index 49fd4a7d5..3b2cf76b9 100644 --- a/src/classmap_scanner/discovery_tests.rs +++ b/src/classmap_scanner/discovery_tests.rs @@ -19,7 +19,7 @@ fn scan_directories_finds_classes() { .unwrap(); let vendor_dir_paths = vec![dir.path().join("vendor")]; - let classmap = scan_directories(&[src], &vendor_dir_paths); + let classmap = scan_directories(&[src], &vendor_dir_paths, None); assert_eq!(classmap.len(), 2); assert!(classmap.contains_key("App\\Models\\User")); assert!(classmap.contains_key("App\\Models\\Order")); @@ -32,7 +32,7 @@ fn scan_directories_skips_hidden() { std::fs::create_dir_all(&hidden).unwrap(); std::fs::write(hidden.join("Secret.php"), " ../kdhelp` style link to a framework tree kept outside +// the repository gets indexed with the rest of the project. Every path +// the walk yields keeps the symlink spelling — the contract the index and +// the URIs returned to the editor depend on — and each target is entered +// once however many links reach it. + +#[test] +fn walk_roots_skips_a_link_pointing_at_a_skipped_tree() { + // `orchestra/testbench-core` ships `laravel/vendor -> /vendor`, + // a link back at the vendor tree the walk is already covering through + // `installed.json`. Descending it would index every vendor package a + // second time under a path inside testbench, and a third time under + // that copy's own copy of the link. A skipped tree has to be claimed + // up front the same way a root is. + let dir = tempfile::tempdir().unwrap(); + let vendor = dir.path().join("vendor"); + let pkg = vendor.join("acme/pkg"); + std::fs::create_dir_all(pkg.join("laravel")).unwrap(); + std::fs::create_dir_all(vendor.join("other")).unwrap(); + std::fs::write(vendor.join("other/Other.php"), " = walk_roots(std::slice::from_ref(&pkg), &opts) + .into_iter() + .flatten() + .collect(); + + assert!( + files.iter().any(|p| p.ends_with("Pkg.php")), + "the package's own files must still be found: {files:?}" + ); + assert!( + !files.iter().any(|p| p.starts_with(&link)), + "a link back at the skipped vendor tree must not be walked: {files:?}" + ); +} + +#[test] +fn walk_roots_follows_interior_symlink() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + let real = dir.path().join("real"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&real).unwrap(); + std::fs::write(real.join("Hidden.php"), " = walk_roots(&[root], &opts).into_iter().flatten().collect(); + let linked = files + .iter() + .find(|p| p.ends_with("Hidden.php")) + .unwrap_or_else(|| panic!("linked file must be indexed: {files:?}")); + assert!( + linked.starts_with(&link), + "paths must keep the symlink spelling: {linked:?} vs {link:?}" + ); +} + +#[test] +fn walk_roots_attributes_a_followed_link_to_the_root_that_reached_it() { + // `walk_roots` puts every root in one `ignore` walk and attributes + // each file to a root by its depth, so a root's own files are the + // ones its own descent produced. Following a symlink must not + // disturb that: the walk goes deeper under the link spelling, which + // is still below the root that owns it. A second, unrelated root + // alongside it is what a directory named directly (rather than + // reached through a link) looks like to this walk, and the two must + // not bleed into each other. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + let real = dir.path().join("real"); + let other = dir.path().join("other"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&real).unwrap(); + std::fs::create_dir_all(&other).unwrap(); + std::fs::write(real.join("Linked.php"), " = walk_roots(std::slice::from_ref(&root), &opts) + .into_iter() + .flatten() + .collect(); + let linked = files + .iter() + .find(|p| p.ends_with("Deep.php")) + .unwrap_or_else(|| panic!("nested linked file must be indexed: {files:?}")); + // Both link spellings survive transitively: the yielded path is + // ws/kdhelp/soa/Deep.php, never the real ext1/… / ext2/… targets. + let expected_prefix = root.join("kdhelp").join("soa"); + assert!( + linked.starts_with(&expected_prefix), + "nested links must keep the full symlink prefix: {linked:?} vs {expected_prefix:?}" + ); +} + +/// Create a directory symlink, spelled the way each platform needs. +#[cfg(any(unix, windows))] +fn link_dir(target: &std::path::Path, link: &std::path::Path) { + #[cfg(unix)] + std::os::unix::fs::symlink(target, link).unwrap(); + #[cfg(windows)] + std::os::windows::fs::symlink_dir(target, link).unwrap(); +} + +#[test] +fn walk_roots_walks_a_link_target_once_however_many_links_reach_it() { + // `ignore` only refuses a link pointing at one of its own ancestors, + // so two links to the same tree are not a cycle to it and it walks + // that tree twice. Both copies land in the index, and every class in + // them resolves to whichever spelling happened to win. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + let ext = dir.path().join("ext"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&ext).unwrap(); + std::fs::write(ext.join("Dup.php"), " = walk_roots(&[root], &opts).into_iter().flatten().collect(); + assert_eq!( + files.len(), + 1, + "the linked tree must be walked once, not once per link: {files:?}" + ); +} + +#[test] +fn walk_roots_keeps_the_real_spelling_of_a_link_back_into_a_root() { + // A link pointing back inside the workspace is not a cycle either, + // and the directory it names is one the walk covers anyway. The + // roots are claimed before the walk starts, so the spelling the walk + // already had wins and the link is not descended into. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + let src = root.join("src"); + std::fs::create_dir_all(&src).unwrap(); + std::fs::write(src.join("Inside.php"), " = walk_roots(std::slice::from_ref(&root), &opts) + .into_iter() + .flatten() + .collect(); + assert_eq!( + files, + vec![src.join("Inside.php")], + "a link back into the workspace must lose to the real path: {files:?}" + ); +} + +#[test] +fn walk_roots_does_not_fan_out_through_a_diamond_of_links() { + // Five directories holding two links apiece, each pair pointing at + // the next directory. No link points at an ancestor, so nothing here + // is a cycle and `ignore` walks every one of the 2^5 routes to the + // leaf. Claiming each target the first time a link reaches it turns + // the fan-out back into a single descent. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + std::fs::create_dir_all(&root).unwrap(); + let levels: Vec = (0..6).map(|i| dir.path().join(format!("d{i}"))).collect(); + for level in &levels { + std::fs::create_dir_all(level).unwrap(); + } + std::fs::write(levels[5].join("Leaf.php"), " = walk_roots(&[root], &opts).into_iter().flatten().collect(); + assert_eq!( + files.len(), + 1, + "a diamond of links must not multiply the leaf: {files:?}" + ); +} + +#[test] +fn walk_roots_follows_symlink_cycle_safely() { + // A symlink pointing back at the workspace itself must terminate: + // `ignore`'s parallel walker detects the cycle via dev+inode handles + // and skips the re-entered directory. Without the cycle guard this + // test would walk forever. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::write(root.join("App.php"), " = walk_roots(&[root], &opts).into_iter().flatten().collect(); + assert!( + files.iter().any(|p| p.ends_with("App.php")), + "workspace files must still be found next to a cycle: {files:?}" + ); +} + +#[test] +fn walk_roots_prunes_a_link_into_a_skipped_tree_by_its_target() { + // `skip_dirs` prunes by literal path, which only stops the walk + // reaching a tree directly; a link into it arrives under the link's + // spelling and slips past. Claiming the skipped trees up front closes + // that: a tree another pipeline already covers (a vendor directory + // scanned through `installed.json`, a monorepo subproject) must not be + // indexed a second time just because something links to it. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + let real_vendor = dir.path().join("real-vendor"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&real_vendor).unwrap(); + std::fs::write(real_vendor.join("Pkg.php"), " = walk_roots(&[root], &opts).into_iter().flatten().collect(); + assert!( + files.iter().any(|p| p.ends_with("Own.php")), + "the walk's own files must still be found: {files:?}" + ); + assert!( + !files.iter().any(|p| p.ends_with("Pkg.php")), + "a link into a skipped tree must not walk it: {files:?}" + ); +} + // ── [indexing] exclude / extensions filters ────────────────────── /// Compile filters rooted at the test workspace. @@ -759,7 +1086,7 @@ fn workspace_scan_honors_exclude_globs() { let filters = test_filters(dir.path(), &["generated", "fixtures/"], &[]); let skip = std::collections::HashSet::new(); - let result = scan_workspace_fallback_full(dir.path(), &skip, &filters, None); + let result = scan_workspace_fallback_full(dir.path(), &skip, &filters, None, None); assert!(result.classmap.contains_key("Keep")); assert!( @@ -784,7 +1111,7 @@ fn workspace_scan_honors_extra_extensions() { let filters = test_filters(dir.path(), &[], &["module"]); let skip = std::collections::HashSet::new(); - let result = scan_workspace_fallback_full(dir.path(), &skip, &filters, None); + let result = scan_workspace_fallback_full(dir.path(), &skip, &filters, None, None); assert!(result.classmap.contains_key("HooksHelper")); assert!(result.function_index.contains_key("hooks_help")); diff --git a/src/classmap_scanner/filters.rs b/src/classmap_scanner/filters.rs index 2cc6e43ec..83aa1f196 100644 --- a/src/classmap_scanner/filters.rs +++ b/src/classmap_scanner/filters.rs @@ -32,14 +32,24 @@ use ignore::gitignore::{Gitignore, GitignoreBuilder}; /// exclude` matches are pruned through `filters`. /// /// Dotfiles are skipped unless `include_dotfiles` is set, in which case -/// `.git` itself is still pruned. Callers add further roots, set the -/// thread count, or keep the walk serial as their scan needs. +/// `.git` itself is still pruned. A directory symlink is descended into +/// the first time the walk reaches its target and skipped every later +/// time, so no directory is walked twice under two spellings; see +/// [`LinkClaims`]. Callers add further roots, set the thread count, or +/// keep the walk serial as their scan needs. pub fn workspace_walk_builder( root: &Path, skip_dirs: Arc>, filters: Arc, include_dotfiles: bool, + claims: LinkClaims, ) -> WalkBuilder { + // Pruning a skipped tree by path only stops the walk reaching it + // *directly*. A link pointing into one arrives under a different + // path, so the prune above never fires and the tree is walked after + // all, under a spelling nested inside whatever held the link. + claims.cover(skip_dirs.iter().cloned()); + let mut builder = WalkBuilder::new(root); builder .git_ignore(true) @@ -48,6 +58,7 @@ pub fn workspace_walk_builder( .hidden(!include_dotfiles) .parents(true) .ignore(true) + .follow_links(true) .filter_entry(move |entry| { let is_dir = entry.file_type().is_some_and(|ft| ft.is_dir()); if is_dir { @@ -57,12 +68,184 @@ pub fn workspace_walk_builder( if skip_dirs.iter().any(|dir| dir == entry.path()) { return false; } + // Depth 0 is a root the caller asked for by name, even + // when it is itself a symlink, so it is never a second + // spelling of anything. + if entry.depth() > 0 && entry.path_is_symlink() && !claims.claim(entry.path()) { + return false; + } } !filters.is_excluded_entry(entry.path(), is_dir) }); builder } +/// The link targets a single walk has already committed to descending +/// into, so no directory is walked twice under two spellings. +/// +/// One instance covers one walk. A walk that reports into a +/// [`FollowedLinks`] registry also tells it which links it went through, +/// which is what lets the watcher registration ask the client to watch +/// the trees behind them. +/// +/// `ignore` refuses a symlink pointing at one of its own ancestors, +/// which bounds a true cycle, and nothing else. Three shapes slip past +/// it, all of them real: two links to the same tree, a link pointing +/// back inside the workspace the walk is already covering, and a chain +/// of directories holding two links apiece, which reaches the leaf 2^n +/// ways without ever forming a cycle. Each one walks the same files +/// again under a different path, so a class resolves to an arbitrary +/// spelling of its file, find-references reports every hit as many times +/// as the walk found it, and the fan-out case buys that with exponential +/// work. +/// +/// Claiming a target's real path the first time a link reaches it makes +/// every directory visited once however many ways the links spell it. +/// The walk's own roots are claimed up front, so a link pointing back +/// inside one of them loses to the spelling the walk already had. Two +/// links to the *same* tree outside the roots are equally arbitrary and +/// a parallel walk picks whichever gets there first; that one is not +/// reproducible across runs, where indexing both was reproducibly wrong. +/// +/// A claim covers the whole walk rather than the root that made it, even +/// though [`walk_roots`](super::discovery) otherwise keeps each root's +/// files separate so it can namespace-filter them against that root's own +/// PSR-4 prefix. Two roots whose links converge on one tree therefore see +/// it once between them, which costs nothing real: a PHP file declares a +/// single namespace, so at most one root's prefix could ever have +/// accepted it, and the classmap the rest feeds is a flat first-wins map +/// where the second copy is discarded anyway. +/// +/// Only directories are tracked. A symlinked file costs a bounded amount +/// of duplicate work, and paying a `canonicalize` per file to find that +/// out would cost more than it saves. +#[derive(Clone)] +pub struct LinkClaims { + claimed: Arc>>, + followed: Option, +} + +impl LinkClaims { + /// Start a walk with `roots` already covered, reporting the links it + /// descends through into `followed` when one is given. + /// + /// A root that cannot be canonicalized is left out rather than + /// failing the walk: the walk still yields it, and the only loss is + /// that a link pointing back into it is followed instead of skipped. + pub fn new(roots: impl IntoIterator, followed: Option<&FollowedLinks>) -> Self { + let claimed = roots + .into_iter() + .filter_map(|root| std::fs::canonicalize(root).ok()) + .collect(); + Self { + claimed: Arc::new(parking_lot::Mutex::new(claimed)), + followed: followed.cloned(), + } + } + + /// Treat `paths` as trees the walk already accounts for, so a link + /// pointing into one of them is skipped in favour of the spelling that + /// tree is covered under. + /// + /// This is what a walk's `skip_dirs` need: a vendor tree scanned + /// through `installed.json`, or a monorepo subproject another pipeline + /// walks, is already indexed under its own path. + /// `orchestra/testbench-core` ships `laravel/vendor -> /vendor` + /// and is installed in a great many Laravel projects, so a walk of the + /// vendor packages meets exactly this link and would otherwise index + /// every package a second time beneath it. + pub(crate) fn cover(&self, paths: impl IntoIterator) { + let covered = paths + .into_iter() + .filter_map(|path| std::fs::canonicalize(path).ok()); + self.claimed.lock().extend(covered); + } + + /// Whether the walk should descend through the directory symlink at + /// `link`, claiming its target when it should. + /// + /// A broken link, or one whose target is already covered by a root + /// or by an earlier link, answers `false`. + pub(crate) fn claim(&self, link: &Path) -> bool { + let Ok(target) = std::fs::canonicalize(link) else { + return false; + }; + let mut claimed = self.claimed.lock(); + if claimed.iter().any(|seen| target.starts_with(seen)) { + return false; + } + claimed.push(target.clone()); + drop(claimed); + if let Some(followed) = &self.followed { + followed.record(link.to_path_buf(), target); + } + true + } +} + +/// Every directory symlink the workspace walks have indexed through, +/// paired with the real directory it resolves to. +/// +/// Owned by the `Backend` and filled in by the walks themselves, because +/// a link is only interesting once something was indexed behind it. Two +/// consumers need it, and both would otherwise be guessing: +/// +/// - **Watcher registration.** A client watches its workspace folders, +/// and a tree behind a link is not in one of them. Each link here +/// becomes a [`RelativePattern`](tower_lsp::lsp_types::RelativePattern) +/// watcher based at the link, which is what asks the client for those +/// events. +/// - **Event paths.** A client that resolves the base it was handed +/// reports the real path, which matches nothing in an index that holds +/// the symlink spelling. [`Self::to_link_spelling`] maps it back. +/// +/// Links accumulate: a rediscovery walk re-records the ones still there +/// rather than starting over, so a link deleted from disk leaves a +/// watcher for a path that no longer exists until the session ends. The +/// client is watching a path that produces no events, which costs one +/// dead registration and nothing else. +#[derive(Clone, Default)] +pub struct FollowedLinks { + /// Link path as the walk spelled it → the canonical directory it + /// resolves to. Keyed by link so re-walking is idempotent, and small + /// enough (one entry per symlink a project deliberately checked in) + /// that the reverse lookup is a scan. + links: Arc>>, +} + +impl FollowedLinks { + fn record(&self, link: PathBuf, target: PathBuf) { + self.links.write().insert(link, target); + } + + /// Whether any walk has indexed through a symlink yet. + pub fn is_empty(&self) -> bool { + self.links.read().is_empty() + } + + /// The link spellings, sorted, for the watcher registration to base + /// its patterns on. + pub fn link_paths(&self) -> Vec { + self.links.read().keys().cloned().collect() + } + + /// Rewrite a real path that falls inside a followed link's target + /// into the spelling the index holds, or `None` when it is already a + /// path the walk could have produced. + /// + /// The longest matching target wins, so a link nested inside another + /// link's tree maps to the nearer of the two. + pub fn to_link_spelling(&self, path: &Path) -> Option { + let links = self.links.read(); + let (link, rest) = links + .iter() + .filter(|(link, _)| !path.starts_with(link)) + .filter_map(|(link, target)| path.strip_prefix(target).ok().map(|rest| (link, rest))) + .min_by_key(|(_, rest)| rest.components().count())?; + Some(link.join(rest)) + } +} + /// Compiled exclude matcher and extra-extension set for file discovery. pub struct IndexFilters { /// Compiled `[indexing] exclude` globs, `None` when no valid @@ -346,4 +529,49 @@ mod tests { assert!(f.is_php_file(&PathBuf::from("/ws/foo.php"))); assert!(!f.is_php_file(&PathBuf::from("/ws/foo.module"))); } + + /// A link inside another link's tree has to win for paths under it, + /// or a file two links deep is respelled through the outer link and + /// names a path that does not exist. + #[test] + fn to_link_spelling_prefers_the_nearest_link() { + let dir = tempfile::tempdir().unwrap(); + let outer_target = dir.path().join("outer"); + let inner_target = dir.path().join("inner"); + std::fs::create_dir_all(outer_target.join("nested")).unwrap(); + std::fs::create_dir_all(&inner_target).unwrap(); + + let links = FollowedLinks::default(); + links.record(PathBuf::from("/ws/outer"), outer_target.clone()); + links.record( + PathBuf::from("/ws/outer/nested/inner"), + inner_target.clone(), + ); + + assert_eq!( + links.to_link_spelling(&outer_target.join("A.php")), + Some(PathBuf::from("/ws/outer/A.php")) + ); + assert_eq!( + links.to_link_spelling(&inner_target.join("B.php")), + Some(PathBuf::from("/ws/outer/nested/inner/B.php")) + ); + } + + /// A path already spelled through a link is what the walk produced, so + /// it must pass through untouched rather than being rewritten again. + #[test] + fn to_link_spelling_leaves_an_already_linked_path_alone() { + let links = FollowedLinks::default(); + links.record(PathBuf::from("/ws/link"), PathBuf::from("/opt/real")); + + assert_eq!( + links.to_link_spelling(&PathBuf::from("/ws/link/A.php")), + None + ); + assert_eq!( + links.to_link_spelling(&PathBuf::from("/elsewhere/A.php")), + None + ); + } } diff --git a/src/classmap_scanner/mod.rs b/src/classmap_scanner/mod.rs index a42406891..e82e6f008 100644 --- a/src/classmap_scanner/mod.rs +++ b/src/classmap_scanner/mod.rs @@ -85,7 +85,7 @@ pub use discovery::{ scan_psr4_directories_with_skip, scan_vendor_packages, scan_vendor_packages_with_skip, scan_workspace_fallback, scan_workspace_fallback_full, }; -pub use filters::{IndexFilters, workspace_walk_builder}; +pub use filters::{FollowedLinks, IndexFilters, LinkClaims, workspace_walk_builder}; pub use lexer::{find_classes, find_symbols}; // ─── File reading ──────────────────────────────────────────────────────────── diff --git a/src/indexing/init.rs b/src/indexing/init.rs index 766ebf4f2..395e70eae 100644 --- a/src/indexing/init.rs +++ b/src/indexing/init.rs @@ -177,7 +177,11 @@ impl Backend { } let filters = self.index_filters(); let mut scan = classmap_scanner::scan_workspace_fallback_full( - root, &skip_dirs, &filters, progress, + root, + &skip_dirs, + &filters, + progress, + Some(self.followed_links()), ); // Merge vendor packages (excluded from the workspace @@ -192,6 +196,7 @@ impl Backend { &explicit_deps, &filters, progress, + Some(self.followed_links()), ); let package_roots = std::mem::take(&mut vendor_scan.package_roots); @@ -568,6 +573,7 @@ impl Backend { &skip_dirs, &self.index_filters(), progress, + Some(self.followed_links()), ); self.populate_autoload_indices(&scan); { @@ -623,6 +629,7 @@ impl Backend { &skip_dirs, &self.index_filters(), progress, + Some(self.followed_links()), ); self.populate_autoload_indices(&scan); diff --git a/src/indexing/preload.rs b/src/indexing/preload.rs index 2fd1a0be8..d2c686e2b 100644 --- a/src/indexing/preload.rs +++ b/src/indexing/preload.rs @@ -272,6 +272,7 @@ impl Backend { &root, &vendor_dir_paths, &self.index_filters(), + Some(self.followed_links()), ); tracing::info!( "ensure_workspace_indexed: Phase 2 disk walk found {} PHP and {} resource files in {:?}", diff --git a/src/indexing/scan.rs b/src/indexing/scan.rs index f94628f58..9267235e7 100644 --- a/src/indexing/scan.rs +++ b/src/indexing/scan.rs @@ -108,6 +108,7 @@ impl Backend { &explicit_deps, &self.index_filters(), None, + Some(self.followed_links()), ); // Package roots came out of the same `installed.json` parse // `scan_vendor_packages_with_skip` already did; no need to @@ -527,6 +528,7 @@ impl Backend { &skip_dirs, &self.index_filters(), progress, + Some(self.followed_links()), ); } } @@ -562,6 +564,7 @@ impl Backend { skip_paths, &filters, progress, + Some(self.followed_links()), ); // Scan vendor packages from installed.json. @@ -576,6 +579,7 @@ impl Backend { &explicit_deps, &filters, progress, + Some(self.followed_links()), ); let mut result = WorkspaceScanResult { diff --git a/src/indexing/watch.rs b/src/indexing/watch.rs index 911e4f624..e46306efb 100644 --- a/src/indexing/watch.rs +++ b/src/indexing/watch.rs @@ -64,13 +64,14 @@ impl Backend { crate::virtual_members::laravel::database_schema::MigrationDiscovery::default(); let is_laravel = self.resolved_class_cache.read().is_laravel(); let config_path = root.join(crate::config::CONFIG_FILE_NAME); + let changes = self.spell_changes_as_indexed(¶ms.changes); { let open = self.open_files.read(); let parsed = self.parsed_uris.read(); let indexed = self.symbol_maps.read(); let laravel_config = self.config().laravel; let filters = self.index_filters(); - for change in ¶ms.changes { + for change in changes.iter() { let path_str = change.uri.path(); if path_str.ends_with("/composer.json") || path_str.ends_with("/composer.lock") { composer_changed = true; @@ -331,66 +332,111 @@ impl Backend { self.reconcile_index_for_filter_change(&previous_filters); } + /// Rewrite the paths in a watched-file batch into the spellings the + /// index holds. + /// + /// A directory reached through a symlink is indexed under the link's + /// spelling, but the watcher asking for its events is based at the link + /// and a client is free to resolve that base before it starts watching. + /// Such a client reports `/opt/kdhelp/Help.php` where the index holds + /// `/kdhelp/Help.php`, and every lookup below (open files, parsed + /// URIs, the symbol maps, the exclude filters) would miss. Clients that + /// do not resolve it report the link spelling already and pass through + /// untouched. + /// + /// Borrows the batch unchanged when the project has no followed links, + /// which is all but a handful of projects and every project before its + /// first walk. + fn spell_changes_as_indexed<'a>( + &self, + changes: &'a [FileEvent], + ) -> std::borrow::Cow<'a, [FileEvent]> { + let links = self.followed_links(); + if links.is_empty() { + return std::borrow::Cow::Borrowed(changes); + } + let mut spelled = changes.to_vec(); + for change in &mut spelled { + if let Ok(path) = change.uri.to_file_path() + && let Some(as_indexed) = links.to_link_spelling(&path) + && let Ok(uri) = Url::from_file_path(as_indexed) + { + change.uri = uri; + } + } + std::borrow::Cow::Owned(spelled) + } + /// Build the `workspace/didChangeWatchedFiles` registration for the - /// current `[indexing] extensions` config and Laravel classification. + /// current `[indexing] extensions` config, Laravel classification, and + /// the directory symlinks the index has reached through. /// /// Shared by `initialized`'s first registration and /// [`Self::reregister_watched_files_if_changed`] so a live - /// `.phpantom.toml` reload advertises the same watcher set a fresh - /// session would have started with. Returns the registration - /// alongside the extension list and Laravel flag it was built from, - /// so the caller can record what was actually registered. - pub(crate) fn build_watched_file_registration(&self) -> (Registration, Vec, bool) { + /// `.phpantom.toml` reload, or a walk that discovers a link, advertises + /// the same watcher set a fresh session would have started with. + /// Returns the registration alongside the inputs it was built from, so + /// the caller can record what was actually registered. + pub(crate) fn build_watched_file_registration( + &self, + ) -> (Registration, crate::WatchedFileInputs) { let index_filters = self.index_filters(); let extra_extensions = index_filters.extra_extensions().to_vec(); let is_laravel = self.resolved_class_cache.read().is_laravel(); - let mut watchers: Vec = extra_extensions + // Create, change, and delete: what every watcher but the two + // Composer ones asks for. + let watch_all = WatchKind::Create | WatchKind::Change | WatchKind::Delete; + + // Patterns the workspace folders are watched with. Each one is + // repeated per followed link below, based at the link, because a + // workspace-relative pattern never reaches through a symlink. + let mut patterns: Vec<(String, WatchKind)> = extra_extensions .iter() - .map(|ext| FileSystemWatcher { - glob_pattern: GlobPattern::String(format!("**/*.{ext}")), - kind: Some(WatchKind::Create | WatchKind::Change | WatchKind::Delete), - }) + .map(|ext| (format!("**/*.{ext}"), watch_all)) .collect(); - watchers.extend([ - FileSystemWatcher { - glob_pattern: GlobPattern::String("**/*.php".to_string()), - kind: Some(WatchKind::Create | WatchKind::Change | WatchKind::Delete), - }, - FileSystemWatcher { - glob_pattern: GlobPattern::String("**/*.{yaml,yml,xml}".to_string()), - kind: Some(WatchKind::Create | WatchKind::Change | WatchKind::Delete), - }, - FileSystemWatcher { - glob_pattern: GlobPattern::String("**/*.{yaml,yml,xml}.dist".to_string()), - kind: Some(WatchKind::Create | WatchKind::Change | WatchKind::Delete), - }, - FileSystemWatcher { - glob_pattern: GlobPattern::String("**/composer.json".to_string()), - kind: Some(WatchKind::Change), - }, - FileSystemWatcher { - glob_pattern: GlobPattern::String("**/composer.lock".to_string()), - kind: Some(WatchKind::Change), - }, - FileSystemWatcher { - glob_pattern: GlobPattern::String("**/.phpantom.toml".to_string()), - kind: Some(WatchKind::Create | WatchKind::Change | WatchKind::Delete), - }, + patterns.extend([ + ("**/*.php".to_string(), watch_all), + ("**/*.{yaml,yml,xml}".to_string(), watch_all), + ("**/*.{yaml,yml,xml}.dist".to_string(), watch_all), + ("**/composer.json".to_string(), WatchKind::Change), + ("**/composer.lock".to_string(), WatchKind::Change), + ("**/.phpantom.toml".to_string(), watch_all), ]); if is_laravel { - watchers.extend([ - FileSystemWatcher { - glob_pattern: GlobPattern::String("**/*.sql".to_string()), - kind: Some(WatchKind::Create | WatchKind::Change | WatchKind::Delete), - }, - FileSystemWatcher { - glob_pattern: GlobPattern::String("**/config/database.php".to_string()), - kind: Some(WatchKind::Create | WatchKind::Change | WatchKind::Delete), - }, + patterns.extend([ + ("**/*.sql".to_string(), watch_all), + ("**/config/database.php".to_string(), watch_all), ]); } + let mut watchers: Vec = patterns + .iter() + .map(|(pattern, kind)| FileSystemWatcher { + glob_pattern: GlobPattern::String(pattern.clone()), + kind: Some(*kind), + }) + .collect(); + + // A tree reached through a symlink is indexed under the link's + // spelling but sits outside every workspace folder on disk, so the + // patterns above never match a file in it. Asking for it by base + // URI is the only way to hear about a `git pull` into a linked + // framework, or a file created there by another tool. + let followed_links = self.watchable_followed_links(); + for link in &followed_links { + let Ok(base) = Url::from_file_path(link) else { + continue; + }; + watchers.extend(patterns.iter().map(|(pattern, kind)| FileSystemWatcher { + glob_pattern: GlobPattern::Relative(RelativePattern { + base_uri: OneOf::Right(base.clone()), + pattern: pattern.clone(), + }), + kind: Some(*kind), + })); + } + let registration = Registration { id: WATCHED_FILES_REGISTRATION_ID.to_string(), method: "workspace/didChangeWatchedFiles".to_string(), @@ -400,29 +446,51 @@ impl Backend { ), }; - (registration, extra_extensions, is_laravel) + ( + registration, + crate::WatchedFileInputs { + extra_extensions, + is_laravel, + followed_links, + }, + ) + } + + /// The followed links worth putting in a registration: none unless the + /// client said it can match a pattern against a base URI, since a + /// client that cannot would either ignore the watcher or fail to parse + /// the registration that carries it, taking the workspace watchers + /// down with it. + fn watchable_followed_links(&self) -> Vec { + if !self + .supports_relative_pattern_watchers + .load(Ordering::Acquire) + { + return Vec::new(); + } + self.followed_links().link_paths() } /// Re-push the `workspace/didChangeWatchedFiles` registration when - /// `[indexing] extensions` or the Laravel classification has changed - /// since the last registration. + /// `[indexing] extensions`, the Laravel classification, or the set of + /// followed symlinks has changed since the last registration. /// /// Guarded on an actual change so an unrelated config edit does not /// churn the client's watcher list. Before `initialized` performs the /// first registration, this only records the desired state instead of /// racing that initial `register_capability` call. pub(crate) fn reregister_watched_files_if_changed(&self) { - let (registration, extra_extensions, is_laravel) = self.build_watched_file_registration(); + let (registration, inputs) = self.build_watched_file_registration(); let mut state = self.registered_watcher_state.write(); let Some(previous) = state.clone() else { - *state = Some((extra_extensions, is_laravel)); + *state = Some(inputs); return; }; - if previous == (extra_extensions.clone(), is_laravel) { + if previous == inputs { return; } - *state = Some((extra_extensions, is_laravel)); + *state = Some(inputs); drop(state); if self.client.is_none() { @@ -991,8 +1059,12 @@ mod tests { // before any `.phpantom.toml` extensions were configured. backend.reregister_watched_files_if_changed(); assert_eq!( - *backend.registered_watcher_state.read(), - Some((Vec::new(), true)) + backend.registered_watcher_state.read().clone(), + Some(crate::WatchedFileInputs { + extra_extensions: Vec::new(), + is_laravel: true, + followed_links: Vec::new(), + }) ); std::fs::write( @@ -1003,8 +1075,12 @@ mod tests { backend.reload_config(dir.path()); assert_eq!( - *backend.registered_watcher_state.read(), - Some((vec!["module".to_string()], true)), + backend.registered_watcher_state.read().clone(), + Some(crate::WatchedFileInputs { + extra_extensions: vec!["module".to_string()], + is_laravel: true, + followed_links: Vec::new(), + }), "a reload that adds an extension must update the registered watcher state" ); } @@ -1027,8 +1103,141 @@ mod tests { backend.reload_config(dir.path()); assert_eq!( - *backend.registered_watcher_state.read(), - Some((Vec::new(), true)) + backend.registered_watcher_state.read().clone(), + Some(crate::WatchedFileInputs { + extra_extensions: Vec::new(), + is_laravel: true, + followed_links: Vec::new(), + }) + ); + } + + /// A tree reached through a symlink sits outside every workspace + /// folder, so the plain `**/*.php` watchers never cover it. Without a + /// watcher based at the link, a file created in the linked tree by + /// another tool stays invisible until the window is reloaded. + #[test] + fn a_followed_link_gets_its_own_relative_pattern_watcher() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + let real = dir.path().join("framework"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&real).unwrap(); + std::fs::write(real.join("Help.php"), ", Vec); -/// The `[indexing] extensions` set and Laravel classification last pushed -/// to the client as a `workspace/didChangeWatchedFiles` registration: -/// `(extra_extensions, is_laravel)`. `None` until the first registration. -/// See [`Backend::registered_watcher_state`]. -pub(crate) type WatchedFileRegistrationState = Option<(Vec, bool)>; +/// What the last `workspace/didChangeWatchedFiles` registration pushed to +/// the client was built from, so the next one can tell whether anything +/// it watches has moved. See [`Backend::registered_watcher_state`]. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct WatchedFileInputs { + /// The `[indexing] extensions` set, one watcher each. + pub(crate) extra_extensions: Vec, + /// Whether the project was classified as Laravel, which adds the + /// schema watchers. + pub(crate) is_laravel: bool, + /// Directory symlinks the index reached through, one relative-pattern + /// watcher each. Empty when the client cannot match a relative + /// pattern, since then there is nothing to ask it for. + pub(crate) followed_links: Vec, +} + +/// `None` until the first registration. +pub(crate) type WatchedFileRegistrationState = Option; // ─── Module declarations ──────────────────────────────────────────────────── @@ -903,6 +916,17 @@ pub struct Backend { pub(crate) supports_work_done_progress: Arc, /// Whether the client supports dynamic registration for type hierarchy. pub(crate) supports_type_hierarchy_dynamic_registration: Arc, + /// Whether the client can match a watcher pattern against a base URI + /// (`workspace.didChangeWatchedFiles.relativePatternSupport`). + /// + /// A plain `**/*.php` pattern is matched against the files of the + /// workspace folders, which gives a client no reason to watch a + /// directory living outside them, and nothing obliges it to traverse a + /// symlink to find one. A relative pattern names the link outright, + /// which is the only way in the protocol to ask for those events; + /// without the capability, a tree reached through a link is indexed but + /// not watched. + pub(crate) supports_relative_pattern_watchers: Arc, /// The `[indexing] extensions` set and Laravel classification last /// pushed to the client as a `workspace/didChangeWatchedFiles` /// registration. `None` until `initialized` performs the first @@ -1241,6 +1265,7 @@ impl Backend { std::sync::atomic::AtomicBool::new(false), ), registered_watcher_state: Arc::new(RwLock::new(None)), + supports_relative_pattern_watchers: Arc::new(std::sync::atomic::AtomicBool::new(false)), supports_show_document: Arc::new(std::sync::atomic::AtomicBool::new(false)), supports_semantic_tokens_refresh: Arc::new(std::sync::atomic::AtomicBool::new(false)), supports_code_lens_refresh: Arc::new(std::sync::atomic::AtomicBool::new(false)), @@ -1992,6 +2017,9 @@ impl Backend { supports_type_hierarchy_dynamic_registration: Arc::clone( &self.supports_type_hierarchy_dynamic_registration, ), + supports_relative_pattern_watchers: Arc::clone( + &self.supports_relative_pattern_watchers, + ), registered_watcher_state: Arc::clone(&self.registered_watcher_state), supports_show_document: Arc::clone(&self.supports_show_document), supports_semantic_tokens_refresh: Arc::clone(&self.supports_semantic_tokens_refresh), @@ -2033,6 +2061,14 @@ impl Backend { self.workspace.config.lock().clone() } + /// The directory symlinks the workspace walks have indexed through. + /// + /// Walks report into it as they discover links; the watcher + /// registration and the watched-file handler read it back. + pub(crate) fn followed_links(&self) -> &crate::classmap_scanner::FollowedLinks { + &self.workspace.followed_links + } + /// Replace the current configuration. /// /// Used when (re)loading `.phpantom.toml` and by integration tests diff --git a/src/move_cli/residual.rs b/src/move_cli/residual.rs index 5f7052609..a26a3e7ff 100644 --- a/src/move_cli/residual.rs +++ b/src/move_cli/residual.rs @@ -205,6 +205,7 @@ fn collect_hits( std::sync::Arc::new(vendor_dirs), backend.index_filters(), true, + crate::classmap_scanner::LinkClaims::new([root.to_path_buf()], None), ); builder.threads( std::thread::available_parallelism() diff --git a/src/references/mod.rs b/src/references/mod.rs index bea5ad1f5..6ac4c2d76 100644 --- a/src/references/mod.rs +++ b/src/references/mod.rs @@ -326,9 +326,10 @@ pub(crate) fn collect_php_files_gitignore( root: &Path, vendor_dir_paths: &[PathBuf], filters: &std::sync::Arc, + followed: Option<&crate::classmap_scanner::FollowedLinks>, ) -> Vec { let mut result = Vec::new(); - visit_workspace_files_gitignore(root, vendor_dir_paths, filters, |path| { + visit_workspace_files_gitignore(root, vendor_dir_paths, filters, followed, |path| { if filters.is_php_file(path) { result.push(path.to_path_buf()); } @@ -342,10 +343,11 @@ pub(crate) fn collect_workspace_index_files_gitignore( root: &Path, vendor_dir_paths: &[PathBuf], filters: &std::sync::Arc, + followed: Option<&crate::classmap_scanner::FollowedLinks>, ) -> (Vec, Vec) { let mut php_files = Vec::new(); let mut resource_files = Vec::new(); - visit_workspace_files_gitignore(root, vendor_dir_paths, filters, |path| { + visit_workspace_files_gitignore(root, vendor_dir_paths, filters, followed, |path| { if filters.is_php_file(path) { php_files.push(path.to_path_buf()); } else if crate::resource_navigation::is_resource_path(path) { @@ -359,6 +361,7 @@ fn visit_workspace_files_gitignore( root: &Path, vendor_dir_paths: &[PathBuf], filters: &std::sync::Arc, + followed: Option<&crate::classmap_scanner::FollowedLinks>, mut visit: impl FnMut(&Path), ) { let walker = crate::classmap_scanner::workspace_walk_builder( @@ -366,6 +369,7 @@ fn visit_workspace_files_gitignore( std::sync::Arc::new(vendor_dir_paths.to_vec()), std::sync::Arc::clone(filters), false, + crate::classmap_scanner::LinkClaims::new([root.to_path_buf()], followed), ) .build(); diff --git a/src/references/tests.rs b/src/references/tests.rs index e21462442..b52643266 100644 --- a/src/references/tests.rs +++ b/src/references/tests.rs @@ -862,3 +862,103 @@ async fn laravel_string_key_references_gated_on_is_laravel() { references, got {plain_locs:?}" ); } + +// ─── Interior symlinks: collect_php_files_gitignore (issue #383) ── +// The Find References / rename / preload walker is a *serial* `ignore` +// walk (`.build()` + `flatten()`). The same symlink contract as +// `walk_roots` applies, and a symlink cycle must terminate instead of +// panicking: `flatten()` silently drops `Err` entries, which is where +// the loop error lands. + +#[test] +fn collect_php_files_gitignore_follows_interior_symlink() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + let real = dir.path().join("real"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&real).unwrap(); + std::fs::write(real.join("Hidden.php"), ">, + /// Directory symlinks the workspace walks have indexed through. + /// + /// Shared by `Arc` because the walk that discovers a link runs on a + /// blocking clone while the watcher registration it feeds is built on + /// the clone that answers requests. + pub(crate) followed_links: crate::classmap_scanner::FollowedLinks, /// Counts the filter changes that asked for a workspace rediscovery. /// /// A debounced rediscovery task captures the count it was scheduled @@ -100,6 +106,7 @@ impl WorkspaceEnv { global_config_path, index_filters: Arc::new(RwLock::new(None)), client_indexing: Arc::new(RwLock::new(config::ClientIndexingOptions::default())), + followed_links: crate::classmap_scanner::FollowedLinks::default(), filter_rediscovery_generation: Arc::new(AtomicU64::new(0)), } } @@ -118,6 +125,7 @@ impl Clone for WorkspaceEnv { global_config_path: self.global_config_path.clone(), index_filters: Arc::clone(&self.index_filters), client_indexing: Arc::clone(&self.client_indexing), + followed_links: self.followed_links.clone(), filter_rediscovery_generation: Arc::clone(&self.filter_rediscovery_generation), } } diff --git a/tests/integration/classmap_scanner.rs b/tests/integration/classmap_scanner.rs index 8b2a9ca63..7ebd9c9ae 100644 --- a/tests/integration/classmap_scanner.rs +++ b/tests/integration/classmap_scanner.rs @@ -72,7 +72,8 @@ fn scan_psr4_directories_respects_namespace_filtering() { ) .unwrap(); - let classmap = classmap_scanner::scan_psr4_directories(&[("App\\".to_string(), src)], &[], &[]); + let classmap = + classmap_scanner::scan_psr4_directories(&[("App\\".to_string(), src)], &[], &[], None); assert!(classmap.contains_key("App\\Models\\User")); assert!( !classmap.contains_key("Wrong\\Namespace\\WrongNs"), @@ -89,7 +90,7 @@ fn scan_psr4_directories_handles_classmap_entries() { // classmap entries don't filter by namespace std::fs::write(lib.join("Legacy.php"), "