Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate namespaces in resources/stubs #38

Closed
Faboslav opened this issue Dec 6, 2021 · 11 comments
Closed

Duplicate namespaces in resources/stubs #38

Faboslav opened this issue Dec 6, 2021 · 11 comments
Assignees
Labels
help wanted Extra attention is needed type:question Further information is requested
Milestone

Comments

@Faboslav
Copy link

Faboslav commented Dec 6, 2021

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?

@Faboslav Faboslav changed the title Duplicate namespaces Duplicate namespaces in resources/stubs Dec 6, 2021
@butschster
Copy link
Contributor

Hi @Faboslav.

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

@Faboslav
Copy link
Author

Faboslav commented Dec 6, 2021

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

@butschster
Copy link
Contributor

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>

@butschster butschster added the help wanted Extra attention is needed label Dec 6, 2021
@butschster butschster self-assigned this Dec 6, 2021
@Faboslav
Copy link
Author

Faboslav commented Dec 6, 2021

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
Copy link
Member

roxblnfk commented Dec 6, 2021

It is possible to make annotations with named arguments like this: #34
But we don't have priority in this direction. We have some task about release ORM v2.

@Faboslav
Copy link
Author

Faboslav commented Dec 6, 2021

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
Copy link
Member

roxblnfk commented Dec 7, 2021

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

public function __construct(array $values)

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
Copy link
Author

Faboslav commented Dec 7, 2021

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

@roxblnfk
Copy link
Member

roxblnfk commented Dec 8, 2021

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>

@butschster butschster added this to the v1.0 milestone Dec 8, 2021
@butschster butschster added the type:question Further information is requested label Dec 8, 2021
@roxblnfk
Copy link
Member

roxblnfk commented Dec 29, 2021

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

@Faboslav
Copy link
Author

Faboslav commented Jan 2, 2022

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

@roxblnfk roxblnfk modified the milestones: 2.4.x, 3.0.0 Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed type:question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants