Skip to content

Add Laravel 13 ObservedBy observer migration rectors#528

Open
ivanvasechko wants to merge 5 commits into
driftingly:mainfrom
ivanvasechko:feature/observed-by-attribute
Open

Add Laravel 13 ObservedBy observer migration rectors#528
ivanvasechko wants to merge 5 commits into
driftingly:mainfrom
ivanvasechko:feature/observed-by-attribute

Conversation

@ivanvasechko

@ivanvasechko ivanvasechko commented Jun 30, 2026

Copy link
Copy Markdown

Summary

Adds Laravel 13 support for migrating manual model observer registration such as:

User::observe(UserObserver::class);

into model attributes:

#[ObservedBy([UserObserver::class])]

What changed

  • adds ObserveCallsToObservedByAttributeRector to add or merge #[ObservedBy([...])] on Eloquent models
  • adds RemoveModelObserveCallsFromBootRector to remove safe direct top-level observe() calls from boot() and booted()
  • supports User::observe(...), self::observe(...), and static::observe(...)
  • removes empty boot() / booted() methods after cleanup, including cases with method attributes such as #[Override]
  • preserves unrelated logic inside boot methods
  • skips nested registrations and unsupported existing ObservedBy(...) shapes conservatively
  • wires both rules into the Laravel 13 attributes set
  • documents both rules in docs/rector_rules_overview.md

Precision / safety rules

This change is intentionally conservative:

  • only direct top-level observe() statements are removed
  • nested registrations inside conditionals, loops, or closures are left untouched
  • cleanup only runs when the target model can be updated safely
  • existing ObservedBy(...) attributes are merged and deduplicated when they are in a supported form
  • unsupported existing attribute shapes are left alone

Implementation notes

  • project-wide observer discovery now uses Rector-native file discovery instead of custom recursive scanning
  • fixture companion files were split into PSR-4-compliant files so repository tooling keeps working out of the box
  • added minimal stubs needed for analysis/tests around ObservedBy and Eloquent model observation

Tiny confession: rule started a bit vibe-coded, but ended up heavily constrained and aggressively tested. Cave paintings, but with guard rails.

Tests

Added targeted PHPUnit coverage for:

  • simple observe-to-attribute conversion
  • merging with existing ObservedBy
  • skipping duplicate existing attributes
  • skipping nested observe registrations
  • skipping non-models
  • removing direct observe calls while preserving other boot logic
  • removing empty boot() methods entirely
  • removing empty booted() methods entirely
  • handling static::observe(...) inside model booted()
  • keeping unsupported existing ObservedBy(...) forms untouched
  • Laravel 13 set-level coverage

Also ran the full local CI-equivalent verification on the repo's original tool configuration:

  • composer validate
  • composer lint
  • composer rector-dry-run
  • composer phpstan
  • composer structarmed
  • vendor/bin/phpunit

@ivanvasechko ivanvasechko marked this pull request as ready for review June 30, 2026 14:40
@calebdw

calebdw commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Things may have been updated since the last time I dealt with this, but I have run into situations where converting to the ObservedBy attribute has broken code (mainly when dealing with model inheritance). Please double check to see if this is still the case, and if so, then please write a test to ensure such a case is skipped:

class File extends Model
{
    #[Override]
    protected static function booted(): void
    {
        // cant' use ObserveredBy attribute here because
        // it doesn't work with inherited classes
        static::observe(FileObserver::class);
    }
}

class UserAvatar extends File {} // please ensure that the hooks defined in FileObserver are actually called when dealing with UserAvatar instances

@calebdw

calebdw commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

removes empty boot() / booted() methods after cleanup, including cases with method attributes such as #[Override]

This rule should not be concerned about this cleanup---there's other rector dead-code rules that perform this and we should let them clean it up

/**
* @see RemoveModelObserveCallsFromBootRectorTest
*/
final class RemoveModelObserveCallsFromBootRector extends AbstractRector

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having two separate, but dependent rules like this is a bad architecture---we should only have one rector rule that removes the call from boot/booted and adds the ObservedBy attribute, this will have to operate at the class level

What if someone skipped the other rule but forgot to skip this? Now their codebase is broken

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.

2 participants