Base::unique contains null while type says it does not

[simPod]

Summary

Base::unique property is not initialized in constructor and its type does not allow null value.

Though, the null value is there so the type is wrong and it's a bug.

Versions

	Version
fakerphp/faker	1.y.z

Self-enclosed code snippet for reproduction

See #309 on how to fix the bug

[pimjansen]

What is the codesnippet to reproduce behaviour that is not expected.

It is not initialized which means initially it is null?

[simPod]

It is not initialized which means initially it is null?

Yes, how could it be something else?

final class Ooof extends Base
{
    public function readUnique() : UniqueGenerator
    {
        return $this->unique;
    }
}


$ooof = new Ooof(new Generator());
$ooof->readUnique(); // 🎶

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 1 week if no further activity occurs. Thank you for your contributions.

[bram-pkg]

It is not initialized which means initially it is null?

Yes, how could it be something else?

final class Ooof extends Base
{
    public function readUnique() : UniqueGenerator
    {
        return $this->unique;
    }
}


$ooof = new Ooof(new Generator());
$ooof->readUnique(); // 🎶

Is the the full code snippet, @simPod ?

[simPod]

@bramceulemans yes I guess? What are you asking exactly?

[bram-pkg]

Where is your constructor, your field assignment to $this->unique?

[simPod]

Those are your classes as stated in the issue summary

use Faker\Generator;
use Faker\Provider\Base;
use Faker\UniqueGenerator;

final class Ooof extends Base
{
    public function readUnique() : UniqueGenerator
    {
        return $this->unique;
    }
}

$ooof = new Ooof(new Generator());
$ooof->readUnique(); // 🎶

[bram-pkg]

You are missing the constructor in your Ooof class, or am I misunderstanding you?

In order to initialize the unique property on our Base class, you need to call unique().

[simPod]

If there's missing constructor, Base is the one missing it.

Either the unique property should be nullable or should be set some default value in Base.

[pimjansen]

I do not think we can do an easy fix here without it being a breaking change.

Since we have a 2.x roadmap we can not just push these breaking changes (since this class is not final).

So i think we should live with it for now

[simPod]

I've created the issue to track the bug so others who encounter it as well can subscribe. You've closed my PR that fixes it already. So the version&roadmap this should be fixed in is your decision ofc.

[pimjansen]

I've created the issue to track the bug so others who encounter it as well can subscribe. You've closed my PR that fixes it already. So the version&roadmap this should be fixed in is your decision ofc.

Yeah that is fine. In the future these will be final so we can control it a bit better

[bram-pkg]

I'm not sure that changing a phpdoc block would cause any breaking changes?

[pimjansen]

I'm not sure that changing a phpdoc block would cause any breaking changes?

It will not but initialising it as null will. At this point it has no value.

The main issue is that it can cause an error that the value is used before initialising.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 1 week if no further activity occurs. Thank you for your contributions.

[simPod]

387

[pimjansen]

I am closing this since this can not be fixed without a breaking change we do not allow at this point (see contribution guide). For Faker 2.x we will take this into account. Probably those classes will be final to start with so that this can not be extended and only decorated or something like that.