ITKDev: Code review changes

Co-authored-by: Mikkel Ricky <[email protected]>

Author: cableman

README.md

@@ -47,7 +47,7 @@ See [OS2Web git name convention](https://github.com/OS2Web/docs#git-guideline)
 // CVR lookup
 /** @var \Drupal\os2web_datalookup\Plugin\DataLookupManager $pluginManager */
 $pluginManager = \Drupal::service('plugin.manager.os2web_datalookup');
-/** @var \Drupal\os2web_datalookup\Plugin\os2web\DataLookup\DataLookupInterfaceCompany $cvrPlugin */
+/** @var \Drupal\os2web_datalookup\Plugin\os2web\DataLookup\DataLookupCompanyInterface $cvrPlugin */
 $cvrPlugin = $pluginManager->createDefaultInstanceByGroup('cvr_lookup');
 
 if ($cvrPlugin->isReady()) {
@@ -57,7 +57,7 @@ if ($cvrPlugin->isReady()) {
 // CPR lookup.
 /** @var \Drupal\os2web_datalookup\Plugin\DataLookupManager $pluginManager */
 $pluginManager = \Drupal::service('plugin.manager.os2web_datalookup');
-/** @var \Drupal\os2web_datalookup\Plugin\os2web\DataLookup\DataLookupInterfaceCpr $cprPlugin */
+/** @var \Drupal\os2web_datalookup\Plugin\os2web\DataLookup\DataLookupCprInterface $cprPlugin */
 $cprPlugin = $pluginManager->createDefaultInstanceByGroup('cpr_lookup');
 
 if ($cprPlugin->isReady()) {

composer.json

@@ -19,7 +19,7 @@
         }
     },
     "require-dev": {
-        "dealerdirect/phpcodesniffer-composer-installer": "^0.7.1",
+        "dealerdirect/phpcodesniffer-composer-installer": "^1.0",
         "drupal/coder": "^8.3",
         "mglaman/phpstan-drupal": "^1.1",
         "phpstan/extension-installer": "^1.3",

os2web_datalookup.install

@@ -1,11 +1,16 @@
 <?php
 
+/**
+ * @file
+ * Install, uninstall and update hooks for the module.
+ */
+
 use Drupal\os2web_datalookup\Form\DataLookupPluginGroupSettingsForm;
 
 /**
  * Setting "serviceplatformen_cpr_extended" as default CPR lookup plugin.
  */
-function os2web_datalookup_update_9001() {
+function os2web_datalookup_update_9001(): void {
   $config = \Drupal::service('config.factory')->getEditable(DataLookupPluginGroupSettingsForm::$configName);
   $config->set("cpr_lookup.default_plugin", 'serviceplatformen_cpr_extended');
   $config->save();
@@ -14,7 +19,7 @@ function os2web_datalookup_update_9001() {
 /**
  * Setting "datafordeler_cvr" as default CVR lookup plugin.
  */
-function os2web_datalookup_update_9002() {
+function os2web_datalookup_update_9002(): void {
   $config = \Drupal::service('config.factory')->getEditable(DataLookupPluginGroupSettingsForm::$configName);
   $config->set("cvr_lookup.default_plugin", 'datafordeler_cvr');
   $config->save();
@@ -23,7 +28,7 @@ function os2web_datalookup_update_9002() {
 /**
  * Setting "datafordeler_pnumber" as default P-Number lookup plugin.
  */
-function os2web_datalookup_update_9003() {
+function os2web_datalookup_update_9003(): void {
   $config = \Drupal::service('config.factory')->getEditable(DataLookupPluginGroupSettingsForm::$configName);
   $config->set("pnumber_lookup.default_plugin", 'datafordeler_pnumber');
   $config->save();

phpcs.xml.dist

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <ruleset name="PHP_CodeSniffer">
-  <description>OS2web Audit PHP Code Sniffer configuration</description>
+  <description>OS2web Datalookup PHP Code Sniffer configuration</description>
 
   <file>.</file>
   <exclude-pattern>vendor/</exclude-pattern>

phpstan.neon

@@ -8,21 +8,6 @@ parameters:
     - '*/node_modules/*'
   ignoreErrors:
     # This is how drupal works....
-    - '#Unsafe usage of new static\(\).#'
-    - '#getEditableConfigNames\(\) return type has no value type specified in iterable type array#'
-    - '#buildForm\(\) has parameter \$form with no value type specified in iterable type array.#'
-    - '#buildForm\(\) return type has no value type specified in iterable type array.#'
-    - '#validateForm\(\) has parameter \$form with no value type specified in iterable type array.#'
-    - '#submitForm\(\) has parameter \$form with no value type specified in iterable type array.#'
-    - '#getDerivativeDefinitions\(\) has parameter \$base_plugin_definition with no value type specified in iterable type array.#'
-    - '#getDerivativeDefinitions\(\) return type has no value type specified in iterable type array.#'
-    - '#LoggerManager::__construct\(\) has parameter \$namespaces with no value type specified in iterable type Traversable.#'
-    - '#__construct\(\) has parameter \$configuration with no value type specified in iterable type array.#'
-    - '#getConfiguration\(\) return type has no value type specified in iterable type array.#'
-    - '#setConfiguration\(\) has parameter \$configuration with no value type specified in iterable type array.#'
-    - '#defaultConfiguration\(\) return type has no value type specified in iterable type array.#'
-    - '#buildConfigurationForm\(\) has parameter \$form with no value type specified in iterable type array.#'
-    - '#buildConfigurationForm\(\) return type has no value type specified in iterable type array.#'
-    - '#validateConfigurationForm\(\) has parameter \$form with no value type specified in iterable type array.#'
-    - '#submitConfigurationForm\(\) has parameter \$form with no value type specified in iterable type array.#'
-    - '#getForm\(\) invoked with 2 parameters, 1 required.#'
+    - '#\Drupal calls should be avoided in classes, use dependency injection instead#'
+    - '#The text '@deprecated use lookup\(\) instead.' does not match the standard format: @deprecated in %deprecation-version% and is#'
+    - '#Each @deprecated tag must have a @see tag immediately following it#'

src/Annotation/DataLookup.php

@@ -3,6 +3,7 @@
 namespace Drupal\os2web_datalookup\Annotation;
 
 use Drupal\Component\Annotation\Plugin;
+use Drupal\Core\Annotation\Translation;
 
 /**
  * Defines a AuthProvider annotation object.
@@ -22,7 +23,7 @@ class DataLookup extends Plugin {
    *
    * @var string
    */
-  public $id;
+  public string $id;
 
   /**
    * The human-readable name of the consent storage.
@@ -31,24 +32,24 @@ class DataLookup extends Plugin {
    *
    * @ingroup plugin_translatable
    */
-  public $label;
+  public Translation $label;
 
   /**
    * A brief description of the consent storage.
    *
    * This will be shown when adding or configuring this consent storage.
    *
-   * @var \Drupal\Core\Annotation\Translation
+   * @var \Drupal\Core\Annotation\Translation|string
    *
    * @ingroup plugin_translatable
    */
-  public $description = '';
+  public Translation|string $description = '';
 
   /**
-   * Group of the plugin lookup plugin.
+   * Group of the plugin lookup plugins.
    *
    * @var string
    */
-  public $group = '';
+  public string $group = '';
 
 }

src/Controller/DatalookupController.php

@@ -5,11 +5,10 @@
 use Drupal\Component\Plugin\PluginManagerInterface;
 use Drupal\Core\Controller\ControllerBase;
 use Drupal\Core\Link;
-use Drupal\os2web_datalookup\Plugin\os2web\DataLookup\DataLookupInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
- * Class DatalookupController.
+ * Data lookup controller.
  *
  * @package Drupal\os2web_datalookup\Controller
  */
@@ -20,10 +19,13 @@ class DatalookupController extends ControllerBase {
    *
    * @var \Drupal\Component\Plugin\PluginManagerInterface
    */
-  protected $manager;
+  protected PluginManagerInterface $manager;
 
   /**
-   * {@inheritdoc}
+   * Default constructor.
+   *
+   * @param \Drupal\Component\Plugin\PluginManagerInterface $manager
+   *   The plugin manger.
    */
   public function __construct(PluginManagerInterface $manager) {
     $this->manager = $manager;
@@ -40,8 +42,13 @@ public static function create(ContainerInterface $container) {
 
   /**
    * Status list callback.
+   *
+   * @return array<string, mixed>
+   *   An render array.
+   *
+   * @throws \Drupal\Component\Plugin\Exception\PluginException
    */
-  public function statusList() {
+  public function statusList(): array {
     $headers = [
       'title' => $this
         ->t('Title'),
@@ -55,7 +62,7 @@ public function statusList() {
 
     $rows = [];
     foreach ($this->manager->getDefinitions() as $id => $plugin_definition) {
-      /** @var DataLookupInterface $plugin */
+      /** @var \Drupal\os2web_datalookup\Plugin\os2web\DataLookup\DataLookupInterface $plugin */
       $plugin = $this->manager->createInstance($id);
       $status = $plugin->getStatus();
       $rows[$id] = [

src/Form/DataLookupPluginGroupSettingsForm.php

@@ -6,6 +6,7 @@
 use Drupal\Core\Form\ConfigFormBase;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Link;
+use Drupal\os2web_datalookup\Plugin\DataLookupManager;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -18,16 +19,20 @@ class DataLookupPluginGroupSettingsForm extends ConfigFormBase {
    *
    * @var string
    */
-  public static $configName = 'os2web_datalookup.groups.settings';
+  public static string $configName = 'os2web_datalookup.groups.settings';
 
-  /** @var \Drupal\os2web_datalookup\Plugin\DataLookupManager */
-  protected $dataLookupManager;
+  /**
+   * Data lookup manager.
+   *
+   * @var \Drupal\os2web_datalookup\Plugin\DataLookupManager
+   */
+  protected DataLookupManager $dataLookupManager;
 
   /**
    * Constructs a new DataLookupPluginGroupSettingsForm object.
    *
-   * @param \Drupal\os2web_datalookup\Plugin\DataLookupManager
-   *   Datalookup manager.
+   * @param \Drupal\os2web_datalookup\Plugin\DataLookupManager $dataLookupManager
+   *   Data lookup manager.
    */
   public function __construct(PluginManagerInterface $dataLookupManager) {
     $this->dataLookupManager = $dataLookupManager;
@@ -36,7 +41,7 @@ public function __construct(PluginManagerInterface $dataLookupManager) {
   /**
    * {@inheritdoc}
    */
-  public static function create(ContainerInterface $container) {
+  public static function create(ContainerInterface $container): static {
     return new static(
       $container->get('plugin.manager.os2web_datalookup')
     );
@@ -52,7 +57,7 @@ public function getFormId() {
   /**
    * {@inheritdoc}
    */
-  protected function getEditableConfigNames() {
+  protected function getEditableConfigNames(): array {
     return [DataLookupPluginGroupSettingsForm::$configName];
   }
 
@@ -69,7 +74,7 @@ protected function getGroupIdFromRequest() {
   /**
    * {@inheritdoc}
    */
-  public function buildForm(array $form, FormStateInterface $form_state) {
+  public function buildForm(array $form, FormStateInterface $form_state): array {
     $group_id = $this->getGroupIdFromRequest();
 
     $plugin_definitions = $this->dataLookupManager->getDefinitionsByGroup($group_id);
@@ -114,7 +119,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   /**
    * {@inheritdoc}
    */
-  public function submitForm(array &$form, FormStateInterface $form_state) {
+  public function submitForm(array &$form, FormStateInterface $form_state): void {
     $group_id = $this->getGroupIdFromRequest();
 
     $config = $this->config(DataLookupPluginGroupSettingsForm::$configName);

src/Form/DataLookupPluginSettingsForm.php

@@ -22,7 +22,7 @@ public function __construct(ConfigFactoryInterface $config_factory, PluginManage
   /**
    * {@inheritdoc}
    */
-  public static function create(ContainerInterface $container) {
+  public static function create(ContainerInterface $container): static {
     return new static(
       $container->get('config.factory'),
       $container->get('plugin.manager.os2web_datalookup')
@@ -32,7 +32,7 @@ public static function create(ContainerInterface $container) {
   /**
    * {@inheritdoc}
    */
-  public static function getConfigName() {
+  public static function getConfigName(): string {
     return 'os2web_datalookup';
   }
 

src/Form/PluginSettingsFormBase.php

@@ -7,6 +7,7 @@
  * Abstract class for PluginSettingsForm implementation.
  */
 
+use Drupal\Component\Plugin\PluginManagerInterface;
 use Drupal\Core\Form\ConfigFormBase;
 use Drupal\Core\Form\FormStateInterface;
 
@@ -20,36 +21,37 @@ abstract class PluginSettingsFormBase extends ConfigFormBase implements PluginSe
    *
    * @var \Drupal\Component\Plugin\PluginManagerInterface
    */
-  protected $manager;
+  protected PluginManagerInterface $manager;
 
   /**
    * {@inheritdoc}
    */
-  protected function getEditableConfigNames() {
+  protected function getEditableConfigNames(): array {
     return [$this->getConfigId()];
   }
 
   /**
    * {@inheritdoc}
    */
-  public function getFormId() {
+  public function getFormId(): string {
     return $this->getConfigName() . '_settings_form_' . $this->getPluginIdFromRequest();
   }
 
   /**
    * {@inheritdoc}
    */
-  public function buildForm(array $form, FormStateInterface $form_state) {
+  public function buildForm(array $form, FormStateInterface $form_state): array {
     $plugin_id = $this->getPluginIdFromRequest();
     $instance = $this->getPluginInstance($plugin_id);
     $form = $instance->buildConfigurationForm($form, $form_state);
+
     return parent::buildForm($form, $form_state);
   }
 
   /**
    * {@inheritdoc}
    */
-  public function validateForm(array &$form, FormStateInterface $form_state) {
+  public function validateForm(array &$form, FormStateInterface $form_state): void {
     $plugin_id = $this->getPluginIdFromRequest();
     $instance = $this->getPluginInstance($plugin_id);
     $instance->validateConfigurationForm($form, $form_state);
@@ -58,7 +60,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
   /**
    * {@inheritdoc}
    */
-  public function submitForm(array &$form, FormStateInterface $form_state) {
+  public function submitForm(array &$form, FormStateInterface $form_state): void {
     $plugin_id = $this->getPluginIdFromRequest();
     $instance = $this->getPluginInstance($plugin_id);
     $instance->submitConfigurationForm($form, $form_state);
@@ -77,6 +79,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    */
   protected function getPluginIdFromRequest() {
     $request = $this->getRequest();
+
     return $request->get('_plugin_id');
   }
 
@@ -89,11 +92,12 @@ protected function getPluginIdFromRequest() {
    * @return object
    *   Plugin instance.
    *
-   * @throws PluginException
+   * @throws \Drupal\Component\Plugin\Exception\PluginException
    */
-  public function getPluginInstance($plugin_id) {
+  public function getPluginInstance($plugin_id): object {
     $configuration = $this->config($this->getConfigId())->get();
     $instance = $this->manager->createInstance($plugin_id, $configuration);
+
     return $instance;
   }
 
@@ -103,7 +107,7 @@ public function getPluginInstance($plugin_id) {
    * @return string
    *   Configuration object name.
    */
-  protected function getConfigId() {
+  protected function getConfigId(): string {
     return $this->getConfigName() . '.' . $this->getPluginIdFromRequest();
   }