Generic/ByteOrderMark: small performance improvement

[rodrigoprimo]

Description

The BOM character must be the first character of the file. This means that this sniff only needs to check the file once, but its code was executed for each occurrence of the T_INLINE_HTML token.

As discussed in #278 (comment), this PR changes the sniff code to return the number of tokens in all of its exit points to ensure that PHPCS executes it just a single time per file.

I noticed that some sniffs return $phpcsFile->numTokens while others return ($phpcsFile->numTokens + 1) to ignore the rest of the file. As far as I could check, returning the number of tokens is enough as the code checks if the returned value is greater than the current position in the token array, so I went with this option. It is not clear to me why some sniffs return the number of tokens plus one. Highlighting this in case I'm missing something, and the code in this PR should return ($phpcsFile->numTokens + 1).

Suggested changelog entry

Small performance improvement for the Generic.Files.ByteOrderMark sniff

Related issues/external references

Discussed in #278 (comment)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
• Documentation improvement

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.
• [Required for new sniffs] I have added XML documentation for the sniff.

[fredden]

It is not clear to me why some sniffs return the number of tokens plus one.

See the documentation of the method here:

PHP_CodeSniffer/src/Sniffs/Sniff.php

Lines 74 to 75 in 1d256d1

	* pointer is reached. Return (count($tokens) + 1) to skip
	* the rest of the file.

[jrfnl]

Thanks @rodrigoprimo ! LGTM.

[jrfnl]

I noticed that some sniffs return $phpcsFile->numTokens while others return ($phpcsFile->numTokens + 1) to ignore the rest of the file. As far as I could check, returning the number of tokens is enough as the code checks if the returned value is greater than the current position in the token array, so I went with this option. It is not clear to me why some sniffs return the number of tokens plus one. Highlighting this in case I'm missing something, and the code in this PR should return ($phpcsFile->numTokens + 1).

@rodrigoprimo You are right, returning $phpcsFile->numTokens should be sufficient.

See the documentation of the method here:

PHP_CodeSniffer/src/Sniffs/Sniff.php

Lines 74 to 75 in 1d256d1

	* pointer is reached. Return (count($tokens) + 1) to skip
	* the rest of the file.

@fredden Good catch on the docs being outdated and not giving the best of advise.

I have created PR #363 now to update the docs and to make the sniffs skipping to the end of a file consistent.

For the record for anyone else coming across this discussion:
The tokens stack indexes are 0-based and a token count is 1-based, so it is sufficient to return the token count to skip the rest of the file.

Or in practical terms:
If a file contains 50 tokens, the last $stackPtr / index in the $tokens array will be 49, so there is no need to change the 50 to 51 as 50 is already > 49.