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

Faulty core fixes: Nested foreach incorrectly indented. #1121

Closed
pento opened this issue Sep 4, 2017 · 4 comments
Closed

Faulty core fixes: Nested foreach incorrectly indented. #1121

pento opened this issue Sep 4, 2017 · 4 comments

Comments

@pento
Copy link
Member

pento commented Sep 4, 2017

Given the following example:

<?php

foreach ( $a as $b ) :
?>
<?php
	foreach ( $b as $c ) :
	endforeach;
endforeach;

The inner foreach is incorrectly losing its indent.

@jrfnl
Copy link
Member

jrfnl commented Sep 19, 2017

@pento This is an awkward one. I'm actually tempted to class it as "garbage in, garbage out"...

Both the scope indent sniff as well as the embedded PHP sniff have some code handling the indents here.
The ones from the embeddedPHP sniff have been disabled in the WP rulesets, but would make no significant difference here anyhow.

The problem is one of reverse-logic.

For scope indents to be determined, something is needed as a starting point. The scope indent sniff uses PHP open tags as such and resets expected indent each time one is encountered.

As the input has the PHP open tag on a line with an incorrect indent, the rest of the code is influenced by that and will be incorrectly indented too.

If the input would have been the below, it would have been fine:

foreach ( $a as $b ) :
	?>
	<?php
	foreach ( $b as $c ) :
	endforeach;
endforeach;

I very much doubt this can be fixed by patching a sniff.

P.S.: I'm presuming the subsequent closing and opening PHP tags are only for the example code ? If not, they should be removed completely.

@pento
Copy link
Member Author

pento commented Oct 6, 2017

Yeah, the example code was the most cut down version I could make.

I don't making this wontfix if it's too hard to address, there are only a handful of instances of endforeach in Core.

@jrfnl
Copy link
Member

jrfnl commented Oct 6, 2017

there are only a handful of instances of endforeach in Core.

It's not necessarily the endforeach which is the problem here, but the incorrectly placed PHP open tag just before it.

I kind of think we'll probably need a handful of manual fixes prior to running the fixer anyway, so I'd suggest these ones qualify for that.

@pento
Copy link
Member Author

pento commented Oct 6, 2017

👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Auto-fix WP Core / trac-41057
  
Won't fix (in WPCS)
Development

No branches or pull requests

3 participants