Skip to content

MutatingScope: implemented support for BitwiseNot operator #249

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

MutatingScope: implemented support for BitwiseNot operator #249

wants to merge 2 commits into from

Conversation

dktapps
Copy link
Contributor

@dktapps dktapps commented Jun 18, 2020

No description provided.

ondrejmirtes
ondrejmirtes requested changes Jun 18, 2020
View reviewed changes
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.

Best way to test this is to create a new dataProvider for NodeScopeResolverTest::testFileAsserts.
src/Analyser/MutatingScope.php Outdated
Copy link
Member

Choose a reason for hiding this comment

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

This if isn't necessary
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed it was since other similar operators need it as well. I'll get rid of it.
src/Analyser/MutatingScope.php Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Not covered cases:

  • floats
  • unions of constant strings

Best way to tackle this is to use TypeTraverse::map() which takes care of complex types easily...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Floats get coerced to integer for bitwise not. I hadn't considered the union types problem.
Copy link
Member

Choose a reason for hiding this comment

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

Oh right, floats work (but a unit test would be nice).
src/Rules/Operators/InvalidUnaryOperationRule.php Outdated
Copy link
Member

Choose a reason for hiding this comment

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

The changed rule should be in separate commit along with the related test changes.
@ondrejmirtes
Copy link
Member

Merged as 24246f3, thank you.
@ondrejmirtes ondrejmirtes mentioned this pull request Jun 20, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@dktapps @ondrejmirtes