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

Chore/phpstan upgrade #687

Merged
merged 9 commits into from Jun 4, 2019

Conversation

BackEndTea
Copy link
Member

@BackEndTea BackEndTea commented May 1, 2019

This PR:

  • Upgrades PHPStan to 0.11.5
  • Fixes new errors found by PHPStan
  • Removes the two rules about the Node class missing properties or methods

Blocked by #685 Since that fixes issues surrounding names on nodes for decrement integer

That last one is the biggest change here. In most cases it means updating the doc comment to accurately reflect what is actually going into the method.
In some other cases it means assigning a property to a value, and then telling PHPStan what it is through /** @var */ notation.

The big upside of no longer ignoring these issues is that we now are notified of errors with $node->name during development. Like the ones found in:

What do you think, should we stop ignoring these errors, and have a bit more code, like in this PR?
Or should we ignore the errors like previously?
Or should we add rules for specific files, stating for exactly which files these rules can be ignored?

Copy link

@tests-checker tests-checker bot left a comment

Choose a reason for hiding this comment

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

Thank you for contributing to Infection. We noticed you didn't add any tests. Could you please add them to make sure everything works as expected?

maks-rafalko
maks-rafalko previously approved these changes May 2, 2019
Copy link
Member

@maks-rafalko maks-rafalko left a comment

Choose a reason for hiding this comment

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

👍

What do you think, should we stop ignoring these errors, and have a bit more code, like in this PR?

I'm for it.

devTools/phpstan-src.neon Show resolved Hide resolved
@maks-rafalko
Copy link
Member

Blocked by #685 Since that fixes issues surrounding names on nodes for decrement integer

#685 has been merged

maks-rafalko
maks-rafalko previously approved these changes May 12, 2019
maks-rafalko
maks-rafalko previously approved these changes May 14, 2019
Copy link
Member

@maks-rafalko maks-rafalko left a comment

Choose a reason for hiding this comment

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

👍

@sanmai sanmai self-requested a review May 15, 2019 00:15
Copy link
Member

@sanmai sanmai left a comment

Choose a reason for hiding this comment

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

One of the ParallelProcessRunner issues may be worth fixing.

@BackEndTea
Copy link
Member Author

I fixed the issue in the ParallelProcessRunner, and updated the ignore rule accordingly

@maks-rafalko maks-rafalko added this to the 0.14.0 milestone Jun 4, 2019
Copy link
Member

@maks-rafalko maks-rafalko left a comment

Choose a reason for hiding this comment

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

Thank you @BackEndTea

@maks-rafalko maks-rafalko merged commit ab19cd9 into infection:master Jun 4, 2019
@BackEndTea BackEndTea deleted the chore/phpstan-upgrade branch July 3, 2019 18:34
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 this pull request may close these issues.

None yet

3 participants