Squiz Arrays.ArrayDeclaration and Strings.ConcatenationSpacing fixers causing parse errors with here/nowdocs

[jrfnl]

Came across these little gems when trying to isolate a fixer conflict elsewhere (WordPress/WordPress-Coding-Standards#986 - still working on that one).

The issue exists in both PHPCS 2.x as well as 3.x when using the Squiz standard.

When given this code:

get_current_screen()->add_help_tab( array(
'id'		=> <<<EOD
Here comes some text.
EOD
. '</hr>',
) );

It will be fixed like this with the .'</hr>', having been moved up causing parse errors (quite apart from the fixed spacing not being correct):

get_current_screen()->add_help_tab(
     array(
                                         'id'        => <<<EOD
Here comes some text.
EOD    .'</hr>',
                                    ) 
    );

And when given this code:

get_current_screen()->add_help_tab( array(
		'id'		=> <<<EOD
Here comes some text.
EOD
,
) );

It will be fixed like this with the comma being added in this bit => ,<<<EOD causing parse errors:

get_current_screen()->add_help_tab(
    [
     'id' => ,<<<EOD
Here comes some text.
EOD
    ,
    ]
);

[jrfnl]

FYI: I've fixed this downstream via a new sniff which solely checks comma's within arrays - and by excluding the comma related checks from the Squiz.Arrays.ArrayDeclaration sniff -.

If you like I could pull that sniff here as well.
I suggest I'd add the new sniff to Generic -, add the new sniff to the Squiz standard and remove the comma related parts of the Squiz.Arrays.ArrayDeclaration sniff.

Related: #582

[gsherwood]

If you like I could pull that sniff here as well.
I suggest I'd add the new sniff to Generic -, add the new sniff to the Squiz standard and remove the comma related parts of the Squiz.Arrays.ArrayDeclaration sniff.

Thanks, but as per my comment on #582, this is not something I'm considering for version 3. Rewriting all the array rules is something I am going to personally take on, but the disruption needs to be left for a major version upgrade and I need to make sure re-parsing the same array multiple times isn't going to be a problem once all the new sniffs need to do it themselves. It is likely that I'll need to add more tokenizer support for arrays, or additional helper functions.

[jrfnl]

Thanks, but as per my comment on #582, this is not something I'm considering for version 3.

I totally get that, though that doesn't take anything away from the fact that I'd be perfectly willing to pull the sniff even if it wouldn't be accepted until PHPCS 4.

It is likely that I'll need to add more tokenizer support for arrays, or additional helper functions.

For WPCS as well as for the PHPCompatibility standards, I've written helper functions for this and they seem to perform quite well. To improve the performance even more, the function results could probably be cached in the File class and reused when the parsing is requested again for the same array, though that would increase the memory usage of the application as a trade off.

Rewriting all the array rules is something I am going to personally take on

I'll respect your decision on that, however, as I've basically already done most of the work for this by now and considering your work load appears to be high enough as it is, would you at least consider the offer of collaboration to ease your burden ?

[gsherwood]

I applied fixes to 2 different sniffs that needed to know about here/nowdocs. Looks to be working now. Thanks for the reports.

First code block is now fixed like this:

<?php
get_current_screen()->add_help_tab(
    [
     'id' => <<<EOD
Here comes some text.
EOD
    .'</hr>',
    ]
);

Second code block is now fixed like this:

<?php
get_current_screen()->add_help_tab(
    [
     'id' => <<<EOD
Here comes some text.
EOD
    ,
    ]
);

[jrfnl]

Looks good! Thanks.

My offer for collaboration on the refactor of the Array sniff for PHPCS 4 still stands. The sniffs I've written now have gone through the wringer, what with them being used on WP Core, so let me know if and when you're interested to discuss/have me pull them here.