diff --git a/CHANGELOG.md b/CHANGELOG.md index e4e6431..648d84f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ All notable changes are documented here. The format is based on [Keep a Changelo - `route:list` and `make:action` take an optional `ActionResolver`. [#5](https://github.com/phalcon/crest/issues/5) - `event:list` reads all listeners with one `getListenerMap()` call. [#1](https://github.com/phalcon/crest/issues/1) - `phalcon/talon` moved from `^0.8` to `^1.0.0`. +- phpcs and php-cs-fixer use the shared Phalcon rules from `phalcon/code-quality`, added to `require-dev`. - `stub:publish` without a name skips the `project-*` stubs. Publish them by name; they go to the working directory or `--directory`. [#8](https://github.com/phalcon/crest/issues/8) - Rendering a stub fails when a placeholder has no value. The error names the stub. [#8](https://github.com/phalcon/crest/issues/8) - The config inference error names the missing psr-4 directories. [#18](https://github.com/phalcon/crest/issues/18) diff --git a/composer.json b/composer.json index f5f28df..910b8f3 100644 --- a/composer.json +++ b/composer.json @@ -11,9 +11,9 @@ "friendsofphp/php-cs-fixer": "^3", "pds/composer-script-names": "^1", "pds/skeleton": "^1", + "phalcon/code-quality": "^1.0", "phalcon/phalcon": "v6.0.x-dev", "phalcon/talon": "^1.0.0", - "phalcon/code-quality": "^1.0", "phpstan/phpstan": "^2", "phpunit/phpunit": "^10.5", "squizlabs/php_codesniffer": "^3 || ^4" @@ -29,7 +29,7 @@ }, "bin": ["bin/crest"], "config": { - "optimize-autoloader": true, + "optimize-autoloader": true, "sort-packages": true, "allow-plugins": { "infection/extension-installer": true, diff --git a/composer.lock b/composer.lock index e3b270c..df2d804 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "9749fe461704aa68db6fc261f94c41e2", + "content-hash": "93c3e9b74f67b97c0ce45a654a3db046", "packages": [], "packages-dev": [ { diff --git a/resources/docker/Dockerfile b/resources/docker/Dockerfile index 8314da6..e883789 100644 --- a/resources/docker/Dockerfile +++ b/resources/docker/Dockerfile @@ -12,10 +12,11 @@ ARG USER=crest ARG GROUP=crest # System packages, PHP extensions and an unprivileged user matching the host UID. +# -o accepts an ID that the image already uses, for example GID 20 on macOS. RUN < ../src ../tests + + + */tests/_output/* diff --git a/src/Command/NewCommand.php b/src/Command/NewCommand.php index 5b3dd24..7d246e2 100644 --- a/src/Command/NewCommand.php +++ b/src/Command/NewCommand.php @@ -86,7 +86,7 @@ final class NewCommand extends Command * first. It is a directory name, not a path. It is also the docker * container prefix, which must start with a letter or digit. */ - private const NAME = '/^[A-Za-z0-9][A-Za-z0-9_-]*$/'; + private const NAME = '/^[A-Za-z0-9][A-Za-z0-9_-]*\z/'; /** * Variant => the composer requirement and its constraint. v5 needs 5.18, @@ -101,10 +101,11 @@ final class NewCommand extends Command ]; /** - * major.minor only. The Dockerfile base image `php:-cli` has no - * patch tags, and composer.json uses the same value. + * major.minor only, with no leading zeros. The Dockerfile base image + * `php:-cli` has no patch tags, and composer.json uses the same + * value. */ - private const PHP = '/^\d+\.\d+$/'; + private const PHP = '/^[1-9]\d*\.(?:0|[1-9]\d*)\z/'; /** * The oldest PHP that runs the generated code. It uses readonly promoted diff --git a/src/Command/Stub/PublishCommand.php b/src/Command/Stub/PublishCommand.php index 8e76fe9..5d0d27d 100644 --- a/src/Command/Stub/PublishCommand.php +++ b/src/Command/Stub/PublishCommand.php @@ -49,7 +49,7 @@ final class PublishCommand extends ProjectCommand /** * A packaged stub name. Hyphens are in because `action-view` is one. */ - private const NAME = '/^[A-Za-z0-9_-]+$/'; + private const NAME = '/^[A-Za-z0-9_-]+\z/'; public function define(): Definition { diff --git a/src/Console/Kernel.php b/src/Console/Kernel.php index 590be18..04474fb 100644 --- a/src/Console/Kernel.php +++ b/src/Console/Kernel.php @@ -72,7 +72,7 @@ public static function globals(): Definition { return Definition::for('') ->option('config=s', 'Path to the project configuration file') - ->option('directory=s', 'Project root override') + ->option('directory=s', 'Directory to use instead of the working directory') ->option('trace', 'Show the full exception trace') ->option('help|h', 'Show this help') ->option('quiet|q', 'Suppress non-essential output'); diff --git a/src/Generator/ClassName.php b/src/Generator/ClassName.php index 48c052d..ee82b37 100644 --- a/src/Generator/ClassName.php +++ b/src/Generator/ClassName.php @@ -47,7 +47,7 @@ final class ClassName * Deliberately byte-oriented and not /u: that is exactly how PHP itself * decides what may name a class. */ - private const PATTERN = '/^[A-Za-z_\x80-\xff][A-Za-z0-9_\x80-\xff]*$/'; + private const PATTERN = '/^[A-Za-z_\x80-\xff][A-Za-z0-9_\x80-\xff]*\z/'; /** * A namespace: one name, or more names that backslashes join. Each name diff --git a/src/Process/ShellRunner.php b/src/Process/ShellRunner.php index 1f3770a..af53b5b 100644 --- a/src/Process/ShellRunner.php +++ b/src/Process/ShellRunner.php @@ -15,6 +15,7 @@ use Crest\Console\Exceptions\Exception; +use function basename; use function explode; use function getenv; use function is_dir; @@ -23,7 +24,6 @@ use function proc_close; use function proc_open; use function sprintf; -use function str_contains; use const PATH_SEPARATOR; @@ -61,18 +61,32 @@ public function run(array $command, ?string $directory = null): int /** * Whether the program can run. A path must name an executable file. A * bare name must be an executable file in a directory on the PATH. + * + * Windows: basename() also reads `\` as a separator, so PHP_BINARY is a + * path. PATHEXT gives the endings to try, so `docker` finds `docker.exe`. + * Other systems do not set PATHEXT. + * + * An empty PATH entry is skipped. It does not name the filesystem root. */ private function exists(string $program): bool { - if (true === str_contains($program, '/')) { + if (basename($program) !== $program) { return true === is_file($program) && true === is_executable($program); } + $endings = ['', ...explode(';', (string) getenv('PATHEXT'))]; + foreach (explode(PATH_SEPARATOR, (string) getenv('PATH')) as $directory) { - $candidate = $directory . '/' . $program; + if ('' === $directory) { + continue; + } + + foreach ($endings as $ending) { + $candidate = $directory . '/' . $program . $ending; - if (true === is_file($candidate) && true === is_executable($candidate)) { - return true; + if (true === is_file($candidate) && true === is_executable($candidate)) { + return true; + } } } diff --git a/src/Project/Config.php b/src/Project/Config.php index 9065bce..7383531 100644 --- a/src/Project/Config.php +++ b/src/Project/Config.php @@ -104,7 +104,7 @@ private static function defaultPaths(Flavor $flavor): array { return match ($flavor) { Flavor::ADR => [ - 'action' => 'src/Action', + 'action' => 'src/Action', // Not an ADR artifact: a crest command is the same class in any // flavor. It sits here because ADR is the only populated set, // and moves to a shared one when cli, mvc and micro arrive. @@ -162,8 +162,6 @@ private static function fromArray(array $declared, string $root, string $source) $namespaces = $declared['namespaces']; } - // crest.php never restates the autoload map; namespaceFor() still needs - // it whenever `namespaces` does not answer the question outright. $bootstrap = null; if (true === isset($declared['bootstrap']) && true === is_string($declared['bootstrap'])) { $bootstrap = $declared['bootstrap']; @@ -175,6 +173,9 @@ private static function fromArray(array $declared, string $root, string $source) $root, $paths, $namespaces, + // crest.php never restates the autoload map; namespaceFor() + // still needs it whenever `namespaces` does not answer the + // question outright. self::psr4Map($root), $source, $stated, @@ -279,8 +280,8 @@ public function flavor(): Flavor } /** - * Whether the config file stated this top-level key, as opposed to it - * taking a default. `flavor`, `namespace`, `paths`, `namespaces`. + * Whether the config file stated this key, as opposed to it taking a + * default. `flavor`, `namespace`, `paths`, or `paths.` for one path. */ public function isDeclared(string $key): bool { diff --git a/tests/Support/Project/NonEnumerableFront.php b/tests/Support/Project/NonEnumerableFront.php new file mode 100644 index 0000000..9570498 --- /dev/null +++ b/tests/Support/Project/NonEnumerableFront.php @@ -0,0 +1,29 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Crest\Tests\Support\Project; + +/** + * Boots a container that can find services but cannot list them. No Phalcon + * container is like this, so the test sets the object: Bootstrap creates the + * front, and a test cannot give it the object in another way. + */ +final class NonEnumerableFront +{ + public static ?object $container = null; + + public function boot(): ?object + { + return self::$container; + } +} diff --git a/tests/Unit/Command/Container/ListCommandTest.php b/tests/Unit/Command/Container/ListCommandTest.php index e6ba602..14f83b1 100644 --- a/tests/Unit/Command/Container/ListCommandTest.php +++ b/tests/Unit/Command/Container/ListCommandTest.php @@ -22,12 +22,15 @@ use Crest\Tests\Support\Project\FailingFront; use Crest\Tests\Support\Project\NoBootFront; use Crest\Tests\Support\Project\NonContainerFront; +use Crest\Tests\Support\Project\NonEnumerableFront; use Crest\Tests\Support\Project\ServicesFront; use Crest\Tests\Support\Project\WrongContainerFront; use Crest\Tests\Support\ScratchDirectory; +use Phalcon\Contracts\Container\Service\Collection; use PHPUnit\Framework\TestCase; use function file_put_contents; +use function get_class; use function preg_replace; use function str_replace; @@ -47,6 +50,8 @@ protected function setUp(): void protected function tearDown(): void { + NonEnumerableFront::$container = null; + $this->closeStreams(); $this->removeScratchDirectory(); } @@ -87,6 +92,24 @@ public function testABootThatThrowsIsReportedAsABootFailure(): void ); } + public function testAContainerThatCannotListItsServicesIsReported(): void + { + // Enumerable is an optional contract. A container can find services + // by name and still be unable to name them. + $container = $this->createStub(Collection::class); + + NonEnumerableFront::$container = $container; + $this->declareFront(NonEnumerableFront::class); + + $status = $this->runCommand(); + + $this->assertSame(1, $status); + $this->assertStringContainsString( + get_class($container) . ' cannot list its services', + $this->readStderr() + ); + } + public function testAFrontWithNoBootIsReported(): void { $this->declareFront(NoBootFront::class); diff --git a/tests/Unit/Command/Make/ActionCommandTest.php b/tests/Unit/Command/Make/ActionCommandTest.php index 47a35b2..28101da 100644 --- a/tests/Unit/Command/Make/ActionCommandTest.php +++ b/tests/Unit/Command/Make/ActionCommandTest.php @@ -28,6 +28,8 @@ use function file_put_contents; use function mkdir; +use const PHP_EOL; + final class ActionCommandTest extends TestCase { use GeneratesInAScratchProject; @@ -350,10 +352,17 @@ public function testTheViewResponderReportsTheTemplateItAsksFor(): void // renderer knows where it lives. $this->runCommand(['GET', '/company/all', '--responder=view']); - $output = $this->readStdout(); - - $this->assertStringContainsString('Nothing renders it yet', $output); - $this->assertStringContainsString(' company/all/index', $output); + // Asserted whole: each blank line and each text line is a separate call. + $expected = 'Answers GET /company/all' . PHP_EOL + . PHP_EOL + . 'Nothing renders it yet. The responder asks for this template:' . PHP_EOL + . PHP_EOL + . ' company/all/index' . PHP_EOL + . PHP_EOL + . 'Create it wherever your renderer looks. Renderer::render() takes a name, not a path, ' + . 'so the directory and the extension belong to the renderer rather than to crest.' . PHP_EOL; + + $this->assertStringEndsWith($expected, $this->readStdout()); } public function testUnknownResponderIsRejected(): void diff --git a/tests/Unit/Command/NewCommandTest.php b/tests/Unit/Command/NewCommandTest.php index 50238ac..d2d075f 100644 --- a/tests/Unit/Command/NewCommandTest.php +++ b/tests/Unit/Command/NewCommandTest.php @@ -49,6 +49,18 @@ protected function tearDown(): void $this->endScratchProject(); } + /** + * @return iterable + */ + public static function unusablePhpVersions(): iterable + { + // Each one passes version_compare(), but no `php:-cli` image + // has that tag. + yield 'leading zero in the major' => ['08.4']; + yield 'leading zero in the minor' => ['8.01']; + yield 'trailing newline' => ["8.4\n"]; + } + /** * @return iterable */ @@ -142,6 +154,20 @@ public function testAnUnusableNamespaceIsRejected(): void $this->assertDirectoryDoesNotExist($this->root . '/my-app'); } + /** + * @dataProvider unusablePhpVersions + */ + public function testAnUnusablePhpVersionIsRejected(string $version): void + { + $status = $this->runCommand(['my-app', '--php', $version]); + + $this->assertSame(1, $status); + $this->assertStringContainsString( + sprintf("'%s' is not a PHP version; expected major.minor, e.g. 8.4", $version), + $this->readStderr() + ); + } + public function testAPhpVersionBelowTheFloorIsRejected(): void { $status = $this->runCommand(['my-app', '--php', '8.0']); @@ -189,6 +215,14 @@ public function testAProjectNameThatIsAPathIsRejected(): void $this->assertDirectoryDoesNotExist(dirname($this->root) . '/elsewhere'); } + public function testAProjectNameWithATrailingNewlineIsRejected(): void + { + $status = $this->runCommand(["my-app\n"]); + + $this->assertSame(1, $status); + $this->assertStringContainsString("'my-app\n' is not a usable project name", $this->readStderr()); + } + public function testAProjectStubFromStubPublishIsUsed(): void { // stub:publish must write where `new` reads, with the same @@ -335,6 +369,19 @@ public function testForceWritesIntoANonEmptyDirectory(): void $this->assertFileExists($this->root . '/my-app/composer.json'); } + public function testHelpDoesNotCallTheDirectoryAProjectRoot(): void + { + // `new` has no project yet. The option replaces the working directory, + // and the project goes into it. + $status = $this->runInWorkingDirectory(['--help']); + + $this->assertSame(0, $status); + $this->assertMatchesRegularExpression( + '/--directory +Directory to use instead of the working directory\R/', + $this->readStdout() + ); + } + public function testNameArgumentIsRequired(): void { $status = $this->runCommand([]); diff --git a/tests/Unit/Command/Route/ListCommandTest.php b/tests/Unit/Command/Route/ListCommandTest.php index 3a17592..4c164b6 100644 --- a/tests/Unit/Command/Route/ListCommandTest.php +++ b/tests/Unit/Command/Route/ListCommandTest.php @@ -63,6 +63,29 @@ public function testAClassTheConventionWouldNotProduceIsSkipped(): void $this->assertStringNotContainsString('SomeHelper', $output); } + public function testAClassWithAPathButNoMethodIsSkipped(): void + { + // The framework answers null for both or for neither. A resolver that + // answers only the path shows that either null skips the class. + $this->writeAction('Health', 'GetHealth', 'App\Action\Health'); + + $command = new ListCommand(new StubActionResolver('', '/health')); + + $status = $command->handle( + new Input( + 'route:list', + $command->define()->merge(Kernel::globals())->bind(['--directory', $this->root]) + ), + new Output($this->stdout, $this->stderr, false) + ); + + $this->assertSame(0, $status); + $this->assertSame( + 'no actions found in ' . $this->root . '/src/Action' . PHP_EOL, + $this->readStdout() + ); + } + public function testARootLevelActionIsListed(): void { // No namespace segments at all, so the verb is the entire class name - diff --git a/tests/Unit/Command/Stub/PublishCommandTest.php b/tests/Unit/Command/Stub/PublishCommandTest.php index a3091c0..ab53e30 100644 --- a/tests/Unit/Command/Stub/PublishCommandTest.php +++ b/tests/Unit/Command/Stub/PublishCommandTest.php @@ -196,6 +196,14 @@ public function testAStubNameThatIsAPathIsRejected(): void ); } + public function testAStubNameWithATrailingNewlineIsRejected(): void + { + $status = $this->runCommand(["action\n"]); + + $this->assertSame(1, $status); + $this->assertStringContainsString("'action\n' is not a stub name", $this->readStderr()); + } + public function testAStubThatIsNotPackagedIsReported(): void { $status = $this->runCommand(['nope']); diff --git a/tests/Unit/Generator/ClassNameTest.php b/tests/Unit/Generator/ClassNameTest.php index 37b57fc..cc171a2 100644 --- a/tests/Unit/Generator/ClassNameTest.php +++ b/tests/Unit/Generator/ClassNameTest.php @@ -85,6 +85,14 @@ public function testANamespaceWithAnEmptySegmentIsRejected(): void ClassName::namespace('Acme\\\\Shop'); } + public function testANamespaceWithATrailingNewlineIsRejected(): void + { + $this->expectException(Exception::class); + $this->expectExceptionMessage("'Acme\n' is not a usable namespace"); + + ClassName::namespace("Acme\n"); + } + public function testANameWithASpaceIsRejected(): void { $this->expectException(Exception::class); @@ -93,6 +101,14 @@ public function testANameWithASpaceIsRejected(): void ClassName::suffixed('My Responder', 'Responder'); } + public function testANameWithATrailingNewlineIsRejected(): void + { + $this->expectException(Exception::class); + $this->expectExceptionMessage("'Album\n' is not a usable class name"); + + ClassName::suffixed("Album\n", 'Responder'); + } + public function testAnEmptyNameIsRejected(): void { $this->expectException(Exception::class); diff --git a/tests/Unit/Process/ShellRunnerTest.php b/tests/Unit/Process/ShellRunnerTest.php index cf0b026..17c6b8b 100644 --- a/tests/Unit/Process/ShellRunnerTest.php +++ b/tests/Unit/Process/ShellRunnerTest.php @@ -31,15 +31,19 @@ final class ShellRunnerTest extends TestCase private false | string $savedPath = false; + private false | string $savedPathext = false; + protected function setUp(): void { $this->makeScratchDirectory('shell-runner', 'bin', 'work'); - $this->savedPath = getenv('PATH'); + $this->savedPath = getenv('PATH'); + $this->savedPathext = getenv('PATHEXT'); } protected function tearDown(): void { putenv(false === $this->savedPath ? 'PATH' : 'PATH=' . $this->savedPath); + putenv(false === $this->savedPathext ? 'PATHEXT' : 'PATHEXT=' . $this->savedPathext); $this->removeScratchDirectory(); } @@ -73,6 +77,28 @@ public function testAMissingProgramIsReportedByName(): void (new ShellRunner())->run(['docker', 'compose', 'up', '-d']); } + public function testANameWithItsEndingIsFoundWhenPathextIsSet(): void + { + // Windows: `docker.exe` is found as it is, not as `docker.exe.exe`. + $this->executable('bin/hello.cmd', "#!/bin/sh\nexit 0\n"); + putenv('PATH=' . $this->root . '/bin'); + putenv('PATHEXT=.cmd'); + + $this->expectFoundProgram(); + + (new ShellRunner())->run(['hello.cmd'], $this->root . '/missing'); + } + + public function testAnEmptyPathEntryIsSkipped(): void + { + // An empty entry does not name the filesystem root, and it does not + // stop the search. + $this->executable('bin/hello', "#!/bin/sh\nexit 4\n"); + putenv('PATH=:' . $this->root . '/bin'); + + $this->assertSame(4, (new ShellRunner())->run(['hello'])); + } + public function testANonExecutableFileIsNotRun(): void { file_put_contents($this->root . '/bin/plain', "#!/bin/sh\nexit 0\n"); @@ -104,6 +130,19 @@ public function testAnUnsetPathFindsNothing(): void (new ShellRunner())->run(['sh']); } + public function testPathextGivesTheEndingsToTry(): void + { + // Windows finds `docker` as `docker.exe`. The match is the second + // entry, so every entry is tried. + $this->executable('bin/hello.cmd', "#!/bin/sh\nexit 0\n"); + putenv('PATH=' . $this->root . '/bin'); + putenv('PATHEXT=.BAT;.cmd'); + + $this->expectFoundProgram(); + + (new ShellRunner())->run(['hello'], $this->root . '/missing'); + } + public function testTheExitStatusIsReturned(): void { $this->assertSame(3, (new ShellRunner())->run([PHP_BINARY, '-r', 'exit(3);'])); @@ -124,4 +163,14 @@ private function executable(string $path, string $contents): void file_put_contents($this->root . '/' . $path, $contents); chmod($this->root . '/' . $path, 0o755); } + + /** + * Linux cannot start `hello` as `hello.cmd`. The directory check comes + * after the program check, so its error shows that the program was found. + */ + private function expectFoundProgram(): void + { + $this->expectException(Exception::class); + $this->expectExceptionMessage($this->root . '/missing is not a directory'); + } }