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

fix(YodaStyleFixer): do not touch require(_once), include(_once) and yield from statements #7325

Merged
merged 1 commit into from Sep 26, 2023

Conversation

SpacePossum
Copy link
Contributor

@SpacePossum SpacePossum commented Sep 26, 2023

closes #7280

The removed tests show we would no longer support some very exotic use case of (require|include)(_once)+ statements,
but I think that is no issue compared to the fix this PR offers.

@keradus keradus changed the title fix: Yoda fixer should not touch require(_once), include(_once) and y… fix: Yoda fixer should not touch require(_once), include(_once) and yield from statements Sep 26, 2023
@keradus keradus changed the title fix: Yoda fixer should not touch require(_once), include(_once) and yield from statements fix(yoda_style): should not touch require(_once), include(_once) and yield from statements Sep 26, 2023
@coveralls
Copy link

Coverage Status

coverage: 94.64% (+0.001%) from 94.639% when pulling 0cbd77e on SpacePossum:7280 into 573ab70 on PHP-CS-Fixer:master.

@Wirone Wirone changed the title fix(yoda_style): should not touch require(_once), include(_once) and yield from statements fix(YodaStyleFixer): do not touch require(_once), include(_once) and yield from statements Sep 26, 2023
Copy link
Member

@Wirone Wirone left a comment

Choose a reason for hiding this comment

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

Some people could say that's a breaking change (removed test cases), but I'm OK with this as these scenarios were pretty weird and we should not support them 😆.

👍 from me, but let's wait for @keradus.

@keradus
Copy link
Member

keradus commented Sep 26, 2023

i have some mixed feelings.
the new version, for such removed test scenario, is now producing invalid PHP code. Not a code that is not visually nice (eg wrong whitespaces), but invalid PHP syntax

@SpacePossum
Copy link
Contributor Author

I struggle to see how it produces invalid PHP code,
and, if so, even more problematic would be the test setup would allow the generated result being invalid PHP code.
If it helps I can recover the removed tests as leaving the input as untouched.

@keradus
Copy link
Member

keradus commented Sep 26, 2023

OK, sth wrong on my side - sorry for that.
cannot reproduce the problem i though I see anymore.

@keradus keradus merged commit 0726577 into PHP-CS-Fixer:master Sep 26, 2023
15 checks passed
@SpacePossum SpacePossum deleted the 7280 branch September 27, 2023 04:56
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.

yoda_style does not work with require and ternary
4 participants