diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 639de403d..9e2ffb790 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **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. +- **Class and namespace moves from the command line.** `phpantom_lsp move FROM TO` moves one class or a whole namespace and updates declarations, imports, references, and PSR-4 paths across the project. Both sides can be fully-qualified names or Composer PSR-4 file/directory paths, and `--dry-run --format json` provides a validation-only form for scripts and coding agents. A destination that would overwrite an existing class or file is refused before any changes are made. A move into a namespace no PSR-4 mapping covers is called out rather than reported as a plain success, since the files cannot follow the declarations there and the autoloader stops finding them. A class installed by Composer is refused outright, the same way renaming one in the editor is. Contributed by @calebdw. - **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. - **Class and namespace moves from the command line.** `phpantom_lsp move FROM TO` moves one class or a whole namespace and updates declarations, imports, references, and PSR-4 paths across the project. Both sides can be fully-qualified names or Composer PSR-4 file/directory paths, and `--dry-run` provides a validation-only form for scripts and coding agents. A destination that would overwrite an existing class or file is refused before any changes are made. A move into a namespace no PSR-4 mapping covers is called out rather than reported as a plain success, since the files cannot follow the declarations there and the autoloader stops finding them. A class installed by Composer is refused outright, the same way renaming one in the editor is. A move also reports what it could not reach: a namespace named in a Blade template, a YAML config, or a baseline file, and a directory spelled out inside a path string, are all invisible to a rewriter that works on resolved symbols, so the project is scanned as it will look afterwards and every leftover mention of the old name or location is reported with the file and line it sits on. `files_changed` can then be read against a stated list of what was left alone rather than assumed complete. `--no-colour` and `--format github` are accepted alongside `table` and `json`, matching `analyze` and `fix`, and the JSON is shaped like the object `analyze` emits so a script driving a batch of refactors can consume both the same way. Contributed by @calebdw. - **Document outline for Blade files.** The outline view, breadcrumbs, and go-to-symbol now describe a `.blade.php` file by what it actually writes: the sections and stacks it fills or leaves open (`@section`, `@yield`, `@push`, `@stack` and the rest of that family) and the components it renders, each listed with the class behind it, or, for an anonymous component, the template Laravel renders in its place. A tag no component answers for keeps its bare name. Everything nests the way the template does, so the components inside a section are listed under it, and selecting an entry jumps to the name in the template rather than to a line in the virtual PHP the outline used to be measured against. @@ -25,16 +27,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **A config key a project sets at runtime is a config key.** `Config::set('filesystems.disks.ondemand', [...])`, the array form of the `config([...])` helper, and `Storage::fake()` declare the keys they name, so reading one afterwards is no longer reported as unknown. A test that configures a disk in its `setUp()` before exercising it is the usual shape. Packages get the same treatment from the other side: the configuration a library reads belongs to the application that installs it, which is a file the analysis never sees, so its keys are left alone rather than judged against config files that were never meant to declare them. - **A rewritten import keeps its indentation.** A rename or a `phpantom_lsp move` that carried a `use` statement to a new name replaced the whole line it sat on, so an import written inside a Blade `@php` block or a braced `namespace {}` block came back flattened against the left margin. Only the statement itself is rewritten now, and the whitespace around it is left where the file had it. - - **Moving a class into the global namespace removes its `namespace` declaration.** `phpantom_lsp move 'App\Old\Widget' 'Widget'` rewrote the declaring file's namespace name in place, and since the destination has no name to write there, the file was left holding `namespace ;`, which is a syntax error. The whole statement now goes, along with the line it sits on, and the imports the move has to add for the namespace-siblings the class was reaching by short name are written where it was. A file that opens its namespace as a brace block is refused with a message saying so, rather than having the block it wraps mangled. - - **A class leaving the global namespace is no longer reported as left behind.** Moving `Widget` to `App\Casts\Widget` ended with a warning that the old name still appears in the moved file, pointing at its own `class Widget` line. A global class's old fully-qualified name is a bare short name, and the move keeps the declaration spelled exactly that way on purpose, so the scan for leftover mentions now passes over the declaration it names. - - **The Blade lowering's own declarations stay out of the project's symbols.** The PHP a template is read as opened with a prologue declaring the wrapper its body is lowered into and the marker functions its directives compile to. Those were published like a declaration a file had written, so a workspace-symbol search for "blade" answered with ten entries nobody wrote, completion offered them beside the project's own functions, and every template in the project registered another copy of each. The markers are now declared once for the whole project rather than by every template, and neither they nor the wrapper are offered as a symbol of it. - - **Renames and moves reach Blade templates.** A class named in a `.blade.php` file was invisible to both `textDocument/rename` and `phpantom_lsp move`. The workspace index parsed a template as if it were plain PHP, where everything Blade-specific reads as inline HTML, so the template's symbol map held none of the class references it makes and a move left every one of them naming a class that no longer exists, with `files_changed` reading as a complete count when it was not. Worse, a single open template abandoned the rename outright: a template's symbol map describes the PHP Laravel compiles it to, which is longer than the file on disk, and the guard that protects against stale offsets fired on that difference and dropped every file's edits, down to the moved namespace's own declaration. Templates are now indexed as the PHP they compile to, and each edit comes back through the source map onto the template's own line, so a fully-qualified name written in an `@php` block, in a `@var` docblock, or inside a directive's argument follows the move, as do a `use` statement inside `@php`, the `@use` directive in all of its forms, and the short names an import binds. A method or property renamed from a template's call site lands in the right place too. Find All References sees the same templates, so a class is now listed with the views that name it without having to open them first. - - **A namespace served by two PSR-4 roots is refused up front, with both roots named.** Composer accepts an array of directories per prefix, and naming one of them resolves to the namespace both of them serve, from where the second root is indistinguishable from the first. The move planned to carry both roots onto the same destination and stopped on a file the caller never mentioned, reporting it as a missing file. Such a move is now refused before anything is planned, with both roots named, since honouring the directory that was actually passed means moving only the classes declared beneath it and the rewriter works on whole namespace prefixes. A prefix whose other roots are listed in `composer.json` but hold no files is unaffected: there is still only one directory to move. Renaming a namespace segment in the editor and `phpantom_lsp move` are both covered. - **A namespace move rewrites each reference the way the file holding it actually reads.** Whether a reference was rewritten, and how the replacement was spelled, was decided from the name recorded for it rather than from the text in the file, and neither the recorded name nor its flags say how the file resolves it. A reference written from the global namespace, `\App\Old\Widget::class` in a file that declares a namespace of its own, lost its leading separator and came back as a relative name that resolves inside the enclosing namespace instead. Because `::class` does not require the class to exist, a morph map or a container binding built that way kept running and stored a name that resolves to nothing. In the other direction, a qualified reference written without a leading separator, which is what a file with no `namespace` declaration writes and what Laravel's `config/` is full of, was passed over entirely and kept naming the old namespace, so the breakage surfaced at boot rather than as a diagnostic. Every reference is now resolved through the file's own imports and namespace, and the replacement keeps the qualification the source was written with wherever the file, read as it will be once its own `namespace` and `use` lines are rewritten, still resolves that spelling to the moved class. Where it does not, the name is written out in full from the global namespace. Renaming a namespace segment in the editor and `phpantom_lsp move` are both fixed by this. - **Renaming a namespace to one under a different autoload mapping moves its files to the right place.** The destination directory was worked out by cutting the *source* namespace's PSR-4 prefix off the destination name, which only holds when both sides sit under the same mapping. Renaming `App\Old` to `Lib\Domain` in a project mapping `App\` to `src/` and `Lib\` to `lib/` left the files under `src/` where the autoloader no longer looks, and a destination outside the autoload map entirely scattered them into a directory named after whatever was left of the name once the wrong prefix was cut. A destination name shorter than the prefix being cut crashed the request rather than renaming anything. The files now follow the destination to the mapping that actually covers it, a destination no mapping covers moves nothing and rewrites the declarations in place, and neither case can end the rename early. diff --git a/docs/todo.md b/docs/todo.md index 57c75885e..3ca04add0 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -161,7 +161,6 @@ unlikely to move the needle for most users. | L49 | [Unguarded Eloquent mass assignment diagnostic](todo/laravel.md#l49-unguarded-eloquent-mass-assignment-diagnostic) | Medium | Medium | | L17 | [Additional string contexts without booting](todo/laravel.md#l17-additional-string-contexts-without-booting) (middleware, assets, validation, Inertia) | Medium | Medium-High | | L54 | [Audit custom-builder and relation-closure inference against the PHPStan extensions](todo/laravel.md#l54-audit-custom-builder-and-relation-closure-inference-against-the-phpstan-extensions) | Medium | Medium-High | -| L25 | [Storage disk name strings](todo/laravel.md#l25-storage-disk-name-strings) | Low-Medium | Low | | L31 | [String-key rename, highlight, and semantic tokens](todo/laravel.md#l31-string-key-rename-highlight-and-semantic-tokens) | Low-Medium | Medium | | L42 | [Morph alias completion in array positions](todo/laravel.md#l42-morph-alias-completion-in-array-positions) | Low-Medium | Medium | | L3 | `$dates` array (deprecated) | Low-Medium | Medium | diff --git a/docs/todo/laravel.md b/docs/todo/laravel.md index c53563668..844e17817 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -616,18 +616,6 @@ still partially lack: empty string as the value). No fix when the group file itself doesn't exist yet; that case still just diagnoses. -#### L25. Storage disk name strings - -**Impact: Low-Medium · Complexity: Low** - -`Storage::disk('...')` and the `#[Storage]` container attribute already -complete against `filesystems.disks.*`, navigate to the disk's entry in -`config/filesystems.php`, and flag an unknown disk. `Storage::fake()`, -`persistentFake()`, and `forgetDisk()` still name a disk with none of -that: their return type is patched to `FilesystemAdapter`, but the -disk-name argument itself gets no completion, go-to-definition, or -diagnostic. - #### L27. Legacy `Controller@method` action strings **Impact: Low · Complexity: Low** @@ -688,14 +676,15 @@ moving the Blade file — defer that one until the rest is in place. **Impact: Medium · Complexity: Medium** -L25 (storage disks) is one instance of a general pattern: a method -argument names an entry under a known config subtree, and the config -scanner already parses those files. Auth guards (`auth('...')`, +Storage disks are one instance of a general pattern: a method argument +names an entry under a known config subtree, and the config scanner +already parses those files. Auth guards (`auth('...')`, `Auth::guard()`, `->middleware('auth:web')`), cache stores (`Cache::store()`), log channels (`Log::channel()`), and storage disks -(L25) already complete against their config subtree — but all of them -route through the generic `LaravelStringKind::Config` kind rather than -a dedicated one, so they get completion plus the shared config +(`Storage::disk()`, test fakes, disk eviction, and `#[Storage]`) already +complete against their config subtree — but all of them route through +the generic `LaravelStringKind::Config` kind rather than a dedicated +one, so they get completion plus the shared config diagnostics/go-to-definition and nothing family-specific (a "cache store" hovers with the same generic wording as any other config key). `Log::stack()` (array values) isn't recognized at all. Generalize into diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index bcd4cc2b8..f9ef36783 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -26,6 +26,7 @@ use Database\Factories\AnnotatedPostFactory; use Database\Factories\BlogAuthorFactory; use Database\Factories\EditorialFactory; +use Illuminate\Contracts\Filesystem\Filesystem; use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Http\Client\Factory as HttpFactory; use Illuminate\Http\Client\PendingRequest; @@ -1155,12 +1156,22 @@ public function mixinModel(): void // ── Storage::fake() resolves to the concrete adapter ──────────────── - public function storageFake(): void + public function storageFake( + #[\Illuminate\Container\Attributes\Storage('avatars')] Filesystem $avatars, + ): void { // fake() declares the Filesystem contract but always builds a // FilesystemAdapter, so the adapter-only assertion helpers resolve. + // Disk names complete from config/filesystems.php, hover as their full + // config keys, and navigate back to their declarations — in the + // #[Storage] attribute above as much as in the calls below. Storage::fake('avatars')->assertExists('me.png'); - Storage::persistentFake('logs')->assertMissing('old.log'); + Storage::persistentFake(disk: 'logs')->assertMissing('old.log'); + + // forgetDisk() takes one name or a list of them, and tolerates a disk + // that was never configured, so an unknown name here is not flagged. + Storage::forgetDisk('avatars'); + Storage::forgetDisk(['avatars', 'logs']); } @@ -1169,10 +1180,11 @@ public function storageFake(): void public function storageDisk(): void { // disk()/cloud() declare the Filesystem/Cloud contract, but every - // disk config/filesystems.php configures ('local', 's3') builds a - // FilesystemAdapter, so adapter-only methods like download() + // disk config/filesystems.php configures ('local', 's3', ...) builds + // a FilesystemAdapter, so adapter-only methods like download() // resolve on every configured disk, not just a faked one. Storage::disk('s3')->download('report.pdf'); + Storage::disk(name: 'local')->exists('notes.txt'); Storage::cloud()->assertExists('logo.png'); // The 'pantry' disk uses a driver the framework does not ship. Its @@ -1180,6 +1192,16 @@ public function storageDisk(): void // FilesystemAdapter too, so a custom driver does not cost the rest of // the project its precise disk type. Storage::disk('pantry')->download('sourdough.pdf'); + + // A disk configured at runtime is configured all the same: nothing in + // config/filesystems.php declares 'ondemand' or 'scratch', and neither + // read below is flagged because the write above it establishes the + // disk. Configuring one in a test's setUp() is the usual shape. + Config::set('filesystems.disks.ondemand', ['driver' => 'local']); + Storage::disk('ondemand')->exists('invoice.pdf'); + + Storage::fake('scratch'); + Storage::disk('scratch')->exists('draft.txt'); } diff --git a/examples/laravel/config/filesystems.php b/examples/laravel/config/filesystems.php index e57200260..2a629fc5d 100644 --- a/examples/laravel/config/filesystems.php +++ b/examples/laravel/config/filesystems.php @@ -6,6 +6,26 @@ 'disks' => [ + 'local' => [ + 'driver' => 'local', + 'root' => 'storage/app', + ], + + 'avatars' => [ + 'driver' => 'local', + 'root' => 'storage/app/avatars', + ], + + 'logs' => [ + 'driver' => 'local', + 'root' => 'storage/app/logs', + ], + + 's3' => [ + 'driver' => 's3', + 'bucket' => 'demo', + ], + // A disk whose driver the framework does not ship. It is built by the // `Storage::extend('pantry', ...)` registration in DemoServiceProvider, // and PHPantom reads that closure's return type rather than giving up diff --git a/src/completion/laravel_string_keys.rs b/src/completion/laravel_string_keys.rs index da1dce4a8..41c69b2a4 100644 --- a/src/completion/laravel_string_keys.rs +++ b/src/completion/laravel_string_keys.rs @@ -21,6 +21,7 @@ use tower_lsp::lsp_types::*; use crate::Backend; use crate::symbol_map::LaravelStringKind; use crate::text_position::position_to_offset; +use crate::virtual_members::laravel::{is_storage_facade_name, storage_facade_local_names}; // ─── Context ──────────────────────────────────────────────────────────────── @@ -36,10 +37,185 @@ struct LaravelStringKeyContext { config_sub_prefix: Option<&'static str>, } +#[derive(Clone, Copy, PartialEq, Eq)] +enum StringArgumentShape { + Scalar, + ArrayValue, +} + +struct StringArgumentContext<'a> { + callable: &'a str, + named_argument: Option<&'a str>, + shape: StringArgumentShape, +} + +#[inline] +fn is_unescaped(bytes: &[u8], index: usize) -> bool { + let mut before = index; + while before > 0 && bytes[before - 1] == b'\\' { + before -= 1; + } + (index - before).is_multiple_of(2) +} + +/// Find the unmatched call parenthesis enclosing a named argument. +fn enclosing_call_open_paren(content: &str) -> Option { + let bytes = content.as_bytes(); + let mut parens = 0usize; + let mut brackets = 0usize; + let mut braces = 0usize; + let mut quote = None; + let mut index = bytes.len(); + + while index > 0 { + index -= 1; + let byte = bytes[index]; + if let Some(active_quote) = quote { + if byte == active_quote && is_unescaped(bytes, index) { + quote = None; + } + continue; + } + match byte { + b'\'' | b'"' => quote = Some(byte), + b')' => parens += 1, + b'(' if parens > 0 => parens -= 1, + b'(' if brackets == 0 && braces == 0 => return Some(index), + b']' => brackets += 1, + b'[' if brackets > 0 => brackets -= 1, + b'}' => braces += 1, + b'{' if braces > 0 => braces -= 1, + b';' if parens == 0 && brackets == 0 && braces == 0 => return None, + _ => {} + } + } + + None +} + +/// Return the callable before a scalar first argument or a named argument. +fn callable_before_scalar_argument(before_value: &str) -> Option<(&str, Option<&str>)> { + let before_value = before_value.trim_end(); + if let Some(callable) = before_value.strip_suffix('(') { + return Some((callable.trim_end(), None)); + } + + let colon = before_value.rfind(':')?; + if !before_value[colon + 1..].trim().is_empty() { + return None; + } + let before_label = before_value[..colon].trim_end(); + let label_start = before_label + .rfind(|ch: char| !(ch.is_ascii_alphanumeric() || ch == '_')) + .map_or(0, |index| index + 1); + if label_start == before_label.len() { + return None; + } + let argument = &before_label[label_start..]; + let before_argument = before_label[..label_start].trim_end(); + let open_paren = enclosing_call_open_paren(before_argument)?; + Some((before_argument[..open_paren].trim_end(), Some(argument))) +} + +/// Return the callable owning an array that directly contains this literal. +fn callable_before_array_argument(before_quote: &str) -> Option<(&str, Option<&str>)> { + let bytes = before_quote.as_bytes(); + let mut bracket_depth = 0usize; + let mut paren_depth = 0usize; + let mut brace_depth = 0usize; + let mut string_quote = None; + let mut index = bytes.len(); + + while index > 0 { + index -= 1; + let byte = bytes[index]; + if let Some(quote) = string_quote { + if byte == quote && is_unescaped(bytes, index) { + string_quote = None; + } + continue; + } + + match byte { + b'\'' | b'"' => string_quote = Some(byte), + b']' if paren_depth == 0 && brace_depth == 0 => bracket_depth += 1, + b'[' if paren_depth == 0 && brace_depth == 0 && bracket_depth == 0 => { + return callable_before_scalar_argument(before_quote[..index].trim_end()); + } + b'[' if paren_depth == 0 && brace_depth == 0 => bracket_depth -= 1, + b')' => paren_depth += 1, + b'(' if paren_depth > 0 => paren_depth -= 1, + b'(' if bracket_depth == 0 && brace_depth == 0 => { + let before_open = before_quote[..index].trim_end(); + let mut token_start = before_open.len(); + let token_bytes = before_open.as_bytes(); + while token_start > 0 + && (token_bytes[token_start - 1].is_ascii_alphanumeric() + || token_bytes[token_start - 1] == b'_') + { + token_start -= 1; + } + if before_open[token_start..].eq_ignore_ascii_case("array") { + return callable_before_scalar_argument(before_open[..token_start].trim_end()); + } + return None; + } + b'}' => brace_depth += 1, + b'{' if brace_depth > 0 => brace_depth -= 1, + b'{' if bracket_depth == 0 && paren_depth == 0 => return None, + b';' if bracket_depth == 0 && paren_depth == 0 && brace_depth == 0 => return None, + _ => {} + } + } + + None +} + +/// Resource arrays name values; an associative key is bookkeeping, not a disk. +fn string_literal_is_array_key(content: &str, cursor: usize, quote: u8) -> bool { + let bytes = content.as_bytes(); + let mut index = cursor; + while index < bytes.len() { + if bytes[index] == quote && is_unescaped(bytes, index) { + return content[index + 1..].trim_start().starts_with("=>"); + } + if bytes[index] == b'\n' { + return false; + } + index += 1; + } + false +} + +fn string_argument_context<'a>( + content: &'a str, + before_quote: &'a str, + cursor: usize, + quote: u8, +) -> Option> { + if let Some((callable, named_argument)) = callable_before_array_argument(before_quote) { + if string_literal_is_array_key(content, cursor, quote) { + return None; + } + return Some(StringArgumentContext { + callable, + named_argument, + shape: StringArgumentShape::ArrayValue, + }); + } + + let (callable, named_argument) = callable_before_scalar_argument(before_quote)?; + Some(StringArgumentContext { + callable, + named_argument, + shape: StringArgumentShape::Scalar, + }) +} + // ─── Detection ────────────────────────────────────────────────────────────── -/// Detect if the cursor is inside the first string argument of a Laravel -/// helper function. Returns the key kind and the prefix typed so far. +/// Detect if the cursor is inside a supported string argument of a Laravel +/// helper or facade call. Returns the key kind and the prefix typed so far. fn detect_laravel_string_key_context( content: &str, position: Position, @@ -57,17 +233,9 @@ fn detect_laravel_string_key_context( while i > 0 { i -= 1; let ch = bytes[i]; - if ch == b'\'' || ch == b'"' { - let mut bs = 0; - let mut j = i; - while j > 0 && bytes[j - 1] == b'\\' { - bs += 1; - j -= 1; - } - if bs % 2 == 0 { - quote_pos = Some(i); - break; - } + if (ch == b'\'' || ch == b'"') && is_unescaped(bytes, i) { + quote_pos = Some(i); + break; } if ch == b'\n' { return None; @@ -76,18 +244,10 @@ fn detect_laravel_string_key_context( let quote_pos = quote_pos?; let prefix = content[quote_pos + 1..cursor_offset].to_string(); - // ── Before the quote, expect `(` (first argument) ─────────────── + // ── Locate the call argument that owns this string ───────────── let before_quote = content[..quote_pos].trim_end(); - // The calls that take a list of keys rather than one — `getMany([…])`, - // `Route::is([…])`, `View::first([…])` — spell their first entry behind - // an opening bracket, so it counts as the start of the argument too. - let before_quote = before_quote - .strip_suffix('[') - .map_or(before_quote, str::trim_end); - if !before_quote.ends_with('(') { - return None; - } - let before_paren = before_quote[..before_quote.len() - 1].trim_end(); + let argument = string_argument_context(content, before_quote, cursor_offset, bytes[quote_pos])?; + let before_paren = argument.callable; // ── Extract the function/method name ──────────────────────────── let bp_bytes = before_paren.as_bytes(); @@ -113,25 +273,20 @@ fn detect_laravel_string_key_context( // Check for PHP attribute syntax: #[Config('key')] or // #[\Illuminate\Container\Attributes\Config('key')]. - // Strip trailing `\Identifier` segments to handle FQN attributes, - // then check for `#[`. Never search the entire file prefix — - // an unrelated attribute (e.g. `#[Override]`) would false-positive. - let is_attribute = { - let mut s = trimmed_before; - loop { - let stripped = s.trim_end_matches(|c: char| c.is_ascii_alphanumeric() || c == '_'); - if stripped.len() < s.len() && stripped.ends_with('\\') { - s = &stripped[..stripped.len() - 1]; - } else { - s = stripped; - break; - } - } - s.ends_with("#[") || s.ends_with("#") - }; + // Everything between the nearest `#[` and this final class-name segment + // must itself be a class-name prefix. This recognizes FQN attributes + // without letting an unrelated attribute earlier in the file match. + let is_attribute = trimmed_before.rfind("#[").is_some_and(|start| { + trimmed_before[start + 2..] + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'_' || byte == b'\\') + }); // ── Map container attributes to config sub-prefixes ──────────── let (kind, config_sub_prefix) = if is_attribute { + if argument.shape != StringArgumentShape::Scalar { + return None; + } // Resolve the attribute to its Laravel FQN. When the name is // fully qualified (contains `\`), match the FQN directly. // When it's a short name, verify the file imports it from @@ -176,6 +331,30 @@ fn detect_laravel_string_key_context( } }; let fqn_matches = |expected_short: &str| attr_matches(ATTR_NS, expected_short); + // `#[Storage]` turns any argument into a `filesystems.disks.*` key, + // so an application's own same-named attribute would invent one. + // The short spelling therefore has to be imported by that exact + // name, matching what the symbol map records. + let storage_attr_matches = || { + if is_fqn { + attr_class == format!("{ATTR_NS}Storage") + } else { + short == "Storage" + && crate::text_scan::imports_class_as( + content, + &format!("{ATTR_NS}Storage"), + "Storage", + ) + } + }; + + // `#[Storage(disk: '…')]` is the one container attribute whose + // argument is recognised by name. + if let Some(name) = argument.named_argument + && !(storage_attr_matches() && name.eq_ignore_ascii_case("disk")) + { + return None; + } if fqn_matches("Config") { (Some(LaravelStringKind::Config), None) @@ -188,7 +367,7 @@ fn detect_laravel_string_key_context( (Some(LaravelStringKind::Config), Some("cache.stores.")) } else if fqn_matches("Log") { (Some(LaravelStringKind::Config), Some("logging.channels.")) - } else if fqn_matches("Storage") { + } else if storage_attr_matches() { (Some(LaravelStringKind::Config), Some("filesystems.disks.")) } else if fqn_matches("Auth") || fqn_matches("Authenticated") { (Some(LaravelStringKind::Config), Some("auth.guards.")) @@ -210,50 +389,89 @@ fn detect_laravel_string_key_context( } let class_name = &before_colons[cls_start..]; let short = class_name.rsplit('\\').next().unwrap_or(class_name); + let fn_lower = func_name.to_ascii_lowercase(); + let short_lower = short.to_ascii_lowercase(); + + // The `Storage` facade's disk-name arguments: the parameter the disk + // goes in, and whether that parameter also accepts a list. The + // facade is only resolved through the file's imports once a method + // name has matched — `disk()`, `fake()` and `forgetDisk()` are common + // names on unrelated facades, and resolving scans the whole buffer. + let storage_argument = match fn_lower.as_str() { + "disk" => Some(("name", false)), + "fake" | "persistentfake" => Some(("disk", false)), + "forgetdisk" => Some(("disk", true)), + _ => None, + } + .filter(|_| is_storage_facade_name(class_name, &storage_facade_local_names(content))); + let accepts_array = matches!( + (short_lower.as_str(), fn_lower.as_str()), + ("config", "getmany") | ("route", "is" | "currentroutenamed") + ); - match (short.to_ascii_lowercase().as_str(), fn_lower.as_str()) { - ( - "config", - "get" | "getmany" | "set" | "has" | "boolean" | "array" | "collection" | "prepend" - | "push", - ) => (Some(LaravelStringKind::Config), None), - ("view", "make" | "exists") => (Some(LaravelStringKind::View), None), - ("lang", "get" | "has" | "hasforlocale" | "choice") => { - (Some(LaravelStringKind::Trans), None) + if let Some((expected_name, accepts_array)) = storage_argument { + if argument + .named_argument + .is_some_and(|name| !name.eq_ignore_ascii_case(expected_name)) + || (argument.shape == StringArgumentShape::ArrayValue && !accepts_array) + { + return None; + } + (Some(LaravelStringKind::Config), Some("filesystems.disks.")) + } else if argument.named_argument.is_some() + || (argument.shape != StringArgumentShape::Scalar + && (!accepts_array || !before_quote.trim_end().ends_with('['))) + { + (None, None) + } else { + match (short_lower.as_str(), fn_lower.as_str()) { + ( + "config", + "get" | "getmany" | "set" | "has" | "boolean" | "array" | "collection" + | "prepend" | "push", + ) => (Some(LaravelStringKind::Config), None), + ("view", "make" | "exists") => (Some(LaravelStringKind::View), None), + ("lang", "get" | "has" | "hasforlocale" | "choice") => { + (Some(LaravelStringKind::Trans), None) + } + // Route names reached through the URL-building facades, and the + // "is the current route named …?" predicates. + ( + "url" | "redirect" | "response", + "route" | "signedroute" | "temporarysignedroute" | "redirecttoroute", + ) => (Some(LaravelStringKind::Route), None), + ("route", "is" | "currentroutenamed") => (Some(LaravelStringKind::Route), None), + ("env", "get" | "getorfail") => (Some(LaravelStringKind::Env), None), + // Facade methods that accept config sub-keys: + ("auth", "guard") => (Some(LaravelStringKind::Config), Some("auth.guards.")), + ("db", "connection") => ( + Some(LaravelStringKind::Config), + Some("database.connections."), + ), + ("cache", "store") => (Some(LaravelStringKind::Config), Some("cache.stores.")), + ("log", "channel") => (Some(LaravelStringKind::Config), Some("logging.channels.")), + // Artisan command names. + ("artisan", "call" | "queue") => (Some(LaravelStringKind::Command), None), + ("schedule", "command") => (Some(LaravelStringKind::Command), None), + // Eloquent morph aliases. + ("relation", "getmorphedmodel") => (Some(LaravelStringKind::MorphAlias), None), + ("model", "getactualclassnameformorph") => { + (Some(LaravelStringKind::MorphAlias), None) + } + // Authorization abilities checked through the Gate facade. + ( + "gate", + "allows" | "denies" | "check" | "any" | "none" | "authorize" | "inspect" + | "has" | "define", + ) => (Some(LaravelStringKind::GateAbility), None), + _ => (None, None), } - // Route names reached through the URL-building facades, and the - // "is the current route named …?" predicates. - ( - "url" | "redirect" | "response", - "route" | "signedroute" | "temporarysignedroute" | "redirecttoroute", - ) => (Some(LaravelStringKind::Route), None), - ("route", "is" | "currentroutenamed") => (Some(LaravelStringKind::Route), None), - ("env", "get" | "getorfail") => (Some(LaravelStringKind::Env), None), - // Facade methods that accept config sub-keys: - ("auth", "guard") => (Some(LaravelStringKind::Config), Some("auth.guards.")), - ("db", "connection") => ( - Some(LaravelStringKind::Config), - Some("database.connections."), - ), - ("cache", "store") => (Some(LaravelStringKind::Config), Some("cache.stores.")), - ("log", "channel") => (Some(LaravelStringKind::Config), Some("logging.channels.")), - ("storage", "disk") => (Some(LaravelStringKind::Config), Some("filesystems.disks.")), - // Artisan command names. - ("artisan", "call" | "queue") => (Some(LaravelStringKind::Command), None), - ("schedule", "command") => (Some(LaravelStringKind::Command), None), - // Eloquent morph aliases. - ("relation", "getmorphedmodel") => (Some(LaravelStringKind::MorphAlias), None), - ("model", "getactualclassnameformorph") => (Some(LaravelStringKind::MorphAlias), None), - // Authorization abilities checked through the Gate facade. - ( - "gate", - "allows" | "denies" | "check" | "any" | "none" | "authorize" | "inspect" | "has" - | "define", - ) => (Some(LaravelStringKind::GateAbility), None), - _ => (None, None), } } else if is_instance_method { + if argument.named_argument.is_some() || argument.shape != StringArgumentShape::Scalar { + return None; + } // Whether the receiver is `$this` (used to scope command-running // methods, whose names are too generic to match on any object). let receiver_is_this = { @@ -316,7 +534,21 @@ fn detect_laravel_string_key_context( }; (k, None) } else { - match func_name.to_ascii_lowercase().as_str() { + let fn_lower = func_name.to_ascii_lowercase(); + // The Blade preprocessor lowers `@includeFirst`/`@componentFirst`/ + // `@extendsFirst` and `@canany` to markers that name their candidates + // inside an array literal rather than as a plain first argument. + let accepts_array = matches!( + fn_lower.as_str(), + "blade_view_directive" | "blade_can_directive" + ); + if argument.named_argument.is_some() + || (argument.shape != StringArgumentShape::Scalar + && (!accepts_array || !before_quote.trim_end().ends_with('['))) + { + return None; + } + match fn_lower.as_str() { "route" | "to_route" => (Some(LaravelStringKind::Route), None), "config" => (Some(LaravelStringKind::Config), None), "view" | "blade_view_directive" | "blade_each_directive" => { @@ -831,6 +1063,10 @@ impl Backend { fn string_key_candidates(&self, kind: &LaravelStringKind) -> Vec { match kind { LaravelStringKind::Route => self.cached_route_names(), + // Only the keys `config/` declares: a key written at runtime is + // extracted from the same literal the cursor is inside, so the + // half-typed name of a `Storage::fake('…')` under the cursor + // would be offered back as a completion for itself. LaravelStringKind::Config => self.cached_config_keys(), LaravelStringKind::View => self.cached_view_names(), LaravelStringKind::Trans => self.cached_trans_keys(), @@ -850,8 +1086,8 @@ impl Backend { /// Try Laravel string key completion. /// - /// Detects the cursor inside the first string argument of `route()`, - /// `config()`, `view()`, `__()`, etc. and offers matching key names. + /// Detects the cursor inside a supported string argument of `route()`, + /// `config()`, `Storage::forgetDisk()`, etc. and offers matching names. pub(crate) fn try_laravel_string_key_completion( &self, content: &str, @@ -934,6 +1170,25 @@ mod tests { use super::*; use tower_lsp::lsp_types::Position; + fn detect_at_end(content: &str, value: &str) -> Option { + let cursor = content.rfind(value)? + value.len(); + detect_laravel_string_key_context( + content, + crate::text_position::offset_to_position(content, cursor), + ) + } + + fn storage_source(expression: &str) -> String { + format!(" Vec { + match response { + CompletionResponse::Array(items) => items, + CompletionResponse::List(list) => list.items, + } + } + /// Three kinds are recorded as spans but completed from somewhere else, /// or not at all, so they must offer nothing here rather than an empty /// list dressed up as the answer. @@ -1080,6 +1335,28 @@ mod tests { } } + /// `@includeFirst`, `@componentFirst`, `@extendsFirst` and `@canany` + /// name their candidates inside an array literal, so the marker calls + /// they compile to have to complete there and not only for a plain + /// first argument. + #[test] + fn detects_the_blade_markers_that_list_their_names_in_an_array() { + for (marker, value, kind) in [ + ("blade_view_directive", "partials.", LaravelStringKind::View), + ("blade_can_directive", "upd", LaravelStringKind::GateAbility), + ] { + let content = format!(">(), + vec!["archive"] + ); + assert_eq!( + items[0].text_edit, + Some(CompletionTextEdit::Edit(TextEdit { + range: Range::new( + crate::text_position::offset_to_position(content, cursor - 2), + position, + ), + new_text: "archive".to_string(), + })) + ); + } + + #[test] + fn completion_response_item_helper_accepts_list_responses() { + let items = completion_response_items(CompletionResponse::List(CompletionList { + is_incomplete: false, + items: vec![CompletionItem::default()], + })); + assert_eq!(items.len(), 1); + } + + #[test] + fn forget_disk_arrays_complete_values_but_not_associative_keys() { + let content = " 'archive']);\n"; + let key_cursor = content.find("alias").unwrap() + "alias".len(); + assert!( + detect_laravel_string_key_context( + content, + crate::text_position::offset_to_position(content, key_cursor), + ) + .is_none(), + "an associative key is not a disk name" + ); + + let value = detect_at_end(content, "archive").expect("the array value is a disk name"); + assert!(matches!(value.kind, LaravelStringKind::Config)); + assert_eq!(value.prefix, "archive"); + } + + #[test] + fn rejects_invalid_storage_argument_names_and_shapes() { + for expression in [ + "Storage::disk(['archive'])", + "Storage::fake(['archive'])", + "Storage::persistentFake(['archive'])", + "Storage::forgetDisk([['archive']])", + "Storage::disk(disk: 'archive')", + "Storage::fake(name: 'archive')", + "Storage::persistentFake(name: 'archive')", + "Storage::forgetDisk(name: 'archive')", + "#[\\Illuminate\\Container\\Attributes\\Storage(name: 'archive')] class C {}", + "#[\\Illuminate\\Container\\Attributes\\Storage(disk: ['archive'])] class C {}", + "Storage::extend('archive', fn () => null)", + "Config::get(['archive'])", + "route(name: 'archive')", + ] { + let content = storage_source(expression); + assert!( + detect_at_end(&content, "archive").is_none(), + "`{expression}` must not offer disk completion" + ); + } + } + + #[test] + fn storage_argument_scanners_handle_nested_and_malformed_php() { + let before_named = r#"Storage::fake(config: ['message' => 'it\'s', 'factory' => wrap(fn () => new class {})], disk:"#; + assert_eq!( + callable_before_scalar_argument(before_named), + Some(("Storage::fake", Some("disk"))) + ); + + let nested = r#" new class {}), 'it\'s', 'archive']);"#; + let ctx = detect_at_end(nested, "archive") + .expect("balanced nested expressions must not hide the outer array"); + assert_eq!(ctx.config_sub_prefix, Some("filesystems.disks.")); + + assert!(callable_before_scalar_argument("Storage::disk(:").is_none()); + assert!(callable_before_scalar_argument("Storage::disk(name: value").is_none()); + assert!(callable_before_scalar_argument("orphan name:").is_none()); + assert!(enclosing_call_open_paren("completed(); orphan").is_none()); + assert!(enclosing_call_open_paren("orphan").is_none()); + assert!(callable_before_array_argument("factory('archive").is_none()); + assert!(callable_before_array_argument("{ 'archive").is_none()); + assert!(callable_before_array_argument("broken; 'archive").is_none()); + assert!(callable_before_array_argument("orphan").is_none()); + + let escaped_key = r#"'key\'part' => 'archive'"#; + assert!(string_literal_is_array_key( + escaped_key, + "'key".len(), + b'\'' + )); + assert!(!string_literal_is_array_key("'key\n", "'key".len(), b'\'')); + assert!(!string_literal_is_array_key("'key", "'key".len(), b'\'')); + } + #[test] fn detects_view_call() { let content = " bool { }) } +/// Detect whether `package` describes an application rather than a library +/// that something else installs. +/// +/// An application owns its configuration: the `config/` files it ships, plus +/// the framework defaults they merge with, are the whole of what exists, so a +/// key nothing declares is a typo. A library's configuration belongs to +/// whatever application installs it, and that application is a file we never +/// see, so every key the library reads is unjudgeable. +/// +/// Composer's `type` says which it is outright when it is set (`project` is +/// what the Laravel skeleton ships). A `composer.json` that leaves the type +/// at its `library` default is read from what it requires: the framework +/// itself in `require` is an application, while a library depends on the +/// `illuminate/*` components it uses and keeps its copy of the framework in +/// `require-dev` for its test suite. This is why it cannot share +/// [`is_laravel_application`], which counts a dev-only dependency too. +pub(crate) fn is_application_project(package: &ComposerPackage) -> bool { + if let Some(kind) = &package.r#type { + return kind.0.eq_ignore_ascii_case("project"); + } + package.require.keys().any(|name| { + ["laravel/framework", "laravel/laravel"] + .iter() + .any(|app| name.eq_ignore_ascii_case(app)) + }) +} + /// Packages that answer authorization checks from a runtime permission /// table rather than from `Gate::define()` calls or policy classes. /// @@ -1486,6 +1513,35 @@ mod tests { assert!(!is_laravel_application(&library)); } + // ── is_application_project ────────────────────────────────────── + + /// The declared type settles it on its own, whichever way it points. + #[test] + fn a_declared_type_decides_whether_a_project_is_an_application() { + assert!(is_application_project(&pkg( + r#"{"type": "project", "require": {"illuminate/support": "^11.0"}}"# + ))); + assert!(!is_application_project(&pkg( + r#"{"type": "library", "require": {"laravel/framework": "^11.0"}}"# + ))); + } + + /// Without one, requiring the framework itself is what tells an + /// application apart from a package that keeps its copy for tests. + #[test] + fn an_untyped_package_is_read_from_what_it_requires() { + assert!(is_application_project(&pkg( + r#"{"require": {"laravel/framework": "^11.0"}}"# + ))); + assert!(!is_application_project(&pkg( + r#"{"require": {"illuminate/support": "^11.0"}, + "require-dev": {"laravel/framework": "^11.0"}}"# + ))); + assert!(!is_application_project(&pkg( + r#"{"require": {"symfony/console": "^7.0"}}"# + ))); + } + // ── has_runtime_permission_package ────────────────────────────── #[test] diff --git a/src/diagnostics/mod.rs b/src/diagnostics/mod.rs index c3a8b983e..f831b1c4c 100644 --- a/src/diagnostics/mod.rs +++ b/src/diagnostics/mod.rs @@ -785,19 +785,40 @@ impl Backend { } else { (HashSet::new(), Vec::new(), Vec::new()) }; - let config_keys: HashSet = if has_config { - self.cached_config_keys().into_iter().collect() + // A library is installed into an application that declares the + // configuration it reads, and that application is a file we never + // see, so none of its keys can be judged. Only an application owns + // the whole of its configuration. + let has_config = has_config && self.is_application_project(); + let declared_config_keys: Vec = if has_config { + self.cached_config_keys() + } else { + Vec::new() + }; + // A key that `Config::set()` or the array form of the `config()` + // helper establishes is as real as one a `config/` file declares; a + // test that configures a disk in `setUp()` before exercising it is + // the common shape. + let written_config_keys = if has_config { + self.runtime_config_keys() } else { HashSet::new() }; // The config files we managed to enumerate keys from, by name. A // key whose root segment names none of them lives in a file we - // cannot see (a library whose config is supplied by the host - // application), so nothing about it is knowable. - let config_roots: HashSet<&str> = config_keys + // cannot see, so nothing about it is knowable. A runtime write is + // deliberately not a root of its own: the keys one file writes say + // nothing about what the rest of that namespace holds, least of all + // when the writes that established it were spelled dynamically. + let config_roots: HashSet<&str> = declared_config_keys .iter() .map(|key| key.split('.').next().unwrap_or(key.as_str())) .collect(); + let config_keys: HashSet<&str> = declared_config_keys + .iter() + .chain(written_config_keys.iter()) + .map(String::as_str) + .collect(); let view_keys: HashSet = if has_view { self.cached_view_names().into_iter().collect() } else { @@ -934,11 +955,18 @@ impl Backend { continue; } // Config keys may be partial prefixes (e.g. `config('app')`) - // which are valid even without a direct match. - let valid = config_keys.contains(key) + // which are valid even without a direct match. The other + // direction holds for a key written at runtime: the value + // it stored is opaque to us, so every path under it is + // beyond judging as well. + let valid = config_keys.contains(key.as_str()) || config_keys .iter() - .any(|k| k.starts_with(&format!("{}.", key))); + .any(|k| k.starts_with(&format!("{}.", key))) + || written_config_keys.iter().any(|written| { + key.strip_prefix(written.as_str()) + .is_some_and(|rest| rest.starts_with('.')) + }); (valid, "config key", "invalid_laravel_config") } CheckedStringKind::View => { diff --git a/src/indexing/init.rs b/src/indexing/init.rs index 8872ae9d8..a3ea18b5b 100644 --- a/src/indexing/init.rs +++ b/src/indexing/init.rs @@ -73,6 +73,17 @@ impl Backend { .unwrap_or(false); self.resolved_class_cache.write().set_laravel(is_laravel); + // A library's configuration is declared by whatever application + // installs it, so the keys it reads cannot be judged. An `artisan` + // file settles it for an application whose `composer.json` says + // neither way. + self.set_is_application( + composer_json + .as_ref() + .is_some_and(composer::is_application_project) + || root.join("artisan").is_file(), + ); + // A permission package answers authorization checks from the database, // so the abilities this project uses are not written in its source and // the unknown-ability diagnostic has nothing to judge them against. @@ -409,6 +420,10 @@ impl Backend { // authorizing from the database opens the ability space workspace-wide, // since the gate index that judges abilities is shared. let mut any_runtime_permissions = false; + // One application among the subprojects makes the workspace's config + // files the whole configuration; a workspace of libraries alone reads + // keys that only the installing application declares. + let mut any_application = false; for (sub_idx, (sub_root, vendor_dir)) in subprojects.iter().enumerate() { // Each subproject owns an equal slice of the 10..80 range; @@ -436,11 +451,13 @@ impl Backend { } skip_dirs.insert(sub_root.clone()); - if (!any_laravel || !any_runtime_permissions) + if (!any_laravel || !any_runtime_permissions || !any_application) && let Some(pkg) = composer::read_composer_package(sub_root) { any_laravel |= composer::is_laravel_project(&pkg); any_runtime_permissions |= composer::has_runtime_permission_package(&pkg); + any_application |= + composer::is_application_project(&pkg) || sub_root.join("artisan").is_file(); } // ── PSR-4 mappings ────────────────────────────────────── @@ -504,6 +521,7 @@ impl Backend { } self.resolved_class_cache.write().set_laravel(any_laravel); + self.set_is_application(any_application); self.laravel_gates .write() .set_runtime_permission_package(any_runtime_permissions); diff --git a/src/lib.rs b/src/lib.rs index b59919ed7..4421722b3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -764,6 +764,21 @@ pub struct Backend { /// `$user->can()`, `$this->authorize()`, and `@can` check. Empty for /// non-Laravel projects. See [`virtual_members::laravel::gates`]. pub(crate) laravel_gates: Arc>, + /// Config keys the project declares at runtime rather than in a + /// `config/` file (`Config::set()`, the array form of the `config()` + /// helper, `Storage::fake()`), keyed by the file each write is written + /// in so an edit that removes one takes the key with it. A test that + /// configures a disk in `setUp()` before exercising it is the common + /// shape. Empty for non-Laravel projects. + pub(crate) laravel_runtime_config_keys: Arc>>>, + /// Whether the workspace is an application rather than a library. + /// + /// A library's configuration is supplied by whatever application + /// installs it, so the config keys it reads are declared in a file we + /// never see and none of them can be judged. See + /// [`crate::composer::is_application_project`]. Starts `true` so a + /// workspace with no `composer.json` to classify behaves as before. + pub(crate) is_application: Arc, /// Laravel macro seed files (service providers plus the app's provider /// registration files), mapped to the class references each contributed /// at the last macro-index build. An edit that changes a seed's @@ -1143,6 +1158,8 @@ impl Backend { laravel_gates: Arc::new(RwLock::new( virtual_members::laravel::LaravelGateIndex::default(), )), + laravel_runtime_config_keys: Arc::new(RwLock::new(HashMap::new())), + is_application: Arc::new(std::sync::atomic::AtomicBool::new(true)), laravel_macro_seeds: Arc::new(RwLock::new(HashMap::new())), laravel_macro_mixin_uris: Arc::new(RwLock::new(std::collections::HashSet::new())), laravel_date_class: Arc::new(RwLock::new(None)), @@ -1255,6 +1272,8 @@ impl Backend { laravel_gates: Arc::new(RwLock::new( virtual_members::laravel::LaravelGateIndex::default(), )), + laravel_runtime_config_keys: Arc::new(RwLock::new(HashMap::new())), + is_application: Arc::new(std::sync::atomic::AtomicBool::new(true)), laravel_macro_seeds: Arc::new(RwLock::new(HashMap::new())), laravel_macro_mixin_uris: Arc::new(RwLock::new(std::collections::HashSet::new())), laravel_date_class: Arc::new(RwLock::new(None)), @@ -1821,6 +1840,10 @@ impl Backend { // behind. Created/changed files rebuild it when re-parsed // below. self.symbols.uri_globals_index.write().remove(uri); + // A deleted file no longer declares the config keys it wrote + // at runtime. A file that was merely changed re-registers + // them when it is re-parsed below. + self.laravel_runtime_config_keys.write().remove(uri); } } @@ -1909,6 +1932,8 @@ impl Backend { laravel_has_commands: Arc::clone(&self.laravel_has_commands), laravel_morph_map: Arc::clone(&self.laravel_morph_map), laravel_gates: Arc::clone(&self.laravel_gates), + laravel_runtime_config_keys: Arc::clone(&self.laravel_runtime_config_keys), + is_application: Arc::clone(&self.is_application), laravel_macro_seeds: Arc::clone(&self.laravel_macro_seeds), laravel_macro_mixin_uris: Arc::clone(&self.laravel_macro_mixin_uris), laravel_date_class: Arc::clone(&self.laravel_date_class), diff --git a/src/mem_audit.rs b/src/mem_audit.rs index 92cbec5c0..424c8e0a1 100644 --- a/src/mem_audit.rs +++ b/src/mem_audit.rs @@ -1717,6 +1717,7 @@ pub(crate) fn report(backend: &Backend, runner_content_bytes: usize) { *backend.laravel_pivots.write() = Default::default(); *backend.laravel_commands.write() = Default::default(); *backend.laravel_gates.write() = Default::default(); + backend.laravel_runtime_config_keys.write().clear(); }); probe("blade_uris", &mut || backend.blade_uris.write().clear()); probe("phar_archives", &mut || { diff --git a/src/parser/ast_update.rs b/src/parser/ast_update.rs index dc8914994..6c02fe68a 100644 --- a/src/parser/ast_update.rs +++ b/src/parser/ast_update.rs @@ -266,6 +266,12 @@ impl Backend { // files that mention neither `Gate` nor `$policies`. self.refresh_laravel_gates(uri, content); + // Keep the set of config keys the project declares at runtime + // coherent with edits to the files that write them. Reads the spans + // the parse above just stored, so it runs after it rather than + // alongside the token-gated refreshes. + self.refresh_laravel_config_writes(uri); + // Keep the container bindings, package config files, view and // translation directories, route files, and component namespaces // coherent with edits to the providers that register them. Cheap diff --git a/src/symbol_map/extraction/class_like.rs b/src/symbol_map/extraction/class_like.rs index beca67380..bec4afc59 100644 --- a/src/symbol_map/extraction/class_like.rs +++ b/src/symbol_map/extraction/class_like.rs @@ -311,17 +311,28 @@ pub(super) fn extract_from_attribute_lists<'a>( // the file to import from the Illuminate namespace; // that check is cached once per file to avoid repeated // linear scans. - if let Some(kind) = resolve_laravel_container_attr( + if let Some(attribute) = resolve_laravel_container_attr( class_name, &mut ctx.has_laravel_container_attrs, ctx.content, ) { - try_emit_laravel_string_span_partial( - kind, - arg_list, - ctx.content, - &mut ctx.spans, - ); + match attribute { + LaravelContainerAttribute::Config => { + try_emit_laravel_string_span_partial( + crate::symbol_map::LaravelStringKind::Config, + arg_list, + ctx.content, + &mut ctx.spans, + ); + } + LaravelContainerAttribute::StorageDisk => { + try_emit_laravel_storage_disk_span_partial( + arg_list, + ctx.content, + &mut ctx.spans, + ); + } + } } // `#[RedirectToRoute('login')]` on a form request names the diff --git a/src/symbol_map/extraction/expressions/calls.rs b/src/symbol_map/extraction/expressions/calls.rs index 4d9f1e9b9..f5ec0dbef 100644 --- a/src/symbol_map/extraction/expressions/calls.rs +++ b/src/symbol_map/extraction/expressions/calls.rs @@ -333,9 +333,17 @@ fn extract_call<'a>( // Uses if-else to short-circuit (most function calls // won't match) and avoids to_ascii_lowercase() heap // allocations. - let laravel_kind = if name_clean.eq_ignore_ascii_case("config") { - Some(crate::symbol_map::LaravelStringKind::Config) - } else if name_clean.eq_ignore_ascii_case("view") + // The `config()` helper is the one that both reads and + // writes, so it takes a path of its own rather than the + // read-only mapping below. + if name_clean.eq_ignore_ascii_case("config") { + try_emit_laravel_config_helper_spans( + &func_call.argument_list, + ctx.content, + &mut ctx.spans, + ); + } + let laravel_kind = if name_clean.eq_ignore_ascii_case("view") || name_clean.eq_ignore_ascii_case("blade_each_directive") { Some(crate::symbol_map::LaravelStringKind::View) @@ -707,6 +715,14 @@ fn extract_call<'a>( &mut ctx.spans, ); } + try_emit_laravel_storage_disk_spans( + &subject_text, + &member_name, + &static_call.argument_list, + &mut ctx.laravel_storage_facade_names, + ctx.content, + &mut ctx.spans, + ); // The `View` facade proxies the view factory, so every // factory method that names a template does so here too. if clean_subject.eq_ignore_ascii_case("View") diff --git a/src/symbol_map/extraction/laravel.rs b/src/symbol_map/extraction/laravel.rs index 7337a8dea..86a2ee50f 100644 --- a/src/symbol_map/extraction/laravel.rs +++ b/src/symbol_map/extraction/laravel.rs @@ -17,11 +17,18 @@ pub(super) const LARAVEL_CONTAINER_ATTR_NAMES: &[&str] = &[ "Authenticated", ]; +/// The Laravel-specific meaning of a container-injection attribute. +pub(super) enum LaravelContainerAttribute { + /// A full config key, as accepted by `#[Config]` and the pre-existing + /// generic handling for the other container attributes. + Config, + /// A disk name accepted by `#[Storage]`. + StorageDisk, +} + /// Check whether an attribute class name refers to a Laravel container /// attribute (`Config`, `Database`, `Cache`, `Log`, `Storage`, `Auth`, -/// `Authenticated`). Returns the corresponding [`LaravelStringKind`] if -/// so — always `Config` since all container attributes resolve to config -/// sub-keys. +/// `Authenticated`). Returns the corresponding Laravel-specific meaning. /// /// FQN names (containing `\`) are matched directly against /// `Illuminate\Container\Attributes\*`. Short names require the file to @@ -31,24 +38,43 @@ pub(super) fn resolve_laravel_container_attr( class_name: &str, import_cache: &mut Option, content: &str, -) -> Option { - if class_name.contains('\\') { - let stripped = class_name.strip_prefix(LARAVEL_CONTAINER_ATTR_NS)?; - if LARAVEL_CONTAINER_ATTR_NAMES.contains(&stripped) { - return Some(crate::symbol_map::LaravelStringKind::Config); +) -> Option { + let short = if class_name.contains('\\') { + class_name.strip_prefix(LARAVEL_CONTAINER_ATTR_NS)? + } else { + if !LARAVEL_CONTAINER_ATTR_NAMES.contains(&class_name) { + return None; } + let has_import = + *import_cache.get_or_insert_with(|| content.contains(LARAVEL_CONTAINER_ATTR_NS)); + if !has_import { + return None; + } + class_name + }; + if !LARAVEL_CONTAINER_ATTR_NAMES.contains(&short) { return None; } - if !LARAVEL_CONTAINER_ATTR_NAMES.contains(&class_name) { - return None; - } - let has_import = *import_cache - .get_or_insert_with(|| content.contains("use Illuminate\\Container\\Attributes\\")); - if has_import { - Some(crate::symbol_map::LaravelStringKind::Config) - } else { - None + if short != "Storage" { + return Some(LaravelContainerAttribute::Config); } + // Every other container attribute names a whole config key, so a + // dotless argument from an application's own same-named attribute + // records nothing. `#[Storage]` prepends a subtree instead, turning + // any argument into a well-formed key, so the short spelling has to be + // imported by that exact name before it is read as a disk. + (class_name.contains('\\') || imports_laravel_storage_attribute(content)) + .then_some(LaravelContainerAttribute::StorageDisk) +} + +/// Whether the file imports `#[Storage]` from Laravel's container-attribute +/// namespace under that short name. +fn imports_laravel_storage_attribute(content: &str) -> bool { + crate::text_scan::imports_class_as( + content, + &format!("{LARAVEL_CONTAINER_ATTR_NS}Storage"), + "Storage", + ) } /// Namespace prefix for the attributes a form request is configured with. @@ -121,6 +147,163 @@ pub(super) fn try_emit_laravel_string_spans_all( } } +const STORAGE_DISK_CONFIG_PREFIX: &str = "filesystems.disks."; + +/// Emit config-key spans for the disk names accepted by Laravel's `Storage` +/// facade. Registration helpers are writes; `forgetDisk()` is an optional +/// read because forgetting an unconfigured disk is valid. +/// +/// `disk`, `fake` and `forgetDisk` are common method names on other facades +/// (`Http::fake()`, `Mail::fake()`, …), so the names the `Storage` facade +/// answers to in this file are resolved once and cached in `facade_names` +/// rather than rescanning the source at every such call. +pub(super) fn try_emit_laravel_storage_disk_spans( + facade: &str, + member_name: &str, + argument_list: &ArgumentList<'_>, + facade_names: &mut Option>, + content: &str, + spans: &mut Vec, +) { + let (parameter, accepts_array, is_write, is_optional) = + if member_name.eq_ignore_ascii_case("disk") { + ("name", false, false, false) + } else if member_name.eq_ignore_ascii_case("fake") + || member_name.eq_ignore_ascii_case("persistentFake") + { + ("disk", false, true, false) + } else if member_name.eq_ignore_ascii_case("forgetDisk") { + ("disk", true, false, true) + } else { + return; + }; + let facade_names = facade_names.get_or_insert_with(|| { + crate::virtual_members::laravel::storage_facade_local_names(content) + }); + if !crate::virtual_members::laravel::is_storage_facade_name(facade, facade_names) { + return; + } + + let Some(argument) = argument_expr_for_parameter(argument_list, parameter) else { + return; + }; + if accepts_array { + let elements = match argument { + Expression::Array(array) => Some(&array.elements), + Expression::LegacyArray(array) => Some(&array.elements), + _ => None, + }; + if let Some(elements) = elements { + for element in elements.iter() { + let value = match element { + ArrayElement::KeyValue(element) => element.value, + ArrayElement::Value(element) => element.value, + ArrayElement::Variadic(_) | ArrayElement::Missing(_) => continue, + }; + push_storage_disk_span(value, is_write, is_optional, content, spans); + } + return; + } + } + push_storage_disk_span(argument, is_write, is_optional, content, spans); +} + +/// Emit the disk name accepted by Laravel's `#[Storage]` container +/// attribute, selecting the `disk` argument even when named arguments are +/// reordered. +pub(super) fn try_emit_laravel_storage_disk_span_partial( + argument_list: &PartialArgumentList<'_>, + content: &str, + spans: &mut Vec, +) { + if let Some(argument) = partial_argument_expr_for_parameter(argument_list, "disk") { + push_storage_disk_span(argument, false, false, content, spans); + } +} + +fn argument_expr_for_parameter<'a>( + argument_list: &ArgumentList<'a>, + parameter: &str, +) -> Option<&'a Expression<'a>> { + for argument in argument_list.arguments.iter() { + if let Argument::Named(named) = argument + && bytes_to_str(named.name.value).eq_ignore_ascii_case(parameter) + { + return Some(named.value); + } + } + argument_list + .arguments + .iter() + .find_map(|argument| match argument { + Argument::Positional(positional) => Some(positional.value), + Argument::Named(_) => None, + }) +} + +fn partial_argument_expr_for_parameter<'a>( + argument_list: &PartialArgumentList<'a>, + parameter: &str, +) -> Option<&'a Expression<'a>> { + for argument in argument_list.arguments.iter() { + if let PartialArgument::Named(named) = argument + && bytes_to_str(named.name.value).eq_ignore_ascii_case(parameter) + { + return Some(named.value); + } + } + argument_list + .arguments + .iter() + .find_map(|argument| match argument { + PartialArgument::Positional(positional) => Some(positional.value), + _ => None, + }) +} + +fn push_storage_disk_span( + expression: &Expression<'_>, + is_write: bool, + is_optional: bool, + content: &str, + spans: &mut Vec, +) { + let Some((start, end, disk)) = string_literal_content(expression, content) else { + return; + }; + let mut key = String::with_capacity(STORAGE_DISK_CONFIG_PREFIX.len() + disk.len()); + key.push_str(STORAGE_DISK_CONFIG_PREFIX); + key.push_str(disk); + spans.push(SymbolSpan { + start, + end, + kind: SymbolKind::LaravelStringKey { + key, + kind: crate::symbol_map::LaravelStringKind::Config, + is_write, + is_optional, + }, + }); +} + +/// The offsets and text of a plain string literal's content, between the +/// quotes. An interpolated or concatenated expression, or an empty string, +/// names nothing and yields `None`. +fn string_literal_content<'a>( + expr: &Expression<'_>, + content: &'a str, +) -> Option<(u32, u32, &'a str)> { + let Expression::Literal(literal::Literal::String(string)) = expr else { + return None; + }; + let start = string.span.start.offset + 1; + let end = string.span.end.offset - 1; + if start >= end || end as usize > content.len() { + return None; + } + Some((start, end, &content[start as usize..end as usize])) +} + /// Emit the section- or stack-name span for one of the marker calls the /// Blade preprocessor lowers `@yield`, `@section`, `@stack`, `@push` and /// their helpers to, when `name` is one of them. @@ -168,9 +351,13 @@ pub(super) fn try_emit_laravel_config_key_span( if member_name.eq_ignore_ascii_case("getMany") { return try_emit_config_get_many_spans(argument_list, content, spans); } + let is_write = member_name.eq_ignore_ascii_case("set"); + if is_write && try_emit_config_write_array_spans(argument_list, content, spans) { + return; + } emit_laravel_string_span( crate::symbol_map::LaravelStringKind::Config, - member_name.eq_ignore_ascii_case("set"), + is_write, 0, argument_list, content, @@ -178,6 +365,60 @@ pub(super) fn try_emit_laravel_config_key_span( ); } +/// Emit the config-key spans for the `config()` helper, which both reads and +/// writes: `config('app.name')` names the key it reads, while +/// `config(['app.name' => 'Acme'])` declares every key it lists. +pub(super) fn try_emit_laravel_config_helper_spans( + argument_list: &ArgumentList<'_>, + content: &str, + spans: &mut Vec, +) { + if try_emit_config_write_array_spans(argument_list, content, spans) { + return; + } + emit_laravel_string_span( + crate::symbol_map::LaravelStringKind::Config, + false, + 0, + argument_list, + content, + spans, + ); +} + +/// Push a write span for every key the `['app.name' => 'Acme']` argument of a +/// `set()`-shaped call declares, reporting whether that argument was an array +/// at all so the caller can fall back to the single-key spelling. +/// +/// The value side is left alone: only the key names a config path. +fn try_emit_config_write_array_spans( + argument_list: &ArgumentList<'_>, + content: &str, + spans: &mut Vec, +) -> bool { + let Some(first_arg) = argument_list.arguments.iter().next() else { + return false; + }; + let elements = match first_arg.value() { + Expression::Array(array) => &array.elements, + Expression::LegacyArray(array) => &array.elements, + _ => return false, + }; + for element in elements.iter() { + if let ArrayElement::KeyValue(kv) = element { + push_laravel_string_span( + crate::symbol_map::LaravelStringKind::Config, + true, + false, + kv.key, + content, + spans, + ); + } + } + true +} + /// Push a config-key span for every key a `getMany()` argument names. /// /// The array takes both spellings the repository reads: a bare entry is the @@ -236,18 +477,9 @@ fn push_laravel_string_span( content: &str, spans: &mut Vec, ) { - let Expression::Literal(literal::Literal::String(s)) = expr else { + let Some((inner_start, mut inner_end, mut key)) = string_literal_content(expr, content) else { return; }; - let inner_start = s.span.start.offset + 1; - let mut inner_end = s.span.end.offset - 1; - if inner_start >= inner_end || inner_end as usize > content.len() { - return; - } - let mut key = &content[inner_start as usize..inner_end as usize]; - if key.is_empty() { - return; - } if kind == crate::symbol_map::LaravelStringKind::Config && !key.contains('.') { // Require at least one dot: bare keys like 'app' are not valid config paths. @@ -1682,3 +1914,45 @@ pub(super) fn laravel_route_scan_expr( _ => {} } } + +#[cfg(test)] +mod storage_disk_tests { + use super::*; + + #[test] + fn fully_qualified_container_attributes_keep_their_distinct_meanings() { + let mut import_cache = None; + assert!(matches!( + resolve_laravel_container_attr( + "Illuminate\\Container\\Attributes\\Storage", + &mut import_cache, + "" + ), + Some(LaravelContainerAttribute::StorageDisk) + )); + assert!(matches!( + resolve_laravel_container_attr( + "Illuminate\\Container\\Attributes\\Config", + &mut import_cache, + "" + ), + Some(LaravelContainerAttribute::Config) + )); + } + + #[test] + fn storage_attribute_without_a_disk_argument_emits_no_config_key() { + let php = r#" { /// Whether the file imports from `Illuminate\Container\Attributes\` /// (checked once lazily, cached for all attribute inspections). has_laravel_container_attrs: Option, + /// The local names Laravel's `Storage` facade answers to in this file, + /// resolved once lazily. `disk()`, `fake()` and `forgetDisk()` are + /// common method names on unrelated facades, so the question comes up + /// often enough that rescanning the source each time is wasteful. + laravel_storage_facade_names: Option>, /// Whether the file imports from `PHPUnit\Framework\Attributes\`, cached /// the same way as [`Self::has_laravel_container_attrs`]. has_phpunit_attrs: Option, @@ -173,6 +178,7 @@ pub(crate) fn extract_symbol_map(program: &Program<'_>, content: &str) -> Symbol cond_nesting_depth: 0, cond_block_end_stack: Vec::new(), has_laravel_container_attrs: None, + laravel_storage_facade_names: None, has_phpunit_attrs: None, has_laravel_http_attrs: None, in_console_command: false, diff --git a/src/symbol_map/tests.rs b/src/symbol_map/tests.rs index 8ff61e082..3ae0dab99 100644 --- a/src/symbol_map/tests.rs +++ b/src/symbol_map/tests.rs @@ -4601,7 +4601,259 @@ fn real_member_access_is_not_marked_as_array_callable() { } } -// ── Container binding key spans ───────────────────────────────────── +// ── Storage disk config-key spans ─────────────────────────────────── + +/// Every config-key span whose canonical path names a filesystem disk, with +/// its access flags, in source order. +fn storage_disk_keys(map: &SymbolMap) -> Vec<(String, bool, bool)> { + map.spans + .iter() + .filter_map(|span| match &span.kind { + SymbolKind::LaravelStringKey { + kind: LaravelStringKind::Config, + key, + is_write, + is_optional, + } if key.starts_with("filesystems.disks.") => { + Some((key.clone(), *is_write, *is_optional)) + } + _ => None, + }) + .collect() +} + +#[test] +fn every_storage_disk_method_records_a_canonical_config_key() { + for (method, is_write, is_optional) in [ + ("disk", false, false), + ("fake", true, false), + ("persistentFake", true, false), + ("forgetDisk", false, true), + ] { + let php = format!(" 'private'], disk: 'testing'); +Storage::persistentFake(config: [], disk: 'persistent'); +Storage::forgetDisk(disk: 'forgotten'); +"#; + assert_eq!( + storage_disk_keys(&parse_and_extract(php)), + vec![ + ("filesystems.disks.archive".to_string(), false, false), + ("filesystems.disks.testing".to_string(), true, false), + ("filesystems.disks.persistent".to_string(), true, false,), + ("filesystems.disks.forgotten".to_string(), false, true), + ] + ); +} + +#[test] +fn forget_disk_accepts_scalar_and_both_array_spellings() { + let php = r#" 'backup', $dynamic, ...$more]); +Storage::forgetDisk(array('legacy', 'label' => 'cold', $dynamic)); +"#; + assert_eq!( + storage_disk_keys(&parse_and_extract(php)), + vec![ + ("filesystems.disks.scalar".to_string(), false, true), + ("filesystems.disks.archive".to_string(), false, true), + ("filesystems.disks.backup".to_string(), false, true), + ("filesystems.disks.legacy".to_string(), false, true), + ("filesystems.disks.cold".to_string(), false, true), + ] + ); +} + +#[test] +fn fully_qualified_storage_facade_records_a_disk_key() { + let php = r#" null)", + "$storage->disk('archive')", + ] { + let map = parse_and_extract(&format!(" Vec<(String, bool)> { + map.spans + .iter() + .filter_map(|span| match &span.kind { + SymbolKind::LaravelStringKey { + kind: LaravelStringKind::Config, + key, + is_write, + .. + } => Some((key.clone(), *is_write)), + _ => None, + }) + .collect() +} + +/// The array form of a `set()`-shaped call declares every key it lists: the +/// keys are on the left, and the value each is given says nothing. +#[test] +fn the_array_form_of_a_config_write_declares_every_key_it_lists() { + for call in [ + "config(['app.name' => 'Acme', 'app.timezone' => 'UTC'])", + "Config::set(['app.name' => 'Acme', 'app.timezone' => 'UTC'])", + "config()->set(array('app.name' => 'Acme', 'app.timezone' => 'UTC'))", + ] { + let map = parse_and_extract(&format!(" Option<&str> { None } +// ─── `use` statement scanning ─────────────────────────────────────────────── + +/// Call `visit` with the fully-qualified name and local name of every class +/// `use` item in the file, including the members of a group import. +pub(crate) fn for_each_class_import(content: &str, visit: &mut dyn FnMut(&str, &str)) { + for statement in content.split(';') { + let Some(clause) = use_clause(statement) else { + continue; + }; + // `use function …` / `use const …` import other symbol tables. + let mut words = clause.split_ascii_whitespace(); + if words.next().is_some_and(|word| { + word.eq_ignore_ascii_case("function") || word.eq_ignore_ascii_case("const") + }) { + continue; + } + + match clause.split_once('{') { + Some((prefix, items)) => { + let Some(items) = items.rsplit_once('}').map(|(items, _)| items) else { + continue; + }; + let prefix = prefix + .trim() + .trim_start_matches('\\') + .trim_end_matches('\\'); + for item in items.split(',') { + if let Some((name, local)) = use_item(item) { + visit(&format!("{prefix}\\{name}"), local); + } + } + } + None => { + for item in clause.split(',') { + if let Some((name, local)) = use_item(item) { + visit(name, local); + } + } + } + } + } +} + +/// Whether the file imports `fqn` under the local name `local`. +pub(crate) fn imports_class_as(content: &str, fqn: &str, local: &str) -> bool { + let mut imported = false; + for_each_class_import(content, &mut |imported_fqn, imported_local| { + imported |= + imported_local.eq_ignore_ascii_case(local) && imported_fqn.eq_ignore_ascii_case(fqn); + }); + imported +} + +/// The text after the `use` keyword of a statement that opens with one. +/// +/// Only the line the keyword sits on is examined, so an expression that +/// happens to precede the statement does not turn into an import. +fn use_clause(statement: &str) -> Option<&str> { + let mut offset = 0usize; + for line in statement.split_inclusive('\n') { + let trimmed = line.trim_start(); + if trimmed + .get(..4) + .is_some_and(|prefix| prefix.eq_ignore_ascii_case("use ")) + { + let leading = line.len() - trimmed.len(); + return Some(statement[offset + leading + 4..].trim()); + } + offset += line.len(); + } + None +} + +/// Split one `use` item into its imported name and the local name it binds. +fn use_item(item: &str) -> Option<(&str, &str)> { + let mut words = item.split_whitespace(); + let name = words.next()?.trim_start_matches('\\'); + let local = match words.next() { + Some(keyword) if keyword.eq_ignore_ascii_case("as") => words.next()?, + // Anything else is not an import PHP would accept. + Some(_) => return None, + None => name.rsplit('\\').next().unwrap_or(name), + }; + if words.next().is_some() || name.is_empty() || local.is_empty() { + return None; + } + Some((name, local)) +} + +/// Whether the file declares a namespace. An unqualified name in a file +/// without one resolves in the global namespace, where Laravel's class +/// aliases live. +pub(crate) fn source_declares_namespace(content: &str) -> bool { + content.lines().any(|line| { + let mut line = line.trim_start(); + if let Some(rest) = line.strip_prefix("( } } +// ─── Keys declared at runtime ───────────────────────────────────────────────── + +/// The config keys a single file declares at runtime, read from the +/// [`SymbolKind::LaravelStringKey`] spans the extractor already marked as +/// writes. +fn config_write_keys(symbol_map: &SymbolMap) -> Vec { + let mut keys: Vec = symbol_map + .spans + .iter() + .filter_map(|span| match &span.kind { + SymbolKind::LaravelStringKey { + kind: crate::symbol_map::LaravelStringKind::Config, + key, + is_write: true, + .. + } => Some(key.clone()), + _ => None, + }) + .collect(); + keys.sort(); + keys.dedup(); + keys +} + +impl Backend { + /// Record which config keys `uri` declares at runtime, so a later read of + /// one is judged against it. + /// + /// Called after every re-parse: a write the edit removed has to take its + /// key with it, which is why the file's whole set is replaced rather than + /// merged. Vendor files are left out for the same reason the enumeration + /// of `config/` files leaves them out: they are parsed on demand, so + /// including them would make what the diagnostic knows depend on which + /// classes happened to be loaded. + pub(crate) fn refresh_laravel_config_writes(&self, uri: &str) { + if !self.resolved_class_cache.read().is_laravel() { + return; + } + let keys = self + .symbol_maps + .read() + .get(uri) + .map(|map| config_write_keys(map)) + .unwrap_or_default(); + + if keys.is_empty() { + // Only take the write lock when there is something to forget. + if self.laravel_runtime_config_keys.read().contains_key(uri) { + self.laravel_runtime_config_keys.write().remove(uri); + } + return; + } + if self + .workspace + .vendor_uri_prefixes + .lock() + .iter() + .any(|prefix| uri.starts_with(prefix.as_str())) + { + return; + } + let mut index = self.laravel_runtime_config_keys.write(); + if index.get(uri) != Some(&keys) { + index.insert(uri.to_string(), keys); + } + } + + /// Every config key the project declares at runtime. + /// + /// `Config::set('filesystems.disks.ondemand', […])` in a test's `setUp()` + /// establishes a key no `config/` file declares, and a read of it + /// afterwards is as valid as a read of one that ships on disk. + pub(crate) fn runtime_config_keys(&self) -> std::collections::HashSet { + self.laravel_runtime_config_keys + .read() + .values() + .flatten() + .cloned() + .collect() + } + + /// Whether the workspace is an application rather than a library; see + /// [`Backend::is_application`](crate::Backend::is_application). + pub(crate) fn is_application_project(&self) -> bool { + self.is_application + .load(std::sync::atomic::Ordering::Relaxed) + } + + /// Record the application/library classification, once the workspace's + /// `composer.json` files have been read. + pub(crate) fn set_is_application(&self, is_application: bool) { + self.is_application + .store(is_application, std::sync::atomic::Ordering::Relaxed); + } +} + // ─── Public cross-file query API ────────────────────────────────────────────── /// Find all references for a Laravel config key across the project. @@ -356,6 +452,33 @@ pub(crate) fn resolve_config_key_definition_fallback( mod tests { use super::*; + /// The index tracks the file, not the key: an edit that takes the write + /// away has to take what it declared with it, or the key outlives the + /// call that made it. + #[test] + fn a_runtime_write_lasts_exactly_as_long_as_the_call_that_makes_it() { + let backend = Backend::new_test(); + backend.resolved_class_cache.write().set_laravel(true); + let uri = "file:///project/tests/FixtureTest.php"; + + backend.update_ast( + uri, + &Arc::new(" Vec { + let mut names: Vec = Vec::new(); + let mut short_name_taken = false; + + crate::text_scan::for_each_class_import(content, &mut |imported, local| { + if imported.eq_ignore_ascii_case(STORAGE_FACADE_FQN) { + names.push(local.to_string()); + } else if local.eq_ignore_ascii_case("Storage") { + short_name_taken = true; + } + }); + + if !short_name_taken + && !names + .iter() + .any(|name| name.eq_ignore_ascii_case("Storage")) + && !crate::text_scan::source_declares_namespace(content) + { + names.push("Storage".to_string()); + } + names +} + +/// Whether a written class name is Laravel's `Storage` facade, given the +/// local names [`storage_facade_local_names`] found for the file. +pub(crate) fn is_storage_facade_name(class_name: &str, local_names: &[String]) -> bool { + let is_root_qualified = class_name.starts_with('\\'); + let class_name = class_name.trim_start_matches('\\'); + if class_name.eq_ignore_ascii_case(STORAGE_FACADE_FQN) { + return true; + } + if class_name.contains('\\') { + return false; + } + if is_root_qualified { + // `\Storage` names the global alias whatever the file imports. + return class_name.eq_ignore_ascii_case("Storage"); + } + local_names + .iter() + .any(|local| local.eq_ignore_ascii_case(class_name)) +} + #[cfg(test)] #[path = "storage_tests.rs"] mod tests; diff --git a/src/virtual_members/laravel/storage_tests.rs b/src/virtual_members/laravel/storage_tests.rs index 78836b38a..bd4d76f62 100644 --- a/src/virtual_members/laravel/storage_tests.rs +++ b/src/virtual_members/laravel/storage_tests.rs @@ -320,3 +320,73 @@ fn non_contract_return_is_left_untouched() { original.to_string() ); } + +// ─── Storage facade name resolution ───────────────────────────────────────── + +fn resolves(content: &str, class_name: &str) -> bool { + is_storage_facade_name(class_name, &storage_facade_local_names(content)) +} + +#[test] +fn the_facade_answers_to_its_fqn_its_imports_and_the_global_alias() { + let namespaced = " Vec { +/// The shapes that declare a key at runtime, each followed by a read of what +/// it declared. +const RUNTIME_WRITER: &str = "\ + 'local']); + config(['filesystems.disks.inline' => ['driver' => 'local']]); + Storage::fake('scratch'); + } + + public function demo(): void { + Storage::disk('ondemand'); + config('filesystems.disks.inline.driver'); + Storage::disk('scratch'); + Storage::disk('nowhere'); + } +} +"; + +async fn config_diagnostics_for( + composer_json: &str, + consumer_path: &str, + consumer: &str, +) -> Vec { let (backend, dir) = create_psr4_workspace( - COMPOSER_JSON, + composer_json, &[ ("config/app.php", APP_CONFIG), ("config/filesystems.php", FILESYSTEMS_CONFIG), - ("src/Settings.php", CONSUMER), + (consumer_path, consumer), ], ); backend.initialized(InitializedParams {}).await; - let uri = Url::from_file_path(dir.path().join("src/Settings.php")).unwrap(); + let uri = Url::from_file_path(dir.path().join(consumer_path)).unwrap(); backend .did_open(DidOpenTextDocumentParams { text_document: TextDocumentItem { uri: uri.clone(), language_id: "php".to_string(), version: 1, - text: CONSUMER.to_string(), + text: consumer.to_string(), }, }) .await; let mut diags = Vec::new(); - backend.collect_slow_diagnostics(uri.as_str(), CONSUMER, &mut diags); + backend.collect_slow_diagnostics(uri.as_str(), consumer, &mut diags); diags .iter() @@ -85,7 +124,7 @@ async fn config_diagnostics() -> Vec { #[tokio::test] async fn only_a_typo_in_a_config_file_we_read_is_reported() { - let messages = config_diagnostics().await; + let messages = config_diagnostics_for(COMPOSER_JSON, "src/Settings.php", CONSUMER).await; assert_eq!( messages.len(), @@ -98,3 +137,30 @@ async fn only_a_typo_in_a_config_file_we_read_is_reported() { messages[0] ); } + +#[tokio::test] +async fn a_key_written_at_runtime_is_a_declaration() { + let messages = config_diagnostics_for(COMPOSER_JSON, "src/Fixtures.php", RUNTIME_WRITER).await; + + assert_eq!( + messages.len(), + 1, + "only the disk nothing configures is unknown, got: {messages:?}" + ); + assert!( + messages[0].contains("filesystems.disks.nowhere"), + "the flagged key should be the unconfigured disk, got: {}", + messages[0] + ); +} + +#[tokio::test] +async fn a_library_reads_config_its_host_application_declares() { + let messages = + config_diagnostics_for(PACKAGE_COMPOSER_JSON, "src/Settings.php", CONSUMER).await; + + assert!( + messages.is_empty(), + "a package's config comes from the application that installs it, got: {messages:?}" + ); +} diff --git a/tests/integration/laravel_storage_disk_names.rs b/tests/integration/laravel_storage_disk_names.rs new file mode 100644 index 000000000..5326c5bbf --- /dev/null +++ b/tests/integration/laravel_storage_disk_names.rs @@ -0,0 +1,371 @@ +//! End-to-end coverage for Laravel storage disk names backed by config keys. + +use crate::common::create_psr4_workspace; +use phpantom_lsp::Backend; +use tower_lsp::LanguageServer; +use tower_lsp::lsp_types::*; + +const COMPOSER_JSON: &str = r#"{ + "require": { "laravel/framework": "^12.0" }, + "autoload": { "psr-4": { "App\\": "app/" } } +}"#; + +const FILESYSTEMS_CONFIG: &str = r#" 'local', + 'disks' => [ + 'local' => ['driver' => 'local'], + 'archive' => ['driver' => 'local'], + 'backup' => ['driver' => 's3'], + ], +]; +"#; + +fn position_after(content: &str, unique_prefix: &str) -> Position { + let offset = content + .find(unique_prefix) + .unwrap_or_else(|| panic!("missing `{unique_prefix}`")) + + unique_prefix.len(); + let before = &content[..offset]; + let line = before.bytes().filter(|byte| *byte == b'\n').count() as u32; + let character = before + .rsplit_once('\n') + .map_or(before.len(), |(_, tail)| tail.len()) as u32; + Position::new(line, character) +} + +async fn open_workspace(source: &str) -> (Backend, tempfile::TempDir, Url) { + let (backend, dir) = create_psr4_workspace( + COMPOSER_JSON, + &[ + ("config/filesystems.php", FILESYSTEMS_CONFIG), + ("app/DiskConsumer.php", source), + ], + ); + backend.initialized(InitializedParams {}).await; + + let uri = Url::from_file_path(dir.path().join("app/DiskConsumer.php")).unwrap(); + backend + .did_open(DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: uri.clone(), + language_id: "php".to_string(), + version: 1, + text: source.to_string(), + }, + }) + .await; + + (backend, dir, uri) +} + +async fn completion_labels(backend: &Backend, uri: &Url, position: Position) -> Vec { + let response = backend + .completion(CompletionParams { + text_document_position: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: uri.clone() }, + position, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + context: None, + }) + .await + .expect("completion request should succeed"); + + match response { + Some(CompletionResponse::Array(items)) => { + items.into_iter().map(|item| item.label).collect() + } + Some(CompletionResponse::List(list)) => { + list.items.into_iter().map(|item| item.label).collect() + } + None => Vec::new(), + } +} + +fn definition_location(response: GotoDefinitionResponse) -> Location { + match response { + GotoDefinitionResponse::Scalar(location) => location, + GotoDefinitionResponse::Array(mut locations) => locations.remove(0), + GotoDefinitionResponse::Link(mut links) => { + let link = links.remove(0); + Location::new(link.target_uri, link.target_selection_range) + } + } +} + +fn sorted_locations(locations: Vec) -> Vec { + let mut locations = locations + .into_iter() + .map(|location| { + format!( + "{}:{}:{}", + location.uri, location.range.start.line, location.range.start.character + ) + }) + .collect::>(); + locations.sort_unstable(); + locations +} + +#[tokio::test] +async fn scalar_array_and_named_storage_arguments_complete_configured_disks() { + let source = r#" true]); +Storage::forgetDisk(array('ba' => true)); +#[\Illuminate\Container\Attributes\Storage(disk: ['a'])] +class DiskConsumer {} +"#; + let (backend, _dir, uri) = open_workspace(source).await; + + for prefix in [ + "Storage::disk(['a", + "Storage::fake(['a", + "Storage::persistentFake(['a", + "Storage::forgetDisk(config: 'a", + "Storage::forgetDisk(['ar", + "Storage::forgetDisk(array('ba", + "Attributes\\Storage(disk: ['a", + ] { + let labels = completion_labels(&backend, &uri, position_after(source, prefix)).await; + assert!( + labels + .iter() + .all(|label| !["archive", "backup", "local"].contains(&label.as_str())), + "invalid call shape at `{prefix}` offered storage disks: {labels:?}" + ); + } +} + +#[tokio::test] +async fn every_storage_context_navigates_and_hovers_as_its_full_config_key() { + let source = r#">(); + assert_eq!(invalid_config.len(), 2, "got: {invalid_config:#?}"); + + let messages = invalid_config + .iter() + .map(|diagnostic| diagnostic.message.as_str()) + .collect::>(); + assert!( + messages + .iter() + .any(|message| message.contains("filesystems.disks.missing-disk")) + ); + assert!( + messages + .iter() + .any(|message| message.contains("filesystems.disks.missing-attribute")) + ); + for optional_or_written in [ + "testing", + "persistent-testing", + "already-forgotten", + "forgotten-one", + "forgotten-two", + "forgotten-three", + ] { + assert!( + messages + .iter() + .all(|message| !message.contains(optional_or_written)), + "`{optional_or_written}` should not be diagnosed: {messages:?}" + ); + } +} + +#[tokio::test] +async fn storage_calls_generic_config_access_and_declaration_share_references_symmetrically() { + let source = r#"