Skip to content

[13.x] Consolidate Artisan command $name/getArguments()/getOptions() into $signature - #60926

Merged
taylorotwell merged 3 commits into
laravel:13.xfrom
lucasmichot:feat/command-signature-tests
Jul 30, 2026
Merged

[13.x] Consolidate Artisan command $name/getArguments()/getOptions() into $signature#60926
taylorotwell merged 3 commits into
laravel:13.xfrom
lucasmichot:feat/command-signature-tests

Conversation

@lucasmichot

@lucasmichot lucasmichot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds characterization tests f681028 that record every Artisan command's name, aliases, arguments, and options. These tests pass both before and after the refactor commit, so the conversion does not change behavior.
  • Converts commands from the separate $name + getArguments()/getOptions() style to the single $signature string.

Excluded commands

These commands keep the old array-based style, because each has an option whose value is required, and the $signature DSL can only make option values optional. Supporting required values would need a ! marker in the signature DSL, which was proposed but not merged in #60888:

  • migrate:rollback (--batch)
  • make:component (--path)
  • dev:list (--filter)
  • make:controller (--type)

Snapshots the name, aliases, arguments, and options of every Artisan
command into a data-provider test, so a future refactor consolidating
each command's $name/getArguments()/getOptions() into a single
$signature string can be verified to leave behavior unchanged.
@github-actions

Copy link
Copy Markdown

Thanks for submitting a PR!

Note that draft PRs are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@lucasmichot lucasmichot changed the title Consolidate Artisan command $name/getArguments()/getOptions() into $signature [13.x] Consolidate Artisan command $name/getArguments()/getOptions() into $signature Jul 29, 2026
Converts most framework Artisan commands that used the separate
$name + getArguments()/getOptions() style over to the single
$signature string DSL, as verified by CommandSignatureTest.

Commands with a genuinely required-value option (migrate:rollback
--batch, make:component --path, dev:list --filter, make:controller
--type) are left on the array-based $name/getOptions() style, since
the signature DSL can only ever produce VALUE_OPTIONAL options.

Command gained a configureDefaults() hook, called once the
argument/option definition is built, for defaults that can't be
expressed as static signature text: environment-dependent values
(ServeCommand's --host/--port) and non-string literal defaults
(StatusCommand's --pending and MailMakeCommand's --markdown/--view,
which default to false rather than null so handle() can tell "not
passed" apart from "passed with no value").

ServeCommand's --tries default changes from int 10 to string '10',
since the signature DSL only ever produces string defaults -- this
already matches every other numeric-looking default on every command
in the framework that was already signature-based (e.g. WorkCommand's
--memory=128), so it's not a new inconsistency.

Also removes a dead, unused $name property left over on
ShowModelCommand from an earlier partial migration to $signature.
@lucasmichot
lucasmichot force-pushed the feat/command-signature-tests branch from 97acdd2 to 36218d6 Compare July 29, 2026 17:00
@lucasmichot
lucasmichot marked this pull request as ready for review July 29, 2026 17:04
@taylorotwell
taylorotwell merged commit 08bb3da into laravel:13.x Jul 30, 2026
53 checks passed
@lucasmichot
lucasmichot deleted the feat/command-signature-tests branch August 3, 2026 18:08
@stancl

stancl commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

FWIW this might be a breaking change, see archtechx/tenancy#1474

I'm not sure if extending a Laravel command and overriding just $name is sanctioned by Laravel's backwards compatibility guarantees, it might just be too low level and something userland code and packages have to carry the responsibility for, however I wouldn't be surprised if our this did break things in more packages and applications than just our Tenancy package.

Assuming we can migrate to $signature and support both Laravel <13.24.0 and >=13.24.0, the fix shouldn't be too difficult in classes that extend Laravel commands. If for any reason that wouldn't work though we'd have to perhaps have separate command classes for different Laravel versions. Will post an update.

Update: Managed to do just a conditional if (version_compare(...)) $this->signature = ...; else $this->name = ... in a constructor.

@lucasmichot

lucasmichot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Isn't Command::configure done especially for that ?
cf. archtechx/tenancy#1478

@fgibaux

fgibaux commented Aug 5, 2026

Copy link
Copy Markdown

it looks like this change breaks the migrate command when using the schema loader.

I'm used to call artisan db:wipe --env=testing && artisan migrate --seed -v --env=testing and it's failing in 13.24.0

EDIT: it's actually because we're using https://github.com/jlorente/laravel-data-migrations which extends Illuminate\Database\Console\Migrations\InstallCommand while overriding $name and not $signature. it makes it register itself as the migrate:install command instead of the Laravel one.

sorry for the noise

shanecp added a commit to elegantmedia/Oxygen-Foundation that referenced this pull request Aug 7, 2026
* fix: preserve Oxygen seed command registration

Laravel framework PR #60926 consolidated inherited command metadata into $signature, causing the previous $name override to be ignored and the package command to register as db:seed.

Rename the command through configure() and verify that oxygen:seed remains distinct from Laravel’s native db:seed.

Upstream breaking change: laravel/framework#60926
Upstream commit: laravel/framework@08bb3da

* ci: run tests for pull requests targeting 6.x
@noefleury

Copy link
Copy Markdown
Contributor

Breaking change if we extend one of these command and override getOptions().

Custom options will just be ignored as we now use $signature instead of $name.

Solution : move custom options inside $signature declaration.

@lucasmichot

lucasmichot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

If you're extending one of these commands, the proper way is overriding configure() and adding the option there with $this->getDefinition()->addOption(...) - that runs after the parent's definition is built regardless of whether the parent uses $name/getOptions() or $signature, so it survives refactors like this one either way. Editing $signature only works if you own the whole command. Example: archtechx/tenancy#1478

jaspertey added a commit to jaspertey/laravel-ddd that referenced this pull request Aug 9, 2026
Laravel 13.24 changed how the framework generator commands are defined
(laravel/framework#60926), which breaks ddd:* command registration under
v2 (#115). Rather than backporting the fix, v2 is
now end of life: a composer conflict with illuminate/console >= 13.24.0
surfaces the incompatibility at dependency resolution time, and the
README directs users to the automated ddd:upgrade path to v3.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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