Fix relationship property and BelongsTo diagnostics for Laravel models

Author: AJenbo

docs/CHANGELOG.md

@@ -57,6 +57,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - **Variable reassignment inside `try` blocks now tracked by the analyzer.** When a variable was reassigned to a different type inside a `try` block, subsequent accesses within the same block still resolved against the original type, producing false-positive `unknown_member` diagnostics. The same applied to `catch` and `finally` blocks. Assignments preceding the cursor within the same block are now treated as sequential.
+- **Types with `covariant` or `contravariant` variance annotations in generic args now parse correctly.** PHPStan/Larastan annotations like `BelongsTo<Category, covariant $this>` previously failed to parse, causing the entire type to become unresolvable. This broke Eloquent relationship property synthesis (virtual properties were not generated when the `@return` annotation used variance modifiers) and member lookup on relationship method return types (e.g. `$model->category()->associate()` reported `associate()` as not found). The variance keywords are now stripped from generic parameter positions before parsing.
 - **Diagnostics now work for vendor files open in the editor.** Projects using `--prefer-source` or monorepo setups where libraries are edited from the `vendor/` directory no longer have diagnostics suppressed. Previously all vendor files were unconditionally skipped, hiding syntax errors, deprecation warnings, and all other diagnostics. The `analyze` command also accepts vendor paths as explicit overrides (e.g. `phpantom_lsp analyze vendor/some-package/src/`).
 - **Full-line diagnostic suppression no longer requires a code mapping.** Full-line diagnostics (from tools like PHPStan that only report line numbers) are now suppressed whenever any precise diagnostic exists on the same line, regardless of error codes. Previously suppression relied on a hand-maintained mapping between error codes from different tools, which was incomplete and allowed redundant full-line underlines to obscure the precise location of the issue.
 

docs/todo.md

@@ -23,7 +23,6 @@ within the same impact tier.
 
 | #   | Item                                                                                                                                             | Impact | Effort |
 | --- | ------------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ------ |
-| B4  | [Relationship property access and BelongsTo return type not resolved](todo/bugs.md#b4-relationship-property-access-and-belongsto-return-type-not-resolved-by-analyzer) | Medium | Medium |
 | B5  | [`$this->items` on custom Collection subclass not typed](todo/bugs.md#b5-thisitems-on-custom-collection-subclass-not-typed)                      | Low    | Medium |
 | B6  | [Scope methods not found on Builder in analyzer chains](todo/bugs.md#b6-scope-methods-not-found-on-builder-in-analyzer-chains)                   | High   | Medium |
 | B7  | [PHPDoc `@param` generic array type not merged with native `array` hint](todo/bugs.md#b7-phpdoc-param-generic-array-type-not-merged-with-native-array-hint) | Low | Medium |

docs/todo/bugs.md

@@ -1,48 +1,5 @@
 # PHPantom — Bug Fixes
 
-## B4: Relationship property access and BelongsTo return type not resolved by analyzer
-
-Eloquent relationship methods accessed as properties (without `()`) are
-resolved correctly in the completion engine via `LaravelModelProvider`,
-which synthesizes virtual properties (e.g. `translations()` returning
-`HasMany<T>` produces `$translations` typed as `Collection<T>`). However,
-the analyzer's member-access checker does not find these synthesized
-properties in some cross-file scenarios, reporting
-`unresolved_member_access`.
-
-Additionally, calling a relationship method WITH `()` (e.g.
-`$translation->category()`) returns a `BelongsTo` type that the LSP can
-resolve, but then member lookup on that `BelongsTo` fails to find
-methods like `associate()`. The `covariant $this` syntax in generic args
-(e.g. `BelongsTo<NotificationCategory, covariant $this>`) may interfere
-with type parsing.
-
-A separate sub-issue: `FlowService:477` accesses `$order->orderProducts`
-(camelCase) while the model declares the property and relationship method
-as `orderproducts` (all lowercase). Laravel normalises via `Str::snake()`
-at runtime, but the LSP does a case-sensitive property lookup.
-
-Affected diagnostics:
-
-- `NotificationCategory:52` — `$this->translations` property not resolved
-  (HasMany relationship, no `@property` annotation)
-- `NotificationObject:114` — `$this->imageFile` property not resolved
-  (HasOne relationship, no `@property` annotation)
-- `NotificationCategoryService:37` — `$translation->category()->associate()`
-  — `BelongsTo` return type resolves but `associate()` not found on it
-
-`FlowService:477` (`$order->orderProducts`) is a case-sensitivity issue
-filed separately as B9.
-
-`FlowService:517` is a compound failure: `$reorder->order->orderproducts`
-is a relationship property (this bug), then `->reduce()` returns `mixed`
-instead of `Decimal` (generic return type inference gap), then `->add()`
-fails on the unresolved type.
-
-**Impact:** 4 diagnostics in the shared project
-(`NotificationCategory:52`, `NotificationObject:114`,
-`NotificationCategoryService:37`, `FlowService:517`).
-
 ## B5: `$this->items` on custom Collection subclass not typed
 
 When a class extends `Collection<int, T>` via `@extends`, accessing

example.php

@@ -1655,6 +1655,11 @@ public function demo(): void
         $bakery->vendor_count;        // relationship count         → int
         $bakery->warmth;              // $appends (no cast/attr)    → mixed
         // MUST NOT appear: secret_ingredient (private $attributes field)
+
+        // BelongsTo relationship property + method call with covariant $this
+        $post = new BlogPost();
+        $post->author;                // relationship BelongsTo     → BlogAuthor
+        $post->author()->associate($post->author); // associate() on BelongsTo
     }
 }
 
@@ -4801,6 +4806,9 @@ class BlogPost extends \Illuminate\Database\Eloquent\Model
 {
     public function getTitle(): string { return ''; }
     public function getSlug(): string { return ''; }
+
+    /** @return \Illuminate\Database\Eloquent\Relations\BelongsTo<BlogAuthor, covariant $this> */
+    public function author(): mixed { return $this->belongsTo(BlogAuthor::class); }
 }
 
 class AuthorProfile extends \Illuminate\Database\Eloquent\Model
@@ -5740,7 +5748,10 @@ public function orderBy(string $column, string $direction = 'asc'): static { ret
     }
     class HasMany extends Relation {}
     class HasOne extends Relation {}
-    class BelongsTo extends Relation {}
+    class BelongsTo extends Relation {
+        public function associate(mixed $model): static { return $this; }
+        public function dissociate(): static { return $this; }
+    }
     class BelongsToMany extends Relation {}
     class MorphOne extends Relation {}
     class MorphMany extends Relation {}

src/php_type.rs

@@ -137,18 +137,28 @@ impl PhpType {
     ///
     /// This never fails. If the input cannot be parsed by `mago_type_syntax`,
     /// returns `PhpType::Raw(input)`.
+    ///
+    /// PHPStan/Larastan variance annotations (`covariant`, `contravariant`)
+    /// inside generic parameter positions are stripped before parsing so
+    /// that types like `BelongsTo<Category, covariant $this>` parse as
+    /// `Generic("BelongsTo", [Named("Category"), Named("$this")])` instead
+    /// of falling back to `Raw(…)`.
     pub fn parse(input: &str) -> PhpType {
         if input.is_empty() {
             return PhpType::Raw(String::new());
         }
 
+        // Strip variance annotations that mago_type_syntax cannot parse.
+        let cleaned = strip_variance_annotations_from_type(input);
+        let effective: &str = &cleaned;
+
         let span = Span::new(
             FileId::zero(),
             Position::new(0),
-            Position::new(input.len() as u32),
+            Position::new(effective.len() as u32),
         );
 
-        match mago_type_syntax::parse_str(span, input) {
+        match mago_type_syntax::parse_str(span, effective) {
             Ok(ty) => convert(&ty),
             Err(_) => PhpType::Raw(input.to_owned()),
         }
@@ -1496,6 +1506,56 @@ impl PhpType {
 ///
 /// This is a superset of [`is_scalar_name`] that also includes PHPDoc-only
 /// pseudo-types and special names that `resolve_type_string` skips.
+/// Strip `covariant ` and `contravariant ` prefixes from generic type
+/// arguments so that `mago_type_syntax` can parse the type.
+///
+/// Only strips the keywords when they appear immediately after `<` or `,`
+/// (with optional whitespace), i.e. inside generic parameter positions.
+/// Returns the input unchanged (no allocation) when no annotations are
+/// found.
+fn strip_variance_annotations_from_type(s: &str) -> std::borrow::Cow<'_, str> {
+    // Fast path: no variance annotations at all.
+    if !s.contains("covariant ") && !s.contains("contravariant ") {
+        return std::borrow::Cow::Borrowed(s);
+    }
+
+    let mut cleaned = String::with_capacity(s.len());
+    let bytes = s.as_bytes();
+    let mut i = 0usize;
+
+    while i < bytes.len() {
+        // Check whether the preceding non-whitespace is `<` or `,`,
+        // meaning we are inside a generic parameter position.
+        let preceded_by_generic_delimiter = || -> bool {
+            let mut j = i;
+            while j > 0 {
+                j -= 1;
+                if !bytes[j].is_ascii_whitespace() {
+                    return bytes[j] == b'<' || bytes[j] == b',';
+                }
+            }
+            false
+        };
+
+        if i + "covariant ".len() <= bytes.len()
+            && &bytes[i..i + "covariant ".len()] == b"covariant "
+            && preceded_by_generic_delimiter()
+        {
+            i += "covariant ".len();
+        } else if i + "contravariant ".len() <= bytes.len()
+            && &bytes[i..i + "contravariant ".len()] == b"contravariant "
+            && preceded_by_generic_delimiter()
+        {
+            i += "contravariant ".len();
+        } else {
+            cleaned.push(bytes[i] as char);
+            i += 1;
+        }
+    }
+
+    std::borrow::Cow::Owned(cleaned)
+}
+
 fn is_keyword_type(name: &str) -> bool {
     if is_scalar_name(name) {
         return true;
@@ -2215,6 +2275,127 @@ mod tests {
         assert_round_trip("non-empty-array<string>");
     }
 
+    #[test]
+    fn parse_generic_with_covariant_this() {
+        // Laravel/Larastan uses `covariant $this` in generic args, e.g.
+        // `BelongsTo<Category, covariant $this>`.  The parser should
+        // still extract the base class name (`BelongsTo`) so that
+        // member lookup works on the relationship class.
+        let ty = PhpType::parse("BelongsTo<Category, covariant $this>");
+        let base = ty.base_name();
+        assert_eq!(
+            base,
+            Some("BelongsTo"),
+            "base_name should be 'BelongsTo' even with 'covariant $this' arg, got: {:?} from {:?}",
+            base,
+            ty,
+        );
+    }
+
+    #[test]
+    fn parse_generic_with_covariant_preserves_structure() {
+        // The full Generic structure should be preserved after stripping.
+        let ty = PhpType::parse("HasMany<Post, covariant $this>");
+        match &ty {
+            PhpType::Generic(name, args) => {
+                assert_eq!(name, "HasMany");
+                assert_eq!(args.len(), 2);
+                assert_eq!(args[0].to_string(), "Post");
+                assert_eq!(args[1].to_string(), "$this");
+            }
+            other => panic!("expected Generic, got: {:?}", other),
+        }
+    }
+
+    #[test]
+    fn parse_generic_with_contravariant() {
+        let ty = PhpType::parse("Comparator<contravariant T>");
+        assert_eq!(
+            ty.base_name(),
+            Some("Comparator"),
+            "base_name should work with contravariant annotation",
+        );
+        match &ty {
+            PhpType::Generic(_, args) => {
+                assert_eq!(args.len(), 1);
+                assert_eq!(args[0].to_string(), "T");
+            }
+            other => panic!("expected Generic, got: {:?}", other),
+        }
+    }
+
+    #[test]
+    fn parse_generic_with_covariant_fqn() {
+        // Fully-qualified relationship type with covariant $this.
+        let ty = PhpType::parse(
+            "Illuminate\\Database\\Eloquent\\Relations\\BelongsTo<Category, covariant $this>",
+        );
+        assert_eq!(
+            ty.base_name(),
+            Some("Illuminate\\Database\\Eloquent\\Relations\\BelongsTo"),
+        );
+    }
+
+    #[test]
+    fn parse_generic_with_multiple_covariant_args() {
+        let ty = PhpType::parse("Map<covariant TKey, covariant TValue>");
+        match &ty {
+            PhpType::Generic(name, args) => {
+                assert_eq!(name, "Map");
+                assert_eq!(args.len(), 2);
+                assert_eq!(args[0].to_string(), "TKey");
+                assert_eq!(args[1].to_string(), "TValue");
+            }
+            other => panic!("expected Generic, got: {:?}", other),
+        }
+    }
+
+    #[test]
+    fn parse_no_false_strip_of_covariant_class_name() {
+        // A class named `covariant` (unlikely but possible) should not
+        // be stripped when it is NOT inside a generic parameter position.
+        // It appears at the top level, not after `<` or `,`.
+        let ty = PhpType::parse("covariant");
+        // mago may or may not parse this as a Named type; the key is
+        // that stripping should NOT remove it since it's not after < or ,.
+        assert_ne!(ty.to_string(), "", "should not produce empty string");
+    }
+
+    #[test]
+    fn parse_generic_without_covariant_unchanged() {
+        // Normal generics without variance annotations should be unaffected.
+        let ty = PhpType::parse("Collection<int, User>");
+        match &ty {
+            PhpType::Generic(name, args) => {
+                assert_eq!(name, "Collection");
+                assert_eq!(args.len(), 2);
+            }
+            other => panic!("expected Generic, got: {:?}", other),
+        }
+    }
+
+    #[test]
+    fn parse_covariant_array_shape_in_generic() {
+        // `covariant array{...}` inside a generic — the array shape
+        // should still parse after stripping the variance keyword.
+        let ty = PhpType::parse(
+            "Collection<int, covariant array{customer: Customer, contact: Contact|null}>",
+        );
+        assert_eq!(ty.base_name(), Some("Collection"));
+        match &ty {
+            PhpType::Generic(_, args) => {
+                assert_eq!(args.len(), 2);
+                // The second arg should be an array shape, not Raw.
+                assert!(
+                    matches!(&args[1], PhpType::ArrayShape(_)),
+                    "second arg should be ArrayShape after stripping covariant, got: {:?}",
+                    args[1],
+                );
+            }
+            other => panic!("expected Generic, got: {:?}", other),
+        }
+    }
+
     #[test]
     fn round_trip_class_references() {
         assert_round_trip("Foo\\Bar");