From 959b44f03f6cb2e94139e9db40d54ae0e5998f84 Mon Sep 17 00:00:00 2001
From: David McIntosh <804610+mctofu@users.noreply.github.com>
Date: Fri, 9 Sep 2022 18:38:42 -0700
Subject: [PATCH] Handle removed dependencies in existing PRs

---
 updater/lib/dependabot/updater.rb       | 23 ++++---
 updater/spec/dependabot/updater_spec.rb | 80 +++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 8 deletions(-)

diff --git a/updater/lib/dependabot/updater.rb b/updater/lib/dependabot/updater.rb
index 5131919cd1f..751dda4eb19 100644
--- a/updater/lib/dependabot/updater.rb
+++ b/updater/lib/dependabot/updater.rb
@@ -282,7 +282,11 @@ def check_and_create_pull_request(dependency)
         record_pull_request_exists_for_security_update(existing_pr) if job.security_updates_only?
 
         deps = existing_pr.map do |dep|
-          "#{dep.fetch('dependency-name')}@#{dep.fetch('dependency-version')}"
+          if dep.fetch("dependency-removed", false)
+            "#{dep.fetch('dependency-name')}@removed"
+          else
+            "#{dep.fetch('dependency-name')}@#{dep.fetch('dependency-version')}"
+          end
         end
 
         return logger_info(
@@ -425,8 +429,9 @@ def record_pull_request_exists_for_security_update(existing_pull_request)
       updated_dependencies = existing_pull_request.map do |dep|
         {
           "dependency-name": dep.fetch("dependency-name"),
-          "dependency-version": dep.fetch("dependency-version")
-        }
+          "dependency-version": dep.fetch("dependency-version", nil),
+          "dependency-removed": dep.fetch("dependency-removed", nil)
+        }.compact
       end
       record_error(
         {
@@ -574,7 +579,7 @@ def pr_exists_for_latest_version?(checker)
         select { |pr| pr.count == 1 }.
         map(&:first).
         select { |pr| pr.fetch("dependency-name") == checker.dependency.name }.
-        any? { |pr| pr.fetch("dependency-version") == latest_version }
+        any? { |pr| pr.fetch("dependency-version", nil) == latest_version }
     end
 
     def existing_pull_request(updated_dependencies)
@@ -582,8 +587,9 @@ def existing_pull_request(updated_dependencies)
         updated_dependencies.map do |dep|
           {
             "dependency-name" => dep.name,
-            "dependency-version" => dep.version
-          }
+            "dependency-version" => dep.version,
+            "dependency-removed" => dep.removed? ? true : nil
+          }.compact
         end
       )
 
@@ -744,8 +750,9 @@ def create_pull_request(dependencies, updated_dependency_files, pr_message)
       created_pull_requests << dependencies.map do |dep|
         {
           "dependency-name" => dep.name,
-          "dependency-version" => dep.version
-        }
+          "dependency-version" => dep.version,
+          "dependency-removed" => dep.removed? ? true : nil
+        }.compact
       end
     end
 
diff --git a/updater/spec/dependabot/updater_spec.rb b/updater/spec/dependabot/updater_spec.rb
index 9d4bd6da341..662e4f13988 100644
--- a/updater/spec/dependabot/updater_spec.rb
+++ b/updater/spec/dependabot/updater_spec.rb
@@ -1006,6 +1006,86 @@ def expect_update_checker_with_ignored_versions(versions)
       end
     end
 
+    context "when a PR already exists for a removed dependency" do
+      let(:existing_pull_requests) do
+        [
+          [
+            {
+              "dependency-name" => "dummy-pkg-c",
+              "dependency-version" => "1.4.0"
+            },
+            {
+              "dependency-name" => "dummy-pkg-b",
+              "dependency-removed" => true
+            }
+          ]
+        ]
+      end
+
+      let(:security_updates_only) { true }
+      let(:security_advisories) do
+        [{ "dependency-name" => "dummy-pkg-b",
+           "affected-versions" => ["1.1.0"] }]
+      end
+
+      before do
+        allow(checker).
+          to receive(:latest_version).
+          and_return(Gem::Version.new("1.3.0"))
+        allow(checker).to receive(:vulnerable?).and_return(true)
+        allow(checker).to receive(:updated_dependencies).and_return([
+          Dependabot::Dependency.new(
+            name: "dummy-pkg-b",
+            package_manager: "bundler",
+            previous_version: "1.1.0",
+            requirements: [],
+            previous_requirements: [],
+            removed: true
+          ),
+          Dependabot::Dependency.new(
+            name: "dummy-pkg-c",
+            package_manager: "bundler",
+            version: "1.4.0",
+            previous_version: "1.3.0",
+            requirements: [
+              { file: "Gemfile", requirement: "~> 1.4.0", groups: [], source: nil }
+            ],
+            previous_requirements: [
+              { file: "Gemfile", requirement: "~> 1.3.0", groups: [], source: nil }
+            ]
+          )
+        ])
+      end
+
+      it "creates an update job error and short-circuits" do
+        expect(checker).to receive(:up_to_date?).and_return(false)
+        expect(checker).to receive(:can_update?).and_return(true)
+        expect(updater).to_not receive(:generate_dependency_files_for)
+        expect(service).to_not receive(:create_pull_request)
+        expect(service).to receive(:record_update_job_error).
+          with(
+            1,
+            error_type: "pull_request_exists_for_security_update",
+            error_details: {
+              "updated-dependencies": [
+                {
+                  "dependency-name": "dummy-pkg-c",
+                  "dependency-version": "1.4.0"
+                },
+                {
+                  "dependency-name": "dummy-pkg-b",
+                  "dependency-removed": true
+                }
+              ]
+            }
+          )
+        expect(logger).
+          to receive(:info).
+          with("<job_1> Pull request already exists for dummy-pkg-c@1.4.0, dummy-pkg-b@removed")
+        updater.run
+      end
+    end
+
     context "when a list of dependencies is specified" do
       let(:requested_dependencies) { ["dummy-pkg-b"] }