[ticket/17135] Fix Psalm and other issues

PHPBB3-17135

phpBB/config/default/container/services_messenger.yml

@@ -25,6 +25,7 @@ services:
             - '@template.twig.extensions.collection'
             - '@template.twig.lexer'
             - '%core.template.cache_path%'
+            - '%core.php_ext%'
 
     messenger.method.email:
         class: phpbb\messenger\method\phpbb_email

phpBB/phpbb/message/message.php

@@ -245,6 +245,7 @@ class message
 
 		foreach ($this->recipients as $recipient)
 		{
+			/** @psalm-suppress InvalidTemplateParam */
 			$messenger_collection_iterator = $messenger->getIterator();
 			while ($messenger_collection_iterator->valid())
 			{

phpBB/phpbb/messenger/method/base.php

@@ -22,7 +22,6 @@ use phpbb\log\log_interface;
 use phpbb\path_helper;
 use phpbb\request\request;
 use phpbb\messenger\queue;
-use phpbb\template\template;
 use phpbb\template\twig\lexer;
 use phpbb\user;
 
@@ -58,13 +57,16 @@ abstract class base
 	/** @var  path_helper */
 	protected $path_helper;
 
+	/** @var string */
+	protected $root_path;
+
 	/** @var  request */
 	protected $request;
 
 	/** @var string */
 	protected $subject = '';
 
-	/** @var template */
+	/** @var \phpbb\template\template */
 	protected $template;
 
 	/** @var string */
@@ -97,6 +99,7 @@ abstract class base
 	 * @param service_collection $twig_extensions_collection
 	 * @param lexer $twig_lexer
 	 * @param string $template_cache_path
+	 * @param string $phpbb_root_path
 	 */
 	function __construct(
 		config $config,
@@ -110,7 +113,8 @@ abstract class base
 		manager $ext_manager,
 		service_collection $twig_extensions_collection,
 		lexer $twig_lexer,
-		$template_cache_path
+		$template_cache_path,
+		$phpbb_root_path
 	)
 	{
 		$this->config = $config;
@@ -125,6 +129,7 @@ abstract class base
 		$this->twig_extensions_collection = $twig_extensions_collection;
 		$this->twig_lexer = $twig_lexer;
 		$this->template_cache_path = $template_cache_path;
+		$this->root_path = $phpbb_root_path;
 
 		$this->set_use_queue();
 	}
@@ -135,6 +140,12 @@ abstract class base
 	 */
 	abstract public function get_id();
 
+	/**
+	 * Check if the messenger method is enabled
+	 * @return bool
+	 */
+	abstract public function is_enabled();
+
 	/**
 	 * Sets the use of messenger queue flag
 	 *
@@ -361,7 +372,7 @@ abstract class base
 		 *
 		 * @event core.modify_notification_template
 		 * @var	string	subject		The message subject
-		 * @var template 		template	The (readonly) template object
+		 * @var string	template	The (readonly) template object
 		 * @since 3.2.4-RC1
 		 * @changed 4.0.0-a1 Removed vars: method, break.
 		 */
@@ -455,7 +466,7 @@ abstract class base
 	 */
 	protected function setup_template()
 	{
-		if (isset($this->template) && $this->template instanceof template)
+		if (isset($this->template) && $this->template instanceof \phpbb\template\template)
 		{
 			return;
 		}
@@ -487,8 +498,8 @@ abstract class base
 	/**
 	 * Set template paths to load
 	 *
-	 * @param string $path_name Email template path name
-	 * @param string $paths 	Email template paths
+	 * @param string|array $path_name	Email template path name
+	 * @param string|array $paths		Email template paths
 	 * @return void
 	 */
 	protected function set_template_paths($path_name, $paths)

phpBB/phpbb/messenger/method/phpbb_email.php

@@ -17,7 +17,13 @@ use Symfony\Component\Mailer\Transport;
 use Symfony\Component\Mailer\Mailer;
 use Symfony\Component\Mime\Address;
 use Symfony\Component\Mime\Email;
+use Symfony\Component\Mime\Header\DateHeader;
 use Symfony\Component\Mime\Header\Headers;
+use Symfony\Component\Mime\Header\IdentificationHeader;
+use Symfony\Component\Mime\Header\MailboxHeader;
+use Symfony\Component\Mime\Header\MailboxListHeader;
+use Symfony\Component\Mime\Header\PathHeader;
+use Symfony\Component\Mime\Header\UnstructuredHeader;
 
 /**
  * Messenger class
@@ -55,7 +61,10 @@ class phpbb_email extends base
 	 */
 	protected $dsn = '';
 
-	/** @var string */
+	/** @var Email */
+	protected $email;
+
+	/** @var Address */
 	protected $from;
 
 	/** @var Headers */
@@ -73,13 +82,13 @@ class phpbb_email extends base
 	 */
 	protected $mail_priority = Email::PRIORITY_NORMAL;
 
-	/** @var queue */
+	/** @var \phpbb\messenger\queue */
 	protected $queue;
 
-	/** @var string */
-	protected $replyto = '';
+	/** @var Address */
+	protected $replyto;
 
-	/** @var Transport */
+	/** @var \Symfony\Component\Mailer\Transport\AbstractTransport */
 	protected $transport;
 
 	/**
@@ -99,8 +108,7 @@ class phpbb_email extends base
 	}
 
 	/**
-	 * Check if the messenger method is enabled
-	 * @return void
+	 * {@inheritDoc}
 	 */
 	public function is_enabled()
 	{
@@ -114,13 +122,12 @@ class phpbb_email extends base
 	{
 		$this->email = new Email();
 		$this->headers = $this->email->getHeaders();
-		$this->msg = $this->replyto = $this->from = '';
+		$this->subject =  $this->msg = '';
 		$this->mail_priority = Email::PRIORITY_NORMAL;
 
-		$this->subject = $this->additional_headers = [];
-		$this->msg = '';
+		$this->additional_headers = [];
 		$this->use_queue = true;
-		unset($this->template);
+		unset($this->template, $this->replyto, $this->from);
 	}
 
 	/**
@@ -180,7 +187,7 @@ class phpbb_email extends base
 		}
 
 		$cc = new Address($address, trim($realname));
-		$this->email->getCc() ? $this->email->addCc($to) : $this->email->cc($to);
+		$this->email->getCc() ? $this->email->addCc($cc) : $this->email->cc($cc);
 	}
 
 	/**
@@ -198,7 +205,7 @@ class phpbb_email extends base
 		}
 
 		$bcc = new Address($address, trim($realname));
-		$this->email->getBcc() ? $this->email->addBcc($to) : $this->email->bcc($to);
+		$this->email->getBcc() ? $this->email->addBcc($bcc) : $this->email->bcc($bcc);
 	}
 
 	/**
@@ -397,7 +404,7 @@ class phpbb_email extends base
 	/**
 	 * Get Symfony Mailer transport DSN
 	 *
-	 * @return void
+	 * @return string
 	 */
 	public function get_dsn()
 	{
@@ -418,7 +425,7 @@ class phpbb_email extends base
 
 		$this->transport = Transport::fromDsn($this->dsn);
 
-		if ($this->config['smtp_delivery'] && !in_array($this->dsn, ['null://null', 'sendmail://default']))
+		if ($this->config['smtp_delivery'] && method_exists($this->transport, 'getStream'))
 		{
 			// Set ssl context options, see http://php.net/manual/en/context.ssl.php
 			$options['ssl'] = [
@@ -460,8 +467,8 @@ class phpbb_email extends base
 			 * Event to send message via external transport
 			 *
 			 * @event core.notification_message_process
-			 * @var	bool		break	Flag indicating if the function return after hook
-			 * @var	Email		email	The Symfony Email object
+			 * @var	string		break	Flag indicating if the function return after hook
+			 * @var	string		email	The Symfony Email object
 			 * @since 3.2.4-RC1
 			 * @changed 4.0.0-a1 Added vars: email. Removed vars: addresses, subject, msg.
 			 */
@@ -477,25 +484,25 @@ class phpbb_email extends base
 				{
 					$mailer->send($email);
 				}
-				catch (TransportExceptionInterface $e)
+				catch (\Symfony\Component\Mailer\Exception\TransportExceptionInterface $e)
 				{
-					$this->error('EMAIL', $e->getDebug());
+					$this->error($e->getDebug());
 					continue;
 				}
 			}
 		}
 
 		// No more data for this object? Unset it
-		if (!count($this->queue_data[$queue_object_name]['data']))
+		if (!count($queue_data[$queue_object_name]['data']))
 		{
-			unset($this->queue_data[$queue_object_name]);
+			unset($queue_data[$queue_object_name]);
 		}
 	}
 
 	/**
 	 * Get mailer transport object
 	 *
-	 * @return Transport Symfony Mailer transport object
+	 * @return \Symfony\Component\Mailer\Transport\TransportInterface Symfony Mailer transport object
 	 */
 	public function get_transport()
 	{
@@ -526,7 +533,7 @@ class phpbb_email extends base
 		 * @event core.notification_message_email
 		 * @var	string	subject	The message subject
 		 * @var	string	msg		The message text
-		 * @var	Email	email	The Symfony Email object
+		 * @var	string	email	The Symfony Email object
 		 * @since 3.2.4-RC1
 		 * @changed 4.0.0-a1 Added vars: email. Removed vars: addresses, break
 		 */
@@ -567,7 +574,7 @@ class phpbb_email extends base
 			 * Modify data before sending out emails with PHP's mail function
 			 *
 			 * @event core.phpbb_mail_before
-			 * @var	Email	email		The Symfony Email object
+			 * @var	string	email		The Symfony Email object
 			 * @var	string	subject		The message subject
 			 * @var	string	msg			The message text
 			 * @var string	headers		The email headers
@@ -591,7 +598,7 @@ class phpbb_email extends base
 			{
 				$mailer->send($this->email);
 			}
-			catch (TransportExceptionInterface $e)
+			catch (\Symfony\Component\Mailer\Exception\TransportExceptionInterface $e)
 			{
 				$this->error($e->getDebug());
 				return false;
@@ -601,7 +608,7 @@ class phpbb_email extends base
 			 * Execute code after sending out emails with PHP's mail function
 			 *
 			 * @event core.phpbb_mail_after
-			 * @var	Email	email		The Symfony Email object
+			 * @var	string	email		The Symfony Email object
 			 * @var	string	subject		The message subject
 			 * @var	string	msg			The message text
 			 * @var string	headers		The email headers

phpBB/phpbb/messenger/method/phpbb_jabber.php

@@ -29,7 +29,7 @@ class phpbb_jabber extends base
 	/** @var string */
 	protected $connect_server;
 
-	/** @var resource */
+	/** @var resource|null */
 	protected $connection = null;
 
 	/** @var bool */
@@ -47,7 +47,7 @@ class phpbb_jabber extends base
 	/** @var string */
 	protected $password;
 
-	/** @var string */
+	/** @var int */
 	protected $port;
 
 	/** @var string */
@@ -127,8 +127,7 @@ class phpbb_jabber extends base
 	}
 
 	/**
-	 * Check if the messenger method is enabled
-	 * @return void
+	 * {@inheritDoc}
 	 */
 	public function is_enabled()
 	{
@@ -187,10 +186,10 @@ class phpbb_jabber extends base
 	 * Set port to connect to server
 	 * use_ssl flag should be set first
 	 *
-	 * @param string $port Port to connect to server
+	 * @param int $port Port to connect to server
 	 * @return $this
 	 */
-	public function port($port = '')
+	public function port($port = 5222)
 	{
 		$this->port	= ($port) ? $port : 5222;
 
@@ -334,6 +333,7 @@ class phpbb_jabber extends base
 			}
 
 			$this->session = [];
+			/** @psalm-suppress InvalidPropertyAssignmentValue */
 			return fclose($this->connection);
 		}
 
@@ -352,7 +352,7 @@ class phpbb_jabber extends base
 	/**
 	 * Initiates login (using data from contructor, after calling connect())
 	 *
-	 * @return bool
+	 * @return bool|void
 	 */
 	public function login()
 	{
@@ -401,11 +401,8 @@ class phpbb_jabber extends base
 	 */
 	public function reset()
 	{
-		$this->msg = '';
-		$this->to = [];
-
-		$this->subject = $this->additional_headers = [];
-		$this->msg = '';
+		$this->subject = $this->msg = '';
+		$this->additional_headers = $this->to = [];
 		$this->use_queue = true;
 		unset($this->template);
 	}
@@ -438,7 +435,6 @@ class phpbb_jabber extends base
 
 		$package_size = $queue_data[$queue_object_name]['package_size'] ?? 0;
 		$num_items = (!$package_size || $messages_count < $package_size) ? $messages_count : $package_size;
-		$mailer = new Mailer($this->transport);
 
 		for ($i = 0; $i < $num_items; $i++)
 		{
@@ -447,30 +443,30 @@ class phpbb_jabber extends base
 
 			if (!$this->connect())
 			{
-				$this->error('JABBER', $this->user->lang['ERR_JAB_CONNECT'] . '<br />' . $this->get_log());
-				return false;
+				$this->error($this->user->lang['ERR_JAB_CONNECT'] . '<br />' . $this->get_log());
+				return;
 			}
 
 			if (!$this->login())
 			{
-				$this->error('JABBER', $this->user->lang['ERR_JAB_AUTH'] . '<br />' . $this->get_log());
-				return false;
+				$this->error($this->user->lang['ERR_JAB_AUTH'] . '<br />' . $this->get_log());
+				return;
 			}
 
 			foreach ($addresses as $address)
 			{
 				if ($this->send_message($address, $msg, $subject) === false)
 				{
-					$this->error('JABBER', $this->get_log());
+					$this->error($this->get_log());
 					continue;
 				}
 			}
 		}
 
 		// No more data for this object? Unset it
-		if (!count($this->queue_data[$queue_object_name]['data']))
+		if (!count($queue_data[$queue_object_name]['data']))
 		{
-			unset($this->queue_data[$queue_object_name]);
+			unset($queue_data[$queue_object_name]);
 		}
 
 		$this->disconnect();
@@ -500,13 +496,13 @@ class phpbb_jabber extends base
 		{
 			if (!$this->connect())
 			{
-				$this->error('JABBER', $this->user->lang['ERR_JAB_CONNECT'] . '<br />' . $this->get_log());
+				$this->error($this->user->lang['ERR_JAB_CONNECT'] . '<br />' . $this->get_log());
 				return false;
 			}
 
 			if (!$this->login())
 			{
-				$this->error('JABBER', $this->user->lang['ERR_JAB_AUTH'] . '<br />' . $this->get_log());
+				$this->error($this->user->lang['ERR_JAB_AUTH'] . '<br />' . $this->get_log());
 				return false;
 			}
 
@@ -514,7 +510,7 @@ class phpbb_jabber extends base
 			{
 				if ($this->send_message($address, $this->msg, $this->subject) === false)
 				{
-					$this->error('JABBER', $this->get_log());
+					$this->error($this->get_log());
 					continue;
 				}
 			}
@@ -542,7 +538,7 @@ class phpbb_jabber extends base
 	 *
 	 * @param string $xml
 	 *
-	 * @return bool
+	 * @return int|bool
 	 */
 	public function send_xml($xml)
 	{
@@ -655,7 +651,7 @@ class phpbb_jabber extends base
 	/**
 	 * Initiates account registration (based on data used for contructor)
 	 *
-	 * @return bool
+	 * @return bool|void
 	 */
 	public function register()
 	{
@@ -676,7 +672,7 @@ class phpbb_jabber extends base
 	 * @param string	$type			dnd, away, chat, xa or nothing
 	 * @param bool		$unavailable	set this to true if you want to become unavailable
 	 *
-	 * @return bool
+	 * @return int|bool
 	 */
 	function send_presence($message = '', $type = '', $unavailable = false)
 	{
@@ -702,7 +698,7 @@ class phpbb_jabber extends base
 	 *
 	 * @param array $xml
 	 *
-	 * @return bool
+	 * @return bool|void
 	 */
 	function response($xml)
 	{
@@ -1021,7 +1017,7 @@ class phpbb_jabber extends base
 	 * @param string $subject	Message subject
 	 * @param string $type		Message type
 	 *
-	 * @return string
+	 * @return int|bool
 	 */
 	public function send_message($to, $text, $subject = '', $type = 'normal')
 	{

phpBB/phpbb/messenger/queue.php

@@ -35,7 +35,7 @@ class queue
 	/** @var dispatcher */
 	protected $dispatcher;
 
-	/** @var phpbb\filesystem\filesystem_interface */
+	/** @var \phpbb\filesystem\filesystem_interface */
 	protected $filesystem;
 
 	/** @var service_collection */
@@ -117,6 +117,7 @@ class queue
 
 		include($this->cache_file);
 
+		/** @psalm-suppress InvalidTemplateParam */
 		$messenger_collection_iterator = $this->messenger_method_collection->getIterator();
 		while ($messenger_collection_iterator->valid())
 		{

phpBB/phpbb/notification/method/messenger_base.php

@@ -112,6 +112,7 @@ abstract class messenger_base extends \phpbb\notification\method\base
 				continue;
 			}
 
+			/** @psalm-suppress InvalidTemplateParam */
 			$messenger_collection_iterator = $this->messenger->getIterator();
 			while ($messenger_collection_iterator->valid())
 			{

tests/email/email_parsing_test.php

@@ -126,7 +126,7 @@ class phpbb_email_parsing_test extends phpbb_test_case
 		$this->email = new \phpbb\messenger\method\phpbb_email(
 			$config, $dispatcher, $lang, $log, $request, $user, $messenger_queue,
 			$phpbb_path_helper, $extension_manager, $twig_extensions_collection, $twig_lexer,
-			$cache_path
+			$cache_path, $phpbb_root_path
 		);
 		$phpbb_container->set('messenger.method.email', $this->email);