Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Dynamically type getters and hassers for parameters #152

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

Merged
ondrejmirtes merged 7 commits into phpstan:master from Kocal:GH-127
May 2, 2021

Conversation

@Kocal
Copy link
Contributor

@Kocal Kocal commented May 1, 2021
edited
Loading

Hi!
This is a proposal for #127.

I've re-used the same logic for services:

  • read and parse the Symfony container XML dump (it would have been simpler with a PHP dump 😛)
  • create a ParameterMap which contains all parameters

Dynamic return types have been implemented for:

  • ParameterBagInterface#get and ParameterBagInterface#has
  • ContainerInterface#getParameter and ContainerInterface#hasParameter
  • AbstractController#getParameter and Controller#getParameter

(削除) However, I'm not able to do the same for AbstractController#getParameter and Controller#getParameter when using $this->getParameter('...') inside a AbstractController and Controller. It always use the return type from AbstractController#getParameter. Is this a limitation from PHPStan or I did something wrong @ondrejmirtes? (削除ここまで)

(削除) Also in Symfony 4.0 (lowest deps), there is no method AbstractController#getParameter() and it make the tests failing. What is the best way to support lowest and highest Symfony version in tests and extension? What about using Symfony 4.4 as minimal requirements and dropping Controller usage? This is the only 4.x supported version right now. (削除ここまで)
EDIT: Nevermind I fixed in c2d3f79 by using 4.4 as lowest version for symfony/framework-bundle, and manually create the default type if a parameter does not exist.

mhujer and tristanbes reacted with thumbs up emoji
@Kocal Kocal changed the title (削除) feat: create Parameter, ParameterMap, ParameterMapFactory, etc... (削除ここまで) (追記) Dynamically type getters and hassers for parameters (追記ここまで) May 1, 2021
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

Hi, I love this! This is well thought out and I found only a single minor problem :) I wish that more people like you contributed to PHPStan :) Thank you.

Kocal and loic425 reacted with heart emoji
Copy link
Member

Alright: a586e24

I'm gonna squash this and release it :)

@ondrejmirtes ondrejmirtes merged commit a207db0 into phpstan:master May 2, 2021
Copy link
Member

Thank you!

Copy link
Contributor

mhujer commented May 2, 2021
edited
Loading

@Kocal 👍 👏 Just tested your branch on our project and it found the following:
(all of them are valid reports which need to be fixed in our code - all are related to request parameter)

  • Offset 0 does not exist on array<string>|string|null.
  • Cannot cast array<string>|string|null to int. (at (int) $rawAmount)
  • Parameter #1 $input of method FooBar::fromString() expects string|null, array<string>|string|null given. (when passing a $request->query->all()['amount'])
  • Parameter #1 $parameters of method BarBaz::filterParameters() expects array<string>, array<string, array<string>|string> given when passing $request->query->all()
Kocal reacted with heart emoji

Copy link
Member

Enjoy it in phpstan-symfony 0.12.29 :)

mhujer reacted with thumbs up emoji

@Kocal Kocal deleted the GH-127 branch May 2, 2021 09:35
Copy link
Contributor Author

Kocal commented May 2, 2021

Nice! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ondrejmirtes ondrejmirtes ondrejmirtes approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /