cycle / annotated

Schema generation using annotated entities and mappers
MIT License
23 stars 12 forks source link

Duplicate namespaces in resources/stubs #38

Closed Faboslav closed 2 years ago

Faboslav commented 2 years ago

For example there is namespace Cycle\Annotated\Annotation; located in resources/stubs and also in src/Annotation. This can't be right, right? It is messing with psalm and other alike tools. Is there any solution for that?

Error generated by psalm looks like this:

ERROR: InvalidNamedArgument - app/Entity/Trait/CreatedAtTrait.php:12:17 - Parameter $type does not exist on function Cycle\Annotated\Annotation\Column::__construct (see https://psalm.dev/238)
        #[Column(type: 'datetime')]

It is probably using the first occurence in alphabet for that namespace?

butschster commented 2 years ago

Hi @Faboslav.

resources/stubs is used only for IDE autocompletion and you can ignore it.

Faboslav commented 2 years ago

Oh, i am ignoring it all right, but PSALM and PHPStan doesnt know that.

butschster commented 2 years ago

Oh, i am ignoring it all right, but PSALM and PHPStan doesnt know that.

You can ignore vendor directory via psalm config

<?xml version="1.0"?>
<psalm>
    <projectFiles>
        <directory name="src" />
        <ignoreFiles>
            <directory name="vendor" />
        </ignoreFiles>
    </projectFiles>
</psalm>
Faboslav commented 2 years ago

But i want to resolve that error which Is generated by PSALM caused by two identical namespaces in this package, which Is in general bad practice, right?

roxblnfk commented 2 years ago

It is possible to make annotations with named arguments like this: https://github.com/cycle/annotated/pull/34 But we don't have priority in this direction. We have some task about release ORM v2.

Faboslav commented 2 years ago

What, do we understand each other? Annotations are working great, but why do we need to have two identical namespaces? That Is highly unusual right? And static analytic can't deal with that.

roxblnfk commented 2 years ago

Psalm does not check fake attributes (stubs) because they are not added in composer.json psr-4 section. Static analysis cannot handle the signature of real attributes because they don't use named arguments.


Psalm sees this signature

https://github.com/cycle/annotated/blob/57be2acd6e962fc11e8a5a2bffe74b523a2a96c6/src/Annotation/Column.php#L54

and says that your definition #[Column(type: 'datetime')] is uncompatible. But it works because under hood the spiral/attributes package converts type: 'datetime' from metadata to ['type' => 'datetime'] and pass it in the attribute constructor.


There are two solutions: 1 Configure suppression InvalidNamedArgument for Cycle Attributes 2 Update attributes to using named arguments like this: #34

Faboslav commented 2 years ago

Okay, i get it now, thanks for your explanation and patience.

roxblnfk commented 2 years ago

No problem.

By the way. Instead of full ignoring some project files you can configure suppression for the InvalidNamedArgument handler only for cycle atributes. psalm.xml for example:

<?xml version="1.0"?>
<psalm
    ...
>
    ...
    <issueHandlers>
        ...
        <InvalidNamedArgument>
            <errorLevel type="suppress">
                <directory name="vendor/cycle/annotated/src/Annotation" />
            </errorLevel>
        </InvalidNamedArgument>
    </issueHandlers>
</psalm>
roxblnfk commented 2 years ago

Fixed in annotated v3.0.0 (Cycle ORM v2.0)

Faboslav commented 2 years ago

Oh this is cool, thank you for your hard work!