Display line diffs in dsl --verify error messages

When `dsl --verify` detects out of date RBI files,
the error message lists which files were added,
removed, or changed. We could make it easier to
understand what has changed by including the line
diff output in the error message as well

This commit grabs the line diff output during RBI
verification and includes it in the error message
when the diff is 250 lines or less

Author: dejmedus

lib/tapioca/commands/abstract_dsl.rb

@@ -1,6 +1,8 @@
 # typed: strict
 # frozen_string_literal: true
 
+require "open3"
+
 module Tapioca
   module Commands
     # @abstract
@@ -240,9 +242,9 @@ def compile_dsl_rbi(constant_name, rbi, outpath: @outpath, quiet: false)
 
       #: (Pathname dir) -> void
       def perform_dsl_verification(dir)
-        diff = verify_dsl_rbi(tmp_dir: dir)
+        diff, diff_output = verify_dsl_rbi(tmp_dir: dir)
 
-        report_diff_and_exit_if_out_of_date(diff, :dsl)
+        report_diff_and_exit_if_out_of_date(diff, diff_output, :dsl)
       ensure
         FileUtils.remove_entry(dir)
       end
@@ -264,9 +266,10 @@ def dsl_rbi_filename(constant_name)
         @outpath / "#{underscore(constant_name)}.rbi"
       end
 
-      #: (tmp_dir: Pathname) -> Hash[String, Symbol]
+      #: (tmp_dir: Pathname) -> [Hash[String, Symbol], String]
       def verify_dsl_rbi(tmp_dir:)
         diff = {}
+        diff_output = String.new
 
         existing_rbis = rbi_files_in(@outpath)
         new_rbis = rbi_files_in(tmp_dir)
@@ -275,12 +278,14 @@ def verify_dsl_rbi(tmp_dir:)
 
         added_files.each do |file|
           diff[file] = :added
+          diff_output << file_diff(file, File::NULL, tmp_dir / file)
         end
 
         removed_files = (existing_rbis - new_rbis)
 
         removed_files.each do |file|
           diff[file] = :removed
+          diff_output << file_diff(file, @outpath / file, File::NULL)
         end
 
         common_files = (existing_rbis & new_rbis)
@@ -291,9 +296,30 @@ def verify_dsl_rbi(tmp_dir:)
 
         changed_files.each do |file|
           diff[file] = :changed
+          diff_output << file_diff(file, @outpath / file, tmp_dir / file)
         end
 
-        diff
+        [diff, diff_output]
+      end
+
+      #: (Pathname filename, String | Pathname old_path, String | Pathname new_path) -> String
+      def file_diff(filename, old_path, new_path)
+        stdout, stderr, status = Open3.capture3(
+          "diff",
+          "-u",
+          "--label",
+          filename.to_s,
+          "--label",
+          filename.to_s,
+          old_path.to_s,
+          new_path.to_s,
+        )
+        raise stderr.chomp unless [0, 1].include?(status.exitstatus)
+
+        stdout
+      rescue => e
+        say_error("Failed to create #{filename} diff. #{e.message}", :yellow)
+        ""
       end
 
       #: (Symbol cause, Array[String] files) -> String
@@ -305,16 +331,20 @@ def build_error_for_files(cause, files)
         "  File(s) #{cause}:\n  - #{filenames}"
       end
 
-      #: (Hash[String, Symbol] diff, Symbol command) -> void
-      def report_diff_and_exit_if_out_of_date(diff, command)
+      #: (Hash[String, Symbol] diff, String diff_output, Symbol command) -> void
+      def report_diff_and_exit_if_out_of_date(diff, diff_output, command)
         if diff.empty?
           say("Nothing to do, all RBIs are up-to-date.")
         else
           reasons = diff.group_by(&:last).sort.map do |cause, diff_for_cause|
             build_error_for_files(cause, diff_for_cause.map(&:first))
           end.join("\n")
 
-          raise Tapioca::Error, <<~ERROR
+          file_diff = if diff_output.lines.count.between?(1, 250)
+            "#{set_color("Diff:", :red)}\n#{diff_output.chomp}"
+          end
+
+          raise Tapioca::Error, <<~ERROR.chomp
             #{set_color("RBI files are out-of-date. In your development environment, please run:", :green)}
               #{set_color("`#{default_command(command)}`", :green, :bold)}
             #{set_color("Once it is complete, be sure to commit and push any changes", :green)}
@@ -323,6 +353,8 @@ def report_diff_and_exit_if_out_of_date(diff, command)
 
             #{set_color("Reason:", :red)}
             #{reasons}
+
+            #{file_diff}
           ERROR
         end
       end

spec/tapioca/cli/dsl_spec.rb

@@ -1934,13 +1934,28 @@ def perform(foo, bar)
 
         after do
           @project.remove!("sorbet/rbi/dsl")
+          @project.remove!("lib/image.rb")
+          @project.write!("lib/post.rb", <<~RB)
+            require "smart_properties"
+
+            class Post
+              include SmartProperties
+              property :title, accepts: String
+            end
+          RB
         end
 
         it "does nothing and returns exit status 0 with no changes" do
           @project.tapioca("dsl")
           result = @project.tapioca("dsl --verify")
 
-          assert_stdout_includes(result, <<~OUT)
+          assert_stdout_equals(<<~OUT, result)
+            Loading DSL extension classes... Done
+            Loading Rails application... Done
+            Loading DSL compiler classes... Done
+            Checking for out-of-date RBIs...
+
+
             Nothing to do, all RBIs are up-to-date.
           OUT
 
@@ -1971,6 +1986,29 @@ def perform(foo, bar)
             Reason:
               File(s) removed:
               - sorbet/rbi/dsl/post.rbi
+
+            Diff:
+            --- post.rbi
+            +++ post.rbi
+            @@ -1,18 +0,0 @@
+            -# typed: true
+            -
+            -# DO NOT EDIT MANUALLY
+            -# This is an autogenerated file for dynamic methods in `Post`.
+            -# Please instead update this file by running `bin/tapioca dsl Post`.
+            -
+            -
+            -class Post
+            -  include SmartPropertiesGeneratedMethods
+            -
+            -  module SmartPropertiesGeneratedMethods
+            -    sig { returns(T.nilable(::String)) }
+            -    def title; end
+            -
+            -    sig { params(title: T.nilable(::String)).returns(T.nilable(::String)) }
+            -    def title=(title); end
+            -  end
+            -end
           ERROR
 
           refute_success_status(result)
@@ -2010,11 +2048,32 @@ class Image
             Reason:
               File(s) added:
               - sorbet/rbi/dsl/image.rbi
+
+            Diff:
+            --- image.rbi
+            +++ image.rbi
+            @@ -0,0 +1,18 @@
+            +# typed: true
+            +
+            +# DO NOT EDIT MANUALLY
+            +# This is an autogenerated file for dynamic methods in `Image`.
+            +# Please instead update this file by running `bin/tapioca dsl Image`.
+            +
+            +
+            +class Image
+            +  include SmartPropertiesGeneratedMethods
+            +
+            +  module SmartPropertiesGeneratedMethods
+            +    sig { returns(T.nilable(::String)) }
+            +    def title; end
+            +
+            +    sig { params(title: T.nilable(::String)).returns(T.nilable(::String)) }
+            +    def title=(title); end
+            +  end
+            +end
           ERROR
 
           refute_success_status(result)
-
-          @project.remove!("lib/image.rb")
         end
 
         it "advises of modified file(s) and returns exit status 1 with modified file" do
@@ -2060,6 +2119,142 @@ class Post
             Reason:
               File(s) changed:
               - sorbet/rbi/dsl/post.rbi
+
+            Diff:
+            --- post.rbi
+            +++ post.rbi
+            @@ -10,6 +10,12 @@
+            #{" "}
+               module SmartPropertiesGeneratedMethods
+                 sig { returns(T.nilable(::String)) }
+            +    def desc; end
+            +
+            +    sig { params(desc: T.nilable(::String)).returns(T.nilable(::String)) }
+            +    def desc=(desc); end
+            +
+            +    sig { returns(T.nilable(::String)) }
+                 def title; end
+            #{" "}
+                 sig { params(title: T.nilable(::String)).returns(T.nilable(::String)) }
+          ERROR
+
+          refute_success_status(result)
+        end
+
+        it "advises of added and changed files and returns exit status 1" do
+          @project.tapioca("dsl")
+
+          @project.write!("lib/post.rb", <<~RB)
+            require "smart_properties"
+
+            class Post
+              include SmartProperties
+              property :title, accepts: String
+              property :body, accepts: String
+            end
+          RB
+
+          @project.write!("lib/image.rb", <<~RB)
+            require "smart_properties"
+
+            class Image
+              include(SmartProperties)
+
+              property :title, accepts: String
+            end
+          RB
+
+          result = @project.tapioca("dsl --verify")
+
+          assert_stdout_equals(<<~OUT, result)
+            Loading DSL extension classes... Done
+            Loading Rails application... Done
+            Loading DSL compiler classes... Done
+            Checking for out-of-date RBIs...
+
+
+          OUT
+
+          assert_stderr_equals(<<~ERROR, result)
+            RBI files are out-of-date. In your development environment, please run:
+              `bin/tapioca dsl`
+            Once it is complete, be sure to commit and push any changes
+            If you don't observe any changes after running the command locally, ensure your database is in a good
+            state e.g. run `bin/rails db:reset`
+
+            Reason:
+              File(s) added:
+              - sorbet/rbi/dsl/image.rbi
+              File(s) changed:
+              - sorbet/rbi/dsl/post.rbi
+
+            Diff:
+            --- image.rbi
+            +++ image.rbi
+            @@ -0,0 +1,18 @@
+            +# typed: true
+            +
+            +# DO NOT EDIT MANUALLY
+            +# This is an autogenerated file for dynamic methods in `Image`.
+            +# Please instead update this file by running `bin/tapioca dsl Image`.
+            +
+            +
+            +class Image
+            +  include SmartPropertiesGeneratedMethods
+            +
+            +  module SmartPropertiesGeneratedMethods
+            +    sig { returns(T.nilable(::String)) }
+            +    def title; end
+            +
+            +    sig { params(title: T.nilable(::String)).returns(T.nilable(::String)) }
+            +    def title=(title); end
+            +  end
+            +end
+            --- post.rbi
+            +++ post.rbi
+            @@ -10,6 +10,12 @@
+            #{" "}
+               module SmartPropertiesGeneratedMethods
+                 sig { returns(T.nilable(::String)) }
+            +    def body; end
+            +
+            +    sig { params(body: T.nilable(::String)).returns(T.nilable(::String)) }
+            +    def body=(body); end
+            +
+            +    sig { returns(T.nilable(::String)) }
+                 def title; end
+            #{" "}
+                 sig { params(title: T.nilable(::String)).returns(T.nilable(::String)) }
+          ERROR
+
+          refute_success_status(result)
+        end
+
+        it "omits diff output when it exceeds 250 lines" do
+          @project.tapioca("dsl")
+
+          @project.write!("lib/post.rb", <<~RB)
+            require "smart_properties"
+
+            class Post
+              include SmartProperties
+              #{(1..250).map { |i| "property :p#{i}, accepts: String" }.join("\n")}
+            end
+          RB
+
+          result = @project.tapioca("dsl --verify")
+
+          assert_stderr_equals(<<~ERROR, result)
+            RBI files are out-of-date. In your development environment, please run:
+              `bin/tapioca dsl`
+            Once it is complete, be sure to commit and push any changes
+            If you don't observe any changes after running the command locally, ensure your database is in a good
+            state e.g. run `bin/rails db:reset`
+
+            Reason:
+              File(s) changed:
+              - sorbet/rbi/dsl/post.rbi
+
           ERROR
 
           refute_success_status(result)