Skip to content

Quality fixes (PHP Inspections) #184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Command/MakerCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
}

Expand Down
6 changes: 3 additions & 3 deletions src/DependencyBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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) {
Expand All @@ -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 = [];
Expand Down
2 changes: 1 addition & 1 deletion src/Doctrine/DoctrineHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 1 addition & 3 deletions src/Doctrine/EntityRegenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Doctrine/EntityRelation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
2 changes: 1 addition & 1 deletion src/EventRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 2 additions & 3 deletions src/FileManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
Expand Down
33 changes: 17 additions & 16 deletions src/Maker/MakeEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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';
}

Expand All @@ -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('');
Expand All @@ -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);
}

Expand Down Expand Up @@ -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)));
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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())));
}

Expand Down Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/Maker/MakeValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 7 additions & 7 deletions src/Str.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
}

/**
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/Test/MakerTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
8 changes: 3 additions & 5 deletions src/Test/MakerTestEnvironment.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -237,7 +235,7 @@ public function reset()

$diff = array_diff($currentSnapshot, $cleanSnapshot);

if (count($diff)) {
if (\count($diff)) {
$this->fs->remove($diff);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Test/MakerTestProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
4 changes: 2 additions & 2 deletions src/Util/AutoloaderUtil.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading