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
@@ -242,7 +245,7 @@ def compile_dsl_rbi(constant_name, rbi, outpath: @outpath, quiet: false)
       def perform_dsl_verification(dir)
         diff = 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, tmp_dir: dir, command: :dsl)
       ensure
         FileUtils.remove_entry(dir)
       end
@@ -305,26 +308,67 @@ 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, tmp_dir: Pathname, command: Symbol) -> void
+      def report_diff_and_exit_if_out_of_date(diff, tmp_dir:, 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")
+          return
+        end
+
+        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")
+
+        diff_output = build_diff_output(diff, tmp_dir)
+        diff_lines = diff_output.count("\n")
+
+        diff_section =
+          if diff_lines.between?(1, @max_diff_lines)
+            "#{set_color("Diff:", :red)}\n#{diff_output.chomp}"
+          elsif diff_lines > @max_diff_lines
+            truncated_output = diff_output.lines.first(@max_diff_lines).join
+            "#{set_color("Diff truncated to #{@max_diff_lines} lines:", :red)}\n#{truncated_output.rstrip}"
+          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)}
+          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`
 
-          raise Tapioca::Error, <<~ERROR
-            #{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)}
-            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`
+          #{set_color("Reason:", :red)}
+          #{reasons}
 
-            #{set_color("Reason:", :red)}
-            #{reasons}
-          ERROR
+          #{diff_section}
+        ERROR
+      end
+
+      #: (Hash[String, Symbol] diff, Pathname tmp_dir) -> String
+      def build_diff_output(diff, tmp_dir)
+        out = String.new
+        line_count = 0
+
+        diff.each do |file, status|
+          filename = file.to_s
+          old_path = (@outpath / file).to_s
+          new_path = (tmp_dir / file).to_s
+
+          chunk = case status
+          when :added then file_diff(filename, File::NULL, new_path)
+          when :removed then file_diff(filename, old_path, File::NULL)
+          when :changed then file_diff(filename, old_path, new_path)
+          else ""
+          end
+
+          out << chunk
+          line_count += chunk.count("\n")
+          break if line_count > @max_diff_lines
         end
+
+        out
       end
 
       #: (Pathname path) -> Array[Pathname]

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,30 @@ 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
 
+    #: (String filename, String old_path, String new_path) -> String
+    def file_diff(filename, old_path, new_path)
+      stdout, stderr, status = Open3.capture3(
+        "diff",
+        "-u",
+        "--label",
+        filename,
+        "--label",
+        filename,
+        old_path,
+        new_path,
+      )
+
+      unless [0, 1].include?(status.exitstatus)
+        say_error("Failed to create #{filename} diff. #{stderr.chomp}", :red)
+        return ""
+      end
+
+      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