ProperEscapingFunction: Fix short tag detection

[GaryJones]

Suggested fix for #739. Includes unit tests. First commit is separate just to keep the noise down for the actual fix in the second commit.

Applying the suggested fix (reset tracking variable to false in process_token() method) from #739 caused a failure of an existing unit test - presumably because this meant it would get reset when processing a T_STRING token, and not just a T_OPEN_TAG_WITH_ECHO token (both are returned from register()), leading to a case where the tracking variable was incorrect.

I've also made an assumption, that numbered .inc files are always processed in the same logical order (1, then 2, then 3). I tested alternative versions, and could successfully NOT trigger the bug when the new unit test files were numbered in the reverse order (and 1.inc was temporarily moved to 4.inc, as that seemed to trigger the bug as well coincidentally).

ProperEscapingFunction: Prep for multi-file tests

Renames the singular test .inc file to include a number, to allow for more incoming test files.

ProperEscapingFunction: Fix short tag detection

The tracking variable $in_short_echo was never reset when checking different files, meaning that the property would carry over and provide the wrong context to the next file.

By adding a process() method to the ProperEscapingFunctionSniff, we can reset the tracking variable at the start of each file by comparing the currently processing file to the last one stored in a static variable.

Includes two unit test files, numbered in the order needed to trigger the bug if the fix wasn't present.

Fixes #739.

[jrfnl]

@GaryJones Thanks for getting this set-up.

Applying the suggested fix (reset tracking variable to false in process_token() method) from #739 caused a failure of an existing unit test - presumably because this meant it would get reset when processing a T_STRING token, and not just a T_OPEN_TAG_WITH_ECHO token (both are returned from register()), leading to a case where the tracking variable was incorrect.

IMO, we don't actually need the process() method, this snippet can just as easily be added at the top of the process_token() method.
The parent process() method only assigns the $phpcsFile and $tokens properties, it doesn't do anything else.

I have a feeling that if you apply the two suggested fixes (inline comments), the failure of the existing tests will disappear.
I suspect that failure was due to the object comparison instead of comparing the file name.

I've also made an assumption, that numbered .inc files are always processed in the same logical order (1, then 2, then 3)

That's a correct assumption as long as the number of test files stays below 10.
See:

• https://github.com/squizlabs/PHP_CodeSniffer/blob/add95a74551c3ba8fc99ef7651ad05f553b3fbbf/tests/Standards/AbstractSniffUnitTest.php#L91 <= This should probably be changed to sort() with SORT_NATURAL | SORT_FLAG_CASE (both available since PHP 5.4).
• https://3v4l.org/VPO3Z

[jrfnl]

I'd suggest assigning the $current_file to a private property in the sniff (with a default of an empty string), rather than using a static variable.

[GaryJones]

For what reason?

[GaryJones]

Handled in f1cd93c (#748).

[jrfnl]

Reason: consistency (the $in_short_echo tracker is also a property) and reducing "magic".

[GaryJones]

IMO, we don't actually need the process() method, this snippet can just as easily be added at the top of the process_token() method.
The parent process() method only assigns the $phpcsFile and $tokens properties, it doesn't do anything else.

@jrfnl I was under the impression, probably wrongly, that process() ran once per file, and process_token() ran once per registered token, and therefore less work would be done by adding the reset logic to process().

[jrfnl]

IMO, we don't actually need the process() method, this snippet can just as easily be added at the top of the process_token() method.
The parent process() method only assigns the $phpcsFile and $tokens properties, it doesn't do anything else.

@jrfnl I was under the impression, probably wrongly, that process() ran once per file, and process_token() ran once per registered token, and therefore less work would be done by adding the reset logic to process().

It's the register() method which is only called once per sniff.
The process() method is called for each token register-ed to be listened to. Also see: https://github.com/WordPress/WordPress-Coding-Standards/blob/546f59c67854589bb8f6b49a30e642e75ff419ad/WordPress/Sniff.php#L418-L433

[jrfnl]

Follow up on my earlier remark about the test case file sorting: squizlabs/PHP_CodeSniffer#3775