minor #184 Quality fixes (PHP Inspections) (dunglas)

This PR was merged into the 1.0-dev branch. Discussion ---------- Quality fixes (PHP Inspections) Fix some quality issues and add some minor performance improvements (analysis done with [PHP Inspections](https://github.com/kalessil/phpinspectionsea)). Commits ------- 65776ee Quality fixes (PHP Inspections)
symfony · May 14, 2018 · 319a73d · 319a73d
2 parents 967d791 + 65776ee
commit 319a73d
Show file tree

Hide file tree

Showing 16 changed files with 60 additions and 70 deletions.
diff --git a/src/Command/MakerCommand.php b/src/Command/MakerCommand.php
@@ -56,7 +56,7 @@ protected function configure()
     protected function initialize(InputInterface $input, OutputInterface $output)
     {
         $this->io = new ConsoleStyle($input, $output);
-        $this->fileManager->setIo($this->io);
+        $this->fileManager->setIO($this->io);
 
         if ($this->checkDependencies) {
             $dependencies = new DependencyBuilder();
@@ -75,7 +75,7 @@ protected function interact(InputInterface $input, OutputInterface $output)
                 continue;
             }
 
-            if (in_array($argument->getName(), $this->inputConfig->getNonInteractiveArguments(), true)) {
+            if (\in_array($argument->getName(), $this->inputConfig->getNonInteractiveArguments(), true)) {
                 continue;
             }
 

diff --git a/src/DependencyBuilder.php b/src/DependencyBuilder.php
@@ -84,7 +84,7 @@ public function getMissingPackagesMessage(string $commandName): string
             return '';
         }
 
-        $packagesCount = count($packages) + count($packagesDev);
+        $packagesCount = \count($packages) + \count($packagesDev);
 
         $message = sprintf(
             "Missing package%s: to use the %s command, run:\n",
@@ -103,7 +103,7 @@ public function getMissingPackagesMessage(string $commandName): string
         return $message;
     }
 
-    private function getRequiredDependencyNames(array $dependencies)
+    private function getRequiredDependencyNames(array $dependencies): array
     {
         $packages = [];
         foreach ($dependencies as $package) {
@@ -116,7 +116,7 @@ private function getRequiredDependencyNames(array $dependencies)
         return array_unique($packages);
     }
 
-    private function calculateMissingDependencies(array $dependencies)
+    private function calculateMissingDependencies(array $dependencies): array
     {
         $missingPackages = [];
         $missingOptionalPackages = [];

diff --git a/src/Doctrine/DoctrineHelper.php b/src/Doctrine/DoctrineHelper.php
@@ -125,7 +125,7 @@ public function getMetadata(string $classOrNamespace = null, bool $disconnected
                 if (null === $classOrNamespace) {
                     $metadata[$m->getName()] = $m;
                 } else {
-                    if ($m->getName() == $classOrNamespace) {
+                    if ($m->getName() === $classOrNamespace) {
                         return $m;
                     }
 

diff --git a/src/Doctrine/EntityRegenerator.php b/src/Doctrine/EntityRegenerator.php
@@ -28,15 +28,13 @@ final class EntityRegenerator
     private $doctrineHelper;
     private $fileManager;
     private $generator;
-    private $projectDirectory;
     private $overwrite;
 
     public function __construct(DoctrineHelper $doctrineHelper, FileManager $fileManager, Generator $generator, string $projectDirectory, bool $overwrite)
     {
         $this->doctrineHelper = $doctrineHelper;
         $this->fileManager = $fileManager;
         $this->generator = $generator;
-        $this->projectDirectory = $projectDirectory;
         $this->overwrite = $overwrite;
     }
 
@@ -99,7 +97,7 @@ public function regenerateEntities(string $classOrNamespace)
             }
 
             $getIsNullable = function (array $mapping) {
-                if (!isset($mapping['joinColumns'][0]) || !isset($mapping['joinColumns'][0]['nullable'])) {
+                if (!isset($mapping['joinColumns'][0]['nullable'])) {
                     // the default for relationships IS nullable
                     return true;
                 }

diff --git a/src/Doctrine/EntityRelation.php b/src/Doctrine/EntityRelation.php
@@ -39,7 +39,7 @@ final class EntityRelation
 
     public function __construct(string $type, string $owningClass, string $inverseClass)
     {
-        if (!in_array($type, self::getValidRelationTypes())) {
+        if (!\in_array($type, self::getValidRelationTypes())) {
             throw new \Exception(sprintf('Invalid relation type "%s"', $type));
         }
 

diff --git a/src/EventRegistry.php b/src/EventRegistry.php
@@ -96,7 +96,7 @@ public function getEventClassName(string $event)
         }
 
         foreach ($listeners as $listener) {
-            if (!is_array($listener) || 2 !== count($listener)) {
+            if (!\is_array($listener) || 2 !== \count($listener)) {
                 continue;
             }
 

diff --git a/src/FileManager.php b/src/FileManager.php
@@ -36,7 +36,7 @@ public function __construct(Filesystem $fs, AutoloaderUtil $autoloaderUtil, stri
         // update EntityRegeneratorTest to mock the autoloader
         $this->fs = $fs;
         $this->autoloaderUtil = $autoloaderUtil;
-        $this->rootDirectory = rtrim($this->realpath($this->normalizeSlashes($rootDirectory)), '/');
+        $this->rootDirectory = rtrim($this->realPath($this->normalizeSlashes($rootDirectory)), '/');
     }
 
     public function setIO(SymfonyStyle $io)
@@ -196,8 +196,7 @@ private function realPath($absolutePath): string
         $finalPath = implode('/', $finalParts);
         // Normalize: // => /
         // Normalize: /./ => /
-        $finalPath = str_replace('//', '/', $finalPath);
-        $finalPath = str_replace('/./', '/', $finalPath);
+        $finalPath = str_replace(['//', '/./'], '/', $finalPath);
 
         return $finalPath;
     }

diff --git a/src/Maker/MakeEntity.php b/src/Maker/MakeEntity.php
@@ -98,7 +98,7 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma
                 continue;
             }
 
-            $classes[] = str_replace('/', '\\', str_replace('.php', '', $item->getRelativePathname()));
+            $classes[] = str_replace(['.php', '/'], ['', '\\'], $item->getRelativePathname());
         }
 
         $argument = $command->getDefinition()->getArgument('name');
@@ -191,7 +191,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
             $fileManagerOperations = [];
             $fileManagerOperations[$entityPath] = $manipulator;
 
-            if (is_array($newField)) {
+            if (\is_array($newField)) {
                 $annotationOptions = $newField;
                 unset($annotationOptions['fieldName']);
                 $manipulator->addEntityField($newField['fieldName'], $annotationOptions);
@@ -254,7 +254,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
             }
 
             foreach ($fileManagerOperations as $path => $manipulatorOrMessage) {
-                if (is_string($manipulatorOrMessage)) {
+                if (\is_string($manipulatorOrMessage)) {
                     $io->comment($manipulatorOrMessage);
                 } else {
                     $this->fileManager->dumpFile($path, $manipulatorOrMessage->getSourceCode());
@@ -300,28 +300,29 @@ private function askForNextField(ConsoleStyle $io, array $fields, string $entity
                 return $name;
             }
 
-            if (in_array($name, $fields)) {
+            if (\in_array($name, $fields)) {
                 throw new \InvalidArgumentException(sprintf('The "%s" property already exists.', $name));
             }
 
             return Validator::validateDoctrineFieldName($name, $this->doctrineHelper->getRegistry());
         });
 
         if (!$fieldName) {
-            return;
+            return null;
         }
 
         $defaultType = 'string';
         // try to guess the type by the field name prefix/suffix
         // convert to snake case for simplicity
         $snakeCasedField = Str::asSnakeCase($fieldName);
-        if ('_at' == substr($snakeCasedField, -3)) {
+
+        if ('_at' === $suffix = substr($snakeCasedField, -3)) {
             $defaultType = 'datetime';
-        } elseif ('_id' == substr($snakeCasedField, -3)) {
+        } elseif ('_id' === $suffix) {
             $defaultType = 'integer';
-        } elseif ('is_' == substr($snakeCasedField, 0, 3)) {
+        } elseif (0 === strpos($snakeCasedField, 'is_')) {
             $defaultType = 'boolean';
-        } elseif ('has_' == substr($snakeCasedField, 0, 4)) {
+        } elseif (0 === strpos($snakeCasedField, 'has_')) {
             $defaultType = 'boolean';
         }
 
@@ -341,7 +342,7 @@ private function askForNextField(ConsoleStyle $io, array $fields, string $entity
                 $io->writeln('');
 
                 $type = null;
-            } elseif (!in_array($type, $allValidTypes)) {
+            } elseif (!\in_array($type, $allValidTypes)) {
                 $this->printAvailableTypes($io);
                 $io->error(sprintf('Invalid type "%s".', $type));
                 $io->writeln('');
@@ -350,7 +351,7 @@ private function askForNextField(ConsoleStyle $io, array $fields, string $entity
             }
         }
 
-        if ('relation' === $type || in_array($type, EntityRelation::getValidRelationTypes())) {
+        if ('relation' === $type || \in_array($type, EntityRelation::getValidRelationTypes())) {
             return $this->askRelationDetails($io, $entityClass, $type, $fieldName);
         }
 
@@ -414,9 +415,9 @@ private function printAvailableTypes(ConsoleStyle $io)
                 unset($allTypes[$mainType]);
                 $line = sprintf('  * <comment>%s</comment>', $mainType);
 
-                if (is_string($subTypes) && $subTypes) {
+                if (\is_string($subTypes) && $subTypes) {
                     $line .= sprintf(' (%s)', $subTypes);
-                } elseif (is_array($subTypes) && !empty($subTypes)) {
+                } elseif (\is_array($subTypes) && !empty($subTypes)) {
                     $line .= sprintf(' (or %s)', implode(', ', array_map(function ($subType) {
                         return sprintf('<comment>%s</comment>', $subType);
                     }, $subTypes)));
@@ -560,7 +561,7 @@ function ($name) use ($targetClass) {
             }
 
             // recommend an inverse side, except for OneToOne, where it's inefficient
-            $recommendMappingInverse = EntityRelation::ONE_TO_ONE === $relation->getType() ? false : true;
+            $recommendMappingInverse = EntityRelation::ONE_TO_ONE !== $relation->getType();
 
             $getterMethodName = 'get'.Str::asCamelCase(Str::getShortClassName($relation->getOwningClass()));
             if (EntityRelation::ONE_TO_ONE !== $relation->getType()) {
@@ -742,7 +743,7 @@ private function askRelationType(ConsoleStyle $io, string $entityClass, string $
         ));
         $question->setAutocompleterValues(EntityRelation::getValidRelationTypes());
         $question->setValidator(function ($type) {
-            if (!in_array($type, EntityRelation::getValidRelationTypes())) {
+            if (!\in_array($type, EntityRelation::getValidRelationTypes())) {
                 throw new \InvalidArgumentException(sprintf('Invalid type: use one of: %s', implode(', ', EntityRelation::getValidRelationTypes())));
             }
 
@@ -801,7 +802,7 @@ private function doesEntityUseAnnotationMapping(string $className): bool
                 return false;
             }
 
-            $className = (reset($otherClassMetadatas))->getName();
+            $className = reset($otherClassMetadatas)->getName();
         }
 
         $driver = $this->doctrineHelper->getMappingDriverForClass($className);

diff --git a/src/Maker/MakeValidator.php b/src/Maker/MakeValidator.php
@@ -16,7 +16,6 @@
 use Symfony\Bundle\MakerBundle\Generator;
 use Symfony\Bundle\MakerBundle\InputConfiguration;
 use Symfony\Bundle\MakerBundle\Str;
-use Symfony\Bundle\MakerBundle\Validator;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;

diff --git a/src/Str.php b/src/Str.php
@@ -25,7 +25,7 @@ final class Str
      */
     public static function hasSuffix(string $value, string $suffix): bool
     {
-        return 0 === strcasecmp($suffix, substr($value, -strlen($suffix)));
+        return 0 === strcasecmp($suffix, substr($value, -\strlen($suffix)));
     }
 
     /**
@@ -45,7 +45,7 @@ public static function addSuffix(string $value, string $suffix): string
      */
     public static function removeSuffix(string $value, string $suffix): string
     {
-        return self::hasSuffix($value, $suffix) ? substr($value, 0, -strlen($suffix)) : $value;
+        return self::hasSuffix($value, $suffix) ? substr($value, 0, -\strlen($suffix)) : $value;
     }
 
     /**
@@ -117,11 +117,11 @@ public static function asEventMethod(string $eventName): string
 
     public static function getShortClassName(string $fullClassName): string
     {
-        if (!empty(self::getNamespace($fullClassName))) {
-            return substr($fullClassName, strrpos($fullClassName, '\\') + 1);
-        } else {
+        if (empty(self::getNamespace($fullClassName))) {
             return $fullClassName;
         }
+
+        return substr($fullClassName, strrpos($fullClassName, '\\') + 1);
     }
 
     public static function getNamespace(string $fullClassName): string
@@ -141,7 +141,7 @@ public static function singularCamelCaseToPluralCamelCase(string $camelCase): st
     {
         $snake = self::asSnakeCase($camelCase);
         $words = explode('_', $snake);
-        $words[count($words) - 1] = Inflector::pluralize($words[count($words) - 1]);
+        $words[\count($words) - 1] = Inflector::pluralize($words[\count($words) - 1]);
         $reSnaked = implode('_', $words);
 
         return self::asLowerCamelCase($reSnaked);
@@ -151,7 +151,7 @@ public static function pluralCamelCaseToSingular(string $camelCase): string
     {
         $snake = self::asSnakeCase($camelCase);
         $words = explode('_', $snake);
-        $words[count($words) - 1] = Inflector::singularize($words[count($words) - 1]);
+        $words[\count($words) - 1] = Inflector::singularize($words[\count($words) - 1]);
         $reSnaked = implode('_', $words);
 
         return self::asLowerCamelCase($reSnaked);

diff --git a/src/Test/MakerTestCase.php b/src/Test/MakerTestCase.php
@@ -34,13 +34,13 @@ protected function executeMakerCommand(MakerTestDetails $testDetails)
             foreach ($files as $file) {
                 $this->assertTrue($testEnv->fileExists($file));
 
-                if ('.php' == substr($file, -4)) {
+                if ('.php' === substr($file, -4)) {
                     $csProcess = $testEnv->runPhpCSFixer($file);
 
                     $this->assertTrue($csProcess->isSuccessful(), sprintf('File "%s" has a php-cs problem: %s', $file, $csProcess->getOutput()));
                 }
 
-                if ('.twig' == substr($file, -5)) {
+                if ('.twig' === substr($file, -5)) {
                     $csProcess = $testEnv->runTwigCSLint($file);
 
                     $this->assertTrue($csProcess->isSuccessful(), sprintf('File "%s" has a twig-cs problem: %s', $file, $csProcess->getOutput()));

diff --git a/src/Test/MakerTestEnvironment.php b/src/Test/MakerTestEnvironment.php
@@ -142,7 +142,7 @@ public function runMaker()
 
         // We don't need ansi coloring in tests!
         $testProcess = MakerTestProcess::create(
-            sprintf('php bin/console %s %s --no-ansi', ($this->testDetails->getMaker())::getCommandName(), $this->testDetails->getArgumentsString()),
+            sprintf('php bin/console %s %s --no-ansi', $this->testDetails->getMaker()::getCommandName(), $this->testDetails->getArgumentsString()),
             $this->path,
             10
         );
@@ -184,9 +184,7 @@ public function getGeneratedFilesFromOutputText()
 
         preg_match_all('#(created|updated): (.*)\n#iu', $output, $matches, PREG_PATTERN_ORDER);
 
-        $files = array_map('trim', $matches[2]);
-
-        return $files;
+        return array_map('trim', $matches[2]);
     }
 
     public function fileExists(string $file)
@@ -237,7 +235,7 @@ public function reset()
 
         $diff = array_diff($currentSnapshot, $cleanSnapshot);
 
-        if (count($diff)) {
+        if (\count($diff)) {
             $this->fs->remove($diff);
         }
 

diff --git a/src/Test/MakerTestProcess.php b/src/Test/MakerTestProcess.php
@@ -51,7 +51,7 @@ public function run($allowToFail = false): self
     {
         $this->process->run();
 
-        if (!$this->process->isSuccessful() && !$allowToFail) {
+        if (!$allowToFail && !$this->process->isSuccessful()) {
             throw new \Exception(sprintf(
                 'Error running command: "%s". Output: "%s". Error: "%s"',
                 $this->process->getCommandLine(),

diff --git a/src/Util/AutoloaderUtil.php b/src/Util/AutoloaderUtil.php
@@ -77,13 +77,13 @@ private function getClassLoader(): ClassLoader
         if (null === $this->classLoader) {
             $autoloadFunctions = spl_autoload_functions();
             foreach ($autoloadFunctions as $autoloader) {
-                if (is_array($autoloader) && isset($autoloader[0]) && is_object($autoloader[0])) {
+                if (\is_array($autoloader) && isset($autoloader[0]) && \is_object($autoloader[0])) {
                     if ($autoloader[0] instanceof ClassLoader) {
                         $this->classLoader = $autoloader[0];
                         break;
                     }
                     if ($autoloader[0] instanceof DebugClassLoader
-                        && is_array($autoloader[0]->getClassLoader())
+                        && \is_array($autoloader[0]->getClassLoader())
                         && $autoloader[0]->getClassLoader()[0] instanceof ClassLoader) {
                         $this->classLoader = $autoloader[0]->getClassLoader()[0];
                         break;