Packages with no violations should have their files removed

lib/packwerk/offense_collection.rb

@@ -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]) }
@@ -73,6 +72,23 @@ 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(&:dump)
+    end
+
     sig { params(package: Packwerk::Package).returns(Packwerk::PackageTodo) }
     def package_todo_for(package)
       @package_todo[package] ||= Packwerk::PackageTodo.new(

lib/packwerk/package_todo.rb

@@ -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
@@ -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) }

lib/packwerk/parse_run.rb

@@ -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)
@@ -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)}
@@ -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),
@@ -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

lib/packwerk/run_context.rb

@@ -71,6 +71,7 @@ def initialize(
 
       @file_processor = T.let(nil, T.nilable(FileProcessor))
       @context_provider = T.let(nil, T.nilable(ConstantDiscovery))
+      @package_set = T.let(nil, T.nilable(PackageSet))
       # We need to initialize this before we fork the process, see https://github.com/Shopify/packwerk/issues/182
       @cache = T.let(
         Cache.new(enable_cache: @cache_enabled, cache_directory: @cache_directory, config_path: @config_path), Cache
@@ -90,6 +91,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) }
@@ -123,11 +129,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
       [

test/integration/offense_collection_test.rb

@@ -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" => {
@@ -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

test/unit/parse_run_test.rb

@@ -73,6 +73,55 @@ 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(