From af9c18dfb9d965b774437cf04fec3e50d01bffab Mon Sep 17 00:00:00 2001 From: Mike Bronner Date: Mon, 15 Jun 2026 03:05:09 -0700 Subject: [PATCH 1/3] chore: start work on #122 From 2cb9160c50a7884244b295fc9ffcd7023aea759e Mon Sep 17 00:00:00 2001 From: Mike Bronner Date: Mon, 15 Jun 2026 03:09:44 -0700 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20=F0=9F=90=9B=20Canonicalize=20paths?= =?UTF-8?q?=20in=20is=5Fin=5Froutes=5Fdir=20for=20symlinked=20roots.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The routes-dir gate compared `path` against `root/routes` with a purely textual `Path::starts_with`. When the stored project root and a per-request file path resolve through different symlink states (e.g. macOS `/tmp` → `/private/tmp`), that returns a false negative and silently skips the declaration-fallback walk for a real conventional route file. Canonicalize both sides before the component-wise prefix check, mirroring the sibling helper `path_within_root`, and fall back to the textual check when either side can't be canonicalized — no panic, no behaviour change for in-memory paths. Adds tests for symlinked-root resolution, the canonical out-of-routes case, and graceful fallback on a missing path. Fixes: #122 --- laravel-lsp/src/main.rs | 21 +++++++-- laravel-lsp/src/tests/routes_dir_gate.rs | 54 ++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/laravel-lsp/src/main.rs b/laravel-lsp/src/main.rs index 428f2a0b..f8a90ca8 100644 --- a/laravel-lsp/src/main.rs +++ b/laravel-lsp/src/main.rs @@ -18241,10 +18241,25 @@ async fn decl_range_at( /// into the decl walk and never reach its real resolver. When the project root /// is unknown (`None`), default to `false` so the walk never triggers without a /// root to anchor against. +/// +/// Both `path` and the joined `routes/` dir are canonicalized before the +/// component-wise prefix check (issue #122), mirroring the sibling helper +/// `path_within_root`. `root` is stored once (from an earlier `did_open`) while +/// `path` arrives per-request, so the two can resolve through different symlink +/// states — e.g. macOS `/tmp` → `/private/tmp`. A raw textual `starts_with` +/// would then return a false negative and silently skip the decl-fallback walk +/// for a real conventional route file. When either side can't be canonicalized +/// (the file doesn't exist yet, a permission error) we fall back to the textual +/// prefix check rather than guess — no panic, and the same behaviour as before +/// for in-memory paths that aren't on disk. fn is_in_routes_dir(root: Option<&Path>, path: &Path) -> bool { - match root { - Some(r) => path.starts_with(r.join("routes")), - None => false, + let Some(r) = root else { + return false; + }; + let routes_dir = r.join("routes"); + match (path.canonicalize(), routes_dir.canonicalize()) { + (Ok(real_path), Ok(real_routes)) => real_path.starts_with(&real_routes), + _ => path.starts_with(&routes_dir), } } diff --git a/laravel-lsp/src/tests/routes_dir_gate.rs b/laravel-lsp/src/tests/routes_dir_gate.rs index d644afcb..a7375094 100644 --- a/laravel-lsp/src/tests/routes_dir_gate.rs +++ b/laravel-lsp/src/tests/routes_dir_gate.rs @@ -51,3 +51,57 @@ fn rejects_when_root_unknown() { // No project root to anchor against → never trigger the fallback walk. assert!(!is_in_routes_dir(None, Path::new("/any/routes/file.php"))); } + +#[cfg(unix)] +#[test] +fn matches_through_symlinked_root() { + // `root` and `path` can resolve through different symlink states (issue + // #122) — e.g. a stored root reached via a symlink vs. a per-request file + // addressed by its real path. A raw textual `starts_with` would return a + // false negative; canonicalizing both sides makes the gate survive it. + use std::os::unix::fs::symlink; + + let tmp = tempfile::tempdir().unwrap(); + // Real project with a conventional routes/ dir and a route file on disk. + let real_root = tmp.path().join("real_project"); + std::fs::create_dir_all(real_root.join("routes")).unwrap(); + let route_file = real_root.join("routes").join("web.php"); + std::fs::write(&route_file, " Date: Mon, 15 Jun 2026 06:05:39 -0700 Subject: [PATCH 3/3] =?UTF-8?q?test:=20=E2=9C=85=20drive=20the=20(Err,=20O?= =?UTF-8?q?k)=20arm=20in=20is=5Fin=5Froutes=5Fdir=20fallback=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit falls_back_to_textual_when_path_missing never created root/routes/ on disk, so routes_dir.canonicalize() also failed and the test hit the (Err, Err) arm — not the (Err, Ok) arm its comment claimed. Create the routes/ dir so routes_dir canonicalizes while the missing path does not, genuinely exercising the realistic unsaved-buffer case. Add a false-negative assertion for a missing path outside routes/. Addresses Holmes review on #122. --- laravel-lsp/src/tests/routes_dir_gate.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/laravel-lsp/src/tests/routes_dir_gate.rs b/laravel-lsp/src/tests/routes_dir_gate.rs index a7375094..b56e9619 100644 --- a/laravel-lsp/src/tests/routes_dir_gate.rs +++ b/laravel-lsp/src/tests/routes_dir_gate.rs @@ -96,12 +96,23 @@ fn rejects_real_path_outside_routes_dir() { #[test] fn falls_back_to_textual_when_path_missing() { // A path that doesn't exist can't be canonicalized; the gate must fall back - // to the textual component check rather than panic (issue #122). Use a real - // tempdir root so only `path` fails canonicalization. + // to the textual component check rather than panic (issue #122). Create a + // real `routes/` dir on disk so `routes_dir` canonicalizes but the missing + // `path` does not — genuinely driving the `(Err, Ok)` arm. This is the + // realistic case the production doc comment calls out: a brand-new route + // file still in the editor buffer, not yet saved to disk, sitting inside a + // real `routes/` directory. let tmp = tempfile::tempdir().unwrap(); let root = tmp.path().to_path_buf(); + std::fs::create_dir_all(root.join("routes")).unwrap(); let missing = root.join("routes").join("does_not_exist.php"); // No panic, and the textual prefix still matches the project routes/ dir. assert!(is_in_routes_dir(Some(&root), &missing)); + + // False-negative side: a missing path *outside* routes/ must still return + // false through the same textual fallback (routes_dir canonicalizes, the + // path does not — again the `(Err, Ok)` arm). + let missing_outside = root.join("app").join("does_not_exist.php"); + assert!(!is_in_routes_dir(Some(&root), &missing_outside)); }