From 9c052766e1ddc25cd1a73c1930b3e0f56a6c0968 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Mon, 5 Oct 2026 06:04:16 -0400 Subject: [PATCH] Rename commands to laranail::git-commit-checker.* The commands used the bare git-commit-checker: prefix in Artisan's flat command map. They now use the family shape, through a local SupportsNamespacedNames copy because the package requires no laranail/* package. The old names stay as aliases that print a deprecation line, so existing hooks keep working; newly installed hooks call the scoped name. The manifest gets the Packagist exclusion and the maintainer email. --- CHANGELOG.md | 17 +++ README.md | 13 ++- composer.json | 11 ++ docs/architecture.md | 14 ++- .../Concerns/SupportsNamespacedNames.php | 104 ++++++++++++++++++ .../Concerns/WarnsOnDeprecatedAlias.php | 46 ++++++++ src/Commands/InstallCommand.php | 15 ++- src/Commands/PreCommitHookCommand.php | 15 ++- tests/Feature/ConfigTest.php | 6 +- .../NamespacedNamesConformanceTest.php | 83 ++++++++++++++ tests/Feature/NamingConventionTest.php | 62 +++++++++++ 11 files changed, 374 insertions(+), 12 deletions(-) create mode 100644 src/Commands/Concerns/SupportsNamespacedNames.php create mode 100644 src/Commands/Concerns/WarnsOnDeprecatedAlias.php create mode 100644 tests/Feature/NamespacedNamesConformanceTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index c5bc780..bc070c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,11 +21,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **A config published to `config/git-commit-checker.php`** is still honoured. Republish with `--tag=laranail::git-commit-checker-config`, which now writes `config/laranail/git-commit-checker.php`, and delete the old file. +- `git-commit-checker:install` and `git-commit-checker:pre-commit-hook`. Both stay registered as + aliases of the scoped commands and print a deprecation line naming the replacement when used, + so hooks installed before this release keep working. Re-run + `laranail::git-commit-checker.install` to rewrite a hook. The aliases may be removed in the next + minor after 0.1. ### Added - A `Quick start` section in the README. +### Changed + +- **The commands are `laranail::git-commit-checker.install` and + `laranail::git-commit-checker.pre-commit-hook`**, the family's `laranail::.` shape. + The hook script `install` writes calls the scoped name. A local copy of `laranail/console`'s + `SupportsNamespacedNames` trait lets Symfony accept the `::`; the package still requires no + `laranail/*` package. +- The `repositories` block replaces Packagist with a copy that excludes `laranail/*`, so + `laranail/package-tools` can only resolve from its VCS repository. This is the family's standard + block. +- The Imani Manyara author entry carries `imani@simtabi.com`. + ## v0.1.0 ### Changed diff --git a/README.md b/README.md index db1dfcb..a5228cf 100755 --- a/README.md +++ b/README.md @@ -20,8 +20,8 @@ composer require laranail/git-commit-checker ### Getting started -1. Run it from a Git checkout in a local environment: `git-commit-checker:install` refuses to write - a hook when `APP_ENV` is not `local` or there is no `.git` directory. +1. Run it from a Git checkout in a local environment: `laranail::git-commit-checker.install` refuses to + write a hook when `APP_ENV` is not `local` or there is no `.git` directory. 2. Optionally publish the config to change the hooks or the Pint presets. The pre-commit check is on by default; `GIT_COMMIT_CHECKER_ENABLED=false` switches it off. @@ -33,13 +33,18 @@ composer require laranail/git-commit-checker ```bash # Write .git/hooks/pre-commit and, optionally, a pint.json preset -php artisan git-commit-checker:install +php artisan laranail::git-commit-checker.install # Every commit now runs Pint in --test mode over the changed PHP files; # run the same check by hand without committing -php artisan git-commit-checker:pre-commit-hook +php artisan laranail::git-commit-checker.pre-commit-hook ``` +> `git-commit-checker:install` and `git-commit-checker:pre-commit-hook` are deprecated aliases of +> these two commands. They still run, print a deprecation line, and may be removed in the next minor +> after 0.1. A hook installed before the rename calls the old name and keeps working; run +> `laranail::git-commit-checker.install` again to rewrite it with the new one. + How the hook is wired is in [Architecture](docs/architecture.md); everything else is in the [documentation index](#documentation). ## Documentation diff --git a/composer.json b/composer.json index 279a2aa..ab0e8f8 100644 --- a/composer.json +++ b/composer.json @@ -15,6 +15,7 @@ "authors": [ { "name": "Imani Manyara", + "email": "imani@simtabi.com", "role": "Developer", "homepage": "https://simtabi.com" }, @@ -80,6 +81,16 @@ { "type": "vcs", "url": "https://github.com/laranail/package-tools" + }, + { + "type": "composer", + "url": "https://repo.packagist.org", + "exclude": [ + "laranail/*" + ] + }, + { + "packagist.org": false } ] } diff --git a/docs/architecture.md b/docs/architecture.md index 58d1a49..0b59377 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -14,6 +14,7 @@ sibling package, a third-party one, or the consuming application's own. | Config key | `laranail.git-commit-checker` | | View namespace | `laranail/git-commit-checker` | | Publish tags | `laranail::git-commit-checker-*` | +| Artisan commands | `laranail::git-commit-checker.install`, `laranail::git-commit-checker.pre-commit-hook` | Views take the slash form because Laravel interpolates the namespace into the override path, so a published override lands in `resources/views/vendor/laranail/git-commit-checker` — one directory per vendor @@ -23,8 +24,19 @@ rather than thirty siblings flat in the `vendor` root. Its publish tags were already vendor-scoped, which is what made the gap easy to miss by eye: two of the four names were right. +**The two commands were `git-commit-checker:install` and `git-commit-checker:pre-commit-hook`.** +Both names stay registered as aliases of the scoped commands, so scripts and hooks written before the +rename still run, and print a deprecation line naming the replacement when used. They may be removed +in the next minor after 0.1. The hook script `install` writes calls the scoped name. + +Symfony's command-name validator rejects the empty segment in `::`, so the commands use a local copy +of `laranail/console`'s `SupportsNamespacedNames` trait. A copy rather than a dependency: this +package requires no `laranail/*` package, and taking console on for one trait would make every +consumer declare a VCS repository for it. `tests/Feature/NamespacedNamesConformanceTest.php` holds +the copy to the canonical behaviour. + `tests/Feature/NamingConventionTest.php` asserts this against the **live registries** — -`View::getFinder()->getHints()` and the config repository — rather than by grepping the provider, so +`View::getFinder()->getHints()`, the config repository and the Artisan command map — rather than by grepping the provider, so the guard survives a refactor of the registration code. ## Modernisation diff --git a/src/Commands/Concerns/SupportsNamespacedNames.php b/src/Commands/Concerns/SupportsNamespacedNames.php new file mode 100644 index 0000000..0d981f6 --- /dev/null +++ b/src/Commands/Concerns/SupportsNamespacedNames.php @@ -0,0 +1,104 @@ +`. + * + * Symfony's {@see SymfonyCommand::validateName()} regex (`^[^:]++(:[^:]++)*$`) + * rejects the empty segment in `::`, so this trait sets the name (and aliases) + * past that validator by writing the private property directly. Dispatch still + * works because Symfony resolves an exact command name (and its registered + * aliases) before its `:`-splitting namespace lookup runs, which is what lets + * `laranail::git-commit-checker.install` be found at all. + * + * The trait also applies an optional `$commandAliases` list during construction + * (Laravel invokes {@see setName()} while building the command from its + * signature). + * + * **The list is declared here, defaulting to empty**, and that is a fix rather + * than a style choice: it used to be read off the consuming command without + * being declared anywhere, so any command that used the trait *without* + * declaring the property died with `Undefined property: …::$commandAliases` the + * moment Laravel built it. A trait that requires an undeclared property of its + * user is a trap, and the failure lands at boot rather than at the call site. + * A command that wants aliases still declares its own list, which overrides + * this default. + * + * Whatever it declares must itself be vendor-scoped. The only bare names this + * package carries are the two legacy command names, kept as deprecated aliases + * through `#[AsCommand]` so existing scripts and installed git hooks still run. + * + * Self-contained: this is a local copy of the canonical laranail trait in + * `laranail/console`, because this package's `require` holds no `laranail/*` + * entry and taking console on for one trait would make every consumer declare + * a VCS repository for it. Its conformance test declares that reason so + * `package-tools/scripts/verify-trait-copies.py` can falsify it. + * + * @internal + */ +trait SupportsNamespacedNames +{ + public function setName(string $name): static + { + $this->writeCommandName('name', $name); + + $aliases = $this->declaredCommandAliases(); + + if ($aliases !== []) { + $this->setAliases($aliases); + } + + return $this; + } + + /** + * @param iterable $aliases + */ + public function setAliases(iterable $aliases): static + { + $this->writeCommandName('aliases', is_array($aliases) ? $aliases : iterator_to_array($aliases)); + + return $this; + } + + /** + * The consuming command's own `$commandAliases`, if it declares one. + * + * Deliberately NOT a property on this trait. A trait cannot declare a property that the using + * class also declares with a different default -- PHP rejects the composition outright with a + * fatal -- so declaring it here made the documented usage ("a command that wants aliases + * declares its own list") impossible to actually write. + * + * Declaring it was itself the fix for the opposite bug, where reading an undeclared property + * threw at boot. Reading it defensively fixes both at once. + * + * @return list + */ + private function declaredCommandAliases(): array + { + if (! property_exists($this, 'commandAliases') || ! is_array($this->commandAliases)) { + return []; + } + + // Filtered rather than cast: the property is the consuming command's, so its contents are + // not this trait's to assume. A stray null would reach Symfony's setAliases() as a type + // error at boot, which is the failure mode this whole method exists to avoid. + return array_values(array_filter( + $this->commandAliases, + static fn (mixed $alias): bool => is_string($alias) && $alias !== '', + )); + } + + private function writeCommandName(string $property, mixed $value): void + { + // The name/aliases are private on Symfony's base Command; writing them + // directly bypasses validateName()'s rejection of the `::` separator. + (new ReflectionProperty(SymfonyCommand::class, $property))->setValue($this, $value); + } +} diff --git a/src/Commands/Concerns/WarnsOnDeprecatedAlias.php b/src/Commands/Concerns/WarnsOnDeprecatedAlias.php new file mode 100644 index 0000000..bee0696 --- /dev/null +++ b/src/Commands/Concerns/WarnsOnDeprecatedAlias.php @@ -0,0 +1,46 @@ +call()` alike, while `getName()` is always the + * canonical name. + * + * Symfony calls initialize() after binding the input and before interact(), + * so the warning prints before any prompt. + */ +trait WarnsOnDeprecatedAlias +{ + protected function initialize(InputInterface $input, OutputInterface $output): void + { + parent::initialize($input, $output); + + $invokedAs = $input->getFirstArgument(); + + if (! is_string($invokedAs) || $invokedAs === $this->getName() || ! in_array($invokedAs, $this->getAliases(), true)) { + return; + } + + $output->writeln(sprintf( + 'Deprecated: [%s] is a deprecated alias and will be removed in the next minor after 0.1. Use [%s] instead.', + $invokedAs, + (string) $this->getName(), + )); + } +} diff --git a/src/Commands/InstallCommand.php b/src/Commands/InstallCommand.php index d17ee7e..5d18b2a 100644 --- a/src/Commands/InstallCommand.php +++ b/src/Commands/InstallCommand.php @@ -10,12 +10,23 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Attribute\AsCommand; use Simtabi\Laranail\GitCommitChecker\Commands\Concerns\ReadsPackageConfig; - -#[AsCommand('git-commit-checker:install', 'Install "pre_commit" hook into your git.')] +use Simtabi\Laranail\GitCommitChecker\Commands\Concerns\WarnsOnDeprecatedAlias; +use Simtabi\Laranail\GitCommitChecker\Commands\Concerns\SupportsNamespacedNames; + +/** + * Registered as `laranail::git-commit-checker.install`. + * + * `git-commit-checker:install` stays registered as an alias of it and prints a + * deprecation line when used. That alias is deprecated and may be removed in the + * next minor after 0.1. + */ +#[AsCommand('laranail::git-commit-checker.install|git-commit-checker:install', 'Install "pre_commit" hook into your git.')] class InstallCommand extends Command { use ConfirmableTrait; use ReadsPackageConfig; + use SupportsNamespacedNames; + use WarnsOnDeprecatedAlias; public function handle(): int { diff --git a/src/Commands/PreCommitHookCommand.php b/src/Commands/PreCommitHookCommand.php index 48ddf65..fb1b9fd 100644 --- a/src/Commands/PreCommitHookCommand.php +++ b/src/Commands/PreCommitHookCommand.php @@ -12,11 +12,22 @@ use Symfony\Component\Process\Process; use Symfony\Component\Console\Attribute\AsCommand; use Simtabi\Laranail\GitCommitChecker\Commands\Concerns\ReadsPackageConfig; - -#[AsCommand('git-commit-checker:pre-commit-hook', 'Git hook before commit')] +use Simtabi\Laranail\GitCommitChecker\Commands\Concerns\WarnsOnDeprecatedAlias; +use Simtabi\Laranail\GitCommitChecker\Commands\Concerns\SupportsNamespacedNames; + +/** + * Registered as `laranail::git-commit-checker.pre-commit-hook`. + * + * `git-commit-checker:pre-commit-hook` stays registered as an alias of it and prints a + * deprecation line when used. That alias is deprecated and may be removed in the + * next minor after 0.1. + */ +#[AsCommand('laranail::git-commit-checker.pre-commit-hook|git-commit-checker:pre-commit-hook', 'Git hook before commit')] class PreCommitHookCommand extends Command { use ReadsPackageConfig; + use SupportsNamespacedNames; + use WarnsOnDeprecatedAlias; public function handle(): int { diff --git a/tests/Feature/ConfigTest.php b/tests/Feature/ConfigTest.php index 8e5479c..b690f39 100644 --- a/tests/Feature/ConfigTest.php +++ b/tests/Feature/ConfigTest.php @@ -30,14 +30,14 @@ it('runs the hook when enabled at the registered key', function (): void { Config::set('laranail.git-commit-checker.enabled', true); - $this->artisan('git-commit-checker:pre-commit-hook') + $this->artisan('laranail::git-commit-checker.pre-commit-hook') ->doesntExpectOutputToContain('is disabled'); }); it('skips the hook when disabled at the registered key', function (): void { Config::set('laranail.git-commit-checker.enabled', false); - $this->artisan('git-commit-checker:pre-commit-hook') + $this->artisan('laranail::git-commit-checker.pre-commit-hook') ->expectsOutputToContain('is disabled') ->assertSuccessful(); }); @@ -47,6 +47,6 @@ Config::set('git-commit-checker', array_replace(Config::get('laranail.git-commit-checker'), ['enabled' => false])); Config::set('laranail.git-commit-checker.enabled', true); - $this->artisan('git-commit-checker:pre-commit-hook') + $this->artisan('laranail::git-commit-checker.pre-commit-hook') ->expectsOutputToContain('is disabled'); }); diff --git a/tests/Feature/NamespacedNamesConformanceTest.php b/tests/Feature/NamespacedNamesConformanceTest.php new file mode 100644 index 0000000..7ca3d79 --- /dev/null +++ b/tests/Feature/NamespacedNamesConformanceTest.php @@ -0,0 +1,83 @@ +.` naming trait. + * + * The family carries several copies of this trait, each for a reason stated in terms of its own + * manifest. The canonical copy is `laranail/console`'s. + * + * Copies are tolerable. Copies that quietly stop agreeing are not: one of them once read an + * undeclared `$commandAliases` and fataled at boot for any command that used it without declaring + * the property. This file is the same in every package that carries a copy, so a divergence fails + * somewhere instead of shipping. + * + * @trait-copy-reason no-laranail-requirement This package's `require` holds no `laranail/*` + * entry, so a consumer installs it without declaring any laranail VCS repository. Taking + * `laranail/console` on for one trait would change that for every consumer. The slug names a + * condition, so it expires on its own: add a `laranail/*` requirement and + * `scripts/verify-trait-copies.py` in `laranail/package-tools` fails until the copy goes. + */ +function conformanceCommand(): SymfonyCommand +{ + return new class extends SymfonyCommand + { + use NamespacedNames; + }; +} + +it('accepts a name Symfony validateName() would reject', function (): void { + // ^[^:]++(:[^:]++)*$ rejects the empty segment in `::`, which is the entire reason the trait + // exists. Dispatch still works: Symfony matches an exact name before splitting on `:`. + expect(conformanceCommand()->setName('laranail::atlas.doctor')->getName()) + ->toBe('laranail::atlas.doctor'); +}); + +it('accepts namespaced aliases', function (): void { + expect(conformanceCommand()->setAliases(['laranail::atlas.dr'])->getAliases()) + ->toBe(['laranail::atlas.dr']); +}); + +it('takes aliases from any iterable, not just an array', function (): void { + expect(conformanceCommand()->setAliases(new ArrayIterator(['laranail::atlas.x']))->getAliases()) + ->toBe(['laranail::atlas.x']); +}); + +it('returns itself so the calls chain', function (): void { + $command = conformanceCommand(); + + expect($command->setName('laranail::atlas.a'))->toBe($command) + ->and($command->setAliases([]))->toBe($command); +}); + +it('does not fatal when the command declares no alias list', function (): void { + // The bug this file exists for. A copy that reads $commandAliases without declaring it throws + // on every command that does not declare the property -- at boot, for the whole application. + expect(fn (): string => (string) conformanceCommand()->setName('laranail::atlas.b')->getName()) + ->not->toThrow(Throwable::class); +}); + +it('still rejects a plainly invalid name', function (): void { + // The trait bypasses validateName() deliberately, but an empty name is not a namespaced name -- + // it is Symfony's own "no name" state, and getName() should report it as such. + expect(conformanceCommand()->setName('')->getName())->toBe(''); +}); + +it('lets a command declare its own alias list', function (): void { + // Declaring $commandAliases on the trait made this a COMPILE-TIME fatal: PHP rejects a trait + // and a using class declaring the same property with different defaults. The feature was + // documented and unusable at the same time. + $command = new class extends SymfonyCommand + { + use NamespacedNames; + + /** @var list */ + protected array $commandAliases = ['laranail::atlas.dr']; + }; + + expect($command->setName('laranail::atlas.doctor')->getAliases())->toBe(['laranail::atlas.dr']); +}); diff --git a/tests/Feature/NamingConventionTest.php b/tests/Feature/NamingConventionTest.php index c97aa2f..201848b 100644 --- a/tests/Feature/NamingConventionTest.php +++ b/tests/Feature/NamingConventionTest.php @@ -4,6 +4,10 @@ use Illuminate\Support\Facades\View; use Illuminate\Support\Facades\Config; +use Illuminate\Support\Facades\Artisan; +use Illuminate\Contracts\Console\Kernel; +use Simtabi\Laranail\GitCommitChecker\Commands\InstallCommand; +use Simtabi\Laranail\GitCommitChecker\Commands\PreCommitHookCommand; /** * Every name this package registers into a framework-owned registry. @@ -27,3 +31,61 @@ expect($hints)->toHaveKey('laranail/git-commit-checker') ->and($hints)->not->toHaveKey('git-commit-checker'); }); + +/** + * Artisan's command map is flat too. `git-commit-checker:install` is the package's own slug, but + * the family shape is `laranail::.`, so the source of a command is unambiguous and no + * sibling can claim the name. The old names stay as aliases of the same command, so scripts and + * already-installed hooks keep working, and say they are deprecated when used. + */ +dataset('commands', [ + 'install' => ['laranail::git-commit-checker.install', 'git-commit-checker:install', InstallCommand::class], + 'pre-commit hook' => ['laranail::git-commit-checker.pre-commit-hook', 'git-commit-checker:pre-commit-hook', PreCommitHookCommand::class], +]); + +it('registers its commands under vendor and slug', function (string $scoped, string $bare, string $class): void { + $commands = Artisan::all(); + + expect($commands)->toHaveKey($scoped) + ->and($commands[$scoped])->toBeInstanceOf($class) + ->and($commands[$scoped]->getName())->toBe($scoped); +})->with('commands'); + +it('keeps the bare name only as a deprecated alias of the scoped command', function (string $scoped, string $bare): void { + $command = Artisan::all()[$scoped]; + + // Exactly the one legacy name: an extra bare alias would hand back the collision the scoped + // name exists to prevent. + expect($command->getAliases())->toBe([$bare]) + ->and($this->app[Kernel::class]->all()[$bare]->getName())->toBe($scoped); +})->with('commands'); + +it('warns when a command is invoked by its deprecated name', function (): void { + Config::set('laranail.git-commit-checker.enabled', false); + + // Artisan::output() rather than expectsOutputToContain(): the mocked output matches each + // expectation against one write, and both halves of the warning are a single line. + expect(Artisan::call('git-commit-checker:pre-commit-hook'))->toBe(0) + ->and(Artisan::output()) + ->toContain('[git-commit-checker:pre-commit-hook] is a deprecated alias') + ->toContain('Use [laranail::git-commit-checker.pre-commit-hook] instead'); +}); + +it('does not warn when a command is invoked by its scoped name', function (): void { + Config::set('laranail.git-commit-checker.enabled', false); + + $this->artisan('laranail::git-commit-checker.pre-commit-hook') + ->doesntExpectOutputToContain('deprecated') + ->assertSuccessful(); +}); + +it('writes hooks that call the scoped command name', function (): void { + // Hooks installed before the rename call the bare name and keep working through the alias; + // hooks installed from now on must not depend on it. + $install = Artisan::all()['laranail::git-commit-checker.install']; + $script = (new ReflectionMethod($install, 'generateHookScript')) + ->invoke($install, (new PreCommitHookCommand)->getName()); + + expect($script)->toContain('laranail::git-commit-checker.pre-commit-hook') + ->not->toContain(' git-commit-checker:pre-commit-hook'); +});