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 / http_build_query() #920

Merged

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Oct 28, 2021

This fixes an issue with a call 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, this function call 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 nearly all deprecation warnings found when running the tests.

Refs:

@jrfnl jrfnl mentioned this pull request Oct 28, 2021
@ruudk
Copy link

ruudk commented Nov 29, 2021

@jrfnl Could you maybe rebase this PR to trigger a new CI run? It seems that the tests are stuck. Would be great to have this merged :)

@jrfnl
Copy link
Contributor Author

jrfnl commented Nov 29, 2021

@ruudk The branch is already at current master, so there's nothing to rebase. I think one of the maintainers needs to give permission for the actions to be run or press the re-run button.

@ruudk
Copy link

ruudk commented Nov 29, 2021

Ah, instead of rebasing you can also edit your last commit and force push it, to trigger a new run.

The UI doesn't mention "Waiting for approval to run workflow" so it's not an approval thing.

This fixes an issue with a call 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, this function call 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 nearly all 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-call-to-http_build-query branch from 1fa0d27 to 2b38cfd Compare November 29, 2021 11:21
@jrfnl
Copy link
Contributor Author

jrfnl commented Nov 29, 2021

Ah, instead of rebasing you can also edit your last commit and force push it, to trigger a new run.

The UI doesn't mention "Waiting for approval to run workflow" so it's not an approval thing.

Actually it does and the previous push doesn't show in the "Actions" list at all as having been approved/attempted to run before. Either way, pushed again, so let's wait for the maintainers to start looking at all six of my PRs.

@ruudk
Copy link

ruudk commented Nov 29, 2021

Weird, now I see the workflow awaiting approval but I didn't before. Let's wait then :)

@codecov
Copy link

codecov bot commented Nov 29, 2021

Codecov Report

Merging #920 (3b14025) into master (a8eca3f) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##              master      #920   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       180       180           
===========================================
  Files             20        20           
  Lines            464       464           
===========================================
  Hits             464       464           
Impacted Files Coverage Δ
src/Tool/QueryBuilderTrait.php 100.00% <100.00%> (ø)

@ramsey
Copy link
Contributor

ramsey commented Nov 30, 2021

Thanks, @jrfnl!

@ramsey ramsey merged commit 535de5b into thephpleague:master Nov 30, 2021
@jrfnl jrfnl deleted the feature/php-8.1-fix-call-to-http_build-query branch November 30, 2021 08:00
@lyrixx
Copy link

lyrixx commented Dec 6, 2021

Hello, Could you release a new version with this patch please? It triggers deprecation notice on another projet. Thanks

@Slamdunk
Copy link

It seems a release is really neaded to use this fix: no dev alias is available to match league/oauth2-client:^2.0 contraints on downstream projects.

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.

5 participants