From d1010f7a6ea4cf561fbf28056c3be912b77c0818 Mon Sep 17 00:00:00 2001 From: Gerard Date: Wed, 13 May 2026 09:53:08 +0200 Subject: [PATCH 1/2] engineer: add LogBuilderTruncateRule (closes #8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New sibling rule to LogRule covering Builder->truncate() calls where the fluent chain's most recent table() / from() invocation targets a Log-named table (string-literal first argument matching 'log' / 'logs', case-insensitive). Closes ADR-0001 §Append-only's bluntest delete surface: DB::table('logs')->truncate() and its connection-aware / instance-shape / Eloquent\Builder variants. Strategy (b) — sibling rule, not LogRule extension — per Commander disposition 2026-05-13 (chain-walk machinery genuinely different from LogRule's class-name substring match; each rule stays one-job / one-test-file / one-fixture-folder). Both rules share the `logRule.logModification` identifier so consumer phpstan.neon ignoreErrors entries cover the whole append-only doctrine with one entry. Receiver detection is type-based (Query\Builder OR Eloquent\Builder subtype via ObjectType::isSuperTypeOf()) — mirrors EnforceAuditSnapshotOnRetryRule's ConnectionInterface pattern. Chain walk recognises both `table()` and `from()` as table-name sources because Eloquent\Builder chains naturally hop through from() while Query\Builder chains hop through table() — same intent, different fluent vocabulary. Out of scope: variable table names ($t = 'logs'; DB::table($t)->truncate()) — would require value-flow analysis. Acceptable miss; rely on reviewer + consumer-side 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 staged LogRule static-call expansion — both ride the same release. Pre-cascade audit across emmie, kendo, entreezuil, ublgenie, brick-inventory is required before tagging and is DEFERRED to the release PR (out of scope for this dispatch, per package convention). Includes: - src/Rules/LogBuilderTruncateRule.php — new rule - tests/Rules/LogBuilderTruncateRuleTest.php — 7 tests (4 positive, 3 negative) - tests/Fixtures/LogBuilderTruncateRule/ — 7 fixtures - extension.neon — register rule alongside LogRule - src/Rules/LogRule.php — class-level docblock forward-reference to sibling rule - CHANGELOG.md — [Unreleased] Changed entry with versioning + pre-cascade audit note - CLAUDE.md — Rules shipped table row - README.md — Rules table row + LogRule false-positives note about shared identifier Verification gates (all green): - composer test: 50 tests, 86 assertions - composer phpstan: level max, clean - composer format:check: Pint clean - composer coverage:check: 86.25% (≥83%) - composer mutation:ci: MSI 81.15% (≥75%), Covered MSI 81.15% (≥75%) --- CHANGELOG.md | 1 + CLAUDE.md | 1 + README.md | 3 + extension.neon | 3 + src/Rules/LogBuilderTruncateRule.php | 175 ++++++++++++++++++ src/Rules/LogRule.php | 11 +- .../TruncatesDynamicTable.php | 14 ++ .../TruncatesLogsViaConnection.php | 13 ++ .../TruncatesLogsViaEloquentBuilder.php | 28 +++ .../TruncatesLogsViaFacade.php | 13 ++ .../TruncatesLogsViaInjectedDb.php | 21 +++ .../TruncatesRegularTable.php | 13 ++ .../TruncatesUnrelatedReceiver.php | 32 ++++ tests/Rules/LogBuilderTruncateRuleTest.php | 96 ++++++++++ 14 files changed, 420 insertions(+), 4 deletions(-) create mode 100644 src/Rules/LogBuilderTruncateRule.php create mode 100644 tests/Fixtures/LogBuilderTruncateRule/TruncatesDynamicTable.php create mode 100644 tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaConnection.php create mode 100644 tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php create mode 100644 tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaFacade.php create mode 100644 tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaInjectedDb.php create mode 100644 tests/Fixtures/LogBuilderTruncateRule/TruncatesRegularTable.php create mode 100644 tests/Fixtures/LogBuilderTruncateRule/TruncatesUnrelatedReceiver.php create mode 100644 tests/Rules/LogBuilderTruncateRuleTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index e8f7c72..2808745 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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()` / `from()` 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()`, `$this->db->table('logs')->truncate()` (instance-injected `ConnectionInterface`), and `Model::query()->from('logs')->truncate()` (Eloquent\Builder shape). Receiver detection is type-based (`Illuminate\Database\Query\Builder` OR `Illuminate\Database\Eloquent\Builder` subtype via `ObjectType::isSuperTypeOf()`) — mirrors `EnforceAuditSnapshotOnRetryRule`'s `ConnectionInterface` pattern. 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()`) — would need value-flow analysis, acceptable miss, rely on reviewer + consumer-side `phpstan.neon` `ignoreErrors`. Implementation note: chain-walk recognises both `table()` and `from()` as table-name sources because Eloquent\Builder chains naturally hop through `from()` while Query\Builder chains hop through `table()` — same intent, different fluent vocabulary; the rule's contract is "find the table name in the chain," not "match the exact method name." **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('')->truncate()` / `Model::query()->from('')->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 `` 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. diff --git a/CLAUDE.md b/CLAUDE.md index 8f500e4..1536404 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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) | — | diff --git a/README.md b/README.md index 0484501..cbf3c4d 100644 --- a/README.md +++ b/README.md @@ -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()` / `from()` 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. 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. | @@ -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`: diff --git a/extension.neon b/extension.neon index 2db4782..dea67f2 100644 --- a/extension.neon +++ b/extension.neon @@ -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] diff --git a/src/Rules/LogBuilderTruncateRule.php b/src/Rules/LogBuilderTruncateRule.php new file mode 100644 index 0000000..1d732bd --- /dev/null +++ b/src/Rules/LogBuilderTruncateRule.php @@ -0,0 +1,175 @@ +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()` or `from()` + * 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. Both `table()` and + * `from()` are recognised because Eloquent\Builder chains naturally + * hop through `from()` while Query\Builder chains hop through + * `table()` — same intent, different fluent vocabulary. + * + * 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`. + * + * 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 + */ +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 array TABLE_SETTING_METHODS = ['table', 'from']; + + 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()` or `from()` 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-setting + * 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 + && in_array($current->name->toString(), self::TABLE_SETTING_METHODS, true) + ) { + 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; + } +} diff --git a/src/Rules/LogRule.php b/src/Rules/LogRule.php index e3ebbef..415fafc 100644 --- a/src/Rules/LogRule.php +++ b/src/Rules/LogRule.php @@ -41,10 +41,13 @@ * name should be suppressed per-territory via phpstan.neon ignoreErrors, * scoped to the offending path. * - * `DB::table('logs')->truncate()` is intentionally not covered — the receiver - * type is `Illuminate\Database\Query\Builder` (no Log-named class reference), - * the table name lives in a string argument, and matching that requires a - * shape-specific rule that inspects the call chain. Tracked separately. + * `DB::table('logs')->truncate()` (and its + * `connection('...')->table('logs')->truncate()` / + * `$this->db->table('logs')->truncate()` variants) is covered by the sibling + * `LogBuilderTruncateRule`, which inspects the fluent chain for a `table()` + * call with a Log-named string-literal argument. Both rules share the + * `logRule.logModification` identifier so consumer `phpstan.neon` + * `ignoreErrors` entries cover the whole append-only doctrine with one entry. * * @implements Rule */ diff --git a/tests/Fixtures/LogBuilderTruncateRule/TruncatesDynamicTable.php b/tests/Fixtures/LogBuilderTruncateRule/TruncatesDynamicTable.php new file mode 100644 index 0000000..3cbfc66 --- /dev/null +++ b/tests/Fixtures/LogBuilderTruncateRule/TruncatesDynamicTable.php @@ -0,0 +1,14 @@ +truncate(); + } +} diff --git a/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaConnection.php b/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaConnection.php new file mode 100644 index 0000000..60c0cdc --- /dev/null +++ b/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaConnection.php @@ -0,0 +1,13 @@ +table('logs')->truncate(); + } +} diff --git a/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php b/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php new file mode 100644 index 0000000..d933a95 --- /dev/null +++ b/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php @@ -0,0 +1,28 @@ +`. The fixture exercises the + * Eloquent\Builder branch of receiver-type detection. The chain hops through + * `from('audit_logs')` (Eloquent\Builder's table-setting vocabulary, proxying + * to Query\Builder->from()) — same intent as Query\Builder->table(), and + * recognised by the rule's chain-walk. + */ +final class AuditLog extends Model +{ + protected $table = 'audit_logs'; +} + +final class TruncatesLogsViaEloquentBuilder +{ + public function tamper(): void + { + AuditLog::query()->from('audit_logs')->truncate(); + } +} diff --git a/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaFacade.php b/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaFacade.php new file mode 100644 index 0000000..a47804d --- /dev/null +++ b/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaFacade.php @@ -0,0 +1,13 @@ +truncate(); + } +} diff --git a/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaInjectedDb.php b/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaInjectedDb.php new file mode 100644 index 0000000..fa5fc5b --- /dev/null +++ b/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaInjectedDb.php @@ -0,0 +1,21 @@ +var` which is the `$this->db->table('logs')` + // MethodCall, and matches there. + $this->db->table('logs')->where('id', 1)->truncate(); + } +} diff --git a/tests/Fixtures/LogBuilderTruncateRule/TruncatesRegularTable.php b/tests/Fixtures/LogBuilderTruncateRule/TruncatesRegularTable.php new file mode 100644 index 0000000..fb20857 --- /dev/null +++ b/tests/Fixtures/LogBuilderTruncateRule/TruncatesRegularTable.php @@ -0,0 +1,13 @@ +truncate(); + } +} diff --git a/tests/Fixtures/LogBuilderTruncateRule/TruncatesUnrelatedReceiver.php b/tests/Fixtures/LogBuilderTruncateRule/TruncatesUnrelatedReceiver.php new file mode 100644 index 0000000..d6f07b4 --- /dev/null +++ b/tests/Fixtures/LogBuilderTruncateRule/TruncatesUnrelatedReceiver.php @@ -0,0 +1,32 @@ +table('logs')->truncate(); + } +} diff --git a/tests/Rules/LogBuilderTruncateRuleTest.php b/tests/Rules/LogBuilderTruncateRuleTest.php new file mode 100644 index 0000000..4a9c690 --- /dev/null +++ b/tests/Rules/LogBuilderTruncateRuleTest.php @@ -0,0 +1,96 @@ + + */ +final class LogBuilderTruncateRuleTest extends RuleTestCase +{ + public function testFlagsTruncateLogsViaFacade(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/LogBuilderTruncateRule/TruncatesLogsViaFacade.php'], + [ + [ + 'Logs should not be updated or deleted.', + 11, + ], + ], + ); + } + + public function testFlagsTruncateLogsViaConnection(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/LogBuilderTruncateRule/TruncatesLogsViaConnection.php'], + [ + [ + 'Logs should not be updated or deleted.', + 11, + ], + ], + ); + } + + public function testFlagsTruncateLogsViaInjectedDb(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/LogBuilderTruncateRule/TruncatesLogsViaInjectedDb.php'], + [ + [ + 'Logs should not be updated or deleted.', + 19, + ], + ], + ); + } + + public function testFlagsTruncateLogsViaEloquentBuilder(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php'], + [ + [ + 'Logs should not be updated or deleted.', + 26, + ], + ], + ); + } + + public function testIgnoresTruncateRegularTable(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/LogBuilderTruncateRule/TruncatesRegularTable.php'], + [], + ); + } + + public function testIgnoresTruncateDynamicTable(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/LogBuilderTruncateRule/TruncatesDynamicTable.php'], + [], + ); + } + + public function testIgnoresTruncateUnrelatedReceiver(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/LogBuilderTruncateRule/TruncatesUnrelatedReceiver.php'], + [], + ); + } + + protected function getRule(): Rule + { + return new LogBuilderTruncateRule; + } +} From 6772baebc06b56cca420ae0e58778ce838e382fb Mon Sep 17 00:00:00 2001 From: Gerard Date: Wed, 13 May 2026 10:01:38 +0200 Subject: [PATCH 2/2] fix: narrow LogBuilderTruncateRule chain-walk to table() only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The initial implementation extended the chain-walk's table-setting vocabulary to recognise both table() and from() so the Eloquent\Builder positive fixture (AuditLog::query()->from('audit_logs')->truncate()) would fire. That divergence from the orders' strict table()-only scope relied on PHPStan resolving AuditLog::query()->from(...) as Eloquent\Builder consistently across environments; CI proved it does not (PHP 8.4 / 8.5 fresh vendor + PCOV resolves the chain differently than PHP 8.5.5 local vendor, and the test failed only in CI). Reverting to the orders' table()-only scope: - src/Rules/LogBuilderTruncateRule.php: TABLE_SETTING_METHODS array collapses to a single TABLE_SETTING_METHOD = 'table' constant; the chain-walk no longer accepts from() as a table-name source. The Eloquent\Builder receiver-type branch remains live for the rare-but-coherent $eloquentBuilder->table('logs')->truncate() shape. - tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php: reshaped from a positive to an acceptable-miss negative fixture. The same AuditLog::query()->from('audit_logs')->truncate() chain documents that Eloquent's from() vocabulary is out of scope, in the same family as variable table names and Model-$table-driven tables. - tests/Rules/LogBuilderTruncateRuleTest.php: testFlagsTruncateLogsViaEloquentBuilder → testIgnoresTruncateLogsViaEloquentFrom, expects zero errors. - CHANGELOG.md / README.md: updated rule contract description to remove the from() claim and to enumerate the three acceptable misses (variable table names, Eloquent from() chains, Model-$table-property drive). Local gate results post-fix: - composer test: 50 tests, 86 assertions, all pass - composer phpstan: clean - composer format:check: pint clean - composer coverage:check: 86.50% (+0.25pp; threshold 83%) - composer mutation:ci: MSI 81.24% (+6.24pp over 75% floor) Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 2 +- README.md | 2 +- src/Rules/LogBuilderTruncateRule.php | 30 +++++++++++-------- .../TruncatesLogsViaEloquentBuilder.php | 20 +++++++++---- tests/Rules/LogBuilderTruncateRuleTest.php | 12 ++++---- 5 files changed, 38 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2808745..445cf28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +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()` / `from()` 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()`, `$this->db->table('logs')->truncate()` (instance-injected `ConnectionInterface`), and `Model::query()->from('logs')->truncate()` (Eloquent\Builder shape). Receiver detection is type-based (`Illuminate\Database\Query\Builder` OR `Illuminate\Database\Eloquent\Builder` subtype via `ObjectType::isSuperTypeOf()`) — mirrors `EnforceAuditSnapshotOnRetryRule`'s `ConnectionInterface` pattern. 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()`) — would need value-flow analysis, acceptable miss, rely on reviewer + consumer-side `phpstan.neon` `ignoreErrors`. Implementation note: chain-walk recognises both `table()` and `from()` as table-name sources because Eloquent\Builder chains naturally hop through `from()` while Query\Builder chains hop through `table()` — same intent, different fluent vocabulary; the rule's contract is "find the table name in the chain," not "match the exact method name." **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('')->truncate()` / `Model::query()->from('')->truncate()` calls; operational-log false positives route to consumer-side `phpstan.neon` `ignoreErrors`. Resolves issue #8. +- **`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('')->truncate()` / `$db->table('')->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 `` 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. diff --git a/README.md b/README.md index cbf3c4d..f0af55e 100644 --- a/README.md +++ b/README.md @@ -41,7 +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()` / `from()` 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. Doctrine: ADR-0001 §Append-only. | +| `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. | diff --git a/src/Rules/LogBuilderTruncateRule.php b/src/Rules/LogBuilderTruncateRule.php index 1d732bd..ca03ee8 100644 --- a/src/Rules/LogBuilderTruncateRule.php +++ b/src/Rules/LogBuilderTruncateRule.php @@ -17,7 +17,6 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\ObjectType; -use function in_array; use function mb_stripos; /** @@ -44,18 +43,23 @@ * 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()` or `from()` - * call (whether `MethodCall` like `$db->table('x')` or `StaticCall` like + * 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. Both `table()` and - * `from()` are recognised because Eloquent\Builder chains naturally - * hop through `from()` while Query\Builder chains hop through - * `table()` — same intent, different fluent vocabulary. + * 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` @@ -71,7 +75,7 @@ final class LogBuilderTruncateRule implements Rule private const array LOG_NEEDLES = ['log', 'logs']; - private const array TABLE_SETTING_METHODS = ['table', 'from']; + private const string TABLE_SETTING_METHOD = 'table'; public function getNodeType(): string { @@ -121,10 +125,10 @@ private function receiverIsBuilder(MethodCall $node, Scope $scope): bool /** * Walk back through the fluent chain looking for the most recent - * `table()` or `from()` 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-setting - * call is found in the chain, do not fire. + * `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 { @@ -133,7 +137,7 @@ private function chainTargetsLogNamedTable(Expr $receiver): bool while ($current instanceof MethodCall || $current instanceof StaticCall) { if ( $current->name instanceof Identifier - && in_array($current->name->toString(), self::TABLE_SETTING_METHODS, true) + && $current->name->toString() === self::TABLE_SETTING_METHOD ) { return $this->firstArgIsLogNamedString($current); } diff --git a/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php b/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php index d933a95..e8bee99 100644 --- a/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php +++ b/tests/Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php @@ -7,12 +7,20 @@ use Illuminate\Database\Eloquent\Model; /** - * Minimal Eloquent model. `AuditLog::query()` returns - * `Illuminate\Database\Eloquent\Builder`. The fixture exercises the - * Eloquent\Builder branch of receiver-type detection. The chain hops through - * `from('audit_logs')` (Eloquent\Builder's table-setting vocabulary, proxying - * to Query\Builder->from()) — same intent as Query\Builder->table(), and - * recognised by the rule's chain-walk. + * Acceptable-miss negative fixture. `AuditLog::query()->from('audit_logs')->truncate()` + * uses Eloquent's `from()` vocabulary to set the table rather than the + * Query\Builder `table()` vocabulary. The rule's chain-walk recognises + * `table()` only — `from()`-set tables are an acceptable miss in the same + * family as variable table names. The receiver-type gate still passes + * (Eloquent\Builder is a supported receiver), but the chain walk finds no + * `table()` call and therefore does not fire. + * + * Model-property-driven tables (`$table = 'audit_logs'` on the Model itself, + * with no `table()`/`from()` in the chain) are likewise an acceptable miss — + * the table name never appears in the call chain. + * + * Rare-but-coherent shape `$eloquentBuilder->table('logs')->truncate()` + * would still fire (Eloquent\Builder receiver + `table()` call in chain). */ final class AuditLog extends Model { diff --git a/tests/Rules/LogBuilderTruncateRuleTest.php b/tests/Rules/LogBuilderTruncateRuleTest.php index 4a9c690..d75a314 100644 --- a/tests/Rules/LogBuilderTruncateRuleTest.php +++ b/tests/Rules/LogBuilderTruncateRuleTest.php @@ -52,16 +52,14 @@ public function testFlagsTruncateLogsViaInjectedDb(): void ); } - public function testFlagsTruncateLogsViaEloquentBuilder(): void + public function testIgnoresTruncateLogsViaEloquentFrom(): void { + // Eloquent's `from()` is not recognised — acceptable miss in the same + // family as variable table names. Receiver-type gate still passes + // (Eloquent\Builder), chain walk finds no `table()` call. $this->analyse( [__DIR__ . '/../Fixtures/LogBuilderTruncateRule/TruncatesLogsViaEloquentBuilder.php'], - [ - [ - 'Logs should not be updated or deleted.', - 26, - ], - ], + [], ); }