Fix PHPDoc

[dontub]

No description provided.

[nikolaposa]

Actually, neither preRelease or build cannot be null, but either https://github.com/nikolaposa/version/blob/master/src/Extension/NoPreRelease.php or https://github.com/nikolaposa/version/blob/master/src/Extension/NoBuild.php. (Special Case design pattern)

So you've discovered a potential bug, because rest of the Version class assumes preRelease will never be null (for example: https://github.com/nikolaposa/version/blob/master/src/Version.php#L201), but these methods allow null to be set. Therefore we should focus on fixing that.

[dontub]

Actually the result of $version->withPreRelease(null) and $version->withPreRelease(new NoPreRelease()) is the same. Also Version::formPart(1, 0, 0, null) would set the pre release part to NoPreRelease. Using null to set the pre release part to NoPreRelease makes totally sense to me. (The same holds true for the build part.)

[nikolaposa]

Yeah, which is why we need to fix those two methods to convert passed nulls into appropriate Special Case objects. Editing docblock type hints isn't enough.

[dontub]

Yeah, which is why we need to fix those two methods to convert passed nulls into appropriate Special Case objects.

This already happens in the constructor.

[nikolaposa]

Correct, but it doesn't happen in these setter methods. Let me demonstrate what could happen:

$version->withPreRelease(null);
$version->isPreRelease(); //Fatal error: call to a member function isEmpty() on null

Therefore they need to be modified, like so for example:

public function withPreRelease($preRelease) : Version
{
    if (is_string($preRelease)) {
        //...
    } elseif (null === $preRelease) {
        $preRelease = new NoPreRelease();
    }
        
    //...
}

[dontub]

I have to correct myself: it's in fromParts() not in the constructor where null is replaced by an object of type NoPreRelease and NoBuild, respectively.

The code already prevents that any attribute may became null because the only place where attributes are set is in the constructor and it's signature ensures that they won't be null:

__construct(int $major, int $minor, int $patch, PreRelease $preRelease, Build $build)

This test shows that everything is as it should be:

$version1 = Version::fromParts(1, 2, 3, PreRelease::fromIdentifiers('4'));
$this->assertEquals(PreRelease::fromIdentifiers('4'), $version1->getPreRelease());
$this->assertTrue($version1->isPreRelease());
$version2 = $version1->withPreRelease(null);
$this->assertEquals(new NoPreRelease(), $version2->getPreRelease());
$this->assertFalse($version2->isPreRelease());

[nikolaposa]

Yeah, we are at the same page now, got it. 👍