typhoon-php / typhoon

Ultimate type system and reflection for PHP
MIT License
83 stars 1 forks source link

"Unsupported operand types" should not leads to runtime TypeError #64

Closed klimick closed 2 months ago

klimick commented 2 months ago
<?php

declare(strict_types=1);

use Typhoon\Reflection\TyphoonReflector;

require __DIR__ . '/vendor/autoload.php';

final class cls
{
    public const VALUE = [] & 'str';
}

var_dump(
    TyphoonReflector::build()
        ->reflectClass(cls::class)
        ->constants()['VALUE']
        ->value(),
);
klimenko@workstantion:~/IdeaProjects/typhoon$ php ./main.php 
PHP Fatal error:  Uncaught TypeError: Unsupported operand types: array & string in /home/klimenko/IdeaProjects/typhoon/src/Reflection/Internal/ConstantExpression/BinaryOperation.php:35
Stack trace:
#0 /home/klimenko/IdeaProjects/typhoon/src/Reflection/ClassConstantReflection.php(95): Typhoon\Reflection\Internal\ConstantExpression\BinaryOperation->evaluate()
#1 /home/klimenko/IdeaProjects/typhoon/main.php(18): Typhoon\Reflection\ClassConstantReflection->value()
#2 {main}
  thrown in /home/klimenko/IdeaProjects/typhoon/src/Reflection/Internal/ConstantExpression/BinaryOperation.php on line 35
vudaltsov commented 2 months ago

Do you mean that it should throw a different exception?

vudaltsov commented 2 months ago

I think, this is the way it should be. Native reflection behaves the same way:

https://3v4l.org/1vZ7H

klimick commented 2 months ago

https://3v4l.org/1vZ7H

is this really reflection behavior? exactly the same exception is thrown if you just create a class. https://3v4l.org/HHHia

Is it possible to avoid throwing any exception? Maybe return some container with or without a value.

I haven't tried to get inferred type of a constant, but I expect to get never. I don't expect any exception in this case.

I understand when an exception is thrown if a file could not be read due to unix permissions. But a tool that claims to work in a static analysis environment should not panic with exceptions in such simple situations.

vudaltsov commented 2 months ago

is this really reflection behavior?

The declaration of a class itself does not throw anything on the parsing level. TyphoonReflection and ReflectionClassConstant do not throw as well: https://3v4l.org/5KA9s until you request the value.

I haven't tried to get inferred type of a constant, but I expect to get never. I don't expect any exception in this case.

Not exactly, but no exception is thrown:

use Typhoon\Reflection\TypeKind;
use Typhoon\Reflection\TyphoonReflector;

final class C
{
    public const VALUE = [] & 'str';
}

$constant = TyphoonReflector::build()->reflectClass(C::class)->constants()['VALUE'];

var_dump($constant->type(TypeKind::Inferred)); // null
var_dump($constant->type(TypeKind::Resolved)); // types::mixed

But a tool that claims to work in a static analysis environment should not panic with exceptions in such simple situations

Static analysis tools must not use $constantReflection->value(), because it returns the actual runtime values an potentially autoloads smth. For instance, if you have

final class MyClass {}

function myFunction(MyClass $object = new MyClass()): void {}

then TyphoonReflector::build()->reflectFunction('myFunction')->parameters()['object']->defaultValue() will autoload MyClass, because the actual value is requested. There's nothing we can or should do about this.

Maybe return some container with or without a value.

$reflectionClassConstant->value(...) is your container. :)

vudaltsov commented 2 months ago

To summarize:

  1. I think that the current behavior is expected and correct.
  2. Static analysis tools should not call methods, that evaluate smth. Theses methods exist for hydrators, DI containers and other tools that do work at runtime.