From 46fca5385fb4d8443d7dde420f2359c34143e3cd Mon Sep 17 00:00:00 2001 From: Brandon Ferens Date: Mon, 14 Jul 2025 08:39:19 -0700 Subject: [PATCH 1/2] Improves action handling and static analysis Refactors the action handling logic for better exception management and event dispatching. Adds static analysis configuration for improved code quality and maintainability. Updates dependencies and scripts for better development workflow. --- composer.json | 7 ++- phpstan.neon.dist | 12 ++++ src/Action.php | 55 +++++++++++-------- src/ActionsServiceProvider.php | 27 +++++++-- src/Commands/stubs/Action.stub | 12 +--- src/Traits/CanAct.php | 1 - src/helpers.php | 12 +++- .../Fixtures/Actions/ActionWithAllEvents.php | 15 +---- .../Actions/ActionWithCustomException.php | 6 +- .../Fixtures/Actions/ActionWithException.php | 2 - tests/Fixtures/Actions/ActionWithNoEvents.php | 7 +-- .../Actions/ActionWithOnlyAfterEvent.php | 11 +--- .../Actions/ActionWithOnlyBeforeEvent.php | 11 +--- .../Actions/ActionWithoutInterface.php | 7 +-- tests/Fixtures/Events/AfterEvent.php | 18 ------ tests/Fixtures/Events/BeforeEvent.php | 18 ------ tests/Fixtures/Events/FailedEvent.php | 18 ------ 17 files changed, 91 insertions(+), 148 deletions(-) create mode 100644 phpstan.neon.dist diff --git a/composer.json b/composer.json index cd758f5..b85fdc5 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ }, "require-dev": { "laravel/pint": "^1.2", - "nunomaduro/larastan": "^2.0", + "larastan/larastan": "^2.0", "orchestra/testbench": "^7.11.0", "phpunit/phpunit": "^9.5.10" }, @@ -41,13 +41,16 @@ }, "scripts": { "larastan": [ - "./vendor/bin/phpstan analyse" + "./vendor/bin/phpstan analyse src" ], "pint": [ "./vendor/bin/pint" ], "pint-check": [ "./vendor/bin/pint --test" + ], + "test": [ + "./vendor/bin/phpunit tests" ] }, "config": { diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 0000000..064393b --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,12 @@ +includes: + - vendor/larastan/larastan/extension.neon + +parameters: + paths: + - config + - src + + level: max + + checkMissingIterableValueType: true + treatPhpDocTypesAsCertain: false diff --git a/src/Action.php b/src/Action.php index 7ce5d31..e2a7ee2 100644 --- a/src/Action.php +++ b/src/Action.php @@ -2,6 +2,7 @@ namespace Kirschbaum\Actions; +use Exception; use Illuminate\Support\Traits\Macroable; use Kirschbaum\Actions\Contracts\Actionable; use Kirschbaum\Actions\Exceptions\ActionableInterfaceNotFoundException; @@ -13,6 +14,8 @@ class Action /** * Arguments to pass into the action's constructor. + * + * @var array */ protected array $arguments; @@ -21,7 +24,7 @@ class Action * * @param mixed ...$arguments * - * @return mixed + * @return mixed|void * * @throws Throwable */ @@ -35,6 +38,9 @@ public function act(string $action, ...$arguments) /** * Initiate the given action if the given condition is true. * + * @template TValue + * + * @param TValue $condition * @param mixed ...$arguments * * @return mixed|void @@ -53,6 +59,9 @@ public function actWhen($condition, string $action, ...$arguments) /** * Initiate the action if the given condition is false. * + * @template TValue + * + * @param TValue $condition * @param mixed ...$arguments * * @return mixed|void @@ -80,7 +89,11 @@ protected function handle(string $action) { $action = new $action(...$this->arguments); - $this->checkActionForInterface($action); + throw_unless( + $action instanceof Actionable, + ActionableInterfaceNotFoundException::class + ); + $this->raiseBeforeActionEvent($action); try { @@ -103,20 +116,7 @@ protected function actionHasFailedMethod(Actionable $action): bool } /** - * Determine if the action has the proper interface. - * - * @throws Throwable - */ - protected function checkActionForInterface($action): void - { - throw_unless( - $action instanceof Actionable, - ActionableInterfaceNotFoundException::class - ); - } - - /** - * Dispatch appropriate action event. + * Dispatch the appropriate action event. */ protected function dispatchEvent(string $event, Actionable $action): void { @@ -140,19 +140,28 @@ protected function eventExists(Actionable $action, string $event): bool /** * Fire failure event and/or call failed action method if they exist. * - * - * @return mixed + * @return mixed|void * * @throws Throwable */ protected function handleFailure(Actionable $action, Throwable $exception) { if ($this->actionHasFailedMethod($action)) { - return $action->failed($exception); + /** + * @var callable $callback + */ + $callback = [$action, 'failed']; + + return call_user_func($callback, $exception); } if ($this->hasCustomException($action)) { - $exception = $action->exception; + $properties = get_object_vars($action); + + /** + * @var Exception $exception + */ + $exception = $properties['exception']; throw new $exception(); } @@ -161,7 +170,7 @@ protected function handleFailure(Actionable $action, Throwable $exception) } /** - * Check if action has a custom exception. + * Check if the action has a custom exception. */ protected function hasCustomException(Actionable $action): bool { @@ -170,7 +179,7 @@ protected function hasCustomException(Actionable $action): bool } /** - * Raise the before action event. + * Raise the "before" action event. */ protected function raiseBeforeActionEvent(Actionable $action): void { @@ -178,7 +187,7 @@ protected function raiseBeforeActionEvent(Actionable $action): void } /** - * Raise the after action event. + * Raise the "after" action event. */ protected function raiseAfterActionEvent(Actionable $action): void { diff --git a/src/ActionsServiceProvider.php b/src/ActionsServiceProvider.php index 16cd3c9..dca5de6 100644 --- a/src/ActionsServiceProvider.php +++ b/src/ActionsServiceProvider.php @@ -13,11 +13,11 @@ class ActionsServiceProvider extends ServiceProvider { /** - * All of the container bindings that should be registered. + * All the container bindings that should be registered. * - * @var array + * @var array|string,class-string> */ - public $bindings = [ + public array $bindings = [ 'actions' => Action::class, Action::class => Action::class, ]; @@ -49,6 +49,8 @@ public function boot(): void /** * Get the services provided by the provider. + * + * @return list> */ public function provides(): array { @@ -61,6 +63,9 @@ public function provides(): array protected function bootActionMacro(): void { Action::macro('getMacro', function (string $name): callable|object { + /** + * @phpstan-ignore-next-line + */ return static::$macros[$name]; }); } @@ -72,9 +77,14 @@ protected function bootActionMacro(): void */ protected function bootAutoDiscoverActions(): void { - $paths = collect(config('laravel-actions.paths')) + /** + * @var list $configPaths + */ + $configPaths = config('laravel-actions.paths'); + + $paths = collect($configPaths) ->unique() - ->filter(function ($path) { + ->filter(function (string $path): bool { return is_dir($path); }); @@ -82,7 +92,12 @@ protected function bootAutoDiscoverActions(): void return; } - foreach ((new Finder())->in($paths->toArray())->files() as $action) { + /** + * @var list $dirs + */ + $dirs = $paths->toArray(); + + foreach ((new Finder())->in($dirs)->files() as $action) { if (preg_match('#(namespace)(\\s+)([A-Za-z0-9\\\\]+?)(\\s*);#sm', $action->getContents(), $namespaceMatches)) { $action = (string) Str::of($namespaceMatches[3]) ->finish('\\') diff --git a/src/Commands/stubs/Action.stub b/src/Commands/stubs/Action.stub index 0260d95..6c37493 100644 --- a/src/Commands/stubs/Action.stub +++ b/src/Commands/stubs/Action.stub @@ -11,22 +11,16 @@ class DummyClass implements Actionable /** * Event to dispatch before action starts. - * - * @var string */ - public $before = ''; + public string $before = ''; /** * Event to dispatch after action completes. - * - * @var string */ - public $after = ''; + public string $after = ''; /** * Create a new action instance. - * - * @return void */ public function __construct() { @@ -35,8 +29,6 @@ class DummyClass implements Actionable /** * Execute the action. - * - * @return mixed */ public function __invoke() { diff --git a/src/Traits/CanAct.php b/src/Traits/CanAct.php index c06b45f..6e05169 100644 --- a/src/Traits/CanAct.php +++ b/src/Traits/CanAct.php @@ -11,7 +11,6 @@ trait CanAct /** * Handles static method calls by passing them to the Action class. * - * * @return mixed|void */ public static function __callStatic(string $name, array $arguments) diff --git a/src/helpers.php b/src/helpers.php index 1cb3f36..592d147 100644 --- a/src/helpers.php +++ b/src/helpers.php @@ -8,7 +8,7 @@ * * @param mixed ...$arguments * - * @return mixed + * @return mixed|void * * @throws Throwable */ @@ -22,9 +22,12 @@ function act(string $action, ...$arguments) /** * Initiate the given action if the given condition is true. * + * @template TValue + * + * @param TValue $condition * @param mixed ...$arguments * - * @return mixed + * @return mixed|void * * @throws Throwable */ @@ -38,9 +41,12 @@ function act_when($condition, string $action, ...$arguments) /** * Initiate the given action if the given condition is false. * + * @template TValue + * + * @param TValue $condition * @param mixed ...$arguments * - * @return mixed + * @return mixed|void * * @throws Throwable */ diff --git a/tests/Fixtures/Actions/ActionWithAllEvents.php b/tests/Fixtures/Actions/ActionWithAllEvents.php index 478321c..5574374 100644 --- a/tests/Fixtures/Actions/ActionWithAllEvents.php +++ b/tests/Fixtures/Actions/ActionWithAllEvents.php @@ -6,7 +6,6 @@ use Kirschbaum\Actions\Traits\CanAct; use Tests\Fixtures\Events\AfterEvent; use Tests\Fixtures\Events\BeforeEvent; -use Throwable; class ActionWithAllEvents implements Actionable { @@ -14,26 +13,18 @@ class ActionWithAllEvents implements Actionable /** * Event to dispatch before action starts. - * - * @var string */ - public $before = BeforeEvent::class; + public string $before = BeforeEvent::class; /** * Event to dispatch after action completes. - * - * @var string */ - public $after = AfterEvent::class; + public string $after = AfterEvent::class; /** * Execute the action. - * - * @return mixed - * - * @throws Throwable */ - public function __invoke() + public function __invoke(): bool { return true; } diff --git a/tests/Fixtures/Actions/ActionWithCustomException.php b/tests/Fixtures/Actions/ActionWithCustomException.php index 9ea4209..86dc831 100644 --- a/tests/Fixtures/Actions/ActionWithCustomException.php +++ b/tests/Fixtures/Actions/ActionWithCustomException.php @@ -14,16 +14,12 @@ class ActionWithCustomException implements Actionable /** * Event to dispatch if action throws an exception. - * - * @var string */ - public $exception = CustomFailedException::class; + public string $exception = CustomFailedException::class; /** * Execute the action. * - * @return mixed - * * @throws Throwable */ public function __invoke() diff --git a/tests/Fixtures/Actions/ActionWithException.php b/tests/Fixtures/Actions/ActionWithException.php index 029e5d0..1fb074e 100644 --- a/tests/Fixtures/Actions/ActionWithException.php +++ b/tests/Fixtures/Actions/ActionWithException.php @@ -14,8 +14,6 @@ class ActionWithException implements Actionable /** * Execute the action. * - * @return mixed - * * @throws Throwable */ public function __invoke() diff --git a/tests/Fixtures/Actions/ActionWithNoEvents.php b/tests/Fixtures/Actions/ActionWithNoEvents.php index e00451c..c2dc4e2 100644 --- a/tests/Fixtures/Actions/ActionWithNoEvents.php +++ b/tests/Fixtures/Actions/ActionWithNoEvents.php @@ -4,7 +4,6 @@ use Kirschbaum\Actions\Contracts\Actionable; use Kirschbaum\Actions\Traits\CanAct; -use Throwable; class ActionWithNoEvents implements Actionable { @@ -12,12 +11,8 @@ class ActionWithNoEvents implements Actionable /** * Execute the action. - * - * @return mixed - * - * @throws Throwable */ - public function __invoke() + public function __invoke(): bool { return true; } diff --git a/tests/Fixtures/Actions/ActionWithOnlyAfterEvent.php b/tests/Fixtures/Actions/ActionWithOnlyAfterEvent.php index da8b1cc..5adcf0f 100644 --- a/tests/Fixtures/Actions/ActionWithOnlyAfterEvent.php +++ b/tests/Fixtures/Actions/ActionWithOnlyAfterEvent.php @@ -5,7 +5,6 @@ use Kirschbaum\Actions\Contracts\Actionable; use Kirschbaum\Actions\Traits\CanAct; use Tests\Fixtures\Events\AfterEvent; -use Throwable; class ActionWithOnlyAfterEvent implements Actionable { @@ -13,19 +12,13 @@ class ActionWithOnlyAfterEvent implements Actionable /** * Event to dispatch after action completes. - * - * @var string */ - public $after = AfterEvent::class; + public string $after = AfterEvent::class; /** * Execute the action. - * - * @return mixed - * - * @throws Throwable */ - public function __invoke() + public function __invoke(): bool { return true; } diff --git a/tests/Fixtures/Actions/ActionWithOnlyBeforeEvent.php b/tests/Fixtures/Actions/ActionWithOnlyBeforeEvent.php index 4c92a7f..27f006d 100644 --- a/tests/Fixtures/Actions/ActionWithOnlyBeforeEvent.php +++ b/tests/Fixtures/Actions/ActionWithOnlyBeforeEvent.php @@ -5,7 +5,6 @@ use Kirschbaum\Actions\Contracts\Actionable; use Kirschbaum\Actions\Traits\CanAct; use Tests\Fixtures\Events\BeforeEvent; -use Throwable; class ActionWithOnlyBeforeEvent implements Actionable { @@ -13,19 +12,13 @@ class ActionWithOnlyBeforeEvent implements Actionable /** * Event to dispatch before action starts. - * - * @var string */ - public $before = BeforeEvent::class; + public string $before = BeforeEvent::class; /** * Execute the action. - * - * @return mixed - * - * @throws Throwable */ - public function __invoke() + public function __invoke(): bool { return true; } diff --git a/tests/Fixtures/Actions/ActionWithoutInterface.php b/tests/Fixtures/Actions/ActionWithoutInterface.php index 3b3dbc9..afb9147 100644 --- a/tests/Fixtures/Actions/ActionWithoutInterface.php +++ b/tests/Fixtures/Actions/ActionWithoutInterface.php @@ -3,7 +3,6 @@ namespace Tests\Fixtures\Actions; use Kirschbaum\Actions\Traits\CanAct; -use Throwable; class ActionWithoutInterface { @@ -11,12 +10,8 @@ class ActionWithoutInterface /** * Execute the action. - * - * @return mixed - * - * @throws Throwable */ - public function __invoke() + public function __invoke(): void { // We will never get here. } diff --git a/tests/Fixtures/Events/AfterEvent.php b/tests/Fixtures/Events/AfterEvent.php index 7b0d311..96b7c9f 100644 --- a/tests/Fixtures/Events/AfterEvent.php +++ b/tests/Fixtures/Events/AfterEvent.php @@ -3,7 +3,6 @@ namespace Tests\Fixtures\Events; use Illuminate\Broadcasting\InteractsWithSockets; -use Illuminate\Broadcasting\PrivateChannel; use Illuminate\Foundation\Events\Dispatchable; use Illuminate\Queue\SerializesModels; @@ -12,21 +11,4 @@ class AfterEvent use Dispatchable; use InteractsWithSockets; use SerializesModels; - - /** - * Create a new event instance. - * - * @return void - */ - public function __construct() {} - - /** - * Get the channels the event should broadcast on. - * - * @return \Illuminate\Broadcasting\Channel|array - */ - public function broadcastOn() - { - return new PrivateChannel('channel-name'); - } } diff --git a/tests/Fixtures/Events/BeforeEvent.php b/tests/Fixtures/Events/BeforeEvent.php index bcdb3ed..9606f48 100644 --- a/tests/Fixtures/Events/BeforeEvent.php +++ b/tests/Fixtures/Events/BeforeEvent.php @@ -3,7 +3,6 @@ namespace Tests\Fixtures\Events; use Illuminate\Broadcasting\InteractsWithSockets; -use Illuminate\Broadcasting\PrivateChannel; use Illuminate\Foundation\Events\Dispatchable; use Illuminate\Queue\SerializesModels; @@ -12,21 +11,4 @@ class BeforeEvent use Dispatchable; use InteractsWithSockets; use SerializesModels; - - /** - * Create a new event instance. - * - * @return void - */ - public function __construct() {} - - /** - * Get the channels the event should broadcast on. - * - * @return \Illuminate\Broadcasting\Channel|array - */ - public function broadcastOn() - { - return new PrivateChannel('channel-name'); - } } diff --git a/tests/Fixtures/Events/FailedEvent.php b/tests/Fixtures/Events/FailedEvent.php index 326dabe..b1523a8 100644 --- a/tests/Fixtures/Events/FailedEvent.php +++ b/tests/Fixtures/Events/FailedEvent.php @@ -3,7 +3,6 @@ namespace Tests\Fixtures\Events; use Illuminate\Broadcasting\InteractsWithSockets; -use Illuminate\Broadcasting\PrivateChannel; use Illuminate\Foundation\Events\Dispatchable; use Illuminate\Queue\SerializesModels; @@ -12,21 +11,4 @@ class FailedEvent use Dispatchable; use InteractsWithSockets; use SerializesModels; - - /** - * Create a new event instance. - * - * @return void - */ - public function __construct() {} - - /** - * Get the channels the event should broadcast on. - * - * @return \Illuminate\Broadcasting\Channel|array - */ - public function broadcastOn() - { - return new PrivateChannel('channel-name'); - } } From 88401f7812495f7bfa8ace69cf075f4294b83072 Mon Sep 17 00:00:00 2001 From: Brandon Ferens Date: Mon, 14 Jul 2025 08:44:09 -0700 Subject: [PATCH 2/2] Updates Larastan package name in workflow Updates the package name for Larastan in the test workflow due to a change in the package's vendor. --- .github/workflows/run-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index d936b2a..88ca9be 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -167,7 +167,7 @@ jobs: composer require "laravel/framework:${{ matrix.laravel }}" --no-interaction --no-update composer require "phpunit/phpunit:${{ matrix.phpunit }}" --no-interaction --no-update composer require "orchestra/testbench:${{ matrix.testbench }}" --no-interaction --no-update - composer require "nunomaduro/larastan:${{ matrix.larastan }}" --no-interaction --no-update + composer require "larastan/larastan:${{ matrix.larastan }}" --no-interaction --no-update composer update --prefer-dist --no-interaction composer dump