Improve internal codebase

Author: nyamsprod

CHANGELOG.md

@@ -11,6 +11,8 @@ All Notable changes to `bakame/http-strucured-fields` will be documented in this
 ### Fixed
 
 - `Parameters::remove` also removes parameters per indexes
+- `Type::fromValue` throws an `InvalidArgument` exception.
+- `Type::fromValue` and `Type::tryFromValue` should only check the PHP variable type and not take into account the variable value.
 
 ### Deprecated
 

README.md

@@ -145,7 +145,7 @@ use Bakame\Http\StructuredFields\Type;
 
 echo Type::fromValue(42);         // returns Type::Integer
 echo Type::fromValue(42.0)->name; // returns 'Decimal'
-echo Type::fromValue(new SplTempFileObject());    // throws TypeError
+echo Type::fromValue(new SplTempFileObject());    // throws InvalidArgument
 echo Type::tryFromValue(new SplTempFileObject()); // returns null
 ```
 
@@ -236,7 +236,7 @@ use Bakame\Http\StructuredFields\Parameters;
 $value = Parameters::fromHttpValue(';a=foobar']);
 $value->has('b');     // return false
 $value['a']->value(); // return 'foobar'
-$value['b'];          // triggers a SyntaxError exception, the index does not exist
+$value['b'];          // triggers a InvalidOffset exception, the index does not exist
 $value['a'] = 23      // triggers a ForbiddenOperation exception
 unset($value['a']);   // triggers a ForbiddenOperation exception
 ```
@@ -412,7 +412,7 @@ $value = Dictionary::new()
             Item::fromInteger(42),
             Item::fromDecimal(42)
          )],
-         ['c', true]
+         ['c', Item::true()]
      )
     ->unshift(['b', Item::false()])
     ->replace(2, ['c', Item::fromDateString('2022-12-23 13:00:23')])
@@ -433,7 +433,7 @@ If the submitted type is:
 
 -  a `StructuredField` implementing object, it will be passed as is
 -  an iterable structure, it will be converted to an `InnerList` instance using `InnerList::new`
--  otherwise, it is converted into an `Item` using `Item::new` following the conversion rules explained in the table above.
+-  otherwise, it is converted into an `Item` using the `Item::new` named constructor.
 
 If no conversion is possible an `InvalidArgument` exception will be thrown.
 
@@ -466,7 +466,7 @@ echo Dictionary::new()
  // both will return 'b=?0, a=(bar "42" 42 42.0), c=@1671800423
 ```
 
-Of course, it is possible to mix both notation as shown in the example.
+Of course, it is possible to mix both notations.
 
 #### Lists
 
@@ -564,7 +564,7 @@ $field->withoutParameters(string ...$keys): static;
 $field->withoutAnyParameter(): static;
 $field->withParameters(Parameters $parameters): static;
 ```
-Since verrsion `1.1` it is also possible to use the index of each member to perform additional
+Since version `1.1` it is also possible to use the index of each member to perform additional
 modifications.
 
 ```php

src/Dictionary.php

@@ -14,6 +14,7 @@
 use function count;
 use function implode;
 use function is_array;
+use function is_int;
 use function is_iterable;
 use function is_string;
 
@@ -90,15 +91,14 @@ public static function fromAssociative(iterable $members): self
      */
     public static function fromPairs(iterable $pairs): self
     {
-        if ($pairs instanceof MemberOrderedMap) {
-            return new self($pairs);
-        }
-
-        return new self((function (iterable $pairs) {
-            foreach ($pairs as [$key, $member]) {
-                yield $key => $member;
-            }
-        })($pairs));
+        return match (true) {
+            $pairs instanceof MemberOrderedMap => new self($pairs),
+            default => new self((function (iterable $pairs) {
+                foreach ($pairs as [$key, $member]) {
+                    yield $key => $member;
+                }
+            })($pairs)),
+        };
     }
 
     /**
@@ -110,18 +110,15 @@ public static function fromPairs(iterable $pairs): self
      */
     public static function fromHttpValue(Stringable|string $httpValue): self
     {
-        return new self((function (iterable $members) {
-            foreach ($members as $key => $member) {
-                if (!is_array($member[0])) {
-                    yield $key => Item::fromAssociative(...$member);
-
-                    continue;
-                }
+        $converter = fn (array $member): InnerList|Item => match (true) {
+            is_array($member[0]) => InnerList::fromAssociative(
+                array_map(fn (array $item) => Item::fromAssociative(...$item), $member[0]),
+                $member[1]
+            ),
+            default => Item::fromAssociative($member[0], $member[1]),
+        };
 
-                $member[0] = array_map(fn (array $item) => Item::fromAssociative(...$item), $member[0]);
-                yield $key => InnerList::fromAssociative(...$member);
-            }
-        })(Parser::parseDictionary($httpValue)));
+        return new self(array_map($converter, Parser::parseDictionary($httpValue)));
     }
 
     public function toHttpValue(): string
@@ -163,7 +160,7 @@ public function getIterator(): Iterator
     }
 
     /**
-     * @return Iterator<array{0:string, 1:SfMember}>
+     * @return Iterator<int, array{0:string, 1:SfMember}>
      */
     public function toPairs(): Iterator
     {
@@ -199,17 +196,14 @@ public function has(string|int ...$keys): bool
      */
     public function get(string|int $key): StructuredField
     {
-        if (!$this->has($key)) {
-            throw InvalidOffset::dueToKeyNotFound($key);
-        }
-
-        return $this->members[$key];
+        return $this->members[$key] ?? throw InvalidOffset::dueToKeyNotFound($key);
     }
 
     public function hasPair(int ...$indexes): bool
     {
+        $max = count($this->members);
         foreach ($indexes as $index) {
-            if (null === $this->filterIndex($index)) {
+            if (null === $this->filterIndex($index, $max)) {
                 return false;
             }
         }
@@ -220,12 +214,14 @@ public function hasPair(int ...$indexes): bool
     /**
      * Filters and format instance index.
      */
-    private function filterIndex(int $index): int|null
+    private function filterIndex(int $index, int|null $max = null): int|null
     {
-        $max = count($this->members);
+        $max ??= count($this->members);
 
         return match (true) {
-            [] === $this->members, 0 > $max + $index, 0 > $max - $index - 1 => null,
+            [] === $this->members,
+            0 > $max + $index,
+            0 > $max - $index - 1 => null,
             0 > $index => $max + $index,
             default => $index,
         };
@@ -238,10 +234,7 @@ private function filterIndex(int $index): int|null
      */
     public function pair(int $index): array
     {
-        $offset = $this->filterIndex($index);
-        if (null === $offset) {
-            throw InvalidOffset::dueToIndexNotFound($index);
-        }
+        $offset = $this->filterIndex($index) ?? throw InvalidOffset::dueToIndexNotFound($index);
 
         return [...$this->toPairs()][$offset];
     }
@@ -262,29 +255,39 @@ public function add(string $key, iterable|StructuredField|Token|ByteSequence|Dat
      */
     private function newInstance(array $members): self
     {
-        if ($members == $this->members) {
-            return $this;
-        }
-
-        return new self($members);
+        return match (true) {
+            $members == $this->members => $this,
+            default => new self($members),
+        };
     }
 
     public function remove(string|int ...$keys): static
     {
-        /** @var array<array-key, true> $indexes */
-        $indexes = array_fill_keys($keys, true);
-        $pairs = [];
-        foreach ($this->toPairs() as $index => $pair) {
-            if (!isset($indexes[$index]) && !isset($indexes[$pair[0]])) {
-                $pairs[] = $pair;
-            }
-        }
-
-        if (count($this->members) === count($pairs)) {
+        if ([] === $this->members || [] === $keys) {
             return $this;
         }
 
-        return self::fromPairs($pairs);
+        $offsets = array_keys($this->members);
+        $max = count($offsets);
+        $reducer = fn (array $carry, string|int $key): array => match (true) {
+            is_string($key) && (false !== ($position = array_search($key, $offsets, true))),
+            is_int($key) && (null !== ($position = $this->filterIndex($key, $max))) => [$position => true] + $carry,
+            default => $carry,
+        };
+
+        $indices = array_reduce($keys, $reducer, []);
+
+        return match (true) {
+            [] === $indices => $this,
+            $max === count($indices) => self::new(),
+            default => self::fromPairs((function (array $offsets) {
+                foreach ($this->toPairs() as $offset => $pair) {
+                    if (!array_key_exists($offset, $offsets)) {
+                        yield $pair;
+                    }
+                }
+            })($indices)),
+        };
     }
 
     /**
@@ -294,9 +297,8 @@ public function append(string $key, iterable|StructuredField|Token|ByteSequence|
     {
         $members = $this->members;
         unset($members[$key]);
-        $members[MapKey::from($key)->value] = self::filterMember($member);
 
-        return $this->newInstance($members);
+        return $this->newInstance([...$members, MapKey::from($key)->value => self::filterMember($member)]);
     }
 
     /**
@@ -306,53 +308,46 @@ public function prepend(string $key, iterable|StructuredField|Token|ByteSequence
     {
         $members = $this->members;
         unset($members[$key]);
-        $members = [MapKey::from($key)->value => self::filterMember($member), ...$members];
 
-        return $this->newInstance($members);
+        return $this->newInstance([MapKey::from($key)->value => self::filterMember($member), ...$members]);
     }
 
     /**
      * @param array{0:string, 1:SfMember|SfMemberInput} ...$pairs
      */
     public function push(array ...$pairs): self
     {
-        if ([] === $pairs) {
-            return $this;
-        }
-
-        $newPairs = iterator_to_array($this->toPairs());
-        foreach ($pairs as $pair) {
-            $newPairs[] = $pair;
-        }
-
-        return self::fromPairs($newPairs);
+        return match (true) {
+            [] === $pairs => $this,
+            default => self::fromPairs((function (iterable $pairs) {
+                yield from $this->toPairs();
+                yield from $pairs;
+            })($pairs)),
+        };
     }
 
     /**
      * @param array{0:string, 1:SfMember|SfMemberInput} ...$pairs
      */
     public function unshift(array ...$pairs): self
     {
-        if ([] === $pairs) {
-            return $this;
-        }
-
-        foreach ($this->members as $key => $member) {
-            $pairs[] = [$key, $member];
-        }
-
-        return self::fromPairs($pairs);
+        return match (true) {
+            [] === $pairs => $this,
+            default => self::fromPairs((function (iterable $pairs) {
+                yield from $pairs;
+                yield from $this->toPairs();
+            })($pairs)),
+        };
     }
 
     /**
      * @param array{0:string, 1:SfMember|SfMemberInput} ...$members
      */
     public function insert(int $index, array ...$members): static
     {
-        $offset = $this->filterIndex($index);
+        $offset = $this->filterIndex($index) ?? throw InvalidOffset::dueToIndexNotFound($index);
 
         return match (true) {
-            null === $offset => throw InvalidOffset::dueToIndexNotFound($index),
             [] === $members => $this,
             0 === $offset => $this->unshift(...$members),
             count($this->members) === $offset => $this->push(...$members),
@@ -370,18 +365,14 @@ public function insert(int $index, array ...$members): static
      */
     public function replace(int $index, array $member): static
     {
-        $offset = $this->filterIndex($index);
-        if (null === $offset) {
-            throw InvalidOffset::dueToIndexNotFound($index);
-        }
-
+        $offset = $this->filterIndex($index) ?? throw InvalidOffset::dueToIndexNotFound($index);
         $member[1] = self::filterMember($member[1]);
         $pairs = iterator_to_array($this->toPairs());
-        if ($pairs[$offset] == $member) {
-            return $this;
-        }
 
-        return self::fromPairs(array_replace($pairs, [$offset => $member]));
+        return match (true) {
+            $pairs[$offset] == $member => $this,
+            default => self::fromPairs(array_replace($pairs, [$offset => $member])),
+        };
     }
 
     /**

src/DictionaryTest.php

@@ -255,6 +255,10 @@ public function it_can_delete_a_member_via_remove_method(): void
         self::assertSame($newInstance, $newInstance2);
 
         self::assertFalse($newInstance->remove('foo')->hasMembers());
+        self::assertSame($newInstance, $newInstance->remove('baz', 'bar', 'yolo-not-there', 325));
+
+        $instanceWithoutMember = Dictionary::new();
+        self::assertSame($instanceWithoutMember->remove(), $instanceWithoutMember);
     }
 
     #[Test]
@@ -375,4 +379,24 @@ public function it_returns_the_same_instance_if_nothing_is_replaced(): void
 
         self::assertSame($field->replace(0, ['a', true]), $field);
     }
+
+    #[Test]
+    public function it_can_create_a_new_instance_using_parameters_position_modifying_methods(): void
+    {
+        $instance1 = Dictionary::new();
+        $instance2 = $instance1
+            ->push(['a', true], ['v', ByteSequence::fromDecoded('I will be removed')], ['c', 'true'])
+            ->unshift(['b', Item::false()])
+            ->replace(1, ['a', 'false'])
+            ->remove(-2, 'toto')
+            ->insert(1, ['d', Token::fromString('*/*')]);
+
+        self::assertTrue($instance1->hasNoMembers());
+        self::assertTrue($instance2->hasMembers());
+        self::assertCount(4, $instance2);
+        self::assertEquals(['d', Item::fromToken('*/*')], $instance2->pair(1));
+        self::assertEquals(['b', Item::false()], $instance2->pair(0));
+        self::assertEquals(['c', Item::fromString('true')], $instance2->pair(-1));
+        self::assertSame('b=?0, d=*/*, a="false", c="true"', $instance2->toHttpValue());
+    }
 }