[BUGFIX] partially satisfy psalm for src/HtmlProcessor/AbstractHtmlProcessor.php #805
Conversation
This commit resolves one of the PossiblyNullPropertyAssignmentValue issues identified in src/HtmlProcessor/AbstractHtmlProcessor.php at the expense of identifying various further Null issues regarding: MyIntervals#537
This commit resolves the PossiblyNullReference issues introduced by a previous [BUGFIX] in src/HtmlProcessor/AbstractHtmlProcessor.php by refactoring `$this->domDocument->` to `$this->getDomDocument()->` This necessarily alters the body of the previously identified NullableReturnStatement issue. regarding: MyIntervals#537
This commit resolves 1 InvalidNullableReturnType and 1 NullableReturnStatement issue identified in src/HtmlProcessor/AbstractHtmlProcessor.php by throwing an exception if `AbstractHtmlProcessor::$domDocument` has not been set. regarding: MyIntervals#537
|
Yoda is speaking :)) |
| */ | ||
| public function getDomDocument(): \DOMDocument | ||
| { | ||
| if (null === $this->domDocument) { |
There was a problem hiding this comment.
looks like this can't be cleanly cherry-picked, so it'll be addressed directly.
There was a problem hiding this comment.
resolved in 451e379
| throw new \UnexpectedValueException( | ||
| self::class . | ||
| '::setDomDocument() has not yet been called on ' . | ||
| static::class | ||
| ); |
There was a problem hiding this comment.
Could we have an exception code based on the current UNIX timestamp? (It will also mark your change on the timeline of all history, up until at least 2037, etc. :>)
There was a problem hiding this comment.
Also I prefer to have the . operators on the beginning of the next line rather than end of the preceding. But I don't think we have any stylistic rule about that - @oliverklee ?
It might become clearer when you add the 2nd argument - is the code easier to read with the . operator at the start of a new line? YMMV, and it's no big deal...
There was a problem hiding this comment.
It might become clearer when you add the 2nd argument - is the code easier to read with the
.operator at the start of a new line? YMMV, and it's no big deal...
my general preference is parenthesis.
There was a problem hiding this comment.
my general preference is parenthesis.
Good idea. LGTM now :)
This commit resolves the yoda conditions introduced in previous [BUGFIX] commits. regarding: * MyIntervals#794 * MyIntervals#778 (comment) * MyIntervals#805 (review)
This commit adjusts the indentation separate from the addition of parenthesis in a previous commit, on the basis of making indentation & line break changes separate from functional changes. regarding: MyIntervals#805 (comment)
No description provided.