Php Core Standards — pekral/arch-app-services

## Project Context - The project tech stack is defined in `composer.json`. - Use the PHP version and major package versions defined in `composer.json` as the source of truth. - Prefer existing project conventions over introducing new patterns. ## AI Behavior - Do not apologize. - Do not invent changes, files, implementations, or results. - Preserve existing code and do not remove unrelated logic. - Verify visible context before proposing changes. - Do not speculate when the answer can be derived from the repository context. - Do not ask the user to verify something that is already visible in the provided code or files. - Do not suggest file changes when no actual modification is needed. - Fix obvious grammatical issues in user-facing text you modify. - Default to autonomous execution: proceed without asking when the answer can be inferred from the issue, the codebase, the project configuration, or prior conversation context. - Only ask the user when the next step is genuinely ambiguous **and** the ambiguity cannot be resolved from the available context. State the specific ambiguity that blocks the work. - Never ask the user to confirm a fact the agent can verify itself (tests passing, fixers clean, branch up-to-date, file exists, etc.) — verify it directly instead. - Never gate on user approval for work the assignment already authorizes (creating commits, opening PRs, applying review fixes mandated by the calling skill). - When user input is genuinely required, batch all questions into a single round; do not serialize one question at a time. ## Naming - Use clear, descriptive names that reveal purpose. - Prefer descriptive method and variable names over comments. - Classes: PascalCase. - Methods and variables: camelCase. - Routes: kebab-case URLs and dot notation route names. - Config keys: snake_case. - Controllers: plural noun + `Controller`. - Jobs: action-oriented names. - Events: past-tense or domain-event names. - Enums: descriptive case names. ## Structure - Keep one clear responsibility per class and per method. - Avoid god classes and mixed responsibilities. - Prefer composition over inheritance unless inheritance is clearly justified. - Expose clear interfaces and keep implementation details internal. - Extract deeply nested conditionals into well-named methods where it improves readability. - Prefer small, simple classes or functions unless state is genuinely needed. - Prefer typed DTOs over raw arrays across important boundaries. - When a method, function, closure, constructor, `__invoke()`, or other callable requires more than 4 parameters, introduce a dedicated typed DTO (preferably `final readonly` with promoted constructor properties) and pass it as a single argument instead of a long parameter list. Promoted constructor properties count as parameters; a variadic (`...$args`) counts as one. The only exempt cases are entry-point signatures fixed by an external framework / vendor contract that the project cannot change — controller actions whose argument list is resolved by the service container, magic methods bound by the framework, event listeners whose signature is dictated by the dispatched event, and methods overriding a parent / interface owned by `vendor/`. The exemption must be cited explicitly wherever the rule is enforced (CR finding, refactoring proposal) with the parent class / interface and a one-line reason. - When a **public** method returns a structured set of values — an associative array representing a record, a multi-key payload, or an array shape with named string keys — return a typed DTO (preferably `final readonly` with promoted constructor properties) instead of the raw array. A DTO names and types each field, turning an opaque `array` return type into a self-documenting contract the caller can rely on. This does **not** apply to: a single scalar / `bool` / `string` / `int` / `enum` / `void` return; a homogeneous list or collection of one type (`list<int>` of IDs, `array<int, OrderData>`, `Collection<int, UserModel>`); or a return whose array shape is fixed by an external framework / vendor contract the project cannot change — `toArray()`, `jsonSerialize()`, `Arrayable::toArray()`, Eloquent `casts()` / `$attributes`, FormRequest `rules()`, a config callback the framework invokes, or a method overriding a parent / interface owned by `vendor/`. The exemption must be cited explicitly wherever the rule is enforced (CR finding, refactoring proposal) with the contract and a one-line reason. - Use `readonly` for immutable data where practical. - Use PHP attributes when they provide a cleaner and more idiomatic solution than manual wiring. ## Code Style - Follow active PHP-FIG standards used by the project. - Never reformat unrelated existing code. - Keep formatting readable and consistent with surrounding code. - Add blank lines between distinct logical blocks, but avoid unnecessary vertical noise. - Avoid cramped formatting and avoid excessive empty lines. ## PHP Practices - Always declare explicit return types. - Use the PHP version supported by `composer.json`. - Prefer modern PHP syntax when supported by the project. - Use `match` where it improves clarity. - Use named arguments when they improve readability and reduce ambiguity. - Document iterable value types with generics where applicable. - Use array shapes for fixed structured arrays when DTOs are not appropriate (e.g. an internal / private helper, or a framework / vendor contract); for a **public** method returning a structured record the *Structure* section's public-return rule applies — return a DTO. - Avoid magic numbers; extract meaningful constants when the value has domain meaning. - Prefer specific exceptions over generic ones. - Catch specific exceptions only when recovery or translation is meaningful. - Never suppress errors with `@`. - **Never introduce a static-analysis / linter suppression.** No suppression is ever compliant, and the ban is about the annotation actually taking effect: a pattern must never appear as an **effective** annotation on a line of code the change adds or modifies — quoting the pattern in this rule's own prose or in a test assertion that pins it is not a suppression. Covered patterns: a PHPCS ignore (`// phpcs:ignore`, `// phpcs:disable`, `@phpcsSuppress`, `@codingStandardsIgnoreStart` / `…Line` / `…End`), a PHPStan ignore (`@phpstan-ignore`, `@phpstan-ignore-line`, `@phpstan-ignore-next-line`, a new `ignoreErrors` entry or baseline addition in `phpstan.neon` / `phpstan-baseline.neon`), a Psalm / Phan suppression (`@psalm-suppress`, `@phan-suppress`, baseline addition), a PHPMD `@SuppressWarnings(...)`, or the PHP `@` operator — each silences the tool instead of fixing what it flagged. The ban reaches just as far when the silencing happens in the analyzer's own configuration instead of an inline annotation: a new `excludePaths` entry or a lowered `level:` in `phpstan.neon`, a removed rule or a new `<exclude name>` / `<exclude-pattern>` in `phpcs.xml` / a custom ruleset, a disabled rule in `pint.json`, a new `skip()` entry in `rector.php`, or a removed `analyse` / `phpcs` / checker step from `composer.json` scripts or a CI workflow — these weaken the tool's coverage with the same effect as an inline suppression and leave no per-line marker a diff-scoped review could catch, so a config change of this shape on a line the diff adds, modifies, or removes is the same finding **when it narrows the coverage of code the diff itself changes, or answers a reported analyzer finding** — narrowing coverage for an unrelated, independently justified reason (excluding a directory of generated code the diff does not touch, dropping a rule the project never actually enabled) is ordinary configuration maintenance, not a suppression. This does not reach configuration the diff leaves untouched — an existing skip is not retroactively non-compliant; only a new or widened one that narrows coverage the diff's own code relies on, or answers a reported finding, is. **Write the code so the finding stops being true**: restructure the block, narrow or correct the type, extract a method, introduce a value object, or remove the dead code the tool is actually reacting to — the fix changes the code, never the tool's opinion of it or the tool's configured coverage. When a finding truly cannot be resolved by rewriting the code — a genuinely unfixable third-party / framework false positive — the agent does not add a suppression or weaken the configuration to quiet the tool: it **stops** implementing that change and reports it as a blocker to whoever is driving the run (`Blocked: <reason>`, per the running agent's blocked-handoff contract), leaving the flagged line unsuppressed. Filing the underlying defect as its own tracked issue is a decision the caller makes only **after** that stop, for follow-up work — it is never a branch the implementing agent picks instead of stopping, and it never lets the current diff ship carrying the suppression or the unresolved finding either way. `assert($variable !== null)` for a required-but-unused variable (see below) resolves the warning outright and is not a suppression. CR severity for a suppression found in the diff: **Moderate**. - When fixing an unused variable (e.g. PHPCS `UnusedVariable`): delete the variable if it is not required. If the variable is required (e.g. assigned from a function call with side effects), resolve the warning with `assert($variable !== null)` or similar `assert()` instead of removing the assignment. ## Named Arguments - Many parameters is a code smell. It often means the method does too much or should accept a DTO/value object. - Named arguments help most with booleans, null, array values, and unclear strings. For example, `true`, `null`, `[]` without context say nothing. - They improve call-site readability without unnecessary variables. Instead of `$status = 'active'` just for a hint, use `status: 'active'`. - They should not replace good design. When a method has 8 parameters, named arguments do not fix the problem. They just make it bearable. - In public APIs, parameter names become part of the contract. Renaming a parameter in a public method can break consumers using named arguments. - Keep arguments in the original method signature order even when using named arguments. - See `rules/php/examples/named-arguments.md` for usage examples. ## Design Principles - Follow SOLID pragmatically, not dogmatically. - Keep I/O at the edges where practical. - Prefer deterministic behavior based on explicit inputs. - Extract repeated logic into reusable abstractions only when repetition is real and meaningful. - Keep related code together and maintain a logical folder structure. - Leave touched code cleaner than you found it. - Do not add speculative (YAGNI) parameters. Add a parameter to a method, function, action, or constructor only when at least one current caller actually needs it. Optional knobs, "in case" defaults, and parameters introduced solely for hypothetical future callers must be removed; add them later when a real use case appears. - Do not introduce PHP `interface` types speculatively. Define a project-owned interface only when it has **at least two non-test consumers, and/or at least two non-test implementations** — test doubles, mocks, and fakes do not count toward either threshold. Implementing a framework or third-party interface (e.g. `ShouldQueue`, `HasLabel`, `Arrayable`, contract interfaces from `vendor/`) is always allowed; the rule applies only to interfaces declared inside this project. When refactoring, collapse single-implementation, single-consumer interfaces back into their concrete class unless they exist for a documented architectural reason — a published package API surface, or a plugin extension point with a written contract documented in code or in the package README. ## Documentation - Prefer self-documenting code over explanatory comments. - **A comment is the exception, not the default.** Do not write a comment for code a reader can already follow. Narration of the statement below it, a restatement of the signature, dividing "section" banners, commented-out code, and changelog / history notes are noise — they add reading cost and zero information. A comment is warranted only when it clears one of two bars: it **carries knowledge the code cannot express** — the *why* (rationale, considered alternatives, a workaround for an external bug), the meaning of a domain term, or a navigation pointer to the other half of a flow — or **the logic itself is genuinely complex** (a non-obvious algorithm, an ordering or concurrency constraint, an external-system quirk) **and that complexity cannot be removed by writing the code more clearly**. Attempt the removal first: rename the symbol, extract an intention-revealing method, split a mixed-responsibility method, replace a magic value with a named constant or enum case. **The *why* bar covers only the residue.** Apply the same removal attempt before writing a *why* comment, and keep only the rationale no name can express — preferably shrunk to a `@see <issue / ADR>` pointer. A multi-line rationale block sitting on a condition built from unnamed literals is a restructuring signal, not a warranted comment. **Clean, readable code always beats a comment**, because the code is verified by the compiler, the tests, and every later change, while a comment is verified by nobody and quietly decays into a lie the next reader acts on. A comment that clears neither bar describes the *what* and must not be written. - **The codebase's default state is no comments — delete the ones that never earned their place.** The bar above governs the comments you *write*; it governs the ones already sitting in the file exactly as much. Whenever you open a file to change it, delete every unnecessary comment in the region you are working in — the *what*-narration, the restatement of the signature, the redundant type docblock the native types already carry, the commented-out code, the section banner, the changelog note, the comment that no longer matches the code — **including comments you did not write**, and including ones on lines your change does not otherwise touch. Removing a comment that carries no information is never a scope expansion: it cannot change behavior, it removes reading cost, and it is the only way a codebase ever converges on self-documenting code, because a rule that constrains only *new* comments leaves every existing one in place forever. Two ceilings bound the sweep: **stay inside the region you are already changing** — the changed hunks and the enclosing method / class member of each — and never open an unrelated file to hunt for comments; and **when the value of a comment is genuinely unclear, keep it and raise it** instead of deleting it — the bar for deletion is "carries no information the code does not already give", not "I personally did not need it". This does not loosen *Code Style* (*Never reformat unrelated existing code*) or *AI Behavior* (*Preserve existing code and do not remove unrelated logic*): a comment inside the region you are already changing is not unrelated, deleting one is not reformatting, and a comment is not logic — those rules keep governing every executable line and every file the change has no business opening. **Never delete a load-bearing comment** — one another rule requires as the *condition* of an exception, so removing it turns compliant code into a violation: the justification a per-row loop needs to stay allowed (`@rules/sql/optimalize.mdc` *Acceptable exception*), and the justification a benign `2>/dev/null` / `curl -s` needs to stay allowed (`@rules/security/backend.md` *Malicious Code & Supply-Chain Indicators*). Both answer *why* and are covered by the keep-bar already; they are named here because the deletion sweep is exactly where they get lost. - **Tooling-mandated annotations are kept.** A docblock a static analyzer still needs (`@param array<string, int>`, `@return list<…>`, `@template`, `@mixin` — the generics the native types cannot express, per *PHP Practices*), a licence / copyright header, and an annotation a framework, generator, or tool reads as input are not comment noise — deleting them removes a guarantee or breaks a tool. This is no licence for a redundant `@param string $name` on an already-typed parameter; that one still goes. - **Write the code so it needs no extensive documentation.** Prose decays — the code keeps changing, the comment does not, and a docblock that no longer matches the code misinforms every later reader. When a block needs a long explanation, that is a signal to **restructure the block**, not to write the explanation: extract intention-revealing private methods (one per step you were about to narrate), rename the symbol the comment was clarifying, split a mixed-responsibility method, replace a magic value with a named constant or enum case. A clean, readable implementation beats a correct description of an unreadable one; never add a multi-line PHPDoc or an inline commentary block as a substitute for that restructuring. - **A comment you keep is a comment you maintain.** Whenever you change a line, update or delete the comment / PHPDoc attached to it in the same change — a stale `@param`, a docblock describing a removed branch, or a `TODO` you just completed is a defect, not leftover noise. - Document only what the code cannot state for itself: public API contracts, genuinely complex business rules, non-obvious side effects, and important constraints. - PHPDoc should describe intent and domain meaning, not restate the method name or implementation steps. - Keep PHPDoc high-level and useful for maintainers. - See `rules/php/examples/self-documenting-code.md` for a worked before/after. ## Testing - Add or update tests for every meaningful behavior change. - Write all new tests using Pest syntax (`it()` / `test()` functional blocks); do not introduce PHPUnit-style class-based tests for new test files. - Require 100% test coverage for every changed or added code path. - **In code review / pre-push contexts (CR skills, `process-code-review`, `create-missing-tests-in-pr`, `create-test`) verify coverage for the changed files only** — every line, branch, and condition added or modified by the current changes must be covered, but do not gate on a project-wide coverage percentage. Use the coverage tooling the project already provides (a Phing coverage target, a Composer `test:coverage` / `coverage` script, or a direct `vendor/bin/pest --coverage-clover=<file>` / PHPUnit `--coverage-clover` invocation); do not add a new bespoke coverage script to the project. Full-suite coverage commands remain the release / CI gate. - Scope the run to the changed source files whenever the runner allows it (`--coverage-clover` plus a path filter, PCOV `pcov.directory` scoped to the changed directories); otherwise generate the report and read off only the changed files. - **Delete any auto-generated coverage report file (the `--coverage-clover` output or other coverage artifact) as soon as it has been read**, so it is never accidentally committed to git, and keep such artifacts in `.gitignore` as a second line of defence. - **Report the coverage result short by default (issue #528 follow-up).** Run the coverage check on every change, but report it on the published CR / tracker comment **only** when there is something the reader must act on — uncovered changed lines (listed as Critical findings) or unavailable coverage tooling (also a Critical finding). When every changed line is at 100% coverage and the tool ran successfully, omit the `## Coverage` section, the `Coverage:` header line, and the `coverage …` slot from the summary line. The Counts line carries the clean signal; the omission is the report. The check itself still runs unconditionally on every CR / pre-push gate. - Prefer deterministic tests. - Use the arrange-act-assert structure when it improves readability. - Use clear, human-readable test names. - Do not use `describe()` in Pest tests. - Prefer top-level `it()` or `test()` blocks only. - Prefer data providers through test arguments rather than PHPDoc annotations. - Avoid reflection in tests unless there is no reasonable alternative. - Split complex conditional test setups into separate test cases instead of branching inside a test. - Never generate `covers()` methods unless the repository explicitly requires them. - Mock only external services or exception paths that are otherwise hard to reach. - Prefer partial mocks over full mocks when mocking is necessary. - Follow existing project testing conventions unless introducing a new pattern is clearly justified. ## Bug Fix Workflow - Write or update a failing test before fixing a bug when practical. - Refactor only after behavior is covered. - Run the most relevant tests for changed code first. ## Fluent API Design - Prefer readable, intention-revealing APIs over vague helper utilities. - Use factory methods like `of()` or `from()` when they match existing project style. - Prefer explicit domain language over generic utility method naming. - Avoid god utility classes with many unrelated static helpers. ## Globals - Do not use global variables. - Do not introduce `global` state in application or test code unless the platform truly requires it. ## Async Review Note - When reviewing asynchronous jobs, first verify whether retry, timeout, and backoff behavior is defined in the job itself or centrally in configuration.

Project Context

The project tech stack is defined in composer.json.
Use the PHP version and major package versions defined in composer.json as the source of truth.
Prefer existing project conventions over introducing new patterns.

AI Behavior

Do not apologize.
Do not invent changes, files, implementations, or results.
Preserve existing code and do not remove unrelated logic.
Verify visible context before proposing changes.
Do not speculate when the answer can be derived from the repository context.
Do not ask the user to verify something that is already visible in the provided code or files.
Do not suggest file changes when no actual modification is needed.
Fix obvious grammatical issues in user-facing text you modify.
Default to autonomous execution: proceed without asking when the answer can be inferred from the issue, the codebase, the project configuration, or prior conversation context.
Only ask the user when the next step is genuinely ambiguous and the ambiguity cannot be resolved from the available context. State the specific ambiguity that blocks the work.
Never ask the user to confirm a fact the agent can verify itself (tests passing, fixers clean, branch up-to-date, file exists, etc.) — verify it directly instead.
Never gate on user approval for work the assignment already authorizes (creating commits, opening PRs, applying review fixes mandated by the calling skill).
When user input is genuinely required, batch all questions into a single round; do not serialize one question at a time.

Naming

Use clear, descriptive names that reveal purpose.
Prefer descriptive method and variable names over comments.
Classes: PascalCase.
Methods and variables: camelCase.
Routes: kebab-case URLs and dot notation route names.
Config keys: snake_case.
Controllers: plural noun + Controller.
Jobs: action-oriented names.
Events: past-tense or domain-event names.
Enums: descriptive case names.

Structure

Keep one clear responsibility per class and per method.
Avoid god classes and mixed responsibilities.
Prefer composition over inheritance unless inheritance is clearly justified.
Expose clear interfaces and keep implementation details internal.
Extract deeply nested conditionals into well-named methods where it improves readability.
Prefer small, simple classes or functions unless state is genuinely needed.
Prefer typed DTOs over raw arrays across important boundaries.
When a method, function, closure, constructor, __invoke(), or other callable requires more than 4 parameters, introduce a dedicated typed DTO (preferably final readonly with promoted constructor properties) and pass it as a single argument instead of a long parameter list. Promoted constructor properties count as parameters; a variadic (...$args) counts as one. The only exempt cases are entry-point signatures fixed by an external framework / vendor contract that the project cannot change — controller actions whose argument list is resolved by the service container, magic methods bound by the framework, event listeners whose signature is dictated by the dispatched event, and methods overriding a parent / interface owned by vendor/. The exemption must be cited explicitly wherever the rule is enforced (CR finding, refactoring proposal) with the parent class / interface and a one-line reason.
When a public method returns a structured set of values — an associative array representing a record, a multi-key payload, or an array shape with named string keys — return a typed DTO (preferably final readonly with promoted constructor properties) instead of the raw array. A DTO names and types each field, turning an opaque array return type into a self-documenting contract the caller can rely on. This does not apply to: a single scalar / bool / string / int / enum / void return; a homogeneous list or collection of one type (list<int> of IDs, array<int, OrderData>, Collection<int, UserModel>); or a return whose array shape is fixed by an external framework / vendor contract the project cannot change — toArray(), jsonSerialize(), Arrayable::toArray(), Eloquent casts() / $attributes, FormRequest rules(), a config callback the framework invokes, or a method overriding a parent / interface owned by vendor/. The exemption must be cited explicitly wherever the rule is enforced (CR finding, refactoring proposal) with the contract and a one-line reason.
Use readonly for immutable data where practical.
Use PHP attributes when they provide a cleaner and more idiomatic solution than manual wiring.

Code Style

Follow active PHP-FIG standards used by the project.
Never reformat unrelated existing code.
Keep formatting readable and consistent with surrounding code.
Add blank lines between distinct logical blocks, but avoid unnecessary vertical noise.
Avoid cramped formatting and avoid excessive empty lines.

PHP Practices

Always declare explicit return types.
Use the PHP version supported by composer.json.
Prefer modern PHP syntax when supported by the project.
Use match where it improves clarity.
Use named arguments when they improve readability and reduce ambiguity.
Document iterable value types with generics where applicable.
Use array shapes for fixed structured arrays when DTOs are not appropriate (e.g. an internal / private helper, or a framework / vendor contract); for a public method returning a structured record the Structure section's public-return rule applies — return a DTO.
Avoid magic numbers; extract meaningful constants when the value has domain meaning.
Prefer specific exceptions over generic ones.
Catch specific exceptions only when recovery or translation is meaningful.
Never suppress errors with @.
Never introduce a static-analysis / linter suppression. No suppression is ever compliant, and the ban is about the annotation actually taking effect: a pattern must never appear as an effective annotation on a line of code the change adds or modifies — quoting the pattern in this rule's own prose or in a test assertion that pins it is not a suppression. Covered patterns: a PHPCS ignore (// phpcs:ignore, // phpcs:disable, @phpcsSuppress, @codingStandardsIgnoreStart / …Line / …End), a PHPStan ignore (@phpstan-ignore, @phpstan-ignore-line, @phpstan-ignore-next-line, a new ignoreErrors entry or baseline addition in phpstan.neon / phpstan-baseline.neon), a Psalm / Phan suppression (@psalm-suppress, @phan-suppress, baseline addition), a PHPMD @SuppressWarnings(...), or the PHP @ operator — each silences the tool instead of fixing what it flagged. The ban reaches just as far when the silencing happens in the analyzer's own configuration instead of an inline annotation: a new excludePaths entry or a lowered level: in phpstan.neon, a removed rule or a new <exclude name> / <exclude-pattern> in phpcs.xml / a custom ruleset, a disabled rule in pint.json, a new skip() entry in rector.php, or a removed analyse / phpcs / checker step from composer.json scripts or a CI workflow — these weaken the tool's coverage with the same effect as an inline suppression and leave no per-line marker a diff-scoped review could catch, so a config change of this shape on a line the diff adds, modifies, or removes is the same finding when it narrows the coverage of code the diff itself changes, or answers a reported analyzer finding — narrowing coverage for an unrelated, independently justified reason (excluding a directory of generated code the diff does not touch, dropping a rule the project never actually enabled) is ordinary configuration maintenance, not a suppression. This does not reach configuration the diff leaves untouched — an existing skip is not retroactively non-compliant; only a new or widened one that narrows coverage the diff's own code relies on, or answers a reported finding, is. Write the code so the finding stops being true: restructure the block, narrow or correct the type, extract a method, introduce a value object, or remove the dead code the tool is actually reacting to — the fix changes the code, never the tool's opinion of it or the tool's configured coverage. When a finding truly cannot be resolved by rewriting the code — a genuinely unfixable third-party / framework false positive — the agent does not add a suppression or weaken the configuration to quiet the tool: it stops implementing that change and reports it as a blocker to whoever is driving the run (Blocked: <reason>, per the running agent's blocked-handoff contract), leaving the flagged line unsuppressed. Filing the underlying defect as its own tracked issue is a decision the caller makes only after that stop, for follow-up work — it is never a branch the implementing agent picks instead of stopping, and it never lets the current diff ship carrying the suppression or the unresolved finding either way. assert($variable !== null) for a required-but-unused variable (see below) resolves the warning outright and is not a suppression. CR severity for a suppression found in the diff: Moderate.
When fixing an unused variable (e.g. PHPCS UnusedVariable): delete the variable if it is not required. If the variable is required (e.g. assigned from a function call with side effects), resolve the warning with assert($variable !== null) or similar assert() instead of removing the assignment.

Named Arguments

Many parameters is a code smell. It often means the method does too much or should accept a DTO/value object.
Named arguments help most with booleans, null, array values, and unclear strings. For example, true, null, [] without context say nothing.
They improve call-site readability without unnecessary variables. Instead of $status = 'active' just for a hint, use status: 'active'.
They should not replace good design. When a method has 8 parameters, named arguments do not fix the problem. They just make it bearable.
In public APIs, parameter names become part of the contract. Renaming a parameter in a public method can break consumers using named arguments.
Keep arguments in the original method signature order even when using named arguments.
See rules/php/examples/named-arguments.md for usage examples.

Design Principles

Follow SOLID pragmatically, not dogmatically.
Keep I/O at the edges where practical.
Prefer deterministic behavior based on explicit inputs.
Extract repeated logic into reusable abstractions only when repetition is real and meaningful.
Keep related code together and maintain a logical folder structure.
Leave touched code cleaner than you found it.
Do not add speculative (YAGNI) parameters. Add a parameter to a method, function, action, or constructor only when at least one current caller actually needs it. Optional knobs, "in case" defaults, and parameters introduced solely for hypothetical future callers must be removed; add them later when a real use case appears.
Do not introduce PHP interface types speculatively. Define a project-owned interface only when it has at least two non-test consumers, and/or at least two non-test implementations — test doubles, mocks, and fakes do not count toward either threshold. Implementing a framework or third-party interface (e.g. ShouldQueue, HasLabel, Arrayable, contract interfaces from vendor/) is always allowed; the rule applies only to interfaces declared inside this project. When refactoring, collapse single-implementation, single-consumer interfaces back into their concrete class unless they exist for a documented architectural reason — a published package API surface, or a plugin extension point with a written contract documented in code or in the package README.

Documentation

Prefer self-documenting code over explanatory comments.
A comment is the exception, not the default. Do not write a comment for code a reader can already follow. Narration of the statement below it, a restatement of the signature, dividing "section" banners, commented-out code, and changelog / history notes are noise — they add reading cost and zero information. A comment is warranted only when it clears one of two bars: it carries knowledge the code cannot express — the why (rationale, considered alternatives, a workaround for an external bug), the meaning of a domain term, or a navigation pointer to the other half of a flow — or the logic itself is genuinely complex (a non-obvious algorithm, an ordering or concurrency constraint, an external-system quirk) and that complexity cannot be removed by writing the code more clearly. Attempt the removal first: rename the symbol, extract an intention-revealing method, split a mixed-responsibility method, replace a magic value with a named constant or enum case. The why bar covers only the residue. Apply the same removal attempt before writing a why comment, and keep only the rationale no name can express — preferably shrunk to a @see <issue / ADR> pointer. A multi-line rationale block sitting on a condition built from unnamed literals is a restructuring signal, not a warranted comment. Clean, readable code always beats a comment, because the code is verified by the compiler, the tests, and every later change, while a comment is verified by nobody and quietly decays into a lie the next reader acts on. A comment that clears neither bar describes the what and must not be written.
The codebase's default state is no comments — delete the ones that never earned their place. The bar above governs the comments you write; it governs the ones already sitting in the file exactly as much. Whenever you open a file to change it, delete every unnecessary comment in the region you are working in — the what-narration, the restatement of the signature, the redundant type docblock the native types already carry, the commented-out code, the section banner, the changelog note, the comment that no longer matches the code — including comments you did not write, and including ones on lines your change does not otherwise touch. Removing a comment that carries no information is never a scope expansion: it cannot change behavior, it removes reading cost, and it is the only way a codebase ever converges on self-documenting code, because a rule that constrains only new comments leaves every existing one in place forever. Two ceilings bound the sweep: stay inside the region you are already changing — the changed hunks and the enclosing method / class member of each — and never open an unrelated file to hunt for comments; and when the value of a comment is genuinely unclear, keep it and raise it instead of deleting it — the bar for deletion is "carries no information the code does not already give", not "I personally did not need it". This does not loosen Code Style (Never reformat unrelated existing code) or AI Behavior (Preserve existing code and do not remove unrelated logic): a comment inside the region you are already changing is not unrelated, deleting one is not reformatting, and a comment is not logic — those rules keep governing every executable line and every file the change has no business opening. Never delete a load-bearing comment — one another rule requires as the condition of an exception, so removing it turns compliant code into a violation: the justification a per-row loop needs to stay allowed (@rules/sql/optimalize.mdc Acceptable exception), and the justification a benign 2>/dev/null / curl -s needs to stay allowed (@rules/security/backend.md Malicious Code & Supply-Chain Indicators). Both answer why and are covered by the keep-bar already; they are named here because the deletion sweep is exactly where they get lost.
Tooling-mandated annotations are kept. A docblock a static analyzer still needs (@param array<string, int>, @return list<…>, @template, @mixin — the generics the native types cannot express, per PHP Practices), a licence / copyright header, and an annotation a framework, generator, or tool reads as input are not comment noise — deleting them removes a guarantee or breaks a tool. This is no licence for a redundant @param string $name on an already-typed parameter; that one still goes.
Write the code so it needs no extensive documentation. Prose decays — the code keeps changing, the comment does not, and a docblock that no longer matches the code misinforms every later reader. When a block needs a long explanation, that is a signal to restructure the block, not to write the explanation: extract intention-revealing private methods (one per step you were about to narrate), rename the symbol the comment was clarifying, split a mixed-responsibility method, replace a magic value with a named constant or enum case. A clean, readable implementation beats a correct description of an unreadable one; never add a multi-line PHPDoc or an inline commentary block as a substitute for that restructuring.
A comment you keep is a comment you maintain. Whenever you change a line, update or delete the comment / PHPDoc attached to it in the same change — a stale @param, a docblock describing a removed branch, or a TODO you just completed is a defect, not leftover noise.
Document only what the code cannot state for itself: public API contracts, genuinely complex business rules, non-obvious side effects, and important constraints.
PHPDoc should describe intent and domain meaning, not restate the method name or implementation steps.
Keep PHPDoc high-level and useful for maintainers.
See rules/php/examples/self-documenting-code.md for a worked before/after.

Testing

Add or update tests for every meaningful behavior change.
Write all new tests using Pest syntax (it() / test() functional blocks); do not introduce PHPUnit-style class-based tests for new test files.
Require 100% test coverage for every changed or added code path.
In code review / pre-push contexts (CR skills, process-code-review, create-missing-tests-in-pr, create-test) verify coverage for the changed files only — every line, branch, and condition added or modified by the current changes must be covered, but do not gate on a project-wide coverage percentage. Use the coverage tooling the project already provides (a Phing coverage target, a Composer test:coverage / coverage script, or a direct vendor/bin/pest --coverage-clover=<file> / PHPUnit --coverage-clover invocation); do not add a new bespoke coverage script to the project. Full-suite coverage commands remain the release / CI gate.
Scope the run to the changed source files whenever the runner allows it (--coverage-clover plus a path filter, PCOV pcov.directory scoped to the changed directories); otherwise generate the report and read off only the changed files.
Delete any auto-generated coverage report file (the --coverage-clover output or other coverage artifact) as soon as it has been read, so it is never accidentally committed to git, and keep such artifacts in .gitignore as a second line of defence.
Report the coverage result short by default (issue #528 follow-up). Run the coverage check on every change, but report it on the published CR / tracker comment only when there is something the reader must act on — uncovered changed lines (listed as Critical findings) or unavailable coverage tooling (also a Critical finding). When every changed line is at 100% coverage and the tool ran successfully, omit the ## Coverage section, the Coverage: header line, and the coverage … slot from the summary line. The Counts line carries the clean signal; the omission is the report. The check itself still runs unconditionally on every CR / pre-push gate.
Prefer deterministic tests.
Use the arrange-act-assert structure when it improves readability.
Use clear, human-readable test names.
Do not use describe() in Pest tests.
Prefer top-level it() or test() blocks only.
Prefer data providers through test arguments rather than PHPDoc annotations.
Avoid reflection in tests unless there is no reasonable alternative.
Split complex conditional test setups into separate test cases instead of branching inside a test.
Never generate covers() methods unless the repository explicitly requires them.
Mock only external services or exception paths that are otherwise hard to reach.
Prefer partial mocks over full mocks when mocking is necessary.
Follow existing project testing conventions unless introducing a new pattern is clearly justified.

Bug Fix Workflow

Write or update a failing test before fixing a bug when practical.
Refactor only after behavior is covered.
Run the most relevant tests for changed code first.

Fluent API Design

Prefer readable, intention-revealing APIs over vague helper utilities.
Use factory methods like of() or from() when they match existing project style.
Prefer explicit domain language over generic utility method naming.
Avoid god utility classes with many unrelated static helpers.

Globals

Do not use global variables.
Do not introduce global state in application or test code unless the platform truly requires it.

Async Review Note

When reviewing asynchronous jobs, first verify whether retry, timeout, and backoff behavior is defined in the job itself or centrally in configuration.