Improve Directive usage and implementations

Author: nyamsprod

components/Components/Directives/Factory.php

@@ -0,0 +1,36 @@
+<?php
+
+/**
+ * League.Uri (https://uri.thephpleague.com)
+ *
+ * (c) Ignace Nyamagana Butera <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace League\Uri\Components\Directives;
+
+use Stringable;
+
+final class Factory
+{
+    /**
+     * Parse a Directive string representation.
+     *
+     * A Directive syntax is name[=value] where the
+     * separator `=` is not present when no value
+     * is attached to it
+     */
+    public static function parse(Stringable|string $directive): Directive
+    {
+        $directive = (string) $directive;
+
+        return match (true) {
+            str_starts_with($directive, 'text=') => TextDirective::fromString($directive),
+            default => GenericDirective::fromString($directive),
+        };
+    }
+}

components/Components/Directives/GenericDirective.php

@@ -23,8 +23,13 @@
 
 final class GenericDirective implements Directive
 {
-    private function __construct(private string $name, private ?string $value = null)
-    {
+    /**
+     * @param non-empty-string $name
+     */
+    private function __construct(
+        private readonly string $name,
+        private readonly ?string $value = null,
+    ) {
     }
 
     /**
@@ -33,7 +38,7 @@ private function __construct(private string $name, private ?string $value = null
     public static function fromString(Stringable|string $value): self
     {
         [$name, $value] = explode('=', (string) $value, 2) + [1 => null];
-        (null !== $name && !str_contains($name, '&')) || throw new SyntaxError('The submitted text is not a valid directive.');
+        (null !== $name && '' !== $name && !str_contains($name, '&')) || throw new SyntaxError('The submitted text is not a valid directive.');
 
         return new self($name, $value);
     }

components/Components/Directives/TextDirective.php

@@ -34,12 +34,20 @@ final class TextDirective implements Directive
         (?:,-(?<suffix>.+))?     # optional suffix (to end)
     $/x';
 
+    /**
+     * @param non-empty-string $start
+     * @param ?non-empty-string $end
+     * @param ?non-empty-string $prefix
+     * @param ?non-empty-string $suffix
+     */
     public function __construct(
         public readonly string $start,
         public readonly ?string $end = null,
         public readonly ?string $prefix = null,
         public readonly ?string $suffix = null,
     ) {
+        ('' !== $this->start && '' !== $this->end && '' !== $this->prefix && '' !== $this->suffix)
+        || throw new SyntaxError('The start part can not be the empty string.');
     }
 
     /**
@@ -64,18 +72,20 @@ public static function fromValue(Stringable|string $text): self
             $matches['prefix'] = null;
         }
 
-        $matches['suffix'] ??= null;
+        /** @var non-empty-string $start */
+        $start = (string) self::decode($matches['start']);
+        /** @var ?non-empty-string $prefix */
+        $prefix = self::decode($matches['prefix']);
+        /** @var ?non-empty-string $suffix */
+        $suffix = self::decode($matches['suffix'] ?? null);
         $matches['end'] ??= null;
         if ('' === $matches['end']) {
             $matches['end'] = null;
         }
+        /** @var ?non-empty-string $end */
+        $end = self::decode($matches['end']);
 
-        return new self(
-            (string) self::decode($matches['start']),
-            self::decode($matches['end']),
-            self::decode($matches['prefix']),
-            self::decode($matches['suffix']),
-        );
+        return new self($start, $end, $prefix, $suffix);
     }
 
     private static function encode(?string $value): ?string
@@ -85,7 +95,11 @@ private static function encode(?string $value): ?string
 
     private static function decode(?string $value): ?string
     {
-        return null !== $value ? str_replace('%20', ' ', (string) Encoder::decodeFragment($value)) : null;
+        if (null === $value) {
+            return null;
+        }
+
+        return str_replace('%20', ' ', (string) Encoder::decodeFragment($value));
     }
 
     public function name(): string
@@ -162,6 +176,8 @@ public function equals(mixed $directive): bool
      *
      * This method MUST retain the state of the current instance, and return
      * an instance that contains the new start portion.
+     *
+     * @param non-empty-string $text
      */
     public function startsWith(string $text): self
     {
@@ -179,6 +195,8 @@ public function startsWith(string $text): self
      *
      * This method MUST retain the state of the current instance, and return
      * an instance that contains the new end portion.
+     *
+     * @param ?non-empty-string $text
      */
     public function endsWith(?string $text): self
     {
@@ -196,6 +214,8 @@ public function endsWith(?string $text): self
      *
      * This method MUST retain the state of the current instance, and return
      * an instance that contains the new suffix portion.
+     *
+     * @param ?non-empty-string $text
      */
     public function followedBy(?string $text): self
     {
@@ -211,6 +231,8 @@ public function followedBy(?string $text): self
      *
      *  This method MUST retain the state of the current instance, and return
      *  an instance that contains the new prefix portion.
+     *
+     * @param ?non-empty-string $text
      */
     public function precededBy(?string $text): self
     {

components/Components/FragmentDirectives.php

@@ -16,8 +16,7 @@
 use Countable;
 use IteratorAggregate;
 use League\Uri\Components\Directives\Directive;
-use League\Uri\Components\Directives\GenericDirective;
-use League\Uri\Components\Directives\TextDirective;
+use League\Uri\Components\Directives\Factory;
 use League\Uri\Contracts\FragmentInterface;
 use League\Uri\Contracts\UriComponentInterface;
 use League\Uri\Contracts\UriInterface;
@@ -46,12 +45,13 @@
 use function is_string;
 use function sprintf;
 use function str_replace;
-use function strlen;
 use function substr;
 
 use const ARRAY_FILTER_USE_BOTH;
 
 /**
+ * @see https://wicg.github.io/scroll-to-text-fragment/
+ *
  * @implements IteratorAggregate<int, Directive>
  */
 final class FragmentDirectives implements FragmentInterface, IteratorAggregate, Countable
@@ -60,7 +60,7 @@ final class FragmentDirectives implements FragmentInterface, IteratorAggregate,
     public const SEPARATOR = '&';
 
     /** @var list<Directive> */
-    private array $directives;
+    private readonly array $directives;
 
     public function __construct(Directive|Stringable|string ...$directives)
     {
@@ -81,13 +81,7 @@ public static function new(Stringable|string|null $value): self
         $value = (string) $value;
         str_starts_with($value, self::DELIMITER) || throw new SyntaxError('The value "'.$value.'" is not a valid fragment directive.');
 
-        return new self(...array_map(
-            self::filterDirective(...),
-            explode(
-                self::SEPARATOR,
-                substr($value, strlen(self::DELIMITER))
-            )
-        ));
+        return new self(...explode(self::SEPARATOR, substr($value, 3)));
     }
 
     private static function filterDirective(Directive|Stringable|string $directive): Directive
@@ -96,9 +90,7 @@ private static function filterDirective(Directive|Stringable|string $directive):
             return $directive;
         }
 
-        $directive = (string) $directive;
-
-        return str_starts_with($directive, 'text=') ? TextDirective::fromString($directive) : GenericDirective::fromString($directive);
+        return Factory::parse($directive);
     }
 
     public static function tryNew(Stringable|string|null $value): ?self
@@ -293,7 +285,7 @@ public function append(FragmentDirectives|Directive|Stringable|string ...$direct
             return $this;
         }
 
-        return new self(...$this->directives, ...array_map(self::filterDirective(...), $items));
+        return new self(...$this->directives, ...$items);
     }
 
     /**
@@ -314,7 +306,7 @@ public function prepend(FragmentDirectives|Directive|Stringable|string ...$direc
             return $this;
         }
 
-        return new self(...array_map(self::filterDirective(...), $items), ...$this->directives);
+        return new self(...$items, ...$this->directives);
     }
 
     /**

docs/components/7.0/fragment-directives.md

@@ -103,8 +103,12 @@ $fragment->equals($newFragment); //returns false
 
 ## The supported Directives
 
-The package supports the Text Directive and the Generic Directive. Both directives implement
-the `Directive` interface.
+While the `FragmentDirectives` class allows submitting Directives as string, it is highly recommended to
+use a dedicated class to do so, as the grammar around building or parsing directives may be complex in
+regard to encoding characters and/or delimiters usage.
+
+The `FragmentDirectives` class supports the `TextDirective` and the `GenericDirective` classes. Both classes implement
+the following `Directive` interface.
 
 ```php
 Directive::name(): string
@@ -115,10 +119,10 @@ Directive::__toString(): string
 ```
 
 A directive is composed of two parts separated by the `=` separator. The name is required as it
-defines the directive syntax, while its value **MAY** be optional. The `name()` and `value()`
-methods return the **decoded** value of the directive part whereas the `toString()` method
-returns the encoded string representation of the full directive. The `__toString()` method
-is an alias of the `toString()` method.
+defines the directive syntax, while its value **MAY** be optional. When the value is not defined,
+the separator is omitted. The `name()` and `value()` methods return the **decoded** value of
+the directive part whereas the `toString()` method returns its encoded string representation.
+The `__toString()` method is an alias of the `toString()` method.
 
 ### Text Directive
 
@@ -138,6 +142,9 @@ echo $directive->value(); //display "Deprecated-,attributes,attribute,-instead"
 echo $directive;          //display "text=Deprecated-,attributes,attribute,-instead"
 ```
 
+<p class="message-notice">the <code>-</code>, <code>&</code> and <code>,</code> characters
+are special and <strong>must</strong> be encoded if found in the text to avoid parsing errors.</p>
+
 when added in a fragment directive and applied on a webpage the text range which
 starts with `attributes` and ends with `attribute` and which is preceded by
 `Deprecated` and followed by `instead` will be highlighted. Depending on the
@@ -172,8 +179,9 @@ $directive->toString(); // returns "text=john-,y%26lo,bar,-doe"
 
 ### Generic Directive
 
-This directive is marked generic because it has no special effect.
-If can only be instantiated from a directive string representation.
+This directive is considered **generic** because it only meets the minimal syntax requirements of a Directive.
+Unlike the `TextDirective` class, it does not perform any additional parsing or validation around the directive value.
+As a result, it can only be instantiated from a directive’s string representation.
 
 ```php
 use League\Uri\Components\Directives\GenericDirective;
@@ -183,15 +191,12 @@ $directive->value(); //returns "bar"
 $directive->name();  //returns "fo&o"
 ```
 
-This class holds the minimum information needed to generate a `Directive`. It's use case
-is to handle all the other `Directives` as long as they don't have their own specific syntax.
-
+It's use case is to handle all the other `Directives` as long as they don't have their own specific syntax.
 
 ### Directive equality
 
-the `equals()` method allows comparing two directives against their string representation.
-If the string representation is identical then the `equals()` method will return `true`; 
-otherwise `false` will be returned.
+The `equals()` method compares two directives based on their string representations.
+It returns `true` if both representations are identical, and `false` otherwise.
 
 ```php
 use League\Uri\Components\Directives\GenericDirective;