Inconsistent behavior of PHP_CodeSniffer_File::findEndOfStatement

[nkovacs]

PHP_CodeSniffer_File::findEndOfStatement sometimes returns end token, but sometimes it returns the last not empty token before that.

E.g. if $start is new:

• in [new Datetime] it returns ].
• in array(new Datetime) it returnsDatetime.
• in new Datetime; it returns ;.

The difference between array() and [] is probably a mistake, because [] was added later in 4df216f, but I don't understand why there's a difference at all.
The examples above are from a sniff that adds () to new. If findEndOfStatement consistently returned the last non-empty statement (like it does for array(new Datetime)), I could insert the () after that.

[gsherwood]

First, the array() and [] difference is almost certainly a bug. Thanks for letting me know about that.

For the method itself, the goal is to find the end of the statement, which is typically a semicolon. But when you are within a different scope, like an array, there is no semicolon to return as the ending. If it returns the ] or ) then it is returning a token that actually belongs to the level above. So it returns the last token that is within that single statement.

Hope that makes sense.

[gsherwood]

I've fixed the method to ensure the two array formats return the same result.

[nkovacs]

The problem is that here:

foo(new Bar, new Baz);

findEndOfStatement returns ,, not Bar, and Baz. This doesn't make sense.

I had to duplicate $endTokens in my sniff and check if the token returned is one of those, then find the previous non-whitespace token if it is.

I also don't understand why it returns on => and , and :. Those aren't statements, they're expressions, and it looks like findEndOfStatement actually finds the end of an expression, but it's used in a limited way in PHPCS (I guess mostly for skipping over nested expressions), so the inconsistency didn't cause any problems.

My sniff on the other hand finds all object creation expressions, and since they could be in most of the positions that findEndOfStatement deals with, the inconsistent behavior caused problems.

E.g. the first version did this:

foo(new Bar(), new ()Baz);