Make diagnostics for extra paramters configurable

Author: AJenbo

docs/CHANGELOG.md

@@ -22,7 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Document Links.** `require`/`include` paths are now Ctrl+Clickable. Path resolution supports string literals, `__DIR__` concatenation, `dirname(__DIR__)`, `dirname(__FILE__)`, and nested `dirname` with levels.
 - **Syntax error diagnostic.** Parse errors from the Mago parser now appear as Error-severity diagnostics instantly as you type.
 - **Implementation error diagnostic.** Concrete classes that fail to implement all required methods from their interfaces or abstract parents are now flagged with an Error-severity diagnostic on the class name. The existing "Implement missing methods" quick-fix appears inline alongside the error. Cyclic hierarchies are handled gracefully.
-- **Argument count diagnostic.** Flags function and method calls that pass too few or too many arguments. Variadic parameters and argument unpacking are handled correctly.
+- **Argument count diagnostic.** Flags function and method calls that pass too few arguments. The "too many arguments" check is off by default (PHP silently ignores extra arguments) and can be enabled with `extra-arguments = true` in the `[diagnostics]` section of `.phpantom.toml`. Variadic parameters and argument unpacking are handled correctly.
 - **Change visibility.** Code action on any method, property, constant, or promoted constructor parameter offers to change its visibility (`public`, `protected`, `private`).
 - **Update docblock.** Code action on a function or method whose existing docblock is out of sync with its signature. Adds missing `@param` tags, removes stale ones, reorders to match the signature, fixes contradicted types, and removes redundant `@return void`. Refinement types and unrelated tags are preserved. Handles `@param $name` (no type) correctly.
 - **PHPDoc block generation.** Typing `/**` above any declaration generates a docblock skeleton. Tags are only emitted when the native type hint needs enrichment. Properties and constants always get `@var`. Class-likes with templated parents or interfaces get `@extends`/`@implements` tags. Uncaught exceptions get `@throws` with auto-import. Works both via completion and on-type formatting.

docs/todo.md

@@ -23,7 +23,6 @@ within the same impact tier.
 
 | #   | Item                                                                                                                                                           | Impact | Effort |
 | --- | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ------ |
-| B13 | [Argument count: too many arguments off by default](todo/bugs.md#b13-argument-count-diagnostic-flags-too-many-arguments-by-default)                            | High   | Low    |
 | E0  | [Switch embedded stubs to master + LanguageLevelTypeAware](todo/external-stubs.md#e0-switch-embedded-stubs-to-master-and-apply-languageleveltypeaware-patches) | High   | Low    |
 
 ## Sprint 4 — Refactoring toolkit

docs/todo/bugs.md

@@ -55,49 +55,3 @@ In practice this is unlikely because pruning only ever removes entries
 **Fix:** Replace the length comparison with a content comparison, or
 unconditionally write the pruned set back into the cache (the extra
 write is negligible).
-
----
-
-## B13. Argument count diagnostic flags too many arguments by default
-
-**Impact: High · Effort: Low**
-
-The "too many arguments" half of the argument count diagnostic produces
-frequent false positives on real-world PHP codebases. PHP itself does
-not error on extra arguments to user-defined functions — the extras are
-silently ignored. Many libraries (Laravel in particular) exploit this to
-implement flexible APIs: a function declared as `foo(array $items)` is
-commonly called as `foo('a', 'b', 'c')` because the caller is actually
-passing variadic strings and relying on PHP's permissive argument
-handling, or the signature is simply underdocumented.
-
-The "too few arguments" half is genuinely useful — passing too few
-arguments always causes a `TypeError` at runtime — and should remain
-on by default.
-
-### Desired behaviour
-
-- **Too few arguments:** always reported, `Error` severity (current
-  behaviour, keep as-is).
-- **Too many arguments:** off by default, opt-in via
-  `[diagnostics] extra-arguments = true` in `.phpantom.toml`.
-
-### Implementation
-
-- Add an `extra_arguments` field to `DiagnosticsConfig` in
-  `src/config.rs`, defaulting to `false` (same pattern as
-  `unresolved_member_access`).
-- In `src/diagnostics/argument_count.rs`, gate the "too many arguments"
-  block on `self.config().diagnostics.extra_arguments_enabled()`.
-- Add `extra-arguments = true` (commented out) to
-  `DEFAULT_CONFIG_CONTENT` in `src/config.rs` alongside the existing
-  `unresolved-member-access` entry.
-- Update the test helper `collect()` in `argument_count.rs` tests to
-  verify that extra-argument diagnostics are suppressed by default and
-  appear when the config flag is set.
-
-### Files to change
-
-- `src/config.rs` — new field + accessor + default config comment.
-- `src/diagnostics/argument_count.rs` — gate the too-many block.
-- `tests/` — update or add tests asserting the default-off behaviour.

src/config.rs

@@ -51,6 +51,15 @@ pub struct DiagnosticsConfig {
     /// diagnostics on codebases without comprehensive type annotations.
     #[serde(rename = "unresolved-member-access")]
     pub unresolved_member_access: Option<bool>,
+
+    /// Report calls that pass more arguments than the function accepts.
+    ///
+    /// Off by default. PHP does not error on extra arguments to
+    /// user-defined functions (the extras are silently ignored), and
+    /// many libraries exploit this for flexible APIs. Enable this if
+    /// you want stricter checking.
+    #[serde(rename = "extra-arguments")]
+    pub extra_arguments: Option<bool>,
 }
 
 impl DiagnosticsConfig {
@@ -60,6 +69,13 @@ impl DiagnosticsConfig {
     pub fn unresolved_member_access_enabled(&self) -> bool {
         self.unresolved_member_access.unwrap_or(false)
     }
+
+    /// Whether the extra-arguments diagnostic is enabled.
+    ///
+    /// Defaults to `false` (off) when not explicitly set.
+    pub fn extra_arguments_enabled(&self) -> bool {
+        self.extra_arguments.unwrap_or(false)
+    }
 }
 
 /// `[formatting]` section — controls the formatting strategy.
@@ -253,6 +269,9 @@ pub const DEFAULT_CONFIG_CONTENT: &str = r#"# PHPantom project configuration
 # Report member access on subjects whose type could not be resolved.
 # Useful for discovering gaps in type coverage. Off by default.
 # unresolved-member-access = true
+# Report calls that pass more arguments than the function accepts.
+# PHP silently ignores extra arguments, so this is off by default.
+# extra-arguments = true
 
 [indexing]
 # How PHPantom discovers classes across the workspace.
@@ -406,6 +425,7 @@ mod tests {
         let config: Config = toml::from_str(DEFAULT_CONFIG_CONTENT).unwrap();
         assert!(config.php.version.is_none());
         assert!(!config.diagnostics.unresolved_member_access_enabled());
+        assert!(!config.diagnostics.extra_arguments_enabled());
         assert_eq!(config.indexing.strategy, IndexingStrategy::Composer);
         assert!(config.formatting.php_cs_fixer.is_none());
         assert!(config.formatting.phpcbf.is_none());
@@ -423,6 +443,7 @@ mod tests {
         let config = load_config(dir.path()).unwrap();
         assert!(config.php.version.is_none());
         assert!(!config.diagnostics.unresolved_member_access_enabled());
+        assert!(!config.diagnostics.extra_arguments_enabled());
         assert_eq!(config.indexing.strategy, IndexingStrategy::Composer);
         assert!(config.formatting.php_cs_fixer.is_none());
         assert!(config.formatting.phpcbf.is_none());
@@ -437,6 +458,7 @@ mod tests {
         let config = load_config(dir.path()).unwrap();
         assert!(config.php.version.is_none());
         assert!(!config.diagnostics.unresolved_member_access_enabled());
+        assert!(!config.diagnostics.extra_arguments_enabled());
         assert_eq!(config.indexing.strategy, IndexingStrategy::Composer);
         assert!(config.formatting.php_cs_fixer.is_none());
         assert!(config.formatting.phpcbf.is_none());
@@ -470,6 +492,24 @@ mod tests {
         assert!(!config.diagnostics.unresolved_member_access_enabled());
     }
 
+    #[test]
+    fn extra_arguments_defaults_to_false() {
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join(CONFIG_FILE_NAME);
+        std::fs::write(&path, "[diagnostics]\n").unwrap();
+        let config = load_config(dir.path()).unwrap();
+        assert!(!config.diagnostics.extra_arguments_enabled());
+    }
+
+    #[test]
+    fn parses_extra_arguments() {
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join(CONFIG_FILE_NAME);
+        std::fs::write(&path, "[diagnostics]\nextra-arguments = true\n").unwrap();
+        let config = load_config(dir.path()).unwrap();
+        assert!(config.diagnostics.extra_arguments_enabled());
+    }
+
     #[test]
     fn invalid_toml_returns_parse_error() {
         let dir = tempfile::tempdir().unwrap();
@@ -567,6 +607,7 @@ version = "8.2"
 
 [diagnostics]
 unresolved-member-access = true
+extra-arguments = true
 
 [indexing]
 strategy = "self"
@@ -586,6 +627,7 @@ timeout = 30000
         let config = load_config(dir.path()).unwrap();
         assert_eq!(config.php.version.as_deref(), Some("8.2"));
         assert!(config.diagnostics.unresolved_member_access_enabled());
+        assert!(config.diagnostics.extra_arguments_enabled());
         assert_eq!(config.indexing.strategy, IndexingStrategy::SelfScan);
         assert_eq!(config.formatting.php_cs_fixer.as_deref(), Some(""));
         assert_eq!(

src/diagnostics/argument_count.rs

@@ -128,6 +128,10 @@ impl Backend {
             }
 
             // ── Too many arguments ──────────────────────────────────
+            if !self.config().diagnostics.extra_arguments_enabled() {
+                continue;
+            }
+
             if let Some(max) = max_count
                 && actual_args > max
             {
@@ -174,8 +178,16 @@ mod tests {
     use super::*;
     use std::collections::HashMap;
 
+    /// Enable the `extra-arguments` diagnostic on the given backend.
+    fn enable_extra_args(backend: &Backend) {
+        let mut cfg = backend.config.lock().clone();
+        cfg.diagnostics.extra_arguments = Some(true);
+        *backend.config.lock() = cfg;
+    }
+
     /// Helper: create a test backend with minimal function stubs and
-    /// collect argument-count diagnostics.
+    /// collect argument-count diagnostics.  Extra-arguments checking
+    /// is **off** (the default).
     fn collect(php: &str) -> Vec<Diagnostic> {
         let backend = Backend::new_test();
         let uri = "file:///test.php";
@@ -185,10 +197,21 @@ mod tests {
         out
     }
 
-    /// Helper that includes minimal stub functions so that built-in
-    /// functions like `strlen` are resolvable.
-    fn collect_with_stubs(php: &str) -> Vec<Diagnostic> {
-        let stub_fn_index: HashMap<&'static str, &'static str> = HashMap::from([
+    /// Like [`collect`] but with the `extra-arguments` diagnostic
+    /// enabled so that "too many arguments" errors are reported.
+    fn collect_extra(php: &str) -> Vec<Diagnostic> {
+        let backend = Backend::new_test();
+        enable_extra_args(&backend);
+        let uri = "file:///test.php";
+        backend.update_ast(uri, php);
+        let mut out = Vec::new();
+        backend.collect_argument_count_diagnostics(uri, php, &mut out);
+        out
+    }
+
+    /// Minimal stub function index shared by stub-aware helpers.
+    fn stub_fn_index() -> HashMap<&'static str, &'static str> {
+        HashMap::from([
             ("strlen", "<?php\nfunction strlen(string $string): int {}\n"),
             (
                 "array_map",
@@ -214,9 +237,28 @@ mod tests {
                 "substr",
                 "<?php\nfunction substr(string $string, int $offset, ?int $length = null): string {}\n",
             ),
-        ]);
+        ])
+    }
+
+    /// Helper that includes minimal stub functions so that built-in
+    /// functions like `strlen` are resolvable.  Extra-arguments
+    /// checking is **off** (the default).
+    fn collect_with_stubs(php: &str) -> Vec<Diagnostic> {
+        let backend =
+            Backend::new_test_with_all_stubs(HashMap::new(), stub_fn_index(), HashMap::new());
+        let uri = "file:///test.php";
+        backend.update_ast(uri, php);
+        let mut out = Vec::new();
+        backend.collect_argument_count_diagnostics(uri, php, &mut out);
+        out
+    }
+
+    /// Like [`collect_with_stubs`] but with the `extra-arguments`
+    /// diagnostic enabled.
+    fn collect_with_stubs_extra(php: &str) -> Vec<Diagnostic> {
         let backend =
-            Backend::new_test_with_all_stubs(HashMap::new(), stub_fn_index, HashMap::new());
+            Backend::new_test_with_all_stubs(HashMap::new(), stub_fn_index(), HashMap::new());
+        enable_extra_args(&backend);
         let uri = "file:///test.php";
         backend.update_ast(uri, php);
         let mut out = Vec::new();
@@ -289,16 +331,67 @@ function test(): void {
         );
     }
 
-    // ── Too many arguments ──────────────────────────────────────────
+    // ── Too many arguments (default off) ────────────────────────────
 
     #[test]
-    fn flags_too_many_args_to_function() {
+    fn too_many_args_suppressed_by_default() {
         let php = r#"<?php
 function test(): void {
     strlen("hello", "extra");
 }
 "#;
         let diags = collect_with_stubs(php);
+        assert!(
+            diags.is_empty(),
+            "Extra-arguments diagnostic should be off by default, got: {diags:?}",
+        );
+    }
+
+    #[test]
+    fn too_many_args_to_user_function_suppressed_by_default() {
+        let php = r#"<?php
+function myHelper(string $a): void {}
+function test(): void {
+    myHelper("x", "y");
+}
+"#;
+        let diags = collect(php);
+        assert!(
+            diags.is_empty(),
+            "Extra-arguments diagnostic should be off by default, got: {diags:?}",
+        );
+    }
+
+    #[test]
+    fn too_many_args_to_method_suppressed_by_default() {
+        let php = r#"<?php
+class Greeter {
+    public function greet(string $name): string {
+        return "Hello, " . $name;
+    }
+}
+function test(): void {
+    $g = new Greeter();
+    $g->greet("world", "extra", "more");
+}
+"#;
+        let diags = collect(php);
+        assert!(
+            diags.is_empty(),
+            "Extra-arguments diagnostic should be off by default, got: {diags:?}",
+        );
+    }
+
+    // ── Too many arguments (opt-in) ─────────────────────────────────
+
+    #[test]
+    fn flags_too_many_args_to_function() {
+        let php = r#"<?php
+function test(): void {
+    strlen("hello", "extra");
+}
+"#;
+        let diags = collect_with_stubs_extra(php);
         assert_eq!(diags.len(), 1, "got: {diags:?}");
         assert!(
             diags[0].message.contains("got 2"),
@@ -320,7 +413,7 @@ function test(): void {
     $g->greet("world", "extra", "more");
 }
 "#;
-        let diags = collect(php);
+        let diags = collect_extra(php);
         assert!(
             diags.iter().any(|d| d.message.contains("got 3")),
             "Expected too-many-args diagnostic, got: {diags:?}",
@@ -462,7 +555,7 @@ function test(): void {
     myHelper("x", "y");
 }
 "#;
-        let diags = collect(php);
+        let diags = collect_extra(php);
         assert!(
             diags
                 .iter()
@@ -534,7 +627,7 @@ function test(): void {
     new User("Alice", "extra");
 }
 "#;
-        let diags = collect(php);
+        let diags = collect_extra(php);
         assert!(
             diags.iter().any(|d| d.message.contains("got 2")),
             "Expected too-many-args diagnostic for constructor, got: {diags:?}",
@@ -581,7 +674,7 @@ function test(): void {
     helper("x", "y", "z");
 }
 "#;
-        let diags = collect(php);
+        let diags = collect_extra(php);
         assert!(
             diags.iter().any(|d| d.message.contains("at most 2")),
             "Expected 'at most' wording, got: {diags:?}",
@@ -600,10 +693,34 @@ function test(): void {
     two(1, 2, 3);
 }
 "#;
-        let diags = collect(php);
+        let diags = collect_extra(php);
         assert_eq!(diags.len(), 2, "Expected 2 diagnostics, got: {diags:?}",);
     }
 
+    #[test]
+    fn too_few_still_reported_when_extra_args_disabled() {
+        // "Too few" must always fire regardless of the extra-arguments flag.
+        let php = r#"<?php
+function one(int $a): void {}
+function two(int $a, int $b): void {}
+function test(): void {
+    one();
+    two(1, 2, 3);
+}
+"#;
+        let diags = collect(php);
+        assert_eq!(
+            diags.len(),
+            1,
+            "Only the too-few diagnostic should fire by default, got: {diags:?}",
+        );
+        assert!(
+            diags[0].message.contains("got 0"),
+            "message: {}",
+            diags[0].message,
+        );
+    }
+
     // ── Scope methods (Laravel) ─────────────────────────────────────
 
     #[test]