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
4.x psalm #13601
Conversation
I am done for this round. Will make a separate PR for further updates. |
*/ | ||
protected $_err; | ||
|
||
/** | ||
* Console input mock | ||
* | ||
* @var \Cake\Console\ConsoleInput|\PHPUnit\Framework\MockObject\MockObject|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.
Weren't these added to fix an IDE? Are these still needed?
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 exec()
method set's the property to concrete class \Cake\Console\ConsoleInput
so the |\PHPUnit\Framework\MockObject\MockObject
isn't necessary.
@@ -105,6 +105,7 @@ public function exec(string $command, array $input = []): void | |||
try { | |||
$this->_exitCode = $runner->run($args, $io); | |||
} catch (StopException $exception) { | |||
/** @var int $this->_exitCode */ | |||
$this->_exitCode = $exception->getCode(); |
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.
Should we use a cast instead of a comment?
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.
All our exception classes do actually return int so a casting is necessary, hence the ignore. I'll try and get the issue sorted out with psalm itself to allow us to override the return type for our Cake\Core\Exception\Exception
class.
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 psalm issue is fixed vimeo/psalm#2095.
So once a new release is done we can remove all unnecessary annotations / casting where getCode()
is used 🙂.
@@ -296,7 +296,7 @@ protected function _toggledLink($text, $enabled, $options, $templates): string | |||
$text = $options['escape'] ? h($text) : $text; | |||
|
|||
$templater = $this->templater(); | |||
$newTemplates = !empty($options['templates']) ? $options['templates'] : false; | |||
$newTemplates = $options['templates'] ?? false; |
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.
Do we care if templates are ''
?
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.
I made StringTemplate::load()
throw an exception instead if argument is empty string.
No description provided.