From 41c66b1a2d2e7e8fd576c3bb3f422e5930d71928 Mon Sep 17 00:00:00 2001 From: Guilliam Xavier Date: Mon, 24 May 2021 10:15:10 +0200 Subject: [PATCH 1/4] Revert "Update docs and add more helpful output to validate command, refs #9782" This reverts commit 458bd41d8fe8df7e3da7dad8cfd09ea4dfc0d3cd. --- doc/04-schema.md | 4 +--- src/Composer/Util/ConfigValidator.php | 10 ---------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/doc/04-schema.md b/doc/04-schema.md index 95e18ae49..05149d331 100644 --- a/doc/04-schema.md +++ b/doc/04-schema.md @@ -94,9 +94,7 @@ Out of the box, Composer supports four types: CMSs like the [SilverStripe installer](https://github.com/silverstripe/silverstripe-installer) or full fledged applications distributed as packages. This can for example be used by IDEs to provide listings of projects to initialize when creating - a new workspace. Setting the type to `project` also makes the `name` and - `description` fields optional, making it a good choice for closed source - projects wishing to use `composer validate`. + a new workspace. - **metapackage:** An empty package that contains requirements and will trigger their installation, but contains no files and will not write anything to the filesystem. As such, it does not require a dist or source key to be diff --git a/src/Composer/Util/ConfigValidator.php b/src/Composer/Util/ConfigValidator.php index f98bbd74f..dd2cbcb0d 100644 --- a/src/Composer/Util/ConfigValidator.php +++ b/src/Composer/Util/ConfigValidator.php @@ -63,16 +63,6 @@ class ConfigValidator $json->validateSchema(); } catch (JsonValidationException $e) { foreach ($e->getErrors() as $message) { - if ($message === 'type : The property type is required') { - $message .= ' (see https://getcomposer.org/doc/04-schema.md#type)'; - } - if ($message === 'name : The property name is required') { - $message .= ' (or set "type" to "project" to remove this requirement)'; - } - if ($message === 'description : The property description is required') { - $message .= ' (or set "type" to "project" to remove this requirement)'; - } - if ($laxValid) { $publishErrors[] = $message; } else { From 9e2cb30dfb50f83baec05a7a0302a76c86ec37cd Mon Sep 17 00:00:00 2001 From: Guilliam Xavier Date: Mon, 24 May 2021 10:15:16 +0200 Subject: [PATCH 2/4] Revert "Merge remote-tracking branch 'BoShurik/schema'" This reverts commit 89c3045e2b437dd04d52394d0989ee4c07d7f16c, reversing changes made to 991985792d236a002dce6adfb50f2bf0625fbab0. --- res/composer-schema.json | 21 +------------------ src/Composer/Json/JsonFile.php | 2 +- .../Composer/Test/Json/ComposerSchemaTest.php | 20 ------------------ 3 files changed, 2 insertions(+), 41 deletions(-) diff --git a/res/composer-schema.json b/res/composer-schema.json index 836409b43..d6e149c9d 100644 --- a/res/composer-schema.json +++ b/res/composer-schema.json @@ -3,26 +3,7 @@ "title": "Package", "type": "object", "additionalProperties": false, - "oneOf": [ - { - "properties": { - "type": { - "not": { - "enum": ["project"] - } - } - }, - "required": ["name", "description"] - }, - { - "properties": { - "type": { - "enum": ["project"] - } - }, - "required": ["type"] - } - ], + "required": [ "name", "description" ], "properties": { "name": { "type": "string", diff --git a/src/Composer/Json/JsonFile.php b/src/Composer/Json/JsonFile.php index ff4dcc8c8..b15098d56 100644 --- a/src/Composer/Json/JsonFile.php +++ b/src/Composer/Json/JsonFile.php @@ -198,7 +198,7 @@ class JsonFile if ($schema === self::LAX_SCHEMA) { $schemaData->additionalProperties = true; - $schemaData->oneOf = null; + $schemaData->required = array(); } $validator = new Validator(); diff --git a/tests/Composer/Test/Json/ComposerSchemaTest.php b/tests/Composer/Test/Json/ComposerSchemaTest.php index 877ef0666..0eece664c 100644 --- a/tests/Composer/Test/Json/ComposerSchemaTest.php +++ b/tests/Composer/Test/Json/ComposerSchemaTest.php @@ -41,38 +41,18 @@ class ComposerSchemaTest extends TestCase { $json = '{ }'; $result = $this->check($json); - $this->assertContains(array('property' => 'type', 'message' => 'The property type is required', 'constraint' => 'required'), $result); $this->assertContains(array('property' => 'name', 'message' => 'The property name is required', 'constraint' => 'required'), $result); $this->assertContains(array('property' => 'description', 'message' => 'The property description is required', 'constraint' => 'required'), $result); - $this->assertContains(array('property' => '', 'message' => 'Failed to match exactly one schema', 'constraint' => 'oneOf'), $result); $json = '{ "name": "vendor/package" }'; $this->assertEquals(array( - array('property' => 'type', 'message' => 'The property type is required', 'constraint' => 'required'), array('property' => 'description', 'message' => 'The property description is required', 'constraint' => 'required'), - array('property' => '', 'message' => 'Failed to match exactly one schema', 'constraint' => 'oneOf'), ), $this->check($json)); $json = '{ "description": "generic description" }'; $this->assertEquals(array( - array('property' => 'type', 'message' => 'The property type is required', 'constraint' => 'required'), array('property' => 'name', 'message' => 'The property name is required', 'constraint' => 'required'), - array('property' => '', 'message' => 'Failed to match exactly one schema', 'constraint' => 'oneOf'), ), $this->check($json)); - - $json = '{ "type": "library" }'; - $this->assertEquals(array( - array('property' => 'type', 'message' => 'Does not have a value in the enumeration ["project"]', 'constraint' => 'enum', 'enum' => array('project')), - array('property' => 'name', 'message' => 'The property name is required', 'constraint' => 'required'), - array('property' => 'description', 'message' => 'The property description is required', 'constraint' => 'required'), - array('property' => '', 'message' => 'Failed to match exactly one schema', 'constraint' => 'oneOf'), - ), $this->check($json)); - - $json = '{ "type": "project" }'; - $this->assertTrue($this->check($json)); - - $json = '{ "name": "vendor/package", "description": "description" }'; - $this->assertTrue($this->check($json)); } public function testOptionalAbandonedProperty() From 2d21dd675a52339a0868d91680dea20a9c401ef2 Mon Sep 17 00:00:00 2001 From: Guilliam Xavier Date: Mon, 24 May 2021 11:51:04 +0200 Subject: [PATCH 3/4] Invert strict/lax schema validation --- res/composer-schema.json | 2 - src/Composer/Json/JsonFile.php | 6 +- .../Composer/Test/Json/ComposerSchemaTest.php | 18 ---- tests/Composer/Test/Json/JsonFileTest.php | 84 +++++++++++++++++++ 4 files changed, 87 insertions(+), 23 deletions(-) diff --git a/res/composer-schema.json b/res/composer-schema.json index d6e149c9d..96b708f88 100644 --- a/res/composer-schema.json +++ b/res/composer-schema.json @@ -2,8 +2,6 @@ "$schema": "https://json-schema.org/draft-04/schema#", "title": "Package", "type": "object", - "additionalProperties": false, - "required": [ "name", "description" ], "properties": { "name": { "type": "string", diff --git a/src/Composer/Json/JsonFile.php b/src/Composer/Json/JsonFile.php index b15098d56..07a6f922f 100644 --- a/src/Composer/Json/JsonFile.php +++ b/src/Composer/Json/JsonFile.php @@ -196,9 +196,9 @@ class JsonFile $schemaData = (object) array('$ref' => $schemaFile); - if ($schema === self::LAX_SCHEMA) { - $schemaData->additionalProperties = true; - $schemaData->required = array(); + if ($schema !== self::LAX_SCHEMA) { + $schemaData->additionalProperties = false; + $schemaData->required = array('name', 'description'); } $validator = new Validator(); diff --git a/tests/Composer/Test/Json/ComposerSchemaTest.php b/tests/Composer/Test/Json/ComposerSchemaTest.php index 0eece664c..5c53b2937 100644 --- a/tests/Composer/Test/Json/ComposerSchemaTest.php +++ b/tests/Composer/Test/Json/ComposerSchemaTest.php @@ -37,24 +37,6 @@ class ComposerSchemaTest extends TestCase $this->assertEquals($expectedError, $this->check($json)); } - public function testRequiredProperties() - { - $json = '{ }'; - $result = $this->check($json); - $this->assertContains(array('property' => 'name', 'message' => 'The property name is required', 'constraint' => 'required'), $result); - $this->assertContains(array('property' => 'description', 'message' => 'The property description is required', 'constraint' => 'required'), $result); - - $json = '{ "name": "vendor/package" }'; - $this->assertEquals(array( - array('property' => 'description', 'message' => 'The property description is required', 'constraint' => 'required'), - ), $this->check($json)); - - $json = '{ "description": "generic description" }'; - $this->assertEquals(array( - array('property' => 'name', 'message' => 'The property name is required', 'constraint' => 'required'), - ), $this->check($json)); - } - public function testOptionalAbandonedProperty() { $json = '{"name": "vendor/package", "description": "description", "abandoned": true}'; diff --git a/tests/Composer/Test/Json/JsonFileTest.php b/tests/Composer/Test/Json/JsonFileTest.php index 8f8b6a2c2..3a19b2fe7 100644 --- a/tests/Composer/Test/Json/JsonFileTest.php +++ b/tests/Composer/Test/Json/JsonFileTest.php @@ -14,6 +14,7 @@ namespace Composer\Test\Json; use Seld\JsonLint\ParsingException; use Composer\Json\JsonFile; +use Composer\Json\JsonValidationException; use Composer\Test\TestCase; class JsonFileTest extends TestCase @@ -93,6 +94,89 @@ class JsonFileTest extends TestCase { $json = new JsonFile(__DIR__.'/Fixtures/composer.json'); $this->assertTrue($json->validateSchema()); + $this->assertTrue($json->validateSchema(JsonFile::LAX_SCHEMA)); + } + + public function testSchemaValidationError() + { + $file = tempnam(sys_get_temp_dir(), 'c'); + file_put_contents($file, '{ "name": null }'); + $json = new JsonFile($file); + $expectedMessage = sprintf('"%s" does not match the expected JSON schema', $file); + $expectedError = 'name : NULL value found, but a string is required'; + try { + $json->validateSchema(); + $this->fail('Expected exception to be thrown (strict)'); + } catch (JsonValidationException $e) { + $this->assertEquals($expectedMessage, $e->getMessage()); + $this->assertContains($expectedError, $e->getErrors()); + } + try { + $json->validateSchema(JsonFile::LAX_SCHEMA); + $this->fail('Expected exception to be thrown (lax)'); + } catch (JsonValidationException $e) { + $this->assertEquals($expectedMessage, $e->getMessage()); + $this->assertContains($expectedError, $e->getErrors()); + } + unlink($file); + } + + public function testSchemaValidationLaxAdditionalProperties() + { + $file = tempnam(sys_get_temp_dir(), 'c'); + file_put_contents($file, '{ "name": "vendor/package", "description": "generic description", "foo": "bar" }'); + $json = new JsonFile($file); + try { + $json->validateSchema(); + $this->fail('Expected exception to be thrown (strict)'); + } catch (JsonValidationException $e) { + $this->assertEquals(sprintf('"%s" does not match the expected JSON schema', $file), $e->getMessage()); + $this->assertEquals(array('The property foo is not defined and the definition does not allow additional properties'), $e->getErrors()); + } + $this->assertTrue($json->validateSchema(JsonFile::LAX_SCHEMA)); + unlink($file); + } + + public function testSchemaValidationLaxRequired() + { + $file = tempnam(sys_get_temp_dir(), 'c'); + $json = new JsonFile($file); + + $expectedMessage = sprintf('"%s" does not match the expected JSON schema', $file); + + file_put_contents($file, '{ }'); + try { + $json->validateSchema(); + $this->fail('Expected exception to be thrown (strict)'); + } catch (JsonValidationException $e) { + $this->assertEquals($expectedMessage, $e->getMessage()); + $errors = $e->getErrors(); + $this->assertContains('name : The property name is required', $errors); + $this->assertContains('description : The property description is required', $errors); + } + $this->assertTrue($json->validateSchema(JsonFile::LAX_SCHEMA)); + + file_put_contents($file, '{ "name": "vendor/package" }'); + try { + $json->validateSchema(); + $this->fail('Expected exception to be thrown (strict)'); + } catch (JsonValidationException $e) { + $this->assertEquals($expectedMessage, $e->getMessage()); + $this->assertEquals(array('description : The property description is required'), $e->getErrors()); + } + $this->assertTrue($json->validateSchema(JsonFile::LAX_SCHEMA)); + + file_put_contents($file, '{ "description": "generic description" }'); + try { + $json->validateSchema(); + $this->fail('Expected exception to be thrown (strict)'); + } catch (JsonValidationException $e) { + $this->assertEquals($expectedMessage, $e->getMessage()); + $this->assertEquals(array('name : The property name is required'), $e->getErrors()); + } + $this->assertTrue($json->validateSchema(JsonFile::LAX_SCHEMA)); + + unlink($file); } public function testParseErrorDetectMissingCommaMultiline() From 393c9a594635fe184d38045828c7cbbfa946a6ea Mon Sep 17 00:00:00 2001 From: Guilliam Xavier Date: Mon, 24 May 2021 14:42:23 +0200 Subject: [PATCH 4/4] Add more tests --- tests/Composer/Test/Json/JsonFileTest.php | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/Composer/Test/Json/JsonFileTest.php b/tests/Composer/Test/Json/JsonFileTest.php index 3a19b2fe7..eb0646ae4 100644 --- a/tests/Composer/Test/Json/JsonFileTest.php +++ b/tests/Composer/Test/Json/JsonFileTest.php @@ -176,6 +176,34 @@ class JsonFileTest extends TestCase } $this->assertTrue($json->validateSchema(JsonFile::LAX_SCHEMA)); + file_put_contents($file, '{ "type": "library" }'); + try { + $json->validateSchema(); + $this->fail('Expected exception to be thrown (strict)'); + } catch (JsonValidationException $e) { + $this->assertEquals($expectedMessage, $e->getMessage()); + $errors = $e->getErrors(); + $this->assertContains('name : The property name is required', $errors); + $this->assertContains('description : The property description is required', $errors); + } + $this->assertTrue($json->validateSchema(JsonFile::LAX_SCHEMA)); + + file_put_contents($file, '{ "type": "project" }'); + try { + $json->validateSchema(); + $this->fail('Expected exception to be thrown (strict)'); + } catch (JsonValidationException $e) { + $this->assertEquals($expectedMessage, $e->getMessage()); + $errors = $e->getErrors(); + $this->assertContains('name : The property name is required', $errors); + $this->assertContains('description : The property description is required', $errors); + } + $this->assertTrue($json->validateSchema(JsonFile::LAX_SCHEMA)); + + file_put_contents($file, '{ "name": "vendor/package", "description": "generic description" }'); + $this->assertTrue($json->validateSchema()); + $this->assertTrue($json->validateSchema(JsonFile::LAX_SCHEMA)); + unlink($file); }