To Yoda, or not to Yoda, that is the question

[jaymcp]

WPCS are proposing to remove the enforcement of Yoda conditionals in WPCS v3.0.0, potentially with a goal of disallowing Yoda conditionals in WPCS v4.0.0, including changing all occurrences in WP Core.
WordPress/WordPress-Coding-Standards#1624
https://make.wordpress.org/core/2022/06/14/upcoming-disallow-assignments-in-conditions-and-remove-the-yoda-condition-requirement-for-php/

If they go ahead with removing Yoda conditionals, should we follow suit?
Further, should we simply remove the requirement, or enforce non-yoda?

Edit (clarification): If we did make this change, we would release a new major version of our PHPCS config; this would allow existing projects to choose to remain as they are, or to adopt the change at their own pace. Releasing as a new major version will prevent composer/CI from pulling in something which could cause build failures.

[PaulREnglish]

I find both Yoda and non-Yoda easy to read but I would definitely rather we go with one or the other. I definitely do not want violence 😅

[ampersarnie]

I've never been a fan of Yoda-conditionals and don't think I've ever hid that fact. Though it's not the end of the world it almost feels non-standard in comparison to projects outside of WordPress, and not to mention as a dyslexic it's annoying to read.

And I know it's not part of the question but thanks for putting this down as a breaking change, that will prevent a lot of headaches.

[jaymcp]

thanks for putting this down as a breaking change

you're welcome 😊 to do otherwise would be the absolute worst. it would cause so many issues on so many projects. it's not worth it.

[jasonagnew]

I choose violence 🤣 On serious note I think we should remove Yoga and enforce non-yoga going forward. I would be happy allowing both but its best being strict and if theres issue at project level they can remove the rule.

[jrmd]

I personally don't like yoda conditionals, i never have. They're a PITA and non standard from the rest of the PHP world. I find my self frequently having to still switch them round as logically in my mind i am checking if the variable is equal to a value not the other way around.

Plus it would bring more consistency between JS and PHP

[jaymcp]

consistency between JS and PHP

I'm going to avoid heading down that rabbit hole in this discussion 😝

[jrmd]

@jonmcp yep, if you force spaces around parenthesis in js i'll be big sad

[g-elwell]

I chose option 4, which avoids introducing a breaking change and protects against any actual issues. Beyond that it's a minor formatting preference IMO and not worth enforcing.

I'd change my mind if it was an a11y issue but haven't seen any non-anecdotal evidence of this.

Either way, I'd say the direction WordPress take is going to have the biggest impact on what we do in turn since it will probably make the most sense to follow suit.

[jaymcp]

avoids introducing a breaking change

i feel i should clarify. we would do this in a way that avoids impacting existing projects by introducing the change in a new major version. this would prevent composer from auto-updating to the version that introduces the change. existing projects could opt into the new version, if they so chose, but it would be a choice.

[g-elwell]

Apologies if I'm missing something... I get the need for a major version update if we're enforcing non-Yoda conditionals, but my understanding is that option 4 would replace the Yoda sniff with alternatives that look for the actual problems that Yoda prevents you from hitting. So a codebase using Yoda conditionals wouldn't fail any of the new sniffs and could be updated without issue?

[jaymcp]

Ah! I should apologise for my lack of clarity, even after trying to clarify!

Our ruleset already checks for the issues that this rule is supposed to prevent. We disallow:

• assignment in conditionals
• loose comparisons
• loose mode for in_array
• some stylistic preferences

So this rule doesn't actually do anything useful other than enforce a style convention.
Removing it or replacing it will only really impact the ordering of the comparisons themselves.

[g-elwell]

Gotcha, thanks! I think this lines up with my reasoning, since with those rules already in place the only possible codebase errors would be if we began enforcing non-yoda style. If we're simply removing the current yoda sniff and allowing both variants to be accepted, there's no breaking change or major version update required. That's all I meant by 'avoids introducing a breaking change'

[jaymcp]

I think I understand now. Thanks 🙂
Yeah, so if we went just with removal, we would release a minor version; if we go with enforcing non-yoda, we'll release a new major version. Either way, we'll ensure that there's the absolute minimum of friction for existing projects.
If we go with the latter option, I'll make sure to write out some migration instructions to make the adoption process as straightforward as possible (we can automate a lot of it).

[jaymcp]

Update: core decided to keep Yoda, primarily to avoid having to update thousands of patches, as far as I can tell.

Nevertheless, since it seems that folks don't like Yoda conditionals for various reasons, so just because core have kept them, it doesn't mean we have to.

React 👍 to this comment if we should remove Yoda.
React 👎 to this comment if we should retain Yoda.

Please re-familiarise yourself with the discussions prior to voting.