Support generics in docblock

[tm1000]

This supports the following:

    /**
	 * @param Collection<int, TelephoneNumber> $telephoneNumbers
	 */
	public function myFunction(
		Collection $telephoneNumbers,

Where-as before it would say that the typehint needs to be Collection<int, TelephoneNumber> now it will correctly deduce that the type hint needs to be Collection and will not return an error

I based this off of #708 which is similar in scope.

Unfortunately I was unable to run the unit tests because I'm on PHP 8.1 and something in the code continues to try to use the function each which does not exist in php 8.0

Edit: There is now a test!

[jrfnl]

Unfortunately I was unable to run the unit tests because I'm on PHP 8.1 and something in the code continues to try to use the function each which does not exist in php 8.0

Please add tests for this feature.

To run the tests locally on PHP 8.1:

• Run composer install --ignore-platform-reqs to get PHPUnit 7.5.
• Then run vendor/bin/phpunit tests/AllTests.php --no-configuration --bootstrap=tests/bootstrap.php --dont-report-useless-tests

See:

PHP_CodeSniffer/.github/workflows/test.yml

Lines 121 to 142 in 3e6ce10

	# For PHP 8.0+, we need to install with ignore platform reqs as PHPUnit 7 is still used.
	- name: Install Composer dependencies - with ignore platform
	if: ${{ matrix.php >= '8.0' }}
	uses: "ramsey/composer-install@v1"
	with:
	composer-options: --ignore-platform-reqs
	# Note: The code style check is run multiple times against every PHP version
	# as it also acts as an integration test.
	- name: 'PHPCS: set the path to PHP'
	run: php bin/phpcs --config-set php_path php
	- name: 'PHPUnit: run the tests'
	if: ${{ matrix.php != '8.1' && matrix.php != '8.2' }}
	run: vendor/bin/phpunit tests/AllTests.php
	# We need to ignore the config file so that PHPUnit doesn't try to read it.
	# The config file causes an error on PHP 8.1+ with PHPunit 7, but it's not needed here anyway
	# as we can pass all required settings in the phpunit command.
	- name: 'PHPUnit: run the tests on PHP > 8.0'
	if: ${{ matrix.php == '8.1' || matrix.php == '8.2' }}
	run: vendor/bin/phpunit tests/AllTests.php --no-configuration --bootstrap=tests/bootstrap.php --dont-report-useless-tests

[tm1000]

@jrfnl got it to work! I had to delete the composer.lock file and vendor dir then run with --ignore-platform-reqs

I deleted my other comment because I was stuck on phpunit 4 because I didnt run with --ignore-platform-reqs

Thanks for pointing that out. much appreciated. There is now a test!

[jrfnl]

Looks good, just two small notes.

[jrfnl]

1. For performance optimization, I'd recommend having this condition as the last condition in the if/elseif chain, not the first.
2. I'd also suggest optimizing the regex a little more, like so:
   Suggested change

   	if (preg_match('/^(.*)<.*>$/', $suggestedName, $matches)) {
   	if (preg_match('/^([^<]+)<[^>]+>$/', $suggestedName, $matches)) {

[jrfnl]

Missing new line at end of file (also for fixed).

[tm1000]

@jrfnl Did as you asked however I could not make the if/then last. I was able to make it second to last. The reasoning is because the last if/then is a "if not in list then do this". The suggestedName before the expression will never be in that array, thus the last if/then would always match

[jrfnl]

Linking issue #3589 for visibility.

[tm1000]

@jrfnl @gsherwood Now that its been about 9 months what is the process to get this merged?

[jrfnl]

@tm1000 Probably not what you want to hear, but patiently wait until Greg has some time to review this. I can review (which I did), but I don't have commit, so there's nothing more I can do.

[WillemHoman]

@jrfnl would love to see this merged, is that going to be possible?

[tm1000]

Ref: PHPCSStandards/PHP_CodeSniffer#750