A plain function's @return docblock is checked against its body

Author: AJenbo

docs/CHANGELOG.md

@@ -124,6 +124,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - **The type engine itself now knows that a class is an `object` and that a `Traversable` is `iterable`.** Those facts, together with the equivalence between the ways one array type can be written (`array<int, Cat>`, `list<Cat>`, and `Cat[]` all describe the same values), used to be reachable only from the argument-type check, so everything else reasoned without them: to completion filtering and hover a `Collection<User>` was not an `object` and an `ArrayIterator` was not `iterable`, and an array whose element type only matched through a parent class did not match at all once the two sides were spelled at different arities. They live in the shared subtype check now, so every feature reads the same answer. This does tighten one case: a value that may be `null` no longer satisfies an `object` or `iterable` parameter, matching both what PHP does at runtime and how a nullable value has always been judged against every other parameter type.
 - **A ternary inside a `throw` narrows its branches.** `throw new RuntimeException($model ? get_class($model) : '')` still read `$model` as nullable inside the arm the check proves it is not, so whatever that arm passed on was reported against the wider type. The identical ternary in a `return`, an assignment, or a call argument narrowed correctly. A thrown value is walked like any other expression now.
+- **A plain function's `@return` docblock is checked against its body.** `/** @return array<string, int> */ function bad(): array { return ['a' => 'x']; }` went unreported, and so did every other return that satisfied the native hint but not the docblock behind it. The check read the `array` written in the signature and stopped there, so the type that says what the array actually holds was never compared to anything, while the same mistake passed to a parameter was reported as it should be. The docblock and the hint are now merged the way they are everywhere else, so a function is held to the type it documents rather than the weaker one it declares. Methods were never affected, which is what made the gap look like an array-shape problem rather than a function one.
 - **A generic call keeps the alternatives its argument's return type declares.** `takesCarbon(passthrough(Carbon::create(2024)))` was reported as fine even though `Carbon::create()` returns `?Carbon`. The pass that works out what a `@template` binds to reads its argument as text, and that reading answers with the classes an expression can be, so the `null` arm was dropped and the template bound a plain `Carbon`. Every use of the substituted type then claimed the value could never be null: a missed report where the result is consumed, and an invented one where a parameter is checked against it. Writing the call to a variable first bound the nullable correctly, which is what made the two forms disagree. The alternatives a call's return type declares and the class walk cannot name, `null` and scalars alike, are now put back, so both forms bind the same thing. A call a check has already narrowed keeps the narrowed type rather than having the declared one restored over it.
 - **A one-line function no longer inherits the previous function's `@param`.** `function g3(array $s): void { foreach ($s as $x) { doThing($x); } }` opens and closes its body on the signature line, and the backward scan that looks for an enclosing docblock relied on watching brace depth rise and fall to spot where a sibling function ends; a body written entirely on one line never moves that depth, so the scan walked straight past `g3` into its docblock and handed `g4(Status $s)` the `@param array<Status> $s` written for a different function entirely, reporting a type error against a type the parameter never had. The scan now also recognises a `function` keyword sitting at a depth it has already fully backed out of, which catches a body collapsed onto one line the same as one spread across several.
 - **A guard on a call narrows the same call written again.** `if (currentUser()) { render(currentUser()); }` is the shape a nullable accessor is written for, and it was reported as passing a `?User`. A check on `$holder->get()` already carried to the next `$holder->get()`; a plain function call and a static call did not, because the scope had no entry for either to narrow. Both are now recorded under the call's own text like a method call is, so a repeated `currentUser()` or `Session::current()` inside the guard reads what the guard proved. A call that takes arguments is keyed with them, so checking one call still says nothing about a different one.

docs/todo.md

@@ -38,7 +38,6 @@ contributor even though it's short.
 | B78  | [A standalone `@var` cast above `return` is ignored](todo/bugs.md#b78-a-standalone-var-cast-above-return-is-ignored)                                        | Medium      | Medium      |
 | B82  | [A nullsafe chain compared `===` to a non-nullable value does not narrow the receiver](todo/bugs.md#b82-a-nullsafe-chain-compared--to-a-non-nullable-value-does-not-narrow-the-receiver) | Medium      | Medium      |
 | B83  | [A `match (true)` arm's condition does not narrow inside the arm's result](todo/bugs.md#b83-a-match-true-arms-condition-does-not-narrow-inside-the-arms-result) | Medium      | Medium      |
-| B84  | [Return-position compatibility ignores an array shape's value types](todo/bugs.md#b84-return-position-compatibility-ignores-an-array-shapes-value-types)      | Medium      | Medium      |
 | B79  | [`array_filter()` without a callback keeps `null` on values that share the array with a `?bool`](todo/bugs.md#b79-array_filter-without-a-callback-keeps-null-on-values-that-share-the-array-with-a-bool) | Low-Medium  | Medium      |
 | B81  | [Foreach element extraction widens `false` to `bool`](todo/bugs.md#b81-foreach-element-extraction-widens-false-to-bool)                                     | Low-Medium  | Medium      |
 | B76  | [Blade variables typed from a component class are immune to condition narrowing](todo/bugs.md#b76-blade-variables-typed-from-a-component-class-are-immune-to-condition-narrowing) | Medium      | Medium-High |

docs/todo/bugs.md

@@ -22,9 +22,7 @@ block; several sit at the same source sites as fixed bugs from the
 previous sweep (B50, B54, B59, B62), where the coarse defect was fixed
 and a finer one behind it became visible. **B83** was filed the same
 day from the follow-up re-run at `c618c8aa`, whose compatibility
-tightening surfaced one previously-swallowed site, and **B84** from a
-probe at `66a524bc` showing the return-position side of that
-tightening is still missing.
+tightening surfaced one previously-swallowed site.
 
 ## Crashes
 
@@ -73,45 +71,6 @@ sample projects first: a file that declares `strict_types=1` and
 leans on `__toString()` would start reporting, and those reports are
 correct.
 
-### B84. Return-position compatibility ignores an array shape's value types
-
-**Impact: Medium · Complexity: Medium**
-
-```php
-/** @return array<string, int> */
-function bad(): array {
-    return ['a' => 'x'];   // not reported
-}
-
-/** @param array<string, int> $m */
-function takesIntMap(array $m): void {}
-
-function alsoBad(): void {
-    takesIntMap(['a' => 'x']);   // reported, as expected
-}
-```
-
-Needs investigation: `type_mismatch_argument` correctly reports an
-array shape whose values do not satisfy the declared map or list value
-type (`array{a: 'x'}` vs `array<string, int>`, `array{'x', 'y'}` vs
-`list<int>`), but `type_mismatch_return` accepts the identical
-mismatch silently. The return side is not skipping shapes entirely —
-a nullability mismatch in a shape value (`array{a: ?bool}` vs
-`array<string, int>`) is reported in return position — so the two
-diagnostics are reaching different verdicts for the same shape-vs-map
-comparison somewhere below the nullability check. `array<string,
-never>` as the declared type is the extreme case: any all-optional-keys
-shape (e.g. an `array_filter()` result) passes against it, which is
-what kept masking scratch probes during the 2026-08-15 sweeps.
-
-Found by probe at `66a524bc`, after the argument-side tightening
-landed; no sample-project site currently hits it.
-
-**Fix:** find where the return-position compatibility path diverges
-from the argument-position path for shape-to-map value checks and
-unify them; the recently tightened argument behaviour is the correct
-one.
-
 ## Standard-library return types
 
 ### B80. `max()`/`min()` argument-dependent return type is a malformed union
@@ -372,8 +331,7 @@ both keep their `null` arms.
 Filed 2026-08-15, after the evening sweep: the argument-compatibility
 tightening in `c618c8aa` surfaced it — the resulting shape mismatch
 was previously swallowed by the diagnostic layer's array leniency
-(the argument side has since been tightened further; the return-side
-remainder is B84).
+(the argument side has since been tightened further).
 
 Sample site: `luxplus-website app/Contexts/Api/Resources/ProductResource.php:210`
 (discount label `$textArgs` built from `?int` properties the arm

src/diagnostics/return_type_errors.rs

@@ -182,6 +182,7 @@ impl Backend {
                 for stmt in program.statements.iter() {
                     process_top_level_statement(
                         stmt,
+                        uri,
                         content,
                         &file_ctx,
                         &class_loader,
@@ -375,10 +376,48 @@ fn resolve_return_and_push(
     }
 }
 
+/// The return type the index holds for the function `uri` declares at
+/// `func_offset`, with the `@return` docblock already merged over the native
+/// hint.
+///
+/// Only this file's own declaration will do.  A same-named function in another
+/// file has a docblock that says nothing about the body being checked, so the
+/// index entry is accepted only when it was contributed by `uri`, and a name
+/// another file won the race for is looked up among the runners-up instead.
+fn indexed_return_type(
+    backend: &Backend,
+    file_ctx: &crate::types::FileContext,
+    uri: &str,
+    func_name: &str,
+    func_offset: u32,
+) -> Option<PhpType> {
+    let fqn = file_ctx.resolve_name_at(func_name, func_offset);
+
+    {
+        let fmap = backend.global_functions().read();
+        for name in [fqn.as_str(), func_name] {
+            if let Some((decl_uri, fi)) = fmap.get(name)
+                && decl_uri == uri
+            {
+                return fi.return_type.clone();
+            }
+        }
+    }
+
+    let dups = backend.symbols.duplicate_functions.read();
+    for name in [fqn.as_str(), func_name] {
+        if let Some(fi) = dups.get(name).and_then(|by_uri| by_uri.get(uri)) {
+            return fi.return_type.clone();
+        }
+    }
+    None
+}
+
 #[allow(clippy::too_many_arguments)]
 /// Walk a top-level statement looking for function/class declarations.
 fn process_top_level_statement(
     stmt: &Statement<'_>,
+    uri: &str,
     content: &str,
     file_ctx: &crate::types::FileContext,
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
@@ -392,6 +431,7 @@ fn process_top_level_statement(
             for inner in ns.statements().iter() {
                 process_top_level_statement(
                     inner,
+                    uri,
                     content,
                     file_ctx,
                     class_loader,
@@ -462,28 +502,17 @@ fn process_top_level_statement(
             let func_name = bytes_to_str(func.name.value);
             let func_offset = func.name.span.start.offset;
 
-            // Extract the declared return type.  Prefer the AST's native
-            // return type hint (always available for the current file),
-            // then fall back to the global function index (which may
-            // carry a richer docblock-enriched type).
-            let declared_return = func
-                .return_type_hint
-                .as_ref()
-                .map(|rth| crate::parser::extract_hint_type(&rth.hint))
-                .or_else(|| {
-                    let fqn = file_ctx.resolve_name_at(func_name, func_offset);
-                    backend
-                        .global_functions()
-                        .read()
-                        .get(&fqn)
-                        .and_then(|(_, fi)| fi.return_type.clone())
-                        .or_else(|| {
-                            backend
-                                .global_functions()
-                                .read()
-                                .get(func_name)
-                                .and_then(|(_, fi)| fi.return_type.clone())
-                        })
+            // Extract the declared return type.  Prefer the indexed
+            // `FunctionInfo`, where the parser has already merged the
+            // `@return` docblock over the native hint, so a body is checked
+            // against `array<string, int>` rather than bare `array`.  Falling
+            // back to the AST hint covers a function the index has not caught
+            // up with yet.
+            let declared_return =
+                indexed_return_type(backend, file_ctx, uri, func_name, func_offset).or_else(|| {
+                    func.return_type_hint
+                        .as_ref()
+                        .map(|rth| crate::parser::extract_hint_type(&rth.hint))
                 });
 
             let declared_return = match declared_return {
@@ -556,6 +585,7 @@ fn process_top_level_statement(
                 DeclareBody::Statement(inner) => {
                     process_top_level_statement(
                         inner,
+                        uri,
                         content,
                         file_ctx,
                         class_loader,
@@ -569,6 +599,7 @@ fn process_top_level_statement(
                     for s in body.statements.iter() {
                         process_top_level_statement(
                             s,
+                            uri,
                             content,
                             file_ctx,
                             class_loader,

tests/integration/diagnostics_return_type_errors.rs

@@ -3920,3 +3920,94 @@ function gate(?string $grade): string {
         return_error_messages(&collect(php)).join("; ")
     );
 }
+
+// ── A function's `@return` docblock refines its native `array` hint ──────────
+
+#[test]
+fn flags_shape_value_mismatch_against_docblock_map_on_plain_function() {
+    let php = r#"<?php
+/** @return array<string, int> */
+function bad(): array {
+    return ['a' => 'x'];
+}
+"#;
+    let diags = collect(php);
+    let msgs = return_error_messages(&diags);
+    assert!(
+        msgs.iter().any(|m| m.contains("array<string, int>")),
+        "Expected the docblock map type to be checked, not the bare `array` hint, got: {msgs:?}"
+    );
+}
+
+#[test]
+fn flags_shape_value_mismatch_against_docblock_list_on_plain_function() {
+    let php = r#"<?php
+/** @return list<int> */
+function bad(): array {
+    return ['x', 'y'];
+}
+"#;
+    let diags = collect(php);
+    let msgs = return_error_messages(&diags);
+    assert!(
+        msgs.iter().any(|m| m.contains("list<int>")),
+        "Expected the docblock list type to be checked, got: {msgs:?}"
+    );
+}
+
+#[test]
+fn no_diagnostic_when_shape_satisfies_docblock_map_on_plain_function() {
+    let php = r#"<?php
+/** @return array<string, int> */
+function good(): array {
+    return ['a' => 1];
+}
+
+/** @return list<int> */
+function goodList(): array {
+    return [1, 2];
+}
+
+/** @return array<string, int> */
+function goodEmpty(): array {
+    return [];
+}
+"#;
+    let diags = collect(php);
+    assert!(
+        !has_return_error(&diags),
+        "Expected no return type error, got: {:?}",
+        return_error_messages(&diags)
+    );
+}
+
+#[test]
+fn docblock_return_type_of_another_files_function_is_not_borrowed() {
+    let backend = create_test_backend();
+    let other = "file:///other.php";
+    backend.update_ast(
+        other,
+        r#"<?php
+/** @return array<string, int> */
+function dup(): array {
+    return [];
+}
+"#,
+    );
+
+    let uri = "file:///test.php";
+    let php = r#"<?php
+function dup(): array {
+    return ['a' => 'x'];
+}
+"#;
+    backend.update_ast(uri, php);
+    let mut diags = Vec::new();
+    backend.collect_return_type_diagnostics(uri, php, &mut diags);
+    assert!(
+        !has_return_error(&diags),
+        "This file declares `dup(): array` with no docblock, so the other file's \
+         `@return array<string, int>` must not be checked against this body, got: {:?}",
+        return_error_messages(&diags)
+    );
+}