Merge pull request #205 from palantirnet/test-test

Update code standards and run checks on Circle

.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

.gitignore

@@ -1 +1,2 @@
 /vendor/
+.idea

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

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.

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)) {

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);

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);

src/TheBuild/IncludeResourceTask.php

@@ -67,14 +67,15 @@ 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()));
       $this->source->copyTo($this->dest);
     }
     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());

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);

src/TheBuild/SelectPropertyKeyTask.php

@@ -53,14 +53,16 @@ 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;
       }
     }
 
     // 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);