diff --git a/.circleci/config.yml b/.circleci/config.yml index a9f1e98b..7ba305d9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -4,7 +4,7 @@ version: 2.1 jobs: build: - working_directory: ~/example + working_directory: ~/project docker: - image: cimg/php:8.1-browsers - image: cimg/mysql:5.7 @@ -31,6 +31,27 @@ jobs: name: Update Composer command: | sudo composer self-update + + # Run code quality checks + - checkout + - run: + name: Run code reviews + command: | + composer install + echo "phpcs for modules" + vendor/bin/phpcs defaults/standard/modules --standard="Drupal,DrupalPractice" -n --extensions="php,module,inc,install,test,profile,theme" + echo "phpmd for modules" + vendor/bin/phpmd defaults/standard/modules text defaults/standard/phpmd.xml "php,inc,module,theme,profile,install,test" + echo "phpstan for modules" + vendor/bin/phpstan analyse defaults/standard/modules --level=2 + echo "phpcs for tasks" + vendor/bin/phpcs src --standard="Drupal,DrupalPractice" -n --extensions="php,module,inc,install,test,profile,theme" + echo "phpmd for tasks" + vendor/bin/phpmd src text defaults/standard/phpmd.xml "php,inc,module,theme,profile,install,test" + echo "phpstan for tasks" + vendor/bin/phpstan analyse src --level=2 + + # Install a drupal test project. - run: name: Create artifacts directory command: mkdir /tmp/artifacts @@ -58,6 +79,7 @@ jobs: - run: name: Replace the default version of the-build with this one command: composer require --dev --with-all-dependencies palantirnet/the-build:dev-${CIRCLE_BRANCH} + working_directory: ~/example # Source cache - update when branch changes - save_cache: @@ -69,16 +91,19 @@ jobs: - run: name: Install the-build in the project command: printf '\n\n\nn' | vendor/bin/the-build-installer + working_directory: ~/example - run: name: Wait for DB # Dockerize is preinstalled in circleci/* docker image command: dockerize -wait tcp://127.0.0.1:3306 -timeout 120s + working_directory: ~/example # Install Drupal (separately, so that we can see it fail separately) - run: name: Install Drupal command: printf 'y' | vendor/bin/phing install -Ddrupal.validate_clean_config.bypass=yes + working_directory: ~/example # Composer package cache - update when the contents of the Composer cache directory # change. This cache is saved after installing Drupal, as the install process uses @@ -87,18 +112,20 @@ jobs: - save_cache: key: composer-v1-{{ checksum "/tmp/composer-cache.txt" }} paths: - - ~/.cache/composer + - ~/.cache/composer # Add a multisite - run: name: Add a multisite to the project command: printf 'intranet\nintranet.example.ddev.site' | vendor/bin/phing drupal-add-multisite + working_directory: ~/example - run: name: Run Behat tests command: | - nohup php -S example.ddev.site:8000 -t $(pwd)/${DRUPAL_ROOT}/ > /tmp/artifacts/phpd.log 2>&1 & - vendor/bin/phing test -Dbuild.env=circleci + nohup php -S example.ddev.site:8000 -t $(pwd)/${DRUPAL_ROOT}/ > /tmp/artifacts/phpd.log 2>&1 & + vendor/bin/phing test -Dbuild.env=circleci + working_directory: ~/example - store_artifacts: path: /tmp/artifacts diff --git a/.gitignore b/.gitignore index 57872d0f..0dca1458 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ /vendor/ +.idea diff --git a/code-review.sh b/code-review.sh new file mode 100755 index 00000000..d1393126 --- /dev/null +++ b/code-review.sh @@ -0,0 +1,30 @@ +#!/bin/bash + +echo "Running PHPCBF on modules" +echo "--------------" +vendor/bin/phpcbf defaults/standard/modules --standard="Drupal,DrupalPractice" -n --extensions="php,module,inc,install,test,profile,theme" +# find what is left to fix +echo "Running PHPCS on modules" +echo "-------------" +vendor/bin/phpcs defaults/standard/modules --standard="Drupal,DrupalPractice" -n --extensions="php,module,inc,install,test,profile,theme" +echo "Running PHPMD on modules" +echo "-------------" +vendor/bin/phpmd defaults/standard/modules text defaults/standard/phpmd.xml "php,inc,module,theme,profile,install,test" +echo "Running PHPStan on modules" +echo "---------------" +vendor/bin/phpstan analyse defaults/standard/modules --level=2 + +# auto-fix what can be fixed +echo "Running PHPCBF on tasks" +echo "--------------" +vendor/bin/phpcbf src --standard="Drupal,DrupalPractice" -n --extensions="php,module,inc,install,test,profile,theme" +# find what is left to fix +echo "Running PHPCS on tasks" +echo "-------------" +vendor/bin/phpcs src --standard="Drupal,DrupalPractice" -n --extensions="php,module,inc,install,test,profile,theme" +echo "Running PHPMD on tasks" +echo "-------------" +vendor/bin/phpmd src text defaults/standard/phpmd.xml "php,inc,module,theme,profile,install,test" +echo "Running PHPStan on tasks" +echo "---------------" +vendor/bin/phpstan analyse src --level=2 diff --git a/docs/development.md b/docs/development.md index 3d368577..7a34e511 100644 --- a/docs/development.md +++ b/docs/development.md @@ -8,6 +8,16 @@ To develop and test changes to the-build, you'll generally need to have a Drupal The major thing to watch out for here is that your copy of the-build is temporary -- in certain cases (especially when switching branches) when you run composer commands, your repo may be replaced with a different version. +## Running code reviews on the-build + +Our build tool should follow the same quality standards as our projects. + +We run automated tests using PHP Code Beautifier, PHP Code Sniffer, PHP MD, and PHPStan. You can invoke all of these with a single command from the project root: + +`./code-review.sh` + +These tests -- except for PHP Code Beautifier -- also run on CircleCI. + ## Testing on an existing site You can clone the-build into the vendor directory of an existing site. If it doesn't have a version of the-build already installed, you'll need to require it with `composer require palantirnet/the-build:dev-release-2.0 --prefer-source --dev` first. @@ -28,4 +38,4 @@ composer require palantirnet/the-build:dev-release-2.0 --prefer-source ``` ---- -Copyright 2018 Palantir.net, Inc. +Copyright 2018-2022 Palantir.net, Inc. diff --git a/src/TheBuild/Acquia/AcquiaTask.php b/src/TheBuild/Acquia/AcquiaTask.php index 5b75b23b..dc53c1c0 100644 --- a/src/TheBuild/Acquia/AcquiaTask.php +++ b/src/TheBuild/Acquia/AcquiaTask.php @@ -47,6 +47,8 @@ abstract class AcquiaTask extends \Task { * * @throws \IOException * @throws \NullPointerException + * + * @SuppressWarnings(PHPMD.Superglobals) */ protected function loadCredentials() { if (empty($this->mail) || empty($this->key)) { diff --git a/src/TheBuild/Acquia/GetLatestBackupTask.php b/src/TheBuild/Acquia/GetLatestBackupTask.php index 59b776e8..3f11cf13 100644 --- a/src/TheBuild/Acquia/GetLatestBackupTask.php +++ b/src/TheBuild/Acquia/GetLatestBackupTask.php @@ -91,11 +91,6 @@ class GetLatestBackupTask extends AcquiaTask { public function main() { $this->validate(); - // If the Acquia database name isn't set, default to using the site name. - if (empty($this->database)) { - $this->database = $this->site; - } - // Store the Acquia Cloud API JSON database backup records in our backups // directory.. $this->backupsFile = new \PhingFile($this->dir, "backups-{$this->site}-{$this->database}-{$this->env}.json"); @@ -187,7 +182,7 @@ protected function downloadBackup(array $backup, \PhingFile $destination) { $response = $request->send(); fclose($stream); - $this->log("Downloaded " . $response->getHeader('content-length') / 1000000 . "MB to " . $destination->getAbsolutePath()); + $this->log("Downloaded " . intval($response->getHeader('content-length')) / 1000000 . "MB to " . $destination->getAbsolutePath()); } /** @@ -231,6 +226,8 @@ protected function getCurrentBackupRecords() { * Acquia backup info array. * * @throws \BuildException + * + * @SuppressWarnings(PHPMD.ShortVariable) */ protected function getBackupRecords(\PhingFile $file) { if ($file->exists()) { @@ -360,6 +357,11 @@ public function setPropertyName(string $value) { * Verify that the required parameters are available. */ protected function validate() { + // If the Acquia database name isn't set, default to using the site name. + if (empty($this->database)) { + $this->database = $this->site; + } + // Check the build attributes. foreach (['dir', 'realm', 'site', 'env'] as $attribute) { if (empty($this->$attribute)) { throw new \BuildException("$attribute attribute is required.", $this->location); diff --git a/src/TheBuild/ForeachKeyTask.php b/src/TheBuild/ForeachKeyTask.php index bd4a6c05..3c148dc0 100644 --- a/src/TheBuild/ForeachKeyTask.php +++ b/src/TheBuild/ForeachKeyTask.php @@ -75,11 +75,11 @@ public function main() { // Extract matching keys from the properties array. $keys = []; $project = $this->getProject(); - foreach ($project->getProperties() as $name => $value) { + foreach (array_keys($project->getProperties()) as $name) { if (strpos($name, $this->prefix) === 0) { $property_children = substr($name, strlen($this->prefix)); // phpcs:ignore - [$key, $property_grandchildren] = explode('.', $property_children, 2); + [$key] = explode('.', $property_children, 2); $keys[$key] = $key; } } @@ -88,7 +88,7 @@ public function main() { $keys = array_diff($keys, $this->omitKeys); // Iterate over each extracted key. - foreach ($keys as $key => $prefix) { + foreach (array_keys($keys) as $key) { $prop = $this->callee->createProperty(); $prop->setOverride(TRUE); $prop->setName($this->keyParam); diff --git a/src/TheBuild/IncludeResourceTask.php b/src/TheBuild/IncludeResourceTask.php index 813010a0..aba4328c 100644 --- a/src/TheBuild/IncludeResourceTask.php +++ b/src/TheBuild/IncludeResourceTask.php @@ -67,7 +67,7 @@ public function main() { } } - // Link or copy the source artifact. + // Link or copy the source artifact. @phpstan-ignore-next-line $this->dest->getParentFile()->mkdirs(); if ($this->mode == 'copy') { $this->log(sprintf("Copying '%s' to '%s'", $this->source->getPath(), $this->dest->getPath())); @@ -75,6 +75,7 @@ public function main() { } else { $this->log(sprintf("Linking '%s' to '%s'", $this->source->getPath(), $this->dest->getPath())); + /** @var \SymlinkTask $symlink_task */ $symlink_task = $this->project->createTask("symlink"); $symlink_task->setTarget($this->source->getPath()); $symlink_task->setLink($this->dest->getPath()); diff --git a/src/TheBuild/SelectOneTask.php b/src/TheBuild/SelectOneTask.php index 8aa0e8d1..2c298356 100644 --- a/src/TheBuild/SelectOneTask.php +++ b/src/TheBuild/SelectOneTask.php @@ -50,6 +50,8 @@ public function main() { $keys = array_map('trim', explode($this->delimiter, $this->list)); + $value = NULL; + if (count($keys) > 1) { // Prompt for input. $request = new MenuInputRequest($this->message); diff --git a/src/TheBuild/SelectPropertyKeyTask.php b/src/TheBuild/SelectPropertyKeyTask.php index 71910f2c..86ae06bf 100644 --- a/src/TheBuild/SelectPropertyKeyTask.php +++ b/src/TheBuild/SelectPropertyKeyTask.php @@ -53,7 +53,7 @@ public function main() { if (strpos($name, $this->prefix) === 0) { $property_children = substr($name, strlen($this->prefix)); // phpcs:ignore - [$key, $property_grandchildren] = explode('.', $property_children, 2); + [$key] = explode('.', $property_children, 2); $keys[$key] = $key; } } @@ -61,6 +61,8 @@ public function main() { // Remove keys based on the 'omitKeys' attribute. $keys = array_diff($keys, $this->omitKeys); + $value = NULL; + if (count($keys) > 1) { // Prompt for input. $request = new MenuInputRequest($this->message);