fix(set): map uniqueness

[abenerd]

Description

When mapping over a set it is possible to introduce duplicate values which is undesirable.

$set = new \Ramsey\Collection\Set('integer', [1, 2, 3]);
$set->map(fn () => 42);

Motivation and context

This change is so that the behavior of the implementation is respected such as in this case the duplicate values in a Set.

How has this been tested?

Tests have been added for the Set and Collection implementations that a instance of themselves is returned and that when mapping over an existing Set not duplicate values can be introduced.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  The actual return types as declared have not changed but in the case of the Set a new Set is now returned instead of a Collection as well as the new mapping behavior for the Set that does now not allow duplicate values anymore.

PR checklist

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING.md document.
• I have added tests to cover my changes.

[SimoTod]

I'm not sure map was meant to preserve the class implementation. It just maps all the values to a generic collection (you can also see the value type is mixed instead of using the current type). It will likely break things for a few projects.

[abenerd]

I see the BC concern but I also feel like when I'm manipulating a set I want it to behave like a set. Maybe this can be merged into a v3 branch but I think this is an important change.

[SimoTod]

Laravel's collections have a mapInto function accepting the collection class name as a second parameter. If the maintainer is open to something like that maybe it could be a good compromise for your use case.

[ramsey]

I think the best approach would be to override the map() method on the Set class, rather than introducing the protected $collection property.

That said, map() returns a CollectionInterface. It's not intended to make guarantees about the data it returns. The library is designed to be generic and flexible, allowing you to extend it for your application's needs. I think you could extend it with something like this to achieve your goals:

/**
 * @extends \Ramsey\Collection\AbstractSet<Foo>
 */
class MySet extends \Ramsey\Collection\AbstractSet
{
    public function getType(): string
    {
        return MyType::class;
    }

    /**
     * @param callable(MyType): TCallbackReturn $callback A callable to apply to each
     *     item of the collection.
     *
     * @return MySet
     *
     * @template TCallbackReturn as MyType
     */
    public function map(callable $callback): \Ramsey\Collection\CollectionInterface
    {
        return new self(array_map($callback, $this->data));
    }
}

[abenerd]

I see your point about not introducing a $collection property but I found the map implementations too similar to override them, alternatively one could in the AbstractCollection create a new static collection this way you'd automatically get the correct implementation, although we would then have to take care with the constructor arguments.

I don't think this change would take away any flexibility of the library we would simply ensure the correct behavior for any implementation of a collection and it still allows users to extend any collection they want to add their own behavior.

[ramsey]

we would simply ensure the correct behavior

Is the correct behavior for the map function to always return the same type of collection?

[abenerd]

I would argue so yes, when I create a new Set I expect it to behave a certain way and until I explicitly say make this Set into a Collection, or Whatever I want to be sure in the assumption that I am working with a Set.