Make sure all thor CLI commands have the right exit code (#98)

* bundle update

* Make sure all commands exit with proper error codes

* bump version

* rubocop

Author: Alex Evanczuk

Gemfile.lock

@@ -1,9 +1,9 @@
 GIT
   remote: https://github.com/Shopify/packwerk.git
-  revision: 95caea012661e69e9f5d268044f8277c77de2743
+  revision: e850241a4572a984bc9def8307faef7271c6890b
   branch: main
   specs:
-    packwerk (3.0.1)
+    packwerk (3.1.0)
       activesupport (>= 6.0)
       ast
       better_html
@@ -17,7 +17,7 @@ GIT
 PATH
   remote: .
   specs:
-    packs (0.0.23)
+    packs (0.0.24)
       code_ownership (>= 1.33.0)
       colorize
       packs-specification
@@ -32,19 +32,20 @@ PATH
 GEM
   remote: https://rubygems.org/
   specs:
-    actionview (7.0.4.3)
-      activesupport (= 7.0.4.3)
+    actionview (7.0.7)
+      activesupport (= 7.0.7)
       builder (~> 3.1)
       erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.1, >= 1.2.0)
-    activesupport (7.0.4.3)
+    activesupport (7.0.7)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 1.6, < 2)
       minitest (>= 5.1)
       tzinfo (~> 2.0)
     ast (2.4.2)
-    better_html (2.0.1)
+    base64 (0.1.1)
+    better_html (2.0.2)
       actionview (>= 6.0)
       activesupport (>= 6.0)
       ast (~> 2.0)
@@ -66,26 +67,28 @@ GEM
     crass (1.0.6)
     diff-lcs (1.5.0)
     erubi (1.12.0)
-    i18n (1.13.0)
+    i18n (1.14.1)
       concurrent-ruby (~> 1.0)
     json (2.6.3)
+    language_server-protocol (3.17.0.3)
     loofah (2.21.3)
       crass (~> 1.0.2)
       nokogiri (>= 1.12.0)
     method_source (1.0.0)
-    mini_portile2 (2.8.2)
-    minitest (5.18.0)
+    mini_portile2 (2.8.4)
+    minitest (5.19.0)
     netrc (0.11.0)
-    nokogiri (1.15.0)
+    nokogiri (1.15.3)
       mini_portile2 (~> 2.8.2)
       racc (~> 1.4)
     packs-specification (0.0.9)
       sorbet-runtime
     parallel (1.23.0)
     parse_packwerk (0.19.3)
       sorbet-runtime
-    parser (3.2.2.1)
+    parser (3.2.2.3)
       ast (~> 2.4.1)
+      racc
     pastel (0.8.0)
       tty-color (~> 0.5)
     pry (0.14.2)
@@ -94,21 +97,23 @@ GEM
     pry-byebug (3.10.1)
       byebug (~> 11.0)
       pry (>= 0.13, < 0.15)
-    racc (1.6.2)
-    rails-dom-testing (2.0.3)
-      activesupport (>= 4.2.0)
+    racc (1.7.1)
+    rails-dom-testing (2.2.0)
+      activesupport (>= 5.0.0)
+      minitest
       nokogiri (>= 1.6)
-    rails-html-sanitizer (1.5.0)
-      loofah (~> 2.19, >= 2.19.1)
+    rails-html-sanitizer (1.6.0)
+      loofah (~> 2.21)
+      nokogiri (~> 1.14)
     rainbow (3.1.1)
     rake (13.0.6)
-    rbi (0.0.16)
+    rbi (0.0.17)
       ast
-      parser (>= 2.6.4.0)
+      parser (>= 3.0.0)
       sorbet-runtime (>= 0.5.9204)
-      unparser
-    regexp_parser (2.8.0)
-    rexml (3.2.5)
+      unparser (>= 0.5.6)
+    regexp_parser (2.8.1)
+    rexml (3.2.6)
     rspec (3.12.0)
       rspec-core (~> 3.12.0)
       rspec-expectations (~> 3.12.0)
@@ -118,25 +123,27 @@ GEM
     rspec-expectations (3.12.3)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.12.0)
-    rspec-mocks (3.12.5)
+    rspec-mocks (3.12.6)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.12.0)
-    rspec-support (3.12.0)
-    rubocop (1.51.0)
+    rspec-support (3.12.1)
+    rubocop (1.56.0)
+      base64 (~> 0.1.1)
       json (~> 2.3)
+      language_server-protocol (>= 3.17.0)
       parallel (~> 1.10)
-      parser (>= 3.2.0.0)
+      parser (>= 3.2.2.3)
       rainbow (>= 2.2.2, < 4.0)
       regexp_parser (>= 1.8, < 3.0)
       rexml (>= 3.2.5, < 4.0)
-      rubocop-ast (>= 1.28.0, < 2.0)
+      rubocop-ast (>= 1.28.1, < 2.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (>= 2.4.0, < 3.0)
-    rubocop-ast (1.28.1)
+    rubocop-ast (1.29.0)
       parser (>= 3.2.1.0)
     rubocop-capybara (2.18.0)
       rubocop (~> 1.41)
-    rubocop-factory_bot (2.23.0)
+    rubocop-factory_bot (2.23.1)
       rubocop (~> 1.33)
     rubocop-packs (0.0.42)
       activesupport
@@ -145,7 +152,7 @@ GEM
       rubocop
       rubocop-sorbet
       sorbet-runtime
-    rubocop-rspec (2.22.0)
+    rubocop-rspec (2.23.2)
       rubocop (~> 1.33)
       rubocop-capybara (~> 2.17)
       rubocop-factory_bot (~> 2.22)
@@ -155,27 +162,27 @@ GEM
       rexml
     ruby-progressbar (1.13.0)
     smart_properties (1.17.0)
-    sorbet (0.5.10826)
-      sorbet-static (= 0.5.10826)
-    sorbet-runtime (0.5.10826)
-    sorbet-static (0.5.10826-universal-darwin-14)
-    sorbet-static (0.5.10826-universal-darwin-15)
-    sorbet-static (0.5.10826-universal-darwin-16)
-    sorbet-static (0.5.10826-universal-darwin-17)
-    sorbet-static (0.5.10826-universal-darwin-18)
-    sorbet-static (0.5.10826-universal-darwin-19)
-    sorbet-static (0.5.10826-universal-darwin-20)
-    sorbet-static (0.5.10826-universal-darwin-21)
-    sorbet-static (0.5.10826-universal-darwin-22)
-    sorbet-static (0.5.10826-x86_64-linux)
-    sorbet-static-and-runtime (0.5.10826)
-      sorbet (= 0.5.10826)
-      sorbet-runtime (= 0.5.10826)
+    sorbet (0.5.10957)
+      sorbet-static (= 0.5.10957)
+    sorbet-runtime (0.5.10957)
+    sorbet-static (0.5.10957-universal-darwin-14)
+    sorbet-static (0.5.10957-universal-darwin-15)
+    sorbet-static (0.5.10957-universal-darwin-16)
+    sorbet-static (0.5.10957-universal-darwin-17)
+    sorbet-static (0.5.10957-universal-darwin-18)
+    sorbet-static (0.5.10957-universal-darwin-19)
+    sorbet-static (0.5.10957-universal-darwin-20)
+    sorbet-static (0.5.10957-universal-darwin-21)
+    sorbet-static (0.5.10957-universal-darwin-22)
+    sorbet-static (0.5.10957-x86_64-linux)
+    sorbet-static-and-runtime (0.5.10957)
+      sorbet (= 0.5.10957)
+      sorbet-runtime (= 0.5.10957)
     spoom (1.2.1)
       sorbet (>= 0.5.10187)
       sorbet-runtime (>= 0.5.9204)
       thor (>= 0.19.2)
-    tapioca (0.11.6)
+    tapioca (0.11.8)
       bundler (>= 2.2.25)
       netrc (>= 0.11.0)
       parallel (>= 1.21.0)
@@ -198,7 +205,7 @@ GEM
     tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.4.2)
-    unparser (0.6.7)
+    unparser (0.6.8)
       diff-lcs (~> 1.3)
       parser (>= 3.2.0)
     visualize_packs (0.3.1)
@@ -213,7 +220,7 @@ GEM
     yard-sorbet (0.8.1)
       sorbet-runtime (>= 0.5)
       yard (>= 0.9)
-    zeitwerk (2.6.8)
+    zeitwerk (2.6.11)
 
 PLATFORMS
   ruby
@@ -230,6 +237,7 @@ DEPENDENCIES
   rubocop-rspec
   sorbet
   sorbet-static
+  spoom (= 1.2.1)
   tapioca
 
 BUNDLED WITH

lib/.DS_Store

@@ -36,6 +36,21 @@ def self.start_interactive_mode!
     Private::InteractiveCli.start!
   end
 
+  sig { returns(T::Boolean) }
+  def self.update
+    Private.system_with('bin/packwerk update-todo')
+  end
+
+  sig { returns(T::Boolean) }
+  def self.validate
+    Private.system_with('bin/packwerk validate')
+  end
+
+  sig { params(files: T::Array[String]).returns(T::Boolean) }
+  def self.check(files)
+    Private.system_with("bin/packwerk check #{files.join(' ')}")
+  end
+
   sig do
     params(
       pack_name: String,
@@ -222,32 +237,6 @@ def self.bust_cache!
     Specification.bust_cache!
   end
 
-  #
-  # execute_command is like `run` except it does not `exit`
-  #
-  sig { params(argv: T.untyped, formatter: T.nilable(Packwerk::OffensesFormatter)).void }
-  def self.execute(argv, formatter = nil)
-    Private::PackwerkWrapper.with_safe_exit_if_no_files_found do
-      Private::PackwerkWrapper.packwerk_cli(formatter).execute_command(argv)
-    end
-  end
-
-  sig { params(files: T::Array[String]).returns(T::Array[Packwerk::ReferenceOffense]) }
-  def self.get_offenses_for_files(files)
-    formatter = Private::PackwerkWrapper::OffensesAggregatorFormatter.new
-    Private::PackwerkWrapper.packwerk_cli_execute_safely(['check', *files], formatter)
-    formatter.aggregated_offenses.compact
-  end
-
-  sig { params(files: T::Array[String]).returns(T::Array[Packwerk::ReferenceOffense]) }
-  def self.get_offenses_for_files_by_package(files)
-    packages = Private::PackwerkWrapper.package_names_for_files(files)
-    argv = ['check', '--packages', packages.join(',')]
-    formatter = Private::PackwerkWrapper::OffensesAggregatorFormatter.new
-    Private::PackwerkWrapper.packwerk_cli_execute_safely(argv, formatter)
-    formatter.aggregated_offenses.compact
-  end
-
   sig { void }
   def self.lint_package_todo_yml_files!
     Private.lint_package_todo_yml_files!

lib/packs.rb

@@ -10,6 +10,7 @@ class CLI < Thor
     sig { params(pack_name: String).void }
     def create(pack_name)
       Packs.create_pack!(pack_name: pack_name)
+      exit_successfully
     end
 
     desc 'add_dependency packs/from_pack packs/to_pack', 'Add packs/to_pack to packs/from_pack/package.yml list of dependencies'
@@ -27,6 +28,7 @@ def add_dependency(from_pack, to_pack)
         pack_name: from_pack,
         dependency_name: to_pack
       )
+      exit_successfully
     end
 
     desc 'list_top_dependency_violations packs/your_pack', 'List the top dependency violations of packs/your_pack'
@@ -44,6 +46,7 @@ def list_top_dependency_violations(pack_name)
         pack_name: pack_name,
         limit: options[:limit]
       )
+      exit_successfully
     end
 
     desc 'list_top_privacy_violations packs/your_pack', 'List the top privacy violations of packs/your_pack'
@@ -61,6 +64,7 @@ def list_top_privacy_violations(pack_name)
         pack_name: pack_name,
         limit: options[:limit]
       )
+      exit_successfully
     end
 
     desc 'make_public path/to/file.rb path/to/directory', 'Make files or directories public API'
@@ -75,6 +79,7 @@ def make_public(*paths)
         paths_relative_to_root: paths,
         per_file_processors: [Packs::RubocopPostProcessor.new, Packs::CodeOwnershipPostProcessor.new]
       )
+      exit_successfully
     end
 
     desc 'move packs/destination_pack path/to/file.rb path/to/directory', 'Move files or directories from one pack to another'
@@ -91,6 +96,7 @@ def move(pack_name, *paths)
         paths_relative_to_root: paths,
         per_file_processors: [Packs::RubocopPostProcessor.new, Packs::CodeOwnershipPostProcessor.new]
       )
+      exit_successfully
     end
 
     desc 'lint_package_todo_yml_files', 'Lint `package_todo.yml` files to check for formatting issues'
@@ -108,37 +114,40 @@ def lint_package_yml_files(*pack_names)
     desc 'validate', 'Run bin/packwerk validate (detects cycles)'
     sig { void }
     def validate
-      system('bin/packwerk validate')
+      Private.exit_with(Packs.validate)
     end
 
     desc 'check [ packs/my_pack ]', 'Run bin/packwerk check'
     sig { params(paths: String).void }
     def check(*paths)
-      Packs.execute(['check', *paths])
+      Private.exit_with(Packs.check(paths))
     end
 
     desc 'update', 'Run bin/packwerk update-todo'
     sig { void }
     def update
-      system('bin/packwerk update-todo')
+      Private.exit_with(Packs.update)
     end
 
     desc 'get_info [ packs/my_pack packs/my_other_pack ]', 'Get info about size and violations for packs'
     sig { params(pack_names: String).void }
     def get_info(*pack_names)
       Private.get_info(packs: parse_pack_names(pack_names))
+      exit_successfully
     end
 
     desc 'visualize [ packs/my_pack packs/my_other_pack ]', 'Visualize packs'
     sig { params(pack_names: String).void }
     def visualize(*pack_names)
       Private.visualize(packs: parse_pack_names(pack_names))
+      exit_successfully
     end
 
     desc 'rename', 'Rename a pack'
     sig { void }
     def rename
       puts Private.rename_pack
+      exit_successfully
     end
 
     desc 'move_to_parent packs/child_pack packs/parent_pack ', 'Set packs/child_pack as a child of packs/parent_pack'
@@ -149,6 +158,7 @@ def move_to_parent(child_pack_name, parent_pack_name)
         pack_name: child_pack_name,
         per_file_processors: [Packs::RubocopPostProcessor.new, Packs::CodeOwnershipPostProcessor.new]
       )
+      exit_successfully
     end
 
     private
@@ -159,6 +169,11 @@ def move_to_parent(child_pack_name, parent_pack_name)
       def parse_pack_names(pack_names)
         pack_names.empty? ? Packs.all : pack_names.map { |p| Packs.find(p.gsub(%r{/$}, '')) }.compact
       end
+
+      sig { void }
+      def exit_successfully
+        Private.exit_with(true)
+      end
     end
   end
 end