Skip to content

Commit

Permalink
feature #21774 [Yaml] deprecate implicit string casting of mapping ke…
Browse files Browse the repository at this point in the history
…ys (xabbuh)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[Yaml] deprecate implicit string casting of mapping keys

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #21650 (comment)
| License       | MIT
| Doc PR        |

Commits
-------

3c5d915 deprecate implicit string casting of mapping keys
  • Loading branch information
fabpot committed Mar 6, 2017
2 parents 638bdc8 + 3c5d915 commit 323529c
Show file tree
Hide file tree
Showing 28 changed files with 367 additions and 109 deletions.
31 changes: 31 additions & 0 deletions UPGRADE-3.3.md
Expand Up @@ -246,6 +246,37 @@ Workflow
Yaml
----

* Deprecated support for implicitly parsing non-string mapping keys as strings. Mapping keys that are no strings will
lead to a `ParseException` in Symfony 4.0. Use the `PARSE_KEYS_AS_STRINGS` flag to opt-in for keys to be parsed as
strings.

Before:

```php
$yaml = <<<YAML
null: null key
true: boolean true
1: integer key
2.0: float key
YAML;

Yaml::parse($yaml);
```

After:

```php

$yaml = <<<YAML
null: null key
true: boolean true
1: integer key
2.0: float key
YAML;

Yaml::parse($yaml, Yaml::PARSE_KEYS_AS_STRINGS);
```

* Omitting the key of a mapping is deprecated and will throw a `ParseException` in Symfony 4.0.

* The constructor arguments `$offset`, `$totalNumberOfLines` and
Expand Down
30 changes: 30 additions & 0 deletions UPGRADE-4.0.md
Expand Up @@ -463,6 +463,36 @@ Workflow
Yaml
----

* Removed support for implicitly parsing non-string mapping keys as strings. Mapping keys that are no strings will
result in a `ParseException`. Use the `PARSE_KEYS_AS_STRINGS` flag to opt-in for keys to be parsed as strings.

Before:

```php
$yaml = <<<YAML
null: null key
true: boolean true
1: integer key
2.0: float key
YAML;

Yaml::parse($yaml);
```

After:

```php

$yaml = <<<YAML
null: null key
true: boolean true
1: integer key
2.0: float key
YAML;

Yaml::parse($yaml, Yaml::PARSE_KEYS_AS_STRINGS);
```

* Omitting the key of a mapping is not supported anymore and throws a `ParseException`.

* Mappings with a colon (`:`) that is not followed by a whitespace are not
Expand Down
Expand Up @@ -602,7 +602,7 @@ protected function loadFile($file)
}

try {
$configuration = $this->yamlParser->parse(file_get_contents($file), Yaml::PARSE_CONSTANT | Yaml::PARSE_CUSTOM_TAGS);
$configuration = $this->yamlParser->parse(file_get_contents($file), Yaml::PARSE_CONSTANT | Yaml::PARSE_CUSTOM_TAGS | Yaml::PARSE_KEYS_AS_STRINGS);
} catch (ParseException $e) {
throw new InvalidArgumentException(sprintf('The file "%s" does not contain valid YAML.', $file), 0, $e);
}
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/Routing/Loader/YamlFileLoader.php
Expand Up @@ -17,6 +17,7 @@
use Symfony\Component\Yaml\Exception\ParseException;
use Symfony\Component\Yaml\Parser as YamlParser;
use Symfony\Component\Config\Loader\FileLoader;
use Symfony\Component\Yaml\Yaml;

/**
* YamlFileLoader loads Yaml routing files.
Expand Down Expand Up @@ -58,7 +59,7 @@ public function load($file, $type = null)
}

try {
$parsedConfig = $this->yamlParser->parse(file_get_contents($path));
$parsedConfig = $this->yamlParser->parse(file_get_contents($path), Yaml::PARSE_KEYS_AS_STRINGS);
} catch (ParseException $e) {
throw new \InvalidArgumentException(sprintf('The file "%s" does not contain valid YAML.', $path), 0, $e);
}
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Component/Routing/composer.json
Expand Up @@ -21,15 +21,16 @@
"require-dev": {
"symfony/config": "~2.8|~3.0",
"symfony/http-foundation": "~2.8|~3.0",
"symfony/yaml": "~2.8|~3.0",
"symfony/yaml": "~3.3",
"symfony/expression-language": "~2.8|~3.0",
"symfony/dependency-injection": "~2.8|~3.0",
"doctrine/annotations": "~1.0",
"doctrine/common": "~2.2",
"psr/log": "~1.0"
},
"conflict": {
"symfony/config": "<2.8"
"symfony/config": "<2.8",
"symfony/yaml": "<3.3"
},
"suggest": {
"symfony/http-foundation": "For using a Symfony Request object",
Expand Down
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Serializer\Mapping\AttributeMetadata;
use Symfony\Component\Serializer\Mapping\ClassMetadataInterface;
use Symfony\Component\Yaml\Parser;
use Symfony\Component\Yaml\Yaml;

/**
* YAML File Loader.
Expand Down Expand Up @@ -113,7 +114,7 @@ private function getClassesFromYaml()
$this->yamlParser = new Parser();
}

$classes = $this->yamlParser->parse(file_get_contents($this->file));
$classes = $this->yamlParser->parse(file_get_contents($this->file), Yaml::PARSE_KEYS_AS_STRINGS);

if (empty($classes)) {
return array();
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Serializer/composer.json
Expand Up @@ -19,7 +19,7 @@
"php": ">=5.5.9"
},
"require-dev": {
"symfony/yaml": "~3.1",
"symfony/yaml": "~3.3",
"symfony/config": "~2.8|~3.0",
"symfony/property-access": "~2.8|~3.0",
"symfony/http-foundation": "~2.8|~3.0",
Expand All @@ -34,7 +34,7 @@
"symfony/dependency-injection": "<3.2",
"symfony/property-access": ">=3.0,<3.0.4|>=2.8,<2.8.4",
"symfony/property-info": "<3.1",
"symfony/yaml": "<3.1"
"symfony/yaml": "<3.3"
},
"suggest": {
"psr/cache-implementation": "For using the metadata cache.",
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/Translation/Loader/YamlFileLoader.php
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Translation\Exception\LogicException;
use Symfony\Component\Yaml\Parser as YamlParser;
use Symfony\Component\Yaml\Exception\ParseException;
use Symfony\Component\Yaml\Yaml;

/**
* YamlFileLoader loads translations from Yaml files.
Expand All @@ -39,7 +40,7 @@ protected function loadResource($resource)
}

try {
$messages = $this->yamlParser->parse(file_get_contents($resource));
$messages = $this->yamlParser->parse(file_get_contents($resource), Yaml::PARSE_KEYS_AS_STRINGS);
} catch (ParseException $e) {
throw new InvalidResourceException(sprintf('Error parsing YAML, invalid file "%s"', $resource), 0, $e);
}
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Component/Translation/composer.json
Expand Up @@ -22,11 +22,12 @@
"require-dev": {
"symfony/config": "~2.8|~3.0",
"symfony/intl": "^2.8.18|^3.2.5",
"symfony/yaml": "~2.8|~3.0",
"symfony/yaml": "~3.3",
"psr/log": "~1.0"
},
"conflict": {
"symfony/config": "<2.8"
"symfony/config": "<2.8",
"symfony/yaml": "<3.3"
},
"suggest": {
"symfony/config": "",
Expand Down
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\Validator\Mapping\ClassMetadata;
use Symfony\Component\Yaml\Exception\ParseException;
use Symfony\Component\Yaml\Parser as YamlParser;
use Symfony\Component\Yaml\Yaml;

/**
* Loads validation metadata from a YAML file.
Expand Down Expand Up @@ -115,7 +116,7 @@ protected function parseNodes(array $nodes)
private function parseFile($path)
{
try {
$classes = $this->yamlParser->parse(file_get_contents($path));
$classes = $this->yamlParser->parse(file_get_contents($path), Yaml::PARSE_KEYS_AS_STRINGS);
} catch (ParseException $e) {
throw new \InvalidArgumentException(sprintf('The file "%s" does not contain valid YAML.', $path), 0, $e);
}
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Component/Validator/composer.json
Expand Up @@ -23,7 +23,7 @@
"require-dev": {
"symfony/http-foundation": "~2.8|~3.0",
"symfony/intl": "^2.8.18|^3.2.5",
"symfony/yaml": "~2.8|~3.0",
"symfony/yaml": "~3.3",
"symfony/config": "~2.8|~3.0",
"symfony/expression-language": "~2.8|~3.0",
"symfony/cache": "~3.1",
Expand All @@ -32,7 +32,8 @@
"egulias/email-validator": "^1.2.8|~2.0"
},
"conflict": {
"phpunit/phpunit": "<4.8.35|<5.4.3,>=5.0"
"phpunit/phpunit": "<4.8.35|<5.4.3,>=5.0",
"symfony/yaml": "<3.3"
},
"suggest": {
"psr/cache-implementation": "For using the metadata cache.",
Expand Down
31 changes: 31 additions & 0 deletions src/Symfony/Component/Yaml/CHANGELOG.md
Expand Up @@ -4,6 +4,37 @@ CHANGELOG
3.3.0
-----

* Deprecated support for implicitly parsing non-string mapping keys as strings. Mapping keys that are no strings will
lead to a `ParseException` in Symfony 4.0. Use the `PARSE_KEYS_AS_STRINGS` flag to opt-in for keys to be parsed as
strings.

Before:

```php
$yaml = <<<YAML
null: null key
true: boolean true
1: integer key
2.0: float key
YAML;

Yaml::parse($yaml);
```

After:

```php

$yaml = <<<YAML
null: null key
true: boolean true
1: integer key
2.0: float key
YAML;

Yaml::parse($yaml, Yaml::PARSE_KEYS_AS_STRINGS);
```

* Omitted mapping values will be parsed as `null`.

* Omitting the key of a mapping is deprecated and will throw a `ParseException` in Symfony 4.0.
Expand Down
8 changes: 8 additions & 0 deletions src/Symfony/Component/Yaml/Inline.php
Expand Up @@ -485,6 +485,14 @@ private static function parseMapping($mapping, $flags, &$i = 0, $references = ar
@trigger_error('Omitting the key of a mapping is deprecated and will throw a ParseException in 4.0.', E_USER_DEPRECATED);
}

if (!(Yaml::PARSE_KEYS_AS_STRINGS & $flags)) {
$evaluatedKey = self::evaluateScalar($key, $flags, $references);

if ('' !== $key && $evaluatedKey !== $key && !is_string($evaluatedKey)) {
@trigger_error('Implicit casting of incompatible mapping keys to strings is deprecated since version 3.3 and will throw \Symfony\Component\Yaml\Exception\ParseException in 4.0. Pass the PARSE_KEYS_AS_STRING flag to explicitly enable the type casts.', E_USER_DEPRECATED);
}
}

if (':' !== $key && (!isset($mapping[$i + 1]) || !in_array($mapping[$i + 1], array(' ', ',', '[', ']', '{', '}'), true))) {
@trigger_error('Using a colon that is not followed by an indication character (i.e. " ", ",", "[", "]", "{", "}" is deprecated since version 3.2 and will throw a ParseException in 4.0.', E_USER_DEPRECATED);
}
Expand Down
7 changes: 6 additions & 1 deletion src/Symfony/Component/Yaml/Parser.php
Expand Up @@ -180,14 +180,19 @@ public function parse($value, $flags = 0)
Inline::parse(null, $flags, $this->refs);
try {
Inline::$parsedLineNumber = $this->getRealCurrentLineNb();
$key = Inline::parseScalar($values['key']);
$i = 0;
$key = Inline::parseScalar($values['key'], 0, null, $i, !(Yaml::PARSE_KEYS_AS_STRINGS & $flags));
} catch (ParseException $e) {
$e->setParsedLine($this->getRealCurrentLineNb() + 1);
$e->setSnippet($this->currentLine);

throw $e;
}

if (!(Yaml::PARSE_KEYS_AS_STRINGS & $flags) && !is_string($key)) {
@trigger_error('Implicit casting of incompatible mapping keys to strings is deprecated since version 3.3 and will throw \Symfony\Component\Yaml\Exception\ParseException in 4.0. Pass the PARSE_KEYS_AS_STRING flag to explicitly enable the type casts.', E_USER_DEPRECATED);
}

// Convert float keys to strings, to avoid being converted to integers by PHP
if (is_float($key)) {
$key = (string) $key;
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Yaml/Tests/DumperTest.php
Expand Up @@ -125,7 +125,7 @@ public function testSpecifications()
// TODO
} else {
eval('$expected = '.trim($test['php']).';');
$this->assertSame($expected, $this->parser->parse($this->dumper->dump($expected, 10)), $test['test']);
$this->assertSame($expected, $this->parser->parse($this->dumper->dump($expected, 10), Yaml::PARSE_KEYS_AS_STRINGS), $test['test']);
}
}
}
Expand Down
Expand Up @@ -556,21 +556,6 @@ php: |
'fixed' => 1230.15,
)
---
test: Miscellaneous
spec: 2.21
yaml: |
null: ~
true: true
false: false
string: '12345'
php: |
array(
'' => null,
1 => true,
0 => false,
'string' => '12345'
)
---
test: Timestamps
todo: true
spec: 2.22
Expand Down Expand Up @@ -1533,18 +1518,6 @@ ruby: |
}
---
test: Boolean
yaml: |
false: used as key
logical: true
answer: false
php: |
array(
false => 'used as key',
'logical' => true,
'answer' => false
)
---
test: Integer
yaml: |
canonical: 12345
Expand Down
14 changes: 0 additions & 14 deletions src/Symfony/Component/Yaml/Tests/Fixtures/YtsTypeTransfers.yml
Expand Up @@ -210,20 +210,6 @@ php: |
'negative one-thousand' => -1000.0
)
---
test: Integers as Map Keys
brief: >
An integer can be used a dictionary key.
yaml: |
1: one
2: two
3: three
php: |
array(
1 => 'one',
2 => 'two',
3 => 'three'
)
---
test: Floats
dump_skip: true
brief: >
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/Yaml/Tests/Fixtures/booleanMappingKeys.yml
@@ -0,0 +1,11 @@
--- %YAML:1.0
test: Miscellaneous
spec: 2.21
yaml: |
true: true
false: false
php: |
array(
'true' => true,
'false' => false,
)

0 comments on commit 323529c

Please sign in to comment.