Not dumping the contents of the data structures might be considered a bug by some people. Personally I'd like to see it in 3.x because our project is using LTS Symfony and not seeing the data is often a problem for my colleagues when debugging. On the other hand it should be possible to only upgrade symfony/var-dumper to 4.x and keep the rest as LTS. Let me know if you want me to target an older branch and I'll rebase it.

Btw. should I add : array return types?

this is a new feature, so it goes in master to me.

Please add a line about it in the changelog of the component.

Btw. should I add : array return types?

yes please

Btw. should I add : array return types?

yes please

Note that the array return type is not present in other casters.

Another thing is the $isNested parameter which sounds like a boolean but doesn't have the typehint in other cases.

Also I'm not actually using $stub and $isNested so I could just remove them and it should still work.

Most of the existing code date from pre-PHP7, that's the reason. New code can use new language features. Let's keep the extra arguments as adding them later would be a BC break, and also because that's the signature of casters.

Ok, all done.

I suggesting adding the bool typehint on $isNested too.

@stof According to docs Casters can also take a fifth parameter $filter. Should I add it too?

the docs says this about it:

A bit field of Caster ::EXCLUDE_* constants.

https://symfony.com/doc/current/components/var_dumper/advanced.html#casters

Almost no casters use it so no need.

Thank you @enumag.

We've tried to use it and my colleague @keksa found a bug. I identified the underlying cause but I'm not exactly sure how to fix it.

The problem is with dumping Ds\Map. Bacause Map contains object as keys we call Map::pairs() in the caster to get the keys and values as Ds\Pair instances and dump those instead. The problem is that since these pairs are only used when dumping, they get destroyed and their object ids can be reused which causes the bug.

Code to reproduce:

$map = unserialize('C:6:"Ds\Map":58:{i:1;C:6:"Ds\Map":8:{i:1;i:2;}i:2;C:6:"Ds\Map":8:{i:1;i:2;}}');
dump($map);

Dumped result:

Map {#96 ▼
  +0: Pair {#218 ▼
    +key: 1
    +value: Map {#94 ▼
      +0: Pair {#218} // Reused ID from above, dumping is skipped.
      capacity: 8
    }
  }
  +1: Pair {#217 ▶}
  capacity: 8
}

I'm not exactly sure how to fix it. An easy fix would be to hold the pairs instances in some global variable and clean it when the dumping is complete but of course that's not exactly clean. Did you encounter a similar problem in any other casters?

I think then we should drop the Pair object from the dump and make it a simple array with two virtual keys. Would that work for you?

I thought about that. While it would work I think it's a worse solution than dumping pairs. People who work with Ds are aware about Map being represented by Ds\Pairs so it's more clear that the pairs in dump are just representation of the inner structure and not actual objects in the map. Also var_dump() also dumps Map as Pair instances so I'd like for SF dump to work the same way. So yeah, arrays can fix the issue but I'd very much prever avoiding that if possible.

From what I understand, a new instance is created all the time, so that the reference id is useless?
We should find a way to hide it then.
Using a stub attribute should allow doing that.

The ID being useless is not really a problem. The problem is that dumper (or at least HTML dumper) doesn't render a dump for an object with the same ID as an object rendered before because it assumes they are the same object which is not true in this case.

See #30349