Skip to content

Feat: PHP 8.4, Laravel 12/13, a test suite and CI - #3

Merged
khanzadimahdi merged 1 commit into
masterfrom
modernize/php-8.4-and-laravel-12-13
Aug 16, 2026
Merged

khanzadimahdi merged 1 commit into
masterfrom
modernize/php-8.4-and-laravel-12-13

Conversation

@khanzadimahdi

Copy link
Copy Markdown
Member

Part of the modernization series across the shetabit packages.

The dynamic scopes were half broken

__call() recognised Post::unpublished() and then forwarded the method name to the scope instead of the stamp name:

return $this->forwardCallTo($this->newQuery(), $methodName, [$method]);
//                                                           ^^^^^^^ 'unpublished'

scopeUnstamped($query, 'unpublished') then looked unpublished up in the stamp list, found nothing, and ran whereNull(null). The positive form only ever worked because there the method name happens to equal the stamp name.

Two more in the same helper:

  • getStampScope() compared a lower-cased method name against the stamp names as declared, so a stamp named isVerified had no working scope at all.
  • It looped over every stamp without stopping at the one it matched, so a later stamp could overwrite an earlier match and a scope's meaning depended on declaration order.

A stamp that does not exist used to fail quietly

isStampedBy('typo') answered false — which reads as "this record is not published" for what is really a typo. markAsStamped(), markAsUnstamped() and both scopes raised an undefined array key and then wrote to, or queried, a column named null.

They throw StampNotFoundException now, and the message names the stamps that do exist. hasStamp() asks the same question without throwing, and getStampField() resolves a stamp to its column.

Also

  • The private getFreshTimestamp() is gone; the model's own freshTimestamp() is used, so a stamp is written in the same format as created_at instead of a date() string when outside Laravel.
  • __call() no longer carries an unreachable branch for increment/decrement that would have called itself until the stack ran out. A test covers that both still work, along with relations, through-calls, dynamic wheres and plain forwarding.
  • 44 tests — real Eloquent models on an in-memory sqlite database through Testbench — at 100% line coverage. PHPStan level 7 with larastan, no baseline.
  • Three workflows (tests matrix, code style, static analysis), a Dockerfile with pcov and a Makefile.

Breaking

PHP 8.4 minimum (was 7.1) and Laravel 12/13 only (was 5.1+). Every method of Contracts\Stampable and of the trait declares its types, so an implementation of the contract has to declare compatible ones. An unknown stamp throws instead of failing quietly. illuminate/database is now required explicitly — the trait has always used Eloquent's query builder.

Verified

composer ci green on PHP 8.4 and 8.5 × lowest and highest (4 runs), rector clean, composer validate --strict passes.

🤖 Generated with Claude Code

The dynamic scopes were half broken. `Post::unpublished()` forwarded the
method name to the scope instead of the stamp name, so the scope looked
up `unpublished` in the stamp list, found nothing, and queried a column
called null; the positive form only worked because there the method name
happens to equal the stamp name. A stamp whose name is not all lower
case had no working scope at all, and a scope's meaning depended on the
order the stamps were declared in.

A stamp the model does not declare used to fail quietly: the two
questions answered false — which reads as "this record is not
published" for what is really a typo — and the two writes and the two
scopes raised an undefined array key and then wrote to, or queried, a
column named null. It throws a StampNotFoundException now, naming the
stamps that do exist.

44 tests, on real Eloquent models on sqlite through Testbench, at 100%
line coverage. The package requires PHP 8.4 and Laravel 12 or 13 now,
declares types throughout, and is checked by GitHub Actions across
PHP 8.4/8.5 x Laravel 12/13 x lowest/highest dependencies.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@khanzadimahdi
khanzadimahdi merged commit faf5559 into master Aug 16, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant