Skip to content

Comments

Enable PHPStan in tests#765

Merged
jwage merged 4 commits intomasterfrom
phpstan-tests
Dec 5, 2018
Merged

Enable PHPStan in tests#765
jwage merged 4 commits intomasterfrom
phpstan-tests

Conversation

@jwage
Copy link
Member

@jwage jwage commented Nov 5, 2018

Q A
Type improvement
BC Break no

Summary

Also enabling PHPStan in tests/ on L7 + strict.

@jwage jwage changed the title Phpstan tests Enable PHPStan in tests Nov 5, 2018
@jwage jwage requested a review from Majkl578 November 5, 2018 21:17
Copy link
Member

@lcobucci lcobucci left a comment

Choose a reason for hiding this comment

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

Some small things, nice job 👍

@jwage jwage requested a review from lcobucci November 12, 2018 20:13
@jwage jwage requested review from Majkl578 and lcobucci and removed request for Majkl578 and lcobucci December 5, 2018 01:53

/**
* @return Connection|Configuration[]
* @return object[] [Connection, Configuration]
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct? Didn't see this syntax before 😛

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, looks weird, imho has no meaning in PHPStan context.

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 am not familiar with this either. I didn't make the change. It was in the first commits by @Majkl578 bf980f3

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 think it should be this:

@return Connection[]|Configuration[]

Copy link
Contributor

Choose a reason for hiding this comment

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

Afaik it was intended to indicate a tuple consisting of Connection and Configuration, not array of those.

Copy link
Member Author

Choose a reason for hiding this comment

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

phpstan didn't complain about the syntax. Should I change it back?

Copy link
Member

@lcobucci lcobucci left a comment

Choose a reason for hiding this comment

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

Just this tiny thing 👍

@jwage jwage merged commit 5650e82 into master Dec 5, 2018
@jwage jwage deleted the phpstan-tests branch December 5, 2018 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants