From 02f3ca66c087fad847ddab24204999cedba30fe9 Mon Sep 17 00:00:00 2001 From: Grey Baker Date: Tue, 29 Aug 2017 18:26:07 +0100 Subject: [PATCH 1/3] Include development dependencies in FileParsers::Php::Composer --- lib/dependabot/file_parsers/php/composer.rb | 42 +++++++++++++++---- .../file_parsers/php/composer_spec.rb | 36 ++++++++++++++-- 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/lib/dependabot/file_parsers/php/composer.rb b/lib/dependabot/file_parsers/php/composer.rb index e417433c7e..bf374057c5 100644 --- a/lib/dependabot/file_parsers/php/composer.rb +++ b/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 } diff --git a/spec/dependabot/file_parsers/php/composer_spec.rb b/spec/dependabot/file_parsers/php/composer_spec.rb index 9bdc0fd311..180d2af40b 100644 --- a/spec/dependabot/file_parsers/php/composer_spec.rb +++ b/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 From e5b9b4b94383770340d0e1505ead3ff747968c14 Mon Sep 17 00:00:00 2001 From: Grey Baker Date: Tue, 29 Aug 2017 18:29:44 +0100 Subject: [PATCH 2/3] Add pending spec for PHP file updater with a development dependency --- spec/dependabot/file_updaters/php/composer_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/dependabot/file_updaters/php/composer_spec.rb b/spec/dependabot/file_updaters/php/composer_spec.rb index 528eb4818c..14fad8fa0c 100644 --- a/spec/dependabot/file_updaters/php/composer_spec.rb +++ b/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 + + pending { is_expected.to include "\"monolog/monolog\":\"1.22.1\"" } + end end describe "the updated lockfile" do From 3b18d377b94b71e1b4a44e13168f290320c3d721 Mon Sep 17 00:00:00 2001 From: Pete Hamilton Date: Sat, 9 Sep 2017 13:05:10 +0100 Subject: [PATCH 3/3] Handle development dependencies for PHP projects As well as updating production dependencies in the "require" section of a `composer.json` file, we also want to ensure we're keeping development dependencies in the `require-dev`[1] section up to date (e.g., PHPUnit). We can safely attempt to update both sections because if a dependency exists in both require and require-dev, composer will complain about conflicts itself, so there shouldn't be any composer.json files in that state. [1]: See: https://getcomposer.org/doc/04-schema.md#require-dev --- helpers/php/src/Updater.php | 35 +++++++++++++++++-- .../file_updaters/php/composer_spec.rb | 2 +- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/helpers/php/src/Updater.php b/helpers/php/src/Updater.php index 4154ffb167..0dd5353475 100644 --- a/helpers/php/src/Updater.php +++ b/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; + } } diff --git a/spec/dependabot/file_updaters/php/composer_spec.rb b/spec/dependabot/file_updaters/php/composer_spec.rb index 14fad8fa0c..293fe4d758 100644 --- a/spec/dependabot/file_updaters/php/composer_spec.rb +++ b/spec/dependabot/file_updaters/php/composer_spec.rb @@ -94,7 +94,7 @@ fixture("php", "composer_files", "development_dependencies") end - pending { is_expected.to include "\"monolog/monolog\":\"1.22.1\"" } + it { is_expected.to include "\"monolog/monolog\":\"1.22.1\"" } end end