Merge pull request #18 from palantirnet/feature/code-quality-tools

Code Quality Tools

build.dist.xml

@@ -32,6 +32,17 @@
     <!-- Target: test -->
     <target name="test" description="Run all the tests.">
         <echo>Configure this target to run the tests.</echo>
+        <!-- You may want to include one or more of the commands from the 'code-review' target here. -->
+    </target>
+
+
+    <!-- Target: code-review -->
+    <target name="code-review" description="Run the automated code reviews.">
+        <!-- These are called directly rather than using <import> because they are single-purpose task files. -->
+        <phing phingfile="${build.dir}/vendor/palantirnet/the-build/tasks/code_review/drupal_code_sniffer.xml" />
+        <phing phingfile="${build.dir}/vendor/palantirnet/the-build/tasks/code_review/phpmd.xml" />
+        <phing phingfile="${build.dir}/vendor/palantirnet/the-build/tasks/code_review/phplint.xml" />
+        <phing phingfile="${build.dir}/vendor/palantirnet/the-build/tasks/code_review/phptodo.xml" />
     </target>
 
 

circle.dist.yml

@@ -42,4 +42,4 @@ test:
     - vendor/bin/phing build install migrate
 
   override:
-    - vendor/bin/phing test
+    - vendor/bin/phing code-review test

composer.json

@@ -11,6 +11,9 @@
     "require": {
         "phing/phing": "^2.14",
         "continuousphp/phing-drush-task": "dev-master",
+        "drupal/coder": "^8.2",
+        "phpmd/phpmd" : "^2.4",
+        "nilportugues/php_todo": "^1.0",
         "pear/versioncontrol_git": "@dev"
     }
 }

conf/php_todo_finder.yml

@@ -0,0 +1,7 @@
+todo_finder:
+  total_allowed: 5
+  expressions:
+    - @todo
+    - TODO
+    - refactor
+    - FIX ME

conf/phpmd.xml

@@ -0,0 +1,78 @@
+<?xml version="1.0"?>
+<ruleset xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="PMD Ruleset for Drupal" xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
+  <description>
+    A PMD Ruleset for Drupal coding standards.
+  </description>
+
+  <!--
+    Include each rule explicitly so we know what we have.
+    @see https://github.com/phpmd/phpmd/blob/master/src/main/resources/rulesets/
+  -->
+
+  <!-- Clean Code -->
+  <!--
+  These don't align with Drupal standards, so they are excluded.
+  @todo Static calls are hard to test and extend, is there a way to whitelist the ones that are OK?
+  <rule ref="rulesets/cleancode.xml/BooleanArgumentFlag"/>
+  <rule ref="rulesets/cleancode.xml/ElseExpression"/>
+  <rule ref="rulesets/cleancode.xml/StaticAccess"/>
+  -->
+
+  <!-- Code Size -->
+  <rule ref="rulesets/codesize.xml/CyclomaticComplexity"/>
+  <rule ref="rulesets/codesize.xml/NPathComplexity"/>
+  <rule ref="rulesets/codesize.xml/ExcessiveMethodLength"/>
+  <rule ref="rulesets/codesize.xml/ExcessiveClassLength"/>
+  <rule ref="rulesets/codesize.xml/ExcessiveParameterList"/>
+  <rule ref="rulesets/codesize.xml/ExcessivePublicCount"/>
+  <rule ref="rulesets/codesize.xml/TooManyFields"/>
+
+  <!-- Controversial -->
+  <rule ref="rulesets/controversial.xml/Superglobals"/>
+  <!--
+  These checks do not need to be included since PHPCS will check for style.
+  <rule ref="rulesets/controversial.xml/CamelCaseClassName"/>
+  <rule ref="rulesets/controversial.xml/CamelCasePropertyName"/>
+  <rule ref="rulesets/controversial.xml/CamelCaseMethodName"/>
+  <rule ref="rulesets/controversial.xml/CamelCaseParameterName"/>
+  <rule ref="rulesets/controversial.xml/CamelCaseVariableName"/>
+  -->
+
+  <!-- Design -->
+  <rule ref="rulesets/design.xml/ExitExpression"/>
+  <rule ref="rulesets/design.xml/EvalExpression"/>
+  <rule ref="rulesets/design.xml/GotoStatement"/>
+  <rule ref="rulesets/design.xml/NumberOfChildren"/>
+  <rule ref="rulesets/design.xml/DepthOfInheritance"/>
+  <rule ref="rulesets/design.xml/CouplingBetweenObjects"/>
+  <rule ref="rulesets/design.xml/DevelopmentCodeFragment"/>
+
+  <!-- Naming -->
+  <rule ref="rulesets/naming.xml/ShortVariable">
+    <properties>
+      <!-- Allow $id as a variable name. -->
+      <property name="exceptions" description="Comma-separated list of exceptions" value="id"/>
+    </properties>
+  </rule>
+  <rule ref="rulesets/naming.xml/LongVariable">
+    <properties>
+      <!-- Bump variable length to a more reasonable number. -->
+      <property name="maximum" description="The variable length reporting threshold" value="35"/>
+    </properties>
+  </rule>
+  <rule ref="rulesets/naming.xml/ShortMethodName"/>
+  <rule ref="rulesets/naming.xml/ConstructorWithNameAsEnclosingClass"/>
+  <rule ref="rulesets/naming.xml/ConstantNamingConventions"/>
+  <rule ref="rulesets/naming.xml/BooleanGetMethodName"/>
+
+  <!-- Unused Code -->
+  <rule ref="rulesets/unusedcode.xml/UnusedPrivateField"/>
+  <rule ref="rulesets/unusedcode.xml/UnusedLocalVariable"/>
+  <rule ref="rulesets/unusedcode.xml/UnusedPrivateMethod"/>
+  <!--
+  Hooks often have unused parameters, so ignore this warning.
+  @todo is there a way to allow unused parameters in hooks but not elsewhere?
+  <rule ref="rulesets/unusedcode.xml/UnusedFormalParameter"/>
+  -->
+
+</ruleset>

docs/code_review.md

@@ -0,0 +1,40 @@
+# Code Review in the-build
+
+From your project where you have installed the-build, run `vendor/bin/phing code-review` to run the default set of code reviews. This target is provided in the default `build.xml` created for your project during install.
+
+This will review your code with:
+
+* Drupal Codesniffer
+* PHPmd
+* PHP Lint
+* PHPtodo
+
+Generally, you should configure your `build.xml` to run code reviews as part of your `test` target so that developers run the reviews by default.
+
+### PHP Lint
+
+PHP Lint uses the PHP interpreter directly to check for syntax errors. There is no configuration for this review.
+
+### [PHPMD](https://phpmd.org/)
+
+A more complicated and more general PHP code review than the Drupal Codesniffer standard. The default config for this review can be found within the-build at `conf/phpmd.xml`. To customize this config, copy that file to your project's `conf/` directory and add the build property:
+
+```
+phpmd.rulesets=conf/phpmd.xml
+```
+
+### Drupal Codesniffer
+
+Runs codesniffer using the standard provided by Drupal's [Coder](https://www.drupal.org/project/coder) module. Generally, you should not change the configuration for this review, but if do need to you can provide a different standard:
+
+```
+drupal_code_sniffer.standard=vendor/drupal/coder/coder_sniffer/Drupal/ruleset.xml
+```
+
+### [PHP To-do Finder](https://github.com/nilportugues/php-todo-finder)
+
+Sets a threshold for the number of "to do" comments allowable in a codebase. The default config for this review can be found within the-build at `conf/php_todo_finder.yml`. To customize this config, copy that file to your project's `conf/` directory and add the build property:
+
+```
+phptodo.config=conf/php_todo_finder.yml
+```

tasks/code_review/drupal_code_sniffer.xml

@@ -0,0 +1,45 @@
+<?xml version="1.0"?>
+<project name="drupal_code_sniffer" default="test-run-code-sniffer">
+
+    <property name="drupal_code_sniffer.standard" value="vendor/drupal/coder/coder_sniffer/Drupal/ruleset.xml" />
+
+
+    <!-- Target: test-run-code-sniffer -->
+    <target name="test-run-code-sniffer">
+        <phpcodesniffer
+            standard="${drupal_code_sniffer.standard}"
+            haltonerror="false"
+            verbosity="1"
+        >
+            <!-- Modules -->
+            <fileset dir="${build.dir}/web/modules/custom">
+                <include name="**/*.php" />
+                <include name="**/*.module" />
+                <include name="**/*.inc" />
+                <include name="**/*.test" />
+                <include name="**/*.profile" />
+                <include name="**/*.theme" />
+            </fileset>
+            <!-- Themes -->
+            <fileset dir="${build.dir}/web/themes/custom">
+                <include name="**/*.php" />
+                <include name="**/*.module" />
+                <include name="**/*.inc" />
+                <include name="**/*.test" />
+                <include name="**/*.profile" />
+                <include name="**/*.theme" />
+            </fileset>
+            <!-- Profiles -->
+            <fileset dir="${build.dir}/web/profiles/custom">
+                <include name="**/*.php" />
+                <include name="**/*.module" />
+                <include name="**/*.inc" />
+                <include name="**/*.test" />
+                <include name="**/*.profile" />
+                <include name="**/*.theme" />
+            </fileset>
+        </phpcodesniffer>
+    </target>
+
+
+</project>

tasks/code_review/phplint.xml

@@ -0,0 +1,25 @@
+<?xml version="1.0"?>
+<project name="phplint" default="test-run-phplint">
+
+
+    <!-- Target: test-run-phplint -->
+    <target name="test-run-phplint">
+        <phplint haltonfailure="false">
+            <!-- modules -->
+            <fileset dir="${build.dir}/web/modules/custom">
+                <include name="**/*.php" />
+            </fileset>
+            <!-- profiles -->
+            <fileset dir="${build.dir}/web/profiles/custom">
+                <include name="**/*.php" />
+            </fileset>
+            <!-- themes -->
+            <fileset dir="${build.dir}/web/themes/custom">
+                <include name="**/*.php" />
+            </fileset>
+        </phplint>
+        <echo>PHP Lint was successful</echo>
+    </target>
+
+
+</project>

tasks/code_review/phpmd.xml

@@ -0,0 +1,18 @@
+<?xml version="1.0"?>
+<project name="phpmd" default="test-run-phpmd">
+
+    <property name="phpmd.rulesets" value="vendor/palantirnet/the-build/conf/phpmd.xml" />
+
+    <!-- Target: test-run-phpmd -->
+    <target name="test-run-phpmd">
+        <phpmd rulesets="${build.dir}/${phpmd.rulesets}">
+            <fileset dir="${build.dir}/web/modules/custom">
+                <include name="**/*.php" />
+                <include name="**/*.module" />
+                <include name="**/*.inc" />
+            </fileset>
+        </phpmd>
+    </target>
+
+
+</project>

tasks/code_review/phptodo.xml

@@ -0,0 +1,26 @@
+<?xml version="1.0"?>
+<project name="phptodo" default="test-run-phptodo">
+
+    <property name="phptodo.config" value="vendor/palantirnet/the-build/conf/php_todo_finder.yml" />
+
+    <!-- Target: test-run-phptodo -->
+    <target name="test-run-phptodo">
+        <echo>Checking modules</echo>
+        <exec
+            command="${build.dir}/vendor/bin/php_todo --config=${build.dir}/${phptodo.config} find ${build.dir}/web/modules/custom"
+            passthru="true"
+        />
+        <echo>Checking profiles</echo>
+        <exec
+            command="${build.dir}/vendor/bin/php_todo --config=${build.dir}/${phptodo.config} find ${build.dir}/web/profiles/custom"
+            passthru="true"
+        />
+        <echo>Checking themes</echo>
+        <exec
+            command="${build.dir}/vendor/bin/php_todo --config=${build.dir}/${phptodo.config} find ${build.dir}/web/themes/custom"
+            passthru="true"
+        />
+    </target>
+
+
+</project>