8000 feature #51172 [Serializer] Add support for seld/jsonlint (ostrolucky) · symfony/symfony@3b34568 · GitHub
[go: up one dir, main page]

Skip to content

Commit 3b34568

Browse files
committed
feature #51172 [Serializer] Add support for seld/jsonlint (ostrolucky)
This PR was merged into the 6.4 branch. Discussion ---------- [Serializer] Add support for seld/jsonlint | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | | License | MIT | Doc PR | This is kinda RFC proposal for improving JSON parsing errors in API context. At the moment, pretty much any time there is something wrong with input payload, framework will return something like so ```json { "title": "Deserialization Failed", "detail": "Syntax error" } ``` This provides very minimal info to go on. Also, "Syntax error" is quite confusing for majority of PHP devs I know (and _especially_ frontend developers, to whom these errors display), as they don't realize this is how PHP's json_decode reports these issues. Wouldn't it be better if we got something like ```json { "title": "Deserialization Failed", "detail": "Parse error on line 1:\n{'foo': 'bar'}\n^\nInvalid string, it appears you used single quotes instead of double quotes" } ``` or ```json { "title": "Deserialization Failed", "detail": "Parse error on line 1:\nkaboom!\n^\nExpected one of: 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '['" } ``` ^ scenarios above are from https://github.com/symfony/symfony/blob/fe3094630f59739a506b34471b86cc2d1a271179/src/Symfony/Component/Serializer/Tests/Encoder/JsonDecodeTest.php#L70-L71 ? This is what my patch will do. All you need to do is install `@Seldaek`'s [seld/jsonlint](https://packagist.org/packages/seld/jsonlint). Now, initially I've just implemented support for this only in case jsonlint is installed, because I know how much Symfony tries to avoid adding dependencies. But if you are up for it, we could also make this required dependency, or embed its parser in Symfony. Or not do anything about it, saying PHP itself should improve this, but not sure how likely is that going to happen any time soon. Commits ------- c61a43d [Serializer] Add support for seld/jsonlint in order to enhance error messages
2 parents 489c887 + c61a43d commit 3b34568

File tree

5 files changed

+34
-7
lines changed

5 files changed

+34
-7
lines changed

composer.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@
150150
"predis/predis": "^1.1|^2.0",
151151
"psr/http-client": "^1.0",
152152
"psr/simple-cache": "^1.0|^2.0|^3.0",
153+
"seld/jsonlint": "^1.10",
153154
"symfony/mercure-bundle": "^0.3",
154155
"symfony/phpunit-bridge": "^5.4|^6.0|^7.0",
155156
"symfony/runtime": "self.version",

src/Symfony/Component/Serializer/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ CHANGELOG
77
* Deprecate Doctrine annotations support in favor of native attributes
88
* Deprecate passing an annotation reader to the constructor of `AnnotationLoader`
99
* Allow the `Groups` attribute/annotation on classes
10+
* JsonDecode: Add `json_decode_detailed_errors` option
1011

1112
6.3
1213
---

src/Symfony/Component/Serializer/Encoder/JsonDecode.php

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111

1212
namespace Symfony\Component\Serializer\Encoder;
1313

14+
use Seld\JsonLint\JsonParser;
1415
use Symfony\Component\Serializer\Exception\NotEncodableValueException;
16+
use Symfony\Component\Serializer\Exception\UnsupportedException;
1517

1618
/**
1719
* Decodes JSON data.
@@ -30,6 +32,11 @@ class JsonDecode implements DecoderInterface
3032
*/
3133
public const ASSOCIATIVE = 'json_decode_associative';
3234

35+
/**
36+
* True to enable seld/jsonlint as a source for more specific error messages when json_decode fails.
37+
*/
38+
public const DETAILED_ERROR_MESSAGES = 'json_decode_detailed_errors';
39+
3340
public const OPTIONS = 'json_decode_options';
3441

3542
/**
@@ -39,6 +46,7 @@ class JsonDecode implements DecoderInterface
3946

4047
private array $defaultContext = [
4148
self::ASSOCIATIVE => false,
49+
self::DETAILED_ERROR_MESSAGES => false,
4250
self::OPTIONS => 0,
4351
self::RECURSION_DEPTH => 512,
4452
];
@@ -69,6 +77,10 @@ public function __construct(array $defaultContext = [])
6977
* json_decode_options: integer
7078
* Specifies additional options as per documentation for json_decode
7179
*
80+
* json_decode_detailed_errors: bool
81+
* If true, enables seld/jsonlint as a source for more specific error messages when json_decode fails.
82+
* If false or not specified, this method will use default error messages from PHP's json_decode
83+
*
7284
* @throws NotEncodableValueException
7385
*
7486
* @see https://php.net/json_decode
@@ -89,11 +101,20 @@ public function decode(string $data, string $format, array $context = []): mixed
89101
return $decodedData;
90102
}
91103

92-
if (\JSON_ERROR_NONE !== json_last_error()) {
93-
throw new NotEncodableValueException(json_last_error_msg());
104+
if (\JSON_ERROR_NONE === json_last_error()) {
105+
return $decodedData;
106+
}
107+
$errorMessage = json_last_error_msg();
108+
109+
if (!($context[self::DETAILED_ERROR_MESSAGES] ?? $this->defaultContext[self::DETAILED_ERROR_MESSAGES])) {
110+
throw new NotEncodableValueException($errorMessage);
111+
}
112+
113+
if (!class_exists(JsonParser::class)) {
114+
throw new UnsupportedException(sprintf('Enabling "%s" serializer option requires seld/jsonlint. Try running "composer require seld/jsonlint".', self::DETAILED_ERROR_MESSAGES));
94115
}
95116

96-
return $decodedData;
117+
throw new NotEncodableValueException((new JsonParser())->lint($data)?->getMessage() ?: $errorMessage);
97118
}
98119

99120
public function supportsDecoding(string $format): bool

src/Symfony/Component/Serializer/Tests/Encoder/JsonDecodeTest.php

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,20 @@ public static function decodeProvider()
5858
/**
5959
* @dataProvider decodeProviderException
6060
*/
61-
public function testDecodeWithException($value)
61+
public function testDecodeWithException(string $value, string $expectedExceptionMessage, array $context)
6262
{
6363
$this->expectException(UnexpectedValueException::class);
64-
$this->decode->decode($value, JsonEncoder::FORMAT);
64+
$this->expectExceptionMessage($expectedExceptionMessage);
65+
$this->decode->decode($value, JsonEncoder::FORMAT, $context);
6566
}
6667

6768
public static function decodeProviderException()
6869
{
6970
return [
70-
["{'foo': 'bar'}"],
71-
['kaboom!'],
71+
["{'foo': 'bar'}", 'Syntax error', []],
72+
["{'foo': 'bar'}", 'single quotes instead of double quotes', ['json_decode_detailed_errors' => true]],
73+
['kaboom!', 'Syntax error', ['json_decode_detailed_errors' => false]],
74+
['kaboom!', "Expected one of: 'STRING', 'NUMBER', 'NULL',", ['json_decode_detailed_errors' => true]],
7275
];
7376
}
7477
}

src/Symfony/Component/Serializer/composer.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
},
2323
"require-dev": {
2424
"doctrine/annotations": "^1.12|^2",
25+
"seld/jsonlint": "^1.10",
2526
"phpdocumentor/reflection-docblock": "^3.2|^4.0|^5.0",
2627
"symfony/cache": "^5.4|^6.0|^7.0",
2728
"symfony/config": "^5.4|^6.0|^7.0",

0 commit comments

Comments
 (0)
0