diff --git a/go_modules/lib/dependabot/go_modules/file_parser.rb b/go_modules/lib/dependabot/go_modules/file_parser.rb index b6b1050d0a4..0f5fe6e83ee 100644 --- a/go_modules/lib/dependabot/go_modules/file_parser.rb +++ b/go_modules/lib/dependabot/go_modules/file_parser.rb @@ -53,7 +53,7 @@ def dependency_from_details(details) Dependency.new( name: details["Path"], version: version, - requirements: details["Indirect"] || dependency_is_replaced(details) ? [] : reqs, + requirements: details["Indirect"] ? [] : reqs, package_manager: "go_modules" ) end @@ -155,14 +155,13 @@ def git_revision(dep) end def skip_dependency?(dep) - return true if dep["Indirect"] + # Updating replaced dependencies is not supported + return true if dependency_is_replaced(dep) - begin - path_uri = URI.parse("https://#{dep['Path']}") - !path_uri.host.include?(".") - rescue URI::InvalidURIError - false - end + path_uri = URI.parse("https://#{dep['Path']}") + !path_uri.host.include?(".") + rescue URI::InvalidURIError + false end def dependency_is_replaced(details) diff --git a/go_modules/lib/dependabot/go_modules/file_updater.rb b/go_modules/lib/dependabot/go_modules/file_updater.rb index 8b1f66a735d..04dfc3ec9e7 100644 --- a/go_modules/lib/dependabot/go_modules/file_updater.rb +++ b/go_modules/lib/dependabot/go_modules/file_updater.rb @@ -28,7 +28,7 @@ def self.updated_files_regex def updated_dependency_files updated_files = [] - if go_mod && file_changed?(go_mod) + if go_mod && dependency_changed?(go_mod) updated_files << updated_file( file: go_mod, @@ -56,6 +56,11 @@ def updated_dependency_files private + def dependency_changed?(go_mod) + # file_changed? only checks for changed requirements. Need to check for indirect dep version changes too. + file_changed?(go_mod) || dependencies.any? { |dep| dep.previous_version != dep.version } + end + def check_required_files return if go_mod diff --git a/go_modules/lib/dependabot/go_modules/update_checker.rb b/go_modules/lib/dependabot/go_modules/update_checker.rb index 5ad2e1ef9f9..bc030865a42 100644 --- a/go_modules/lib/dependabot/go_modules/update_checker.rb +++ b/go_modules/lib/dependabot/go_modules/update_checker.rb @@ -13,17 +13,6 @@ class UpdateChecker < Dependabot::UpdateCheckers::Base require_relative "update_checker/latest_version_finder" def latest_resolvable_version - # We don't yet support updating indirect dependencies for go_modules - # - # To update indirect dependencies we'll need to promote the indirect - # dependency to the go.mod file forcing the resolver to pick this - # version (possibly as `// indirect`) - unless dependency.top_level? - return unless dependency.version - - return current_version - end - latest_version_finder.latest_version end @@ -37,12 +26,6 @@ def latest_version def lowest_resolvable_security_fix_version raise "Dependency not vulnerable!" unless vulnerable? - unless dependency.top_level? - return unless dependency.version - - return current_version - end - lowest_security_fix_version end diff --git a/go_modules/lib/dependabot/go_modules/update_checker/latest_version_finder.rb b/go_modules/lib/dependabot/go_modules/update_checker/latest_version_finder.rb index 39b14fd588b..795b5bec84e 100644 --- a/go_modules/lib/dependabot/go_modules/update_checker/latest_version_finder.rb +++ b/go_modules/lib/dependabot/go_modules/update_checker/latest_version_finder.rb @@ -75,6 +75,10 @@ def fetch_lowest_security_fix_version end def available_versions + @available_versions ||= fetch_available_versions + end + + def fetch_available_versions SharedHelpers.in_a_temporary_directory do SharedHelpers.with_git_configured(credentials: credentials) do manifest = parse_manifest diff --git a/go_modules/spec/dependabot/go_modules/file_parser_spec.rb b/go_modules/spec/dependabot/go_modules/file_parser_spec.rb index 4111b9c348f..56b98c9ea12 100644 --- a/go_modules/spec/dependabot/go_modules/file_parser_spec.rb +++ b/go_modules/spec/dependabot/go_modules/file_parser_spec.rb @@ -40,7 +40,7 @@ describe "parse" do subject(:dependencies) { parser.parse } - its(:length) { is_expected.to eq(3) } + its(:length) { is_expected.to eq(4) } describe "top level dependencies" do subject(:dependencies) do @@ -130,16 +130,34 @@ end end + describe "indirect dependencies" do + subject(:dependencies) do + parser.parse.reject(&:top_level?) + end + + its(:length) { is_expected.to eq(2) } + + describe "a dependency that uses go modules" do + subject(:dependency) do + dependencies.find { |d| d.name == "github.com/mattn/go-isatty" } + end + + it "has the right details" do + expect(dependency).to be_a(Dependabot::Dependency) + expect(dependency.name).to eq("github.com/mattn/go-isatty") + expect(dependency.version).to eq("0.0.4") + expect(dependency.requirements).to be_empty + end + end + end + describe "a dependency that is replaced" do subject(:dependency) do dependencies.find { |d| d.name == "rsc.io/qr" } end - it "has the right details" do - expect(dependency).to be_a(Dependabot::Dependency) - expect(dependency.name).to eq("rsc.io/qr") - expect(dependency.version).to eq("0.1.0") - expect(dependency.requirements).to eq([]) + it "is skipped as unsupported" do + expect(dependency).to be_nil end end @@ -274,9 +292,8 @@ dependencies.find { |d| d.name == "rsc.io/qr" } end - it "has the right details" do - expect(dependency).to be_a(Dependabot::Dependency) - expect(dependency.name).to eq("rsc.io/qr") + it "is skipped as unsupported" do + expect(dependency).to be_nil end end @@ -325,7 +342,6 @@ it "parses root file" do expect(dependencies.map(&:name)). to eq(%w( - github.com/dependabot/vgotest/common rsc.io/qr )) end @@ -337,7 +353,6 @@ it "parses nested file" do expect(dependencies.map(&:name)). to eq(%w( - github.com/dependabot/vgotest/common rsc.io/qr )) end diff --git a/go_modules/spec/dependabot/go_modules/file_updater_spec.rb b/go_modules/spec/dependabot/go_modules/file_updater_spec.rb index 1e833fa0df4..554c814ff82 100644 --- a/go_modules/spec/dependabot/go_modules/file_updater_spec.rb +++ b/go_modules/spec/dependabot/go_modules/file_updater_spec.rb @@ -84,6 +84,19 @@ expect(updated_files.find { |f| f.name == "go.sum" }).to_not be_nil end + context "with an indirect dependency update" do + let(:requirements) { [] } + let(:previous_requirements) { [] } + + it "includes an updated go.mod" do + expect(updated_files.find { |f| f.name == "go.mod" }).to_not be_nil + end + + it "includes an updated go.sum" do + expect(updated_files.find { |f| f.name == "go.sum" }).to_not be_nil + end + end + context "with an invalid module path" do let(:stderr) do <<~STDERR diff --git a/go_modules/spec/dependabot/go_modules/update_checker_spec.rb b/go_modules/spec/dependabot/go_modules/update_checker_spec.rb index 980136f1710..e8a8a7405d0 100644 --- a/go_modules/spec/dependabot/go_modules/update_checker_spec.rb +++ b/go_modules/spec/dependabot/go_modules/update_checker_spec.rb @@ -63,12 +63,11 @@ module foobar end end - context "doesn't update indirect dependencies (not supported)" do + context "updates indirect dependencies" do let(:requirements) { [] } - it do - is_expected.to eq( - Dependabot::GoModules::Version.new(dependency.version) - ) + + it "updates to the newer version" do + is_expected.to eq(Dependabot::GoModules::Version.new("1.1.0")) end end @@ -123,12 +122,11 @@ module foobar end end - context "doesn't update indirect dependencies (not supported)" do + context "updates indirect dependencies" do let(:requirements) { [] } - it do - is_expected.to eq( - Dependabot::GoModules::Version.new(dependency.version) - ) + + it "updates to the least new supported version" do + is_expected.to eq(Dependabot::GoModules::Version.new("1.0.5")) end end