From 062b377e5bbab00d9c18de60982fe3c4527a3d3c Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Fri, 9 Sep 2022 17:07:30 -0400 Subject: [PATCH 1/8] Adds #transitive_multidependency_intro Checks for a mix of top_level? and !top_level? dependencies to determine whether a PR updates a transitive dependency. Might be enough to just check for !top_level? --- .../pull_request_creator/message_builder.rb | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 51004e8976a..2d4039f97c0 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -193,13 +193,18 @@ def requirement_commit_message_intro # rubocop:disable Metrics/PerceivedComplexity def version_commit_message_intro + dependency = dependencies.first + return multidependency_property_intro if dependencies.count > 1 && updating_a_property? return dependency_set_intro if dependencies.count > 1 && updating_a_dependency_set? + return transitive_multidependency_intro if dependencies.count > 1 && + dependencies.any?{ |dep| dep.top_level? } && + dependencies.any?{ |dep| !dep.top_level? } + return multidependency_intro if dependencies.count > 1 - dependency = dependencies.first msg = "Bumps #{dependency_links.first} " \ "#{from_version_msg(previous_version(dependency))}" \ "to #{new_version(dependency)}." @@ -239,6 +244,22 @@ def multidependency_intro "dependencies needed to be updated together." end + def transitive_multidependency_intro + dependency = dependencies.first + + msg = "Bumps #{dependency_links[0]} to #{new_version(dependency)}" + + msg += if dependencies.count > 2 + " and updates ancestor dependencies #{dependency_links[0..-2].join(', ')} and #{dependency_links[-1]}. " + else + " and updates ancestor dependency #{dependency_links[1]}. " + end + + msg += "These dependencies need to be updated together." + + msg + end + def from_version_msg(previous_version) return "" unless previous_version From 19e35a988b2ea6ada1d92b3b1b28004f80b149dd Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Mon, 12 Sep 2022 19:14:29 -0400 Subject: [PATCH 2/8] Add newline after intro message --- common/lib/dependabot/pull_request_creator/message_builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 2d4039f97c0..f5b799e916e 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -255,7 +255,7 @@ def transitive_multidependency_intro " and updates ancestor dependency #{dependency_links[1]}. " end - msg += "These dependencies need to be updated together." + msg += "These dependencies need to be updated together.\n" msg end From edb9d85ae22900a17e8634cecc9badc1bbcef50e Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Mon, 12 Sep 2022 19:14:46 -0400 Subject: [PATCH 3/8] Fix rubocop complexity errors --- .../pull_request_creator/message_builder.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index f5b799e916e..53964696212 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -192,6 +192,8 @@ def requirement_commit_message_intro end # rubocop:disable Metrics/PerceivedComplexity + # rubocop:disable Metrics/CyclomaticComplexity + # rubocop:disable Metrics/AbcSize def version_commit_message_intro dependency = dependencies.first @@ -200,8 +202,8 @@ def version_commit_message_intro return dependency_set_intro if dependencies.count > 1 && updating_a_dependency_set? return transitive_multidependency_intro if dependencies.count > 1 && - dependencies.any?{ |dep| dep.top_level? } && - dependencies.any?{ |dep| !dep.top_level? } + dependencies.any?(&:top_level?) && + dependencies.any? { |dep| !dep.top_level? } return multidependency_intro if dependencies.count > 1 @@ -221,6 +223,8 @@ def version_commit_message_intro end # rubocop:enable Metrics/PerceivedComplexity + # rubocop:enable Metrics/CyclomaticComplexity + # rubocop:enable Metrics/AbcSize def multidependency_property_intro dependency = dependencies.first @@ -250,7 +254,8 @@ def transitive_multidependency_intro msg = "Bumps #{dependency_links[0]} to #{new_version(dependency)}" msg += if dependencies.count > 2 - " and updates ancestor dependencies #{dependency_links[0..-2].join(', ')} and #{dependency_links[-1]}. " + " and updates ancestor dependencies #{dependency_links[0..-2].join(', ')} " \ + "and #{dependency_links[-1]}. " else " and updates ancestor dependency #{dependency_links[1]}. " end From 439f4e7794a16897e10d33adf78e9a3ea5ab3515 Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Tue, 13 Sep 2022 11:59:51 -0400 Subject: [PATCH 4/8] Avoid transitive_multidependency_intro if any dependencies are removed --- common/lib/dependabot/pull_request_creator/message_builder.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 53964696212..352452d39b1 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -203,7 +203,8 @@ def version_commit_message_intro return transitive_multidependency_intro if dependencies.count > 1 && dependencies.any?(&:top_level?) && - dependencies.any? { |dep| !dep.top_level? } + dependencies.any? { |dep| !dep.top_level? } && + dependencies.none?(&:removed?) return multidependency_intro if dependencies.count > 1 From 953a19a091fcd2536db91eff45823a91e758356a Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Tue, 13 Sep 2022 12:40:47 -0400 Subject: [PATCH 5/8] Test transitive_multidependency_intro message --- .../message_builder_spec.rb | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index 6e9c37615cf..67618bce5ca 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -1242,6 +1242,71 @@ def commits_details(base:, head:) end end + context "and transitive security vulnerabilities fixed" do + let(:dependencies) { [transitive_dependency, dependency] } + let(:transitive_dependency) do + Dependabot::Dependency.new( + name: "statesman", + version: "1.6.0", + previous_version: "1.5.0", + package_manager: "dummy", + requirements: [], + previous_requirements: [] + ) + end + + before do + statesman_repo_url = + "https://api.github.com/repos/gocardless/statesman" + stub_request(:get, statesman_repo_url). + to_return(status: 200, + body: fixture("github", "statesman_repo.json"), + headers: json_header) + stub_request(:get, "#{statesman_repo_url}/contents/"). + to_return(status: 200, + body: fixture("github", "statesman_files.json"), + headers: json_header) + stub_request(:get, "#{statesman_repo_url}/releases?per_page=100"). + to_return(status: 200, + body: fixture("github", "business_releases.json"), + headers: json_header) + stub_request(:get, "https://api.github.com/repos/gocardless/" \ + "statesman/contents/CHANGELOG.md?ref=master"). + to_return(status: 200, + body: fixture("github", "changelog_contents.json"), + headers: json_header) + stub_request(:get, "https://rubygems.org/api/v1/gems/statesman.json"). + to_return( + status: 200, + body: fixture("ruby", "rubygems_response_statesman.json") + ) + + service_pack_url = + "https://github.com/gocardless/statesman.git/info/refs" \ + "?service=git-upload-pack" + stub_request(:get, service_pack_url). + to_return( + status: 200, + body: fixture("git", "upload_packs", "no_tags"), + headers: { + "content-type" => "application/x-git-upload-pack-advertisement" + } + ) + end + + it "includes details of both dependencies" do + expect(pr_message). + to start_with( + "Bumps [statesman](https://github.com/gocardless/statesman) to 1.6.0 " \ + "and updates ancestor dependency [business](https://github.com/gocardless/business). " \ + "These dependencies need to be updated together.\n\n" \ + "Updates `statesman` from 1.5.0 to 1.6.0\n" \ + "
\n" \ + "Release notes\n" + ) + end + end + context "and an upgrade guide that can be pulled in" do let(:dependency) do Dependabot::Dependency.new( From 946eacc5aeb82355051f696e62bd50d7f0dab457 Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Tue, 13 Sep 2022 20:34:05 -0400 Subject: [PATCH 6/8] Undo moving dependency = dependencies.first --- common/lib/dependabot/pull_request_creator/message_builder.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 352452d39b1..87cde6cbc3f 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -195,8 +195,6 @@ def requirement_commit_message_intro # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/AbcSize def version_commit_message_intro - dependency = dependencies.first - return multidependency_property_intro if dependencies.count > 1 && updating_a_property? return dependency_set_intro if dependencies.count > 1 && updating_a_dependency_set? @@ -208,6 +206,7 @@ def version_commit_message_intro return multidependency_intro if dependencies.count > 1 + dependency = dependencies.first msg = "Bumps #{dependency_links.first} " \ "#{from_version_msg(previous_version(dependency))}" \ "to #{new_version(dependency)}." From 306413f012f903f8dc23721b1e73a044aac45d05 Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Tue, 13 Sep 2022 20:34:25 -0400 Subject: [PATCH 7/8] Factor out updating_top_level_and_transitive_dependencies? --- .../dependabot/pull_request_creator/message_builder.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 87cde6cbc3f..843b5d1d5f1 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -200,8 +200,7 @@ def version_commit_message_intro return dependency_set_intro if dependencies.count > 1 && updating_a_dependency_set? return transitive_multidependency_intro if dependencies.count > 1 && - dependencies.any?(&:top_level?) && - dependencies.any? { |dep| !dep.top_level? } && + updating_top_level_and_transitive_dependencies? && dependencies.none?(&:removed?) return multidependency_intro if dependencies.count > 1 @@ -283,6 +282,11 @@ def updating_a_dependency_set? any? { |r| r.dig(:metadata, :dependency_set) } end + def updating_top_level_and_transitive_dependencies? + dependencies.any?(&:top_level?) && + dependencies.any? { |dep| !dep.top_level? } + end + def property_name @property_name ||= dependencies.first.requirements. find { |r| r.dig(:metadata, :property_name) }&. From b2bf14c3f5977c2f23024a7be7fd5949c90c7aa8 Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Tue, 13 Sep 2022 20:36:53 -0400 Subject: [PATCH 8/8] Removes rubocop:disable CyclomaticComplexity --- common/lib/dependabot/pull_request_creator/message_builder.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 843b5d1d5f1..ce23bffbddd 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -192,7 +192,6 @@ def requirement_commit_message_intro end # rubocop:disable Metrics/PerceivedComplexity - # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/AbcSize def version_commit_message_intro return multidependency_property_intro if dependencies.count > 1 && updating_a_property? @@ -222,7 +221,6 @@ def version_commit_message_intro end # rubocop:enable Metrics/PerceivedComplexity - # rubocop:enable Metrics/CyclomaticComplexity # rubocop:enable Metrics/AbcSize def multidependency_property_intro