nextras / orm

Orm with clean object design, smart relationship loading and powerful collections.
https://nextras.org/orm
MIT License
310 stars 57 forks source link

Entity property with unknown type is ignored #521

Open mabar opened 3 years ago

mabar commented 3 years ago

Describe the bug

Properties using generic array syntax and defining key type are ignored.

To Reproduce

  1. Create an entity (of type Nextras\Orm\Entity\Entity)
  2. Add property @property array<string, string> $example
  3. Assign value to $example property, EntityMetadata should throw exception

Expected behavior

@property array<string, string> $example is a valid property, same as (currently valid) @property array<string> $example or at least throw an exception describing syntax is not supported.

Versions::

hrach commented 3 years ago

The question is what is the proper fix. I'd rather refrain from depending on some phodoc parser able to handle this 🤔

mabar commented 3 years ago

I would say phpstan/phpdoc-parser is reasonable dependency. But not supporting it and just informing by throwing exception might be good enough too. Entity implementation using real properties and attributes may come with an alternative anyway so the opcache.save_comments could be enabled.

hrach commented 3 years ago

I would say there are three tasks:

The validation is rather too complicated and I would refrain from doing so 🤔 Supporting just the parsing could be nicely and easily done without an dependency. I do not expect any other more complex types like functional types and so.

hrach commented 2 years ago

With the increased amount of what types can be (newly union types in PHP 8.1), we should use PHPStans phpdoc parser.

The question is how the validation should work. Probably full validation should be implemented.

hrach commented 2 years ago

"Full" validation actually means just:

mabar commented 2 years ago

What will be the behavior for not supported types? Throw exception or note in documentation type may not be true? Exception is safer but it will likely lead to requests for types users actually do use.

hrach commented 2 years ago

Either:

/shrug