From ba028c223c19ec6bf7b341d85e3b94d1dd3befcd Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tim.dijen@minbzk.nl>
Date: Thu, 15 Aug 2019 16:07:12 +0200
Subject: [PATCH] Fully typehint lib/XHTML/*.php

---
 lib/SimpleSAML/XHTML/IdPDisco.php             | 42 +++++++++----------
 lib/SimpleSAML/XHTML/Template.php             | 30 ++++++-------
 .../XHTML/TemplateControllerInterface.php     |  4 +-
 lib/SimpleSAML/XHTML/TemplateLoader.php       |  6 ++-
 4 files changed, 40 insertions(+), 42 deletions(-)

diff --git a/lib/SimpleSAML/XHTML/IdPDisco.php b/lib/SimpleSAML/XHTML/IdPDisco.php
index b81df6870..32cb60a97 100644
--- a/lib/SimpleSAML/XHTML/IdPDisco.php
+++ b/lib/SimpleSAML/XHTML/IdPDisco.php
@@ -118,10 +118,8 @@ class IdPDisco
      *
      * @throws \Exception If the request is invalid.
      */
-    public function __construct(array $metadataSets, $instance)
+    public function __construct(array $metadataSets, string $instance)
     {
-        Assert::string($instance);
-
         // initialize standard classes
         $this->config = Configuration::getInstance();
         $this->metadata = MetaDataStorageHandler::getMetadataHandler();
@@ -179,7 +177,7 @@ class IdPDisco
      * @param string $message The message which should be logged.
      * @return void
      */
-    protected function log($message)
+    protected function log(string $message): void
     {
         Logger::info('idpDisco.' . $this->instance . ': ' . $message);
     }
@@ -195,7 +193,7 @@ class IdPDisco
      *
      * @return string|null The value of the cookie with the given name, or null if no cookie with that name exists.
      */
-    protected function getCookie($name)
+    protected function getCookie(string $name): ?string
     {
         $prefixedName = 'idpdisco_' . $this->instance . '_' . $name;
         if (array_key_exists($prefixedName, $_COOKIE)) {
@@ -216,7 +214,7 @@ class IdPDisco
      * @param string $value The value of the cookie.
      * @return void
      */
-    protected function setCookie($name, $value)
+    protected function setCookie(string $name, string $value): void
     {
         $prefixedName = 'idpdisco_' . $this->instance . '_' . $name;
 
@@ -242,7 +240,7 @@ class IdPDisco
      *
      * @return string|null The entity id if it is valid, null if not.
      */
-    protected function validateIdP($idp)
+    protected function validateIdP(?string $idp): ?string
     {
         if ($idp === null) {
             return null;
@@ -275,7 +273,7 @@ class IdPDisco
      *
      * @return string|null The entity id of the IdP the user has chosen, or null if the user has made no choice.
      */
-    protected function getSelectedIdP()
+    protected function getSelectedIdP(): ?string
     {
         /* Parameter set from the Extended IdP Metadata Discovery Service Protocol, indicating that the user prefers
          * this IdP.
@@ -311,7 +309,7 @@ class IdPDisco
      *
      * @return string|null The entity id of the IdP the user has saved, or null if the user hasn't saved any choice.
      */
-    protected function getSavedIdP()
+    protected function getSavedIdP(): ?string
     {
         if (!$this->config->getBoolean('idpdisco.enableremember', false)) {
             // saving of IdP choices is disabled
@@ -337,7 +335,7 @@ class IdPDisco
      *
      * @return string|null The entity id of the previous IdP the user used, or null if this is the first time.
      */
-    protected function getPreviousIdP()
+    protected function getPreviousIdP(): ?string
     {
         return $this->validateIdP($this->getCookie('lastidp'));
     }
@@ -348,7 +346,7 @@ class IdPDisco
      *
      * @return string|null  The entity ID of the IdP if one is found, or null if not.
      */
-    protected function getFromCIDRhint()
+    protected function getFromCIDRhint(): ?string
     {
         foreach ($this->metadataSets as $metadataSet) {
             $idp = $this->metadata->getPreferredEntityIdFromCIDRhint($metadataSet, $_SERVER['REMOTE_ADDR']);
@@ -369,7 +367,7 @@ class IdPDisco
      *
      * @return string|null The entity id of the IdP the user should most likely use.
      */
-    protected function getRecommendedIdP()
+    protected function getRecommendedIdP(): ?string
     {
         $idp = $this->getPreviousIdP();
         if ($idp !== null) {
@@ -394,10 +392,8 @@ class IdPDisco
      * @param string $idp The entityID of the IdP.
      * @return void
      */
-    protected function setPreviousIdP($idp)
+    protected function setPreviousIdP(string $idp): void
     {
-        Assert::string($idp);
-
         $this->log('Choice made [' . $idp . '] Setting cookie.');
         $this->setCookie('lastidp', $idp);
     }
@@ -408,7 +404,7 @@ class IdPDisco
      *
      * @return boolean True if the choice should be saved, false otherwise.
      */
-    protected function saveIdP()
+    protected function saveIdP(): bool
     {
         if (!$this->config->getBoolean('idpdisco.enableremember', false)) {
             // saving of IdP choices is disabled
@@ -428,7 +424,7 @@ class IdPDisco
      *
      * @return string|null The entity id of the IdP the user should be sent to, or null if the user should choose.
      */
-    protected function getTargetIdP()
+    protected function getTargetIdP(): ?string
     {
         // first, check if the user has chosen an IdP
         $idp = $this->getSelectedIdP();
@@ -464,7 +460,7 @@ class IdPDisco
      *
      * @return array An array with entityid => metadata mappings.
      */
-    protected function getIdPList()
+    protected function getIdPList(): array
     {
         $idpList = [];
         foreach ($this->metadataSets as $metadataSet) {
@@ -485,7 +481,7 @@ class IdPDisco
      *
      * @return array An array of IdP entities
      */
-    protected function getScopedIDPList()
+    protected function getScopedIDPList(): array
     {
         return $this->scopedIDPList;
     }
@@ -501,7 +497,7 @@ class IdPDisco
      *
      * @return array An associative array containing metadata for the IdPs that were not filtered out.
      */
-    protected function filterList($list)
+    protected function filterList(array $list): array
     {
         foreach ($list as $entity => $metadata) {
             if (array_key_exists('hide.from.discovery', $metadata) && $metadata['hide.from.discovery'] === true) {
@@ -517,7 +513,7 @@ class IdPDisco
      *
      * @return void If there is no IdP targeted and this is not a passive request.
      */
-    protected function start()
+    protected function start(): void
     {
         $idp = $this->getTargetIdP();
         if ($idp !== null) {
@@ -553,7 +549,7 @@ class IdPDisco
      * The IdP disco parameters should be set before calling this function.
      * @return void
      */
-    public function handleRequest()
+    public function handleRequest(): void
     {
         $this->start();
 
@@ -654,7 +650,7 @@ class IdPDisco
      * @param string $language
      * @return string|null
      */
-    private function getEntityDisplayName(array $idpData, string $language)
+    private function getEntityDisplayName(array $idpData, string $language): ?string
     {
         if (isset($idpData['UIInfo']['DisplayName'][$language])) {
             return $idpData['UIInfo']['DisplayName'][$language];
diff --git a/lib/SimpleSAML/XHTML/Template.php b/lib/SimpleSAML/XHTML/Template.php
index b2404b826..86224427d 100644
--- a/lib/SimpleSAML/XHTML/Template.php
+++ b/lib/SimpleSAML/XHTML/Template.php
@@ -92,7 +92,6 @@ class Template extends Response
      */
     private $useNewUI = false;
 
-
     /**
      * A template controller, if any.
      *
@@ -104,7 +103,6 @@ class Template extends Response
      */
     private $controller = null;
 
-
     /**
      * Whether we are using a non-default theme or not.
      *
@@ -116,6 +114,7 @@ class Template extends Response
      */
     private $theme = ['module' => null, 'name' => 'default'];
 
+
     /**
      * Constructor
      *
@@ -123,7 +122,7 @@ class Template extends Response
      * @param string                   $template Which template file to load
      * @param string|null              $defaultDictionary The default dictionary where tags will come from.
      */
-    public function __construct(Configuration $configuration, $template, $defaultDictionary = null)
+    public function __construct(Configuration $configuration, string $template, string $defaultDictionary = null)
     {
         $this->configuration = $configuration;
         $this->template = $template;
@@ -172,7 +171,7 @@ class Template extends Response
      * @param string|null $module
      * @return string
      */
-    public function asset($asset, $module = null)
+    public function asset(string $asset, string $module = null): string
     {
         $baseDir = $this->configuration->getBaseDir();
         if (is_null($module)) {
@@ -204,7 +203,7 @@ class Template extends Response
      *
      * @return string The name of the template to use.
      */
-    public function getTemplateName()
+    public function getTemplateName() : string
     {
         return $this->normalizeTemplateName($this->template);
     }
@@ -356,6 +355,7 @@ class Template extends Response
         return $twig;
     }
 
+
     /**
      * Add overriding templates from the configured theme.
      *
@@ -425,7 +425,7 @@ class Template extends Response
      * @throws \InvalidArgumentException If the module is not enabled or it has no templates directory.
      * @return void
      */
-    public function addTemplatesFromModule($module)
+    public function addTemplatesFromModule(string $module): void
     {
         $dir = TemplateLoader::getModuleTemplateDir($module);
         /** @var \Twig\Loader\FilesystemLoader $loader */
@@ -440,7 +440,7 @@ class Template extends Response
      *
      * @return array|null The array containing information of all available languages.
      */
-    private function generateLanguageBar()
+    private function generateLanguageBar(): ?array
     {
         $languages = $this->translator->getLanguage()->getLanguageList();
         ksort($languages);
@@ -472,7 +472,7 @@ class Template extends Response
      * Set some default context
      * @return void
      */
-    private function twigDefaultContext()
+    private function twigDefaultContext(): void
     {
         // show language bar by default
         if (!isset($this->data['hideLanguageBar'])) {
@@ -509,7 +509,7 @@ class Template extends Response
      * @return string The HTML rendered by this template, as a string.
      * @throws \Exception if the template cannot be found.
      */
-    protected function getContents()
+    protected function getContents(): string
     {
         $this->twigDefaultContext();
         if ($this->controller) {
@@ -529,7 +529,7 @@ class Template extends Response
      * @return Response This response.
      * @throws \Exception if the template cannot be found.
      */
-    public function send()
+    public function send(): Response
     {
         $this->content = $this->getContents();
         return parent::send();
@@ -566,7 +566,7 @@ class Template extends Response
      *
      * @throws \Exception If the template file couldn't be found.
      */
-    private function findTemplatePath(string $template, bool $throw_exception = true)
+    private function findTemplatePath(string $template, bool $throw_exception = true): ?string
     {
         $extensions = ['.tpl.php', '.php'];
 
@@ -637,7 +637,7 @@ class Template extends Response
      *
      * @return \SimpleSAML\Locale\Translate The translator that will be used with this template.
      */
-    public function getTranslator()
+    public function getTranslator(): Translate
     {
         return $this->translator;
     }
@@ -648,7 +648,7 @@ class Template extends Response
      *
      * @return \SimpleSAML\Locale\Localization The localization object that will be used with this template.
      */
-    public function getLocalization()
+    public function getLocalization(): Localization
     {
         return $this->localization;
     }
@@ -659,7 +659,7 @@ class Template extends Response
      *
      * @return \Twig\Environment The Twig instance in use, or null if Twig is not used.
      */
-    public function getTwig()
+    public function getTwig(): \Twig\Environment
     {
         return $this->twig;
     }
@@ -670,7 +670,7 @@ class Template extends Response
      *
      * @return array
      */
-    private function getLanguageList()
+    private function getLanguageList(): array
     {
         return $this->translator->getLanguage()->getLanguageList();
     }
diff --git a/lib/SimpleSAML/XHTML/TemplateControllerInterface.php b/lib/SimpleSAML/XHTML/TemplateControllerInterface.php
index 68e1a07f9..e72027f0d 100644
--- a/lib/SimpleSAML/XHTML/TemplateControllerInterface.php
+++ b/lib/SimpleSAML/XHTML/TemplateControllerInterface.php
@@ -21,7 +21,7 @@ interface TemplateControllerInterface
      *
      * @return void
      */
-    public function setUpTwig(Environment &$twig);
+    public function setUpTwig(Environment &$twig) : void;
 
 
     /**
@@ -33,5 +33,5 @@ interface TemplateControllerInterface
      *
      * @return void
      */
-    public function display(&$data);
+    public function display(array &$data) : void;
 }
diff --git a/lib/SimpleSAML/XHTML/TemplateLoader.php b/lib/SimpleSAML/XHTML/TemplateLoader.php
index 4b6034b95..5430a0273 100644
--- a/lib/SimpleSAML/XHTML/TemplateLoader.php
+++ b/lib/SimpleSAML/XHTML/TemplateLoader.php
@@ -24,6 +24,8 @@ class TemplateLoader extends \Twig\Loader\FilesystemLoader
      * @param string $name
      * @param bool $throw
      * @return string|false|null
+     *
+     * NOTE: cannot typehint due to upstream restrictions
      */
     protected function findTemplate($name, $throw = true)
     {
@@ -44,7 +46,7 @@ class TemplateLoader extends \Twig\Loader\FilesystemLoader
      * @return array An array with the corresponding namespace and name of the template. The namespace defaults to
      * \Twig\Loader\FilesystemLoader::MAIN_NAMESPACE, if none was specified in $name.
      */
-    protected function parseModuleName($name, $default = self::MAIN_NAMESPACE)
+    protected function parseModuleName(string $name, string $default = self::MAIN_NAMESPACE) : array
     {
         if (strpos($name, ':')) {
             // we have our old SSP format
@@ -67,7 +69,7 @@ class TemplateLoader extends \Twig\Loader\FilesystemLoader
      *
      * @throws \InvalidArgumentException If the module is not enabled or it has no templates directory.
      */
-    public static function getModuleTemplateDir($module)
+    public static function getModuleTemplateDir(string $module) : string
     {
         if (!Module::isModuleEnabled($module)) {
             throw new \InvalidArgumentException('The module \'' . $module . '\' is not enabled.');
-- 
GitLab