Nullable type not resolved to its base class, incorrect namespace

applied to @see

Author: AJenbo

docs/CHANGELOG.md

@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Null-safe method chain resolution.** Null-safe method calls (`$obj?->method()`) now resolve the return type correctly for variable type inference, including cross-file chains. Previously `?->` calls were ignored by the RHS resolution pipeline, losing the type for any variable assigned from a null-safe chain.
+- **Nullable and generic types in class lookup.** Variables typed as `?ClassName` or `Collection<Item>` now resolve correctly across all code paths. Previously the `?` prefix and generic parameters were not stripped before class lookup, causing the type engine to treat them as unknown types. This fixes completion, hover, go-to-definition, and false-positive diagnostics for any variable whose type uses the nullable shorthand or carries generic arguments.
+- **`@see` references with qualified class names.** Docblock `@see Fully\Qualified\ClassName` references no longer have the file's namespace prepended, which previously produced doubled names like `App\Models\App\Models\User`. Qualified names in `@see` tags are now treated as fully-qualified.
 - **False-positive diagnostics for same-named variables in different methods.** When two methods in the same class both used a variable like `$order`, the diagnostic cache resolved it once and reused that type for every other method in the class. The second method saw the wrong type and flagged valid member accesses as unknown. Both the per-file subject cache and the cross-collector resolution cache are now scoped to the enclosing function/method/closure body, so each method resolves variables independently.
 - **False-positive diagnostics for `$this` inside traits.** Accessing host-class members via `$this->`, `self::`, `static::`, or `parent::` inside a trait method no longer produces "not found" warnings. Traits are incomplete by nature and expect the host class to provide these members.
 - **Type narrowing inside `return` statements.** `instanceof` checks in `&&` chains and ternary conditions now narrow the variable type when the expression is the operand of a `return` statement. Previously, `return $e instanceof QueryException && $e->errorInfo;` would flag `errorInfo` as unknown because narrowing only applied inside standalone expression statements and `if` conditions.

docs/todo.md

@@ -101,11 +101,8 @@ unlikely to move the needle for most users.
 | #   | Item                                                                                                                                                         | Impact      | Effort      |
 | --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------- | ----------- |
 |     | **[Bugs](todo/bugs.md)**                                                                                                                                     |             |             |
-| B1  | [Nullable type prefix not stripped during diagnostic class lookup](todo/bugs.md#b1-nullable-type-prefix-not-stripped-during-diagnostic-class-lookup)           | Medium-High | Low         |
-| B2  | [Generic type parameters not stripped during diagnostic class lookup](todo/bugs.md#b2-generic-type-parameters-not-stripped-during-diagnostic-class-lookup)     | Medium      | Low         |
 | B3  | [Trait static/self suppression not applied inside closures](todo/bugs.md#b3-trait-staticself-suppression-not-applied-inside-closures)                         | Medium      | Low         |
 | B4  | [Variable reassignment loses type when parameter name is reused](todo/bugs.md#b4-variable-reassignment-loses-type-when-parameter-name-is-reused)             | Medium      | Medium      |
-| B5  | [Docblock `@see` reference prepends file namespace](todo/bugs.md#b5-docblock-see-reference-prepends-file-namespace)                                          | Low         | Low         |
 | B6  | [Empty subject string in diagnostic messages](todo/bugs.md#b6-empty-subject-string-in-diagnostic-messages)                                                   | Low         | Low         |
 | B7  | [Overloaded built-in function signatures in stubs](todo/bugs.md#b7-overloaded-built-in-function-signatures-in-stubs)                                         | Low         | Low         |
 | B8  | [`getCode`/`getMessage` not found through deep inheritance chains](todo/bugs.md#b8-getcodemessage-not-found-through-deep-inheritance-chains)                 | Low         | Low         |

docs/todo/bugs.md

@@ -15,66 +15,6 @@ within the same impact tier.
 
 ---
 
-#### B1. Nullable type not resolved to its base class
-
-| | |
-|---|---|
-| **Impact** | Medium-High |
-| **Effort** | Low |
-
-When a variable's type is `?ClassName`, the type engine fails to
-resolve it to `ClassName`. The `?` prefix is not stripped before class
-lookup, so the engine treats `?Foo` as an unknown type even though
-`Foo` is a valid, loadable class. This breaks completion, hover, and
-go-to-definition for any variable whose type includes the nullable
-shorthand.
-
-The completion pipeline's `type_hint_to_classes` already strips `?`
-(line 90 of `completion/types/resolution.rs`), but other entry points
-into the type engine (e.g. `resolve_variable_assignment_raw_type`,
-`resolve_variable_type_string`) can return `?ClassName` strings that
-are never cleaned before class lookup.
-
-**Observed:** 3 cases in `shared` for
-`?Luxplus\Core\Database\Model\Subscriptions\Subscription`. The class
-exists and loads fine as `Subscription`.
-
-**Fix:** Ensure all paths that convert a raw type string to a
-`ClassInfo` strip the nullable prefix before class lookup. This may
-mean normalising the return value of the raw type resolvers, or
-ensuring every consumer calls `strip_nullable` / `clean_type` before
-passing type strings to `class_loader`.
-
----
-
-#### B2. Generic type parameters prevent class resolution
-
-| | |
-|---|---|
-| **Impact** | Medium |
-| **Effort** | Low |
-
-When a resolved type string includes generic parameters (e.g.
-`PaymentOptionLocaleCollection<PaymentOptionLocale>`), the type engine
-uses the full parameterised string as the class lookup key. The lookup
-fails because no class is registered under the name that includes
-`<...>`. This breaks completion and hover for any variable whose
-resolved type carries generic arguments.
-
-**Observed:** 3 cases in `shared` for
-`PaymentOptionLocaleCollection<Luxplus\Core\Database\Model\Payments\PaymentOptionLocale>`.
-Methods like `getTotalWeight()`, `isNotEmpty()`, and `first()` all
-exist on the class or its parent `Collection`, but the type engine
-never reaches them.
-
-**Fix:** Strip everything from the first `<` onward before performing
-class lookup. The same raw-type-to-class conversion paths identified
-in B1 are affected. `base_class_name()` in `type_strings.rs` already
-combines `clean_type` + `strip_generics` and should be used
-consistently.
-
----
-
 #### B3. Type engine does not resolve `$this`/`static` inside traits
 
 | | |
@@ -139,28 +79,6 @@ within the same scope at the cursor offset.
 
 ---
 
-#### B5. Docblock `@see` reference prepends file namespace
-
-| | |
-|---|---|
-| **Impact** | Low |
-| **Effort** | Low |
-
-When a docblock contains `@see Fully\Qualified\ClassName`, PHPantom
-prepends the current file's namespace to the reference, producing an
-invalid doubled namespace like
-`Luxplus\Core\Database\Model\Products\Filters\Luxplus\Core\Elasticsearch\Queries\ProductQuery`.
-
-**Observed:** 1 diagnostic in `ProductFilterTermCollection.php` where
-`@see Luxplus\Core\Elasticsearch\Queries\ProductQuery::search_with_filter()`
-becomes an unknown class with a doubled namespace prefix.
-
-**Fix:** Treat `@see` references the same as `use` imports: if the
-reference is already fully qualified (starts with the root namespace or
-matches a known class), do not prepend the file namespace.
-
----
-
 #### B6. Empty subject string in type resolution
 
 | | |

src/docblock/type_strings.rs

@@ -404,7 +404,8 @@ pub fn clean_type(raw: &str) -> String {
 /// - `"Foo|null"` → `"Foo"`
 pub fn base_class_name(raw: &str) -> String {
     let cleaned = clean_type(raw);
-    strip_generics(&cleaned)
+    let stripped = strip_nullable(&cleaned);
+    strip_generics(stripped)
 }
 
 /// Strip generic parameters and array shape braces from a (already

src/resolution.rs

@@ -39,6 +39,7 @@ use std::path::Path;
 
 use crate::Backend;
 use crate::composer;
+use crate::docblock::type_strings::{strip_generics, strip_nullable};
 use crate::types::{ClassInfo, FileContext, FunctionInfo, PhpVersion};
 use crate::util::short_name;
 
@@ -53,6 +54,18 @@ impl Backend {
     ///
     /// Returns a shared `Arc<ClassInfo>` if found, or `None`.
     pub(crate) fn find_or_load_class(&self, class_name: &str) -> Option<Arc<ClassInfo>> {
+        // Defensively strip nullable prefix (`?Foo` → `Foo`) and generic
+        // parameters (`Collection<int, User>` → `Collection`) so that
+        // callers don't need to normalise before lookup.
+        let class_name = strip_nullable(class_name);
+        let owned_name;
+        let class_name = if class_name.contains('<') || class_name.contains('{') {
+            owned_name = strip_generics(class_name);
+            owned_name.as_str()
+        } else {
+            class_name
+        };
+
         // The class name stored in ClassInfo is just the short name (e.g. "Customer"),
         // so we match against the last segment of the namespace-qualified name.
         let last_segment = short_name(class_name);

src/symbol_map/docblock.rs

@@ -1190,6 +1190,20 @@ fn emit_see_reference(reference: &str, file_offset: u32, spans: &mut Vec<SymbolS
     // Strip trailing `()` if present (used on both methods and functions).
     let reference = reference.strip_suffix("()").unwrap_or(reference);
 
+    // `@see` references that contain `\` are almost always fully-qualified
+    // class names (e.g. `@see App\Models\User`).  Without a leading `\`,
+    // `class_ref_span` would set `is_fqn = false`, causing downstream
+    // consumers to prepend the current file's namespace and produce a
+    // doubled name like `App\Models\App\Models\User`.  Treat any
+    // backslash-containing reference as FQN by prepending `\`.
+    let owned_reference;
+    let reference = if reference.contains('\\') && !reference.starts_with('\\') {
+        owned_reference = format!("\\{reference}");
+        &owned_reference
+    } else {
+        reference
+    };
+
     // Check for `Class::member` form.
     if let Some(sep_pos) = reference.find("::") {
         let class_part = &reference[..sep_pos];

tests/completion_variables.rs

@@ -16888,3 +16888,207 @@ async fn test_completion_nullsafe_method_chain_cross_file() {
         _ => panic!("Expected CompletionResponse::Array"),
     }
 }
+
+// ─── Nullable type resolution (B1) ──────────────────────────────────────────
+
+/// A parameter typed `?ClassName` should resolve to `ClassName` for completion.
+#[tokio::test]
+async fn test_completion_nullable_param_type_hint() {
+    let backend = create_test_backend();
+
+    let uri = Url::parse("file:///nullable_param.php").unwrap();
+    let text = concat!(
+        "<?php\n",
+        "class Invoice {\n",
+        "    public function total(): int { return 0; }\n",
+        "    public function isPaid(): bool { return false; }\n",
+        "}\n",
+        "class Processor {\n",
+        "    public function handle(?Invoice $inv): void {\n",
+        "        $inv->\n",
+        "    }\n",
+        "}\n",
+    );
+
+    let open_params = DidOpenTextDocumentParams {
+        text_document: TextDocumentItem {
+            uri: uri.clone(),
+            language_id: "php".to_string(),
+            version: 1,
+            text: text.to_string(),
+        },
+    };
+    backend.did_open(open_params).await;
+
+    let completion_params = CompletionParams {
+        text_document_position: TextDocumentPositionParams {
+            text_document: TextDocumentIdentifier { uri },
+            position: Position {
+                line: 7,
+                character: 14,
+            },
+        },
+        work_done_progress_params: WorkDoneProgressParams::default(),
+        partial_result_params: PartialResultParams::default(),
+        context: None,
+    };
+
+    let result = backend.completion(completion_params).await.unwrap();
+    assert!(
+        result.is_some(),
+        "Completion should return results for ?Invoice parameter"
+    );
+
+    match result.unwrap() {
+        CompletionResponse::Array(items) => {
+            let names: Vec<&str> = items
+                .iter()
+                .filter(|i| i.kind == Some(CompletionItemKind::METHOD))
+                .map(|i| i.filter_text.as_deref().unwrap())
+                .collect();
+            assert!(
+                names.contains(&"total"),
+                "Should include 'total' from Invoice via ?Invoice param, got: {:?}",
+                names
+            );
+            assert!(
+                names.contains(&"isPaid"),
+                "Should include 'isPaid' from Invoice via ?Invoice param, got: {:?}",
+                names
+            );
+        }
+        _ => panic!("Expected CompletionResponse::Array"),
+    }
+}
+
+/// A `@var ?ClassName` docblock annotation should resolve for completion.
+#[tokio::test]
+async fn test_completion_nullable_var_docblock() {
+    let backend = create_test_backend();
+
+    let uri = Url::parse("file:///nullable_var.php").unwrap();
+    let text = concat!(
+        "<?php\n",
+        "class Widget {\n",
+        "    public function render(): string { return ''; }\n",
+        "}\n",
+        "class Page {\n",
+        "    public function show(): void {\n",
+        "        /** @var ?Widget $w */\n",
+        "        $w = getWidget();\n",
+        "        $w->\n",
+        "    }\n",
+        "}\n",
+    );
+
+    let open_params = DidOpenTextDocumentParams {
+        text_document: TextDocumentItem {
+            uri: uri.clone(),
+            language_id: "php".to_string(),
+            version: 1,
+            text: text.to_string(),
+        },
+    };
+    backend.did_open(open_params).await;
+
+    let completion_params = CompletionParams {
+        text_document_position: TextDocumentPositionParams {
+            text_document: TextDocumentIdentifier { uri },
+            position: Position {
+                line: 8,
+                character: 12,
+            },
+        },
+        work_done_progress_params: WorkDoneProgressParams::default(),
+        partial_result_params: PartialResultParams::default(),
+        context: None,
+    };
+
+    let result = backend.completion(completion_params).await.unwrap();
+    assert!(
+        result.is_some(),
+        "Completion should return results for @var ?Widget"
+    );
+
+    match result.unwrap() {
+        CompletionResponse::Array(items) => {
+            let names: Vec<&str> = items
+                .iter()
+                .filter(|i| i.kind == Some(CompletionItemKind::METHOD))
+                .map(|i| i.filter_text.as_deref().unwrap())
+                .collect();
+            assert!(
+                names.contains(&"render"),
+                "Should include 'render' from Widget via @var ?Widget, got: {:?}",
+                names
+            );
+        }
+        _ => panic!("Expected CompletionResponse::Array"),
+    }
+}
+
+/// A nullable return type `?ClassName` should resolve when chaining.
+#[tokio::test]
+async fn test_completion_nullable_return_type_chain() {
+    let backend = create_test_backend();
+
+    let uri = Url::parse("file:///nullable_return.php").unwrap();
+    let text = concat!(
+        "<?php\n",
+        "class Order {\n",
+        "    public function total(): int { return 0; }\n",
+        "}\n",
+        "class OrderService {\n",
+        "    public function find(int $id): ?Order { return null; }\n",
+        "    public function test(): void {\n",
+        "        $order = $this->find(1);\n",
+        "        $order->\n",
+        "    }\n",
+        "}\n",
+    );
+
+    let open_params = DidOpenTextDocumentParams {
+        text_document: TextDocumentItem {
+            uri: uri.clone(),
+            language_id: "php".to_string(),
+            version: 1,
+            text: text.to_string(),
+        },
+    };
+    backend.did_open(open_params).await;
+
+    let completion_params = CompletionParams {
+        text_document_position: TextDocumentPositionParams {
+            text_document: TextDocumentIdentifier { uri },
+            position: Position {
+                line: 8,
+                character: 16,
+            },
+        },
+        work_done_progress_params: WorkDoneProgressParams::default(),
+        partial_result_params: PartialResultParams::default(),
+        context: None,
+    };
+
+    let result = backend.completion(completion_params).await.unwrap();
+    assert!(
+        result.is_some(),
+        "Completion should return results for nullable return type ?Order"
+    );
+
+    match result.unwrap() {
+        CompletionResponse::Array(items) => {
+            let names: Vec<&str> = items
+                .iter()
+                .filter(|i| i.kind == Some(CompletionItemKind::METHOD))
+                .map(|i| i.filter_text.as_deref().unwrap())
+                .collect();
+            assert!(
+                names.contains(&"total"),
+                "Should include 'total' from Order via ?Order return, got: {:?}",
+                names
+            );
+        }
+        _ => panic!("Expected CompletionResponse::Array"),
+    }
+}