Merge pull request #137 from City-of-Helsinki/UHF-X-use-symfony-files…

…ystem

Use symfony filesystem to write logs, run phpstan, make sure JsonLog message is a string

.github/workflows/ci.yml

@@ -65,8 +65,11 @@ jobs:
 
       - name: Run PHPCS
         working-directory: ${{ env.DRUPAL_ROOT }}
-        run: |
-          vendor/bin/phpcs $MODULE_FOLDER --standard=Drupal --extensions=php,module,inc,install,test,info
+        run: vendor/bin/phpcs $MODULE_FOLDER --standard=Drupal,DrupalPractice --extensions=php,module,install
+
+      - name: Run phpstan
+        working-directory: ${{ env.DRUPAL_ROOT }}
+        run: vendor/bin/phpstan analyze -c $MODULE_FOLDER/phpstan.neon $MODULE_FOLDER
 
       - name: Start services
         working-directory: ${{ env.DRUPAL_ROOT }}
@@ -90,6 +93,6 @@ jobs:
         with:
           name: results
           path: |
-            ${{ env.DRUPAL_ROOT }}/sites/simpletest/browser_output/
+            ${{ env.DRUPAL_ROOT }}/public/sites/simpletest/browser_output/
             ${{ env.DRUPAL_ROOT }}/coverage.xml
           retention-days: 1

helfi_api_base.services.yml

@@ -73,10 +73,15 @@ services:
     tags:
       - { name: event_subscriber }
 
+  helfi_api_base.logger_filesystem:
+    class: Symfony\Component\Filesystem\Filesystem
+    public: false
+
   helfi_api_base.logger_json:
     class: Drupal\helfi_api_base\Logger\JsonLog
     arguments:
       - '@logger.log_message_parser'
+      - '@helfi_api_base.logger_filesystem'
       - '%helfi_api_base.json_logger_path%'
       - '%helfi_api_base.logger_enabled%'
     tags:

phpstan.neon

@@ -0,0 +1,20 @@
+parameters:
+  fileExtensions:
+      - php
+      - module
+      - install
+  paths:
+    - ./
+  excludePaths:
+    - vendor
+  level: 3
+  checkMissingIterableValueType: false
+  treatPhpDocTypesAsCertain: false
+  ignoreErrors:
+    -
+      message: '#^Method Symfony\\Contracts\\EventDispatcher\\EventDispatcherInterface::dispatch\(\) invoked with 2 parameters, 1 required.#'
+      path: src/Commands/DeployCommands.php
+      count: 2
+    -
+      message: '#^Unsafe usage of new static#'
+      path: src/Plugin/migrate/source/HttpSourcePluginBase.php

src/Azure/PubSub/PubSubManager.php

@@ -118,23 +118,22 @@ public function setTimeout(int $timeout) : self {
    *
    * This is used to exit early if required settings are not populated.
    */
-  private function assertSettings() : self {
+  private function assertSettings() : void {
     $vars = get_object_vars($this->settings);
 
     foreach ($vars as $key => $value) {
       if (empty($this->settings->{$key})) {
         throw new ConnectionException("Azure PubSub '$key' is not configured.");
       }
     }
-    return $this;
   }
 
   /**
    * {@inheritdoc}
    */
   public function sendMessage(array $message) : self {
-    $this->assertSettings()
-      ->joinGroup();
+    $this->assertSettings();
+    $this->joinGroup();
 
     $this->client
       ->text(
@@ -155,8 +154,9 @@ public function sendMessage(array $message) : self {
    * {@inheritdoc}
    */
   public function receive() : string {
-    $this->assertSettings()
-      ->joinGroup();
+    $this->assertSettings();
+    $this->joinGroup();
+
     $message = (string) $this->client->receive();
     $json = $this->decodeMessage($message);
 

src/Azure/PubSub/SettingsFactory.php

@@ -29,7 +29,7 @@ public function __construct(
    *   The PubSub settings object.
    */
   public function create() : Settings {
-    $data = (object) [
+    $data = [
       'hub' => '',
       'group' => '',
       'endpoint' => '',
@@ -41,9 +41,10 @@ public function create() : Settings {
         if (!isset($settings->data()->{$key})) {
           continue;
         }
-        $data->{$key} = $settings->data()->{$key};
+        $data[$key] = $settings->data()->{$key};
       }
     }
+    $data = (object) $data;
 
     return new Settings(
       $data->hub ?: '',

src/Entity/Form/MenuLinkFormTrait.php

@@ -4,6 +4,7 @@
 
 namespace Drupal\helfi_api_base\Entity\Form;
 
+use Drupal\Core\Entity\FieldableEntityInterface;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Menu\MenuParentFormSelectorInterface;
 use Drupal\Core\Site\Settings;
@@ -13,8 +14,8 @@
 /**
  * A trait to allow entity forms to provide a menu link form.
  *
- * To use this, call the ::attachMenuLinkForm() method in your
- * ::buildForm() method, like:
+ * To use this, call the '::attachMenuLinkForm()' method in your
+ * '::buildForm()' method, like:
  *
  * @code
  * $form = $this->attachMenuLinkForm($form, $form_state);
@@ -48,6 +49,7 @@ trait MenuLinkFormTrait {
    */
   protected function getDefaultMenuLink() : MenuLinkContentInterface {
     $entity = $this->getEntity();
+    assert($entity instanceof FieldableEntityInterface);
 
     if (!$menuLink = $entity->get($this->menuLinkFieldName)->entity) {
       $storage = $this->entityTypeManager
@@ -64,7 +66,10 @@ protected function getDefaultMenuLink() : MenuLinkContentInterface {
       $menuLink = empty($results) ? MenuLinkContent::create([]) : MenuLinkContent::load(reset($results));
     }
 
-    return $this->entityRepository->getTranslationFromContext($menuLink);
+    $entity = $this->entityRepository->getTranslationFromContext($menuLink);
+    assert($entity instanceof MenuLinkContentInterface);
+
+    return $entity;
   }
 
   /**
@@ -173,6 +178,7 @@ protected function attachMenuLinkForm(array $form, FormStateInterface $formState
    */
   public function attachMenuLinkFormSubmit(array $form, FormStateInterface $formState) : void {
     $entity = $this->getEntity();
+    assert($entity instanceof FieldableEntityInterface);
     /** @var \Drupal\menu_link_content\MenuLinkContentInterface $menuLink */
     $menuLink = $formState->get('menu_link');
 

src/Entity/Form/RevisionRevertTranslationForm.php

@@ -5,8 +5,11 @@
 namespace Drupal\helfi_api_base\Entity\Form;
 
 use Drupal\Core\Entity\RevisionableInterface;
+use Drupal\Core\Entity\RevisionLogInterface;
+use Drupal\Core\Entity\TranslatableRevisionableInterface;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Language\LanguageManagerInterface;
+use Drupal\Core\StringTranslation\TranslatableMarkup;
 use Drupal\entity\Form\RevisionRevertForm;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\Request;
@@ -33,7 +36,7 @@ final class RevisionRevertTranslationForm extends RevisionRevertForm {
   /**
    * {@inheritdoc}
    */
-  public static function create(ContainerInterface $container) {
+  public static function create(ContainerInterface $container) : self {
     $instance = parent::create($container);
     $instance->languageManager = $container->get('language_manager');
     return $instance;
@@ -42,14 +45,15 @@ public static function create(ContainerInterface $container) {
   /**
    * {@inheritdoc}
    */
-  public function getFormId() {
+  public function getFormId() : string {
     return 'revision_revert_translation_confirm';
   }
 
   /**
    * {@inheritdoc}
    */
-  public function getQuestion() {
+  public function getQuestion() : TranslatableMarkup {
+    assert($this->revision instanceof RevisionLogInterface);
     return $this->t('Are you sure you want to revert @language translation to the revision from %revision-date?', [
       '@language' => $this->languageManager->getLanguageName($this->langcode),
       '%revision-date' => $this->dateFormatter->format($this->revision->getRevisionCreationTime()),
@@ -59,14 +63,14 @@ public function getQuestion() {
   /**
    * {@inheritdoc}
    */
-  public function getDescription() {
-    return '';
+  public function getDescription() : TranslatableMarkup {
+    return $this->t('This action cannot be undone.');
   }
 
   /**
    * {@inheritdoc}
    */
-  public function buildForm(array $form, FormStateInterface $form_state, $_entity_revision = NULL, Request $request = NULL) {
+  public function buildForm(array $form, FormStateInterface $form_state, $_entity_revision = NULL, Request $request = NULL) : array {
     $this->langcode = $request->attributes->get('langcode');
 
     return parent::buildForm($form, $form_state, $_entity_revision);
@@ -75,7 +79,8 @@ public function buildForm(array $form, FormStateInterface $form_state, $_entity_
   /**
    * {@inheritdoc}
    */
-  protected function prepareRevision(RevisionableInterface $revision) {
+  protected function prepareRevision(RevisionableInterface $revision) : RevisionableInterface {
+    assert($revision instanceof TranslatableRevisionableInterface);
     $translation = $revision->getTranslation($this->langcode);
     return parent::prepareRevision($translation);
   }

src/Entity/Routing/RevisionRouteProvider.php

@@ -10,7 +10,7 @@
 use Symfony\Component\Routing\Route;
 
 /**
- * Providers extended revision routes for content entikties.
+ * Providers extended revision routes for content entities.
  */
 final class RevisionRouteProvider extends EntityRevisionRouteProvider {
 
@@ -31,11 +31,12 @@ public function getRoutes(EntityTypeInterface $entity_type) {
   /**
    * {@inheritdoc}
    */
-  protected function getRevisionHistoryRoute(EntityTypeInterface $entity_type) {
+  protected function getRevisionHistoryRoute(EntityTypeInterface $entity_type): ?Route {
     if ($route = parent::getRevisionHistoryRoute($entity_type)) {
       $route->setDefault('_controller', RevisionController::class . '::revisionOverviewController');
       return $route;
     }
+    return NULL;
   }
 
   /**

src/EventSubscriber/MigrationSubscriber.php

@@ -116,7 +116,9 @@ public function onPostImport(MigrateImportEvent $event) : void {
       ->execute();
 
     foreach ($results as $id) {
-      $storage->load($id)->delete(TRUE);
+      $entity = $storage->load($id);
+      assert($entity instanceof RemoteEntityBase);
+      $entity->delete(TRUE);
     }
   }
 

src/Logger/JsonLog.php

@@ -9,6 +9,7 @@
 use Drupal\Core\Logger\RfcLogLevel;
 use Drupal\Core\StringTranslation\TranslatableMarkup;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Filesystem\Filesystem;
 
 /**
  * This class allows logging to stdout and stderr.
@@ -22,28 +23,27 @@ final class JsonLog implements LoggerInterface {
    *
    * @param \Drupal\Core\Logger\LogMessageParserInterface $parser
    *   The parser to use when extracting message variables.
+   * @param \Symfony\Component\Filesystem\Filesystem $filesystem
+   *   The file system.
    * @param string $stream
    *   The output path.
    * @param bool $loggerEnabled
    *   Whether logger is enabled or not.
    */
   public function __construct(
-    private LogMessageParserInterface $parser,
-    private string $stream,
-    private bool $loggerEnabled = TRUE
+    private readonly LogMessageParserInterface $parser,
+    private readonly Filesystem $filesystem,
+    private readonly string $stream,
+    private readonly bool $loggerEnabled = TRUE
   ) {
   }
 
   /**
-   * Outputs the given entry.
-   *
-   * @param array $entry
-   *   The log entry.
+   * Outputs the messages.
    */
-  private function output(array $entry) : void {
-    $stream = fopen($this->stream, 'a');
-    fwrite($stream, json_encode(['message' => $entry]) . PHP_EOL);
-    fclose($stream);
+  private function output(array $message) : void {
+    $this->filesystem
+      ->appendToFile($this->stream, json_encode(['message' => $message]) . PHP_EOL);
   }
 
   /**
@@ -61,7 +61,7 @@ public function log($level, $message, array $context = []) : void {
     }
     // Populate the message placeholders and then replace them in the message.
     $variables = $this->parser->parseMessagePlaceholders($message, $context);
-    $message = empty($variables) ? $message : strtr($message, $variables);
+    $message = empty($variables) ? $message : strtr((string) $message, $variables);
 
     $this->output([
       'base_url'    => $base_url,

src/Plugin/Action/MigrationUpdateAction.php

@@ -25,7 +25,7 @@
  *   deriver = "Drupal\helfi_api_base\Plugin\Derivative\MigrationUpdateActionDerivative",
  * )
  */
-class MigrationUpdateAction extends ActionBase implements ContainerFactoryPluginInterface {
+final class MigrationUpdateAction extends ActionBase implements ContainerFactoryPluginInterface {
 
   use MigrateTrait;
 
@@ -51,8 +51,8 @@ public static function create(
     array $configuration,
     $plugin_id,
     $plugin_definition
-  ) {
-    $instance = new static($configuration, $plugin_definition, $plugin_definition);
+  ) : self {
+    $instance = new self($configuration, $plugin_definition, $plugin_definition);
     $instance->migrationPluginManager = $container->get('plugin.manager.migration');
     $instance->entityTypeManager = $container->get('entity_type.manager');
 

src/Plugin/DebugDataItem/Composer.php

@@ -19,7 +19,7 @@
  *   description = @Translation("Composer")
  * )
  */
-class Composer extends DebugDataItemPluginBase implements ContainerFactoryPluginInterface {
+final class Composer extends DebugDataItemPluginBase implements ContainerFactoryPluginInterface {
 
   /**
    * The composer info.
@@ -38,8 +38,8 @@ class Composer extends DebugDataItemPluginBase implements ContainerFactoryPlugin
   /**
    * {@inheritdoc}
    */
-  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
-    $instance = new static($configuration, $plugin_id, $plugin_definition);
+  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) : self {
+    $instance = new self($configuration, $plugin_id, $plugin_definition);
     $instance->composerInfo = $container->get('helfi_api_base.composer_info');
     return $instance;
   }

src/Plugin/DebugDataItem/MaintenanceMode.php

@@ -18,7 +18,7 @@
  *   description = @Translation("Maintenance mode")
  * )
  */
-class MaintenanceMode extends DebugDataItemPluginBase implements ContainerFactoryPluginInterface {
+final class MaintenanceMode extends DebugDataItemPluginBase implements ContainerFactoryPluginInterface {
 
   /**
    * The state service.
@@ -30,8 +30,8 @@ class MaintenanceMode extends DebugDataItemPluginBase implements ContainerFactor
   /**
    * {@inheritdoc}
    */
-  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
-    $instance = new static($configuration, $plugin_id, $plugin_definition);
+  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) : self {
+    $instance = new self($configuration, $plugin_id, $plugin_definition);
     $instance->state = $container->get('state');
     return $instance;
   }