From 80552a8d20eec5b746f7616da7bc119073c5f95c Mon Sep 17 00:00:00 2001 From: Mikkel Ricky Date: Wed, 1 May 2024 12:49:43 +0200 Subject: [PATCH] Defined coding standards and applied in select places --- .markdownlintrc | 19 +++++++ README.md | 50 ++++++++++++++++-- composer.json | 40 ++++++++++++-- phpcs.xml.dist | 22 ++++++++ phpstan.neon | 13 +++++ scripts/code-analysis | 47 +++++++++++++++++ src/Exception/RuntimeException.php | 6 ++- .../os2web/DataLookup/DataLookupBase.php | 29 ++++++----- .../os2web/DataLookup/DatafordelerBase.php | 52 +++++++++---------- .../os2web/DataLookup/DatafordelerCVR.php | 4 +- .../os2web/DataLookup/DatafordelerPNumber.php | 4 +- .../DataLookup/ServiceplatformenBase.php | 28 +++++----- .../DataLookup/ServiceplatformenCPR.php | 4 +- .../ServiceplatformenCPRExtended.php | 2 +- 14 files changed, 248 insertions(+), 72 deletions(-) create mode 100644 .markdownlintrc create mode 100644 phpcs.xml.dist create mode 100644 phpstan.neon create mode 100755 scripts/code-analysis diff --git a/.markdownlintrc b/.markdownlintrc new file mode 100644 index 0000000..be08e16 --- /dev/null +++ b/.markdownlintrc @@ -0,0 +1,19 @@ +{ + // @see https://github.com/DavidAnson/markdownlint/blob/main/schema/.markdownlint.jsonc + // https://github.com/DavidAnson/markdownlint/blob/main/doc/md013.md + "MD013": { + // Exclude code blocks + "code_blocks": false, + "line_length": 120 + }, + + // Prevent complaining on duplicated headings in CHANGELOG.md + // https://github.com/DavidAnson/markdownlint/blob/main/doc/md024.md + "MD024": { + "siblings_only": true + } +} + +// Local Variables: +// mode: json +// End: diff --git a/README.md b/README.md index bde9d52..87ccae9 100644 --- a/README.md +++ b/README.md @@ -1,22 +1,26 @@ # OS2Web Data lookup [![Build Status](https://travis-ci.org/OS2web/os2web_datalookup.svg?branch=8.x)](https://travis-ci.org/OS2web/os2web_datalookup) + ## Install OS2Web Data lookup provides integration with Danish data lookup services such as Service platformen or Datafordeler. Module is available to download via composer. -``` + +```shell composer require os2web/os2web_datalookup drush en os2web_datalookup ``` ## Update + Updating process for OS2Web Data lookup module is similar to usual Drupal 8 module. Use Composer's built-in command for listing packages that have updates available: -``` +```shell composer outdated os2web/os2web_datalookup ``` ## Automated testing and code quality + See [OS2Web testing and CI information](https://github.com/OS2Web/docs#testing-and-ci) ## Contribution @@ -28,14 +32,16 @@ For issue description there is expected that you will provide clear and sufficient information about your feature request or bug report. ### Code review policy + See [OS2Web code review policy](https://github.com/OS2Web/docs#code-review) ### Git name convention + See [OS2Web git name convention](https://github.com/OS2Web/docs#git-guideline) ### Using services in other modules -``` +```php // CVR lookup /** @var \Drupal\os2web_datalookup\Plugin\DataLookupManager $pluginManager */ $pluginManager = \Drupal::service('plugin.manager.os2web_datalookup'); @@ -66,7 +72,7 @@ if ($cprPlugin->isReady()) { ## New services/features -### Datafordeler integration (https://datafordeler.dk) +### Datafordeler integration () In scope of os2forms project already implemented light integration with Danmarks Adresseregister (DAR) via fetching data for form elements @@ -76,7 +82,9 @@ As soon as it is clear how the integration is going to be used, then os2forms_dawa will be refactored to OS2Web Data lookup plugin plugin. ## Important notes + ### Serviceplatformen plugins + Settings for CPR and CVR serviceplantormen plugins are storing as configuration in db and will(could) be exported as `yml` file via Drupal configuration management system. And afterwards could be tracked by `git`. @@ -87,3 +95,37 @@ will be exposed for third persons. To avoid/prevent this behavior we recommend use `Config ignore` module, where you can add all settings you do not want to export/import via configuration management system. + +## Coding standards + +Our coding are checked by GitHub Actions (cf. +[.github/workflows/pr.yml](.github/workflows/pr.yml)). Use the commands below to +run the checks locally. + +### PHP + +```shell +docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer install +# Fix (some) coding standards issues +docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-apply +# Check that code adheres to the coding standards +docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-check +``` + +### Markdown + +```shell +docker run --rm --volume $PWD:/md peterdavehello/markdownlint markdownlint --ignore vendor --ignore LICENSE.md '**/*.md' --fix +docker run --rm --volume $PWD:/md peterdavehello/markdownlint markdownlint --ignore vendor --ignore LICENSE.md '**/*.md' +``` + +## Code analysis + +We use [PHPStan](https://phpstan.org/) for static code analysis. + +Running statis code analysis on a standalone Drupal module is a bit tricky, so we use a helper script to run the +analysis: + +```shell +docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm ./scripts/code-analysis +``` diff --git a/composer.json b/composer.json index 2204b03..2250a9d 100644 --- a/composer.json +++ b/composer.json @@ -5,10 +5,6 @@ "minimum-stability": "dev", "prefer-stable": true, "license": "EUPL-1.2", - "require": { - "ext-soap": "*", - "os2web/os2web_key": "dev-os2web_key" - }, "repositories": { "os2web/os2web_key": { "type": "vcs", @@ -22,5 +18,41 @@ "type": "composer", "url": "https://asset-packagist.org" } + }, + "require": { + "ext-soap": "*", + "os2web/os2web_key": "dev-os2web_key" + }, + "require-dev": { + "dealerdirect/phpcodesniffer-composer-installer": "*", + "drupal/coder": "*", + "mglaman/phpstan-drupal": "*", + "phpstan/extension-installer": "*", + "phpstan/phpstan-deprecation-rules": "*" + }, + "scripts": { + "coding-standards-check/phpcs": [ + "phpcs --standard=phpcs.xml.dist" + ], + "coding-standards-check": [ + "@coding-standards-check/phpcs" + ], + "coding-standards-apply/phpcs": [ + "phpcbf --standard=phpcs.xml.dist" + ], + "coding-standards-apply": [ + "@coding-standards-apply/phpcs" + ] + }, + "config": { + "sort-packages": true, + "allow-plugins": { + "cweagans/composer-patches": true, + "dealerdirect/phpcodesniffer-composer-installer": true, + "phpstan/extension-installer": true, + "simplesamlphp/composer-module-installer": true, + "vaimo/composer-patches": true, + "zaporylie/composer-drupal-optimizations": true + } } } diff --git a/phpcs.xml.dist b/phpcs.xml.dist new file mode 100644 index 0000000..191cd5a --- /dev/null +++ b/phpcs.xml.dist @@ -0,0 +1,22 @@ + + + OS2Forms PHP Code Sniffer configuration + + . + vendor/ + node_modules/ + + + + + + + + + + + + + + + diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..e51b69f --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,13 @@ +parameters: + level: 6 + paths: + - src + + ignoreErrors: + - "#Method [a-zA-Z0-9\\_\\\\:\\(\\)]+ has parameter \\$[a-zA-Z0-9_]+ with no value type specified in iterable type array#" + - "#Method [a-zA-Z0-9\\_\\\\:\\(\\)]+ return type has no value type specified in iterable type array#" + - '#Unsafe usage of new static\(\).#' + +# Local Variables: +# mode: yaml +# End: diff --git a/scripts/code-analysis b/scripts/code-analysis new file mode 100755 index 0000000..73ec750 --- /dev/null +++ b/scripts/code-analysis @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +script_dir=$(pwd) +module_name=$(basename "$script_dir") +drupal_dir=vendor/drupal-module-code-analysis +# Relative to $drupal_dir +module_path=web/modules/contrib/$module_name + +cd "$script_dir" || exit + +drupal_composer() { + composer --working-dir="$drupal_dir" --no-interaction "$@" +} + +# # Create new Drupal 9 project +# if [ ! -f "$drupal_dir/composer.json" ]; then +# composer --no-interaction create-project drupal/recommended-project:^9 "$drupal_dir" +# fi +# # Copy our code into the modules folder + +# # Clean up +# rm -fr "${drupal_dir:?}/$module_path" + +# # https://stackoverflow.com/a/15373763 +# # rsync --archive --compress . --filter=':- .gitignore' --exclude "$drupal_dir" --exclude .git "$drupal_dir/$module_path" + +# # The rsync command in not available in itkdev/php8.1-fpm + +# git config --global --add safe.directory /app +# # Copy module files into module path +# for f in $(git ls-files); do +# mkdir -p "$drupal_dir/$module_path/$(dirname "$f")" +# cp "$f" "$drupal_dir/$module_path/$f" +# done + +# drupal_composer config minimum-stability dev + +# # Allow ALL plugins +# # https://getcomposer.org/doc/06-config.md#allow-plugins +# drupal_composer config --no-plugins allow-plugins true + +# drupal_composer require wikimedia/composer-merge-plugin +# drupal_composer config extra.merge-plugin.include "$module_path/composer.json" +# # https://www.drupal.org/project/drupal/issues/3220043#comment-14845434 +# drupal_composer require --dev symfony/phpunit-bridge + +# Run PHPStan +(cd "$drupal_dir" && vendor/bin/phpstan --configuration="$module_path/phpstan.neon") diff --git a/src/Exception/RuntimeException.php b/src/Exception/RuntimeException.php index f1b163c..0dd1e03 100644 --- a/src/Exception/RuntimeException.php +++ b/src/Exception/RuntimeException.php @@ -2,7 +2,9 @@ namespace Drupal\os2web_datalookup\Exception; -class RuntimeException extends \RuntimeException -{ +/** + * + */ +class RuntimeException extends \RuntimeException { } diff --git a/src/Plugin/os2web/DataLookup/DataLookupBase.php b/src/Plugin/os2web/DataLookup/DataLookupBase.php index 7fe97aa..c8505ba 100644 --- a/src/Plugin/os2web/DataLookup/DataLookupBase.php +++ b/src/Plugin/os2web/DataLookup/DataLookupBase.php @@ -44,20 +44,25 @@ abstract class DataLookupBase extends PluginBase implements ContainerFactoryPlug /** * {@inheritdoc} */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, + public function __construct( + array $configuration, + $plugin_id, + $plugin_definition, protected KeyRepositoryInterface $keyRepository, - protected FileSystem $fileSystem + protected FileSystem $fileSystem, ) { parent::__construct($configuration, $plugin_id, $plugin_definition); $this->setConfiguration($configuration); $this->init(); } - public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) - { - /** @var KeyRepositoryInterface $keyRepository */ + /** + * + */ + public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { + /** @var \Drupal\key\KeyRepositoryInterface $keyRepository */ $keyRepository = $container->get('key.repository'); - /** @var FileSystem $fileSystem */ + /** @var \Drupal\Core\File\FileSystem $fileSystem */ $fileSystem = $container->get('file_system'); return new static( @@ -72,11 +77,10 @@ public static function create(ContainerInterface $container, array $configuratio /** * Plugin init method. */ - protected function init() - { + protected function init() { } - /** + /** * {@inheritdoc} */ public function label() { @@ -152,7 +156,7 @@ protected function getCertificate(): string { * The local certificate path. */ protected function createLocalCertPath(): string { - $this->localCertPath = $this->fileSystem->getTempDirectory().'/'.uniqid('os2web_datalookup_local_cert_'); + $this->localCertPath = $this->fileSystem->getTempDirectory() . '/' . uniqid('os2web_datalookup_local_cert_'); return $this->localCertPath; } @@ -161,10 +165,9 @@ protected function createLocalCertPath(): string { * Write certificate to temporary certificate file. * * @return string - * The local certificate path. + * The local certificate path. */ - protected function writeCertificateToFile(): string - { + protected function writeCertificateToFile(): string { // Write certificate to local_cert location. $certificate = $this->getCertificate(); $localCertPath = $this->localCertPath; diff --git a/src/Plugin/os2web/DataLookup/DatafordelerBase.php b/src/Plugin/os2web/DataLookup/DatafordelerBase.php index fc875df..70aff91 100644 --- a/src/Plugin/os2web/DataLookup/DatafordelerBase.php +++ b/src/Plugin/os2web/DataLookup/DatafordelerBase.php @@ -2,7 +2,6 @@ namespace Drupal\os2web_datalookup\Plugin\os2web\DataLookup; -use Drupal\Core\File\FileSystem; use Drupal\Core\Form\FormStateInterface; use Drupal\os2web_datalookup\Exception\RuntimeException; use GuzzleHttp\Client; @@ -16,19 +15,18 @@ abstract class DatafordelerBase extends DataLookupBase { /** * The client. * - * @var Client + * @var \GuzzleHttp\Client */ private $httpClient; /** * {@inheritdoc} */ - public function defaultConfiguration() - { + public function defaultConfiguration() { return [ - 'cert_path_live' => '', - 'cert_passphrase_live' => '', - ] + parent::defaultConfiguration(); + 'cert_path_live' => '', + 'cert_passphrase_live' => '', + ] + parent::defaultConfiguration(); } /** @@ -107,25 +105,25 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta ], ], - 'cert_path_live' => [ - '#type' => 'textfield', - '#title' => $this->t('Certificate (LIVE)'), - '#description' => $this->t('Path to the certificate'), - '#default_value' => $this->configuration['cert_path_live'], - '#states' => [ - 'required' => [':input[name="certificate_provider"]' => ['value' => self::PROVIDER_TYPE_FORM]], - 'visible' => [':input[name="certificate_provider"]' => ['value' => self::PROVIDER_TYPE_FORM]], - ], - ], - - 'cert_passphrase_live' => [ - '#type' => 'password', - '#title' => $this->t('Certificate passphrase (LIVE)'), - '#description' => $this->t('leave empty if not used'), - '#default_value' => $this->configuration['cert_passphrase_live'], - '#states' => [ - 'visible' => [':input[name="certificate_provider"]' => ['value' => self::PROVIDER_TYPE_FORM]], + 'cert_path_live' => [ + '#type' => 'textfield', + '#title' => $this->t('Certificate (LIVE)'), + '#description' => $this->t('Path to the certificate'), + '#default_value' => $this->configuration['cert_path_live'], + '#states' => [ + 'required' => [':input[name="certificate_provider"]' => ['value' => self::PROVIDER_TYPE_FORM]], + 'visible' => [':input[name="certificate_provider"]' => ['value' => self::PROVIDER_TYPE_FORM]], + ], ], + + 'cert_passphrase_live' => [ + '#type' => 'password', + '#title' => $this->t('Certificate passphrase (LIVE)'), + '#description' => $this->t('leave empty if not used'), + '#default_value' => $this->configuration['cert_passphrase_live'], + '#states' => [ + 'visible' => [':input[name="certificate_provider"]' => ['value' => self::PROVIDER_TYPE_FORM]], + ], ], ]; @@ -151,8 +149,7 @@ public function submitConfigurationForm(array &$form, FormStateInterface $form_s /** * Get response. */ - protected function getResponse(string $uri, array $options): ResponseInterface - { + protected function getResponse(string $uri, array $options): ResponseInterface { try { $localCertPath = $this->writeCertificateToFile(); @@ -184,4 +181,5 @@ protected function getCertificate(): string { return file_get_contents($filename); } + } diff --git a/src/Plugin/os2web/DataLookup/DatafordelerCVR.php b/src/Plugin/os2web/DataLookup/DatafordelerCVR.php index 56bcac8..1fc0198 100644 --- a/src/Plugin/os2web/DataLookup/DatafordelerCVR.php +++ b/src/Plugin/os2web/DataLookup/DatafordelerCVR.php @@ -24,8 +24,8 @@ class DatafordelerCVR extends DatafordelerBase implements DataLookupInterfaceCom */ public function defaultConfiguration() { return [ - 'webserviceurl_live' => 'https://s5-certservices.datafordeler.dk/CVR/HentCVRData/1/REST/', - ] + parent::defaultConfiguration(); + 'webserviceurl_live' => 'https://s5-certservices.datafordeler.dk/CVR/HentCVRData/1/REST/', + ] + parent::defaultConfiguration(); } /** diff --git a/src/Plugin/os2web/DataLookup/DatafordelerPNumber.php b/src/Plugin/os2web/DataLookup/DatafordelerPNumber.php index 28debb8..3b078f2 100644 --- a/src/Plugin/os2web/DataLookup/DatafordelerPNumber.php +++ b/src/Plugin/os2web/DataLookup/DatafordelerPNumber.php @@ -24,8 +24,8 @@ class DatafordelerPNumber extends DatafordelerBase implements DataLookupInterfac */ public function defaultConfiguration() { return [ - 'webserviceurl_live' => 'https://s5-certservices.datafordeler.dk/CVR/HentCVRData/1/REST/', - ] + parent::defaultConfiguration(); + 'webserviceurl_live' => 'https://s5-certservices.datafordeler.dk/CVR/HentCVRData/1/REST/', + ] + parent::defaultConfiguration(); } /** diff --git a/src/Plugin/os2web/DataLookup/ServiceplatformenBase.php b/src/Plugin/os2web/DataLookup/ServiceplatformenBase.php index cad7869..6ee0896 100644 --- a/src/Plugin/os2web/DataLookup/ServiceplatformenBase.php +++ b/src/Plugin/os2web/DataLookup/ServiceplatformenBase.php @@ -2,9 +2,7 @@ namespace Drupal\os2web_datalookup\Plugin\os2web\DataLookup; -use Drupal\Core\File\FileSystem; use Drupal\Core\Form\FormStateInterface; -use Drupal\key\KeyRepositoryInterface; use Drupal\os2web_datalookup\Exception\RuntimeException; /** @@ -31,19 +29,19 @@ abstract class ServiceplatformenBase extends DataLookupBase { */ public function defaultConfiguration() { return [ - 'mode_selector' => 0, - 'serviceagreementuuid' => '', - 'serviceuuid' => '', - 'wsdl' => '', - 'location' => '', - 'location_test' => '', - 'usersystemuuid' => '', - 'useruuid' => '', - 'accountinginfo' => '', - 'certfile_passphrase' => '', - 'certfile' => '', - 'certfile_test' => '', - ] + parent::defaultConfiguration(); + 'mode_selector' => 0, + 'serviceagreementuuid' => '', + 'serviceuuid' => '', + 'wsdl' => '', + 'location' => '', + 'location_test' => '', + 'usersystemuuid' => '', + 'useruuid' => '', + 'accountinginfo' => '', + 'certfile_passphrase' => '', + 'certfile' => '', + 'certfile_test' => '', + ] + parent::defaultConfiguration(); } /** diff --git a/src/Plugin/os2web/DataLookup/ServiceplatformenCPR.php b/src/Plugin/os2web/DataLookup/ServiceplatformenCPR.php index 32114ee..4ae8d6f 100644 --- a/src/Plugin/os2web/DataLookup/ServiceplatformenCPR.php +++ b/src/Plugin/os2web/DataLookup/ServiceplatformenCPR.php @@ -23,8 +23,8 @@ class ServiceplatformenCPR extends ServiceplatformenBase implements DataLookupIn */ public function defaultConfiguration() { return [ - 'test_mode_fixed_cpr' => '', - ] + parent::defaultConfiguration(); + 'test_mode_fixed_cpr' => '', + ] + parent::defaultConfiguration(); } /** diff --git a/src/Plugin/os2web/DataLookup/ServiceplatformenCPRExtended.php b/src/Plugin/os2web/DataLookup/ServiceplatformenCPRExtended.php index 38d680e..afd9a3b 100644 --- a/src/Plugin/os2web/DataLookup/ServiceplatformenCPRExtended.php +++ b/src/Plugin/os2web/DataLookup/ServiceplatformenCPRExtended.php @@ -55,7 +55,7 @@ class ServiceplatformenCPRExtended extends ServiceplatformenBase implements Data public function defaultConfiguration() { return [ 'test_mode_fixed_cpr' => '', - ] + parent::defaultConfiguration(); + ] + parent::defaultConfiguration(); } /**