Skip to content

Try to provide literal int types when possible (fixes #6966)#7071

Merged
orklah merged 11 commits intovimeo:masterfrom
ricardoboss:issue-6966-argumenttypecoercion_for_typecasted_vars
Jan 16, 2022
Merged

Try to provide literal int types when possible (fixes #6966)#7071
orklah merged 11 commits intovimeo:masterfrom
ricardoboss:issue-6966-argumenttypecoercion_for_typecasted_vars

Conversation

@ricardoboss
Copy link
Copy Markdown
Contributor

@ricardoboss ricardoboss commented Dec 5, 2021 •
edited
Loading

This PR fixes #6966.

I added a return type provider for the round function, which will now try to return a literal float, which can then be used for further analysis.
The second change involves modifying the CastAnalyzer to also return a literal int when possible. I also tried to simplify the code a bit.

@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Dec 5, 2021

Thanks!

You have to enable the return type provider in FunctionReturnTypeProvider first

@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Dec 5, 2021

I made an error in my explanation. round only returns floats:

https://3v4l.org/ETvDv

That shouldn't be an issue however, what you have to do to fix #6966 is check if you have a TLiteralInt or a TLiteralFloat in the num_arg. If it is, you should return a TLiteralFloat with the rounded value of what was in num_arg.

@ricardoboss
Copy link
Copy Markdown
Contributor Author

ricardoboss commented Dec 5, 2021

I need to implement the CastAnalyzer too. So this is should really be a draft PR.

@ricardoboss ricardoboss changed the title Fixed vimeo/psalm#6966 WIP: Fix vimeo/psalm#6966 Dec 5, 2021
@ricardoboss ricardoboss changed the title WIP: Fix vimeo/psalm#6966 Try to provide literal int types when possible (fixes #6966) Dec 5, 2021
@ricardoboss
Copy link
Copy Markdown
Contributor Author

ricardoboss commented Dec 5, 2021

I'm struggling to get the actual values with which the round method was called. As you can see, I'm trying to get them with $call_args[1]->value, which gives a PhpParser\Node\Expr. How do I get the actual value?

@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Dec 5, 2021

try a $statements_analyzer->node_data->getType(...) on that. It may be a complex expression but Psalm should already have inferred it

@weirdan
Copy link
Copy Markdown
Collaborator

weirdan commented Dec 5, 2021

E.g.

psalm/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFillReturnTypeProvider.php

Lines 27 to 30 in 25614ec

$first_arg_type = isset($call_args[0]) ? $statements_source->node_data->getType($call_args[0]->value) : null;
$second_arg_type = isset($call_args[1]) ? $statements_source->node_data->getType($call_args[1]->value) : null;
$third_arg_type = isset($call_args[2]) ? $statements_source->node_data->getType($call_args[2]->value) : null;

@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Dec 8, 2021

@ricardoboss is this ready for review?

@ricardoboss
Copy link
Copy Markdown
Contributor Author

ricardoboss commented Dec 8, 2021

Yes! Sorry I didn't notify you...

@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Dec 8, 2021

No worries, we'll check that :)

orklah
orklah reviewed Dec 8, 2021
View reviewed changes
Comment thread src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php Outdated
orklah
orklah reviewed Dec 8, 2021
View reviewed changes
Comment thread src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php Outdated
orklah
orklah reviewed Dec 8, 2021
View reviewed changes
Comment thread src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php Outdated
@ricardoboss
Copy link
Copy Markdown
Contributor Author

ricardoboss commented Dec 9, 2021

@orklah can you take a look again?

orklah
orklah requested changes Dec 9, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@orklah orklah left a comment

Choose a reason for hiding this comment

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

See comment above. Can you add some tests too?

@orklah orklah marked this pull request as draft December 14, 2021 23:53
@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Dec 29, 2021

@ricardoboss do you need some help to design the tests?

Could you also rebase to solve the conflicts?

@ricardoboss
Copy link
Copy Markdown
Contributor Author

ricardoboss commented Dec 30, 2021

Will do a rebase! I haven't had time to write tests yet. Haven't forgot it either.

@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Jan 15, 2022

Sorry for not coming back to this earlier, I don't get notifications for commits so I didn't see this change. I'll take a look. Don't hesitate to ping me or leave a comment when you change things

@orklah orklah added the release:feature The PR will be included in 'Features' section of the release notes label Jan 15, 2022
@orklah orklah marked this pull request as ready for review January 15, 2022 10:47
@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Jan 15, 2022

Seems fine. Can you rebase and fix the Code Style error please?

@ricardoboss
Copy link
Copy Markdown
Contributor Author

ricardoboss commented Jan 15, 2022

Can you write a test for this? I would like to see how you test this behavior.

I will do a rebase when I get time to do so

@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Jan 15, 2022

For the cast one, you can add a test in ValueTest.php in providerValidCodeParse with the code: $a = (int)'5'; and in the assertions you can check $a=== => '5'

For the round one, you can add a test in FunctionCallTest.php in providerValidCodeParse with the code $a = round(5.111, 2); and in the assertions you can check $a=== => 'float(5.11)'

@orklah
Copy link
Copy Markdown
Collaborator

orklah commented Jan 16, 2022

Thanks!

@orklah orklah merged commit 26dd4c5 into vimeo:master Jan 16, 2022
@ricardoboss ricardoboss deleted the issue-6966-argumenttypecoercion_for_typecasted_vars branch January 16, 2022 20:34
@kkmuffme
Copy link
Copy Markdown
Contributor

kkmuffme commented Sep 9, 2022 •
edited
Loading

The missing cast analyzer things are added in this PR now #8366 and this PR is cherry-picked to 4.x

kkmuffme pushed a commit to kkmuffme/psalm that referenced this pull request Sep 11, 2022
@ricardoboss @kkmuffme
Cherry-pick: Try to provide literal int types when possible (fixes vi…
b643855 
…meo#6966) (vimeo#7071)

* Fixed vimeo#6966

* Only accept >= 0 values for mode argument in round()

* Made round() only return float or literal float values and remove unneeded test

* Registered RoundReturnTypeProvider

* Updated cast analyzer to handle single string literal int values as literal ints

* Fixed psalm errors

* Fix invalid property accesses

* Addressed comments

* Added Tests

* Marked RoundReturnTypeProvider as internal

* Fixed CS
kkmuffme pushed a commit to kkmuffme/psalm that referenced this pull request Sep 11, 2022
@ricardoboss @kkmuffme
Cherry-pick: Try to provide literal int types when possible (fixes vi…
893a6f8 
…meo#6966) (vimeo#7071)

* Fixed vimeo#6966

* Only accept >= 0 values for mode argument in round()

* Made round() only return float or literal float values and remove unneeded test

* Registered RoundReturnTypeProvider

* Updated cast analyzer to handle single string literal int values as literal ints

* Fixed psalm errors

* Fix invalid property accesses

* Addressed comments

* Added Tests

* Marked RoundReturnTypeProvider as internal

* Fixed CS
kkmuffme pushed a commit to kkmuffme/psalm that referenced this pull request Sep 11, 2022
@ricardoboss @kkmuffme
Cherry-pick: Try to provide literal int types when possible (fixes vi…
aebe936 
…meo#6966) (vimeo#7071)

* Fixed vimeo#6966

* Only accept >= 0 values for mode argument in round()

* Made round() only return float or literal float values and remove unneeded test

* Registered RoundReturnTypeProvider

* Updated cast analyzer to handle single string literal int values as literal ints

* Fixed psalm errors

* Fix invalid property accesses

* Addressed comments

* Added Tests

* Marked RoundReturnTypeProvider as internal

* Fixed CS
kkmuffme pushed a commit to kkmuffme/psalm that referenced this pull request Sep 15, 2022
@ricardoboss @kkmuffme
Cherry-pick: Try to provide literal int types when possible (fixes vi…
b095f44 
…meo#6966) (vimeo#7071)

* Fixed vimeo#6966

* Only accept >= 0 values for mode argument in round()

* Made round() only return float or literal float values and remove unneeded test

* Registered RoundReturnTypeProvider

* Updated cast analyzer to handle single string literal int values as literal ints

* Fixed psalm errors

* Fix invalid property accesses

* Addressed comments

* Added Tests

* Marked RoundReturnTypeProvider as internal

* Fixed CS
kkmuffme pushed a commit to kkmuffme/psalm that referenced this pull request Sep 15, 2022
@ricardoboss @kkmuffme
Cherry-pick: Try to provide literal int types when possible (fixes vi…
bdb66e9 
…meo#6966) (vimeo#7071)

* Fixed vimeo#6966

* Only accept >= 0 values for mode argument in round()

* Made round() only return float or literal float values and remove unneeded test

* Registered RoundReturnTypeProvider

* Updated cast analyzer to handle single string literal int values as literal ints

* Fixed psalm errors

* Fix invalid property accesses

* Addressed comments

* Added Tests

* Marked RoundReturnTypeProvider as internal

* Fixed CS
kkmuffme pushed a commit to kkmuffme/psalm that referenced this pull request Sep 15, 2022
@ricardoboss @kkmuffme
Cherry-pick: Try to provide literal int types when possible (fixes vi…
5ab90d4 
…meo#6966) (vimeo#7071)

* Fixed vimeo#6966

* Only accept >= 0 values for mode argument in round()

* Made round() only return float or literal float values and remove unneeded test

* Registered RoundReturnTypeProvider

* Updated cast analyzer to handle single string literal int values as literal ints

* Fixed psalm errors

* Fix invalid property accesses

* Addressed comments

* Added Tests

* Marked RoundReturnTypeProvider as internal

* Fixed CS
kkmuffme pushed a commit to kkmuffme/psalm that referenced this pull request Sep 19, 2022
@ricardoboss @kkmuffme
Cherry-pick: Try to provide literal int types when possible (fixes vi…
1afc543 
…meo#6966) (vimeo#7071)

* Fixed vimeo#6966

* Only accept >= 0 values for mode argument in round()

* Made round() only return float or literal float values and remove unneeded test

* Registered RoundReturnTypeProvider

* Updated cast analyzer to handle single string literal int values as literal ints

* Fixed psalm errors

* Fix invalid property accesses

* Addressed comments

* Added Tests

* Marked RoundReturnTypeProvider as internal

* Fixed CS
kkmuffme pushed a commit to kkmuffme/psalm that referenced this pull request Sep 19, 2022
@ricardoboss @kkmuffme
Cherry-pick: Try to provide literal int types when possible (fixes vi…
efab90e 
…meo#6966) (vimeo#7071)

* Fixed vimeo#6966

* Only accept >= 0 values for mode argument in round()

* Made round() only return float or literal float values and remove unneeded test

* Registered RoundReturnTypeProvider

* Updated cast analyzer to handle single string literal int values as literal ints

* Fixed psalm errors

* Fix invalid property accesses

* Addressed comments

* Added Tests

* Marked RoundReturnTypeProvider as internal

* Fixed CS
kkmuffme pushed a commit to kkmuffme/psalm that referenced this pull request Sep 19, 2022
@ricardoboss @kkmuffme
Cherry-pick: Try to provide literal int types when possible (fixes vi…
bf1c032 
…meo#6966) (vimeo#7071)

* Fixed vimeo#6966

* Only accept >= 0 values for mode argument in round()

* Made round() only return float or literal float values and remove unneeded test

* Registered RoundReturnTypeProvider

* Updated cast analyzer to handle single string literal int values as literal ints

* Fixed psalm errors

* Fix invalid property accesses

* Addressed comments

* Added Tests

* Marked RoundReturnTypeProvider as internal

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

Reviewers

@orklah orklah orklah approved these changes

Assignees

No one assigned

Labels

release:feature The PR will be included in 'Features' section of the release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ArgumentTypeCoercion for typecasted variables

4 participants

@ricardoboss @orklah @weirdan @kkmuffme