Skip to content

feat(rules): configurable controllerNamespacePrefixes on the three controller-scoped rules (WR-0283)#48

Merged
Goosterhof merged 1 commit into
mainfrom
feat/configurable-controller-namespace-prefixes
Jul 3, 2026
Merged

feat(rules): configurable controllerNamespacePrefixes on the three controller-scoped rules (WR-0283)#48
Goosterhof merged 1 commit into
mainfrom
feat/configurable-controller-namespace-prefixes

Conversation

@Goosterhof

Copy link
Copy Markdown
Contributor

What & why

The three controller-scoped rules — ForbidEloquentMutationInControllersRule, EnforceCurrentUserAttributeRule, ForbidResourceWrappedInJsonResponseRule — each hardcoded private const CONTROLLER_NAMESPACE_PREFIX = 'App\Http\Controllers'. emmie ships controllers under App\Http\Client\Controllers (4) + App\Http\Admin\Controllers (9), so their inline Eloquent mutations / $request->user() / resource-wraps were rule-invisible (surfaced by the emmie WR-0275 migration pilot). ForbidEloquentMutationInControllersRule's own docblock already prescribed the fix: "lift this into a controllerNamespacePrefixes."

Fleet survey (2026-07-02): emmie is the only consumer with sub-namespaced controllers — kendo/ublgenie/entreezuil/codebook use canonical App\Http\Controllers, already fully covered.

Change

  • New optional PHPStan parameter controllerNamespacePrefixes (default ['App\Http\Controllers']), wired through extension.neon on all three rules — mirrors the formRequestBaseClass precedent. The namespace gate now matches when the class namespace starts with any configured prefix.
  • Consumers with sub-namespaced controllers enroll them in their phpstan.neon; everyone else is unaffected.

Versioning — backward-compatible MINOR

Default ['App\Http\Controllers'] reproduces the prior str_starts_with behaviour byte-for-byte (kendo's App\Http\Controllers\Central\* still flags under default) ⇒ zero new errors in any consumer at the default. New violations appear only when a consumer opts a divergent namespace in. Same shape as v0.6.0's formRequestToDtoExemptClasses.

Tests / gates

  • +9 tests (124 → 133): per rule, a sub-namespaced-controller fixture proving CLEAN-under-default + FLAGGED-when-configured, plus a container-resolved test exercising the shipped NEON default + %controllerNamespacePrefixes% wiring (the guard against a NEON double-backslash no-op regression).
  • All pre-existing fixtures/tests green (canonical + Central\* still flag under default).
  • composer test 133/199 · phpstan level max clean · Pint clean · coverage 88.61% · Infection MSI 84.72% (zero escaped mutants in the new gate code) · audit clean.

Downstream

emmie opt-in tracked as WR-0291 (enroll its two sub-namespaces + baseline-absorb the ~32 surfaced mutations, folding into the WR-0275 migration campaign). EnforceCurrentUserAttribute surfaces 0 on emmie (already #[CurrentUser]); ForbidResourceWrapped a small surface.

🤖 Generated with Claude Code

…ntroller-scoped rules — ForbidEloquentMutationInControllers, EnforceCurrentUserAttribute, ForbidResourceWrappedInJsonResponse (WR-0283)

Replaces the hardcoded `private const CONTROLLER_NAMESPACE_PREFIX =
'App\Http\Controllers'` gate on all three controller-scoped rules with a shared
constructor-injected `array $controllerNamespacePrefixes` (default
`['App\Http\Controllers']`). A class is in scope when its namespace
str_starts_with ANY configured prefix, so canonical sub-namespaces (kendo's
`App\Http\Controllers\Central\*`) still match under the default byte-for-byte.

Wired through extension.neon mirroring the formRequestBaseClass precedent:
`controllerNamespacePrefixes` parameter default + `listOf(string())` schema +
`%controllerNamespacePrefixes%` argument on all three rules' service
registrations. Consumers with divergent controller namespaces (emmie's
`App\Http\Client\Controllers` / `App\Http\Admin\Controllers`) opt them in from
phpstan.neon — no consumer namespace hardcoded in a rule body. Resolves the
ForbidEloquentMutationInControllersRule docblock's standing "lift this into a
controllerNamespacePrefixes parameter" TODO, applied uniformly so one knob
governs the whole controller surface.

Default-unchanged invariant pinned end-to-end by a container-resolved test per
rule (testRuleResolvesFromExtensionNeonAndFiresOnDefaultPrefix) plus every
pre-existing fixture/test. New sub-namespaced-controller fixture per rule proves
the emmie shape is CLEAN under the default and FLAGGED once the prefix is
configured. Backward-compatible MINOR (default reproduces current behaviour;
zero new errors in any consumer at the default). Not tagged.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016BD7TLVaCmFLakYWkxaudp
@Goosterhof Goosterhof requested a review from a team as a code owner July 2, 2026 14:09
@Goosterhof Goosterhof added the Agent Review Requested Requesting review of specialized AI review agents. label Jul 2, 2026
@jasperboerhof

Copy link
Copy Markdown
Contributor

Town Crier Review · 9/10 · PASS · 🔎 Independent

phpstan-warroom-rules #48 · AC anchor: PR description (no reachable board) · head 78ecd57f41 · via the town-crier bus (request #220)

Tip

Reviewed a contract-package feature that lifts the hardcoded App\Http\Controllers gate into a shared configurable controllerNamespacePrefixes parameter across all three controller-scoped rules; verified the default is byte-for-byte identical (single-element list loop == prior single str_starts_with), that no fourth rule still hardcodes the prefix (CONTROLLER_NAMESPACE_PREFIX fully removed; remaining getNamespace() rules are Action-scoped), that all three rules are wired in extension.neon with a listOf(string()) schema, and that the new sub-namespace fixtures genuinely fire via the tests/Fixtures/ classmap autoload rather than being vacuous. The one sharp edge — NEON list params replace, so a consumer overriding the list must re-include the canonical prefix or silently lose coverage — is standard PHPStan behavior and correctly documented in the README recipe; CI is pending (8.4/8.5), not red.

No findings — clean against the review checklist.

@jasperboerhof jasperboerhof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved — Town Crier verdict PASS @Head, CI green, no open MAJOR+ thread. Our approval is our independent vote (approve-alongside): a peer's review / CHANGES_REQUESTED never withholds it — we verify every blocker ourselves, and a real one drops our own verdict below PASS. See the verdict comment + inline notes.

@Goosterhof Goosterhof left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approve-worthy

0 blockers · 0 concerns · 1 nit · 2 praise · 0 inline

CI green (8.4/8.5). Bus thread: dispatch posted an independent PASS at 14:16 (no findings, correctly flagged the standard NEON-list-replace semantics as documented, not a defect); Jasper's harness auto-approved on that verdict. I independently re-derived the same result — confirming, not just deferring to, dispatch's read.

Lifts the hardcoded App\Http\Controllers gate on the three controller-scoped rules (ForbidEloquentMutationInControllersRule, EnforceCurrentUserAttributeRule, ForbidResourceWrappedInJsonResponseRule) into a shared configurable controllerNamespacePrefixes parameter, mirroring the formRequestBaseClass/resourceDataBaseClass precedent — closes the emmie sub-namespace blind spot (WR-0283).

Verification

  • Default-preservation claim confirmed: all three rules replace str_starts_with($ns, self::CONST) with a foreach-any-prefix loop over a single-element default array — byte-identical match set at the default.
  • "No fourth rule still hardcodes the prefix" claim confirmed by direct grep of src/Rules/*.phpCONTROLLER_NAMESPACE_PREFIX is fully gone from the tree; the only other namespace-scoped rules (EnforceActionTransactionsRule, ForbidDatabaseManagerInActionsRule) gate on App\Actions, unaffected.
  • extension.neon wiring confirmed on all three rules' service definitions + parametersSchema: listOf(string()).
  • Container-resolution tests (testRuleResolvesFromExtensionNeonAndFiresOnDefaultPrefix) correctly close the gap a constructor-default-only test would miss — a NEON single/double-backslash quoting slip would silently no-op the whole rule for every default consumer, and only an extension.neon-loaded test catches that class of regression.
  • Constructor shape (private array $controllerNamespacePrefixes = ['App\Http\Controllers']) matches the existing formRequestBaseClass/resourceDataBaseClass single-param precedent style — no stylistic drift.

Nit

  • The constructor property is typed bare array (phpdoc-only list<string>) on all three rules — matches existing precedent ($formRequestBaseClass is typed string, so there's no established list<string>-native pattern here either), so this isn't a regression, just worth tightening to array<int, string>/list<string> via a phpstan-level self-hint if the package ever raises its own bar on iterable value types.

Praise

  • The testRuleResolvesFromExtensionNeonAndFiresOnDefaultPrefix gate per rule — pinning the shipped NEON default, not the PHP constructor default, closes exactly the failure class this kind of config-lift usually ships with.
  • CHANGELOG entry states the versioning call explicitly (backward-compatible MINOR, contrasted against the v0.5.0 candidate-major shape) — keeps the release-decision reasoning auditable instead of asserted.

Automated war-room agent review — posted because this PR carries the Agent Review Requested label.

@Goosterhof Goosterhof merged commit cbf5775 into main Jul 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Agent Review Requested Requesting review of specialized AI review agents.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants