Feature/blt phpcbf (#1019)

* BLT-977: Adds phpcbf task.

BLT-977: Updates fix phing targets.

BLT-977: Cleans up comments and fix:phpcbf:files target.

BLT-977: Adds files.php.custom.themes to phpcbf filesets.

* Adds phpunit test for missing file param.

* BLT-977: Prevents file-list script from hanging.

* BAC-977: Adds additional phpunit tests for Fix task.

* BAC-977: Fixes PHPCS violations. Ironically.

* BAC-977: Updates phpunit tests to include phpcbf tests.

phing/build.xml

@@ -71,6 +71,9 @@
   <!-- Contains Drupal SimpleSAMLphp tasks. -->
   <import file="${phing.dir}/tasks/simplesamlphp.xml"/>
 
+  <!-- Contains tasks that fix custom code. -->
+  <import file="${phing.dir}/tasks/fix.xml"/>
+
   <!-- Disable targets defined in the disable-targets array in project.yml or project.local.yml. -->
   <!-- This must be executed after all targets are defined. -->
   <disabletargets file="${blt.config-files.project}" property="disable-targets"/>

phing/build.yml

@@ -106,6 +106,13 @@ phpcs:
   haltonerror: true
   haltonwarning: true
 
+phpcbf:
+  filesets:
+    - files.php.custom.modules
+    - files.php.tests
+    - files.php.custom.themes
+    - files.frontend.custom.themes
+
 project:
   local:
     uri: ${project.local.protocol}://${project.local.hostname}

phing/tasks/fix.xml

@@ -0,0 +1,61 @@
+<project name="fix" default="fix">
+
+  <target name="fix" description="Runs all fix targets."
+          depends="fix:all"/>
+
+  <!-- Run all fix targets. -->
+  <target name="fix:all" description="Runs all fix targets."
+          depends="fix:phpcbf"
+          hidden="true" />
+
+  <!-- Run PHP Code Beautifier against all custom code. -->
+  <target name="fix:phpcbf" description="Runs PHP Code Beautifier utility against custom code.">
+    <property name="phpcbf.ruleset" value="${repo.root}/vendor/drupal/coder/coder_sniffer/Drupal/ruleset.xml"/>
+    <foreach list="${phpcbf.filesets}" target="fix:phpcbf:fileset" param="fileset_refid"/>
+  </target>
+
+  <!-- Run PHP Code Beautifier against defined filesets. -->
+  <target name="fix:phpcbf:fileset" description="Return list of files with phpcs violations." hidden="true">
+    <property name="phpcbf.ruleset" value="${repo.root}/vendor/drupal/coder/coder_sniffer/Drupal/ruleset.xml"/>
+
+    <!-- Generate a file to list files that need to be fixed. -->
+    <phpcodesniffer
+            standard="${phpcbf.ruleset}"
+            showSniffs="false"
+            showSources="false"
+            showWarnings="true"
+            haltonerror="false"
+            haltonwarning="false"
+            verbosity="0">
+      <fileset refid="${fileset_refid}"/>
+      <formatter type="csv" usefile="true" outfile="${reports.localDir}/${fileset_refid}-out.txt" />
+    </phpcodesniffer>
+
+    <!-- Convert PHPCS output into a usable listFile. -->
+    <echo>Generating Code Sniffer report: ${reports.localDir}/${fileset_refid}-out.txt</echo>
+    <exec dir="${repo.root}" command="${blt.root}/scripts/blt/phpcbf-file-list.sh ${reports.localDir}/${fileset_refid}-out.txt" logoutput="true"/>
+
+    <!-- Fix each listed file. -->
+    <foreach param="file" target="fix:phpcbf:file">
+      <filelist dir="${repo.root}" listfile="${reports.localDir}/${fileset_refid}-out.txt" />
+    </foreach>
+
+    <!-- Clean up. -->
+    <delete file="${reports.localDir}/${fileset_refid}-out.txt" />
+  </target>
+
+  <!-- Run PHP Code Beautifier on an comma-separated list of files. -->
+  <target name="fix:phpcbf:files" description="Run PHP Code Beautifier on an arbitrary list of files." hidden="true">
+    <fail unless="files" message="Missing files parameter."/>
+    <property name="phpcbf.ruleset" value="${repo.root}/vendor/drupal/coder/coder_sniffer/Drupal/ruleset.xml"/>
+    <foreach list="${files}" target="fix:phpcbf:file" param="file"/>
+  </target>
+
+  <!-- Run PHP Code Beautifier on a specific file. -->
+  <target name="fix:phpcbf:file" description="Run PHP Code Beautifier on a specific file." hidden="true">
+    <fail unless="file" message="Missing file parameter."/>
+    <property name="phpcbf.ruleset" value="${repo.root}/vendor/drupal/coder/coder_sniffer/Drupal/ruleset.xml"/>
+    <exec dir="${repo.root}" command="${repo.root}/vendor/bin/phpcbf phpcbf --standard=${phpcbf.ruleset} ${file}" logoutput="true" />
+  </target>
+
+</project>

scripts/blt/phpcbf-file-list.sh

@@ -0,0 +1,36 @@
+#!/usr/bin/env bash
+
+GIT_ROOT=$( git rev-parse --show-toplevel )
+PHPCBF_BIN=${GIT_ROOT}/vendor/bin/phpcbf
+FILENAME=$1
+
+if [ ! -f $PHPCBF_BIN ];
+  then
+    echo "PHP Code Beautifier was not found in this project's bin directory. Please run composer install."
+    exit 1
+fi
+
+if [ -z $FILENAME ];
+  then
+    echo "Missing file list parameter."
+    exit 1
+fi
+
+# Get list of files to fix from input csv file.
+LIST=$( cat $FILENAME | awk -F '^\"|\",\"*' '{print $2}' | sort -u )
+
+# Replace report file contents with contents of $LIST.
+> $FILENAME
+if [ -z "$LIST" ];
+  then
+    echo "No fixable files found."
+    exit
+  else
+    for i in $LIST; do
+      echo  $i >> $FILENAME
+    done
+    echo "Files that can be fixed by PHPCBF:"
+    cat $FILENAME
+fi
+
+exit

tests/phpunit/BltProject/FixTest.php

@@ -0,0 +1,159 @@
+<?php
+
+namespace Acquia\Blt\Tests\BltProject;
+
+use Acquia\Blt\Tests\BltProjectTestBase;
+
+/**
+ * Class FixTasksTest.
+ *
+ * Verifies that fix related tasks work as expected.
+ */
+class FixTasksTest extends BltProjectTestBase {
+
+  private $returnVar;
+  private $output = [];
+
+  /**
+   * Tests for existence of phpcbf script.
+   */
+  public function testFixPhpcbfExists() {
+    $this->executePhpcbfFileListScript();
+    $this->assertNotContains("PHP Code Beautifier was not found in this project's bin directory. Please run composer install.", $this->output);
+    $this->assertEquals(1, $this->returnVar);
+  }
+
+  /**
+   * Tests the handling of a missing file parameter.
+   */
+  public function testFixPhpcbfExitsWhenMissingArguments() {
+    $this->executePhpcbfFileListScript();
+    $this->assertContains("Missing file list parameter.", $this->output);
+    $this->assertEquals(1, $this->returnVar);
+  }
+
+  /**
+   * Tests that a file list is not generated, given a file without violations.
+   */
+  public function testFixPhpcbfWithNotFixableFile() {
+    $file = $this->createTestFile('file_without_violations.php', FALSE);
+    $report = $this->createTestReport('report_without_violations.txt', $file);
+    $this->executePhpcbfFileListScript($report);
+    $this->assertNotContains("Missing file list parameter.", $this->output);
+    $this->assertContains("No fixable files found.", $this->output);
+    $this->assertEquals(0, $this->returnVar);
+  }
+
+  /**
+   * Tests that a file list is generated, given a file with a fixable violation.
+   */
+  public function testFixPhpcbfWithFixableFile() {
+    $file = $this->createTestFile('file_with_violations.php');
+    $report = $this->createTestReport('report_with_violations.txt', $file);
+    $this->executePhpcbfFileListScript($report);
+    $this->assertNotContains("Missing file list parameter.", $this->output);
+
+    foreach ($this->output as $value) {
+      if (!file_exists($value)) {
+        $this->assertEquals('Files that can be fixed by PHPCBF:', $value);
+      }
+      else {
+        $this->assertStringEndsWith('file_with_violations.php', $value);
+        $this->assertStringEndsNotWith('file_without_violations.php', $value);
+        $phpcbf_first_pass = $this->executePhpcbf($value);
+        $this->assertContains("Processing file_with_violations.php", $phpcbf_first_pass);
+
+        // Run phpcbf a second time to ensure the file has been fixed.
+        $phpcbf_second_pass = $this->executePhpcbf($value);
+        $this->assertContains("No fixable errors were found", $phpcbf_second_pass);
+      }
+    }
+    $this->assertEquals(0, $this->returnVar);
+  }
+
+  /**
+   * Helper method that executes the phpcbf-file-list.sh script.
+   *
+   * @param string $file
+   *   The full path to the file to be processed.
+   */
+  protected function executePhpcbfFileListScript($file = '') {
+    chdir($this->projectDirectory);
+    $command = "sh vendor/acquia/blt/scripts/blt/phpcbf-file-list.sh $file";
+    exec($command, $this->output, $this->returnVar);
+  }
+
+  /**
+   * Helper method to run phpcbf.
+   *
+   * @param string $file
+   *   The full path to the file to be processed.
+   *
+   * @return string
+   *   The output of the executed phpcbf command.
+   */
+  protected function executePhpcbf($file) {
+    chdir($this->projectDirectory);
+    return shell_exec("./vendor/bin/phpcbf --standard=vendor/drupal/coder/coder_sniffer/Drupal/ruleset.xml {$file}");
+  }
+
+  /**
+   * Helper method to create a phpcs csv file report.
+   *
+   * @param string $report_name
+   *   The name of the report file to be generated.
+   * @param string $file
+   *   The full path to the file to be processed.
+   *
+   * @return string
+   *   The full path to the generated csv report file.
+   */
+  protected function createTestReport($report_name, $file) {
+    chdir($this->projectDirectory);
+    $report_path = sys_get_temp_dir() . '/' . $report_name;
+    exec("./vendor/bin/phpcs --standard=vendor/drupal/coder/coder_sniffer/Drupal/ruleset.xml --report=csv --report-file={$report_path} {$file}");
+    return $report_path;
+  }
+
+  /**
+   * Helper method to create a php file.
+   *
+   * @param string $file_name
+   *   The name of the file to be generated.
+   * @param bool $with_violations
+   *   Boolean indicating whether the file should contain a fixable violation.
+   *
+   * @return string
+   *   The full path to the generated php file.
+   */
+  protected function createTestFile($file_name, $with_violations = TRUE) {
+    $file_path = sys_get_temp_dir() . '/' . $file_name;
+
+    $file_contents = "<?php
+
+/**
+ * @file
+ * A file to fix.
+ */
+
+/**
+ * A function that doesn't need to be fixed by phpcbf.
+ */
+function first_test_function() {
+}
+";
+
+    if ($with_violations) {
+      $file_contents .= "
+/**
+ * A function that needs to be fixed by phpcbf.
+ */
+function second_test_function(){
+}
+";
+    }
+    file_put_contents($file_path, $file_contents);
+    return $file_path;
+  }
+
+}