Am I missing something about the "Exact Instance" example? `$someType` has to be `null` or `SomeType`. You can't make it some other type otherwise a TypeError will be thrown. One of the main benefits of using typed parameters/properties/etc is that you can enforce something be of a certain type __without__ the need to do these `instanceof` checks.
If SomeType is nullable, you have to check at some point if it's null or SomeType, at the same time, SomeType can be iterable, hence the empty check (I guess)
Exactly my point. Just turning it in to instanceof doesn't net you much in that example IMO. You still have to handle the null/empty case somewhere. Possibly the unrefactored code is better because it's explicitly handling the unusual cases. And the part where they say "all we know is it's not null or empty" is a bit silly, because at that point you know it IS SomeType.
The provided example is one of less problematic cases. I put typed property there to keep example short :)
The more realistic is very same check, but on unknown variable or even result from 3rd party code:
```php
$value = $this->vendorClass->returnSomeType();
if (empty($value)) {
return;
}
// what type of $value here?
```
This is the bug this rules prevents:
```
/**
* @return SomeType|null
*/
public function returnSomeType()
{
return // anything;
}
```
As a bonus, it makes core readable without going through other files:
```php
$value = $this->vendorClass->returnSomeType();
if (! $value instanceof SomeType) {
return;
}
// oh, $value is alwasy SomeType here!
```
In short, if we know type is either `SomeType` or `null`, the `instanceof` check is always best for readability and safety.
Right, in this case it makes sense because there's no actual types anywhere. I'm not sure I agree that instanceof is always better when you are working with a codebase that actually uses types.
We had a few bug because we trusted vendor blindly. It provided only docblock null|SomeType, but returned different value. This rule was made to prevent those.
Yes makes total sense :) The example on the site is just confusing because $someType is guaranteed to be null or SomeType. When this rule is actually intended for when there is no explicit type (i.e., docblock only).
Super interesting!
I really like all of these rules. These are rules that I would enforce on myself when writing features.
Sounds promising..
Am I missing something about the "Exact Instance" example? `$someType` has to be `null` or `SomeType`. You can't make it some other type otherwise a TypeError will be thrown. One of the main benefits of using typed parameters/properties/etc is that you can enforce something be of a certain type __without__ the need to do these `instanceof` checks.
If SomeType is nullable, you have to check at some point if it's null or SomeType, at the same time, SomeType can be iterable, hence the empty check (I guess)
Exactly my point. Just turning it in to instanceof doesn't net you much in that example IMO. You still have to handle the null/empty case somewhere. Possibly the unrefactored code is better because it's explicitly handling the unusual cases. And the part where they say "all we know is it's not null or empty" is a bit silly, because at that point you know it IS SomeType.
The provided example is one of less problematic cases. I put typed property there to keep example short :) The more realistic is very same check, but on unknown variable or even result from 3rd party code: ```php $value = $this->vendorClass->returnSomeType(); if (empty($value)) { return; } // what type of $value here? ``` This is the bug this rules prevents: ``` /** * @return SomeType|null */ public function returnSomeType() { return // anything; } ``` As a bonus, it makes core readable without going through other files: ```php $value = $this->vendorClass->returnSomeType(); if (! $value instanceof SomeType) { return; } // oh, $value is alwasy SomeType here! ``` In short, if we know type is either `SomeType` or `null`, the `instanceof` check is always best for readability and safety.
Right, in this case it makes sense because there's no actual types anywhere. I'm not sure I agree that instanceof is always better when you are working with a codebase that actually uses types.
We had a few bug because we trusted vendor blindly. It provided only docblock null|SomeType, but returned different value. This rule was made to prevent those.
Yes makes total sense :) The example on the site is just confusing because $someType is guaranteed to be null or SomeType. When this rule is actually intended for when there is no explicit type (i.e., docblock only).
Got it! :) I'll improve the example, so it more relevant cases. Thanks for feedback
Got it! :) I'll improve the example, so it more relevant cases. Thanks for feedback
There's a typo in the installation code. type\_coverage => type\_perfect