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

feat: create PhpStan type resolvers #456

Merged

Conversation

nikophil
Copy link
Member

@nikophil nikophil commented Apr 23, 2023

Here is a draft suggestion to add PHPStan type resolvers for factories' methods.

This would override the @phpstan-method stuff in the factories, and it has the big advantage to give us back the control over the type system, we could then update these as long as we're updating the code.

It has the drawback that we may need to support two versions of this: one for PHPStan, and another one for Psalm (and I'll have to learn how to make this for Psalm 😅) For now we can only drop the @phpstan-method generation with maker, and keep the @psalm-method annotation.

Some stuff still needs to be done:

  • I would like to merge refactor: split Factory class #452 before we will merge it in the branch feat/split-factories
  • We will be able to fix all type problems in refactor: split Factory class #452 (see here and here)
  • add tests
  • provide a foundry-phpstan.neon file
  • update doc: remove references to @phpstan-method and show how it should be configured
  • update make:factory
  • create a CHANGELOG.md to explain existing @phpstan-method can be dropped in userland

we can even imagine that once we've made the subtree split, we'd create a zenstruck/foundry-phpstan package which will be a phpstan extension and be auto installed with phpstan/extension-installer

WDYT of this?

Copy link
Member

@kbond kbond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! Yeah, I agree to wait for #452

@nikophil nikophil force-pushed the feat/phpstan-type-resolvers branch 2 times, most recently from a749c32 to b0980c3 Compare May 9, 2023 14:11
@nikophil nikophil changed the base branch from 1.x to feat/split-factories May 9, 2023 14:15
@nikophil nikophil force-pushed the feat/phpstan-type-resolvers branch from b0980c3 to 4b2dd2c Compare May 9, 2023 14:18
@nikophil nikophil force-pushed the feat/split-factories branch from 6d75461 to 6b8fb17 Compare May 10, 2023 12:19
@nikophil nikophil force-pushed the feat/phpstan-type-resolvers branch from 4b2dd2c to 8957818 Compare May 10, 2023 12:19
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't get rid of all these phpstan errors 🤷

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more and more thinking to analyse ./tests folder with phpstan... this will be useful to battle test the phpstan extension

@nikophil nikophil force-pushed the feat/phpstan-type-resolvers branch from 8957818 to 6e4ffd7 Compare May 10, 2023 12:29
@nikophil nikophil requested a review from kbond May 10, 2023 12:29
@nikophil nikophil marked this pull request as ready for review May 10, 2023 12:29
Copy link
Member

@kbond kbond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good to merge this into feat/split-factories

What about the todos though?

phpstan-foundry.neon Show resolved Hide resolved
@nikophil nikophil force-pushed the feat/split-factories branch 2 times, most recently from 831f799 to 4496bac Compare May 10, 2023 16:54
@nikophil nikophil force-pushed the feat/phpstan-type-resolvers branch 5 times, most recently from 0b94a41 to 657fb6f Compare May 14, 2023 18:35
@nikophil nikophil force-pushed the feat/phpstan-type-resolvers branch from 657fb6f to 5f631ac Compare May 14, 2023 18:41
@nikophil nikophil merged commit 8e856f4 into zenstruck:feat/split-factories May 14, 2023
@nikophil nikophil deleted the feat/phpstan-type-resolvers branch May 14, 2023 18:53
nikophil added a commit that referenced this pull request May 15, 2023
nikophil added a commit that referenced this pull request May 24, 2023
@nikophil nikophil mentioned this pull request Jun 9, 2023
nikophil added a commit that referenced this pull request Jul 21, 2023
nikophil added a commit that referenced this pull request Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants