WordPress-Coding-Standards: `WordPress.Arrays.ArrayDeclarationSpacing` fails on some files (fixer conflict)

While running this rule on Core, it causes some files to exceed the pass limit. It seems to be getting stuck in a loop of changing how lines are indented, as each rule violates some other rule.

For example, src/wp-content/themes/twentyeleven/404.php will get stuck.

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Comments: 22 (21 by maintainers)

Most upvoted comments

The thing is, if exclusions are causing the conflicts in fixers, then we should address that problem and not try to work around it.

That means in practice any of the following solutions:

  • Remove exclusions & live with the consequences
  • If possible/relevant, pull fixes upstream if that would ease the pain (though that’s awkward now with PHPCS 2.x vs 3.x)
  • Or find or write a better sniff for the sniff (or parts thereof) where we needed the exclusions

Adjusting an unrelated sniff to work round exclusions made to another sniff feels like the wrong way to go.

I don’t think we’d want there to be warnings or errors ever to surface due to these sniffs, since this coding style is pervasive. For example: https://codex.wordpress.org/Function_Reference/add_shortcode

So excluding them and removing them from the fixer seems like the best way to go in the short term, if I understand the options sufficiently.

If these exclusions are removed, would it not cause these very common core WP patterns to be marked as invalid?

	$atts = shortcode_atts( array(
		'order'      => 'ASC',
		'orderby'    => 'menu_order ID',
		'id'         => $post ? $post->ID : 0,
		'itemtag'    => $html5 ? 'figure'     : 'dl',
		'icontag'    => $html5 ? 'div'        : 'dt',
		'captiontag' => $html5 ? 'figcaption' : 'dd',
		'columns'    => 3,
		'size'       => 'thumbnail',
		'include'    => '',
		'exclude'    => '',
		'link'       => ''
	), $attr, 'gallery' );

and

	$link = sprintf( '<a href="%1$s" title="%2$s" rel="author">%3$s</a>',
		esc_url( get_author_posts_url( $authordata->ID, $authordata->user_nicename ) ),
		/* translators: %s: author's display name */
		esc_attr( sprintf( __( 'Posts by %s' ), get_the_author() ) ),
		get_the_author()
	);

and

		$set = wp_parse_args( $settings, array(
			'wpautop'             => true,
			'media_buttons'       => true,
			// ...
		) );

@JDGrimes @GaryJones Thanks for your input. I’ve got option three working, so I’ll tidy up, run some more tests and will pull it if I don’t encounter any more issues with it.

(though there is an argument that default values should only ever be null, and that logic like default values should be handled inside the function, not as part of its signature).

That… is a completely different and unrelated issue IMO and pretty opinionated at that. Thinking of WP core and the “average” level of abstraction in themes and plugins, I don’t think that a sniff for something like that would be appropriate for WPCS anyhow. At least not for the foreseeable future.

I’ll see if I can get a fix sorted later today.

Minimum reproducible code:

the_widget( 'WP_Widget_Recent_Posts', array( 'number' => 10 ), array( 'widget_id' => '404' ) );

Basically the issue is that there are two associative arrays on the same line.

I think that the sniff that is actually WordPress.Arrays.ArrayDeclarationSpacing that is causing the issue. It is trying to fix the second array to be multiline after fixing the first array, and that is where it fails. Remove the second associative array and the code is fixed without problem.