Skip to content

Conversation

@szepeviktor
Copy link

@szepeviktor szepeviktor commented Aug 5, 2023

Please be gentle with me. I am very bad at arguing.

In writeTo and in performWrite "return-false world" and exception world (OOP) collide 💥
Please thrown an exception, do not return a boolean.

OR! :) https://phpstan.org/r/d87dbd47-070a-4cd7-a85c-0a44067e5fdd

* If this Preloader should run
*
* @var callable
* @var callable|null
Copy link
Author

@szepeviktor szepeviktor Aug 5, 2023

Choose a reason for hiding this comment

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

$condition property could be uninitialized.

public function getStatus() : array
{
if ($this->status ??= opcache_get_status(true)) {
if ($this->status = $this->status ?: opcache_get_status(true)) {
Copy link
Author

Choose a reason for hiding this comment

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

status is not nullable.

Comment on lines -155 to 157
{
if (file_put_contents($path, $this->prepareCompiler($path)->compile(), LOCK_EX)) {
return true;
if (file_put_contents($path, $this->prepareCompiler($path)->compile(), LOCK_EX) === false) {
throw new RuntimeException("Preloader couldn't write the script to [$path].");
}
Copy link
Author

@szepeviktor szepeviktor Aug 5, 2023

Choose a reason for hiding this comment

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

Put the unhappy path in the conditional.

source

* @var false|string
*/
public string $contents;
public $contents;
Copy link
Author

Choose a reason for hiding this comment

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

$contents could be false.

* @return string|string[]
*/
public function compile() : string
public function compile()
Copy link
Author

Choose a reason for hiding this comment

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

compile() can return an array.

]);

return str_replace(array_keys($replacing), $replacing, $this->contents);
return str_replace(array_keys($replacing), $replacing, $this->contents ?: []);
Copy link
Author

Choose a reason for hiding this comment

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

$contents could be false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant