Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
- **Doctrine:** corrected publish-channel framing in `CLAUDE.md` (L11 and the Release process section) and the `release.yml` header comment. Public packagist.org has no OIDC Trusted Publishing option today — OIDC is a Private Packagist–only feature (`packagist/artifact-publish-github-action`, GA February 2026). The package's actual publish channel is the standard `https://packagist.org/api/github` push-event webhook (`dev-*` aliases on branch push, versioned releases on tag push via `release.yml`). Migration to Private Packagist would change ally-side Composer consumption (private repo URL + token in `composer.json`) and is a commercial decision; tracking continues on Issue #11. Closes Sapper M1 Finding #2 (doctrine drift on publish channel) and resolves Issue #11 audit. **Versioning:** none (doctrine alignment, no consumer-visible behaviour).
- **Governance:** added `.github/CODEOWNERS` routing all changes to `@script-development/phpstan-warroom-admins`. A separate rule-authors team is intentionally not split out today — the admins team and the rule-design reviewer set are identical at the current shop size; revisit if the contributor base grows or rule-design review becomes a distinct concern from operational repo administration. Pairs with branch-protection update enabling `require_code_owner_reviews=true`. Closes Sapper M1 Finding #5 (no CODEOWNERS file). **Versioning:** none (governance change, no consumer-visible surface).
- **`LogRule` (BREAKING):** extended to cover the static-call shapes `Model::destroy(...)` and `Model::forceDestroy(...)` on Log-named classes. `getNodeType()` broadened from `MethodCall::class` to `CallLike::class` and `processNode` branches on `MethodCall` vs `StaticCall`. Both shapes emit the same `logRule.logModification` identifier so consumer `phpstan.neon` `ignoreErrors` entries cover the whole rule with one identifier (the previous rule's compliance teeth depended on `delete`/`forceDelete` instance shapes; on a non-soft-delete log model `Model::destroy([1])` purges and `Model::forceDestroy([1])` always purges — both slipped through). `DB::table('logs')->truncate()` is intentionally still out of scope — Builder receiver type carries no Log-named class reference and the table name lives in a string argument; matching that needs a shape-specific call-chain rule. Tracked separately. Versioning: per ADR-0021 §Versioning, this is a Major bump (new errors in code that previously passed); within 0.x this ships as `v0.3.0`. **Pre-cascade audit required across emmie, kendo, entreezuil, ublgenie before tagging** — surface any `::destroy(`/`::forceDestroy(` calls on Log-named classes and route operational-log false positives to consumer-side `phpstan.neon` `ignoreErrors` (same convention used in v0.2.0 for `ublgenie/app/Actions/DeleteBranch.php`). Resolves issue #4.
- **`LogBuilderTruncateRule` (BREAKING):** new sibling rule to `LogRule`, sharing the `logRule.logModification` identifier so consumer `phpstan.neon` `ignoreErrors` entries cover the whole append-only doctrine with one entry. Flags `Builder->truncate()` calls where the fluent chain's most recent `table()` invocation targets a Log-named table (string-literal first argument containing `'log'` / `'logs'`, case-insensitive substring match). Covers `DB::table('logs')->truncate()`, `DB::connection('central')->table('logs')->truncate()`, and `$this->db->table('logs')->truncate()` (instance-injected `ConnectionInterface`). Receiver detection is type-based (`Illuminate\Database\Query\Builder` OR `Illuminate\Database\Eloquent\Builder` subtype via `ObjectType::isSuperTypeOf()`) — mirrors `EnforceAuditSnapshotOnRetryRule`'s `ConnectionInterface` pattern. The Eloquent\Builder receiver branch covers the rare-but-coherent `$eloquentBuilder->table('logs')->truncate()` shape; Eloquent chains that set the table via the Model's `$table` property (`AuditLog::query()->truncate()`) or via Eloquent's `from()` vocabulary (`AuditLog::query()->from('logs')->truncate()`) are an acceptable miss in the same family as variable table names — the table name does not appear as a `table()`-call string-literal in the chain. Doctrine: ADR-0001 §Append-only — `truncate()` is the bluntest delete and bypasses Eloquent events, observers, and audit triggers entirely. Out of scope: variable table names (`$t = 'logs'; DB::table($t)->truncate()`), Eloquent `from('logs')` chains, and Model-property-driven tables — all would need value-flow or model-graph inspection; acceptable misses, rely on reviewer + consumer-side `phpstan.neon` `ignoreErrors`. **Versioning: per ADR-0021 §Versioning, this is a Major bump (new errors in code that previously passed); within 0.x this rolls into v0.3.0 alongside the `LogRule` static-call expansion** — both ride the same release. **Pre-cascade audit required across emmie, kendo, entreezuil, ublgenie, brick-inventory before tagging** — same convention as v0.2.0 / the LogRule static-call expansion. Surface any `DB::table('<log-named>')->truncate()` / `$db->table('<log-named>')->truncate()` calls; operational-log false positives route to consumer-side `phpstan.neon` `ignoreErrors`. Resolves issue #8.
- **CI:** added PHP 8.5 to the `ci.yml` and `release.yml` test matrices alongside 8.4 (`['8.4']` → `['8.4', '8.5']`). PHP 8.5.0 was released 2025-11-20; the war-room dev environment already runs 8.5.5 locally, so PRs were getting ad-hoc 8.5 coverage during pre-push but no CI signal. Adding (rather than replacing) keeps 8.4 — the `composer.json` `^8.4` contractual minimum — covered. `shivammathur/setup-php@v2` supports 8.5 since GA. Resolves issue #5.
- **CI:** added line-coverage measurement and a threshold gate. `ci.yml` switches `coverage: none` → `coverage: pcov` on both 8.4 and 8.5 matrix legs (PCOV is line-coverage-only and faster than Xdebug — debugger features aren't needed). New composer scripts: `test:coverage` (runs PHPUnit with `--coverage-clover=build/logs/clover.xml --coverage-text`) and `coverage:check` (runs `bin/coverage-check.php`, a standalone clover parser — no extra runtime dependency added to a static-analysis package for a single CI gate). Two new CI steps replace the `Tests` step: **Tests with coverage** and **Coverage threshold gate**. Clover XML is uploaded as a per-leg artifact (`clover-php-${{ matrix.php }}`, 14-day retention) so reviewers can inspect uncovered lines without spelunking through workflow logs. **Initial threshold: 83%** — the measured baseline is 83.92% (240/286 lines across `src/`), set 0.92 percentage points lower to absorb trivial fluctuation on equivalent-but-renamed code. Class coverage (0/6) and method coverage (39%) are intentionally unmeasured by the gate v1; per the issue's deliberation, line coverage is the right v1 signal and branch/method coverage is a follow-up after the line gate is bedded in. The 16-percentage-point gap to 100% audits as defensive guard clauses on unexpected node shapes (the kind of branch the issue itself flagged as "genuinely hard to fixture" — `LogRule`'s static-call branch falls back when `$node->class` is `Expr` rather than `Name`); a follow-up issue will audit and ratchet the threshold upward to 90%+. Versioning: none (pure CI/test-infra, no consumer-visible behaviour). Resolves issue #9.
- **CI:** added Infection mutation testing gate, layered on top of the line-coverage gate. New `infection/infection ^0.32.7` dev dependency, `infection.json5` config (`@default` mutator profile, `src/` source scope, fixtures stay out via PHPUnit's existing `<source>` block, `--testsuite=Rules`), and two new composer scripts: `mutation` (local, `--threads=max --show-mutations` for inspecting escaped mutants) and `mutation:ci` (CI: `--threads=4 --no-progress --logger-github --min-msi=75 --min-covered-msi=75` — GitHub annotations on escaped mutants surface inline in PR diffs). Two new CI steps after the coverage gate: **Mutation testing** and **Upload mutation report** (per-leg `infection-php-${{ matrix.php }}` artifact, 14-day retention). `composer config allow-plugins.infection/extension-installer true` was set to permit the framework-adapter installer plugin. **Initial thresholds: 75% MSI and 75% Covered Code MSI** — measured baseline is 78.5% MSI (241 killed / 307 mutants, 100% Mutation Code Coverage), set 3.5 percentage points lower to absorb mutator-shape fluctuation on equivalent code. Same shape as the line-coverage gate: lock in current state, audit gaps, ratchet upward. The 22% surviving-mutant population audits as a mix of (a) genuinely-equivalent mutants the issue itself anticipated — `mb_stripos` ↔ `stripos` on PSR-4 ASCII-only class names in `LogRule`, defensive guard inversions (`LogicalNot`/`IfNegation`) on early returns that filter the same nodes by either condition — and (b) genuinely-uncovered branch logic that warrants new fixtures. A follow-up issue will audit each survivor, kill where realistic, `@infection-ignore-for-mutator`-annotate where equivalent, and ratchet thresholds to the issue's target of 80% MSI / 90% Covered Code MSI. Versioning: none (pure CI/test-infra, no consumer-visible behaviour; `infection` is `require-dev` only). Resolves issue #10.
Expand Down
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev
| `ForbidDatabaseManagerInActionsRule` | ADR-0021 §Why ConnectionInterface | `forbidDatabaseManager.inAction` |
| `ForbidAbortHelperRule` | War-room §Explicit over implicit | `forbidAbortHelper.abortUsed` |
| `LogRule` | ADR-0001 §Append-only | `logRule.logModification` (covers instance `update`/`delete`/`forceDelete`/`forceDeleteQuietly`; static `Model::destroy()` / `Model::forceDestroy()` ship with v0.3.0 per `[Unreleased]`) |
| `LogBuilderTruncateRule` | ADR-0001 §Append-only | `logRule.logModification` (shared with `LogRule`; covers `Builder->truncate()` on Log-named tables — ships with v0.3.0 per `[Unreleased]`) |
| `EnforceAuditSnapshotOnRetryRule` | ADR-0001 §Snapshot-on-Retry Safety | `enforceAuditSnapshotOnRetry.firstStatementMustResetState` |
| `EnforceResourceDataValidatorOptInRule` | ADR-0009 §EAGER_LOAD validator opt-in | `enforceResourceDataValidatorOptIn.missingValidatorCall` |
| `ConnectionTransactionReturnTypeExtension` | (type extension, no rule) | — |
Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ includes:
| `ForbidDatabaseManagerInActionsRule` | `forbidDatabaseManager.inAction` | Action constructors | Constructor parameter typed `DatabaseManager` is an error. Inject `ConnectionInterface` instead. |
| `ForbidAbortHelperRule` | `forbidAbortHelper.abortUsed` | Function calls | `abort()`, `abort_if()`, `abort_unless()` are errors. Throw an explicit `HttpException` subclass instead. |
| `LogRule` | `logRule.logModification` | `update()` / `delete()` calls | If the receiver type's class name contains `"Log"` or `"logs"` (case-insensitive), error. |
| `LogBuilderTruncateRule` | `logRule.logModification` | `Builder->truncate()` calls | If the fluent chain's most recent `table()` call targets a Log-named table (string-literal argument matching `"log"` / `"logs"`, case-insensitive), error. Sibling rule to `LogRule`; shares the `logRule.logModification` identifier so a single `ignoreErrors` entry covers both. Eloquent `from()` chains and Model-`$table`-property-driven tables are acceptable misses. Doctrine: ADR-0001 §Append-only. |
| `EnforceAuditSnapshotOnRetryRule` | `enforceAuditSnapshotOnRetry.firstStatementMustResetState` | `App\Actions\*` whose constructor injects an entity audit logger | The first statement inside `$connection->transaction(...)` must reset the model's in-memory state (`$model->refresh()`, fresh fetch, or fresh instantiation). Doctrine: ADR-0001 §Snapshot-on-Retry Safety. |
| `EnforceResourceDataValidatorOptInRule` | `enforceResourceDataValidatorOptIn.missingValidatorCall` | Classes extending `App\Http\Resources\ResourceData` | If the class declares a non-empty `EAGER_LOAD_COUNT` / `EAGER_LOAD_SUM` constant but never calls `validateRelationsLoaded()` in any method, error. |

Expand All @@ -66,6 +67,8 @@ parameters:

Each ignore should carry a comment with rationale. Future versions may add an explicit allow-list parameter — file an issue if you have a recurring need.

`LogBuilderTruncateRule` shares the `logRule.logModification` identifier with `LogRule`. A single `ignoreErrors` entry keyed on `logRule.logModification` therefore covers both rules for the suppressed path.

### `EnforceResourceDataValidatorOptInRule` — configurable base class

The rule scopes to classes extending `App\Http\Resources\ResourceData` by default. If a territory ships its abstract resource base under a different FQCN, override the `resourceDataBaseClass` parameter in `phpstan.neon`:
Expand Down
3 changes: 3 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ services:
-
class: ScriptDevelopment\PhpstanWarroomRules\Rules\LogRule
tags: [phpstan.rules.rule]
-
class: ScriptDevelopment\PhpstanWarroomRules\Rules\LogBuilderTruncateRule
tags: [phpstan.rules.rule]
-
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceAuditSnapshotOnRetryRule
tags: [phpstan.rules.rule]
Expand Down
179 changes: 179 additions & 0 deletions src/Rules/LogBuilderTruncateRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
<?php

declare(strict_types = 1);

namespace ScriptDevelopment\PhpstanWarroomRules\Rules;

use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
use Illuminate\Database\Query\Builder as QueryBuilder;
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Identifier;
use PhpParser\Node\Scalar\String_;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ObjectType;

use function mb_stripos;

/**
* Forbids `Builder->truncate()` calls where the fluent chain's most recent
* `table()` invocation targets a Log-named table (string-literal first
* argument containing `"log"` / `"logs"`, case-insensitive substring match).
*
* Doctrine source: ADR-0001 §Append-only — audit records have no UPDATE, no
* DELETE; `truncate()` is the bluntest delete and bypasses Eloquent events,
* observers, and audit triggers entirely.
*
* Sibling rule to `LogRule`. Shares the `logRule.logModification` identifier
* so consumer `phpstan.neon` `ignoreErrors` entries cover the whole
* append-only doctrine with one entry. Closes the structural gap deliberately
* deferred from PR #7 (issue #8): `LogRule` matches on Log-named class
* receivers, but `DB::table('logs')->truncate()` carries no Log-named class
* reference — the receiver is `Illuminate\Database\Query\Builder` (or
* `Illuminate\Database\Eloquent\Builder` for `Model::query()->truncate()`),
* and the "Log-ness" lives in a string-literal argument to a prior `table()`
* call in the same fluent chain.
*
* Detection (all three must hold to fire):
* 1. The `MethodCall` name is `truncate`.
* 2. The receiver type is a (subtype of) `Illuminate\Database\Query\Builder`
* or `Illuminate\Database\Eloquent\Builder` — type-based via
* `ObjectType::isSuperTypeOf()`, not property-name string matching.
* 3. Walking back through the chain, the most recent `table()` call (whether
* `MethodCall` like `$db->table('x')` or `StaticCall` like
* `DB::table('x')`) has a `Scalar\String_` first argument whose value
* matches `'log'` / `'logs'` case-insensitively.
*
* Variable table names (`$t = 'logs'; DB::table($t)->truncate()`) are out of
* scope — would require value-flow analysis. Acceptable miss; rely on
* reviewer + consumer-side `phpstan.neon` `ignoreErrors`.
*
* Eloquent\Builder chains that set the table via `from('logs')` rather than
* `table('logs')` are also out of scope — `from()` is Eloquent's fluent
* vocabulary and is not recognised by the chain-walk. Model-property-driven
* tables (`$table = 'audit_logs'` on the Model class) are likewise an
* acceptable miss because the table name does not appear in the call chain.
* The Eloquent\Builder receiver-type branch remains live for the rare but
* coherent shape `$eloquentBuilder->table('logs')->truncate()`.
*
* Substring matching is intentionally broad. False positives on tables named
* `catalog`, `blog`, `terminology`, or domain tables that include `log` in
* the name should be suppressed per-territory via `phpstan.neon`
* `ignoreErrors`, scoped to the offending path. Same convention as `LogRule`.
*
* @implements Rule<MethodCall>
*/
final class LogBuilderTruncateRule implements Rule
{
private const string QUERY_BUILDER_FQCN = QueryBuilder::class;

private const string ELOQUENT_BUILDER_FQCN = EloquentBuilder::class;

private const array LOG_NEEDLES = ['log', 'logs'];

private const string TABLE_SETTING_METHOD = 'table';

public function getNodeType(): string
{
return MethodCall::class;
}

public function processNode(Node $node, Scope $scope): array
{
if (!$node->name instanceof Identifier || $node->name->toString() !== 'truncate') {
return [];
}

if (!$this->receiverIsBuilder($node, $scope)) {
return [];
}

if (!$this->chainTargetsLogNamedTable($node->var)) {
return [];
}

return [
RuleErrorBuilder::message('Logs should not be updated or deleted.')
->identifier('logRule.logModification')
->build(),
];
}

/**
* Type-based receiver gate: `$scope->getType($node->var)` must be a
* (subtype of) `Illuminate\Database\Query\Builder` or
* `Illuminate\Database\Eloquent\Builder`. Mirrors the canonical pattern
* in `EnforceAuditSnapshotOnRetryRule::receiverIsConnectionInterface()`.
*/
private function receiverIsBuilder(MethodCall $node, Scope $scope): bool
{
$receiverType = $scope->getType($node->var);
$queryBuilderType = new ObjectType(self::QUERY_BUILDER_FQCN);

if ($queryBuilderType->isSuperTypeOf($receiverType)->yes()) {
return true;
}

$eloquentBuilderType = new ObjectType(self::ELOQUENT_BUILDER_FQCN);

return $eloquentBuilderType->isSuperTypeOf($receiverType)->yes();
}

/**
* Walk back through the fluent chain looking for the most recent
* `table()` call (`MethodCall` or `StaticCall`). Inspect its first
* argument: fire on a Log-named `Scalar\String_`; otherwise (variable,
* concat, function call) do not fire. If no `table()` call is found in
* the chain, do not fire.
*/
private function chainTargetsLogNamedTable(Expr $receiver): bool
{
$current = $receiver;

while ($current instanceof MethodCall || $current instanceof StaticCall) {
if (
$current->name instanceof Identifier
&& $current->name->toString() === self::TABLE_SETTING_METHOD
) {
return $this->firstArgIsLogNamedString($current);
}

if ($current instanceof MethodCall) {
$current = $current->var;

continue;
}

// StaticCall is a root — its arguments are inspected above; if
// its name is not `table` the chain has no earlier hops to walk.
return false;
}

return false;
}

private function firstArgIsLogNamedString(MethodCall|StaticCall $call): bool
{
if (!isset($call->args[0]) || !$call->args[0] instanceof Node\Arg) {
return false;
}

$value = $call->args[0]->value;

if (!$value instanceof String_) {
return false;
}

foreach (self::LOG_NEEDLES as $needle) {
if (mb_stripos($value->value, $needle) !== false) {
return true;
}
}

return false;
}
}
Loading
Loading