Fix some consistency issues between tools

Author: AJenbo

docs/todo.md

@@ -99,6 +99,8 @@ unlikely to move the needle for most users.
 
 | #   | Item                                                                                                                                                         | Impact      | Effort      |
 | --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------- | ----------- |
+|     | **[Bug Fixes](todo/bugs.md)**                                                                                                                                |             |             |
+| B1  | [Dual type-resolution engines cause hover / completion divergence](todo/bugs.md#b1-dual-type-resolution-engines-cause-hover--completion-divergence)           | Medium      | Medium-High |
 |     | **[Completion](todo/completion.md)**                                                                                                                         |             |             |
 | C1  | Array functions needing new code paths                                                                                                                       | Medium     | High        |
 | C9  | [Lazy documentation via `completionItem/resolve`](todo/completion.md#c9-lazy-documentation-via-completionitemresolve)                                         | Medium     | Medium      |

docs/todo/bugs.md

@@ -15,6 +15,68 @@ within the same impact tier.
 
 ---
 
+#### B1. Dual type-resolution engines cause hover / completion divergence
+
+| | |
+|---|---|
+| **Impact** | Medium |
+| **Effort** | Medium-High |
+
+Variable type resolution has two parallel RHS expression resolvers that
+must be kept in sync manually:
+
+1. **`resolve_rhs_expression`** in `completion/variable/rhs_resolution.rs`
+   — returns `Vec<ClassInfo>`, used by the completion pipeline.
+2. **`resolve_rhs_raw_type`** in `completion/variable/raw_type_inference.rs`
+   — returns `Option<String>`, used by hover's type-string path
+   (`resolve_variable_type_string` → step 5).
+
+Hover tries the type-string path first.  When it returns `Some(…)` the
+ClassInfo fallback never fires, so hover shows whatever the raw-type
+engine inferred — even if it is wrong.  The completion pipeline uses the
+ClassInfo engine directly and gets the correct answer.
+
+The two resolvers handle **different sets of expression types**:
+
+| Expression kind          | ClassInfo engine | Raw-type engine |
+| ------------------------ | :--------------: | :-------------: |
+| `clone`                  | ✓                | ✗               |
+| `\|>` (pipe)             | ✓                | ✗               |
+| `Closure` / arrow fn     | ✓ (→ `\Closure`) | ✗               |
+| `yield` (send type)      | ✓                | ✗               |
+| `Call` (full resolution) | ✓                | partial¹        |
+| `Access` (property)      | ✓                | partial¹        |
+| Scalar literals          | ✗                | ✓               |
+| Array literal inference  | ✗                | ✓               |
+
+¹ The raw-type engine's `_ =>` catch-all delegates to
+`extract_rhs_iterable_raw_type`, which covers some calls and accesses
+but not all.
+
+The `??` null-coalesce handling also diverges: the ClassInfo engine
+checks whether the LHS *AST node* is syntactically non-nullable (pattern
+match on `Clone`, `Literal`, etc.), while the raw-type engine checks
+whether the resolved *type string* is non-nullable.  When the raw-type
+engine cannot resolve the LHS at all (e.g. `clone $x` → `None`), it
+falls through to returning only the RHS type, which is how
+`clone $pen ?? new Marker()` shows `Marker` on hover but `Pen` on
+completion.
+
+**Possible approaches:**
+
+- **Unify into one engine** — make `resolve_rhs_expression` the single
+  source of truth and derive the type string from its result.  Hover
+  would call the ClassInfo path and format the names into a union
+  string, falling back to `resolve_variable_type_string` only for
+  scalar / generic types that ClassInfo cannot represent.  This
+  eliminates the synchronisation burden entirely.
+- **Exhaustiveness enforcement** — if keeping both engines, add a shared
+  enum of "RHS expression kinds" and a compile-time check (or test)
+  that both match arms cover the same set, so new expression types
+  cannot be added to one without the other.
+
+---
+
 #### B12. Interface-extends-interface constants (and other members) not merged
 
 | | |

src/code_actions/update_docblock.rs

@@ -25,7 +25,7 @@ use crate::Backend;
 use crate::completion::phpdoc::generation::enrichment_plain;
 use crate::completion::source::throws_analysis::{self, ThrowsContext};
 use crate::docblock::is_compatible_refinement;
-use crate::docblock::type_strings::split_type_token;
+use crate::docblock::type_strings::{split_type_token, split_union_depth0};
 use crate::types::{ClassInfo, FunctionLoader};
 use crate::util::offset_to_position;
 
@@ -785,8 +785,8 @@ fn is_type_contradiction(doc_type: &str, native_type: &str) -> bool {
     // If the native type contains `|`, compare the union components.
 
     // Simple heuristic: normalize both and compare base types.
-    let doc_bases = split_union_types(&doc_clean);
-    let native_bases = split_union_types(&native_clean);
+    let doc_bases = split_union_depth0(&doc_clean);
+    let native_bases = split_union_depth0(&native_clean);
 
     // If every native base appears in doc bases (or a refinement thereof),
     // it's not a contradiction.
@@ -812,18 +812,12 @@ fn normalize_type_for_comparison(t: &str) -> String {
         |rest| format!("{}|null", rest.to_lowercase()),
     );
     // Sort union components.
-    let mut parts: Vec<&str> = t.split('|').map(|s| s.trim()).collect();
+    let mut parts: Vec<&str> = split_union_depth0(&t).into_iter().map(|s| s.trim()).collect();
     parts.sort();
     parts.join("|")
 }
 
-/// Split a union type string into its components.
-fn split_union_types(t: &str) -> Vec<String> {
-    t.split('|')
-        .map(|s| s.trim().to_string())
-        .filter(|s| !s.is_empty())
-        .collect()
-}
+
 
 /// Build the updated docblock text.
 fn build_updated_docblock(

src/diagnostics/deprecated.rs

@@ -113,6 +113,7 @@ impl Backend {
                         &file_use_map,
                         &file_namespace,
                         &local_classes,
+                        span.start,
                     )
                     .and_then(|name| self.find_or_load_class(&name))
                     .map(|arc| ClassInfo::clone(&arc));
@@ -334,23 +335,35 @@ fn resolve_subject_to_class_name(
     file_use_map: &HashMap<String, String>,
     file_namespace: &Option<String>,
     local_classes: &[Arc<ClassInfo>],
+    access_offset: u32,
 ) -> Option<String> {
     let trimmed = subject_text.trim();
 
     match trimmed {
         "self" | "static" => {
             // Find the enclosing class in this file
-            find_enclosing_class_fqn(local_classes, file_namespace)
+            find_enclosing_class_fqn(local_classes, file_namespace, access_offset)
         }
         "parent" => {
             // Find the enclosing class that actually has a parent.
-            // Prefer a class with `parent_class` set — that's the one
-            // where `parent::` is meaningful.  Fall back to the first
-            // non-anonymous class if none has a parent (shouldn't happen
-            // in valid code, but be defensive).
+            // Prefer a class whose offset range contains the access site
+            // and that has `parent_class` set — that's the one where
+            // `parent::` is meaningful.  Fall back to any non-anonymous
+            // class with a parent, then to the first non-anonymous class
+            // (shouldn't happen in valid code, but be defensive).
             let cls = local_classes
                 .iter()
-                .find(|c| !c.name.starts_with("__anonymous@") && c.parent_class.is_some())
+                .find(|c| {
+                    !c.name.starts_with("__anonymous@")
+                        && c.parent_class.is_some()
+                        && access_offset >= c.start_offset
+                        && access_offset <= c.end_offset
+                })
+                .or_else(|| {
+                    local_classes
+                        .iter()
+                        .find(|c| !c.name.starts_with("__anonymous@") && c.parent_class.is_some())
+                })
                 .or_else(|| {
                     local_classes
                         .iter()
@@ -362,7 +375,7 @@ fn resolve_subject_to_class_name(
                     .map(|p| resolve_to_fqn(p, file_use_map, file_namespace))
             })
         }
-        "$this" => find_enclosing_class_fqn(local_classes, file_namespace),
+        "$this" => find_enclosing_class_fqn(local_classes, file_namespace, access_offset),
         _ if is_static && !trimmed.starts_with('$') => {
             // Static access on a class name: `ClassName::method()`
             Some(resolve_to_fqn(trimmed, file_use_map, file_namespace))
@@ -425,11 +438,23 @@ fn resolve_variable_subject(
 fn find_enclosing_class_fqn(
     local_classes: &[Arc<ClassInfo>],
     file_namespace: &Option<String>,
+    offset: u32,
 ) -> Option<String> {
-    // Skip anonymous classes
+    // Find the non-anonymous class whose byte range contains the offset.
+    // Fall back to the first non-anonymous class for top-level code
+    // outside any class body.
     let cls = local_classes
         .iter()
-        .find(|c| !c.name.starts_with("__anonymous@"))?;
+        .find(|c| {
+            !c.name.starts_with("__anonymous@")
+                && offset >= c.start_offset
+                && offset <= c.end_offset
+        })
+        .or_else(|| {
+            local_classes
+                .iter()
+                .find(|c| !c.name.starts_with("__anonymous@"))
+        })?;
     if let Some(ns) = file_namespace {
         Some(format!("{}\\{}", ns, cls.name))
     } else {