Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| } | ||
| privatefunctionrenderException(OutputInterface$output,string$template,Error$exception,string$file =null) | ||
| privatefunctionrenderException(SymfonyStyle$output,string$template,Error$exception,string$file =null) |
There was a problem hiding this comment.
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 aSymfonyStyle and we use the$object as aSymfonyStyle.
| publicfunctionsetPriority(int$priority):self | ||
| { | ||
| $this->message->setPriority($priority); | ||
| $this->message->priority($priority); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There is no function calledsetPriority. However, theEmail::priority() exists.
Im not sure if I should renameWrappedTemplatedEmail::setPriority() topriority() too.
There was a problem hiding this comment.
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
| publicfunctiongetETags() | ||
| { | ||
| returnpreg_split('/\s*,\s*/',$this->headers->get('if_none_match'),null, \PREG_SPLIT_NO_EMPTY); | ||
| returnpreg_split('/\s*,\s*/',$this->headers->get('if_none_match'),-1, \PREG_SPLIT_NO_EMPTY); |
There was a problem hiding this comment.
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 supportsnull in PHP 7. However, in PHP 8 it is required to be an int.
From PHPStorm's stubs:
If specified, then only substrings up tolimit are returned with the rest of the string being placed in the last substring. Alimit of -1, 0 orNULL means "no limit" and, as is standard across PHP, you can useNULL to skip to theflags 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".
| $descriptor->describe($io,$data,$context,$clientId); | ||
| }); | ||
| return0; |
There was a problem hiding this comment.
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.
| if (!\is_array($properties =$value->__serialize())) { | ||
| thrownew \Typerror($class.'::__serialize() must return an array'); | ||
| thrownew \TypeError($class.'::__serialize() must return an array'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Typo
| if (self::class ===$method->getDeclaringClass()->name && ($returnType =$method->getReturnType()) && !$returnType->isBuiltin()) { | ||
| $services[self::class.'::'.$method->name] ='?'.($returnTypeinstanceof \ReflectionNamedType ?$returnType->getName() :$type); | ||
| $services[self::class.'::'.$method->name] ='?'.($returnTypeinstanceof \ReflectionNamedType ?$returnType->getName() :$returnType); |
There was a problem hiding this comment.
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.
| * 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 |
There was a problem hiding this comment.
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.
TheSessionPass is passing "null" as parameter to this method to reset the factory.
| // 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]]))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Third parameter tofopen 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.
nicolas-grekas commentedApr 1, 2021
Thank you@Nyholm. |
When running psalm for branch 4.4, we get around 1500 errors. I browsed though them all and here is a small PR with the actual bugs I think should be fixed.
I'll make review comments about them below.