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

The class ClassMetada is not correctly inferring the constructor of ClassMetadaInfo #8709

Closed
VincentLanglet opened this issue May 23, 2021 · 5 comments · Fixed by #8734
Closed

Comments

@VincentLanglet
Copy link
Contributor

Hi

Currently

  • new ClassMetadataInfo(\DateTime::class) is inferred as ClassMetadataInfo<\DateTime> by both psalm and phpstan
  • new ClassMetadata(\DateTime::class) is inferred as ClassMetadata<\DateTime> by psalm but as ClassMetadata<object> by phpstan

This is related to this phpstan issue phpstan/phpstan#5057
I don't know how and when the issue will be fixed so one way to fix it temporary would be to repeat the constructor:

        /**
	 * @psalm-param class-string<T> $class
	 **/
	public function __construct(string $class)
	{
		parent::__construct($class);
	}

Since you're using phpstan in this project, you might end with this issue while bumping the phpstan level.
Do you prefer I create a PR here or in https://github.com/phpstan/phpstan-doctrine ?
(The second solution won't fix the issue here since you're not using the library here)

@VincentLanglet
Copy link
Contributor Author

I saw you did a lot of static analysis on this repository @greg0ire, maybe you can tell me (or ping the right person) to decide if I should do a PR here or in phpstan repository ? :)

@greg0ire
Copy link
Member

🤔 isn't that going to be fixed by #8708 ?

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented May 24, 2021

Sorry, my first post wasn't clear because I played with my vendor.

Currently

  • new ClassMetadataInfo(\DateTime::class) is not inferred
  • new ClassMetadata(\DateTime::class) is not inferred

After #8708

  • new ClassMetadataInfo(\DateTime::class) will be inferred as ClassMetadataInfo<\DateTime> by both psalm and phpstan
  • new ClassMetadata(\DateTime::class) will be inferred as ClassMetadata<\DateTime> by psalm but as ClassMetadata by phpstan.

    So an issue will still be present with ClassMetadata and phpstan.

@greg0ire
Copy link
Member

I see. I think it should be fixed here, with the repetition you suggested. In order for that constructor not to get removed, please also add something similar to what Marco did in doctrine/dbal#4638

It was already suggested here BTW: #8633 (comment)

@VincentLanglet
Copy link
Contributor Author

Solved in #8734

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants