Skip to content

Commit

Permalink
Packages with no violations should have their files removed
Browse files Browse the repository at this point in the history
  • Loading branch information
rafaelfranca authored and gmcgibbon committed Nov 2, 2022
1 parent be05273 commit 64717de
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 19 deletions.
28 changes: 23 additions & 5 deletions lib/packwerk/offense_collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,10 @@ def stale_violations?(for_files)
end
end

sig { void }
def dump_package_todo_files
@package_todo.each do |_, package_todo_file|
package_todo_file.dump
end
sig { params(package_set: Packwerk::PackageSet).void }
def persist_package_todo_files(package_set)
dump_package_todo_files
cleanup_extra_package_todo_files(package_set)
end

sig { returns(T::Array[Packwerk::Offense]) }
Expand All @@ -73,6 +72,25 @@ def outstanding_offenses

private

sig { params(package_set: Packwerk::PackageSet).void }
def cleanup_extra_package_todo_files(package_set)
packages_without_todos = (package_set.packages.values - @package_todo.keys)

packages_without_todos.each do |package|
Packwerk::PackageTodo.new(
package,
package_todo_file_for(package),
).delete_if_exists
end
end

sig { void }
def dump_package_todo_files
@package_todo.each_value do |package_todo_file|
package_todo_file.dump
end
end

sig { params(package: Packwerk::Package).returns(Packwerk::PackageTodo) }
def package_todo_for(package)
@package_todo[package] ||= Packwerk::PackageTodo.new(
Expand Down
7 changes: 6 additions & 1 deletion lib/packwerk/package_todo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def stale_violations?(for_files)
sig { void }
def dump
if @new_entries.empty?
File.delete(@filepath) if File.exist?(@filepath)
delete_if_exists
else
prepare_entries_for_dump
message = <<~MESSAGE
Expand All @@ -110,6 +110,11 @@ def dump
end
end

sig { void }
def delete_if_exists
File.delete(@filepath) if File.exist?(@filepath)
end

private

sig { returns(EntriesType) }
Expand Down
16 changes: 9 additions & 7 deletions lib/packwerk/parse_run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ def initialize(

sig { returns(Result) }
def detect_stale_violations
offense_collection = find_offenses
run_context = Packwerk::RunContext.from_configuration(@configuration)
offense_collection = find_offenses(run_context)

result_status = !offense_collection.stale_violations?(@relative_file_set)
message = @offenses_formatter.show_stale_violations(offense_collection, @relative_file_set)
Expand All @@ -44,8 +45,9 @@ def detect_stale_violations

sig { returns(Result) }
def update_todo
offense_collection = find_offenses
offense_collection.dump_package_todo_files
run_context = Packwerk::RunContext.from_configuration(@configuration)
offense_collection = find_offenses(run_context)
offense_collection.persist_package_todo_files(run_context.package_set)

message = <<~EOS
#{@offenses_formatter.show_offenses(offense_collection.errors)}
Expand All @@ -57,7 +59,8 @@ def update_todo

sig { returns(Result) }
def check
offense_collection = find_offenses(show_errors: true)
run_context = Packwerk::RunContext.from_configuration(@configuration)
offense_collection = find_offenses(run_context, show_errors: true)

messages = [
@offenses_formatter.show_offenses(offense_collection.outstanding_offenses),
Expand All @@ -72,12 +75,11 @@ def check

private

sig { params(show_errors: T::Boolean).returns(OffenseCollection) }
def find_offenses(show_errors: false)
sig { params(run_context: Packwerk::RunContext, show_errors: T::Boolean).returns(OffenseCollection) }
def find_offenses(run_context, show_errors: false)
offense_collection = OffenseCollection.new(@configuration.root_path)
@progress_formatter.started(@relative_file_set)

run_context = Packwerk::RunContext.from_configuration(@configuration)
all_offenses = T.let([], T::Array[Offense])

process_file = T.let(->(relative_file) do
Expand Down
10 changes: 5 additions & 5 deletions lib/packwerk/run_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ def process_file(relative_file:)
processed_file.offenses + references.flat_map { |reference| reference_checker.call(reference) }
end

sig { returns(PackageSet) }
def package_set
@package_set ||= ::Packwerk::PackageSet.load_all_from(@root_path, package_pathspec: @package_paths)
end

private

sig { returns(FileProcessor) }
Expand Down Expand Up @@ -123,11 +128,6 @@ def resolver
)
end

sig { returns(PackageSet) }
def package_set
::Packwerk::PackageSet.load_all_from(@root_path, package_pathspec: @package_paths)
end

sig { returns(T::Array[ConstantNameInspector]) }
def constant_name_inspectors
[
Expand Down
9 changes: 8 additions & 1 deletion test/integration/offense_collection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ class OffenseCollectionTest < Minitest::Test
message: "some message",
violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE
)

@offense_collection.add_offense(offense1)
@offense_collection.add_offense(offense2)
@offense_collection.dump_package_todo_files
dump_package_todo_files

expected = {
"components/destination" => {
Expand All @@ -49,6 +50,12 @@ class OffenseCollectionTest < Minitest::Test
}
assert_equal expected, YAML.load_file("package_todo.yml")
end

private

def dump_package_todo_files
@offense_collection.persist_package_todo_files(PackageSet.new([]))
end
end
end
end
50 changes: 50 additions & 0 deletions test/unit/parse_run_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,56 @@ class ParseRunTest < Minitest::Test
refute result.status
end

test "#update_todo cleans up old package_todo files" do
use_template(:minimal)

FileUtils.mkdir_p("app/models")
File.write("app/models/my_model.rb", <<~YML.strip)
class MyModel
def something
Order
end
end
YML

File.write("package_todo.yml", <<~YML.strip)
---
"components/sales":
"::Order":
violations:
- privacy
files:
- app/models/my_model.rb
YML

File.write("components/sales/package_todo.yml", <<~YML.strip)
---
"components/destination":
"::SomeName":
violations:
- privacy
files:
- a/b/c.rb
YML

parse_run = Packwerk::ParseRun.new(
relative_file_set: Set.new(["app/models/my_model.rb", "components/sales/app/models/order.rb"]),
configuration: Configuration.from_path
)
result = parse_run.update_todo


expected = <<~EOS
No offenses detected
✅ `package_todo.yml` has been updated.
EOS
assert_equal expected, result.message
assert result.status

assert File.exist?("package_todo.yml")
refute File.exist?("components/sales/package_todo.yml")
end

test "#check only reports error progress for unlisted violations" do
use_template(:minimal)
offense = ReferenceOffense.new(
Expand Down

0 comments on commit 64717de

Please sign in to comment.