Skip to content

feat: hook handler argument types (sniff + PHPStan rule)#28

Draft
d4mation wants to merge 3 commits into
mainfrom
feature/hook-handler-types
Draft

feat: hook handler argument types (sniff + PHPStan rule)#28
d4mation wants to merge 3 commits into
mainfrom
feature/hook-handler-types

Conversation

@d4mation

@d4mation d4mation commented Jul 23, 2026

Copy link
Copy Markdown

What

Adds two complementary checks that forbid native parameter and return types on WordPress hook handlers, where the types are not guaranteed.

A hook's argument and return types are never guaranteed: inaccurate documentation can lead to the wrong native type, and any third party can dispatch one of our hooks via apply_filters() / do_action() with arguments of a different type. Either way, a value that does not match a native type on the handler causes a runtime fatal. Enforcement follows the hook type:

  • Filter handlers: no native type on any parameter and no native return type.
  • Action handlers: no native type on any parameter; a native void return type is allowed (actions always return void).

There is no prefix or first-party concept — the check applies to every hook, and needs zero configuration.

1. PHPCS sniff — StellarWP.Hooks.HookHandlerTypes (auto-fixable)

Analyses at the add_action() / add_filter() call site and resolves same-file handlers — inline closures/arrows, [ $this, 'method' ] / [ self::class, 'method' ] (including the old [ &$this, 'method' ] idiom), and 'function_name' globals — then strips only the offending native types (verified to never reformat: nullable/union/intersection/DNF/FQN/variadic/by-ref/attribute forms and multiline layouts, idempotent). Enable with <rule ref="StellarWP.Hooks.HookHandlerTypes"/>. Optional allow_void_return_on_actions property (default true).

2. PHPStan rule (auto-discovered extension)

Whole-codebase, never diff-limited. Resolves all callback forms across files via reflection (methods, Class::method, global functions, closures) and resolves hook names via type inference (so a $hook = '...' variable is caught). Report-only; the message names the exact handler. Auto-registers via phpstan/extension-installer with no configuration; optional allowVoidReturnOnActions parameter (default true).

The two are complementary: the sniff gives instant, auto-fixable feedback for the same-file case; the PHPStan rule is the authoritative gate that catches handlers whose declaration lives in a different file from the hook call — the case a diff-limited phpcs run misses.

Tests

  • AbstractSniffUnitTest (sniff) — asserts exact error lines + compares phpcbf output to a .inc.fixed baseline.
  • RuleTestCase (PHPStan rule) — asserts exact messages/lines.
  • composer test runs ruleset-explain + both suites. Green on PHP 7.4 (the rule test skips due to a php-parser emulation quirk on the 7.4 test runtime; the rule itself runs fine on 7.4) and 8.3.

Real-world validation

Installed into learndash-notifications (zero config). Both tools flag a real WPML fatal that had shipped and been hotfixed separately:

Uncaught TypeError: WPML::filter_has_access(): Argument #3 ($user_id) must be of type int, null given
apply_filters( 'sfwd_lms_has_access', true, 28, null )

Each tool reports 16 findings across 5 files, including all 8 in WPML.php.

Notes

  • New dev deps: phpunit/phpunit, phpstan/phpstan. New autoload (StellarWP\ psr-4), extra.phpstan.includes, and a working composer test.

Draft: opened for review before merge.

- Add StellarWP.Hooks.HookHandlerTypes phpcs sniff (auto-fixable) that forbids native parameter/return types on handlers of hooks whose name does not match a configured prefix
- Add companion PHPStan rule (auto-discovered extension) for whole-codebase, cross-file coverage and type-inferred hook names
- Add PHPUnit sniff (AbstractSniffUnitTest) and rule (RuleTestCase) tests plus phpunit harness and wired composer test scripts

Native types on WP core / third-party hook handlers can cause runtime fatals; these checks enforce type-less handlers for hooks we do not own.
@d4mation d4mation self-assigned this Jul 23, 2026
@d4mation d4mation added code review Status: requires a code review. and removed code review Status: requires a code review. labels Jul 23, 2026
d4mation added 2 commits July 23, 2026 20:29
- Filters (any prefix, first-party or not) must now be fully type-less on every parameter and the return type, not just non-first-party hooks
- A real WPML fatal (TypeError: filter_has_access(): Argument #3 $user_id must be of type int, null given) showed that even first-party filter context arguments are unsafe: core dispatches apply_filters( 'sfwd_lms_has_access', true, 28, null )
- First-party actions remain unrestricted; non-first-party actions unchanged (params type-less, void return allowed)
- Sniff now resolves the old-style [ &$this, 'method' ] callback idiom
- Update sniff + PHPStan rule fixtures/tests and the README enforcement matrix
- Action handlers now also forbid native types on every parameter (a native void return type is still allowed); filters already forbid all param and return types
- Hook argument/return types are never guaranteed regardless of who defines the hook, so the first-party prefix concept is removed entirely
- Remove the sniff `prefixes` property, the rule `prefixes` parameter, and all is_first_party logic; the rule now auto-registers with zero configuration
- Update fixtures, tests, extension.neon and the README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant