Argument-count and argument-type diagnostics no longer mix up calls that

share the same text but resolve differently

Author: AJenbo

docs/BUILDING.md

@@ -69,7 +69,12 @@ php -l examples/laravel/app/Demo.php
 phpantom_lsp analyze --project-root examples/laravel --no-colour
 ```
 
-All eight must pass with zero warnings and zero failures.
+All eight must pass with zero warnings and zero failures, except the
+final `analyze` run: `app/Demo.php` intentionally calls
+`Artisan::call('does:not-exist')` to demonstrate the
+`invalid_laravel_command` diagnostic, so it must report exactly
+`[ERROR] Found 1 error` on that line. Any other count, or an error on
+a different line, is a regression.
 
 ### Manual LSP Testing
 

docs/CHANGELOG.md

@@ -120,6 +120,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Calls to functions declared in another `namespace` block of the same file resolve their return type.** In a file that declares more than one `namespace`, a call to a function from a later block used to leave the returned value untyped unless the function carried an `@return` docblock. The call and its return type now resolve regardless, so completion, hover, and diagnostics see the value's type.
 - **`@template` bindings resolve correctly when a call uses named arguments.** A generic function or method called with named arguments (for example `process(class: Product::class, flag: true, function ($p) { ... })`) previously bound its template parameter from the wrong argument whenever the named arguments were out of declaration order, leaving closure parameters untyped. Named arguments now route to the parameter they actually target.
 - **Vendor-provided functions and constants no longer rank as project-native in completion.** With the `self`, `full`, and `composer` indexing strategies, functions and constants discovered while scanning vendor packages lost their package origin when merged into the workspace scan, so they sorted ahead of other vendor symbols as if declared in the project itself instead of by their actual dependency tier.
+- **Argument-count and argument-type diagnostics no longer mix up calls that share the same text but resolve differently.** A per-file cache reused the first resolved target for every call site with the same expression text, without accounting for the fact that `self::method()`, `static::method()`, and `parent::method()` resolve differently depending on which class they appear in, and `$var->method()` resolves differently depending on what type `$var` holds at that call site. Two classes each declaring their own `self::make()` with a different required argument count, or two methods each assigning a different type to a same-named variable before calling the same method name on it, could silently report the wrong argument-count or argument-type diagnostic (or miss one) on every call after the first. These forms are now always resolved fresh per call site.
 
 ## [0.9.0] - 2026-07-20
 

docs/CONTRIBUTING.md

@@ -22,7 +22,7 @@ php -l examples/laravel/app/Demo.php
 phpantom_lsp analyze --project-root examples/laravel --no-colour
 ```
 
-Note that clippy runs twice, once for library code and once including test code. The `php -l` check ensures `examples/demo.php` remains valid PHP. The `php -d zend.assertions=1` run executes `runDemoAssertions()` to verify that scaffolding stubs actually return what their docblocks claim. The final `php -l` and `phpantom_lsp analyze` runs check `examples/laravel/` for syntax errors and diagnostic regressions. The analyze run must report `[OK] No errors`.
+Note that clippy runs twice, once for library code and once including test code. The `php -l` check ensures `examples/demo.php` remains valid PHP. The `php -d zend.assertions=1` run executes `runDemoAssertions()` to verify that scaffolding stubs actually return what their docblocks claim. The final `php -l` and `phpantom_lsp analyze` runs check `examples/laravel/` for syntax errors and diagnostic regressions. `app/Demo.php` intentionally calls `Artisan::call('does:not-exist')` to demonstrate the `invalid_laravel_command` diagnostic, so the analyze run must report exactly `[ERROR] Found 1 error` on that line, not `[OK] No errors`; any other count, or an error on a different line, is a regression.
 
 ## Code Style
 

docs/todo.md

@@ -107,9 +107,7 @@ unlikely to move the needle for most users.
 | D3  | [Deprecated rendering — chain subject resolution](todo/diagnostics.md#d3-deprecated-rendering-chain-subject-resolution)                                                    | Low-Medium  | Medium      |
 | D5  | [External tool diagnostic suppression actions](todo/diagnostics.md#d5-external-tool-diagnostic-suppression-actions)                                                         | Low         | Low         |
 | D15 | [Unused parameter diagnostic](todo/diagnostics.md#d15-unused-parameter-diagnostic)                                                                                          | Low         | Low         |
-|     | **[Bug Fixes](todo/bugs.md)**                                                                                                                                               |             |             |
-| B117 | [Per-file callable-target caches are keyed by call-expression text alone](todo/bugs.md#b117-per-file-callable-target-caches-are-keyed-by-call-expression-text-alone)         | Medium      | Low-Medium  |
-| B118 | [`analyze` on `examples/laravel` reports one error, but CI requires none](todo/bugs.md#b118-phpantom_lsp-analyze-on-exampleslaravel-reports-one-error-but-ci-requires-none)  | Low         | Low         |
+|     | **[Bug Fixes](todo/bugs.md)** — no outstanding items                                                                                                                        |             |             |
 |     | **[Code Actions](todo/actions.md)**                                                                                                                                         |             |             |
 | A40 | [Generate method from call](todo/actions.md#a40-generate-method-from-call)                                                                                                  | Medium-High | Medium      |
 | A41 | [Create class from non-existing name](todo/actions.md#a41-create-class-from-non-existing-name)                                                                              | Medium      | Medium      |

docs/todo/bugs.md

@@ -7,58 +7,5 @@ pipeline so it produces correct data. Downstream consumers
 (diagnostics, hover, completion, definition) should never need
 to second-guess upstream output.
 
-## B117. Per-file callable-target caches are keyed by call-expression text alone
+No outstanding items.
 
-**Impact: Medium · Effort: Low-Medium**
-
-Both argument diagnostic passes memoize `resolve_callable_target`
-in a per-file `HashMap<String, Option<ResolvedCallableTarget>>`
-keyed only on the call expression, then resolve the entry using
-whichever call site happened to be reached first. Callable-target
-resolution is position-dependent (it picks the enclosing class via
-`find_class_at_offset` and resolves receiver variables against the
-scope at the cursor), so any expression whose meaning varies within
-the file gets the wrong target at every site after the first:
-
-- `argument_count.rs` caches *every* expression, including
-  variable-based calls. Two methods that both call
-  `$parser->parse(…)` on different types share one cached
-  signature, so the argument count is checked against the wrong
-  method.
-- `type_errors/mod.rs` deliberately skips variable-based calls
-  (`expr.starts_with('$')`), but `self::`, `static::`, and
-  `parent::` do not start with `$` and are equally
-  position-dependent. In a file declaring two classes that each
-  have a zero-argument `self::make()`, the second class resolves
-  to the first class's method.
-
-Both are latent today because the wrong target usually has a
-compatible signature, and a wrong resolution can produce either a
-false positive or a silently missed diagnostic.
-
-The cache key must cover what resolution actually depends on.
-Restricting the cache to position-independent expressions (plain
-function calls and `Fqn::method` where the class part is not
-`self`/`static`/`parent`) is the smallest sound fix; keying on
-`(expr, enclosing_class_start_offset)` keeps more hits for the
-keyword forms. Note that per-site resolution is no longer
-expensive now that the offset round trip is gone, so dropping the
-unsound entries costs little.
-
----
-
-## B118. `phpantom_lsp analyze` on `examples/laravel` reports one error, but CI requires none
-
-**Impact: Low · Effort: Low**
-
-`docs/CONTRIBUTING.md` lists
-`phpantom_lsp analyze --project-root examples/laravel --no-colour`
-as a required check and states it "must report `[OK] No errors`",
-but `examples/laravel/app/Demo.php` intentionally contains
-`Artisan::call('does:not-exist')` to demonstrate the
-`invalid_laravel_command` diagnostic, so the command reports
-`Found 1 error`. Either the demo needs a form that shows the
-diagnostic without failing the check (the way `examples/demo.php`
-keeps its intentional diagnostics out of the analyze gate), or the
-documented expectation needs to name the diagnostics the Laravel
-example is expected to produce.

src/diagnostics/argument_count.rs

@@ -30,7 +30,7 @@ use crate::Backend;
 use crate::parser::with_parse_cache;
 use crate::types::ResolvedCallableTarget;
 
-use super::helpers::make_diagnostic;
+use super::helpers::{is_position_independent_call_expression, make_diagnostic};
 
 /// Diagnostic code used for argument-count diagnostics.
 pub(crate) const ARGUMENT_COUNT_MISMATCH_CODE: &str = "argument_count_mismatch";
@@ -246,8 +246,15 @@ impl Backend {
         let _parse_guard = with_parse_cache(content);
 
         // Call-expression resolution cache: avoids re-resolving the
-        // same call expression (e.g. `$purchaseFile->save`) at every
-        // call site that uses it.
+        // same call expression (e.g. `Fqn::method`) at every call site
+        // that uses it.
+        //
+        // Only expressions that are guaranteed to resolve to the same
+        // target everywhere in the file are cached (see
+        // `is_position_independent_call_expression`). Variable-based
+        // calls (`$purchaseFile->save`) and `self::`/`static::`/
+        // `parent::` calls are resolved fresh every time because their
+        // target depends on the call site's position.
         let mut call_cache: HashMap<String, Option<ResolvedCallableTarget>> = HashMap::new();
 
         // ── Walk every call site ────────────────────────────────────
@@ -261,17 +268,26 @@ impl Backend {
             let expr = &call_site.call_expression;
 
             // Look up or populate the call expression cache.
-            let resolved = call_cache
-                .entry(expr.clone())
-                .or_insert_with(|| {
-                    self.resolve_callable_target_at_offset(
-                        expr,
-                        content,
-                        call_site.args_start,
-                        &file_ctx,
-                    )
-                })
-                .clone();
+            let resolved = if is_position_independent_call_expression(expr) {
+                call_cache
+                    .entry(expr.clone())
+                    .or_insert_with(|| {
+                        self.resolve_callable_target_at_offset(
+                            expr,
+                            content,
+                            call_site.args_start,
+                            &file_ctx,
+                        )
+                    })
+                    .clone()
+            } else {
+                self.resolve_callable_target_at_offset(
+                    expr,
+                    content,
+                    call_site.args_start,
+                    &file_ctx,
+                )
+            };
 
             // Resolve the call expression to a callable target.
             let resolved = match resolved {

src/diagnostics/helpers.rs

@@ -774,6 +774,33 @@ pub(crate) fn find_innermost_enclosing_class(
         .map(|(c, _)| c.as_ref())
 }
 
+/// Returns `true` when a call expression's `resolve_callable_target*`
+/// result is guaranteed to be the same at every call site in a file, so
+/// it is safe to memoize by expression text alone in a per-file cache.
+///
+/// Excludes:
+/// - Variable-based calls (`$subject->method`) — the receiver
+///   variable's type comes from the assignments visible at the cursor
+///   and can differ between call sites that share the same text (e.g.
+///   two methods that each assign a different type to `$parser`).
+/// - `self::`, `static::`, `parent::`, `new self`, `new static`, and
+///   `new parent` — resolved via the enclosing class at the cursor
+///   offset (`find_class_at_offset`), which differs between call sites
+///   in different classes within the same file.
+///
+/// Plain function calls and calls through a literal class name
+/// (`Fqn::method`, `new Fqn`) resolve identically regardless of where
+/// in the file they appear, so those remain safe to cache by text.
+pub(crate) fn is_position_independent_call_expression(expr: &str) -> bool {
+    if expr.starts_with('$') {
+        return false;
+    }
+    if expr.starts_with("self::") || expr.starts_with("static::") || expr.starts_with("parent::") {
+        return false;
+    }
+    !matches!(expr, "new self" | "new static" | "new parent")
+}
+
 /// Build a standard diagnostic with the common fields pre-filled.
 ///
 /// Most diagnostic collectors build `Diagnostic` values with `source`

src/diagnostics/type_errors/mod.rs

@@ -33,7 +33,9 @@ use crate::type_engine::resolver::{Loaders, VarResolutionCtx};
 use crate::type_engine::variable::foreach_resolution::resolve_expression_type;
 use crate::types::{ClassInfo, ResolvedCallableTarget};
 
-use super::helpers::{find_innermost_enclosing_class, make_diagnostic};
+use super::helpers::{
+    find_innermost_enclosing_class, is_position_independent_call_expression, make_diagnostic,
+};
 
 /// Diagnostic code used for argument type mismatch diagnostics.
 pub(crate) const TYPE_MISMATCH_ARGUMENT_CODE: &str = "type_mismatch_argument";
@@ -487,12 +489,15 @@ impl Backend {
         // call site that uses it.
         //
         // Only expressions that are guaranteed to resolve to the same
-        // target everywhere in the file are cached.  Variable-based
+        // target everywhere in the file are cached (see
+        // `is_position_independent_call_expression`).  Variable-based
         // calls (`$listener->handle`, `$repo->save`) are NOT cached
         // because the same variable name can hold different types in
-        // different methods or after reassignment.  Static calls
-        // (`Foo::bar`) and plain function calls (`array_map`) are
-        // safe to cache.
+        // different methods or after reassignment, and neither are
+        // `self::`/`static::`/`parent::` calls, whose target depends on
+        // the enclosing class at the call site.  Calls through a
+        // literal class name (`Foo::bar`) and plain function calls
+        // (`array_map`) are safe to cache.
         let mut call_cache: HashMap<String, Option<ResolvedCallableTarget>> = HashMap::new();
 
         // ── Walk every call site ────────────────────────────────────
@@ -506,11 +511,11 @@ impl Backend {
             let expr = &call_site.call_expression;
 
             // Look up or populate the call expression cache.
-            // Variable-based calls are resolved fresh every time
-            // because the receiver variable may hold different types
-            // at different call sites (e.g. `$listener->handle` in
-            // two different test methods with different assignments).
-            let is_variable_call = expr.starts_with('$');
+            // Variable-based and `self::`/`static::`/`parent::` calls
+            // are resolved fresh every time because their target
+            // depends on the call site's position (the receiver
+            // variable's assigned type, or the enclosing class).
+            let is_position_dependent_call = !is_position_independent_call_expression(expr);
 
             // Extract the raw argument text from the source so that
             // method-level @template parameters can be resolved from
@@ -536,13 +541,13 @@ impl Backend {
             // Method-level template substitution uses the per-site
             // argument text extracted above.
             //
-            // Variable-based calls are always resolved per-site
-            // because the receiver variable may hold different types
-            // at different call sites.  Static/function calls are
-            // also resolved per-site when argument text is available
-            // (method-level template subs depend on per-site args);
-            // only zero-arg calls can be cached.
-            let resolved = if is_variable_call || call_args_text.is_some() {
+            // Position-dependent calls are always resolved per-site
+            // because their target may differ at different call sites.
+            // Position-independent calls are also resolved per-site
+            // when argument text is available (method-level template
+            // subs depend on per-site args); only zero-arg calls can
+            // be cached.
+            let resolved = if is_position_dependent_call || call_args_text.is_some() {
                 self.resolve_callable_target_with_args_at_offset(
                     expr,
                     content,

tests/integration/diagnostics_argument_count.rs

@@ -962,4 +962,69 @@ function test(): void {
             "Expected both $a and $b reported missing, got: {diags:?}",
         );
     }
+
+    #[test]
+    fn variable_call_target_not_shared_across_different_receiver_types() {
+        // Two unrelated classes each expose a `parse` method with a
+        // different required arity, and two functions each assign a
+        // same-named local variable a different one of those classes.
+        // Caching the resolved target by the expression text
+        // `$parser->parse` alone (ignoring what type `$parser` holds at
+        // each call site) would make the second call reuse the first's
+        // resolved target and miss its own too-few-args error.
+        let php = r#"<?php
+class ParserA {
+    public function parse(int $a): void {}
+}
+class ParserB {
+    public function parse(int $a, int $b): void {}
+}
+function testA(): void {
+    $parser = new ParserA();
+    $parser->parse(1);
+}
+function testB(): void {
+    $parser = new ParserB();
+    $parser->parse(1);
+}
+"#;
+        let diags = collect(php);
+        assert!(
+            diags
+                .iter()
+                .any(|d| d.message.contains("Expected 2 arguments") && d.message.contains("got 1")),
+            "Expected too-few-args diagnostic for ParserB::parse via $parser, got: {diags:?}",
+        );
+    }
+
+    #[test]
+    fn self_call_target_not_shared_across_classes() {
+        // Two classes each declare a zero-arg-looking `self::make` call
+        // with a different required arity. `self::make` resolution
+        // depends on the enclosing class at the call site, so caching
+        // the target by expression text alone (ignoring which class the
+        // call appears in) would make the second class reuse the
+        // first's resolved target and miss its own too-few-args error.
+        let php = r#"<?php
+class First {
+    public static function make(int $a): void {}
+    public function run(): void {
+        self::make(1);
+    }
+}
+class Second {
+    public static function make(int $a, int $b): void {}
+    public function run(): void {
+        self::make(1);
+    }
+}
+"#;
+        let diags = collect(php);
+        assert!(
+            diags
+                .iter()
+                .any(|d| d.message.contains("Expected 2 arguments") && d.message.contains("got 1")),
+            "Expected too-few-args diagnostic for Second::make via self::, got: {diags:?}",
+        );
+    }
 }