diff --git a/Gemfile.lock b/Gemfile.lock index 3f18028..d2ac4fa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - use_packwerk (0.65.0) + use_packwerk (0.66.0) code_ownership colorize packwerk (>= 2.2.1) diff --git a/lib/use_packwerk.rb b/lib/use_packwerk.rb index 0c3d084..b2d0e58 100644 --- a/lib/use_packwerk.rb +++ b/lib/use_packwerk.rb @@ -245,4 +245,40 @@ def self.get_offenses_for_files_by_package(files) Private::PackwerkWrapper.packwerk_cli_execute_safely(argv, formatter) formatter.aggregated_offenses.compact end + + sig { void } + def self.lint_deprecated_references! + contents_before = Private.get_deprecated_references_contents + UsePackwerk.execute(['update-deprecations']) + contents_after = Private.get_deprecated_references_contents + diff = Private.diff_deprecated_references_yml(contents_before, contents_after) + + if diff == '' + # No diff generated by `update-deprecations` + exit 0 + else + output = <<~OUTPUT + All `deprecated_references.yml` files must be up-to-date and that no diff is generated when running `bin/packwerk update-deprecations`. + This helps ensure a high quality signal in other engineers' PRs when inspecting new violations by ensuring there are no unrelated changes. + + There are three main reasons there may be a diff: + 1) Most likely, you may have stale violations, meaning there are old violations that no longer apply. + 2) You may have some sort of auto-formatter set up somewhere (e.g. something that reformats YML files) that is, for example, changing double quotes to single quotes. Ensure this is turned off for these auto-generated files. + 3) You may have edited these files manually. It's recommended to use the `bin/packwerk update-deprecations` command to make changes to `deprecated_references.yml` files. + + In all cases, you can run `bin/packwerk update-deprecations` to update these files. + + Here is the diff generated after running `update-deprecations`: + ``` + #{diff} + ``` + + OUTPUT + + puts output + UsePackwerk.config.on_deprecated_references_lint_failure.call(output) + + exit 1 + end + end end diff --git a/lib/use_packwerk/cli.rb b/lib/use_packwerk/cli.rb index 9c5d980..3f28f6a 100644 --- a/lib/use_packwerk/cli.rb +++ b/lib/use_packwerk/cli.rb @@ -77,5 +77,11 @@ def move_to_parent(parent_name, pack_name) per_file_processors: [UsePackwerk::RubocopPostProcessor.new, UsePackwerk::CodeOwnershipPostProcessor.new] ) end + + desc 'lint_deprecated_references', 'Ensures `deprecated_references.yml` files are up to date' + sig { void } + def lint_deprecated_references + UsePackwerk.lint_deprecated_references! + end end end diff --git a/lib/use_packwerk/configuration.rb b/lib/use_packwerk/configuration.rb index 0586397..d930a0f 100644 --- a/lib/use_packwerk/configuration.rb +++ b/lib/use_packwerk/configuration.rb @@ -13,10 +13,18 @@ class Configuration sig { returns(UserEventLogger) } attr_accessor :user_event_logger + OnDeprecatedReferencesLintFailure = T.type_alias do + T.proc.params(output: String).void + end + + sig { returns(OnDeprecatedReferencesLintFailure) } + attr_accessor :on_deprecated_references_lint_failure + sig { void } def initialize @enforce_dependencies = T.let(default_enforce_dependencies, T::Boolean) @user_event_logger = T.let(DefaultUserEventLogger.new, UserEventLogger) + @on_deprecated_references_lint_failure = T.let(->(output) {}, OnDeprecatedReferencesLintFailure) end sig { returns(T::Boolean) } diff --git a/lib/use_packwerk/private.rb b/lib/use_packwerk/private.rb index 1c43af4..0c6eb0e 100644 --- a/lib/use_packwerk/private.rb +++ b/lib/use_packwerk/private.rb @@ -389,6 +389,52 @@ def self.bust_cache! # otherwise `PackageProtections.config` will attempt to reload the client configuratoin. @loaded_client_configuration = false end + + sig { returns(T::Hash[String, String]) } + def self.get_deprecated_references_contents + deprecated_references = {} + ParsePackwerk.all.each do |package| + deprecated_references_yml = ParsePackwerk::DeprecatedReferences.for(package).pathname + if deprecated_references_yml.exist? + deprecated_references[deprecated_references_yml.to_s] = deprecated_references_yml.read + end + end + + deprecated_references + end + + DeprecatedReferencesFiles = T.type_alias do + T::Hash[String, T.nilable(String)] + end + + sig { params(before: DeprecatedReferencesFiles, after: DeprecatedReferencesFiles).returns(String) } + def self.diff_deprecated_references_yml(before, after) + dir_containing_contents_before = Dir.mktmpdir + dir_containing_contents_after = Dir.mktmpdir + begin + write_deprecated_references_to_tmp_folder(before, dir_containing_contents_before) + write_deprecated_references_to_tmp_folder(after, dir_containing_contents_after) + + diff = `diff -r #{dir_containing_contents_before}/ #{dir_containing_contents_after}/` + # For ease of reading, sub out the tmp directory from the diff + diff.gsub(dir_containing_contents_before, '').gsub(dir_containing_contents_after, '') + ensure + FileUtils.remove_entry dir_containing_contents_before + FileUtils.remove_entry dir_containing_contents_after + end + end + + sig { params(deprecated_references_files: DeprecatedReferencesFiles, tmp_folder: String).void } + def self.write_deprecated_references_to_tmp_folder(deprecated_references_files, tmp_folder) + deprecated_references_files.each do |filename, contents| + next if contents.nil? + + tmp_folder_pathname = Pathname.new(tmp_folder) + temp_deprecated_references_yml = tmp_folder_pathname.join(filename) + FileUtils.mkdir_p(temp_deprecated_references_yml.dirname) + temp_deprecated_references_yml.write(contents) + end + end end private_constant :Private diff --git a/spec/use_packwerk_spec.rb b/spec/use_packwerk_spec.rb index 8d579fc..204e434 100644 --- a/spec/use_packwerk_spec.rb +++ b/spec/use_packwerk_spec.rb @@ -1292,6 +1292,139 @@ def expect_files_to_not_exist(files) end end + describe 'lint_deprecated_references' do + let(:args) { ['lint_deprecated_references'] } + + context 'no diff after running update-deprecations' do + it 'exits successfully' do + expect_any_instance_of(Packwerk::Cli).to receive(:execute_command).with(['update-deprecations']) + expect(UsePackwerk).to receive(:exit).with(0) + expect(UsePackwerk).to_not receive(:puts) + UsePackwerk.lint_deprecated_references! + end + end + + context 'some stale violations removed after running update-deprecations' do + it 'exits in a failure' do + write_file('packs/my_pack/package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependnecy: true + CONTENTS + + write_file('packs/my_pack/deprecated_references.yml', <<~CONTENTS) + --- + packs/my_other_pack: + "::SomeConstant": + violations: + - privacy + files: + - packs/my_pack/app/services/my_pack.rb + - packs/my_pack/app/services/my_pack_2.rb + CONTENTS + + expect_any_instance_of(Packwerk::Cli).to receive(:execute_command).with(['update-deprecations']) do + write_file('packs/my_pack/deprecated_references.yml', <<~CONTENTS) + --- + packs/my_other_pack: + "::SomeConstant": + violations: + - privacy + files: + - packs/my_pack/app/services/my_pack.rb + CONTENTS + end + + expect(UsePackwerk).to receive(:puts).with(<<~EXPECTED) + All `deprecated_references.yml` files must be up-to-date and that no diff is generated when running `bin/packwerk update-deprecations`. + This helps ensure a high quality signal in other engineers' PRs when inspecting new violations by ensuring there are no unrelated changes. + + There are three main reasons there may be a diff: + 1) Most likely, you may have stale violations, meaning there are old violations that no longer apply. + 2) You may have some sort of auto-formatter set up somewhere (e.g. something that reformats YML files) that is, for example, changing double quotes to single quotes. Ensure this is turned off for these auto-generated files. + 3) You may have edited these files manually. It's recommended to use the `bin/packwerk update-deprecations` command to make changes to `deprecated_references.yml` files. + + In all cases, you can run `bin/packwerk update-deprecations` to update these files. + + Here is the diff generated after running `update-deprecations`: + ``` + diff -r /packs/my_pack/deprecated_references.yml /packs/my_pack/deprecated_references.yml + 8d7 + < - packs/my_pack/app/services/my_pack_2.rb + + ``` + + EXPECTED + + expect(UsePackwerk).to receive(:exit).with(1) + UsePackwerk.lint_deprecated_references! + end + end + + context 'some formatting changes after running update-deprecations' do + it 'exits in a failure' do + callback_invocation = false + UsePackwerk.configure do |config| + config.on_deprecated_references_lint_failure = ->(output) { callback_invocation = output } + end + write_file('packs/my_pack/package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependnecy: true + CONTENTS + + write_file('packs/my_pack/deprecated_references.yml', <<~CONTENTS) + --- + packs/my_other_pack: + '::SomeConstant': + violations: + - privacy + files: + - packs/my_pack/app/services/my_pack.rb + - packs/my_pack/app/services/my_pack_2.rb + CONTENTS + + expect_any_instance_of(Packwerk::Cli).to receive(:execute_command).with(['update-deprecations']) do + write_file('packs/my_pack/deprecated_references.yml', <<~CONTENTS) + --- + packs/my_other_pack: + "::SomeConstant": + violations: + - privacy + files: + - packs/my_pack/app/services/my_pack.rb + - packs/my_pack/app/services/my_pack_2.rb + CONTENTS + end + + expect(UsePackwerk).to receive(:puts).with(<<~EXPECTED) + All `deprecated_references.yml` files must be up-to-date and that no diff is generated when running `bin/packwerk update-deprecations`. + This helps ensure a high quality signal in other engineers' PRs when inspecting new violations by ensuring there are no unrelated changes. + + There are three main reasons there may be a diff: + 1) Most likely, you may have stale violations, meaning there are old violations that no longer apply. + 2) You may have some sort of auto-formatter set up somewhere (e.g. something that reformats YML files) that is, for example, changing double quotes to single quotes. Ensure this is turned off for these auto-generated files. + 3) You may have edited these files manually. It's recommended to use the `bin/packwerk update-deprecations` command to make changes to `deprecated_references.yml` files. + + In all cases, you can run `bin/packwerk update-deprecations` to update these files. + + Here is the diff generated after running `update-deprecations`: + ``` + diff -r /packs/my_pack/deprecated_references.yml /packs/my_pack/deprecated_references.yml + 3c3 + < '::SomeConstant': + --- + > "::SomeConstant": + + ``` + + EXPECTED + + expect(UsePackwerk).to receive(:exit).with(1) + UsePackwerk.lint_deprecated_references! + expect(callback_invocation).to include('All `deprecated_references.yml` files must be up-to-date') + end + end + end + # This will soon be moved into `query_packwerk` describe 'query_packwerk' do before do diff --git a/use_packwerk.gemspec b/use_packwerk.gemspec index 6a24204..7539c21 100644 --- a/use_packwerk.gemspec +++ b/use_packwerk.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'use_packwerk' - spec.version = '0.65.0' + spec.version = '0.66.0' spec.authors = ['Gusto Engineers'] spec.email = ['dev@gusto.com']