From 77c58c78d98df0011384cab0c02cb889dbe7be03 Mon Sep 17 00:00:00 2001 From: Julia Boutin Date: Thu, 2 Apr 2026 16:51:11 -0600 Subject: [PATCH] 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 50 lines or less --- lib/tapioca/commands/abstract_dsl.rb | 31 +++- spec/tapioca/cli/dsl_spec.rb | 203 ++++++++++++++++++++++++++- 2 files changed, 225 insertions(+), 9 deletions(-) diff --git a/lib/tapioca/commands/abstract_dsl.rb b/lib/tapioca/commands/abstract_dsl.rb index 72320c46c..c0a94317b 100644 --- a/lib/tapioca/commands/abstract_dsl.rb +++ b/lib/tapioca/commands/abstract_dsl.rb @@ -240,9 +240,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 +264,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 = "" existing_rbis = rbi_files_in(@outpath) new_rbis = rbi_files_in(tmp_dir) @@ -275,12 +276,14 @@ def verify_dsl_rbi(tmp_dir:) added_files.each do |file| diff[file] = :added + diff_output += file_diff(file, "/dev/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, "/dev/null") end common_files = (existing_rbis & new_rbis) @@ -291,9 +294,17 @@ 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) + %x(diff -u --label #{filename} --label #{filename} #{old_path} #{new_path}) + "\n" + rescue + "" end #: (Symbol cause, Array[String] files) -> String @@ -305,8 +316,8 @@ 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 @@ -314,7 +325,11 @@ def report_diff_and_exit_if_out_of_date(diff, command) 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, 50) + "#{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 +338,8 @@ def report_diff_and_exit_if_out_of_date(diff, command) #{set_color("Reason:", :red)} #{reasons} + + #{file_diff} ERROR end end diff --git a/spec/tapioca/cli/dsl_spec.rb b/spec/tapioca/cli/dsl_spec.rb index 909a437a1..f0fa08cd3 100644 --- a/spec/tapioca/cli/dsl_spec.rb +++ b/spec/tapioca/cli/dsl_spec.rb @@ -1934,6 +1934,15 @@ 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 @@ -1971,6 +1980,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 +2042,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 +2113,152 @@ 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 diff exceeds 50 lines" do + @project.tapioca("dsl") + + @project.write!("lib/post.rb", <<~RB) + require "smart_properties" + + class Post + include SmartProperties + property :title, accepts: String + property :a, accepts: String + property :b, accepts: String + property :c, accepts: String + property :d, accepts: String + property :e, accepts: String + property :f, accepts: String + property :g, accepts: String + property :h, accepts: String + property :i, accepts: String + 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)