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

README.md

@@ -502,6 +502,8 @@ Options:
   -w,        [--workers=N]                                                                         # Number of parallel workers to use when generating RBIs (default: auto)
              [--rbi-max-line-length=N]                                                             # Set the max line length of generated RBIs. Signatures longer than the max line length will be wrapped
                                                                                                    # Default: 120
+             [--max-diff-lines=N]                                                                  # Max number of diff lines to include in the `dsl --verify` output
+                                                                                                   # Default: 250
   -e,        [--environment=ENVIRONMENT]                                                           # The Rack/Rails environment to use when generating RBIs
                                                                                                    # Default: development
   -l,        [--list-compilers], [--no-list-compilers], [--skip-list-compilers]                    # List all loaded compilers
@@ -960,6 +962,7 @@ dsl:
   quiet: false
   workers: 1
   rbi_max_line_length: 120
+  max_diff_lines: 250
   environment: development
   list_compilers: false
   app_root: "."

lib/tapioca.rb

@@ -32,6 +32,7 @@ class Error < StandardError; end
 
   DEFAULT_RBI_MAX_LINE_LENGTH = 120
   DEFAULT_ENVIRONMENT = "development"
+  DEFAULT_MAX_DIFF_LINES = 250
 
   CENTRAL_REPO_ROOT_URI = "https://raw.githubusercontent.com/Shopify/rbi-central/main"
   CENTRAL_REPO_INDEX_PATH = "index.json"

lib/tapioca/cli.rb

@@ -116,6 +116,10 @@ def todo
       type: :numeric,
       desc: "Set the max line length of generated RBIs. Signatures longer than the max line length will be wrapped",
       default: DEFAULT_RBI_MAX_LINE_LENGTH
+    option :max_diff_lines,
+      type: :numeric,
+      desc: "Max number of diff lines to include in the `dsl --verify` output",
+      default: DEFAULT_MAX_DIFF_LINES
     option :environment,
       aliases: ["-e"],
       type: :string,
@@ -166,6 +170,7 @@ def dsl(*constant_or_paths)
         halt_upon_load_error: options[:halt_upon_load_error],
         compiler_options: options[:compiler_options],
         lsp_addon: self.class.addon_mode,
+        max_diff_lines: options[:max_diff_lines],
       }
 
       command = if options[:verify]

lib/tapioca/commands/abstract_dsl.rb

@@ -26,7 +26,8 @@ class AbstractDsl < CommandWithoutTracker
       #|   ?app_root: String,
       #|   ?halt_upon_load_error: bool,
       #|   ?compiler_options: Hash[String, untyped],
-      #|   ?lsp_addon: bool
+      #|   ?lsp_addon: bool,
+      #|   ?max_diff_lines: Integer
       #| ) -> void
       def initialize(
         requested_constants:,
@@ -46,7 +47,8 @@ def initialize(
         app_root: ".",
         halt_upon_load_error: true,
         compiler_options: {},
-        lsp_addon: false
+        lsp_addon: false,
+        max_diff_lines: DEFAULT_MAX_DIFF_LINES
       )
         @requested_constants = requested_constants
         @requested_paths = requested_paths
@@ -66,6 +68,7 @@ def initialize(
         @skip_constant = skip_constant
         @compiler_options = compiler_options
         @lsp_addon = lsp_addon
+        @max_diff_lines = max_diff_lines
 
         super()
       end
@@ -240,9 +243,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 +267,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 +279,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 +297,10 @@ 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
 
       #: (Symbol cause, Array[String] files) -> String
@@ -305,16 +312,25 @@ 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
+          diff_lines = diff_output.lines.count
+          file_diff = if diff_lines.between?(1, @max_diff_lines)
+            "#{set_color("Diff:", :red)}\n#{diff_output.chomp}"
+          elsif diff_lines > @max_diff_lines
+            set_color("Diff not displayed as it exceeds #{@max_diff_lines} lines", :red)
+          else
+            ""
+          end
+
+          raise Tapioca::Error, <<~ERROR.rstrip
             #{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 +339,8 @@ def report_diff_and_exit_if_out_of_date(diff, command)
 
             #{set_color("Reason:", :red)}
             #{reasons}
+
+            #{file_diff}
           ERROR
         end
       end

lib/tapioca/helpers/rbi_files_helper.rb

@@ -1,6 +1,8 @@
 # typed: strict
 # frozen_string_literal: true
 
+require "open3"
+
 module Tapioca
   # @requires_ancestor: Thor::Shell
   # @requires_ancestor: SorbetHelper
@@ -137,6 +139,26 @@ def validate_rbi_files(command:, gem_dir:, dsl_dir:, auto_strictness:, gems: [],
       Kernel.raise Tapioca::Error, error_messages.join("\n") if parse_errors.any?
     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,
+      )
+      Kernel.raise stderr.chomp unless [0, 1].include?(status.exitstatus)
+
+      stdout
+    rescue => e
+      say_error("Failed to create #{filename} diff. #{e.message}", :red)
+      ""
+    end
+
     private
 
     #: (RBI::Index index, Array[String] files, number_of_workers: Integer?) -> void