Skip to content

[CountOnNullRector] fix Rector applying on properties with phpdocs array#2637

Merged
TomasVotruba merged 1 commit intorectorphp:masterfrom
gnutix:CountOnNullRector/fix-should-not-apply-on-properties-with-phpdocs-array
Jan 11, 2020
Merged

[CountOnNullRector] fix Rector applying on properties with phpdocs array#2637
TomasVotruba merged 1 commit intorectorphp:masterfrom
gnutix:CountOnNullRector/fix-should-not-apply-on-properties-with-phpdocs-array

Conversation

@gnutix
Copy link
Copy Markdown
Contributor

@gnutix gnutix commented Jan 11, 2020

Fixes #2474.

@gnutix gnutix requested a review from TomasVotruba January 11, 2020 00:45
@gnutix gnutix added the bug label Jan 11, 2020
@TomasVotruba
Copy link
Copy Markdown
Member

Test case is needed in this PR so CI can verify it

@gnutix
Copy link
Copy Markdown
Contributor Author

gnutix commented Jan 11, 2020

I couldn't reproduce it within your Fixtures. But I could in my playground repo : gnutix/rector-playground#3 (test is currently failing, but if you merge this in master and restart it, it should pass).

I can't do better than that (tried for hours already), sorry.

@TomasVotruba
Copy link
Copy Markdown
Member

I see.

We should discover the point of origin, since it's probably cause of other related bugs.

@gnutix
Copy link
Copy Markdown
Contributor Author

gnutix commented Jan 11, 2020

@TomasVotruba I checked the history of the condition I removed before doing the change : it's not used anywhere else, and the test case it was added with passes without this condition (so it was not necessary in the first place apparently - or it was not covered by the test... ?).

So I think there is not much risk of consequences to merge this (but as you wrote this workaround code in the first place, you probably have a better understanding of its scope, so it's up to you).

@TomasVotruba
Copy link
Copy Markdown
Member

So I think there is not much risk of consequences to merge this (

I believe your testing, let's give it a try then.

@TomasVotruba TomasVotruba merged commit 57b4359 into rectorphp:master Jan 11, 2020
@TomasVotruba TomasVotruba deleted the CountOnNullRector/fix-should-not-apply-on-properties-with-phpdocs-array branch January 11, 2020 22:20
TomasVotruba added a commit that referenced this pull request Jul 6, 2022
rectorphp/rector-src@53bddfb [DX] make use of TypeCombinator to detect null type (#2637)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[PHP] CountOnNull on properties with PHPDoc shouldn't apply

2 participants