Merge pull request #46 from dependabot/php-improvements

[WIP] Include development dependencies in FileParsers::Php::Composer

helpers/php/src/Updater.php

@@ -14,9 +14,19 @@ public static function update($args)
 
     $composerJson = json_decode(file_get_contents('composer.json'), true);
 
-    $existingDependencyVersion = $composerJson["require"][$dependencyName];
-    $newDependencyVersion = self::relaxVersionToUserPreference($existingDependencyVersion, $dependencyVersion);
-    $composerJson["require"][$dependencyName] = $newDependencyVersion;
+    $composerJson = self::updateComposerJsonSection(
+      $composerJson,
+      "require",
+      $dependencyName,
+      $dependencyVersion
+    );
+
+    $composerJson = self::updateComposerJsonSection(
+      $composerJson,
+      "require-dev",
+      $dependencyName,
+      $dependencyVersion
+    );
 
     // When encoding JSON in PHP, it'll escape forward slashes by default.
     // We're not expecting this transform from the original data, which means
@@ -88,4 +98,23 @@ public static function relaxVersionToUserPreference($existingDependencyVersion,
 
     return $newDependencyVersion;
   }
+
+  // Given a nested array representing a composer.json file, look for the given
+  // dependency in the provided section (i.e., require, require-dev) and update
+  // the composer data with the new version, before returning a composer
+  // representation with the updated version.
+  //
+  // If the dependency doesn't exist in the section, will return the provided
+  // composer JSON unaltered
+  //
+  // Note: Arrays are passed by value/copy, so this will leave the original composerJson untouched
+  public static function updateComposerJsonSection($composerJson, $section, $dependencyName, $dependencyVersion) {
+    if(isset($composerJson[$section][$dependencyName])) {
+      $existingDependencyVersion = $composerJson[$section][$dependencyName];
+      $newDependencyVersion = self::relaxVersionToUserPreference($existingDependencyVersion, $dependencyVersion);
+      $composerJson[$section][$dependencyName] = $newDependencyVersion;
+    }
+
+    return $composerJson;
+  }
 }

lib/dependabot/file_parsers/php/composer.rb

@@ -9,14 +9,15 @@ module FileParsers
     module Php
       class Composer < Dependabot::FileParsers::Base
         def parse
-          parsed_composer_json = JSON.parse(composer_json.content)
+          runtime_dependencies + development_dependencies
+        end
 
-          dependencies = parsed_composer_json.fetch("require", {})
+        private
 
-          # TODO: Add support for development dependencies. Will need to be
-          # added to file updaters, too.
+        def runtime_dependencies
+          parsed_composer_json = JSON.parse(composer_json.content)
 
-          dependencies.map do |name, requirement|
+          parsed_composer_json.fetch("require", {}).map do |name, req|
             # Ignore dependencies which appear in the composer.json but not the
             # composer.lock. For instance, if a specific PHP version or
             # extension is required, it won't appear in the packages section of
@@ -31,16 +32,41 @@ def parse
               name: name,
               version: dependency_version(name),
               requirements: [{
-                requirement: requirement,
+                requirement: req,
                 file: "composer.json",
-                groups: []
+                groups: ["runtime"]
               }],
               package_manager: "composer"
             )
           end.compact
         end
 
-        private
+        def development_dependencies
+          parsed_composer_json = JSON.parse(composer_json.content)
+
+          parsed_composer_json.fetch("require-dev", {}).map do |name, req|
+            # Ignore dependencies which appear in the composer.json but not the
+            # composer.lock. For instance, if a specific PHP version or
+            # extension is required, it won't appear in the packages section of
+            # the lockfile.
+            next if dependency_version(name).nil?
+
+            # Ignore dependency versions which are non-numeric, since they can't
+            # be compared later in the process.
+            next unless dependency_version(name).match?(/^\d/)
+
+            Dependency.new(
+              name: name,
+              version: dependency_version(name),
+              requirements: [{
+                requirement: req,
+                file: "composer.json",
+                groups: ["development"]
+              }],
+              package_manager: "composer"
+            )
+          end.compact
+        end
 
         def dependency_version(name)
           package = parsed_lockfile["packages"].find { |d| d["name"] == name }

spec/dependabot/file_parsers/php/composer_spec.rb

@@ -41,18 +41,46 @@
         its(:name) { is_expected.to eq("monolog/monolog") }
         its(:version) { is_expected.to eq("1.0.2") }
         its(:requirements) do
-          is_expected.
-            to eq([{ requirement: "1.0.*", file: "composer.json", groups: [] }])
+          is_expected.to eq(
+            [
+              {
+                requirement: "1.0.*",
+                file: "composer.json",
+                groups: ["runtime"]
+              }
+            ]
+          )
         end
       end
     end
 
-    pending "for development dependencies" do
+    context "for development dependencies" do
       let(:composer_json_body) do
         fixture("php", "composer_files", "development_dependencies")
       end
 
-      its(:length) { is_expected.to eq(2) }
+      it "includes development dependencies" do
+        expect(dependencies.length).to eq(2)
+      end
+
+      describe "the first dependency" do
+        subject { dependencies.first }
+
+        it { is_expected.to be_a(Dependabot::Dependency) }
+        its(:name) { is_expected.to eq("monolog/monolog") }
+        its(:version) { is_expected.to eq("1.0.2") }
+        its(:requirements) do
+          is_expected.to eq(
+            [
+              {
+                requirement: "1.0.1",
+                file: "composer.json",
+                groups: ["development"]
+              }
+            ]
+          )
+        end
+      end
     end
 
     context "with the PHP version specified" do

spec/dependabot/file_updaters/php/composer_spec.rb

@@ -88,6 +88,14 @@
 
         it { is_expected.to include "\"monolog/monolog\":\"1.22.1\"" }
       end
+
+      context "when the dependency is a development dependency" do
+        let(:composer_body) do
+          fixture("php", "composer_files", "development_dependencies")
+        end
+
+        it { is_expected.to include "\"monolog/monolog\":\"1.22.1\"" }
+      end
     end
 
     describe "the updated lockfile" do