phpspec / prophecy

Highly opinionated mocking framework for PHP 5.3+
MIT License
8.53k stars 241 forks source link

Idea: Using spl_object_id instead of spl_object_hash for stringified errors? (stdClass#1234 instead of stdClass:0000000046ef9d1d000000002ac5521e) #549

Closed TysonAndre closed 2 years ago

TysonAndre commented 2 years ago

https://php.net/spl_object_id requires php 7.2+, and the latest releases of prophecy also require php 7.2+. (aside: it has polyfills such as symfony/polyfill-php72)

I'd personally find the object ids easier to read/distinguish from each other than the spl_object_hash/SplObjectStorage

Current:

      identical(Double\stdClass\P9:0000000046ef9d1d000000002ac5521e Object (
          'objectProphecyClosure' => Closure:0000000046ef8661000000002ac5521e Object (
              0 => Closure:0000000046ef8661000000002ac5521e Object
          )
      ))

Proposed

      identical(Double\stdClass\P9#4480 Object (
          'objectProphecyClosure' => Closure#4458 Object (
              0 => Closure#4458 Object
          )
      ))
diff --git a/src/Prophecy/Util/ExportUtil.php b/src/Prophecy/Util/ExportUtil.php
index 1090a80..218c02d 100644
--- a/src/Prophecy/Util/ExportUtil.php
+++ b/src/Prophecy/Util/ExportUtil.php
@@ -91,7 +91,7 @@ class ExportUtil
             }

             foreach ($value as $key => $val) {
-                $array[spl_object_hash($val)] = array(
+                $array[spl_object_id($val)] = array(
                     'obj' => $val,
                     'inf' => $value->getInfo(),
                 );
@@ -181,11 +181,11 @@ class ExportUtil
         if (is_object($value)) {
             $class = get_class($value);

-            if ($hash = $processed->contains($value)) {
-                return sprintf('%s:%s Object', $class, $hash);
+            if ($processed->contains($value)) {
+                return sprintf('%s#%d Object', $class, spl_object_id($value));
             }

-            $hash   = $processed->add($value);
+            $processed->add($value);
             $values = '';
             $array  = self::toArray($value);

@@ -202,7 +202,7 @@ class ExportUtil
                 $values = "\n" . $values . $whitespace;
             }

-            return sprintf('%s:%s Object (%s)', $class, $hash, $values);
+            return sprintf('%s#%d Object (%s)', $class, spl_object_id($value), $values);
         }

         return var_export($value, true);
diff --git a/src/Prophecy/Util/StringUtil.php b/src/Prophecy/Util/StringUtil.php
index ba4faff..b63181e 100644
--- a/src/Prophecy/Util/StringUtil.php
+++ b/src/Prophecy/Util/StringUtil.php
@@ -56,7 +56,7 @@ class StringUtil
             return get_resource_type($value).':'.$value;
         }
         if (is_object($value)) {
-            return $exportObject ? ExportUtil::export($value) : sprintf('%s:%s', get_class($value), spl_object_hash($value));
+            return $exportObject ? ExportUtil::export($value) : sprintf('%s#%s', get_class($value), spl_object_id($value));
         }
         if (true === $value || false === $value) {
             return $value ? 'true' : 'false';
jdreesen commented 2 years ago

I like that. The hash is actually really difficult to read.

stof commented 2 years ago

Please send a pull request with your change