Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.1: fix deprecation warnings about incorrect default values #10036

Merged
merged 4 commits into from Aug 11, 2021

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Aug 5, 2021

To "ignore" an optional parameter when another optional parameter after it needs to be passed, the default value should be passed, not null.

PHP 8.1/Tests: fix some deprecation warnings

The default value for the preg_split() $limit parameter is -1, not null.

Fixes numerous preg_split(): Passing null to parameter #3 ($limit) of type int is deprecated notices when running the test suite.

Ref: https://www.php.net/manual/en/function.preg-split.php

PHP 8.1/NoProxyPattern: fix deprecation warning

The default value for the preg_split() $limit parameter is -1, not null.

Fixes some preg_split(): Passing null to parameter #3 ($limit) of type int is deprecated notices when running the test suite.

Deprecation triggered by Composer\Test\Util\Http\ProxyManagerTest::testGetProxyForRequest:
preg_split(): Passing null to parameter #3 ($limit) of type int is deprecated

Stack trace:
0 [internal function]: Symfony\Bridge\PhpUnit\DeprecationErrorHandler->handleError(8192, '...', '...', 42)
1 src/Composer/Util/NoProxyPattern.php(42): preg_split('...', '...', NULL, 1)
2 src/Composer/Util/Http/ProxyManager.php(148): Composer\Util\NoProxyPattern->__construct('...')
3 src/Composer/Util/Http/ProxyManager.php(50): Composer\Util\Http\ProxyManager->initProxyData()
4 src/Composer/Util/Http/ProxyManager.php(59): Composer\Util\Http\ProxyManager->__construct()
5 tests/Composer/Test/Util/Http/ProxyManagerTest.php(75): Composer\Util\Http\ProxyManager::getInstance()
...

Ref: https://www.php.net/manual/en/function.preg-split.php

PHP 8.1: fix deprecation warnings / http_build_query()

This fixes all relevant calls to the PHP native http_build_query() function.
The second parameter of which is the optional $numeric_prefix parameter which expects a string.

A parameter being optional, however, does not automatically make it nullable.

As of PHP 8.1, passing null to a non-nullable PHP native function will generate a deprecation notice.
In this case, these function calls yielded a http_build_query(): Passing null to parameter #2 ($numeric_prefix) of type string is deprecated notice.

Changing the null to an empty string fixes this without BC-break.

Fixes a few deprecation warnings found when running the tests.

Refs:

PHP 8.1: fix deprecation notices / PharData::__construct()

This fixes all relevant calls to the PHP native PharData::__construct() method.

The second parameter of this method is the optional $flags parameter which expects an int of flags to be passed to the Phar parent class RecursiveDirectoryIterator.
Fixed by passing the default value for the $flags parameter as per the RecursiveDirectoryIterator::__construct() method.

The third parameter of the method is the optional $alias parameter which expects an string.
Fixed by passing an empty string.

Fixes various notices along the lines of:

Deprecation triggered by Composer\Test\Package\Archiver\ArchiveManagerTest::testArchiveTar:
PharData::__construct(): Passing null to parameter #2 ($flags) of type int is deprecated

Stack trace:
0 [internal function]: Symfony\Bridge\PhpUnit\DeprecationErrorHandler->handleError(8192, '...', '...', 55)
1 src/Composer/Package/Archiver/PharArchiver.php(55): PharData->__construct('...', NULL, NULL, 2)
2 src/Composer/Package/Archiver/ArchiveManager.php(193): Composer\Package\Archiver\PharArchiver->archive('...', '...', '...', Array, false)
3 tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php(65): Composer\Package\Archiver\ArchiveManager->archive(Object(Composer\Package\CompletePackage), '...', '...')
...

Refs:

@jrfnl jrfnl mentioned this pull request Aug 5, 2021
@jrfnl jrfnl changed the title PHP 8.1/Tests: fix bugs in test code PHP 8.1: fix deprecation warnings about incorrect default values Aug 5, 2021
The default value for the `preg_split()` `$limit` parameter is `-1`, not `null`.

Fixes numerous `preg_split(): Passing null to parameter composer#3 ($limit) of type int is deprecated` notices when running the test suite.

Ref: https://www.php.net/manual/en/function.preg-split.php
The default value for the `preg_split()` `$limit` parameter is `-1`, not `null`.

Fixes some `preg_split(): Passing null to parameter composer#3 ($limit) of type int is deprecated` notices when running the test suite.

```
Deprecation triggered by Composer\Test\Util\Http\ProxyManagerTest::testGetProxyForRequest:
preg_split(): Passing null to parameter composer#3 ($limit) of type int is deprecated

Stack trace:
0 [internal function]: Symfony\Bridge\PhpUnit\DeprecationErrorHandler->handleError(8192, '...', '...', 42)
1 src/Composer/Util/NoProxyPattern.php(42): preg_split('...', '...', NULL, 1)
2 src/Composer/Util/Http/ProxyManager.php(148): Composer\Util\NoProxyPattern->__construct('...')
3 src/Composer/Util/Http/ProxyManager.php(50): Composer\Util\Http\ProxyManager->initProxyData()
4 src/Composer/Util/Http/ProxyManager.php(59): Composer\Util\Http\ProxyManager->__construct()
5 tests/Composer/Test/Util/Http/ProxyManagerTest.php(75): Composer\Util\Http\ProxyManager::getInstance()
...
```

Ref: https://www.php.net/manual/en/function.preg-split.php
This fixes all relevant calls to the PHP native `http_build_query()` function.
The second parameter of which is the _optional_ `$numeric_prefix` parameter which expects a `string`.

A parameter being optional, however, does not automatically make it nullable.

As of PHP 8.1, passing `null` to a non-nullable PHP native function will generate a deprecation notice.
In this case, these function calls yielded a `http_build_query(): Passing null to parameter composer#2 ($numeric_prefix) of type string is deprecated` notice.

Changing the `null` to an empty string fixes this without BC-break.

Fixes a few deprecation warnings found when running the tests.

Refs:
* https://www.php.net/manual/en/function.http-build-query.php
* https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg
@jrfnl jrfnl force-pushed the feature/php-8.1-fix-bugs-in-tests branch from e3c633f to d45cc34 Compare August 5, 2021 12:27
@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 5, 2021

I've added a few more commits to this PR which address similar issues (incorrect value passed for skipped optional param) in other places in the code base.
Updated the PR title and description to match.

@jrfnl jrfnl force-pushed the feature/php-8.1-fix-bugs-in-tests branch 2 times, most recently from d2db5b6 to 3bf5833 Compare August 5, 2021 16:54
This fixes all relevant calls to the PHP native `PharData::__construct()` method.

The second parameter of this method is the _optional_ `$flags` parameter which expects an `int` of flags to be passed to the `Phar` parent class `RecursiveDirectoryIterator`.
Fixed by passing the default value for the `$flags` parameter as per the `RecursiveDirectoryIterator::__construct()` method.

The third parameter of the method is the _optional_ `$alias` parameter which expects an `string`.
Fixed by passing an empty string.

Fixes various notices along the lines of:
```
Deprecation triggered by Composer\Test\Package\Archiver\ArchiveManagerTest::testArchiveTar:
PharData::__construct(): Passing null to parameter composer#2 ($flags) of type int is deprecated

Stack trace:
0 [internal function]: Symfony\Bridge\PhpUnit\DeprecationErrorHandler->handleError(8192, '...', '...', 55)
1 src/Composer/Package/Archiver/PharArchiver.php(55): PharData->__construct('...', NULL, NULL, 2)
2 src/Composer/Package/Archiver/ArchiveManager.php(193): Composer\Package\Archiver\PharArchiver->archive('...', '...', '...', Array, false)
3 tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php(65): Composer\Package\Archiver\ArchiveManager->archive(Object(Composer\Package\CompletePackage), '...', '...')
...
```

Refs:
* https://www.php.net/manual/en/phardata.construct.php
* https://www.php.net/manual/en/recursivedirectoryiterator.construct.php
@jrfnl jrfnl force-pushed the feature/php-8.1-fix-bugs-in-tests branch from 3bf5833 to 487ed92 Compare August 5, 2021 17:11
@Seldaek Seldaek merged commit f5a0dfe into composer:master Aug 11, 2021
@Seldaek
Copy link
Member

Seldaek commented Aug 11, 2021

Thanks!

@jrfnl jrfnl deleted the feature/php-8.1-fix-bugs-in-tests branch August 11, 2021 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants