diff --git a/lib/danger-packwerk/check/default_formatter.rb b/lib/danger-packwerk/check/default_formatter.rb index da8542e..484d393 100644 --- a/lib/danger-packwerk/check/default_formatter.rb +++ b/lib/danger-packwerk/check/default_formatter.rb @@ -19,23 +19,23 @@ def initialize(custom_help_message: nil) sig do override.params( - offenses: T::Array[Packwerk::ReferenceOffense], + offenses: T::Array[PksOffense], 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) - reference_offense = T.must(offenses.first) + offense = T.must(offenses.first) violation_types = offenses.map(&:violation_type) - referencing_file = reference_offense.reference.relative_path + referencing_file = offense.file referencing_file_pack = ParsePackwerk.package_from_path(referencing_file).name # We remove leading double colons as they feel like an implementation detail of packwerk. - constant_name = reference_offense.reference.constant.name.delete_prefix('::') + constant_name = offense.constant_name.delete_prefix('::') - constant_source_package_name = reference_offense.reference.constant.package.name + constant_source_package_name = offense.defining_pack_name - constant_location = reference_offense.reference.constant.location + constant_location = Private.constant_resolver.resolve(offense.constant_name)&.location || offense.file constant_source_package = T.must(ParsePackwerk.all.find { |p| p.name == constant_source_package_name }) constant_source_package_ownership_info = Private::OwnershipInformation.for_package(constant_source_package, org_name) diff --git a/lib/danger-packwerk/check/offenses_formatter.rb b/lib/danger-packwerk/check/offenses_formatter.rb index 637f443..6da3f4b 100644 --- a/lib/danger-packwerk/check/offenses_formatter.rb +++ b/lib/danger-packwerk/check/offenses_formatter.rb @@ -12,7 +12,7 @@ module OffensesFormatter sig do abstract.params( - offenses: T::Array[Packwerk::ReferenceOffense], + offenses: T::Array[PksOffense], repo_link: String, org_name: String, repo_url_builder: T.nilable(T.proc.params(constant_path: String).returns(String)) diff --git a/lib/danger-packwerk/danger_packwerk.rb b/lib/danger-packwerk/danger_packwerk.rb index 398e548..a222fd2 100644 --- a/lib/danger-packwerk/danger_packwerk.rb +++ b/lib/danger-packwerk/danger_packwerk.rb @@ -20,8 +20,7 @@ class DangerPackwerk < Danger::Plugin # especially given all violations should fail the build anyways. # We set a max (rather than unlimited) to avoid GitHub rate limiting and general spam if a PR does some sort of mass rename. DEFAULT_MAX_COMMENTS = 15 - # Support both legacy Packwerk::ReferenceOffense and new PksOffense types - OnFailure = T.type_alias { T.proc.params(offenses: T::Array[T.any(Packwerk::ReferenceOffense, PksOffense)]).void } + OnFailure = T.type_alias { T.proc.params(offenses: T::Array[PksOffense]).void } DEFAULT_ON_FAILURE = T.let(->(offenses) {}, OnFailure) DEFAULT_FAIL = false DEFAULT_FAILURE_MESSAGE = 'Packwerk violations were detected! Please resolve them to unblock the build.' @@ -109,8 +108,7 @@ def check( renamed_files = git_filesystem.renamed_files.map { |before_after_file| before_after_file[:after] } offenses_to_care_about = pks_offenses.reject do |offense| - constant_name = offense.reference.constant.name - filepath_that_defines_this_constant = Private.constant_resolver.resolve(constant_name)&.location + filepath_that_defines_this_constant = Private.constant_resolver.resolve(offense.constant_name)&.location # Ignore references that have been renamed renamed_files.include?(filepath_that_defines_this_constant) || # Ignore violations that are not in the allow-list of violation types to leave comments for @@ -123,14 +121,14 @@ def check( case grouping_strategy when CommentGroupingStrategy::PerConstantPerLocation [ - offense.reference.constant.name, - offense.location.line, - offense.reference.relative_path + offense.constant_name, + offense.line, + offense.file ] when CommentGroupingStrategy::PerConstantPerPack [ - offense.reference.constant.name, - ParsePackwerk.package_from_path(offense.reference.relative_path) + offense.constant_name, + ParsePackwerk.package_from_path(offense.file) ] else T.absurd(grouping_strategy) @@ -141,11 +139,10 @@ def check( current_comment_count += 1 offense = T.must(unique_offenses.first) - line_number = offense.location.line - referencing_file = offense.reference.relative_path + line_number = offense.line + referencing_file = offense.file - # PksOffense adapters provide Packwerk::ReferenceOffense-compatible interface - message = offenses_formatter.format_offenses(T.unsafe(unique_offenses), repo_link, org_name, repo_url_builder: repo_url_builder) + message = offenses_formatter.format_offenses(unique_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 diff --git a/lib/danger-packwerk/pks_offense.rb b/lib/danger-packwerk/pks_offense.rb index 47125af..9f9ba79 100644 --- a/lib/danger-packwerk/pks_offense.rb +++ b/lib/danger-packwerk/pks_offense.rb @@ -5,11 +5,7 @@ module DangerPackwerk # # PksOffense represents a violation from pks JSON output. - # It is designed to have a compatible interface with BasicReferenceOffense - # so it can be used with the Update::OffensesFormatter. - # - # It also provides adapter objects (reference, location) for compatibility - # with Packwerk::ReferenceOffense interface used by Check::OffensesFormatter. + # It has a compatible interface with BasicReferenceOffense for use with formatters. # class PksOffense < T::Struct extend T::Sig @@ -24,50 +20,6 @@ class PksOffense < T::Struct const :strict, T::Boolean const :message, String - # - # Adapter classes for Packwerk::ReferenceOffense compatibility - # These allow PksOffense to be used with Check::OffensesFormatter - # - class PackageAdapter < T::Struct - const :name, String - end - - class ConstantAdapter < T::Struct - const :name, String - const :location, String - const :package, PackageAdapter - end - - class ReferenceAdapter < T::Struct - const :relative_path, String - const :constant, ConstantAdapter - end - - class LocationAdapter < T::Struct - const :line, Integer - end - - # Adapter for Packwerk::ReferenceOffense.reference - sig { returns(ReferenceAdapter) } - def reference - package_adapter = PackageAdapter.new(name: defining_pack_name) - constant_adapter = ConstantAdapter.new( - name: constant_name, - location: file, # Best approximation - pks doesn't provide constant definition location - package: package_adapter - ) - ReferenceAdapter.new( - relative_path: file, - constant: constant_adapter - ) - end - - # Adapter for Packwerk::ReferenceOffense.location - sig { returns(LocationAdapter) } - def location - LocationAdapter.new(line: line) - end - # Alias methods for compatibility with BasicReferenceOffense interface sig { returns(String) } def class_name diff --git a/spec/danger_packwerk/danger_packwerk_spec.rb b/spec/danger_packwerk/danger_packwerk_spec.rb index 235bba0..a0d2a00 100644 --- a/spec/danger_packwerk/danger_packwerk_spec.rb +++ b/spec/danger_packwerk/danger_packwerk_spec.rb @@ -57,10 +57,11 @@ module DangerPackwerk include Check::OffensesFormatter def format_offenses(offenses, repo_link, org_name, repo_url_builder:) - constant_location = offenses.first.reference.constant.location - constant_name = offenses.first.reference.constant.name + offense = offenses.first + constant_location = offense.file + constant_name = offense.constant_name url = repo_url_builder&.call(constant_location.to_s) - offenses.map { |offense| "#{offense.message} [#{constant_name}](#{url})" }.join("\n\n") + offenses.map { |o| "#{o.message} [#{constant_name}](#{url})" }.join("\n\n") end end end @@ -601,7 +602,7 @@ def format_offenses(offenses, repo_link, org_name, repo_url_builder:) actual_markdowns = dangerfile.status_report[:markdowns] expect(actual_markdowns.count).to eq 1 actual_markdown = actual_markdowns.first - # The formatter uses offenses.first.reference.constant.location for all URLs + # The formatter uses offense.file for all URLs expect(actual_markdown.message).to eq(<<~MSG.chomp) Vanilla message about privacy violations [::PrivateConstant](https://github.com/MyOrg/my_repo/blob/my_branch/packs/referencing_pack/some_file.rb)