From be072377fdaefd268567c5b1cca9fd588310c9c6 Mon Sep 17 00:00:00 2001 From: kbukum1 Date: Mon, 26 Aug 2024 13:34:41 -0700 Subject: [PATCH] Implement Logging for Bundler v1 Deprecation Warnings (#10466) * replaced warning `message` as `description` to reduce confusion logged deprecation warning. * moved the warning to the top of the job * remove the markdown field, and created generation markdown method. * moved deprecation notice generation in the dependency_snapshot --- common/lib/dependabot/notices.rb | 140 ++++++++++++------ .../pull_request_creator/message_builder.rb | 3 +- common/spec/dependabot/notices_spec.rb | 62 ++++---- .../message_builder_spec.rb | 58 +++++--- updater/lib/dependabot/dependency_snapshot.rb | 25 +++- updater/lib/dependabot/notices_helpers.rb | 74 +++++++++ .../updater/group_update_creation.rb | 10 +- .../create_security_update_pull_request.rb | 18 +-- .../refresh_security_update_pull_request.rb | 18 +-- .../refresh_version_update_pull_request.rb | 18 +-- .../updater/operations/update_all_versions.rb | 18 +-- .../updater/security_update_helpers.rb | 34 ----- ...elpers_spec.rb => notices_helpers_spec.rb} | 60 +++++--- .../create_group_update_pull_request_spec.rb | 21 +-- ...reate_security_update_pull_request_spec.rb | 13 +- ...fresh_security_update_pull_request_spec.rb | 15 +- ...efresh_version_update_pull_request_spec.rb | 10 +- .../operations/update_all_versions_spec.rb | 19 +-- 18 files changed, 367 insertions(+), 249 deletions(-) create mode 100644 updater/lib/dependabot/notices_helpers.rb rename updater/spec/dependabot/{updater/pull_request_helpers_spec.rb => notices_helpers_spec.rb} (69%) diff --git a/common/lib/dependabot/notices.rb b/common/lib/dependabot/notices.rb index 9daef0c302..100732aec2 100644 --- a/common/lib/dependabot/notices.rb +++ b/common/lib/dependabot/notices.rb @@ -6,32 +6,51 @@ module Dependabot class Notice + module NoticeMode + INFO = "INFO" + WARN = "WARN" + ERROR = "ERROR" + end + extend T::Sig sig { returns(String) } - attr_reader :mode, :type, :package_manager_name, :message, :markdown + attr_reader :mode, :type, :package_manager_name, :title, :description + + sig { returns(T::Boolean) } + attr_reader :show_in_pr, :show_alert # Initializes a new Notice object. # @param mode [String] The mode of the notice (e.g., "WARN", "ERROR"). # @param type [String] The type of the notice (e.g., "bundler_deprecated_warn"). # @param package_manager_name [String] The name of the package manager (e.g., "bundler"). - # @param message [String] The main message of the notice. - # @param markdown [String] The markdown formatted message. + # @param title [String] The title of the notice. + # @param description [String] The main description of the notice. + # @param show_in_pr [Boolean] Whether the notice should be shown in a pull request. + # @param show_alert [Boolean] Whether the notice should be shown in alerts. sig do params( mode: String, type: String, package_manager_name: String, - message: String, - markdown: String + title: String, + description: String, + show_in_pr: T::Boolean, + show_alert: T::Boolean ).void end - def initialize(mode:, type:, package_manager_name:, message: "", markdown: "") + def initialize( + mode:, type:, package_manager_name:, + title: "", description: "", + show_in_pr: false, show_alert: false + ) @mode = mode @type = type @package_manager_name = package_manager_name - @message = message - @markdown = markdown + @title = title + @description = description + @show_in_pr = show_in_pr + @show_alert = show_alert end # Converts the Notice object to a hash. @@ -42,23 +61,25 @@ def to_hash mode: @mode, type: @type, package_manager_name: @package_manager_name, - message: @message, - markdown: @markdown + title: @title, + description: @description, + show_in_pr: @show_in_pr, + show_alert: @show_alert } end - # Generates a message for supported versions. + # Generates a description for supported versions. # @param supported_versions [Array, nil] The supported versions of the package manager. # @param support_later_versions [Boolean] Whether later versions are supported. - # @return [String, nil] The generated message or nil if no supported versions are provided. + # @return [String, nil] The generated description or nil if no supported versions are provided. sig do params( supported_versions: T.nilable(T::Array[Dependabot::Version]), support_later_versions: T::Boolean ).returns(String) end - def self.generate_supported_versions_message(supported_versions, support_later_versions) - return "" unless supported_versions&.any? + def self.generate_supported_versions_description(supported_versions, support_later_versions) + return "Please upgrade your package manager version" unless supported_versions&.any? versions_string = supported_versions.map { |version| "`v#{version}`" } @@ -66,11 +87,11 @@ def self.generate_supported_versions_message(supported_versions, support_later_v versions_string = versions_string.join(", ") - later_message = support_later_versions ? ", or later" : "" + later_description = support_later_versions ? ", or later" : "" - return "Please upgrade to version #{versions_string}#{later_message}." if supported_versions.count == 1 + return "Please upgrade to version #{versions_string}#{later_description}." if supported_versions.count == 1 - "Please upgrade to one of the following versions: #{versions_string}#{later_message}." + "Please upgrade to one of the following versions: #{versions_string}#{later_description}." end # Generates a support notice for the given package manager. @@ -100,30 +121,26 @@ def self.generate_support_notice(package_manager) def self.generate_pm_deprecation_notice(package_manager) return nil unless package_manager.deprecated? - mode = "WARN" - supported_versions_message = generate_supported_versions_message( + mode = NoticeMode::WARN + supported_versions_description = generate_supported_versions_description( package_manager.supported_versions, package_manager.support_later_versions? ) - notice_type = "#{package_manager.name}_deprecated_#{mode.downcase}" - message = "Dependabot will stop supporting `#{package_manager.name} v#{package_manager.version}`!" - ## Create a warning markdown message - markdown = "> [!WARNING]\n" - ## Add the deprecation warning to the message - markdown += "> #{message}\n>\n" - - ## Add the supported versions to the message - unless supported_versions_message.empty? - message += "\n#{supported_versions_message}\n" - markdown += "> #{supported_versions_message}\n>\n" - end + notice_type = "#{package_manager.name}_deprecated_warn" + title = "Package manager deprecation notice" + description = "Dependabot will stop supporting `#{package_manager.name} v#{package_manager.version}`!" + + ## Add the supported versions to the description + description += "\n\n#{supported_versions_description}\n" unless supported_versions_description.empty? Notice.new( mode: mode, type: notice_type, package_manager_name: package_manager.name, - message: message, - markdown: markdown + title: title, + description: description, + show_in_pr: true, + show_alert: true ) end @@ -138,31 +155,56 @@ def self.generate_pm_deprecation_notice(package_manager) def self.generate_pm_unsupported_notice(package_manager) return nil unless package_manager.unsupported? - mode = "ERROR" - supported_versions_message = generate_supported_versions_message( + mode = NoticeMode::ERROR + supported_versions_description = generate_supported_versions_description( package_manager.supported_versions, package_manager.support_later_versions? ) - notice_type = "#{package_manager.name}_unsupported_#{mode.downcase}" - message = "Dependabot no longer supports `#{package_manager.name} v#{package_manager.version}`!" - ## Create an error markdown message - markdown = "> [!IMPORTANT]\n" - ## Add the error message to the message - markdown += "> #{message}\n>\n" - - ## Add the supported versions to the message - unless supported_versions_message.empty? - message += "\n#{supported_versions_message}\n" - markdown += "> #{supported_versions_message}\n>\n" - end + notice_type = "#{package_manager.name}_unsupported_error" + title = "Package manager unsupported notice" + description = "Dependabot no longer supports `#{package_manager.name} v#{package_manager.version}`!" + + ## Add the supported versions to the description + description += "\n\n#{supported_versions_description}\n" unless supported_versions_description.empty? Notice.new( mode: mode, type: notice_type, package_manager_name: package_manager.name, - message: message, - markdown: markdown + title: title, + description: description, + show_in_pr: true, + show_alert: true ) end + + sig { params(notice: Notice).returns(T.nilable(String)) } + def self.markdown_from_description(notice) + description = notice.description + + return if description.empty? + + markdown = "> [!#{markdown_mode(notice.mode)}]\n" + # Log each non-empty line of the deprecation notice description + description.each_line do |line| + line = line.strip + markdown += "> #{line}\n" + end + markdown + end + + sig { params(mode: String).returns(String) } + def self.markdown_mode(mode) + case mode + when NoticeMode::INFO + "INFO" + when NoticeMode::WARN + "WARNING" + when NoticeMode::ERROR + "IMPORTANT" + else + "INFO" + end + end end end diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index 1968cfb3f6..98b3161a46 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -143,8 +143,7 @@ def pr_message def pr_notices notices = @notices || [] unique_messages = notices.filter_map do |notice| - markdown = notice.markdown if notice - markdown unless markdown.empty? + Dependabot::Notice.markdown_from_description(notice) if notice.show_in_pr end.uniq message = unique_messages.join("\n\n") diff --git a/common/spec/dependabot/notices_spec.rb b/common/spec/dependabot/notices_spec.rb index c15a64a2bb..c1e4c4edcf 100644 --- a/common/spec/dependabot/notices_spec.rb +++ b/common/spec/dependabot/notices_spec.rb @@ -26,17 +26,17 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end RSpec.describe Dependabot::Notice do - describe ".generate_supported_versions_message" do - subject(:generate_supported_versions_message) do - described_class.generate_supported_versions_message(supported_versions, support_later_versions) + describe ".generate_supported_versions_description" do + subject(:generate_supported_versions_description) do + described_class.generate_supported_versions_description(supported_versions, support_later_versions) end context "when supported_versions has one version" do let(:supported_versions) { [Dependabot::Version.new("2")] } let(:support_later_versions) { false } - it "returns the correct message" do - expect(generate_supported_versions_message) + it "returns the correct description" do + expect(generate_supported_versions_description) .to eq("Please upgrade to version `v2`.") end end @@ -45,8 +45,8 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ let(:supported_versions) { [Dependabot::Version.new("2")] } let(:support_later_versions) { true } - it "returns the correct message" do - expect(generate_supported_versions_message) + it "returns the correct description" do + expect(generate_supported_versions_description) .to eq("Please upgrade to version `v2`, or later.") end end @@ -58,8 +58,8 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end let(:support_later_versions) { false } - it "returns the correct message" do - expect(generate_supported_versions_message) + it "returns the correct description" do + expect(generate_supported_versions_description) .to eq("Please upgrade to one of the following versions: `v2`, `v3`, or `v4`.") end end @@ -71,8 +71,8 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end let(:support_later_versions) { true } - it "returns the correct message" do - expect(generate_supported_versions_message) + it "returns the correct description" do + expect(generate_supported_versions_description) .to eq("Please upgrade to one of the following versions: `v2`, `v3`, `v4`, or later.") end end @@ -82,7 +82,7 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ let(:support_later_versions) { false } it "returns empty string" do - expect(generate_supported_versions_message).to eq("") + expect(generate_supported_versions_description).to eq("Please upgrade your package manager version") end end @@ -91,7 +91,7 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ let(:support_later_versions) { false } it "returns nil" do - expect(generate_supported_versions_message).to eq("") + expect(generate_supported_versions_description).to eq("Please upgrade your package manager version") end end end @@ -124,10 +124,11 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true }) end end @@ -142,10 +143,11 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ mode: "ERROR", type: "bundler_unsupported_error", package_manager_name: "bundler", - message: "Dependabot no longer supports `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!IMPORTANT]\n> Dependabot no longer supports `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager unsupported notice", + description: "Dependabot no longer supports `bundler v1`!" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true }) end end @@ -194,10 +196,11 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true }) end end @@ -222,10 +225,11 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ mode: "ERROR", type: "bundler_unsupported_error", package_manager_name: "bundler", - message: "Dependabot no longer supports `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!IMPORTANT]\n> Dependabot no longer supports `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager unsupported notice", + description: "Dependabot no longer supports `bundler v1`!" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true }) end end 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 d9f6acb748..e8d5389a3f 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -3324,15 +3324,18 @@ def commits_details(base:, head:) mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true )] end it do - expect(pr_message).to start_with(notices[0].markdown) + expect(pr_message).to start_with( + Dependabot::Notice.markdown_from_description(notices.first) + ) end end @@ -3342,24 +3345,28 @@ def commits_details(base:, head:) mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true ), Dependabot::Notice.new( mode: "ERROR", type: "bundler_unsupported_error", package_manager_name: "bundler", - message: "Dependabot no longer supports `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!IMPORTANT]\n> Dependabot no longer supports `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager deprecation notice", + description: "Dependabot no longer supports `bundler v1`!\n" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true )] end it do + markdown1 = Dependabot::Notice.markdown_from_description(notices.first) + markdown2 = Dependabot::Notice.markdown_from_description(notices.last) expect(pr_message).to start_with( - "#{notices[0].markdown}\n\n#{notices[1].markdown}" + "#{markdown1}\n\n#{markdown2}" ) end end @@ -3370,24 +3377,27 @@ def commits_details(base:, head:) mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true ), Dependabot::Notice.new( mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true )] end it "returns a unique message" do - expect(pr_message).to start_with(notices[0].markdown) - expect(pr_message.scan(notices[0].markdown).count).to eq(1) + markdown = Dependabot::Notice.markdown_from_description(notices.first) + expect(pr_message).to start_with(markdown) + expect(pr_message.scan(markdown).count).to eq(1) end end end diff --git a/updater/lib/dependabot/dependency_snapshot.rb b/updater/lib/dependabot/dependency_snapshot.rb index 15e4318ba6..30f3a0b7c7 100644 --- a/updater/lib/dependabot/dependency_snapshot.rb +++ b/updater/lib/dependabot/dependency_snapshot.rb @@ -5,6 +5,7 @@ require "sorbet-runtime" require "dependabot/file_parsers" +require "dependabot/notices_helpers" # This class describes the dependencies obtained from a project at a specific commit SHA # including both the Dependabot::DependencyFile objects at that reference as well as @@ -15,6 +16,7 @@ module Dependabot class DependencySnapshot extend T::Sig + include NoticesHelpers sig do params(job: Dependabot::Job, job_definition: T::Hash[String, T.untyped]).returns(Dependabot::DependencySnapshot) @@ -70,6 +72,13 @@ def package_manager @package_manager[@current_directory] end + sig { returns(T::Array[Dependabot::Notice]) } + def notices + # The notices array in dependency snapshot stay immutable, + # so we can return a copy + @notices[@current_directory]&.dup || [] + end + # Returns the subset of all project dependencies which are permitted # by the project configuration. sig { returns(T::Array[Dependabot::Dependency]) } @@ -173,6 +182,7 @@ def initialize(job:, base_commit_sha:, dependency_files:) # rubocop:disable Metr @dependencies = T.let({}, T::Hash[String, T::Array[Dependabot::Dependency]]) @package_manager = T.let({}, T::Hash[String, T.nilable(Dependabot::PackageManagerBase)]) + @notices = T.let({}, T::Hash[String, T::Array[Dependabot::Notice]]) directories.each do |dir| @current_directory = dir @@ -232,7 +242,20 @@ def dependency_file_parser options: job.experiments ) # Add 'package_manager' to the depedency_snapshopt to use it in operations' - @package_manager[@current_directory] = parser.package_manager + package_manager_for_current_directory = parser.package_manager + @package_manager[@current_directory] = package_manager_for_current_directory + + # Log deprecation notices if the package manager is deprecated + # and add them to the notices array + notices_for_current_directory = [] + + # add deprecation notices for the package manager + add_deprecation_notice( + notices: notices_for_current_directory, + package_manager: package_manager_for_current_directory + ) + @notices[@current_directory] = notices_for_current_directory + parser end diff --git a/updater/lib/dependabot/notices_helpers.rb b/updater/lib/dependabot/notices_helpers.rb new file mode 100644 index 0000000000..20df43604b --- /dev/null +++ b/updater/lib/dependabot/notices_helpers.rb @@ -0,0 +1,74 @@ +# typed: strong +# frozen_string_literal: true + +require "sorbet-runtime" +require "dependabot/notices" +require "dependabot/package_manager" + +# This module extracts helpers for notice generations that can be used +# for showing notices in logs, pr messages and alert ui page. +module Dependabot + module NoticesHelpers + extend T::Sig + extend T::Helpers + + abstract! + + # Add a deprecation notice to the notice list if the package manager is deprecated + # if the package manager is deprecated. + # notices << deprecation_notices if deprecation_notices + sig do + params( + notices: T::Array[Dependabot::Notice], + package_manager: T.nilable(PackageManagerBase) + ) + .void + end + def add_deprecation_notice(notices:, package_manager:) + # Create a deprecation notice if the package manager is deprecated + deprecation_notice = create_deprecation_notice(package_manager) + + return unless deprecation_notice + + log_notice(deprecation_notice) + + notices << deprecation_notice + end + + sig { params(notice: Dependabot::Notice).void } + def log_notice(notice) + # Log each non-empty line of the deprecation notice description + notice.description.each_line do |line| + line = line.strip + next if line.empty? + + case notice.mode + when Dependabot::Notice::NoticeMode::INFO + return Dependabot.logger.info(line) + when Dependabot::Notice::NoticeMode::WARN + Dependabot.logger.warn(line) + when Dependabot::Notice::NoticeMode::ERROR + Dependabot.logger.error(line) + else + Dependabot.logger.info(line) + end + end + end + + private + + sig { params(package_manager: T.nilable(PackageManagerBase)).returns(T.nilable(Dependabot::Notice)) } + def create_deprecation_notice(package_manager) + # Feature flag check if deprecation notice should be added to notices. + return unless Dependabot::Experiments.enabled?(:add_deprecation_warn_to_pr_message) + + return unless package_manager + + return unless package_manager.is_a?(PackageManagerBase) + + Notice.generate_pm_deprecation_notice( + package_manager + ) + end + end +end diff --git a/updater/lib/dependabot/updater/group_update_creation.rb b/updater/lib/dependabot/updater/group_update_creation.rb index 96458e2f3c..202f217865 100644 --- a/updater/lib/dependabot/updater/group_update_creation.rb +++ b/updater/lib/dependabot/updater/group_update_creation.rb @@ -24,7 +24,6 @@ class Updater module GroupUpdateCreation extend T::Sig extend T::Helpers - include PullRequestHelpers abstract! @@ -55,13 +54,8 @@ def compile_all_dependency_changes_for(group) ) original_dependencies = dependency_snapshot.dependencies - notices = [] - - # Add a deprecation notice if the package manager is deprecated - add_deprecation_notice( - notices: notices, - package_manager: dependency_snapshot.package_manager - ) + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + notices = dependency_snapshot.notices Dependabot.logger.info("Updating the #{job.source.directory} directory.") group.dependencies.each do |dependency| diff --git a/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb index ac69fe2d56..24568df99a 100644 --- a/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb @@ -13,7 +13,6 @@ module Operations class CreateSecurityUpdatePullRequest extend T::Sig include SecurityUpdateHelpers - include PullRequestHelpers sig { params(job: Job).returns(T::Boolean) } def self.applies_to?(job:) @@ -45,8 +44,8 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) @error_handler = error_handler # TODO: Collect @created_pull_requests on the Job object? @created_pull_requests = T.let([], T::Array[PullRequest]) - - @pr_notices = T.let([], T::Array[Dependabot::Notice]) + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + @notices = T.let([], T::Array[Dependabot::Notice]) end # TODO: We currently tolerate multiple dependencies for this operation @@ -59,11 +58,9 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) def perform Dependabot.logger.info("Starting security update job for #{job.source.repo}") - # Add a deprecation notice if the package manager is deprecated - add_deprecation_notice( - notices: @pr_notices, - package_manager: dependency_snapshot.package_manager - ) + # Retrieve the list of initial notices from dependency snapshot + @notices = dependency_snapshot.notices + # More notices can be added during the update process target_dependencies = dependency_snapshot.job_dependencies @@ -86,6 +83,9 @@ def perform attr_reader :error_handler sig { returns(T::Array[PullRequest]) } attr_reader :created_pull_requests + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + sig { returns(T::Array[Dependabot::Notice]) } + attr_reader :notices sig { params(dependency: Dependabot::Dependency).void } def check_and_create_pr_with_error_handling(dependency) @@ -180,7 +180,7 @@ def check_and_create_pull_request(dependency) dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, change_source: checker.dependency, - notices: @pr_notices + notices: @notices ) create_pull_request(dependency_change) diff --git a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb index de8ac93120..a818c8dfb9 100644 --- a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb @@ -19,7 +19,6 @@ module Operations class RefreshSecurityUpdatePullRequest extend T::Sig include SecurityUpdateHelpers - include PullRequestHelpers sig { params(job: Job).returns(T::Boolean) } def self.applies_to?(job:) @@ -45,8 +44,8 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) @job = job @dependency_snapshot = dependency_snapshot @error_handler = error_handler - - @pr_notices = T.let([], T::Array[Dependabot::Notice]) + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + @notices = T.let([], T::Array[Dependabot::Notice]) end sig { void } @@ -54,11 +53,9 @@ def perform Dependabot.logger.info("Starting update job for #{job.source.repo}") Dependabot.logger.info("Checking and updating security pull requests...") - # Add a deprecation notice if the package manager is deprecated - add_deprecation_notice( - notices: @pr_notices, - package_manager: dependency_snapshot.package_manager - ) + # Retrieve the list of initial notices from dependency snapshot + @notices = dependency_snapshot.notices + # More notices can be added during the update process check_and_update_pull_request(dependencies) rescue StandardError => e @@ -75,6 +72,9 @@ def perform attr_reader :dependency_snapshot sig { returns(Dependabot::Updater::ErrorHandler) } attr_reader :error_handler + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + sig { returns(T::Array[Dependabot::Notice]) } + attr_reader :notices sig { returns(T::Array[Dependabot::Dependency]) } def dependencies @@ -158,7 +158,7 @@ def check_and_update_pull_request(dependencies) dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, change_source: checker.dependency, - notices: @pr_notices + notices: @notices ) # NOTE: Gradle, Maven and Nuget dependency names can be case-insensitive diff --git a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb index 5da1fce0ac..747926e940 100644 --- a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb @@ -16,7 +16,6 @@ class Updater module Operations class RefreshVersionUpdatePullRequest extend T::Sig - include PullRequestHelpers sig { params(job: Dependabot::Job).returns(T::Boolean) } def self.applies_to?(job:) @@ -46,8 +45,8 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) @job = job @dependency_snapshot = dependency_snapshot @error_handler = error_handler - - @pr_notices = T.let([], T::Array[Dependabot::Notice]) + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + @notices = T.let([], T::Array[Dependabot::Notice]) return unless job.source.directory.nil? && job.source.directories&.count == 1 @@ -60,11 +59,9 @@ def perform Dependabot.logger.info("Checking and updating versions pull requests...") dependency = dependencies.last - # Add a deprecation notice if the package manager is deprecated - add_deprecation_notice( - notices: @pr_notices, - package_manager: dependency_snapshot.package_manager - ) + # Retrieve the list of initial notices from dependency snapshot + @notices = dependency_snapshot.notices + # More notices can be added during the update process check_and_update_pull_request(dependencies) rescue StandardError => e @@ -81,6 +78,9 @@ def perform attr_reader :dependency_snapshot sig { returns(Dependabot::Updater::ErrorHandler) } attr_reader :error_handler + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + sig { returns(T::Array[Dependabot::Notice]) } + attr_reader :notices sig { returns(T::Array[Dependabot::Dependency]) } def dependencies @@ -146,7 +146,7 @@ def check_and_update_pull_request(dependencies) dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, change_source: checker.dependency, - notices: @pr_notices + notices: @notices ) # NOTE: Gradle, Maven and Nuget dependency names can be case-insensitive diff --git a/updater/lib/dependabot/updater/operations/update_all_versions.rb b/updater/lib/dependabot/updater/operations/update_all_versions.rb index 9730766f75..926a81e6ec 100644 --- a/updater/lib/dependabot/updater/operations/update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/update_all_versions.rb @@ -12,7 +12,6 @@ class Updater module Operations class UpdateAllVersions extend T::Sig - include PullRequestHelpers sig { params(_job: Dependabot::Job).returns(T::Boolean) } def self.applies_to?(_job:) @@ -39,8 +38,8 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) @error_handler = error_handler # TODO: Collect @created_pull_requests on the Job object? @created_pull_requests = T.let([], T::Array[PullRequest]) - - @pr_notices = T.let([], T::Array[Dependabot::Notice]) + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + @notices = T.let([], T::Array[Dependabot::Notice]) return unless job.source.directory.nil? && job.source.directories&.count == 1 @@ -52,11 +51,9 @@ def perform Dependabot.logger.info("Starting update job for #{job.source.repo}") Dependabot.logger.info("Checking all dependencies for version updates...") - # Add a deprecation notice if the package manager is deprecated - add_deprecation_notice( - notices: @pr_notices, - package_manager: dependency_snapshot.package_manager - ) + # Retrieve the list of initial notices from dependency snapshot + @notices = dependency_snapshot.notices + # More notices can be added during the update process dependencies.each { |dep| check_and_create_pr_with_error_handling(dep) } end @@ -73,6 +70,9 @@ def perform attr_reader :error_handler sig { returns(T::Array[PullRequest]) } attr_reader :created_pull_requests + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + sig { returns(T::Array[Dependabot::Notice]) } + attr_reader :notices sig { returns(T::Array[Dependabot::Dependency]) } def dependencies @@ -167,7 +167,7 @@ def check_and_create_pull_request(dependency) dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, change_source: checker.dependency, - notices: @pr_notices + notices: @notices ) if dependency_change.updated_dependency_files.empty? diff --git a/updater/lib/dependabot/updater/security_update_helpers.rb b/updater/lib/dependabot/updater/security_update_helpers.rb index 253e8fbd39..558a88234c 100644 --- a/updater/lib/dependabot/updater/security_update_helpers.rb +++ b/updater/lib/dependabot/updater/security_update_helpers.rb @@ -181,39 +181,5 @@ def security_update_not_possible_message(checker, latest_allowed_version, confli end end end - - module PullRequestHelpers - extend T::Sig - extend T::Helpers - - abstract! - - # Add deprecation notices to the list of notices - # if the package manager is deprecated. - # notices << deprecation_notices if deprecation_notices - sig do - params( - notices: T::Array[Dependabot::Notice], - package_manager: T.nilable(PackageManagerBase) - ) - .void - end - def add_deprecation_notice(notices:, package_manager:) - return unless Dependabot::Experiments.enabled?( - :add_deprecation_warn_to_pr_message - ) - return unless package_manager - - return unless package_manager.is_a?(PackageManagerBase) - - deprecation_notice = Notice.generate_pm_deprecation_notice( - package_manager - ) - - return unless deprecation_notice - - notices << deprecation_notice - end - end end end diff --git a/updater/spec/dependabot/updater/pull_request_helpers_spec.rb b/updater/spec/dependabot/notices_helpers_spec.rb similarity index 69% rename from updater/spec/dependabot/updater/pull_request_helpers_spec.rb rename to updater/spec/dependabot/notices_helpers_spec.rb index 977490748c..3c79e0ad45 100644 --- a/updater/spec/dependabot/updater/pull_request_helpers_spec.rb +++ b/updater/spec/dependabot/notices_helpers_spec.rb @@ -5,11 +5,12 @@ require "dependabot/updater" require "dependabot/package_manager" require "dependabot/notices" +require "dependabot/notices_helpers" -RSpec.describe Dependabot::Updater::PullRequestHelpers do +RSpec.describe Dependabot::NoticesHelpers do let(:dummy_class) do Class.new do - include Dependabot::Updater::PullRequestHelpers + include Dependabot::NoticesHelpers attr_accessor :notices @@ -21,6 +22,26 @@ def initialize let(:dummy_instance) { dummy_class.new } + let(:package_manager) do + Class.new(Dependabot::PackageManagerBase) do + def name + "bundler" + end + + def version + Dependabot::Version.new("1") + end + + def deprecated_versions + [Dependabot::Version.new("1")] + end + + def supported_versions + [Dependabot::Version.new("2"), Dependabot::Version.new("3")] + end + end.new + end + before do allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) end @@ -30,26 +51,6 @@ def initialize end describe "#add_deprecation_notice" do - let(:package_manager) do - Class.new(Dependabot::PackageManagerBase) do - def name - "bundler" - end - - def version - Dependabot::Version.new("1") - end - - def deprecated_versions - [Dependabot::Version.new("1")] - end - - def supported_versions - [Dependabot::Version.new("2"), Dependabot::Version.new("3")] - end - end.new - end - context "when package manager is provided and is deprecated" do it "adds a deprecation notice to the notices array" do expect do @@ -62,6 +63,21 @@ def supported_versions expect(notice.type).to eq("bundler_deprecated_warn") expect(notice.package_manager_name).to eq("bundler") end + + it "logs deprecation notices line by line" do + allow(Dependabot.logger).to receive(:warn) + + dummy_instance.add_deprecation_notice(notices: dummy_instance.notices, package_manager: package_manager) + + notice = dummy_instance.notices.first + notice.description.each_line do |line| + line = line.strip + next if line.empty? + + puts "except lines: ##{line}#" + expect(Dependabot.logger).to have_received(:warn).with(line).once + end + end end context "when package manager is not provided" do diff --git a/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb index e3f7fb287c..0f19a7a515 100644 --- a/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/create_group_update_pull_request_spec.rb @@ -122,21 +122,23 @@ mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true ).to_hash ] ) end before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) + allow(Dependabot::UpdateCheckers).to receive(:for_package_manager).and_return(stub_update_checker_class) allow(Dependabot::DependencyChangeBuilder) .to receive(:create_from) .and_return(stub_dependency_change) - allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) end after do @@ -162,10 +164,11 @@ mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true } ) create_group_update_pull_request.perform diff --git a/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb index 4d9a08cb5a..ab7e1c0e25 100644 --- a/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/create_security_update_pull_request_spec.rb @@ -203,6 +203,8 @@ def support_later_versions? let(:mock_package_manager_instance) { concrete_package_manager_class.new } before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) + # Allow for_package_manager to return the stub_update_checker_class allow(Dependabot::UpdateCheckers).to receive(:for_package_manager).and_return(stub_update_checker_class) @@ -219,8 +221,6 @@ def support_later_versions? allow(dependency_snapshot) .to receive(:package_manager) .and_return(mock_package_manager_instance) - - allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) end after do @@ -369,10 +369,11 @@ def support_later_versions? mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true ) ) diff --git a/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb index f2609a16e1..6a25291d84 100644 --- a/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb @@ -112,11 +112,12 @@ end before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) + allow(Dependabot::UpdateCheckers).to receive(:for_package_manager).and_return(stub_update_checker_class) allow(Dependabot::DependencyChangeBuilder) .to receive(:create_from) .and_return(stub_dependency_change) - allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) end after do @@ -206,13 +207,11 @@ mode: "WARN", type: "bundler_deprecated_warn", package_manager_name: "bundler", - details: { - message: "Dependabot will stop supporting `bundler v1`!\n" \ - "Please upgrade to one of the following versions: `v2`, or `v3`.\n", - current_version: "v1", - markdown: "> [!WARNING]\n> Dependabot will stop supporting `bundler v1`!\n>\n" \ - "> Please upgrade to one of the following versions: `v2`, or `v3`.\n>\n" - } + title: "Package manager deprecation notice", + description: "Dependabot will stop supporting `bundler v1`!\n" \ + "\n\nPlease upgrade to one of the following versions: `v2`, or `v3`.\n", + show_in_pr: true, + show_alert: true }]) expect(refresh_security_update_pull_request).to receive(:create_pull_request) refresh_security_update_pull_request.send(:check_and_update_pull_request, [dependency]) diff --git a/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb index 146dcf1fef..8bbe4b685b 100644 --- a/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_version_update_pull_request_spec.rb @@ -143,14 +143,13 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) + allow(Dependabot::UpdateCheckers).to receive(:for_package_manager).and_return(stub_update_checker_class) allow(Dependabot::DependencyChangeBuilder) .to receive(:create_from) .and_return(stub_dependency_change) allow(dependency_snapshot).to receive(:package_manager).and_return(package_manager) - allow(Dependabot::Experiments).to receive(:enabled?).with( - :add_deprecation_warn_to_pr_message - ).and_return(true) end after do @@ -187,11 +186,6 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ expect(mock_error_handler).not_to receive(:handle_dependency_error) perform end - - it "adds a deprecation notice" do - expect(Dependabot::Notice).to receive(:generate_pm_deprecation_notice).with(package_manager).and_call_original - perform - end end end diff --git a/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb b/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb index e50fb27907..e98ff47238 100644 --- a/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb +++ b/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb @@ -14,6 +14,7 @@ require "dependabot/environment" require "dependabot/package_manager" require "dependabot/notices" +require "dependabot/notices_helpers" require "dependabot/bundler" @@ -152,18 +153,15 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:add_deprecation_warn_to_pr_message).and_return(true) + allow(Dependabot::UpdateCheckers).to receive( :for_package_manager ).and_return(stub_update_checker_class) allow(Dependabot::DependencyChangeBuilder).to receive( :create_from ).and_return(stub_dependency_change) - allow(dependency_snapshot).to receive( - :package_manager - ).and_return(package_manager) - allow(Dependabot::Experiments).to receive(:enabled?).with( - :add_deprecation_warn_to_pr_message - ).and_return(true) + allow(dependency_snapshot).to receive_messages(package_manager: package_manager, notices: []) end after do @@ -210,7 +208,7 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ end end - context "when package manager version is 1" do + context "when package manager version is deprecated" do let(:package_manager_version) { "1" } it "creates a pull request" do @@ -218,14 +216,9 @@ def initialize(name:, version:, deprecated_versions: [], unsupported_versions: [ expect(update_all_versions).to receive(:create_pull_request).with(stub_dependency_change) perform end - - it "adds a deprecation notice" do - expect(Dependabot::Notice).to receive(:generate_pm_deprecation_notice).with(package_manager).and_call_original - perform - end end - context "when package manager version is 2" do + context "when package manager version is not deprecated" do let(:package_manager_version) { "2" } it "creates a pull request" do