Fix mis-identification of 'readonly' keyword

[fredden]

The test suite for this package is failing on PHP 8.2. This seems to be due to a mis-identification of the readonly keyword when used as a function name. For example, https://github.com/squizlabs/PHP_CodeSniffer/actions/runs/4318800044/jobs/7537540753.

This pull request fixes that test failure.

I have tested this with PHP version 8.2.3. I also noticed that the test case "testReadonlyUsedAsFunctionCallWithSpaceBetweenKeywordAndParens" seems to have been in the wrong category within the test suite.

[fredden]

Using the following test code on https://onlinephp.io/

<?php

function readonly() {
	print "yes\n";
}

readonly();
readonly ();
readonly /* comment */ ();

I get expected results on PHP versions 8.2.3, 8.0.28, 7.4.33, 7.3.33, 7.2.34, 7.1.33, 7.0.33, 5.6.40, 5.5.38, 5.4.45, 5.3.29, 5.2.17, 5.1.6, 5.0.5, 4.4.9, 4.3.11, 4.2.3, 4.1.2, 4.0.6

However, all versions of PHP 8.1.x produce a parse error on the last line with this code sample.

Do we need to add any special handling for PHP 8.1.x to this readonly detection?

[fredden]

@jrfnl as far as I know, this change should make all the automated tests pass. I expect that may help with the triage process for other pull requests. I don't know where this pull request fits in the list of what to review. I'm highlighting this to hopefully help, but please feel free to ignore this comment.

[jrfnl]

@fredden and me looked at this PR in detail together and this needs more work.

Findings:

• The current fix will break on PHP 8.2 DNF.
• More tests needs to be added.

Basically, we need to make sure that all tests which are included in this PHP Core commit are also included in the PHPCS Tokenizer BackfillReadonlyTest file and that those tests pass correctly: php/php-src@08b7539

Also, in the original implementation of the backfill, the situation where a comment would be between the readonly and the ( for function declarations/calls was not taken into account.
This is not handled in PHP Core, however, the PHPCS token stream should still be consistent for this as it will otherwise break sniffs which specifically look for the T_READONLY token.

As for DNF: that doesn't need to be handled in this PR, but at the very least, the readonly keyword tokenization should not break on it, while it currently would.

Related: in #3667 there are some comments/analysis about the change in the readonly tokenization in PHP Core and how it relates to DNF.

I look forward to a next iteration on this PR.

[jrfnl]

For reference - these are some older PRs related to the tokenization backfill for readonly:

• PHP 8.1: Added support for readonly keyword #3480
• PHP 8.1 | Tokenizer/PHP: readonly bug fix #3507
• PHP 8.1: Resolve readonly earlier to fix some bugs #3584
• And loosely related: Improved tokenizing of context sensitive keywords #3484

[fredden]

@jrfnl is there anything I can do to help progress this?

[jrfnl]

An adjusted version of this PR has been merged in PHPCSStandards/PHP_CodeSniffer#34

[fredden]

Superseded by PHPCSStandards/PHP_CodeSniffer#34

[jrfnl]

FYI: this fix is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).