feature #20232 [DependencyInjection] fixed ini file values conversion (fabpot)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[DependencyInjection] fixed ini file values conversion

| Q | A |
| --- | --- |
| Branch? | master |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no-ish |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | n/a |
| License | MIT |
| Doc PR | n/a |

When using the ini format to load parameters in the Container, the parameter values were converted by PHP directly (`'true'` => `1` for instance). But when using the YAML or XML format, the conversions are much broader and more precise (`'true'` => `true` for instance). This PR fixed fixes this discrepancy by using the same rules as XML (we could use `INI_SCANNER_TYPED` for recent versions of PHP but the rules are not exactly the same, so I prefer consistency here).

One might argue that this is a new feature and that this should be merged into master, which I can accept as well. In master, the `XmlUtils::phpize()` method should be deprecated and replaced by a more generic phpize class.

ping @symfony/deciders

Commits
-------

4ccfce6 [DependencyInjection] fixed ini file values conversion
This commit is contained in:
Fabien Potencier 2016-11-03 16:44:20 -07:00
commit 9e2ad932e9
6 changed files with 139 additions and 10 deletions

View File

@ -12,6 +12,7 @@
namespace Symfony\Component\DependencyInjection\Loader;
use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Config\Util\XmlUtils;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
/**
@ -30,14 +31,18 @@ class IniFileLoader extends FileLoader
$this->container->addResource(new FileResource($path));
// first pass to catch parsing errors
$result = parse_ini_file($path, true);
if (false === $result || array() === $result) {
throw new InvalidArgumentException(sprintf('The "%s" file is not valid.', $resource));
}
// real raw parsing
$result = parse_ini_file($path, true, INI_SCANNER_RAW);
if (isset($result['parameters']) && is_array($result['parameters'])) {
foreach ($result['parameters'] as $key => $value) {
$this->container->setParameter($key, $value);
$this->container->setParameter($key, $this->phpize($value));
}
}
}
@ -49,4 +54,33 @@ class IniFileLoader extends FileLoader
{
return is_string($resource) && 'ini' === pathinfo($resource, PATHINFO_EXTENSION);
}
/**
* Note that the following features are not supported:
* * strings with escaped quotes are not supported "foo\"bar";
* * string concatenation ("foo" "bar").
*/
private function phpize($value)
{
// trim on the right as comments removal keep whitespaces
$value = rtrim($value);
$lowercaseValue = strtolower($value);
switch (true) {
case defined($value):
return constant($value);
case 'yes' === $lowercaseValue || 'on' === $lowercaseValue:
return true;
case 'no' === $lowercaseValue || 'off' === $lowercaseValue || 'none' === $lowercaseValue:
return false;
case isset($value[1]) && (
("'" === $value[0] && "'" === $value[strlen($value) - 1]) ||
('"' === $value[0] && '"' === $value[strlen($value) - 1])
):
// quoted string
return substr($value, 1, -1);
default:
return XmlUtils::phpize($value);
}
}
}

View File

@ -0,0 +1,2 @@
foo = '
bar = bar

View File

@ -0,0 +1,27 @@
[parameters]
true = true
true_comment = true ; comment
false = false
null = null
on = on
off = off
yes = yes
no = no
none = none
constant = PHP_VERSION
12 = 12
12_string = '12'
12_comment = 12 ; comment
12_string_comment = '12' ; comment
12_string_comment_again = "12" ; comment
-12 = -12
0 = 0
1 = 1
0b0110 = 0b0110
11112222333344445555 = 1111,2222,3333,4444,5555
0777 = 0777
255 = 0xFF
100.0 = 1e2
-120.0 = -1.2E2
-10100.1 = -10100.1
-10,100.1 = -10,100.1

View File

@ -17,20 +17,13 @@ use Symfony\Component\Config\FileLocator;
class IniFileLoaderTest extends \PHPUnit_Framework_TestCase
{
protected static $fixturesPath;
protected $container;
protected $loader;
public static function setUpBeforeClass()
{
self::$fixturesPath = realpath(__DIR__.'/../Fixtures/');
}
protected function setUp()
{
$this->container = new ContainerBuilder();
$this->loader = new IniFileLoader($this->container, new FileLocator(self::$fixturesPath.'/ini'));
$this->loader = new IniFileLoader($this->container, new FileLocator(realpath(__DIR__.'/../Fixtures/').'/ini'));
}
public function testIniFileCanBeLoaded()
@ -39,6 +32,68 @@ class IniFileLoaderTest extends \PHPUnit_Framework_TestCase
$this->assertEquals(array('foo' => 'bar', 'bar' => '%foo%'), $this->container->getParameterBag()->all(), '->load() takes a single file name as its first argument');
}
/**
* @dataProvider getTypeConversions
*/
public function testTypeConversions($key, $value, $supported)
{
$this->loader->load('types.ini');
$parameters = $this->container->getParameterBag()->all();
$this->assertSame($value, $parameters[$key], '->load() converts values to PHP types');
}
/**
* @dataProvider getTypeConversions
* @requires PHP 5.6.1
* This test illustrates where our conversions differs from INI_SCANNER_TYPED introduced in PHP 5.6.1
*/
public function testTypeConversionsWithNativePhp($key, $value, $supported)
{
if (defined('HHVM_VERSION_ID')) {
return $this->markTestSkipped();
}
if (!$supported) {
return;
}
$this->loader->load('types.ini');
$expected = parse_ini_file(__DIR__.'/../Fixtures/ini/types.ini', true, INI_SCANNER_TYPED);
$this->assertSame($value, $expected['parameters'][$key], '->load() converts values to PHP types');
}
public function getTypeConversions()
{
return array(
array('true_comment', true, true),
array('true', true, true),
array('false', false, true),
array('on', true, true),
array('off', false, true),
array('yes', true, true),
array('no', false, true),
array('none', false, true),
array('null', null, true),
array('constant', PHP_VERSION, true),
array('12', 12, true),
array('12_string', '12', true),
array('12_comment', 12, true),
array('12_string_comment', '12', true),
array('12_string_comment_again', '12', true),
array('-12', -12, true),
array('1', 1, true),
array('0', 0, true),
array('0b0110', bindec('0b0110'), false), // not supported by INI_SCANNER_TYPED
array('11112222333344445555', '1111,2222,3333,4444,5555', true),
array('0777', 0777, false), // not supported by INI_SCANNER_TYPED
array('255', 0xFF, false), // not supported by INI_SCANNER_TYPED
array('100.0', 1e2, false), // not supported by INI_SCANNER_TYPED
array('-120.0', -1.2E2, false), // not supported by INI_SCANNER_TYPED
array('-10100.1', -10100.1, false), // not supported by INI_SCANNER_TYPED
array('-10,100.1', '-10,100.1', true),
);
}
/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage The file "foo.ini" does not exist (in:
@ -49,7 +104,7 @@ class IniFileLoaderTest extends \PHPUnit_Framework_TestCase
}
/**
* @expectedException \InvalidArgumentException
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
* @expectedExceptionMessage The "nonvalid.ini" file is not valid.
*/
public function testExceptionIsRaisedWhenIniFileCannotBeParsed()
@ -57,6 +112,15 @@ class IniFileLoaderTest extends \PHPUnit_Framework_TestCase
@$this->loader->load('nonvalid.ini');
}
/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
* @expectedExceptionMessage The "almostvalid.ini" file is not valid.
*/
public function testExceptionIsRaisedWhenIniFileIsAlmostValid()
{
@$this->loader->load('almostvalid.ini');
}
public function testSupports()
{
$loader = new IniFileLoader(new ContainerBuilder(), new FileLocator());

View File

@ -164,6 +164,7 @@ class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase
);
$this->assertEquals(array_keys($expected), array_keys($actual), '->load() imports and merges imported files');
$this->assertTrue($actual['imported_from_ini']);
// Bad import throws no exception due to ignore_errors value.
$loader->load('services4_bad_import.xml');

View File

@ -115,6 +115,7 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase
$actual = $container->getParameterBag()->all();
$expected = array('foo' => 'bar', 'values' => array(true, false, PHP_INT_MAX), 'bar' => '%foo%', 'escape' => '@escapeme', 'foo_bar' => new Reference('foo_bar'), 'mixedcase' => array('MixedCaseKey' => 'value'), 'imported_from_ini' => true, 'imported_from_xml' => true);
$this->assertEquals(array_keys($expected), array_keys($actual), '->load() imports and merges imported files');
$this->assertTrue($actual['imported_from_ini']);
// Bad import throws no exception due to ignore_errors value.
$loader->load('services4_bad_import.yml');