From 597f204e25745fa09183cb36c804353f912660d2 Mon Sep 17 00:00:00 2001 From: Sebastian Blum Date: Sat, 20 Jan 2018 15:27:27 +0100 Subject: [PATCH 1/2] add new php-cs-fixer rules - created new rule set as discussed by community - removed broken php-cs-fixer-styleci-bridge usage --- .php_cs.dist | 63 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/.php_cs.dist b/.php_cs.dist index 57cae650e..e9c61dff3 100644 --- a/.php_cs.dist +++ b/.php_cs.dist @@ -1,32 +1,47 @@ setRules(array_merge($config->getRules(), [ - 'header_comment' => ['header' => $header], - ])) - ->setUsingCache(false); - -return $config; +COMMENT; + +$finder = PhpCsFixer\Finder::create() + ->in(__DIR__) + ->ignoreDotFiles(true) + ->ignoreVCS(true) + ->exclude(array('build', 'vendor')) + ->files() + ->name('*.php') +; + +return PhpCsFixer\Config::create() + ->setFinder($finder) + ->setRiskyAllowed(true) + ->setRules([ + '@Symfony' => true, + '@Symfony:risky' => true, + 'array_syntax' => ['syntax' => 'short'], + 'combine_consecutive_unsets' => true, + 'header_comment' => ['header' => $fileHeaderComment, 'separate' => 'both'], + 'heredoc_to_nowdoc' => true, + 'linebreak_after_opening_tag' => true, + 'mb_str_functions' => true, + 'no_php4_constructor' => true, + 'no_unreachable_default_argument_value' => true, + 'no_useless_else' => true, + 'no_useless_return' => true, + 'ordered_imports' => true, + 'phpdoc_order' => true, + 'phpdoc_summary' => false, + 'psr4' => true, + 'semicolon_after_instruction' => true, + 'strict_comparison' => true, + 'strict_param' => true, + ]) + ->setFinder($finder) + ->setUsingCache(true) +; From 2c62b7fcd2ca757771c8d9fc718065f85dd329be Mon Sep 17 00:00:00 2001 From: Rob Frawley 2nd Date: Thu, 15 Feb 2018 19:42:36 -0500 Subject: [PATCH 2/2] add more php-cs-fixer rules and cleanup accordingly - add additional rules and sort all alphabetically - fix code broken by new rules - cleanup .php_cs.dist file and align with cs standards - add .php_cs.cache to .gitignore --- .gitignore | 9 ++- .php_cs.dist | 77 ++++++++++++++----- Tests/Async/ResolveCacheProcessorTest.php | 8 +- .../Resolver/AwsS3ResolverFactoryTest.php | 16 ++-- .../LiipImagineExtensionTest.php | 19 +++-- 5 files changed, 89 insertions(+), 40 deletions(-) diff --git a/.gitignore b/.gitignore index a5674fceb..3e31fdbff 100644 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,10 @@ +/.idea/ +/.php_cs.cache /bin/ -/phpunit.xml /composer.lock /composer.phar -/vendor/ -/Tests/Functional/app/web/media/cache +/phpunit.xml /Tests/Functional/app/public/media/cache -/.idea/ +/Tests/Functional/app/web/media/cache /var/ +/vendor/ diff --git a/.php_cs.dist b/.php_cs.dist index e9c61dff3..9b974a1ce 100644 --- a/.php_cs.dist +++ b/.php_cs.dist @@ -1,47 +1,86 @@ in(__DIR__) - ->ignoreDotFiles(true) - ->ignoreVCS(true) - ->exclude(array('build', 'vendor')) - ->files() - ->name('*.php') -; +HEADER; -return PhpCsFixer\Config::create() - ->setFinder($finder) +return Config::create() + ->setUsingCache(true) ->setRiskyAllowed(true) + ->setFinder((new Finder())->in(__DIR__)) ->setRules([ '@Symfony' => true, '@Symfony:risky' => true, - 'array_syntax' => ['syntax' => 'short'], + '@PHPUnit57Migration:risky' => true, + 'array_syntax' => [ + 'syntax' => 'short' + ], + 'combine_consecutive_issets' => true, 'combine_consecutive_unsets' => true, - 'header_comment' => ['header' => $fileHeaderComment, 'separate' => 'both'], + 'escape_implicit_backslashes' => true, + 'explicit_indirect_variable' => true, + 'final_internal_class' => true, + 'header_comment' => [ + 'header' => $header, + 'separate' => 'both' + ], 'heredoc_to_nowdoc' => true, 'linebreak_after_opening_tag' => true, + 'list_syntax' => [ + 'syntax' => 'short', + ], 'mb_str_functions' => true, + 'multiline_whitespace_before_semicolons' => [ + 'strategy' => 'no_multi_line', + ], 'no_php4_constructor' => true, + 'no_short_echo_tag' => true, 'no_unreachable_default_argument_value' => true, 'no_useless_else' => true, 'no_useless_return' => true, + 'ordered_class_elements' => [ + 'order' => [ + 'use_trait', + 'constant_public', + 'constant_protected', + 'constant_private', + 'property_public', + 'property_protected', + 'property_private', + 'construct', + 'destruct', + 'magic', + 'phpunit', + 'method_public', + 'method_protected', + 'method_private' + ], + ], 'ordered_imports' => true, + 'php_unit_strict' => true, + 'php_unit_no_expectation_annotation' => true, + 'php_unit_test_class_requires_covers' => true, 'phpdoc_order' => true, 'phpdoc_summary' => false, 'psr4' => true, 'semicolon_after_instruction' => true, 'strict_comparison' => true, 'strict_param' => true, - ]) - ->setFinder($finder) - ->setUsingCache(true) -; + ]); diff --git a/Tests/Async/ResolveCacheProcessorTest.php b/Tests/Async/ResolveCacheProcessorTest.php index 4cd5c1469..3127c474e 100644 --- a/Tests/Async/ResolveCacheProcessorTest.php +++ b/Tests/Async/ResolveCacheProcessorTest.php @@ -172,7 +172,7 @@ public function testShouldCreateFilteredImage() $result = $processor->process($message, new NullContext()); - $this->assertEquals(Result::ACK, $result); + $this->assertEquals(Result::ACK, (string) $result); } public function testShouldCreateOneImagePerFilter() @@ -211,7 +211,7 @@ public function testShouldCreateOneImagePerFilter() $result = $processor->process($message, new NullContext()); - $this->assertEquals(Result::ACK, $result); + $this->assertEquals(Result::ACK, (string) $result); } public function testShouldOnlyCreateImageForRequestedFilter() @@ -241,7 +241,7 @@ public function testShouldOnlyCreateImageForRequestedFilter() $result = $processor->process($message, new NullContext()); - $this->assertEquals(Result::ACK, $result); + $this->assertEquals(Result::ACK, (string) $result); } public function testShouldCreateOneImagePerRequestedFilter() @@ -275,7 +275,7 @@ public function testShouldCreateOneImagePerRequestedFilter() $result = $processor->process($message, new NullContext()); - $this->assertEquals(Result::ACK, $result); + $this->assertEquals(Result::ACK, (string) $result); } public function testShouldBurstCacheWhenResolvingForced() diff --git a/Tests/DependencyInjection/Factory/Resolver/AwsS3ResolverFactoryTest.php b/Tests/DependencyInjection/Factory/Resolver/AwsS3ResolverFactoryTest.php index dbea0a7fd..ab5af4487 100644 --- a/Tests/DependencyInjection/Factory/Resolver/AwsS3ResolverFactoryTest.php +++ b/Tests/DependencyInjection/Factory/Resolver/AwsS3ResolverFactoryTest.php @@ -69,7 +69,7 @@ public function testCreateResolverDefinitionOnCreate() $this->assertEquals('liip_imagine.cache.resolver.prototype.aws_s3', $resolverDefinition->getParent()); $this->assertInstanceOf(Reference::class, $resolverDefinition->getArgument(0)); - $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.client', $resolverDefinition->getArgument(0)); + $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.client', (string) $resolverDefinition->getArgument(0)); $this->assertEquals('theBucket', $resolverDefinition->getArgument(1)); $this->assertEquals('theAcl', $resolverDefinition->getArgument(2)); @@ -149,7 +149,7 @@ public function testWrapResolverWithProxyOnCreateWithoutCache() $this->assertEquals('liip_imagine.cache.resolver.prototype.proxy', $resolverDefinition->getParent()); $this->assertInstanceOf(Reference::class, $resolverDefinition->getArgument(0)); - $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.proxied', $resolverDefinition->getArgument(0)); + $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.proxied', (string) $resolverDefinition->getArgument(0)); $this->assertEquals(array('foo'), $resolverDefinition->getArgument(1)); } @@ -183,10 +183,10 @@ public function testWrapResolverWithCacheOnCreateWithoutProxy() $this->assertEquals('liip_imagine.cache.resolver.prototype.cache', $resolverDefinition->getParent()); $this->assertInstanceOf(Reference::class, $resolverDefinition->getArgument(0)); - $this->assertEquals('the_cache_service_id', $resolverDefinition->getArgument(0)); + $this->assertEquals('the_cache_service_id', (string) $resolverDefinition->getArgument(0)); $this->assertInstanceOf(Reference::class, $resolverDefinition->getArgument(1)); - $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.cached', $resolverDefinition->getArgument(1)); + $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.cached', (string) $resolverDefinition->getArgument(1)); } public function testWrapResolverWithProxyAndCacheOnCreate() @@ -216,7 +216,7 @@ public function testWrapResolverWithProxyAndCacheOnCreate() $this->assertEquals('liip_imagine.cache.resolver.prototype.proxy', $cachedResolverDefinition->getParent()); $this->assertInstanceOf(Reference::class, $cachedResolverDefinition->getArgument(0)); - $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.proxied', $cachedResolverDefinition->getArgument(0)); + $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.proxied', (string) $cachedResolverDefinition->getArgument(0)); $this->assertEquals(array('foo'), $cachedResolverDefinition->getArgument(1)); @@ -226,10 +226,10 @@ public function testWrapResolverWithProxyAndCacheOnCreate() $this->assertEquals('liip_imagine.cache.resolver.prototype.cache', $resolverDefinition->getParent()); $this->assertInstanceOf(Reference::class, $resolverDefinition->getArgument(0)); - $this->assertEquals('the_cache_service_id', $resolverDefinition->getArgument(0)); + $this->assertEquals('the_cache_service_id', (string) $resolverDefinition->getArgument(0)); $this->assertInstanceOf(Reference::class, $resolverDefinition->getArgument(1)); - $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.cached', $resolverDefinition->getArgument(1)); + $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.cached', (string) $resolverDefinition->getArgument(1)); } public function testWrapResolverWithProxyMatchReplaceStrategyOnCreate() @@ -259,7 +259,7 @@ public function testWrapResolverWithProxyMatchReplaceStrategyOnCreate() $this->assertEquals('liip_imagine.cache.resolver.prototype.proxy', $cachedResolverDefinition->getParent()); $this->assertInstanceOf(Reference::class, $cachedResolverDefinition->getArgument(0)); - $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.proxied', $cachedResolverDefinition->getArgument(0)); + $this->assertEquals('liip_imagine.cache.resolver.the_resolver_name.proxied', (string) $cachedResolverDefinition->getArgument(0)); $this->assertEquals(array('foo' => 'bar'), $cachedResolverDefinition->getArgument(1)); } diff --git a/Tests/DependencyInjection/LiipImagineExtensionTest.php b/Tests/DependencyInjection/LiipImagineExtensionTest.php index b6ba665af..aec7901e3 100644 --- a/Tests/DependencyInjection/LiipImagineExtensionTest.php +++ b/Tests/DependencyInjection/LiipImagineExtensionTest.php @@ -197,11 +197,20 @@ private function assertHasDefinition($id) */ private function assertDICConstructorArguments(Definition $definition, array $arguments) { - $this->assertEquals($arguments, $definition->getArguments(), "Expected and actual DIC Service constructor arguments of definition '".$definition->getClass()."' don't match."); - } + $castArrayElementsToString = function (array $a): array { + return array_map(function ($v) { return (string) $v; }, $a); + }; - protected function tearDown() - { - unset($this->containerBuilder); + $implodeArrayElements = function (array $a): string { + return sprintf('[%s]:%d', implode(',', $a), count($a)); + }; + + $expectedArguments = $castArrayElementsToString($arguments); + $providedArguments = $castArrayElementsToString($definition->getArguments()); + + $this->assertSame($expectedArguments, $providedArguments, vsprintf('Definition arguments (%s) do not match expected arguments (%s).', [ + $implodeArrayElements($providedArguments), + $implodeArrayElements($expectedArguments), + ])); } }