Php_codesniffer: Add new sniff for inline ternary alignment

Created on 20 May 2016  路  10Comments  路  Source: squizlabs/PHP_CodeSniffer

I recently wrote a custom sniff for inline ternary alignment.

Here's my current test file, which gives an idea of what passes, although I'd probably want to make some of these things configurable if it were to be upstreamed.

Just gauging interest in this sniff, since I've already written the sniff (with autofix) and tests... but would need to work on it further to have it be configurable and ready for public consumption, etc.

If not, not worries, it works great for me, but would love to share if it would benefit others. :)

Enhancement

Most helpful comment

It's been a while, so I'm going to close this. It's best to create a PR if you want to suggest a new sniff be merged into the core. I'd be happy to review it.

All 10 comments

Can you please show sniff code itself? From examples I don't understand what sniff really wants ternary if statements to look like.

Sure, here's the sniff code in its current state.

https://gist.github.com/andyroo2000/e7d5e366665ee07e6722ef1a7ad0feec

The sniff looks like very specific one to me. I haven't seen any standard, where multi-line formatting for ternary IF statements would be enforced.

No prob... Just wanted to make sure. :)

It was just mine opinion. Not sure if it matches with @gsherwood 's one who does PR merging. Maybe you've closed issue too early.

Ah, thanks for letting me know. I'll reopen it and let other people chime in.

I think that a sniff that checks if the ternary operators are aligned sounds ok. I'm not sure about putting the requirement to parenthesis the statement into the same sniff though.

The heavy use of private methods is also not something I'd recommend. They don't make this sniff easier to customise (they're private methods after all) so they only add to the number of function calls during a run, which has a bigger impact on performance than you'd think. So I'd remove all those and just duplicate the logic (it's only tiny anyway).

If you feel that you'd want to change the sniff in those ways, I'd be happy to review a PR for it.

@gsherwood
Nice! I'll work on making the changes.

Will have to modify the logic a little bit if non-wrapped ternary statements are allowed, but it's definitely doable. Thanks for the suggestions on improving the performance... noted. :)

@aik099 it appears that the Pear standard does require alignment of multi-line ternary operators

It's been a while, so I'm going to close this. It's best to create a PR if you want to suggest a new sniff be merged into the core. I'd be happy to review it.

Was this page helpful?
0 / 5 - 0 ratings