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: how to deal with one-line inline control structures ? #974

Closed
jrfnl opened this Issue Jun 17, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@jrfnl
Contributor

jrfnl commented Jun 17, 2017

Summary:

If an inline control structure (no braces) is a one-liner, it is still a one-liner after the fix, but the closing brace is on the next line (except when the one-liner is followed by a PHP close tag).

Most likely cause: Generic.ControlStructures.InlineControlStructure does not differentiate between one-line vs multi-line inline control structures for the fixer.

Question:

Should we enforce a new line after the opening brace in these cases ?
(If so, this would probably need to be a new WP native sniff.)

Example:

File: ./src/wp-admin/custom-background.php around line 583

Source:

if ( ! current_user_can('edit_theme_options') || ! isset( $_POST['attachment_id'] ) ) exit;

Fixed as:

if ( ! current_user_can( 'edit_theme_options' ) || ! isset( $_POST['attachment_id'] ) ) { exit;
}

Expected fix:

if ( ! current_user_can( 'edit_theme_options' ) || ! isset( $_POST['attachment_id'] ) ) {
	exit;
}
@JDGrimes

This comment has been minimized.

Show comment
Hide comment
@JDGrimes

JDGrimes Jun 17, 2017

Contributor

Should we enforce a new line after the opening brace in these cases ?

Absolutely.

Contributor

JDGrimes commented Jun 17, 2017

Should we enforce a new line after the opening brace in these cases ?

Absolutely.

@jrfnl

This comment has been minimized.

Show comment
Hide comment
@jrfnl

jrfnl Jun 17, 2017

Contributor

In that case: how to deal with existing one-line scoped control structures which currently don't trigger a fixer ?

if ( ! current_user_can('edit_theme_options') || ! isset( $_POST['attachment_id'] ) ) { exit; }

and

<input value="<?php if ( isset( $a ) ) { echo $a; } else { echo 'default'; } ?>" />
Contributor

jrfnl commented Jun 17, 2017

In that case: how to deal with existing one-line scoped control structures which currently don't trigger a fixer ?

if ( ! current_user_can('edit_theme_options') || ! isset( $_POST['attachment_id'] ) ) { exit; }

and

<input value="<?php if ( isset( $a ) ) { echo $a; } else { echo 'default'; } ?>" />
@JDGrimes

This comment has been minimized.

Show comment
Hide comment
@JDGrimes

JDGrimes Jun 17, 2017

Contributor

Hmmm... good question. The first should probably be fixed to be multiline. The latter should probably really be using a ternary operator instead. Not sure if that can be reasonably auto-fixed though.

Contributor

JDGrimes commented Jun 17, 2017

Hmmm... good question. The first should probably be fixed to be multiline. The latter should probably really be using a ternary operator instead. Not sure if that can be reasonably auto-fixed though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment