Skip to content

Commit 3555946

Browse files
Merge pull request #22 from script-development/engineer/log-builder-truncate-rule
engineer: add LogBuilderTruncateRule (closes #8)
2 parents a958e3a + c929e88 commit 3555946

14 files changed

Lines changed: 430 additions & 4 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
1919
- **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).
2020
- **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).
2121
- **`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.
22+
- **`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.
2223
- **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.
2324
- **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.
2425
- **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.

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev
2424
| `ForbidDatabaseManagerInActionsRule` | ADR-0021 §Why ConnectionInterface | `forbidDatabaseManager.inAction` |
2525
| `ForbidAbortHelperRule` | War-room §Explicit over implicit | `forbidAbortHelper.abortUsed` |
2626
| `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]`) |
27+
| `LogBuilderTruncateRule` | ADR-0001 §Append-only | `logRule.logModification` (shared with `LogRule`; covers `Builder->truncate()` on Log-named tables — ships with v0.3.0 per `[Unreleased]`) |
2728
| `EnforceAuditSnapshotOnRetryRule` | ADR-0001 §Snapshot-on-Retry Safety | `enforceAuditSnapshotOnRetry.firstStatementMustResetState` |
2829
| `EnforceResourceDataValidatorOptInRule` | ADR-0009 §EAGER_LOAD validator opt-in | `enforceResourceDataValidatorOptIn.missingValidatorCall` |
2930
| `ConnectionTransactionReturnTypeExtension` | (type extension, no rule) ||

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ includes:
4141
| `ForbidDatabaseManagerInActionsRule` | `forbidDatabaseManager.inAction` | Action constructors | Constructor parameter typed `DatabaseManager` is an error. Inject `ConnectionInterface` instead. |
4242
| `ForbidAbortHelperRule` | `forbidAbortHelper.abortUsed` | Function calls | `abort()`, `abort_if()`, `abort_unless()` are errors. Throw an explicit `HttpException` subclass instead. |
4343
| `LogRule` | `logRule.logModification` | `update()` / `delete()` calls | If the receiver type's class name contains `"Log"` or `"logs"` (case-insensitive), error. |
44+
| `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. |
4445
| `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. |
4546
| `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. |
4647

@@ -66,6 +67,8 @@ parameters:
6667

6768
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.
6869

70+
`LogBuilderTruncateRule` shares the `logRule.logModification` identifier with `LogRule`. A single `ignoreErrors` entry keyed on `logRule.logModification` therefore covers both rules for the suppressed path.
71+
6972
### `EnforceResourceDataValidatorOptInRule` — configurable base class
7073

7174
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`:

extension.neon

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ services:
2121
-
2222
class: ScriptDevelopment\PhpstanWarroomRules\Rules\LogRule
2323
tags: [phpstan.rules.rule]
24+
-
25+
class: ScriptDevelopment\PhpstanWarroomRules\Rules\LogBuilderTruncateRule
26+
tags: [phpstan.rules.rule]
2427
-
2528
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceAuditSnapshotOnRetryRule
2629
tags: [phpstan.rules.rule]
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace ScriptDevelopment\PhpstanWarroomRules\Rules;
6+
7+
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
8+
use Illuminate\Database\Query\Builder as QueryBuilder;
9+
use PhpParser\Node;
10+
use PhpParser\Node\Expr;
11+
use PhpParser\Node\Expr\MethodCall;
12+
use PhpParser\Node\Expr\StaticCall;
13+
use PhpParser\Node\Identifier;
14+
use PhpParser\Node\Scalar\String_;
15+
use PHPStan\Analyser\Scope;
16+
use PHPStan\Rules\Rule;
17+
use PHPStan\Rules\RuleErrorBuilder;
18+
use PHPStan\Type\ObjectType;
19+
20+
use function mb_stripos;
21+
22+
/**
23+
* Forbids `Builder->truncate()` calls where the fluent chain's most recent
24+
* `table()` invocation targets a Log-named table (string-literal first
25+
* argument containing `"log"` / `"logs"`, case-insensitive substring match).
26+
*
27+
* Doctrine source: ADR-0001 §Append-only — audit records have no UPDATE, no
28+
* DELETE; `truncate()` is the bluntest delete and bypasses Eloquent events,
29+
* observers, and audit triggers entirely.
30+
*
31+
* Sibling rule to `LogRule`. Shares the `logRule.logModification` identifier
32+
* so consumer `phpstan.neon` `ignoreErrors` entries cover the whole
33+
* append-only doctrine with one entry. Closes the structural gap deliberately
34+
* deferred from PR #7 (issue #8): `LogRule` matches on Log-named class
35+
* receivers, but `DB::table('logs')->truncate()` carries no Log-named class
36+
* reference — the receiver is `Illuminate\Database\Query\Builder` (or
37+
* `Illuminate\Database\Eloquent\Builder` for `Model::query()->truncate()`),
38+
* and the "Log-ness" lives in a string-literal argument to a prior `table()`
39+
* call in the same fluent chain.
40+
*
41+
* Detection (all three must hold to fire):
42+
* 1. The `MethodCall` name is `truncate`.
43+
* 2. The receiver type is a (subtype of) `Illuminate\Database\Query\Builder`
44+
* or `Illuminate\Database\Eloquent\Builder` — type-based via
45+
* `ObjectType::isSuperTypeOf()`, not property-name string matching.
46+
* 3. Walking back through the chain, the most recent `table()` call (whether
47+
* `MethodCall` like `$db->table('x')` or `StaticCall` like
48+
* `DB::table('x')`) has a `Scalar\String_` first argument whose value
49+
* matches `'log'` / `'logs'` case-insensitively.
50+
*
51+
* Variable table names (`$t = 'logs'; DB::table($t)->truncate()`) are out of
52+
* scope — would require value-flow analysis. Acceptable miss; rely on
53+
* reviewer + consumer-side `phpstan.neon` `ignoreErrors`.
54+
*
55+
* Eloquent\Builder chains that set the table via `from('logs')` rather than
56+
* `table('logs')` are also out of scope — `from()` is Eloquent's fluent
57+
* vocabulary and is not recognised by the chain-walk. Model-property-driven
58+
* tables (`$table = 'audit_logs'` on the Model class) are likewise an
59+
* acceptable miss because the table name does not appear in the call chain.
60+
* The Eloquent\Builder receiver-type branch remains live for the rare but
61+
* coherent shape `$eloquentBuilder->table('logs')->truncate()`.
62+
*
63+
* Substring matching is intentionally broad. False positives on tables named
64+
* `catalog`, `blog`, `terminology`, or domain tables that include `log` in
65+
* the name should be suppressed per-territory via `phpstan.neon`
66+
* `ignoreErrors`, scoped to the offending path. Same convention as `LogRule`.
67+
*
68+
* @implements Rule<MethodCall>
69+
*/
70+
final class LogBuilderTruncateRule implements Rule
71+
{
72+
private const string QUERY_BUILDER_FQCN = QueryBuilder::class;
73+
74+
private const string ELOQUENT_BUILDER_FQCN = EloquentBuilder::class;
75+
76+
private const array LOG_NEEDLES = ['log', 'logs'];
77+
78+
private const string TABLE_SETTING_METHOD = 'table';
79+
80+
public function getNodeType(): string
81+
{
82+
return MethodCall::class;
83+
}
84+
85+
public function processNode(Node $node, Scope $scope): array
86+
{
87+
if (!$node->name instanceof Identifier || $node->name->toString() !== 'truncate') {
88+
return [];
89+
}
90+
91+
if (!$this->receiverIsBuilder($node, $scope)) {
92+
return [];
93+
}
94+
95+
if (!$this->chainTargetsLogNamedTable($node->var)) {
96+
return [];
97+
}
98+
99+
return [
100+
RuleErrorBuilder::message('Logs should not be updated or deleted.')
101+
->identifier('logRule.logModification')
102+
->build(),
103+
];
104+
}
105+
106+
/**
107+
* Type-based receiver gate: `$scope->getType($node->var)` must be a
108+
* (subtype of) `Illuminate\Database\Query\Builder` or
109+
* `Illuminate\Database\Eloquent\Builder`. Mirrors the canonical pattern
110+
* in `EnforceAuditSnapshotOnRetryRule::receiverIsConnectionInterface()`.
111+
*/
112+
private function receiverIsBuilder(MethodCall $node, Scope $scope): bool
113+
{
114+
$receiverType = $scope->getType($node->var);
115+
$queryBuilderType = new ObjectType(self::QUERY_BUILDER_FQCN);
116+
117+
if ($queryBuilderType->isSuperTypeOf($receiverType)->yes()) {
118+
return true;
119+
}
120+
121+
$eloquentBuilderType = new ObjectType(self::ELOQUENT_BUILDER_FQCN);
122+
123+
return $eloquentBuilderType->isSuperTypeOf($receiverType)->yes();
124+
}
125+
126+
/**
127+
* Walk back through the fluent chain looking for the most recent
128+
* `table()` call (`MethodCall` or `StaticCall`). Inspect its first
129+
* argument: fire on a Log-named `Scalar\String_`; otherwise (variable,
130+
* concat, function call) do not fire. If no `table()` call is found in
131+
* the chain, do not fire.
132+
*/
133+
private function chainTargetsLogNamedTable(Expr $receiver): bool
134+
{
135+
$current = $receiver;
136+
137+
while ($current instanceof MethodCall || $current instanceof StaticCall) {
138+
if (
139+
$current->name instanceof Identifier
140+
&& $current->name->toString() === self::TABLE_SETTING_METHOD
141+
) {
142+
return $this->firstArgIsLogNamedString($current);
143+
}
144+
145+
if ($current instanceof MethodCall) {
146+
$current = $current->var;
147+
148+
continue;
149+
}
150+
151+
// StaticCall is a root — its arguments are inspected above; if
152+
// its name is not `table` the chain has no earlier hops to walk.
153+
return false;
154+
}
155+
156+
return false;
157+
}
158+
159+
private function firstArgIsLogNamedString(MethodCall|StaticCall $call): bool
160+
{
161+
if (!isset($call->args[0]) || !$call->args[0] instanceof Node\Arg) {
162+
return false;
163+
}
164+
165+
$value = $call->args[0]->value;
166+
167+
if (!$value instanceof String_) {
168+
return false;
169+
}
170+
171+
foreach (self::LOG_NEEDLES as $needle) {
172+
if (mb_stripos($value->value, $needle) !== false) {
173+
return true;
174+
}
175+
}
176+
177+
return false;
178+
}
179+
}

0 commit comments

Comments
 (0)