diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 39c63dcb6..d1ab67e9f 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **UUID and ULID model keys resolve as strings.** Models using Laravel's `HasUuids` or `HasUlids` traits now expose their primary keys as `string` in completion, hover, and type checking, including traits inherited from parent models or composed through other traits. Custom primary-key names are respected. Contributed by @shuvroroy. - **Formatting from the command line.** `phpantom_lsp format` formats every PHP file and Blade template in a project with the same formatter the editor runs on save, and `phpantom_lsp format --check` reports the files that are not formatted and exits non-zero without writing anything, so a CI job can require that a pull request ran the formatter. A run honours whatever the project already formats with, a Laravel Pint, php-cs-fixer, or PHP_CodeSniffer it depends on, and the built-in formatter otherwise, exactly as the editor resolves it, and opens with a line naming what it resolved so a CI log records which formatter enforced the result. Templates whose indentation is output rather than layout are left alone and never fail a check, and formatting turned off in `.phpantom.toml` is reported as such rather than passing as a project where every file happens to be formatted. Paths can be named to restrict the run, `--format github` annotates the pull request diff, and `--format json` is shaped like the object `analyze` and `fix` emit. - **Storage disk names are navigable wherever Laravel accepts one.** `Storage::disk()`, `fake()`, `persistentFake()`, `forgetDisk()`, and the `#[Storage]` container attribute now complete from `config/filesystems.php`; hover shows the config key, Ctrl+Click opens its declaration, and find-references links every use. Calls that require a configured disk report misspellings, while test fakes and disk eviction keep accepting the ad-hoc names Laravel permits at runtime. Contributed by @shuvroroy. - **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. diff --git a/docs/todo.md b/docs/todo.md index dfb549e3b..918798820 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -166,7 +166,6 @@ unlikely to move the needle for most users. | 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 | -| L12 | [`HasUuids` / `HasUlids` trait — `$id` typed as `string`](todo/laravel.md#l12-hasuuids-hasulids-trait-id-typed-as-string) | Low-Medium | Medium | | L44 | [Sibling resource registrations and degenerate resource names](todo/laravel.md#l44-sibling-resource-registrations-and-degenerate-resource-names) | Low-Medium | Medium | | L50 | ["Create route" quick-fix for an unresolved route name](todo/laravel.md#l50-create-route-quick-fix-for-an-unresolved-route-name) | Low-Medium | Medium | | L47 | [Morph aliases in `*_type` column comparisons](todo/laravel.md#l47-morph-aliases-in-_type-column-comparisons) | Low-Medium | Medium-High | diff --git a/docs/todo/laravel.md b/docs/todo/laravel.md index 1bb754d25..97807d7b6 100644 --- a/docs/todo/laravel.md +++ b/docs/todo/laravel.md @@ -354,31 +354,6 @@ methods, or document this as a known limitation. --- -#### L12. `HasUuids` / `HasUlids` trait — `$id` typed as `string` - -**Impact: Low-Medium · Complexity: Medium** - -Models that use `Illuminate\Database\Eloquent\Concerns\HasUuids` or -`HasUlids` have their primary key (`$id` by default) typed as -`string` instead of `int`. Currently PHPantom does not inspect these -traits, so `$model->id` resolves to `int` (from the default Model -stub) instead of `string`. - -Larastan's `bug-2188.php` tests this: `assertType('string', $uuidModel->id)`. - -**Where to change:** In `LaravelModelProvider::provide`, after -synthesizing other virtual properties, check whether the model's -`used_traits` (recursively, including parent traits) contains -`HasUuids` or `HasUlids`. If so, synthesize a virtual `id` property -typed as `string` (or override the existing one). The trait also -overrides `getKeyType()` to return `'string'` and -`getIncrementing()` to return `false`, but for virtual property -purposes just the `id` type is the main gap. - -Alternatively, if the stubs for these traits include `@property` -tags or a typed `$id` override, the PHPDoc provider may handle it -automatically once the traits are loaded. - #### L47. Morph aliases in `*_type` column comparisons **Impact: Low-Medium · Complexity: Medium-High** diff --git a/examples/laravel/app/Demo.php b/examples/laravel/app/Demo.php index 59ed52562..cad013302 100644 --- a/examples/laravel/app/Demo.php +++ b/examples/laravel/app/Demo.php @@ -16,9 +16,11 @@ use App\Http\Requests\UpdateBakeryRequest; use App\Models\Baker; use App\Models\Bakery; +use App\Models\BakeryOrder; use App\Models\BlogAuthor; use App\Models\BlogPost; use App\Models\Customer; +use App\Models\Delivery; use App\Models\Loaf; use App\Models\PostCollection; use App\Models\Review; @@ -54,6 +56,16 @@ class Demo { + // Try: hover or complete the primary keys. HasUuids and HasUlids + // make them strings without a $keyType override or @property tag. + public function uniqueIdentifiers(BakeryOrder $order, Delivery $delivery): string + { + $orderId = $order->id; // HasUuids → string + $trackingId = $delivery->tracking_id; // HasUlids, custom primary key → string + + return $orderId . ':' . $trackingId; + } + // ── Eloquent Virtual Properties ───────────────────────────────────────── // Alphabetical — every property a through w should appear in order. // Trigger completion on `$bakery->` and scan the list. diff --git a/examples/laravel/app/Models/BakeryOrder.php b/examples/laravel/app/Models/BakeryOrder.php new file mode 100644 index 000000000..6e9aada14 --- /dev/null +++ b/examples/laravel/app/Models/BakeryOrder.php @@ -0,0 +1,11 @@ +setUniqueIds(); +$ulidDelivery->setUniqueIds(); + +check('HasUuids generates a string id', is_string($uuidOrder->id)); +check('HasUuids generates a valid UUID', \Illuminate\Support\Str::isUuid($uuidOrder->id)); +check('HasUuids overrides the default key type', $uuidOrder->getKeyType() === 'string'); +check('HasUuids disables incrementing', $uuidOrder->getIncrementing() === false); +check('HasUlids respects a custom primary key', $ulidDelivery->getKeyName() === 'tracking_id'); +check('HasUlids generates a string key', is_string($ulidDelivery->tracking_id)); +check('HasUlids generates a valid ULID', \Illuminate\Support\Str::isUlid($ulidDelivery->tracking_id)); +check('HasUlids overrides the default key type', $ulidDelivery->getKeyType() === 'string'); +check('HasUlids disables incrementing', $ulidDelivery->getIncrementing() === false); +check( + 'Unique identifier demo reads both string keys', + (new \App\Demo())->uniqueIdentifiers($uuidOrder, $ulidDelivery) + === $uuidOrder->id . ':' . $ulidDelivery->tracking_id +); + // ─── Summary ──────────────────────────────────────────────────────────────── echo "\n"; diff --git a/src/virtual_members/laravel/mod.rs b/src/virtual_members/laravel/mod.rs index 48a7eeef3..670ff2b86 100644 --- a/src/virtual_members/laravel/mod.rs +++ b/src/virtual_members/laravel/mod.rs @@ -76,7 +76,8 @@ //! - **Implicit primary key.** Every model exposes a primary key column //! (`id` by default, respecting `$primaryKey`/`$keyType` overrides) //! even when no schema or cast entry describes it, unless the model -//! overrides `getKeyName()`. +//! overrides `getKeyName()`. `HasUuids` and `HasUlids` make the key a +//! string, including when inherited or composed through other traits. //! //! - **Timestamp properties.** `created_at`/`updated_at` (or their //! configured names) are added as the configured Laravel date class, @@ -134,6 +135,7 @@ mod scopes; mod storage; mod string_keys; mod trans_keys; +mod unique_ids; pub(crate) mod validated_shape; pub(crate) mod validation_rules; mod view_data; @@ -847,7 +849,9 @@ impl VirtualMemberProvider for LaravelModelProvider { if !laravel.has_get_key_name_method { let primary_key = laravel.primary_key.as_deref().unwrap_or("id"); if seen_props.insert(primary_key.to_string()) { - let php_type = if laravel.key_type.as_deref() == Some("string") { + let php_type = if laravel.key_type.as_deref() == Some("string") + || unique_ids::uses_unique_string_ids(class, class_loader) + { PhpType::string() } else { PhpType::int() diff --git a/src/virtual_members/laravel/unique_ids.rs b/src/virtual_members/laravel/unique_ids.rs new file mode 100644 index 000000000..4106a2578 --- /dev/null +++ b/src/virtual_members/laravel/unique_ids.rs @@ -0,0 +1,71 @@ +//! Primary-key types supplied by Laravel's UUID and ULID traits. + +use std::sync::Arc; + +use crate::atom::AtomSet; +use crate::types::{ClassInfo, MAX_INHERITANCE_DEPTH, MAX_TRAIT_DEPTH}; + +use super::ELOQUENT_MODEL_FQN; + +/// Whether a model uses `HasUuids` or `HasUlids`, including through +/// composed traits and parent models. Base resolution retains only the +/// leaf class's trait names, so inspect the original declarations too. +pub(super) fn uses_unique_string_ids( + class: &ClassInfo, + class_loader: &dyn Fn(&str) -> Option>, +) -> bool { + let mut visited = AtomSet::default(); + if traits_use_unique_string_ids(class, class_loader, &mut visited, 0) { + return true; + } + + let mut parent_name = class.parent_class; + for _ in 0..MAX_INHERITANCE_DEPTH { + let Some(name) = parent_name else { + break; + }; + // The framework base model has no UUID/ULID trait. Avoid walking + // its internal concerns for every ordinary application model. + if name.eq_ignore_ascii_case(ELOQUENT_MODEL_FQN) { + break; + } + let Some(parent) = class_loader(&name) else { + break; + }; + if traits_use_unique_string_ids(&parent, class_loader, &mut visited, 0) { + return true; + } + parent_name = parent.parent_class; + } + false +} + +fn traits_use_unique_string_ids( + class: &ClassInfo, + class_loader: &dyn Fn(&str) -> Option>, + visited: &mut AtomSet, + depth: u32, +) -> bool { + if depth >= MAX_TRAIT_DEPTH { + return false; + } + for name in &class.used_traits { + let fqn = name.trim_start_matches('\\'); + if fqn.eq_ignore_ascii_case("Illuminate\\Database\\Eloquent\\Concerns\\HasUuids") + || fqn.eq_ignore_ascii_case("Illuminate\\Database\\Eloquent\\Concerns\\HasUlids") + { + return true; + } + if visited.insert(*name) + && let Some(trait_class) = class_loader(name) + && traits_use_unique_string_ids(&trait_class, class_loader, visited, depth + 1) + { + return true; + } + } + false +} + +#[cfg(test)] +#[path = "unique_ids_tests.rs"] +mod tests; diff --git a/src/virtual_members/laravel/unique_ids_tests.rs b/src/virtual_members/laravel/unique_ids_tests.rs new file mode 100644 index 000000000..e02ae6cb2 --- /dev/null +++ b/src/virtual_members/laravel/unique_ids_tests.rs @@ -0,0 +1,213 @@ +use super::*; +use crate::atom::atom; +use crate::php_type::PhpType; +use crate::test_fixtures::{make_class, make_method, no_loader}; +use crate::types::{ClassLikeKind, PropertySource, Visibility}; +use crate::virtual_members::laravel::LaravelModelProvider; +use crate::virtual_members::laravel::database_schema::{SchemaIndex, parse_schema_dump}; +use crate::virtual_members::{VirtualMemberProvider, VirtualMembers, new_resolved_class_cache}; + +const UUIDS: &str = "Illuminate\\Database\\Eloquent\\Concerns\\HasUuids"; +const ULIDS: &str = "Illuminate\\Database\\Eloquent\\Concerns\\HasUlids"; + +fn model(traits: &[&str]) -> ClassInfo { + let mut class = make_class("App\\Models\\User"); + class.parent_class = Some(atom(ELOQUENT_MODEL_FQN)); + class.used_traits = traits.iter().map(|name| atom(name)).collect(); + class.laravel_mut(); + class +} + +fn assert_key(members: &VirtualMembers, name: &str, expected: &str) { + let keys: Vec<_> = members + .properties + .iter() + .filter(|p| p.name == name) + .collect(); + assert_eq!(keys.len(), 1, "expected one {name} property"); + assert_eq!(keys[0].type_hint_str().as_deref(), Some(expected)); + assert_eq!(keys[0].visibility, Visibility::Public); + assert!(!keys[0].is_static); +} + +#[test] +fn unique_ids_type_primary_keys_as_strings() { + for trait_name in [ + UUIDS, + ULIDS, + "\\Illuminate\\Database\\Eloquent\\Concerns\\hasuuids", + ] { + let mut class = model(&[trait_name]); + // The traits override the framework's default $keyType = 'int'. + class.laravel_mut().key_type = Some("int".into()); + assert!(uses_unique_string_ids(&class, &no_loader)); + assert_key( + &LaravelModelProvider.provide(&class, &no_loader, None), + "id", + "string", + ); + + class.laravel_mut().primary_key = Some("identifier".into()); + let members = LaravelModelProvider.provide(&class, &no_loader, None); + assert_key(&members, "identifier", "string"); + assert!(members.properties.iter().all(|p| p.name != "id")); + } +} + +#[test] +fn unique_ids_follow_parent_models_and_composed_traits() { + for trait_name in [UUIDS, ULIDS] { + let mut nested = make_class("App\\Concerns\\Identifiers"); + nested.kind = ClassLikeKind::Trait; + nested.used_traits = vec![atom(trait_name)]; + let nested = Arc::new(nested); + let mut wrapper = make_class("App\\Concerns\\ModelTraits"); + wrapper.kind = ClassLikeKind::Trait; + wrapper.used_traits = vec![atom("App\\Concerns\\Missing"), nested.name]; + let wrapper = Arc::new(wrapper); + let mut parent = model(&[&wrapper.name]); + parent.name = atom("App\\Models\\BaseModel"); + let parent = Arc::new(parent); + let mut middle = model(&[]); + middle.name = atom("App\\Models\\Intermediate"); + middle.parent_class = Some(parent.name); + let middle = Arc::new(middle); + let loader = |name: &str| { + [&nested, &wrapper, &parent, &middle] + .into_iter() + .find(|class| class.name == name) + .map(Arc::clone) + }; + let mut class = model(&[]); + class.parent_class = Some(middle.name); + assert_key( + &LaravelModelProvider.provide(&class, &loader, None), + "id", + "string", + ); + class.parent_class = Some(atom(ELOQUENT_MODEL_FQN)); + class.used_traits = vec![wrapper.name]; + assert_key( + &LaravelModelProvider.provide(&class, &loader, None), + "id", + "string", + ); + } +} + +#[test] +fn unique_ids_do_not_match_unrelated_trait_names() { + let class = model(&["HasUuids", "App\\HasUuids", "App\\HasUlids"]); + assert!(!uses_unique_string_ids(&class, &no_loader)); + assert_key( + &LaravelModelProvider.provide(&class, &no_loader, None), + "id", + "int", + ); + let mut plain = make_class("PlainClass"); + plain.used_traits = vec![atom(UUIDS)]; + assert!(!LaravelModelProvider.applies_to(&plain, &no_loader)); +} + +#[test] +fn unique_ids_preserve_explicit_attribute_types() { + for trait_name in [UUIDS, ULIDS] { + let mut class = model(&[trait_name]); + class.laravel_mut().casts_definitions = vec![("id".into(), "integer".into())]; + assert_key( + &LaravelModelProvider.provide(&class, &no_loader, None), + "id", + "int", + ); + class.laravel_mut().casts_definitions.clear(); + class.laravel_mut().attributes_definitions = vec![("id".into(), PhpType::null())]; + assert_key( + &LaravelModelProvider.provide(&class, &no_loader, None), + "id", + "null", + ); + class.laravel_mut().attributes_definitions.clear(); + class + .methods + .push(Arc::new(make_method("getIdAttribute", Some("int")))); + assert_key( + &LaravelModelProvider.provide(&class, &no_loader, None), + "id", + "int", + ); + } +} + +#[test] +fn unique_ids_preserve_schema_metadata_and_dynamic_key_names() { + let mut class = model(&[UUIDS]); + let cache = new_resolved_class_cache(); + cache.write().set_schema_index(SchemaIndex::from_tables( + Some("primary".into()), + parse_schema_dump("primary", "CREATE TABLE users (id varchar(36) NOT NULL);"), + )); + let members = LaravelModelProvider.provide(&class, &no_loader, Some(&cache)); + assert_key(&members, "id", "string"); + assert!(matches!( + members + .properties + .iter() + .find(|p| p.name == "id") + .unwrap() + .source, + Some(PropertySource::DatabaseColumn { .. }) + )); + class.laravel_mut().has_get_key_name_method = true; + let members = LaravelModelProvider.provide(&class, &no_loader, None); + assert!(members.properties.iter().all(|p| p.name != "id")); +} + +#[test] +fn unique_ids_handle_missing_parents_and_trait_cycles() { + let mut class = model(&[]); + class.parent_class = None; + assert!(!uses_unique_string_ids(&class, &no_loader)); + class.parent_class = Some(atom("Missing")); + assert!(!uses_unique_string_ids(&class, &no_loader)); + + let mut cycle = make_class("CyclicTrait"); + cycle.used_traits = vec![cycle.name]; + let cycle = Arc::new(cycle); + let loader = |name: &str| (name == cycle.name).then(|| Arc::clone(&cycle)); + class.used_traits = vec![cycle.name, cycle.name]; + assert!(!uses_unique_string_ids(&class, &loader)); + class.used_traits.push(atom(ULIDS)); + assert!(uses_unique_string_ids(&class, &loader)); +} + +#[test] +fn unique_ids_bound_inheritance_and_trait_depth() { + let mut parent = make_class("CyclicParent"); + parent.parent_class = Some(parent.name); + let parent = Arc::new(parent); + let mut class = model(&[]); + class.parent_class = Some(parent.name); + let loads = std::cell::Cell::new(0); + assert!(!uses_unique_string_ids(&class, &|_| { + loads.set(loads.get() + 1); + Some(Arc::clone(&parent)) + })); + assert_eq!(loads.get(), MAX_INHERITANCE_DEPTH); + + let traits: Vec<_> = (0..MAX_TRAIT_DEPTH) + .map(|i| { + let mut class = make_class(&format!("Trait{i}")); + class.used_traits = vec![atom(&format!("Trait{}", i + 1))]; + Arc::new(class) + }) + .collect(); + let loader = |name: &str| { + traits + .iter() + .find(|class| class.name == name) + .map(Arc::clone) + }; + class.parent_class = None; + class.used_traits = vec![traits[0].name]; + assert!(!uses_unique_string_ids(&class, &loader)); +} diff --git a/tests/integration/laravel_unique_ids.rs b/tests/integration/laravel_unique_ids.rs new file mode 100644 index 000000000..4cb0c382b --- /dev/null +++ b/tests/integration/laravel_unique_ids.rs @@ -0,0 +1,202 @@ +use crate::common::{create_psr4_workspace, open_php}; +use tower_lsp::LanguageServer; +use tower_lsp::lsp_types::*; + +const COMPOSER: &str = r#"{ + "autoload": { "psr-4": { + "App\\": "src/", + "Illuminate\\Database\\Eloquent\\": "vendor/illuminate/Eloquent/" + }} +}"#; + +const MODEL: &str = r#"{key}; }}\nfunction rejects(Example $model): {wrong} {{ return $model->{key}; }}\n$model = new Example();\n$model->{key};\n" + ); + let uri = Url::from_file_path(dir.path().join("src/usage.php")).unwrap(); + open_php(&backend, &uri, &content).await; + + let mut diagnostics = Vec::new(); + backend.collect_slow_diagnostics(uri.as_str(), &content, &mut diagnostics); + assert_eq!(diagnostics.len(), 1, "{declaration}: {diagnostics:?}"); + assert_eq!(diagnostics[0].range.start.line, 3); + assert_eq!( + diagnostics[0].code, + Some(NumberOrString::String("type_mismatch_return".into())) + ); + assert_eq!( + diagnostics[0].message, + format!("Return type {expected} is incompatible with declared return type {wrong}") + ); + + let position = TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: uri.clone() }, + position: Position::new(5, 9), + }; + let hover = backend + .hover(HoverParams { + text_document_position_params: position.clone(), + work_done_progress_params: Default::default(), + }) + .await + .unwrap() + .expect("primary-key hover"); + let HoverContents::Markup(hover) = hover.contents else { + panic!("expected markup hover") + }; + assert!( + hover.value.contains(&format!("`{expected}`")) + || hover.value.contains(&format!("{expected} ${key}")), + "{declaration}: {}", + hover.value + ); + + let completion = backend + .completion(CompletionParams { + text_document_position: TextDocumentPositionParams { + position: Position::new(5, 8), + ..position + }, + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + context: None, + }) + .await + .unwrap() + .expect("model completion"); + let items = match completion { + CompletionResponse::Array(items) => items, + CompletionResponse::List(list) => list.items, + }; + let keys: Vec<_> = items.iter().filter(|item| item.label == key).collect(); + assert_eq!(keys.len(), 1, "{declaration}: {items:?}"); + assert!( + keys[0] + .detail + .as_deref() + .unwrap_or_default() + .contains(expected), + "{:?}", + keys[0] + ); + } +} + +#[tokio::test] +async fn unique_ids_refresh_when_an_inherited_trait_changes() { + let (backend, dir) = create_psr4_workspace( + COMPOSER, + &[ + ("vendor/illuminate/Eloquent/Model.php", MODEL), + ("vendor/illuminate/Eloquent/Concerns/HasUlids.php", ULIDS), + ("src/Concerns/Identified.php", WRAPPER), + ("src/Models/BaseModel.php", PARENT), + ( + "src/Models/Example.php", + "id; }\n"; + open_php(&backend, &uri, content).await; + let mut diagnostics = Vec::new(); + backend.collect_slow_diagnostics(uri.as_str(), content, &mut diagnostics); + assert!(diagnostics.is_empty(), "{diagnostics:?}"); + + let trait_uri = Url::from_file_path(dir.path().join("src/Concerns/Identified.php")).unwrap(); + open_php( + &backend, + &trait_uri, + "