FunctionComment sniff only corrects last param in list

[ElvenSpellmaker]

In this file: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php

On lines 388-390:

                if (count($typeNames) > 1) {
                    continue;
                }

This means it skips the PHP version checking below on line 404, and so it incorrectly expects integer|float, rather than int|float on PHP 7.

If you flip these arguments it expects float|int rather than float|integer (which is correct but not consistent.)

[ElvenSpellmaker]

Hi @gsherwood, I noticed you changed this here: 4079d5f

Is there any reason for this check of more than one? It means it only ever corrects the last param.

[gsherwood]

The Squiz standard wants you to use integer for the type in the comment. Usage of int is for the type hint in the function declaration itself.

If you use int|float in the comment, it's going to want you to change it to integer|float. The count on the params is there because the rest of the code in the loop is trying to determine what the type hint should be in the function declaration. If the comment suggests there are multiple allowed types, the sniff doesn't know what type hint to expect, so it skips the check.

[ElvenSpellmaker]

@gsherwood Is there no way to override this for PHP 7 users who want to use the short forms? The Common class uses static methods and seems it's non trivial to try and replace it with a version which uses short names instead.

It seems a little messy to have to copy the whole Sniff and call out to a different suggestType() method as then I have to keep it updated with changes to your class.

(A similar thing seems to happen too for return types).

Is there no way the sniff can suggest either short or long forms (configurable by a parameter maybe?)

[jrfnl]

What you are suggesting, was previously included in PR #2456, but unfortunately that never got merged.

[ElvenSpellmaker]

@jrfnl Yeah I saw that PR, it's a shame it never got merged in, however that PR seems to change a lot in one, which is always harder to get in. =(

I wonder if the changes needed just for this issue can be merged in without changing 299 files in the process (as good as your PR may have been for the overall structure of the project).

[jrfnl]

however that PR seems to change a lot in one, which is always harder to get in. =(

That and how it would be pulled was all discussed beforehand in #2189 and this way was the preferred way.

I wonder if the changes needed just for this issue can be merged in without changing 299 files in the process.

Those changes will now go into PHPCSUtils. I will not pull any of it here again.
At some point in the future you can expect better comment related sniff in PHPCSExtra, but it may be a while before I get to that.