Generate links in formatter using head ref (#54)

* Add repo_url_builder keyword arg
* Bump minimum Ruby to 3.1
* Pass repo_url_builder to format_offenses
* Use repo_url_builder to generate the link in the default formatter
* Use head ref for url generation in offense formatter
* Bump to 0.15.0 for change to OffenseFormatter interface
  The change is backwards compatible, but sorbet forces everyone to update
  their method signature anyway.
* Test generating urls in the custom formatter to ensure it works

---------

Co-authored-by: Ivy Evans <[email protected]>

Author: martinemde

.rubocop.yml

@@ -3,7 +3,7 @@ plugins:
   - rubocop-performance
 
 AllCops:
-  TargetRubyVersion: 2.6
+  TargetRubyVersion: 3.1
   NewCops: enable
   Exclude:
     - bin/**/*

Gemfile.lock

@@ -18,7 +18,7 @@ GIT
 PATH
   remote: .
   specs:
-    danger-packwerk (0.14.4)
+    danger-packwerk (0.15.0)
       code_ownership
       danger-plugin-api (~> 1.0)
       packs

README.md

@@ -43,8 +43,15 @@ class MyFormatter
   extend T::Sig
   include DangerPackwerk::Check::OffensesFormatter
   # Packwerk::ReferenceOffense: https://github.com/Shopify/packwerk/blob/main/lib/packwerk/reference_offense.rb
-  sig { override.params(offenses: T::Array[Packwerk::ReferenceOffense], repo_link: String, org_name: String).returns(String) }
-  def format_offenses(offenses, repo_link, org_name)
+  sig do
+    override.params(
+      offenses: T::Array[Packwerk::ReferenceOffense],
+      repo_link: String,
+      org_name: String
+      repo_url_builder: T.nilable(T.proc.params(constant_path: String).returns(String))
+    ).returns(String)
+  end
+  def format_offenses(offenses, repo_link, org_name, repo_builder_url: nil)
     # your logic here
   end
 end
@@ -98,8 +105,15 @@ class MyFormatter
   extend T::Sig
   include DangerPackwerk::Update::OffensesFormatter
   # DangerPackwerk::BasicReferenceOffense
-  sig { override.params(offenses: T::Array[DangerPackwerk::BasicReferenceOffense], repo_link: String, org_name: String).returns(String) }
-  def format_offenses(offenses, repo_link, org_name)
+  sig do
+    override.params(
+      offenses: T::Array[Packwerk::ReferenceOffense],
+      repo_link: String,
+      org_name: String
+      repo_url_builder: T.nilable(T.proc.params(constant_path: String).returns(String))
+    ).returns(String)
+  end
+  def format_offenses(offenses, repo_link, org_name, repo_builder_url: nil)
     # your logic here
   end
 end

danger-packwerk.gemspec

@@ -27,7 +27,7 @@ Gem::Specification.new do |spec|
 
   spec.files = Dir['README.md', 'lib/**/*']
   spec.require_paths = ['lib']
-  spec.required_ruby_version = '>= 2.6'
+  spec.required_ruby_version = '>= 3.1'
 
   spec.add_dependency 'code_ownership'
   spec.add_dependency 'danger-plugin-api', '~> 1.0'

lib/danger-packwerk.rb

@@ -5,7 +5,7 @@
 require 'sorbet-runtime'
 
 module DangerPackwerk
-  PACKAGE_TODO_PATTERN = T.let(/.*?package_todo\.yml\z/.freeze, Regexp)
+  PACKAGE_TODO_PATTERN = T.let(/.*?package_todo\.yml\z/, Regexp)
   DEPENDENCY_VIOLATION_TYPE = 'dependency'
   PRIVACY_VIOLATION_TYPE = 'privacy'
 

lib/danger-packwerk/check/default_formatter.rb

@@ -21,10 +21,11 @@ def initialize(custom_help_message: nil)
         override.params(
           offenses: T::Array[Packwerk::ReferenceOffense],
           repo_link: String,
-          org_name: String
+          org_name: String,
+          repo_url_builder: T.nilable(T.proc.params(constant_path: String).returns(String))
         ).returns(String)
       end
-      def format_offenses(offenses, repo_link, org_name)
+      def format_offenses(offenses, repo_link, org_name, repo_url_builder: nil)
         reference_offense = T.must(offenses.first)
         violation_types = offenses.map(&:violation_type)
         referencing_file = reference_offense.reference.relative_path
@@ -45,11 +46,18 @@ def format_offenses(offenses, repo_link, org_name)
         team_to_work_with = constant_source_package_ownership_info.owning_team ? constant_source_package_ownership_info.markdown_link_to_github_members_no_tag : 'the pack owner'
         privacy_violation_message = "- Does API in #{constant_source_package.name}/public support this use case?\n  - If not, can we work with #{team_to_work_with} to create and use a public API?\n  - If `#{constant_name}` should already be public, try `bin/packs make_public #{constant_location}`."
 
+        constant_location_url =
+          if repo_url_builder
+            repo_url_builder.call(constant_location)
+          else
+            "#{repo_link}/blob/main/#{constant_location}"
+          end
+
         if violation_types.include?(::DangerPackwerk::DEPENDENCY_VIOLATION_TYPE) && violation_types.include?(::DangerPackwerk::PRIVACY_VIOLATION_TYPE)
           <<~MESSAGE
             **Packwerk Violation**
             - Type: Privacy :lock: + Dependency :knot:
-            - Constant: [<ins>`#{constant_name}`</ins>](#{repo_link}/blob/main/#{constant_location})
+            - Constant: [<ins>`#{constant_name}`</ins>](#{constant_location_url})
             - Owning pack: #{constant_source_package_name}
               #{constant_source_package_ownership_info.ownership_copy}
 
@@ -69,7 +77,7 @@ def format_offenses(offenses, repo_link, org_name)
           <<~MESSAGE
             **Packwerk Violation**
             - Type: Dependency :knot:
-            - Constant: [<ins>`#{constant_name}`</ins>](#{repo_link}/blob/main/#{constant_location})
+            - Constant: [<ins>`#{constant_name}`</ins>](#{constant_location_url})
             - Owning pack: #{constant_source_package_name}
               #{constant_source_package_ownership_info.ownership_copy}
 
@@ -88,7 +96,7 @@ def format_offenses(offenses, repo_link, org_name)
           <<~MESSAGE
             **Packwerk Violation**
             - Type: Privacy :lock:
-            - Constant: [<ins>`#{constant_name}`</ins>](#{repo_link}/blob/main/#{constant_location})
+            - Constant: [<ins>`#{constant_name}`</ins>](#{constant_location_url})
             - Owning pack: #{constant_source_package_name}
               #{constant_source_package_ownership_info.ownership_copy}
 

lib/danger-packwerk/check/offenses_formatter.rb

@@ -14,10 +14,11 @@ module OffensesFormatter
         abstract.params(
           offenses: T::Array[Packwerk::ReferenceOffense],
           repo_link: String,
-          org_name: String
+          org_name: String,
+          repo_url_builder: T.nilable(T.proc.params(constant_path: String).returns(String))
         ).returns(String)
       end
-      def format_offenses(offenses, repo_link, org_name); end
+      def format_offenses(offenses, repo_link, org_name, repo_url_builder: nil); end
     end
   end
 end

lib/danger-packwerk/danger_package_todo_yml_changes.rb

@@ -42,6 +42,7 @@ def check(
     )
       offenses_formatter ||= Update::DefaultFormatter.new
       repo_link = github.pr_json[:base][:repo][:html_url]
+      repo_url_builder = ->(constant_path) { "#{repo_link}/blob/#{github.pr_json[:head][:ref]}/#{constant_path}" }
       org_name = github.pr_json[:base][:repo][:owner][:login]
 
       git_filesystem = Private::GitFilesystem.new(git: git, root: root_path || '')
@@ -62,7 +63,7 @@ def check(
         location = T.must(violations.first).file_location
 
         markdown(
-          offenses_formatter.format_offenses(violations, repo_link, org_name),
+          offenses_formatter.format_offenses(violations, repo_link, org_name, repo_url_builder: repo_url_builder),
           line: location.line_number,
           file: git_filesystem.convert_to_filesystem(location.file)
         )

lib/danger-packwerk/danger_packwerk.rb

@@ -64,6 +64,7 @@ def check(
     )
       offenses_formatter ||= Check::DefaultFormatter.new
       repo_link = github.pr_json[:base][:repo][:html_url]
+      repo_url_builder = ->(constant_path) { "#{repo_link}/blob/#{github.pr_json[:head][:ref]}/#{constant_path}" }
       org_name = github.pr_json[:base][:repo][:owner][:login]
 
       # This is important because by default, Danger will leave a concantenated list of all its messages if it can't find a commentable place in the
@@ -140,7 +141,7 @@ def check(
         line_number = reference_offense.location&.line
         referencing_file = reference_offense.reference.relative_path
 
-        message = offenses_formatter.format_offenses(unique_packwerk_reference_offenses, repo_link, org_name)
+        message = offenses_formatter.format_offenses(unique_packwerk_reference_offenses, repo_link, org_name, repo_url_builder: repo_url_builder)
         markdown(message, file: git_filesystem.convert_to_filesystem(referencing_file), line: line_number)
       end
 

lib/danger-packwerk/update/default_formatter.rb

@@ -15,8 +15,15 @@ def initialize(custom_help_message: nil)
         @custom_help_message = custom_help_message
       end
 
-      sig { override.params(offenses: T::Array[BasicReferenceOffense], repo_link: String, org_name: String).returns(String) }
-      def format_offenses(offenses, repo_link, org_name)
+      sig do
+        override.params(
+          offenses: T::Array[BasicReferenceOffense],
+          repo_link: String,
+          org_name: String,
+          repo_url_builder: T.nilable(T.proc.params(constant_path: String).returns(String))
+        ).returns(String)
+      end
+      def format_offenses(offenses, repo_link, org_name, repo_url_builder: nil)
         violation = T.must(offenses.first)
         referencing_file_pack = ParsePackwerk.package_from_path(violation.file)
         # We remove leading double colons as they feel like an implementation detail of packwerk.

lib/danger-packwerk/update/offenses_formatter.rb

@@ -14,10 +14,11 @@ module OffensesFormatter
         abstract.params(
           offenses: T::Array[BasicReferenceOffense],
           repo_link: String,
-          org_name: String
+          org_name: String,
+          repo_url_builder: T.nilable(T.proc.params(constant_path: String).returns(String))
         ).returns(String)
       end
-      def format_offenses(offenses, repo_link, org_name); end
+      def format_offenses(offenses, repo_link, org_name, repo_url_builder: nil); end
     end
   end
 end

lib/danger-packwerk/version.rb

@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module DangerPackwerk
-  VERSION = '0.14.4'
+  VERSION = '0.15.0'
 end

spec/danger_packwerk/danger_package_todo_yml_changes_spec.rb

@@ -145,7 +145,7 @@ module DangerPackwerk
             Class.new do
               include Update::OffensesFormatter
 
-              def format_offenses(added_violations, repo_link, org_name)
+              def format_offenses(added_violations, repo_link, org_name, repo_url_builder: nil)
                 <<~MESSAGE
                   There are #{added_violations.count} new violations,
                   with class_names #{added_violations.map(&:class_name).uniq.sort},
@@ -1159,7 +1159,7 @@ def format_offenses(added_violations, repo_link, org_name)
             Class.new do
               include Update::OffensesFormatter
 
-              def format_offenses(added_violations, repo_link, org_name)
+              def format_offenses(added_violations, repo_link, org_name, repo_url_builder: nil)
                 <<~MESSAGE
                   There are #{added_violations.count} new violations,
                   with class_names #{added_violations.map(&:class_name).uniq.sort},