Skip to content

Inconsistent behaviour findPrevious() vs findNext() #8

Open
@jrfnl

Description

@jrfnl

Repost from squizlabs/PHP_CodeSniffer#1319:

I've been tripped up by this a number of times already, so putting this out here for consideration.

If the $start and $end token positions you pass to findPrevious() are the same, the function will work as expected. However, the same can not be said about the findNext() method as in that case it will always return false.

This is caused by the findPrevious() method treating the end position as inclusive for ($i = $start; $i >= $end; $i--), while the findNext() method treats it as exclusive for ($i = $start; $i < $end; $i++).

There are two possible solutions for this:

  • Either check for $start and $end being the same and adding 1 to the $end in that case before entering the loop. This will lead to the least difference with current behaviour. - PR #1320
  • Or make the loop in findNext() inclusive as well. This might lead to more breakage, but would make the functions more consistent in use. - PR #1321

See the original issue for an extensive discussion on the BC breaks involved and possible approaches.

My current line of thinking is on the cautious side: implement option 1 for PHPCS 4.0 and option 2 for PHPCS 5.0.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions