Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bin/packs lint_deprecated_references command #55

Merged
merged 5 commits into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add ability to lint deprecated_references.yml files
  • Loading branch information
Alex Evanczuk committed Nov 17, 2022
commit 537204adf34dc9e98cbaa70ab7a8396dfdc17171
35 changes: 35 additions & 0 deletions lib/use_packwerk.rb
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,39 @@ 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

exit 1
end
end
end
6 changes: 6 additions & 0 deletions lib/use_packwerk/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
46 changes: 46 additions & 0 deletions lib/use_packwerk/private.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
132 changes: 132 additions & 0 deletions spec/use_packwerk_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1292,6 +1292,138 @@ 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)
We require that all `deprecated_references.yml` files 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.

Please come to #ruby-modularity with questions or feedback!

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
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)
We require that all `deprecated_references.yml` files 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.

Please come to #ruby-modularity with questions or feedback!

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!
end
end
end

# This will soon be moved into `query_packwerk`
describe 'query_packwerk' do
before do
Expand Down