Skip to content

[13.x] Fix DumpCommand return type. - #60934

Merged
taylorotwell merged 1 commit into
laravel:13.xfrom
KentarouTakeda:fix-dump-command-return-type
Jul 30, 2026
Merged

[13.x] Fix DumpCommand return type.#60934
taylorotwell merged 1 commit into
laravel:13.xfrom
KentarouTakeda:fix-dump-command-return-type

Conversation

@KentarouTakeda

Copy link
Copy Markdown
Contributor

This PR changes DumpCommand::handle() to return Command::SUCCESS on success and updates its DocBlock from @return void to @return int.

Since the method already returned Command::FAILURE in some cases, the previous @return void annotation was incorrect. No native return type has been added, so this introduces no breaking change.

We encountered this while overriding DumpCommand in our project. The incorrect DocBlock causes PHPStan to report an incompatible method signature, and this change resolves that inconsistency.

* @param \Illuminate\Database\ConnectionResolverInterface $connections
* @param \Illuminate\Contracts\Events\Dispatcher $dispatcher
* @return void
* @return int

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You could narrow this further down:

Suggested change
* @return int
* @return self::SUCCESS|self::FAILURE|self::INVALID

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, but I'd rather keep @return int here: handle() never returns INVALID, so that union would be inaccurate. And narrowing it further would introduce a new incompatible-signature error for subclasses that return any other exit code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How is int not inaccurate if self::INVALID is? That logic escapes me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair — sloppy wording; int is wider than what this method returns today, but not wider than what's allowed. Command::execute() casts whatever handle() returns with (int) and passes it straight through, and Symfony only caps it at 255 — the framework itself already emits other codes via --isolated=<n>. So int is the contract, and the union would document a constraint that isn't there.


$this->components->info($info.' successfully.');

return Command::SUCCESS;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return Command::SUCCESS;
return self::SUCCESS;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Every command under Database/Console uses Command::, including the Command::FAILURE a few lines above in this same method, so I'll keep it consistent.

@taylorotwell
taylorotwell merged commit c11125e into laravel:13.x Jul 30, 2026
55 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.

5 participants