fix: deprecation notice for ReflectionType::__toString() in php 7.4 #33

TysonAndre · 2019-07-27T17:52:54Z

Use getName instead, in php 7.1+

See https://github.com/php/php-src/blob/PHP-7.4/UPGRADING

Reflection:
. Calls to ReflectionType::__toString() now generate a deprecation notice.
This method has been deprecated in favor of ReflectionNamedType::getName()
in the documentation since PHP 7.1, but did not throw a deprecation notice
for technical reasons.

Fixes #32

codecov · 2019-07-27T17:55:22Z

Codecov Report

Merging #33 into master will increase coverage by 0.26%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master      #33      +/-   ##
============================================
+ Coverage     89.65%   89.91%   +0.26%     
- Complexity       57       58       +1     
============================================
  Files             8        8              
  Lines           116      119       +3     
============================================
+ Hits            104      107       +3     
  Misses           12       12

Impacted Files	Coverage Δ	Complexity Δ
lib/Dispatcher.php	`90.27% <100%> (+0.42%)`	`28 <0> (+1)`	⬆️

felixfbecker · 2019-07-27T20:32:54Z

According to the release notes, it was deprecated in favor of ReflectionNamedType::getName. Any reason we can’t use that?

https://github.com/php/php-src/blob/6f57802c59f0a82bed6a61e4ae956acea7d96dd1/UPGRADING#L389-L392

TysonAndre · 2019-07-27T20:47:07Z

According to the release notes, it was deprecated in favor of ReflectionNamedType::getName. Any reason we can’t use that?

Good point, this was copied from a helper method for a more generic use case - I wasn't really looking at how it was used here.

The null checks aren't needed and adding a leading ? doesn't make sense for this use case, and it looks like this should be creating an object because of the is_object() check.

TysonAndre · 2019-08-04T21:56:05Z

The review comments have been addressed.

Also, what are your thoughts on increasing the minimum version of netresearch/jsonmapper to ^1.5.2 in composer.json? (That includes another fix for a php 7.4 deprecation notice for curly brackets in array access)

The deprecation notice might be printed to stdout, which might affect language servers. The notices can be worked around with a custom error handler.

xPaw · 2019-09-06T18:02:02Z

Can you merge and release this fix please? Along with bumping netresearch/jsonmapper, it makes the language server work on 7.4.

felixfbecker

Sorry for the delay!

felixfbecker · 2019-09-10T03:16:06Z

lib/Dispatcher.php

@@ -124,7 +125,15 @@ public function dispatch($msg)
                        // Does the parameter have a type hint?
                        $param = $parameters[$position];
                        if ($param->hasType()) {
-                            $class = (string)$param->getType();
+                            $param_type = $param->getType();


Suggested change

$param_type = $param->getType();

$paramType = $param->getType();

felixfbecker · 2019-09-10T03:16:14Z

lib/Dispatcher.php

@@ -124,7 +125,15 @@ public function dispatch($msg)
                        // Does the parameter have a type hint?
                        $param = $parameters[$position];
                        if ($param->hasType()) {
-                            $class = (string)$param->getType();
+                            $param_type = $param->getType();
+                            if ($param_type instanceof ReflectionNamedType) {


Suggested change

if ($param_type instanceof ReflectionNamedType) {

if ($paramType instanceof ReflectionNamedType) {

felixfbecker · 2019-09-10T03:16:27Z

lib/Dispatcher.php

+                            if ($param_type instanceof ReflectionNamedType) {
+                                // We have object data to map and want the class name.
+                                // This should not include the `?` if the type was nullable.
+                                $class = $param_type->getName();


Suggested change

$class = $param_type->getName();

$class = $paramType->getName();

felixfbecker · 2019-09-10T03:16:35Z

lib/Dispatcher.php

+                                $class = $param_type->getName();
+                            } else {
+                                // Fallback for php 7.0, which is still supported (and doesn't have nullable).
+                                $class = (string)$param_type;


Suggested change

$class = (string)$param_type;

$class = (string)$paramType;

TysonAndre · 2019-09-12T19:41:12Z

Review comments have been addressed.

The decreased code coverage is expected - one of the branches is only reachable in php 7.1+, the other in php 7.0

felixfbecker · 2019-09-12T21:13:12Z

Could you merge in master? I added 7.2 to the build matrix

Use ReflectionNamedType->getName() for php 7.1+, instead. Continue supporting php 7.0 Fixes felixfbecker#23

TysonAndre · 2019-09-12T22:27:12Z

Could you merge in master? I added 7.2 to the build matrix

Done

felixfbecker · 2019-09-12T22:40:34Z

Awesome, coverage changes went away!

felixfbecker · 2019-09-12T22:44:24Z

🎉 This PR is included in version 3.0.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

TysonAndre force-pushed the fix-tostring-deprecation branch from b2d773b to 802d75a Compare July 27, 2019 17:53

TysonAndre force-pushed the fix-tostring-deprecation branch 2 times, most recently from 1732d5e to be6cbd6 Compare July 27, 2019 20:46

felixfbecker reviewed Sep 10, 2019

View reviewed changes

TysonAndre force-pushed the fix-tostring-deprecation branch from be6cbd6 to b69b94c Compare September 10, 2019 12:48

Fix deprecation notice for ReflectionType::__toString() in php 7.4

d6473a9

Use ReflectionNamedType->getName() for php 7.1+, instead. Continue supporting php 7.0 Fixes felixfbecker#23

TysonAndre force-pushed the fix-tostring-deprecation branch from b69b94c to d6473a9 Compare September 12, 2019 22:26

felixfbecker changed the title ~~Fix deprecation notice for ReflectionType::__toString() in php 7.4~~ fix: deprecation notice for ReflectionType::__toString() in php 7.4 Sep 12, 2019

felixfbecker merged commit 887e7fd into felixfbecker:master Sep 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: deprecation notice for ReflectionType::__toString() in php 7.4 #33

fix: deprecation notice for ReflectionType::__toString() in php 7.4 #33

TysonAndre commented Jul 27, 2019 •

edited

Loading

codecov bot commented Jul 27, 2019 •

edited by ghost

Loading

felixfbecker commented Jul 27, 2019 •

edited

Loading

TysonAndre commented Jul 27, 2019

TysonAndre commented Aug 4, 2019 •

edited

Loading

xPaw commented Sep 6, 2019

felixfbecker left a comment

felixfbecker Sep 10, 2019

felixfbecker Sep 10, 2019

felixfbecker Sep 10, 2019

felixfbecker Sep 10, 2019

TysonAndre commented Sep 12, 2019

felixfbecker commented Sep 12, 2019

TysonAndre commented Sep 12, 2019

felixfbecker commented Sep 12, 2019

felixfbecker commented Sep 12, 2019

	$param_type = $param->getType();
	$paramType = $param->getType();

	if ($param_type instanceof ReflectionNamedType) {
	if ($paramType instanceof ReflectionNamedType) {

	$class = $param_type->getName();
	$class = $paramType->getName();

fix: deprecation notice for ReflectionType::__toString() in php 7.4 #33

fix: deprecation notice for ReflectionType::__toString() in php 7.4 #33

Conversation

TysonAndre commented Jul 27, 2019 • edited Loading

codecov bot commented Jul 27, 2019 • edited by ghost Loading

Codecov Report

felixfbecker commented Jul 27, 2019 • edited Loading

TysonAndre commented Jul 27, 2019

TysonAndre commented Aug 4, 2019 • edited Loading

xPaw commented Sep 6, 2019

felixfbecker left a comment

Choose a reason for hiding this comment

felixfbecker Sep 10, 2019

Choose a reason for hiding this comment

felixfbecker Sep 10, 2019

Choose a reason for hiding this comment

felixfbecker Sep 10, 2019

Choose a reason for hiding this comment

felixfbecker Sep 10, 2019

Choose a reason for hiding this comment

TysonAndre commented Sep 12, 2019

felixfbecker commented Sep 12, 2019

TysonAndre commented Sep 12, 2019

felixfbecker commented Sep 12, 2019

felixfbecker commented Sep 12, 2019

TysonAndre commented Jul 27, 2019 •

edited

Loading

codecov bot commented Jul 27, 2019 •

edited by ghost

Loading

felixfbecker commented Jul 27, 2019 •

edited

Loading

TysonAndre commented Aug 4, 2019 •

edited

Loading