8000 Fixed bugs found by psalm by Nyholm · Pull Request #40610 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Fixed bugs found by psalm #40610

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.

Sign up for GitHub

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
Apr 1, 2021
Merged

Fixed bugs found by psalm #40610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/Command/LintCommand.php
8000
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ private function displayJson(OutputInterface $output, array $filesInfo)
return min($errors, 1);
}

private function renderException(OutputInterface $output, string $template, Error $exception, string $file = null)
private function renderException(SymfonyStyle $output, string $template, Error $exception, string $file = null)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a minor. We pass a SymfonyStyle and we use the $object as a SymfonyStyle.

{
$line = $exception->getTemplateLine();

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/Mime/WrappedTemplatedEmail.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public function getBcc(): array
*/
public function setPriority(int $priority): self
{
$this->message->setPriority($priority);
$this->message->priority($priority);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no function called setPriority. However, the Email::priority() exists.

Im not sure if I should rename WrappedTemplatedEmail::setPriority() to priority() too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the class is internal, no need to care about renaming, it won't improve anything


return $this;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/DependencyInjection/Definition.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public function setChanges(array $changes)
/**
* Sets a factory.
*
* @param string|array|Reference $factory A PHP function, reference or an array containing a class/Reference and a method to call
* @param string|array|Reference|null $factory A PHP function, reference or an array containing a class/Reference and a method to call
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also a minor.

The SessionPass is passing "null" as parameter to this method to reset the factory.

*
* @return $this
*/
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Filesystem/Filesystem.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function copy($originFile, $targetFile, $overwriteNewerFiles = false)
}

// Stream context created to allow files overwrite when using FTP stream wrapper - disabled by default
if (false === $target = @fopen($targetFile, 'w', null, stream_context_create(['ftp' => ['overwrite' => true]]))) {
if (false === $target = @fopen($targetFile, 'w', false, stream_context_create(['ftp' => ['overwrite' => true]]))) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Third parameter to fopen bust be a boolean. Default is `false´.

From the docs:

The optional third use_include_path parameter can be set to '1' or true if you want to search for the file in the include_path, too.

Please very this extra carefully.

throw new IOException(sprintf('Failed to copy "%s" to "%s" because target file could not be opened for writing.', $originFile, $targetFile), 0, null, $originFile);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpFoundation/Request.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1564,7 +1564,7 @@ public function getContent($asResource = false)
*/
public function getETags()
{
return preg_split('/\s*,\s*/', $this->headers->get('if_none_match'), null, \PREG_SPLIT_NO_EMPTY);
return preg_split('/\s*,\s*/', $this->headers->get('if_none_match'), -1, \PREG_SPLIT_NO_EMPTY);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preg_split's third argument supports null in PHP 7. However, in PHP 8 it is required to be an int.

From PHPStorm's stubs:

If specified, then only substrings up to limit are returned with the rest of the string being placed in the last substring. A limit of -1, 0 or NULL means "no limit" and, as is standard across PHP, you can use NULL to skip to the flags parameter.

From the docs (PHP 8 ready):

If specified, then only substrings up to limit are returned with the rest of the string being placed in the last substring. A limit of -1 or 0 means "no limit".

}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/VarDumper/Command/ServerDumpCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,5 +95,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$this->server->listen(function (Data $data, array $context, int $clientId) use ($descriptor, $io) {
$descriptor->describe($io, $data, $context, $clientId);
});

return 0;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a command that does not return an int.

}
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/VarExporter/Internal/Exporter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public static function prepare($values, $objectsPool, &$refsPool, &$objectsCount
}

if (!\is_array($properties = $value->__serialize())) {
throw new \Typerror($class.'::__serialize() must return an array');
throw new \TypeError($class.'::__serialize() must return an array');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo

}

goto prepare_value;
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Contracts/Service/ServiceSubscriberTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static function getSubscribedServices(): array
}

if (self::class === $method->getDeclaringClass()->name && ($returnType = $method->getReturnType()) && !$returnType->isBuiltin()) {
$services[self::class.'::'.$method->name] = '?'.($returnType instanceof \ReflectionNamedType ? $returnType->getName() : $type);
$services[self::class.'::'.$method->name] = '?'.($returnType instanceof \ReflectionNamedType ? $returnType->getName() : $returnType);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable $type is never defined in this class. I assume we mean $returnType.

}
}

Expand Down
0