Merge pull request dependabot#5673 from dependabot/mctofu/removed-dep…

…-existing-pr

Handle removed dependencies in existing PRs

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,16 +579,17 @@ 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)
       new_pr_set = Set.new(
         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
 

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"] }