Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 32 additions & 20 deletions laravel-lsp/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18140,7 +18140,9 @@ async fn classify_with_decl_fallback(
// subsequent invocations don't re-parse the file.
if !is_in_routes_dir(root, file_path) {
// A Folio page's `name('...')` helper declares its route name, but the
// page lives outside `routes/` and the bare helper isn't tagged by
// page isn't a conventional route file — it lives outside the
// project-root `routes/`, or nested deeper inside it (a mount under
// `routes/pages/`, issue #105) — and the bare helper isn't tagged by
// php.scm. If the cursor sits on that call, resolve it to the page's
// (mount-prefixed) route name — read from the already-built in-memory
// route index, never a fresh filesystem walk — so find-references /
Expand Down Expand Up @@ -18221,36 +18223,46 @@ async fn decl_range_at(
None
}

/// Is this file a conventional route file — i.e. does it live directly under
/// the project root's `routes/` directory? Used to gate the declaration-fallback
/// Is this file a conventional route file — i.e. is its immediate parent the
/// project root's `routes/` directory? Used to gate the declaration-fallback
/// walk so we don't pay the parse cost on every PHP file.
///
/// Only the project-root `routes/` qualifies. A `routes` component nested
/// deeper — a package's `vendor/.../routes/`, or a Folio mount that happens to
/// carry a `routes` segment (issue #98) — must NOT be treated as the
/// conventional routes dir, or find-references from such a file would be routed
/// 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.
/// Only files sitting *directly* in the project-root `routes/` qualify
/// (`routes/web.php`, `routes/api.php`). Two kinds of `routes` path component
/// must NOT trigger the gate:
/// - a `routes` segment nested elsewhere — a package's `vendor/.../routes/`,
/// or a sibling dir that merely shares the name (issue #98);
/// - a file nested *deeper* inside `routes/` — e.g. a Folio mount under
/// `routes/pages/about.php` (issue #105). Such a page has its own resolver
/// (the Folio branch in `classify_with_decl_fallback`); routing it into the
/// decl walk would bypass that branch and yield no references. Matching the
/// immediate parent — not a `starts_with` prefix — is what keeps the Folio
/// page out of the gate.
///
/// Both `path` and the joined `routes/` dir are canonicalized before the
/// component-wise prefix check (issue #122), mirroring the sibling helper
/// When the project root is unknown (`None`), default to `false` so the walk
/// never triggers without a root to anchor against.
///
/// Both the file's parent and the joined `routes/` dir are canonicalized before
/// the equality 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
/// states — e.g. macOS `/tmp` → `/private/tmp`. A raw textual comparison 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 comparing the
/// uncanonicalized paths rather than guess — no panic, and the same behaviour
/// for in-memory paths that aren't on disk.
fn is_in_routes_dir(root: Option<&Path>, path: &Path) -> bool {
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),
let Some(parent) = path.parent() else {
return false;
};
match (parent.canonicalize(), routes_dir.canonicalize()) {
(Ok(real_parent), Ok(real_routes)) => real_parent == real_routes,
_ => parent == routes_dir,
}
}

Expand Down
69 changes: 54 additions & 15 deletions laravel-lsp/src/tests/routes_dir_gate.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,40 @@
//! Unit tests for `is_in_routes_dir` — the gate deciding whether the
//! declaration-fallback route walk runs for a given file. Only the project
//! root's own `routes/` directory qualifies; a `routes` component nested
//! deeper (a package's `vendor/.../routes/`, or a Folio mount) must not be
//! mistaken for it (issue #98).
//! declaration-fallback route walk runs for a given file. Only a file whose
//! immediate parent is the project root's own `routes/` directory qualifies.
//! A `routes` component nested elsewhere (a package's `vendor/.../routes/`, a
//! sibling that shares the name) must not be mistaken for it (issue #98), and
//! neither must a file nested *deeper* inside `routes/` — e.g. a Folio mount
//! under `routes/pages/` (issue #105), which has its own resolver.

use crate::is_in_routes_dir;
use std::path::Path;

#[test]
fn matches_project_root_routes_dir() {
// Direct children of the project-root `routes/` are conventional route
// files (AC: both web.php and api.php resolve correctly).
assert!(is_in_routes_dir(
Some(Path::new("/project")),
Path::new("/project/routes/web.php")
));
assert!(is_in_routes_dir(
Some(Path::new("/project")),
Path::new("/project/routes/api.php")
));
}

#[test]
fn rejects_folio_page_nested_under_routes() {
// A Folio mount under `routes/pages/` lives *inside* routes/, but deeper
// than its immediate child level (issue #105). It must NOT be gated as a
// conventional route file — its `name('...')` is resolved by the Folio
// branch in `classify_with_decl_fallback`, and routing it into the decl
// walk would bypass that branch and yield no references. The immediate-
// parent check (not a `starts_with` prefix) is what excludes it.
assert!(!is_in_routes_dir(
Some(Path::new("/project")),
Path::new("/project/routes/pages/about.php")
));
}

#[test]
Expand Down Expand Up @@ -46,6 +68,16 @@ fn rejects_false_prefix_sibling() {
));
}

#[test]
fn rejects_routes_dir_outside_project_root() {
// A `routes/` directory that belongs to some *other* tree, not the
// configured project root, must not qualify (AC: root-known case).
assert!(!is_in_routes_dir(
Some(Path::new("/project")),
Path::new("/elsewhere/routes/web.php")
));
}

#[test]
fn rejects_when_root_unknown() {
// No project root to anchor against → never trigger the fallback walk.
Expand Down Expand Up @@ -95,24 +127,31 @@ 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). 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.
// The gate compares the file's *parent* dir against `<root>/routes`. When a
// path can't be canonicalized it must fall back to a direct (uncanonical)
// comparison rather than panic (issue #122). 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/` dir.
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.
// The parent `<root>/routes` exists, so this resolves via the canonical arm
// — no panic on the unsaved file, and the parent matches the 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).
// False-negative side: a missing path *outside* routes/ has a parent
// (`<root>/app`) that doesn't exist either, so its parent can't be
// canonicalized — driving the `(Err, Ok)` textual-fallback arm. It must
// still return false.
let missing_outside = root.join("app").join("does_not_exist.php");
assert!(!is_in_routes_dir(Some(&root), &missing_outside));

// And a route file *and* its parent both missing from disk (neither
// canonicalizes) must still match through the textual fallback — the parent
// `<root>/routes` equals the joined routes dir by direct comparison.
let unsaved_root = tmp.path().join("never_created");
let unsaved_file = unsaved_root.join("routes").join("web.php");
assert!(is_in_routes_dir(Some(&unsaved_root), &unsaved_file));
}