Skip to content

Display line diffs in dsl --verify error messages #2592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
dejmedus wants to merge 1 commit into
base: main
Choose a base branch
from
+225 −9
Draft

Display line diffs in dsl --verify error messages #2592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions lib/tapioca/commands/abstract_dsl.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -305,16 +316,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, 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)}
Expand All @@ -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
Expand Down
203 changes: 201 additions & 2 deletions spec/tapioca/cli/dsl_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Loading