code refactor - low risk

composer.json

@@ -27,11 +27,12 @@
         }
     },
     "require": {
-        "php": "~8.2.0 || ~8.3.0 || ~8.4.0 || ~8.5.0",
+        "php": "~8.3.0 || ~8.4.0 || ~8.5.0 || ~8.6.0",
         "ext-fileinfo": "*",
         "ext-json": "*",
         "dotkernel/dot-event": "^4.0",
         "laminas/laminas-servicemanager": "^3.22 || ^4.0",
+        "laminas/laminas-stdlib": "^3.19",
         "symfony/mailer": "^v7.1.6"
     },
     "require-dev": {

src/Email.php

@@ -398,7 +398,9 @@ private function createAddresses(array $addresses, ?string $name = null): array
     {
         $createdAddresses = [];
         foreach ($addresses as $address) {
-            $createdAddresses[] = new Address($address, $name ?? '');
+            $createdAddresses[] = $address instanceof Address
+                ? $address
+                : new Address($address, $name ?? '');
         }
 
         return $createdAddresses;

src/Event/MailEventListenerAwareTrait.php

@@ -6,6 +6,8 @@
 
 use Laminas\EventManager\EventManagerAwareTrait;
 
+use function array_search;
+
 trait MailEventListenerAwareTrait
 {
     use EventManagerAwareTrait;
@@ -22,16 +24,10 @@ public function detachListener(MailEventListenerInterface $listener): void
     {
         $listener->detach($this->getEventManager());
 
-        $idx = 0;
-        foreach ($this->listeners as $l) {
-            if ($l === $listener) {
-                break;
-            }
-
-            $idx++;
+        $idx = array_search($listener, $this->listeners, true);
+        if ($idx !== false) {
+            unset($this->listeners[$idx]);
         }
-
-        unset($this->listeners[$idx]);
     }
 
     public function clearListeners(): void

src/Factory/AbstractMailFactory.php

@@ -31,7 +31,7 @@ public function canCreate(ContainerInterface $container, $requestedName): bool
     {
         $parts = explode('.', $requestedName);
 
-        if ($parts[0] !== self::DOT_MAIL_PART || $parts[1] !== static::SPECIFIC_PART || count($parts) !== 3) {
+        if (count($parts) !== 3 || $parts[0] !== self::DOT_MAIL_PART || $parts[1] !== static::SPECIFIC_PART) {
             return false;
         }
 

src/Factory/MailServiceAbstractFactory.php

@@ -156,7 +156,7 @@ protected function createTransport(ContainerInterface $container): TransportInte
                 return $this->setupTransportConfig($transport);
             } else {
                 throw new InvalidArgumentException(
-                    'Provided mail_adapter service does not return a ' . TransportInterface::class . ' instance'
+                    'Provided transport service does not return a ' . TransportInterface::class . ' instance'
                 );
             }
         }
@@ -169,9 +169,9 @@ protected function createTransport(ContainerInterface $container): TransportInte
         //the adapter is not valid - throw exception
         throw new InvalidArgumentException(
             sprintf(
-                'mail_adapter must be an instance of %s or string, "%s" provided',
+                '"transport" must be a known alias, a container service or a class implementing %s; "%s" provided',
                 TransportInterface::class,
-                gettype($adapter)
+                $adapter
             )
         );
     }
@@ -230,7 +230,8 @@ protected function getListenerObject(ContainerInterface $container, string $type
 
         if (! $listener instanceof MailEventListenerInterface) {
             throw new RuntimeException(sprintf(
-                'Mail event listener must be an instance of `%s`, but `%s was given`',
+                'Mail event listener "%s" must be an instance of `%s`, but `%s` was given',
+                $type,
                 MailEventListenerInterface::class,
                 is_object($listener) ? $listener::class : gettype($listener)
             ));

src/Options/MailOptions.php

@@ -46,9 +46,9 @@ public function getTransport(): string|TransportInterface
 
     public function setTransport(string|TransportInterface $transport): void
     {
-        if (is_string($transport) && array_key_exists(strtolower($transport), $this->transportMap)) {
-            $transport = $this->transportMap[$transport];
-            foreach ($transport as $class) {
+        $key = is_string($transport) ? strtolower($transport) : null;
+        if ($key !== null && array_key_exists($key, $this->transportMap)) {
+            foreach ($this->transportMap[$key] as $class) {
                 if (class_exists($class)) {
                     $transport = $class;
                     break;

src/Options/SmtpOptions.php

@@ -84,7 +84,7 @@ public function setPort(int $port): static
     {
         if ($port < 1) {
             throw new InvalidArgumentException(sprintf(
-                'Port must be greater than 1; received "%d"',
+                'Port must be greater than 0; received "%d"',
                 $port
             ));
         }

test/EmailTest.php

@@ -66,4 +66,23 @@ public function testEnsureValidity(): void
         $this->expectException(LogicException::class);
         $subject->ensureValidity();
     }
+
+    public function testAddressObjectsInArraysAreKept(): void
+    {
+        $subject = new Email();
+
+        $subject->addTo([new Address('first@example.com', 'First'), 'second@example.com'], 'Ignored');
+        $subject->setCc([new Address('cc@example.com', 'Copy')]);
+
+        $to = $subject->getTo();
+        $this->assertCount(2, $to);
+        $this->assertSame('first@example.com', $to[0]->getAddress());
+        $this->assertSame('First', $to[0]->getName());
+        $this->assertSame('second@example.com', $to[1]->getAddress());
+        $this->assertSame('Ignored', $to[1]->getName());
+
+        $cc = $subject->getCc();
+        $this->assertCount(1, $cc);
+        $this->assertSame('Copy', $cc[0]->getName());
+    }
 }

test/Event/MailEventListenerTest.php

@@ -43,4 +43,23 @@ public function testListenerDetachingAndClearing(): void
         $listeners = $listenersProperty->getValue($trait);
         $this->assertEmpty($listeners);
     }
+
+    /**
+     * @throws Exception
+     * @throws ReflectionException
+     */
+    public function testDetachingUnknownListenerKeepsOthers(): void
+    {
+        $trait    = new class {
+            use MailEventListenerAwareTrait;
+        };
+        $attached = $this->createMock(MailEventListenerInterface::class);
+        $unknown  = $this->createMock(MailEventListenerInterface::class);
+
+        $trait->attachListener($attached);
+        $trait->detachListener($unknown);
+
+        $listenersProperty = (new ReflectionObject($trait))->getProperty('listeners');
+        $this->assertSame([$attached], $listenersProperty->getValue($trait));
+    }
 }

test/Factory/AbstractMailFactoryTest.php

@@ -7,13 +7,17 @@
 use Dot\Mail\Factory\AbstractMailFactory;
 use Dot\Mail\Options\MailOptions;
 use DotTest\Mail\CommonTrait;
+use ErrorException;
 use org\bovigo\vfs\vfsStream;
 use PHPUnit\Framework\MockObject\Exception;
 use PHPUnit\Framework\TestCase;
 use Psr\Container\ContainerExceptionInterface;
 use Psr\Container\ContainerInterface;
 use Psr\Container\NotFoundExceptionInterface;
 
+use function restore_error_handler;
+use function set_error_handler;
+
 class AbstractMailFactoryTest extends TestCase
 {
     use CommonTrait;
@@ -77,4 +81,25 @@ public function testWillNotCreateRequestedNameFromWrongParts(): void
         $result = $this->subject->canCreate($container, $requestedName);
         $this->assertFalse($result);
     }
+
+    /**
+     * @throws Exception
+     * @throws ContainerExceptionInterface
+     * @throws NotFoundExceptionInterface
+     */
+    public function testWillNotCreateRequestedNameWithoutParts(): void
+    {
+        $container = $this->createMock(ContainerInterface::class);
+
+        set_error_handler(static function (int $severity, string $message): never {
+            throw new ErrorException($message, 0, $severity);
+        });
+
+        try {
+            $this->assertFalse($this->subject->canCreate($container, 'dot-mail'));
+            $this->assertFalse($this->subject->canCreate($container, 'dot-mail.testPart'));
+        } finally {
+            restore_error_handler();
+        }
+    }
 }

test/Factory/MailServiceAbstractFactoryTest.php

@@ -196,6 +196,7 @@ public function testGenerateServiceWithoutProvidedAdapter(): void
             ->willReturn($this->smtpOptions);
 
         $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage('Mail event listener "Invalid Listener Test" must be an instance of');
 
         $mailService = ($this->subject)($this->container, $requestedName);
 
@@ -226,6 +227,7 @@ public function testGenerateServiceInvalidAdapter(): void
             ]);
 
         $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessageMatches('/"transport" must be a known alias.*"test" provided/');
         ($this->subject)($this->container, $requestedName);
     }
 

test/Options/MailOptionsTest.php

@@ -9,6 +9,7 @@
 use Dot\Mail\Options\MessageOptions;
 use Dot\Mail\Options\SmtpOptions;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\Mailer\Transport\SendmailTransport;
 use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport;
 
 class MailOptionsTest extends TestCase
@@ -35,4 +36,18 @@ public function testGettersAndSetters(): void
         $this->assertContainsOnlyInstancesOf(SmtpOptions::class, [$subject->getSmtpOptions()]);
         $this->assertSame($eventListeners, $subject->getEventListeners());
     }
+
+    public function testTransportAliasIsCaseInsensitive(): void
+    {
+        $subject = new MailOptions();
+
+        $subject->setTransport('ESMTP');
+        $this->assertSame(EsmtpTransport::class, $subject->getTransport());
+
+        $subject->setTransport('SendMail');
+        $this->assertSame(SendmailTransport::class, $subject->getTransport());
+
+        $subject->setTransport('Custom\\Transport');
+        $this->assertSame('Custom\\Transport', $subject->getTransport());
+    }
 }

test/Options/SmtpOptionsTest.php

@@ -29,4 +29,16 @@ public function testSmtpGettersAndSetters(): void
         $this->expectException(InvalidArgumentException::class);
         $subject->setPort(0);
     }
+
+    public function testPortBoundaries(): void
+    {
+        $subject = new SmtpOptions();
+
+        $subject->setPort(1);
+        $this->assertSame(1, $subject->getPort());
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Port must be greater than 0; received "0"');
+        $subject->setPort(0);
+    }
 }