Inject the file_parser and use its attributes, Add a prepare! hook

Author: brrygrdn

bundler/spec/dependabot/bundler/dependency_grapher_spec.rb

@@ -10,8 +10,7 @@
   context "with a bundler project" do
     subject(:grapher) do
       Dependabot::DependencyGraphers.for_package_manager("bundler").new(
-        dependency_files:,
-        dependencies:
+        file_parser: parser
       )
     end
 
@@ -36,6 +35,8 @@
 
     let(:dependencies) { parser.parse }
 
+    before { grapher.prepare! }
+
     # NOTE: This documents existing behaviour where Gemfile PURLs do not include a resolved version
     #
     # Package URLs deal in resolved versions, so for a Gemfile only project we only have a range

common/lib/dependabot/dependency_graphers/base.rb

@@ -13,23 +13,16 @@ class Base
 
       abstract!
 
-      # TODO(brrygrdn): Inject the Dependency parser instead of pre-parsed `dependencies`
-      #
-      # Semantically it makes sense for the grapher to wrap the parser as a higher order function, but we already know
-      # that some package managers will require extra native commands before, after or during the parse - in extreme
-      # cases it may make sense to use an alternative parser that is more optimal.
-      #
-      # By injecting the parser, this allows the ecosystem to encapsulate the package manager specifics without the
-      # executor needing to manage parser modes / feature flags.
+      sig { returns(T::Boolean) }
+      attr_reader :prepared
+
       sig do
-        params(
-          dependency_files: T::Array[Dependabot::DependencyFile],
-          dependencies: T::Array[Dependabot::Dependency]
-        ).void
+        params(file_parser: Dependabot::FileParsers::Base).void
       end
-      def initialize(dependency_files:, dependencies:)
-        @dependency_files = dependency_files
-        @dependencies = dependencies
+      def initialize(file_parser:)
+        @file_parser = file_parser
+        @dependencies = T.let([], T::Array[Dependabot::Dependency])
+        @prepared = T.let(false, T::Boolean)
       end
 
       # Each grapher must implement a heuristic to determine which dependency file should be used as the owner
@@ -40,8 +33,21 @@ def initialize(dependency_files:, dependencies:)
       sig { abstract.returns(Dependabot::DependencyFile) }
       def relevant_dependency_file; end
 
+      # A grapher may override this method if it needs to perform extra steps around the normal file parser for
+      # the ecosystem.
+      sig { void }
+      def prepare!
+        @dependencies = @file_parser.parse
+        @prepared = true
+      end
+
       sig { returns(T::Hash[String, T.untyped]) }
       def resolved_dependencies
+        unless prepared
+          raise Dependabot::DependabotError,
+                "prepare! must be called before accessing resolved_dependencies"
+        end
+
         @dependencies.each_with_object({}) do |dep, resolved|
           resolved[dep.name] = {
             package_url: build_purl(dep),
@@ -55,6 +61,14 @@ def resolved_dependencies
 
       private
 
+      sig { returns(Dependabot::FileParsers::Base) }
+      attr_reader :file_parser
+
+      sig { returns(T::Array[Dependabot::DependencyFile]) }
+      def dependency_files
+        file_parser.dependency_files
+      end
+
       # Each grapher is expected to implement a method to look up the parents of a given dependency.
       #
       # The strategy that should be used is highly dependent on the ecosystem, in some cases the parser

common/lib/dependabot/dependency_graphers/generic.rb

@@ -25,7 +25,7 @@ def relevant_dependency_file
 
       sig { returns(T::Array[Dependabot::DependencyFile]) }
       def filtered_dependency_files
-        @dependency_files.reject { |f| f.support_file? || f.vendored_file? }
+        dependency_files.reject { |f| f.support_file? || f.vendored_file? }
       end
 
       # Our generic strategy is to check if the parser has attached a `depends_on` key to the Dependency's

go_modules/lib/dependabot/go_modules/dependency_grapher.rb

@@ -11,6 +11,8 @@ module GoModules
     class DependencyGrapher < Dependabot::DependencyGraphers::Base
       sig { override.returns(Dependabot::DependencyFile) }
       def relevant_dependency_file
+        # This cannot realistically happen as the parser will throw a runtime error on init without a go_mod file,
+        # but this will avoid surprises if anything changes.
         raise DependabotError, "No go.mod present in dependency files." unless go_mod
 
         T.must(go_mod)
@@ -32,7 +34,7 @@ def go_mod
         return @go_mod if defined?(@go_mod)
 
         @go_mod = T.let(
-          @dependency_files.find { |f| f.name = "go.mod" },
+          dependency_files.find { |f| f.name = "go.mod" },
           T.nilable(Dependabot::DependencyFile)
         )
       end

go_modules/spec/dependabot/go_modules/dependency_grapher_spec.rb

@@ -7,8 +7,7 @@
 RSpec.describe Dependabot::GoModules::DependencyGrapher do
   subject(:grapher) do
     Dependabot::DependencyGraphers.for_package_manager("go_modules").new(
-      dependency_files:,
-      dependencies:
+      file_parser: parser
     )
   end
 
@@ -34,38 +33,15 @@
   let(:dependencies) { parser.parse }
   let(:dependency_files) { [go_mod] }
 
+  before { grapher.prepare! }
+
   after do
     # Reset the environment variable after each test to avoid side effects
     ENV.delete("GOENV")
     ENV.delete("GOPROXY")
     ENV.delete("GOPRIVATE")
   end
 
-  context "when the go.mod is unexpectedly missing from dependency_files" do
-    # This scenario is very unlikely, it would most likely result from
-    # programmer error where the set of files passed in is malformed.
-    subject(:grapher) do
-      Dependabot::DependencyGraphers.for_package_manager("go_modules").new(
-        dependency_files: [],
-        dependencies:
-      )
-    end
-
-    let(:go_mod) do
-      Dependabot::DependencyFile.new(
-        name: "go.mod",
-        content: fixture("go_mods", "go.mod"),
-        directory: "/"
-      )
-    end
-
-    describe "#relevant_dependency_file" do
-      it "throws an exception" do
-        expect { grapher.relevant_dependency_file }.to raise_error(Dependabot::DependabotError, /No go.mod present/)
-      end
-    end
-  end
-
   context "with a simple project" do
     let(:go_mod) do
       Dependabot::DependencyFile.new(

updater/lib/dependabot/update_graph_processor.rb

@@ -96,7 +96,6 @@ def dependency_files_for(directory)
       ).returns(GithubApi::DependencySubmission)
     end
     def create_submission(source, files)
-      # TODO(brrygrdn): Refactor the grapher to wrap the parser call
       parser = Dependabot::FileParsers.for_package_manager(job.package_manager).new(
         dependency_files: files,
         repo_contents_path: job.repo_contents_path,
@@ -106,10 +105,8 @@ def create_submission(source, files)
         options: job.experiments
       )
 
-      grapher = Dependabot::DependencyGraphers.for_package_manager(job.package_manager).new(
-        dependency_files: files,
-        dependencies: parser.parse
-      )
+      grapher = Dependabot::DependencyGraphers.for_package_manager(job.package_manager).new(file_parser: parser)
+      grapher.prepare!
 
       GithubApi::DependencySubmission.new(
         job_id: job.id.to_s,