nette / php-generator

🐘 Generates neat PHP code for you. Supports new PHP 8.3 features.
https://doc.nette.org/php-generator
Other
2.11k stars 138 forks source link

Namespace not recognizing boolean and integer as keywords #57

Closed TonyBogdanov closed 4 years ago

TonyBogdanov commented 4 years ago

Version: 3.3.4

Bug Description

When generating a namespace the PhpNamespace contains a constant of primitive types to be considered as "keywords" and thus not to be imported. The boolean keyword is missing from the list (bool is present instead), same goes for integer. The problem with this is, that if you are using reflections to determine type hints the returned names would be the full-length names, e.g. integer and not int, which in turn would generate stuff like getTheValue(): \boolean;.

dg commented 4 years ago

I think that reflection always returns short forms: bool, int. Can you show an example when reflection returns boolean or integer?

TonyBogdanov commented 4 years ago

My particular issue came from this ReflectionNamedType::getName, which unfortunately doesn't seem to be well documented.

But it got me curious, so I abused GitHub's VMs on one of my repos to see what happens in practice, and I was actually surprised. Seems like reflection doesn't make any assumptions or perform any normalizations, meaning that if the type is defined as bool in the source, it will return bool, if it is defined as boolean it will return boolean instead.

I ran the experiment on the following combos:

All of them returned the same thing.

Experiment:

<?php

function test(

    string $string,
    int $int,
    integer $integer,
    float $float,
    double $double,
    bool $bool,
    boolean $boolean,
    array $array,
    object $object,
    mixed $explicitMixed,
    $implicitMixed,
    resource $resource

) {}

$reflection = new ReflectionFunction( 'test' );

foreach ( $reflection->getParameters() as $parameter ) {

    /** @var ReflectionNamedType|null $type */
    $type = $parameter->getType();

    echo $parameter->getName() . ' => ';
    if ( isset( $type ) ) {

        echo $type->getName();

    } else {

        echo 'NULL';

    }

    echo "\n";

}

Result:

string => string
int => int
integer => integer
float => float
double => double
bool => bool
boolean => boolean
array => array
object => object
explicitMixed => mixed
implicitMixed => NULL
resource => resource

I guess you can't make any assumptions with reflection. The explicit / implicit mixed is making me quite uneasy too...

dg commented 4 years ago

It is because boolean Is not boolean, it is class named boolean.

TonyBogdanov commented 4 years ago

@dg Uff, you're absolutely right, no idea how I missed that.

I backtracked and now I know where my problem came from. I was using gettype when I was generating a class from an actual value, and reflection when reading back the generated class.

Now I need to find an alternative to gettype that's actually consistent with the rest of the language...

dg commented 4 years ago

Yes, gettype returns obsolete types…

Try this Nette\PhpGenerator\Type::getType($var)

TonyBogdanov commented 4 years ago

Awesome, thanks @dg !