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
liip · Feb 18, 2018 · 2c62b7f · 2c62b7f
1 parent 597f204
commit 2c62b7f
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 40 deletions.
diff --git 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
@@ -1,47 +1,86 @@
 <?php
 
-$fileHeaderComment = <<<COMMENT
+/*
+ * This file is part of the `liip/LiipImagineBundle` project.
+ *
+ * (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
+ *
+ * For the full copyright and license information, please view the LICENSE.md
+ * file that was distributed with this source code.
+ */
+
+use PhpCsFixer\Config;
+use PhpCsFixer\Finder;
+
+$header = <<<HEADER
 This file is part of the `liip/LiipImagineBundle` project.
 
 (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
 
 For the full copyright and license information, please view the LICENSE.md
 file that was distributed with this source code.
-COMMENT;
-
-$finder = PhpCsFixer\Finder::create()
-    ->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
@@ -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
@@ -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
@@ -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),
+        ]));
     }
 }