From e831cb58413f11c52abe13805b0c075d066232db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Thu, 31 May 2018 18:09:57 +0200 Subject: [PATCH] Rework default values (#251) --- doc/configuration.md | 163 +++++++++++++------------- res/schema.json | 56 ++++----- src/Configuration.php | 23 +++- src/Console/Command/Compile.php | 2 +- tests/ConfigurationTest.php | 146 ++++++++++++++++++++++- tests/Console/Command/CompileTest.php | 2 +- 6 files changed, 271 insertions(+), 121 deletions(-) diff --git a/doc/configuration.md b/doc/configuration.md index 82746b881..af7e7beb2 100644 --- a/doc/configuration.md +++ b/doc/configuration.md @@ -46,7 +46,8 @@ This command relies on a configuration file for loading PHAR packaging settings. specified through the `--configuration|-c option`, one of the following files will be used (in order): `box.json`, `box.json.dist`. If no configuration file is found, Box will proceed with the default settings. -The configuration file is a JSON object saved to a file. Note that all settings are optional. +The configuration file is a JSON object saved to a file. Note that **all settings are optional**. If a setting is set +to `null`, then its default value will be picked and is strictly equivalent to not setting the value. ```json { @@ -103,9 +104,9 @@ configuration file is given or the current working directory otherwise. ## Main (`main`) -The main (`string`|`false`) setting is used to specify the file (relative to [`base-path`][base-path]) that will be run -when the PHAR is executed from the command line (To not confuse with the [stub][stub] which is the PHAR bootstrapping -file). +The main (`string`|`false`|`null`) setting is used to specify the file (relative to [`base-path`][base-path]) that will +be run when the PHAR is executed from the command line (To not confuse with the [stub][stub] which is the PHAR +bootstrapping file). When you have a main script file that can be used as a [stub][stub], you can disable the main script by setting it to false: @@ -117,9 +118,9 @@ false: } ``` -When the parameter is not given, Box tries to guess the binary of the application with the `composer.json` file. If the -[Composer `bin`][composer-bin] is set, Box will pick the first value provided. Otherwise it will fallback on the -[PHAR][phar class] default file used which is `index.php`. +When the parameter is not given or set to `null`, Box tries to guess the binary of the application with the +`composer.json` file. If the [Composer `bin`][composer-bin] is set, Box will pick the first value provided. Otherwise it +will fallback on the [PHAR][phar class] default file used which is `index.php`. The main file contents is processed by the [compactors][compactors] as the other files. @@ -129,11 +130,11 @@ If the main file starts with a shebang line (`#!`), it will be automatically rem ## Output (`output`) -The output (`string`) setting specifies the file name and path of the newly built PHAR. If the value of the setting is -not an absolute path, the path will be relative to the base path. +The output (`string`|`null`) setting specifies the file name and path of the newly built PHAR. If the value of the +setting is not an absolute path, the path will be relative to the base path. -If not provided, the default value used will based on the [`main`][main]. For example if the main file is `bin/acme.php` -or `bin/acme` then the output will be `bin/acme.phar`. +If not provided or set to `null`, the default value used will based on the [`main`][main]. For example if the main file +is `bin/acme.php` or `bin/acme` then the output will be `bin/acme.phar`. ## Permissions (`chmod`) @@ -146,12 +147,14 @@ Check the following [link](https://secure.php.net/manual/en/function.chmod.php) ## Check requirements (`check-requirements`) -The check requirements setting (`boolean`, `true` by default) is used to allow the PHAR to check for the application -constraint before running. See more information about it [here][requirement-checker]. +The check requirements setting (`boolean`|`null`, default `true`) is used to allow the PHAR to check for the application +constraint before running. See more information about it [here][requirement-checker]. If not set or set to `null`, then +the requirement checker will be added. Note that this is true only if either the `composer.json` or `composer.lock` +could have been found. -**Warning**: this check is still done within the PHAR. As a result, if [the required extension to open the PHAR][compression] -due to the compression algorithm is not loaded, a hard failure will still appear: the requirement checker _cannot_ be -executed before that. +**Warning**: this check is still done within the PHAR. As a result, if +[the required extension to open the PHAR][compression] due to the compression algorithm is not loaded, a hard failure +will still appear: the requirement checker _cannot_ be executed before that. ## Including files @@ -179,9 +182,9 @@ binary files, the regular file will take precedence. ### Files (`files` and `files-bin`) -The `files` (`string[]`) setting is a list of files paths relative to [`base-path`][base-path] unless absolute. Each -file will be processed by the [compactors][compactors], have their placeholder values replaced (see: `replacements`) -and added to the PHAR. +The `files` (`string[]`|`null` default `[]`) setting is a list of files paths relative to [`base-path`][base-path] +unless absolute. Each file will be processed by the [compactors][compactors], have their placeholder values replaced +(see: [`replacements`][placeholders]) and added to the PHAR. This setting is not affected by the [`blacklist`][blacklist] setting. @@ -192,9 +195,9 @@ such as images, those that contain binary data or simply a file you do not want ### Directories (`directories` and `directories-bin`) -The directories (`string[]`) setting is a list of directory paths relative to [`base-path`][base-path]. All files will -be processed by the [compactors][compactors], have their placeholder values replaced (see: `replacements`) and added to -the PHAR. +The directories (`string[]`|`null` default `[]`) setting is a list of directory paths relative to +[`base-path`][base-path]. All files will be processed by the [compactors][compactors], have their placeholder values +replaced (see: [`replacements`][placeholders]) and added to the PHAR. Files listed in the [`blacklist`][blacklist] will not be added to the PHAR. @@ -205,9 +208,9 @@ compactors. ### Finder (`finder` and `finder-bin`) -The finder (`object[]`) setting is a list of JSON objects. Each object (key, value) tuple is a (method, arguments) -of the [Symfony Finder][symfony-finder] used by Box. If an array of values is provided for a single key, the method will -be called once per value in the array. +The finder (`object[]`|`null` default `[]`) setting is a list of JSON objects. Each object (key, value) tuple is a +(method, arguments) of the [Symfony Finder][symfony-finder] used by Box. If an array of values is provided for a single +key, the method will be called once per value in the array. Note that the paths specified for the `in` method are relative to [`base-path`][base-path] and that the finder will account for the files registered in the [`blacklist`][blacklist]. @@ -244,9 +247,9 @@ Example: ### Blacklist (`blacklist`) -The `blacklist` (`string[]`) setting is a list of files that must not be added. The files blacklisted are the ones found -using the other available configuration settings: [`files`][files], [`files-bin`][files], [`directories`][directories], -[`directories-bin`][directories], [`finder`][finder], [`finder-bin`][finder]. +The `blacklist` (`string[]`|`null` default `[]`) setting is a list of files that must not be added. The files +blacklisted are the ones found using the other available configuration settings: [`files`][files], [`files-bin`][files], +[`directories`][directories], [`directories-bin`][directories], [`finder`][finder], [`finder-bin`][finder]. Note that all the blacklisted paths are relative to the settings configured above. For example if you have the following file structure: @@ -333,7 +336,8 @@ The default PHAR stub file can be used but Box also propose a couple of options ### Stub (`stub`) -The stub (`string`|`boolean`) setting is used to specify the location of a stub file or if one should be generated: +The stub (`string`|`boolean`|`null` default `true`) setting is used to specify the location of a stub file or if one +should be generated: - `string`: Path to the stub file will be used as is inside the PHAR - `true` (default): A new stub will be generated - `false`: The default stub used by the PHAR class will be used @@ -344,29 +348,27 @@ If a custom stub file is provided, none of the other options ([`shebang`][sheban ### Shebang (`shebang`) -The shebang (`string`|`null`) setting is used to specify the shebang line used when generating a new stub. By default, -this line is used: +The shebang (`string`|`false`|`null`) setting is used to specify the shebang line used when generating a new stub. By +default, this line is used: ``` #!/usr/bin/env php ``` -The shebang line can be removed altogether if set to `null`. +The shebang line can be removed altogether if set to `false`. ### Intercept (`intercept`) -The intercept (`boolean`) setting is used when generating a new stub. If setting is set to `true`, the -[Phar::interceptFileFuncs()][phar.interceptfilefuncs] method will be called in the stub. - -This setting is set to `false` by default. +The intercept (`boolean`|`null` default `false`) setting is used when generating a new stub. If setting is set to +`true`, the [Phar::interceptFileFuncs()][phar.interceptfilefuncs] method will be called in the stub. ### Alias (`alias`) -The `alias` (`string`) setting is used when generating a new stub to call the [`Phar::mapPhar()`](phar.mapphar). This -makes it easier to refer to files in the PHAR and ensure the access to internal files will always work regardless of the -location of the PHAR on the file system. +The `alias` (`string`|`null`) setting is used when generating a new stub to call the [`Phar::mapPhar()`](phar.mapphar). +This makes it easier to refer to files in the PHAR and ensure the access to internal files will always work regardless +of the location of the PHAR on the file system. If no alias is provided, a generated unique name will be used for it in order to map the [main file][main]. Note that this may have undesirable effects if you are using the generated [stub][stub-stub] @@ -416,8 +418,7 @@ require 'phar://alias.phar/index.php'; ``` If you are using the default stub, [`Phar::setAlias()`][phar.setalias] will be used. Note however that this will behave -slightly -differently. +slightly differently. Example: @@ -447,8 +448,9 @@ register the alias `box-alias.phar` to that new PHAR but as the alias is already ### Banner (`banner`) -The banner (`string`|`string[]`|`null`) setting is the banner comment that will be used when a new stub is generated. The -value of this setting must not already be enclosed within a comment block as it will be automatically done for you. +The banner (`string`|`string[]`|`false`|`null`) setting is the banner comment that will be used when a new stub is +generated. The value of this setting must not already be enclosed within a comment block as it will be automatically +done for you. For example `Custom banner` will result in the stub file: @@ -488,19 +490,19 @@ Will result in: */ ``` -By default, the Box banner is used. If set to `null`, no banner at all will be used. +By default, the Box banner is used. If set to `false`, no banner at all will be used. The content of this value is discarded if [`banner-file`][banner-file] is set. ### Banner file (`banner-file`) -The banner-file (`string`) setting is like banner, except it is a path (relative to [the base path][base-path]) to the -file that will contain the comment. +The banner-file (`string`|`null` ignored by default) setting is like banner, except it is a path (relative to +[the base path][base-path]) to the file that will contain the comment. Like banner, the comment must not already be enclosed in a comment block. -If this parameter is set, then the value of [`banner`][banner] will be discarded. +If this parameter is set to a different value than `null`, then the value of [`banner`][banner] will be discarded. ## Dumping the Composer autoloader (`dump-autoload`) @@ -529,8 +531,9 @@ by the [compactors][compactors] process. ## Compactors (`compactors`) -The compactors (`string[]`) setting is a list of file contents compacting classes that must be registered. A file -compacting class is used to reduce the size of a specific file type. The following is a simple example: +The compactors (`string[]`|`null` default `[]`) setting is a list of file contents compacting classes that must be +registered. A file compacting class is used to reduce the size of a specific file type. The following is a simple +example: ```php file; } @@ -1717,11 +1718,19 @@ private static function retrieveShebang(stdClass $raw): ?string return self::DEFAULT_SHEBANG; } - if (null === $raw->shebang) { + $shebang = $raw->shebang; + + if (false === $shebang) { return null; } - $shebang = trim($raw->shebang); + if (null === $shebang) { + $shebang = self::DEFAULT_SHEBANG; + } + + Assertion::string($shebang, 'Expected shebang to be either a string, false or null, found true'); + + $shebang = trim($shebang); Assertion::notEmpty($shebang, 'The shebang should not be empty.'); Assertion::true( @@ -1758,15 +1767,17 @@ private static function retrieveSigningAlgorithm(stdClass $raw): int private static function retrieveStubBannerContents(stdClass $raw): ?string { - if (false === array_key_exists('banner', (array) $raw)) { + if (false === array_key_exists('banner', (array) $raw) || null === $raw->banner) { return self::DEFAULT_BANNER; } - if (null === $raw->banner) { + $banner = $raw->banner; + + if (false === $banner) { return null; } - $banner = $raw->banner; + Assertion::true(is_string($banner) || is_array($banner), 'The banner cannot accept true as a value'); if (is_array($banner)) { $banner = implode("\n", $banner); diff --git a/src/Console/Command/Compile.php b/src/Console/Command/Compile.php index 652c542c6..9314c6637 100644 --- a/src/Console/Command/Compile.php +++ b/src/Console/Command/Compile.php @@ -246,7 +246,7 @@ private function removeExistingArtifacts(Configuration $config, BuildLogger $log remove(self::DEBUG_DIR); $date = (new DateTimeImmutable('now', new DateTimeZone('UTC')))->format(DATE_ATOM); - $file = null !== $config->getFile() ? $config->getFile() : 'No config file'; + $file = null !== $config->getConfigurationFile() ? $config->getConfigurationFile() : 'No config file'; remove(self::DEBUG_DIR); diff --git a/tests/ConfigurationTest.php b/tests/ConfigurationTest.php index 801461cef..635e07352 100644 --- a/tests/ConfigurationTest.php +++ b/tests/ConfigurationTest.php @@ -30,9 +30,11 @@ use const DIRECTORY_SEPARATOR; use const PHP_EOL; use function abs; +use function array_fill_keys; use function date_default_timezone_set; use function file_put_contents; use function KevinGH\Box\FileSystem\dump_file; +use function KevinGH\Box\FileSystem\file_contents; use function KevinGH\Box\FileSystem\remove; use function KevinGH\Box\FileSystem\rename; @@ -46,14 +48,14 @@ public function test_it_can_be_created_with_a_file(): void { $config = Configuration::create('box.json', new stdClass()); - $this->assertSame('box.json', $config->getFile()); + $this->assertSame('box.json', $config->getConfigurationFile()); } public function test_it_can_be_created_without_a_file(): void { $config = Configuration::create(null, new stdClass()); - $this->assertNull($config->getFile()); + $this->assertNull($config->getConfigurationFile()); } public function test_a_default_alias_is_generted_if_no_alias_is_registered(): void @@ -112,7 +114,7 @@ public function test_the_alias_must_be_a_string(): void $this->assertSame( <<file" does not match the expected JSON schema: - - alias : Boolean value found, but a string is required + - alias : Boolean value found, but a string or a null is required EOF , $exception->getMessage() @@ -1125,7 +1127,7 @@ public function test_it_cannot_retrieve_the_git_hash_if_not_in_a_git_repository( public function test_the_shebang_can_be_disabled(): void { $this->setConfig([ - 'shebang' => null, + 'shebang' => false, 'files' => [self::DEFAULT_FILE], ]); @@ -1147,6 +1149,20 @@ public function test_cannot_register_an_invalid_shebang(): void $exception->getMessage() ); } + + try { + $this->setConfig([ + 'shebang' => true, + 'files' => [self::DEFAULT_FILE], + ]); + + $this->fail('Expected exception ot be thrown.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'Expected shebang to be either a string, false or null, found true', + $exception->getMessage() + ); + } } public function test_cannot_register_an_empty_shebang(): void @@ -1231,6 +1247,20 @@ public function test_there_is_a_banner_registered_by_default(): void $expected = <<<'BANNER' Generated by Humbug Box. +@link https://github.com/humbug/box +BANNER; + + $this->assertSame($expected, $this->config->getStubBannerContents()); + $this->assertNull($this->config->getStubBannerPath()); + + $this->setConfig([ + 'banner' => null, + 'files' => [self::DEFAULT_FILE], + ]); + + $expected = <<<'BANNER' +Generated by Humbug Box. + @link https://github.com/humbug/box BANNER; @@ -1255,7 +1285,7 @@ public function test_a_custom_banner_can_be_registered(string $banner): void public function test_the_banner_can_be_disabled(): void { $this->setConfig([ - 'banner' => null, + 'banner' => false, 'files' => [self::DEFAULT_FILE], ]); @@ -1263,6 +1293,23 @@ public function test_the_banner_can_be_disabled(): void $this->assertNull($this->config->getStubBannerPath()); } + public function test_the_banner_must_be_a_valid_value(): void + { + try { + $this->setConfig([ + 'banner' => true, + 'files' => [self::DEFAULT_FILE], + ]); + + $this->fail('Expected exception to be thrown.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'The banner cannot accept true as a value', + $exception->getMessage() + ); + } + } + /** * @dataProvider provideUnormalizedCustomBanner */ @@ -1317,6 +1364,28 @@ public function test_a_custom_banner_from_a_file_can_be_registered(): void $this->assertSame($this->tmp.DIRECTORY_SEPARATOR.'banner', $this->config->getStubBannerPath()); } + public function test_the_banner_value_is_discarded_if_a_banner_file_is_registered(): void + { + $comment = <<<'COMMENT' +This is a + +multiline + +comment. +COMMENT; + + file_put_contents('banner', $comment); + + $this->setConfig([ + 'banner' => 'discarded banner', + 'banner-file' => 'banner', + 'files' => [self::DEFAULT_FILE], + ]); + + $this->assertSame($comment, $this->config->getStubBannerContents()); + $this->assertSame($this->tmp.DIRECTORY_SEPARATOR.'banner', $this->config->getStubBannerPath()); + } + public function test_the_content_of_the_custom_banner_file_is_normalized(): void { $comment = <<<'COMMENT' @@ -1482,6 +1551,60 @@ public function test_the_requirement_checker_can_be_disabled(): void $this->assertFalse($this->config->checkRequirements()); } + public function test_it_can_be_created_with_only_default_values(): void + { + $this->setConfig( + array_fill_keys( + $this->retrieveSchemaKeys(), + null + ) + ); + + $this->assertFalse($this->config->checkRequirements()); + $this->assertFalse($this->config->dumpAutoload()); + $this->assertTrue($this->config->excludeComposerFiles()); + $this->assertRegExp('/^box-auto-generated-alias-[\da-zA-Z]{13}\.phar$/', $this->config->getAlias()); + $this->assertSame($this->tmp, $this->config->getBasePath()); + $this->assertSame([], $this->config->getBinaryFiles()); + $this->assertSame([], $this->config->getCompactors()); + $this->assertNull($this->config->getComposerJson()); + $this->assertNull($this->config->getComposerLock()); + $this->assertNull($this->config->getCompressionAlgorithm()); + $this->assertNull($this->config->getDecodedComposerJsonContents()); + $this->assertNull($this->config->getDecodedComposerLockContents()); + $this->assertSame($this->tmp.'/box.json', $this->config->getConfigurationFile()); + $this->assertEquals( + new MapFile($this->tmp, []), + $this->config->getFileMapper() + ); + $this->assertNull($this->config->getFileMode()); + $this->assertSame([], $this->config->getFiles()); + $this->assertSame('', $this->config->getMainScriptContents()); + $this->assertSame($this->tmp.'/index.php', $this->config->getMainScriptPath()); + $this->assertNull($this->config->getMetadata()); + $this->assertSame($this->tmp.'/index.phar', $this->config->getOutputPath()); + $this->assertNull($this->config->getPrivateKeyPassphrase()); + $this->assertNull($this->config->getPrivateKeyPath()); + $this->assertSame([], $this->config->getReplacements()); + $this->assertSame('#!/usr/bin/env php', $this->config->getShebang()); + $this->assertSame(Phar::SHA1, $this->config->getSigningAlgorithm()); + $this->assertSame( + <<<'BANNER' +Generated by Humbug Box. + +@link https://github.com/humbug/box +BANNER + , + $this->config->getStubBannerContents() + ); + $this->assertNull($this->config->getStubPath()); + $this->assertSame($this->tmp.'/index.phar', $this->config->getTmpOutputPath()); + $this->assertTrue($this->config->hasMainScript()); + $this->assertFalse($this->config->isInterceptFileFuncs()); + $this->assertFalse($this->config->isPrivateKeyPrompt()); + $this->assertTrue($this->config->isStubGenerated()); + } + public function provideInvalidCompressionAlgorithms(): Generator { yield 'Invalid string key' => [ @@ -1691,4 +1814,17 @@ function (): void { null, ]; } + + /** + * @return string[] + */ + private function retrieveSchemaKeys(): array + { + $schema = json_decode( + file_contents(__DIR__.'/../res/schema.json'), + true + ); + + return array_keys($schema['properties']); + } } diff --git a/tests/Console/Command/CompileTest.php b/tests/Console/Command/CompileTest.php index c51934492..4cfa1a115 100644 --- a/tests/Console/Command/CompileTest.php +++ b/tests/Console/Command/CompileTest.php @@ -2460,7 +2460,7 @@ public function test_it_can_build_a_PHAR_file_without_a_shebang_line(): void mirror(self::FIXTURES_DIR.'/dir006', $this->tmp); $boxRawConfig = json_decode(file_get_contents('box.json'), true, 512, JSON_PRETTY_PRINT); - $boxRawConfig['shebang'] = null; + $boxRawConfig['shebang'] = false; file_put_contents('box.json', json_encode($boxRawConfig), JSON_PRETTY_PRINT); $this->commandTester->execute(