[VarDumper] add a GMP caster in order to cast GMP resources into string or integer #25237

Simperfit · 2017-12-01T07:51:03Z

Q	A
Branch?	4.1
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#25222
License	MIT
Doc PR	todo

Do you want to dump that kind of resources ? Then it means that the app you are writing is doing some math... why?! :p

It quite nice the little snow that we have in the north of france right now:

stof · 2017-12-01T09:01:21Z

src/Symfony/Component/VarDumper/Caster/GmpCaster.php

+ */
+class GmpCaster
+{
+    public static function convertToInt(\GMP $value): int


this is not what a VarDumper caster is.

stof · 2017-12-01T09:02:29Z

src/Symfony/Component/VarDumper/composer.json

@@ -29,7 +29,8 @@
    },
    "suggest": {
        "ext-iconv": "To convert non-UTF-8 strings to UTF-8 (or symfony/polyfill-iconv in case ext-iconv cannot be used).",
-        "ext-intl": "To show region name in time zone dump"
+        "ext-intl": "To show region name in time zone dump",
+        "ext-gmp": "To dump GMP resources to string or integer"


this does not make any sense: the VarDumper component does not provide any feature needing gmp. It would just improve the dumping when your own code uses gmp. So there is no point suggesting it. It would just be spam in the suggestions

i'll remove it

stof · 2017-12-01T09:04:59Z

.travis.yml

@@ -97,6 +97,7 @@ before_install:
      echo extension = redis.so >> $INI
      echo extension = memcached.so >> $INI
      echo extension = mongodb.so >> $INI
+      echo extension = gmp.so >> $INI


there is no gmp.so on Travis. So this is breaking the CI.

gmp is already available on Travis, as it is built as a bundled extension

i'll remove it too

sroze · 2017-12-01T09:48:37Z

It's clearly a WIP PR. @Simperfit I suggest you either add WIP to your PR title or just not open the PR yet (and enable Travis on your fork if you need the tests to run) :)

nicolas-grekas · 2017-12-01T10:45:40Z

src/Symfony/Component/VarDumper/Caster/GmpCaster.php

+ */
+class GmpCaster
+{
+    public static function castInt(\GMP $value): array


see other casters: that not their typical signature, we should use the same here also

nicolas-grekas · 2017-12-01T10:48:15Z

src/Symfony/Component/VarDumper/Caster/GmpCaster.php

+
+    public static function castString(\GMP $value): array
+    {
+        return array('value' => gmp_strval($value));


This means "value" will be displayed as if it were a public property on the object, which is wrong
should be a virtual property (see other casters)
Also: this will display the value as if it were a string, which is also a bad hint. I suggest to use a ConstStub to wrap the value, to that it is displayed in a more specific way. I invite you to compare with/without and decide.

nicolas-grekas · 2017-12-01T10:48:52Z

src/Symfony/Component/VarDumper/Cloner/AbstractCloner.php

@@ -112,6 +112,8 @@
        'DateTimeZone' => array('Symfony\Component\VarDumper\Caster\DateCaster', 'castTimeZone'),
        'DatePeriod' => array('Symfony\Component\VarDumper\Caster\DateCaster', 'castPeriod'),

+        'GMP' => array('Symfony\Component\VarDumper\Caster\GmpCaster', 'castInt'),


not all GMP object's values can be represented as strings, so castString should be the implementation

nicolas-grekas · 2017-12-01T11:00:20Z

src/Symfony/Component/VarDumper/Cloner/AbstractCloner.php

@@ -126,6 +128,9 @@
        ':persistent stream' => array('Symfony\Component\VarDumper\Caster\ResourceCaster', 'castStream'),
        ':stream-context' => array('Symfony\Component\VarDumper\Caster\ResourceCaster', 'castStreamContext'),
        ':xml' => array('Symfony\Component\VarDumper\Caster\XmlResourceCaster', 'castXml'),
+        ':gmp' => array('Symfony\Component\VarDumper\Caster\GmpCaster', 'castInt'),
+        ':gmp int' => array('Symfony\Component\VarDumper\Caster\GmpCaster', 'castInt'),
+        ':gmp string' => array('Symfony\Component\VarDumper\Caster\GmpCaster', 'castString'),


GMP as resources cannot happen anymore on PHP 7.1, this should be removed.

nicolas-grekas · 2017-12-01T11:00:40Z

src/Symfony/Component/VarDumper/Caster/GmpCaster.php

+        return array('value' => gmp_intval($value));
+    }
+
+    public static function castString(\GMP $value): array


this should be renamed "castGmp"

nicolas-grekas · 2017-12-01T11:01:06Z

src/Symfony/Component/VarDumper/Tests/Caster/GmpCasterTest.php

+{
+    public function testCastInt()
+    {
+        if (false === extension_loaded('gmp')) {


@requires extension gmp instead

nicolas-grekas · 2017-12-01T11:01:40Z

src/Symfony/Component/VarDumper/Tests/Cloner/VarClonerTest.php

+
+EOTXT;
+
+        $this->assertStringMatchesFormat($expected, print_r($clone, true));


let's use assertDumpEquals instead

nicolas-grekas · 2017-12-01T14:43:36Z

src/Symfony/Component/VarDumper/Tests/Cloner/VarClonerTest.php

+"""
+EODUMP;
+
+        $this->assertDumpEquals($expected, print_r($clone, true));


this is playing around assertDumpEquals
just $this->assertDumpEquals($expected, gmp_init('0101')); instead

nicolas-grekas · 2017-12-01T14:44:51Z

src/Symfony/Component/VarDumper/Tests/Cloner/VarClonerTest.php

+    /**
+     * @requires extension gmp
+     */
+    public function testGmpCast()


but in fact, this should not be in VarClonerTest
the existing test in the caster in enough IMO

nicolas-grekas · 2017-12-01T14:45:06Z

src/Symfony/Component/VarDumper/Tests/Caster/GmpCasterTest.php

+    /**
+     * @requires extension gmp
+     */
+    public function testCastString()


nicolas-grekas · 2017-12-01T14:46:47Z

src/Symfony/Component/VarDumper/Caster/GmpCaster.php

+{
+    public static function castGmp(\GMP $gmp, array $a, Stub $stub, $isNested, $filter): array
+    {
+        $a[Caster::PREFIX_VIRTUAL.'gmpString'] = (string) new ConstStub(\GMP::class, gmp_strval($gmp));


s/gmpString/value/
the what's the prupose of the cast to string? looks wrong to me
should be:
$a[Caster::PREFIX_VIRTUAL.'num'] = new ConstStub(gmp_strval($gmp), gmp_strval($gmp));

nicolas-grekas · 2017-12-01T14:47:37Z

src/Symfony/Component/VarDumper/Caster/GmpCaster.php

+use Symfony\Component\VarDumper\Cloner\Stub;
+
+/**
+ * Transform a GMP object to a integer or a string. It need the gmp php extension.


* Casts GMP objects to array representation.

Simperfit · 2017-12-01T15:48:09Z

here is a little gif of the french fields with snow:

…ng or integer

fabpot · 2017-12-01T17:50:30Z

Thank you @Simperfit.

…urces into string or integer (Simperfit) This PR was merged into the 4.1-dev branch. Discussion ---------- [VarDumper] add a GMP caster in order to cast GMP resources into string or integer | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25222 | License | MIT | Doc PR | todo Do you want to dump that kind of resources ? Then it means that the app you are writing is doing some math... why?! :p It quite nice the little snow that we have in the north of france right now: ![img_2844](https://user-images.githubusercontent.com/3451634/33472917-8b48913e-d674-11e7-923f-ad951f7f2966.JPG) Commits ------- ed2c1af [VarDumper] add a GMP caster in order to cast GMP resources into string or integer

carsonbot added Status: Needs Review VarDumper Feature labels Dec 1, 2017

Simperfit force-pushed the feature/add-a-caster-for-gmp-numbers branch from 75d9636 to d4c7ce7 Compare December 1, 2017 07:51

stof reviewed Dec 1, 2017

View reviewed changes

stof requested changes Dec 1, 2017

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Dec 1, 2017

stof requested changes Dec 1, 2017

View reviewed changes

Simperfit changed the title ~~[VarDumper] add a GMP caster in order to cast GMP resources into string or integer~~ [VarDumper] [WIP] add a GMP caster in order to cast GMP resources into string or integer Dec 1, 2017

Simperfit force-pushed the feature/add-a-caster-for-gmp-numbers branch 3 times, most recently from 2a4413b to dd04bd0 Compare December 1, 2017 10:33

nicolas-grekas requested changes Dec 1, 2017

View reviewed changes

Simperfit force-pushed the feature/add-a-caster-for-gmp-numbers branch from dd04bd0 to 452a273 Compare December 1, 2017 14:29

Simperfit changed the title ~~[VarDumper] [WIP] add a GMP caster in order to cast GMP resources into string or integer~~ [VarDumper] add a GMP caster in order to cast GMP resources into string or integer Dec 1, 2017

Simperfit force-pushed the feature/add-a-caster-for-gmp-numbers branch from 452a273 to 516e25d Compare December 1, 2017 14:34

nicolas-grekas reviewed Dec 1, 2017

View reviewed changes

Simperfit force-pushed the feature/add-a-caster-for-gmp-numbers branch from 516e25d to 5f1f345 Compare December 1, 2017 14:44

nicolas-grekas reviewed Dec 1, 2017

View reviewed changes

Simperfit force-pushed the feature/add-a-caster-for-gmp-numbers branch 3 times, most recently from 369e481 to 530d71b Compare December 1, 2017 14:57

Simperfit force-pushed the feature/add-a-caster-for-gmp-numbers branch from 530d71b to 4c7a3f7 Compare December 1, 2017 15:54

[VarDumper] add a GMP caster in order to cast GMP resources into stri…

ed2c1af

…ng or integer

Simperfit force-pushed the feature/add-a-caster-for-gmp-numbers branch from 4c7a3f7 to ed2c1af Compare December 1, 2017 15:54

carsonbot added Status: Reviewed and removed Status: Needs Work labels Dec 1, 2017

nicolas-grekas approved these changes Dec 1, 2017

View reviewed changes

nicolas-grekas added this to the 4.1 milestone Dec 1, 2017

fabpot approved these changes Dec 1, 2017

View reviewed changes

fabpot merged commit ed2c1af into symfony:master Dec 1, 2017

javiereguiluz mentioned this pull request Jan 8, 2018

[VarDumper] add a GMP caster in order to cast GMP resources into string or integer symfony/symfony-docs#9021

Closed

fabpot mentioned this pull request May 7, 2018

Release v4.1.0-BETA1 #27181

Merged

Simperfit deleted the feature/add-a-caster-for-gmp-numbers branch May 30, 2018 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[VarDumper] add a GMP caster in order to cast GMP resources into string or integer #25237

[VarDumper] add a GMP caster in order to cast GMP resources into string or integer #25237

Simperfit commented Dec 1, 2017

stof Dec 1, 2017

stof Dec 1, 2017

Simperfit Dec 1, 2017

stof Dec 1, 2017

Simperfit Dec 1, 2017

sroze commented Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

nicolas-grekas Dec 1, 2017

Simperfit commented Dec 1, 2017

fabpot commented Dec 1, 2017


		EOTXT;

		$this->assertStringMatchesFormat($expected, print_r($clone, true));

[VarDumper] add a GMP caster in order to cast GMP resources into string or integer #25237

[VarDumper] add a GMP caster in order to cast GMP resources into string or integer #25237

Conversation

Simperfit commented Dec 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sroze commented Dec 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Simperfit commented Dec 1, 2017

fabpot commented Dec 1, 2017