feat: introduce modern attribute-based spark commands#10120
feat: introduce modern attribute-based spark commands#10120paulbalandan wants to merge 11 commits intocodeigniter4:4.8from
spark commands#10120Conversation
319425c to
a944b89
Compare
michalsn
left a comment
There was a problem hiding this comment.
I think this is a solid direction overall.
One thing worth adding is a short migration section: which old properties map to which attributes or configure() calls, that run() becomes execute(), and how to swap raw $params and CLI::getOption() for the new validated arguments and options. That's the gap someone with a dozen existing commands will hit on day one.
|
Thanks! Another question I want to pop is: how comfortable are we to migrate freely the built in commands? Since they are not final, theoretically any user can extend them and provide an overriding implementation. Do we (1) mark this PR as potentially BC breaking? or (2) do not allow extensions and advise them to use composition instead? |
Hmm... I totally forgot that users may extend our commands. Although I never did that myself. (2) It's a bit too late for that now. People may already extend these commands |
There was a problem hiding this comment.
Pull request overview
Introduces a new attribute-based “modern” Spark command system (alongside legacy BaseCommand) and migrates several built-in commands and tests/documentation to the new lifecycle, binding, and validation model.
Changes:
- Added modern command framework pieces (
#[Command],AbstractCommand, typedArgument/Optiondefinitions, and new CLI exceptions) and updated discovery/execution to support dual registries. - Migrated built-in commands (
help,list,cache:clear,debugbar:clear,logs:clear,serve) to the modern API and adjusted console routing (--help/-h, legacy vs modern execution). - Updated user guide, changelog, fixtures, and system tests to cover modern/legacy coexistence and behavior changes (e.g., removing
routes -hBC).
Reviewed changes
Copilot reviewed 71 out of 72 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/src/PhpCsFixer/CodeIgniterRuleCustomisationPolicy.php | Updates fixer exclusions for renamed legacy fixtures path. |
| utils/phpstan-baseline/missingType.iterableValue.neon | Removes baseline entries now resolved by refactors. |
| user_guide_src/source/cli/index.rst | Adds modern commands guide to CLI docs index. |
| user_guide_src/source/cli/cli_modern_commands.rst | New documentation for modern command authoring and lifecycle. |
| user_guide_src/source/cli/cli_modern_commands/001.php | Minimal modern command example. |
| user_guide_src/source/cli/cli_modern_commands/002.php | Modern argument definition examples. |
| user_guide_src/source/cli/cli_modern_commands/003.php | Modern option definition examples. |
| user_guide_src/source/cli/cli_modern_commands/004.php | interact() and alias-aware option helpers example. |
| user_guide_src/source/cli/cli_modern_commands/005.php | Validated vs unbound accessors example. |
| user_guide_src/source/cli/cli_modern_commands/006.php | Calling another modern command example. |
| user_guide_src/source/cli/cli_modern_commands/007.php | Additional usage examples via addUsage(). |
| user_guide_src/source/cli/cli_modern_commands/008.php | Forwarding unbound input to another command example. |
| user_guide_src/source/cli/cli_modern_commands/009.php | Legacy BaseCommand example used for migration docs. |
| user_guide_src/source/cli/cli_modern_commands/010.php | Modern equivalent example (attribute + configure/execute). |
| user_guide_src/source/changelogs/v4.8.0.rst | Documents behavior changes, enhancements, and deprecations related to commands. |
| tests/system/Commands/Utilities/RoutesTest.php | Removes test for deprecated routes -h behavior. |
| tests/system/Commands/Translation/LocalizationFinderTest.php | Updates tests to use runLegacy() for legacy command invocation. |
| tests/system/Commands/HelpCommandTest.php | Rewrites assertions for modern help output and legacy help delegation. |
| tests/system/Commands/ConfigurableSortImportsTest.php | Updates fixture paths for renamed legacy command support files. |
| tests/system/Commands/Cache/ClearCacheTest.php | Updates expected error message for migrated cache clear command. |
| tests/system/CLI/SignalTest.php | Updates imports for legacy fixture namespace changes. |
| tests/system/CLI/Input/OptionTest.php | New tests for Option value object invariants and validation. |
| tests/system/CLI/Input/ArgumentTest.php | New tests for Argument value object invariants and validation. |
| tests/system/CLI/ConsoleTest.php | Adds coverage for help-option routing and legacy execution path. |
| tests/system/CLI/CommandsTest.php | Adds dual-registry discovery/execution tests and new deprecation/exception behaviors. |
| tests/system/CLI/BaseCommandTest.php | Updates legacy fixture namespace imports. |
| tests/system/CLI/Attributes/CommandTest.php | New tests for #[Command] attribute validation rules. |
| tests/system/CLI/AbstractCommandTest.php | Comprehensive tests for modern command lifecycle, binding, and validation. |
| tests/_support/_command/DuplicateModern.php | Modern fixture to test duplicate-name discovery warning behavior. |
| tests/_support/_command/DuplicateLegacy.php | Legacy fixture to test duplicate-name discovery warning behavior. |
| tests/_support/_command/AppInfo.php | Renames override fixture to target legacy app:info instead of list. |
| tests/_support/_command/AppAboutCommand.php | Modern override fixture for app:about. |
| tests/_support/InvalidCommands/NoAttributeCommand.php | Fixture for discovery skipping modern commands without #[Command]. |
| tests/_support/InvalidCommands/EmptyCommandName.php | Fixture for discovery logging when attribute instantiation fails. |
| tests/_support/Commands/Modern/TestFixtureCommand.php | Modern fixture command for registry/lifecycle tests. |
| tests/_support/Commands/Modern/InteractFixtureCommand.php | Fixture to test interact() mutations flowing into bind/validate/execute. |
| tests/_support/Commands/Modern/AppAboutCommand.php | Fixture covering binding/validation/accessor behaviors. |
| tests/_support/Commands/Legacy/Unsuffixable.php | Moves legacy fixtures under Tests\\Support\\Commands\\Legacy namespace. |
| tests/_support/Commands/Legacy/SignalCommandNoPosix.php | Moves legacy fixtures under Tests\\Support\\Commands\\Legacy namespace. |
| tests/_support/Commands/Legacy/SignalCommandNoPcntl.php | Moves legacy fixtures under Tests\\Support\\Commands\\Legacy namespace. |
| tests/_support/Commands/Legacy/SignalCommand.php | Moves legacy fixtures under Tests\\Support\\Commands\\Legacy namespace. |
| tests/_support/Commands/Legacy/NullReturningCommand.php | New legacy fixture to test deprecation for null exit codes. |
| tests/_support/Commands/Legacy/LanguageCommand.php | Moves legacy fixtures under Tests\\Support\\Commands\\Legacy namespace. |
| tests/_support/Commands/Legacy/InvalidCommand.php | Improves thrown exception message for invalid legacy fixture. |
| tests/_support/Commands/Legacy/HelpLegacyCommand.php | Legacy fixture to test legacy→modern invocation via runCommand(). |
| tests/_support/Commands/Legacy/Foobar.php | Moves legacy fixture file into Commands/Legacy directory. |
| tests/_support/Commands/Legacy/DestructiveCommand.php | Moves legacy fixtures under Tests\\Support\\Commands\\Legacy namespace. |
| tests/_support/Commands/Legacy/AppInfo.php | Updates legacy fixture behavior to route through legacy help fixture. |
| tests/_support/Commands/Legacy/AbstractInfo.php | Moves legacy fixtures under Tests\\Support\\Commands\\Legacy namespace. |
| system/Language/en/Commands.php | Adds new language strings for modern command validation/errors. |
| system/Language/en/CLI.php | Adds helpAvailableCommands label used by modern list output. |
| system/Commands/Utilities/Routes.php | Removes -h backward compatibility for --handler. |
| system/Commands/Server/Serve.php | Migrates serve to modern AbstractCommand + option definitions. |
| system/Commands/ListCommands.php | Migrates list to modern command and updates rendering logic. |
| system/Commands/Housekeeping/ClearLogs.php | Migrates logs:clear to modern command with interact() confirmation. |
| system/Commands/Housekeeping/ClearDebugbar.php | Migrates debugbar:clear to modern command. |
| system/Commands/Help.php | Migrates help to modern command with modern/legacy branching. |
| system/Commands/Cache/ClearCache.php | Migrates cache:clear to modern command and updates messaging. |
| system/CLI/Input/Option.php | Adds readonly Option definition value object with invariants. |
| system/CLI/Input/Argument.php | Adds readonly Argument definition value object with invariants. |
| system/CLI/Exceptions/UnknownOptionException.php | New typed exception for unknown options. |
| system/CLI/Exceptions/OptionValueMismatchException.php | New typed exception for option/value mismatches. |
| system/CLI/Exceptions/InvalidOptionDefinitionException.php | New typed exception for invalid option definitions. |
| system/CLI/Exceptions/InvalidArgumentDefinitionException.php | New typed exception for invalid argument definitions. |
| system/CLI/Exceptions/CommandNotFoundException.php | New typed exception for unknown commands (programmatic access). |
| system/CLI/Exceptions/ArgumentCountMismatchException.php | New typed exception for argument count mismatches. |
| system/CLI/Console.php | Routes legacy vs modern execution and improves --help handling. |
| system/CLI/Commands.php | Implements dual registries, modern discovery via attributes, and new run APIs. |
| system/CLI/BaseCommand.php | Updates legacy command-to-command calls to use runLegacy(). |
| system/CLI/Attributes/Command.php | Adds #[Command] attribute with validation. |
| system/CLI/AbstractCommand.php | Adds modern command base class, lifecycle, bind/validate, and helpers. |
| rector.php | Updates rector skip paths for moved fixtures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
01f3bc6 to
1868ad4
Compare
There was a problem hiding this comment.
Pull request overview
Introduces a new “modern” Spark command system (attribute-based #[Command] + AbstractCommand) alongside the existing legacy BaseCommand flow, and migrates several built-in commands plus docs/tests to support both registries.
Changes:
- Add modern command infrastructure:
#[Command]attribute,AbstractCommandlifecycle, typedArgument/Optiondefinitions, and typed CLI exceptions. - Update CLI runtime dispatch and built-in commands (
help,list,cache:clear,debugbar:clear,logs:clear,serve) to the modern style while keeping legacy commands working. - Expand documentation and test coverage for discovery, help/list output, validation/binding rules, and legacy/modern coexistence.
Reviewed changes
Copilot reviewed 72 out of 73 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/src/PhpCsFixer/CodeIgniterRuleCustomisationPolicy.php | Update fixture path after legacy command fixture reorg. |
| utils/phpstan-baseline/missingType.iterableValue.neon | Remove baseline entries made obsolete by refactors. |
| user_guide_src/source/cli/index.rst | Add modern commands page to CLI docs index. |
| user_guide_src/source/cli/cli_modern_commands.rst | New user guide page documenting modern commands API and lifecycle. |
| user_guide_src/source/cli/cli_modern_commands/001.php | Minimal modern command example snippet. |
| user_guide_src/source/cli/cli_modern_commands/002.php | Argument definition example snippet. |
| user_guide_src/source/cli/cli_modern_commands/003.php | Option modes example snippet. |
| user_guide_src/source/cli/cli_modern_commands/004.php | interact()/alias-aware option example snippet. |
| user_guide_src/source/cli/cli_modern_commands/005.php | Validated vs unbound accessor example snippet. |
| user_guide_src/source/cli/cli_modern_commands/006.php | Calling other modern commands example snippet. |
| user_guide_src/source/cli/cli_modern_commands/007.php | Usage examples (addUsage) snippet. |
| user_guide_src/source/cli/cli_modern_commands/008.php | Forwarding unbound args/options snippet. |
| user_guide_src/source/cli/cli_modern_commands/009.php | Legacy BaseCommand example snippet. |
| user_guide_src/source/cli/cli_modern_commands/010.php | Modernized version of legacy example snippet. |
| user_guide_src/source/changelogs/v4.8.0.rst | Document behavior changes, enhancements, and deprecations for CLI commands. |
| tests/system/Commands/Utilities/RoutesTest.php | Remove obsolete -h backward-compat test for routes. |
| tests/system/Commands/Translation/LocalizationFinderTest.php | Switch to runLegacy() for legacy command invocation. |
| tests/system/Commands/ListCommandsTest.php | Add coverage for wrapped descriptions + duplicate name precedence behavior. |
| tests/system/Commands/HelpCommandTest.php | Replace legacy help assertions with modern structured help output checks. |
| tests/system/Commands/ConfigurableSortImportsTest.php | Update fixture checksum paths after fixture move. |
| tests/system/Commands/Cache/ClearCacheTest.php | Update expected error message for cache clear failure. |
| tests/system/CLI/SignalTest.php | Update legacy fixture namespaces/paths. |
| tests/system/CLI/Input/OptionTest.php | New tests for Option invariants and normalization rules. |
| tests/system/CLI/Input/ArgumentTest.php | New tests for Argument invariants and defaults. |
| tests/system/CLI/ConsoleTest.php | Cover help-option forwarding fix and ensure legacy dispatch uses runLegacy(). |
| tests/system/CLI/CommandsTest.php | Add extensive tests for dual registries, discovery warnings, deprecations, and getCommand(). |
| tests/system/CLI/BaseCommandTest.php | Update legacy fixture namespace import. |
| tests/system/CLI/Attributes/CommandTest.php | New tests for #[Command] validation rules. |
| tests/system/CLI/AbstractCommandTest.php | New comprehensive tests for binding/validation, unbound helpers, and renderThrowable(). |
| tests/_support/_command/AppInfo.php | Rename override fixture to target app:info instead of list. |
| tests/_support/_command/AppAboutCommand.php | New app override fixture for modern command discovery override tests. |
| tests/_support/InvalidCommands/NoAttributeCommand.php | Fixture: modern subclass missing #[Command] attribute. |
| tests/_support/InvalidCommands/EmptyCommandName.php | Fixture: invalid #[Command] attribute construction. |
| tests/_support/Duplicates/DuplicateModern.php | Fixture: modern command colliding name with legacy command. |
| tests/_support/Duplicates/DuplicateLegacy.php | Fixture: legacy command colliding name with modern command. |
| tests/_support/Commands/Modern/TestFixtureCommand.php | Fixture modern command for binding/validation tests. |
| tests/_support/Commands/Modern/InteractFixtureCommand.php | Fixture for interact() mutation propagation tests. |
| tests/_support/Commands/Modern/AppAboutCommand.php | Fixture modern command for help/list rendering + option/argument behavior. |
| tests/_support/Commands/Legacy/Unsuffixable.php | Move legacy fixtures into Tests\Support\Commands\Legacy namespace. |
| tests/_support/Commands/Legacy/SignalCommandNoPosix.php | Move legacy fixtures into Tests\Support\Commands\Legacy namespace. |
| tests/_support/Commands/Legacy/SignalCommandNoPcntl.php | Move legacy fixtures into Tests\Support\Commands\Legacy namespace. |
| tests/_support/Commands/Legacy/SignalCommand.php | Move legacy fixtures into Tests\Support\Commands\Legacy namespace. |
| tests/_support/Commands/Legacy/NullReturningCommand.php | Fixture: legacy command returning null to exercise deprecation path. |
| tests/_support/Commands/Legacy/LanguageCommand.php | Move legacy fixtures into Tests\Support\Commands\Legacy namespace. |
| tests/_support/Commands/Legacy/InvalidCommand.php | Improve thrown exception message for invalid legacy fixture. |
| tests/_support/Commands/Legacy/HelpLegacyCommand.php | Fixture: legacy command that calls modern help via runCommand(). |
| tests/_support/Commands/Legacy/Foobar.php | Fixture file moved under Legacy/ path for tooling/tests. |
| tests/_support/Commands/Legacy/DestructiveCommand.php | Move legacy fixtures into Tests\Support\Commands\Legacy namespace. |
| tests/_support/Commands/Legacy/AppInfo.php | Update namespace and legacy-to-modern call routing (help:legacy). |
| tests/_support/Commands/Legacy/AbstractInfo.php | Move legacy fixtures into Tests\Support\Commands\Legacy namespace. |
| system/Language/en/Commands.php | New language file for modern command validation/error messages. |
| system/Language/en/CLI.php | Add helpAvailableCommands string used by modern list output. |
| system/Commands/Utilities/Routes.php | Remove legacy -h BC shortcut handling (now reserved for help). |
| system/Commands/Server/Serve.php | Migrate serve to AbstractCommand + typed options. |
| system/Commands/ListCommands.php | Migrate list to AbstractCommand and combine legacy/modern registries. |
| system/Commands/Housekeeping/ClearLogs.php | Migrate logs:clear to AbstractCommand with interactive flow. |
| system/Commands/Housekeeping/ClearDebugbar.php | Migrate debugbar:clear to AbstractCommand. |
| system/Commands/Help.php | Migrate help to AbstractCommand with structured rendering for modern commands. |
| system/Commands/Cache/ClearCache.php | Migrate cache:clear to AbstractCommand and update messaging/output. |
| system/CLI/Input/Option.php | New Option readonly value object with invariant enforcement. |
| system/CLI/Input/Argument.php | New Argument readonly value object with invariant enforcement. |
| system/CLI/Exceptions/UnknownOptionException.php | New typed exception for unknown options. |
| system/CLI/Exceptions/OptionValueMismatchException.php | New typed exception for option/value mismatch. |
| system/CLI/Exceptions/InvalidOptionDefinitionException.php | New typed exception for option definition errors. |
| system/CLI/Exceptions/InvalidArgumentDefinitionException.php | New typed exception for argument definition errors. |
| system/CLI/Exceptions/CommandNotFoundException.php | New typed exception for command lookup failure. |
| system/CLI/Exceptions/ArgumentCountMismatchException.php | New typed exception for argument count mismatches. |
| system/CLI/Console.php | Dispatch to runLegacy() vs runCommand() and improve --help routing behavior. |
| system/CLI/Commands.php | Add modern registry, discovery rules, runLegacy()/runCommand(), and deprecations. |
| system/CLI/BaseCommand.php | Route BaseCommand::call() through runLegacy(). |
| system/CLI/Attributes/Command.php | New #[Command] attribute with validation. |
| system/CLI/AbstractCommand.php | New modern command base class: lifecycle, bind/validate, accessors, helpers. |
| rector.php | Update excluded fixture path after reorg. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1868ad4 to
610a0ed
Compare
610a0ed to
270cba9
Compare
Description
Introduces a new modern
sparkcommand system alongside the existingBaseCommand. Modern commands describe themselves through a#[Command]attribute, build their argument/option surface inside aconfigure()method using readonly value objects (Argument,Option), and implementexecute(array $arguments, array $options): int. The framework parses the command line, applies declared defaults, validates the input, and hands the result toexecute().The legacy
BaseCommandstyle continues to work: both registries are discovered side-by-side, and a warning is printed at discovery when the same name is claimed by both styles.BaseCommandwill remain supported until every built-in command is migrated, at which point it will begin emitting deprecation notices.In this PR, "legacy" commands are those extending
BaseCommandwhile "modern" commands are those extendingAbstractCommand.Benefits / enhancements:
#[Command(name, description, group)]replaces magic properties and is validated at attribute-construction time, so a malformed name fails at discovery rather than at the first invocation.ArgumentandOptionare final readonly value objects whose invariants are enforced in the constructor. Configuration mistakes (required argument with a default, array option withoutrequiresValue, negatable option accepting a value, …) throw a typedInvalidArgumentDefinitionException/InvalidOptionDefinitionExceptionat the point of declaration.$params. The framework binds raw tokens to declared arguments/options, applies defaults, coerces flags, handles--name/-n/--no-namealiasing, and rejects extraneous or missing input beforeexecute()runs.configure()→initialize()→interact()→ bind & validate →execute(). Each hook has one job, and prior-phase mutations flow cleanly into the next phase.hasUnboundOption()/getUnboundOption()letinteract()ask "was this option passed?" without knowing whether the user typed the long name, the shortcut, or the negation form.ArgumentCountMismatchException,OptionValueMismatchException,UnknownOptionException, andCommandNotFoundExceptionreplace opaqueRuntimeExceptionusage and give callers something meaningful to catch.renderThrowable()can be called from any command; it temporarily makes the shared request a CLI request for the render and restores the prior request afterwards.Migrated built-in commands
The following framework commands have been migrated to
AbstractCommandin this PR:help,list,cache:clear,debugbar:clear,logs:clear, andserve. The remaining built-ins will be migrated in follow-up PRs.Interactive mode
Currently, in this PR, commands are interactive (i.e., every command that expects completed inputs can prompt the user via
interact()). In a follow up PR, I am intending to make this configurable so that we can have a non-interactive mode.Acknowledgement
The command lifecycle —
configure/initialize/interact/executeand the bind-before-validate flow — is inspired by Symfony Console. The implementation and public surface are CodeIgniter-specific; the vocabulary and staging idea are the parts we borrowed.AI Disclosure
This PR body and the documentation are assisted by Claude Opus 4.7. Initial review was also AI-assisted. Changes are reviewed before being accepted.
Note
The changed files seems to be too much but in reality the bulk of the changes are from the renaming of the "legacy" command fixtures so that they are distinguishable from the "modern" ones.
Checklist: