-
-
Notifications
You must be signed in to change notification settings - Fork 217
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
Fix PhpStan suggestions #378
base: master
Are you sure you want to change the base?
Conversation
…open-to-window links
src/Tracy/Logger/Logger.php
Outdated
@@ -163,7 +163,6 @@ protected function sendEmail($message): void | |||
|
|||
if ( | |||
$this->email | |||
&& $this->mailer |
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 wrong fix, instead the type of the $mailer
property should be changed to callable|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.
Ok, now I understand. In original implementation property $mailer
was setted by __construct
in all cases. But property is public and can be changed.
I reverted condition here and change property definition:
/** @var callable|null handler for sending emails */
public $mailer;
src/Bridges/Nette/TracyExtension.php
Outdated
$item = new Nette\DI\Statement(['@' . $builder::THIS_CONTAINER, 'getService'], [substr($item, 1)]); | ||
} elseif (is_string($item)) { | ||
$item = new Nette\DI\Statement($item); | ||
if (is_string($item)) { |
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.
why?
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.
Because in original implementation when you verify variable $item
is string, second elseif
condition does not make sense.
src/Tracy/Dumper/Dumper.php
Outdated
@@ -517,7 +518,7 @@ public static function encodeString(string $s, int $maxLength = null): string | |||
$i = $len = 0; | |||
$maxI = $maxLength * 4; // max UTF-8 length | |||
do { | |||
if (($s[$i] < "\x80" || $s[$i] >= "\xC0") && (++$len > $maxLength) || $i >= $maxI) { | |||
if (($s[$i] < "\x80" || $s[$i] >= "\xC0") && ((++$len > $maxLength) || $i >= $maxI)) { |
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.
why?
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.
Operations priority might differ from what programmer expect. Brackets are safe.
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.
if it's not related the phpstan error it should not be part of this PR
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.
you should not mix code changes related to phpstan errors with code style changes in one PR
dbabb37
to
eb2e772
Compare
de3ad52
to
191c0d2
Compare
2b958bb
to
f36b649
Compare
68b0ec8
to
344c772
Compare
c3c8bf0
to
6f7e1c3
Compare
84dee11
to
bd73e33
Compare
7799297
to
a386d38
Compare
f3f04e0
to
0a90e93
Compare
5ee33d7
to
8e708de
Compare
Fixed PhpStan suggestions.
New errors:
Old errors: