Skip to content

Commit 292f17f

Browse files
KaanOzkanKaanOzkan
committed
Preserve Sorbet warnings in formatted srb output
Capture leading Sorbet stderr lines while parsing typecheck output so formatted `spoom srb tc` and `spoom srb bump` output no longer drops warnings emitted before errors. Assisted-By: devx/eb25d669-b4ee-4e28-9b69-129b45e1e178
1 parent f13a8a2 commit 292f17f

7 files changed

Lines changed: 171 additions & 5 deletions

File tree

‎lib/spoom/cli/srb/bump.rb‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ def bump(directory = ".")
135135
end
136136

137137
if result.status
138+
parse_result = Sorbet::Errors::Parser.parse_result(result.err, error_url_base: error_url_base)
139+
parse_result.warnings.each do |warning|
140+
say_error(warning, status: nil, nl: false)
141+
end
138142
Sorbet::Sigils.change_sigil_in_files(files_to_bump, to) unless dry
139143
print_changes(files_to_bump, command: cmd, from: from, to: to, dry: dry, path: exec_path)
140144
exit(files_to_bump.empty?)
@@ -147,7 +151,11 @@ def bump(directory = ".")
147151
exit(1)
148152
end
149153

150-
errors = Sorbet::Errors::Parser.parse_string(result.err, error_url_base: error_url_base)
154+
parse_result = Sorbet::Errors::Parser.parse_result(result.err, error_url_base: error_url_base)
155+
parse_result.warnings.each do |warning|
156+
say_error(warning, status: nil, nl: false)
157+
end
158+
errors = parse_result.errors
151159

152160
all_files = errors.flat_map do |err|
153161
[err.file, *err.files_from_error_sections]

‎lib/spoom/cli/srb/tc.rb‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,11 @@ def tc(*paths_to_select)
7373
exit(1)
7474
end
7575

76-
errors = Spoom::Sorbet::Errors::Parser.parse_string(result.err, error_url_base: error_url_base)
76+
parse_result = Spoom::Sorbet::Errors::Parser.parse_result(result.err, error_url_base: error_url_base)
77+
parse_result.warnings.each do |warning|
78+
say_error(warning, status: nil, nl: false)
79+
end
80+
errors = parse_result.errors
7781
errors_count = errors.size
7882

7983
errors = errors.select { |e| e.code == code } if code

‎lib/spoom/sorbet/errors.rb‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,22 +56,35 @@ class ParseError < Spoom::Error; end
5656
] #: Array[String]
5757

5858
class << self
59+
# Used when only Sorbet errors are needed and leading stderr warnings can be ignored.
5960
#: (String output, ?error_url_base: String) -> Array[Error]
6061
def parse_string(output, error_url_base: DEFAULT_ERROR_URL_BASE)
62+
parse_result(output, error_url_base: error_url_base).errors
63+
end
64+
65+
# Used when callers need both parsed Sorbet errors and leading stderr warnings.
66+
#: (String output, ?error_url_base: String) -> ParseResult
67+
def parse_result(output, error_url_base: DEFAULT_ERROR_URL_BASE)
6168
parser = Spoom::Sorbet::Errors::Parser.new(error_url_base: error_url_base)
62-
parser.parse(output)
69+
parser.parse_result(output)
6370
end
6471
end
6572

6673
#: (?error_url_base: String) -> void
6774
def initialize(error_url_base: DEFAULT_ERROR_URL_BASE)
6875
@errors = [] #: Array[Error]
76+
@warnings = [] #: Array[String]
6977
@error_line_match_regex = error_line_match_regexp(error_url_base) #: Regexp
7078
@current_error = nil #: Error?
7179
end
7280

7381
#: (String output) -> Array[Error]
7482
def parse(output)
83+
parse_result(output).errors
84+
end
85+
86+
#: (String output) -> ParseResult
87+
def parse_result(output)
7588
output.each_line do |line|
7689
break if /^No errors! Great job\./.match?(line)
7790
break if /^Errors: /.match?(line)
@@ -85,10 +98,14 @@ def parse(output)
8598
next
8699
end
87100

88-
append_error(line) if @current_error
101+
if @current_error
102+
append_error(line)
103+
else
104+
append_warning(line)
105+
end
89106
end
90107
close_error if @current_error
91-
@errors
108+
ParseResult.new(@errors, @warnings)
92109
end
93110

94111
private
@@ -144,6 +161,11 @@ def append_error(line)
144161
end
145162
@current_error.more << line
146163
end
164+
165+
#: (String warning) -> void
166+
def append_warning(warning)
167+
@warnings << warning
168+
end
147169
end
148170

149171
class Error
@@ -214,6 +236,20 @@ def to_junit_xml_element
214236
testcase_element
215237
end
216238
end
239+
240+
class ParseResult
241+
#: Array[Error]
242+
attr_reader :errors
243+
244+
#: Array[String]
245+
attr_reader :warnings
246+
247+
#: (Array[Error] errors, Array[String] warnings) -> void
248+
def initialize(errors, warnings)
249+
@errors = errors
250+
@warnings = warnings
251+
end
252+
end
217253
end
218254
end
219255
end

‎rbi/spoom.rbi‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2753,18 +2753,35 @@ class Spoom::Sorbet::Errors::Error
27532753
def to_s; end
27542754
end
27552755

2756+
class Spoom::Sorbet::Errors::ParseResult
2757+
sig { params(errors: T::Array[::Spoom::Sorbet::Errors::Error], warnings: T::Array[::String]).void }
2758+
def initialize(errors, warnings); end
2759+
2760+
sig { returns(T::Array[::Spoom::Sorbet::Errors::Error]) }
2761+
def errors; end
2762+
2763+
sig { returns(T::Array[::String]) }
2764+
def warnings; end
2765+
end
2766+
27562767
class Spoom::Sorbet::Errors::Parser
27572768
sig { params(error_url_base: ::String).void }
27582769
def initialize(error_url_base: T.unsafe(nil)); end
27592770

27602771
sig { params(output: ::String).returns(T::Array[::Spoom::Sorbet::Errors::Error]) }
27612772
def parse(output); end
27622773

2774+
sig { params(output: ::String).returns(::Spoom::Sorbet::Errors::ParseResult) }
2775+
def parse_result(output); end
2776+
27632777
private
27642778

27652779
sig { params(line: ::String).void }
27662780
def append_error(line); end
27672781

2782+
sig { params(warning: ::String).void }
2783+
def append_warning(warning); end
2784+
27682785
sig { void }
27692786
def close_error; end
27702787

@@ -2778,6 +2795,9 @@ class Spoom::Sorbet::Errors::Parser
27782795
def open_error(error); end
27792796

27802797
class << self
2798+
sig { params(output: ::String, error_url_base: ::String).returns(::Spoom::Sorbet::Errors::ParseResult) }
2799+
def parse_result(output, error_url_base: T.unsafe(nil)); end
2800+
27812801
sig { params(output: ::String, error_url_base: ::String).returns(T::Array[::Spoom::Sorbet::Errors::Error]) }
27822802
def parse_string(output, error_url_base: T.unsafe(nil)); end
27832803
end

‎test/spoom/cli/srb/bump_test.rb‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,59 @@ def foo
533533
assert_equal("false", @project.read_file_strictness("file1.rb"))
534534
end
535535

536+
def test_bump_displays_sorbet_warnings
537+
@project.write!("mock_sorbet", <<~RB)
538+
#!/usr/bin/env ruby
539+
$stderr.puts "Option `--enable-experimental-rbs-signatures` has been combined into another option."
540+
$stderr.puts "file.rb:1: Method `foo` does not exist on `T.class_of(<root>)` https://srb.help/7003"
541+
exit(100)
542+
RB
543+
@project.exec("chmod +x mock_sorbet")
544+
@project.write!("file.rb", <<~RB)
545+
# typed: false
546+
foo
547+
RB
548+
549+
result = @project.spoom("srb bump --no-color --sorbet #{@project.absolute_path}/mock_sorbet")
550+
assert_equal(<<~ERR, result.err)
551+
Option `--enable-experimental-rbs-signatures` has been combined into another option.
552+
ERR
553+
assert_equal(<<~OUT, result.out)
554+
Checking files...
555+
556+
No files to bump from `false` to `true`
557+
OUT
558+
assert(result.status)
559+
assert_equal("false", @project.read_file_strictness("file.rb"))
560+
end
561+
562+
def test_bump_displays_sorbet_warnings_without_errors
563+
@project.write!("mock_sorbet", <<~RB)
564+
#!/usr/bin/env ruby
565+
$stderr.puts "Option `--enable-experimental-rbs-signatures` has been combined into another option."
566+
$stderr.puts "No errors! Great job."
567+
exit(0)
568+
RB
569+
@project.exec("chmod +x mock_sorbet")
570+
@project.write!("file.rb", <<~RB)
571+
# typed: false
572+
class Foo; end
573+
RB
574+
575+
result = @project.spoom("srb bump --no-color --sorbet #{@project.absolute_path}/mock_sorbet")
576+
assert_equal(<<~ERR, result.err)
577+
Option `--enable-experimental-rbs-signatures` has been combined into another option.
578+
ERR
579+
assert_equal(<<~OUT, result.out)
580+
Checking files...
581+
582+
Bumped `1` file from `false` to `true`:
583+
+ file.rb
584+
OUT
585+
refute(result.status)
586+
assert_equal("true", @project.read_file_strictness("file.rb"))
587+
end
588+
536589
def test_bump_with_sorbet_segfault
537590
# Create a fake Sorbet that will segfault
538591
@project.write!("mock_sorbet", <<~RB)

‎test/spoom/cli/srb/tc_test.rb‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,24 @@ def test_display_sorbet_error
461461
refute(result.status)
462462
end
463463

464+
def test_display_sorbet_warnings_with_formatted_errors
465+
@project.write!("mock_sorbet", <<~RB)
466+
#!/usr/bin/env ruby
467+
$stderr.puts "Option `--enable-experimental-rbs-signatures` has been combined into another option."
468+
$stderr.puts "foo.rb:1: Method `foo` does not exist on `T.class_of(<root>)` https://srb.help/7003"
469+
exit(100)
470+
RB
471+
@project.exec("chmod +x mock_sorbet")
472+
473+
result = @project.spoom("srb tc --no-color --sorbet #{@project.absolute_path}/mock_sorbet")
474+
assert_equal(<<~MSG, result.err)
475+
Option `--enable-experimental-rbs-signatures` has been combined into another option.
476+
7003 - foo.rb:1: Method `foo` does not exist on `T.class_of(<root>)`
477+
Errors: 1
478+
MSG
479+
refute(result.status)
480+
end
481+
464482
def test_deprecated_command_spoom_tc
465483
@project.remove!("errors")
466484
result = @project.spoom("tc --no-color")

‎test/spoom/sorbet/errors_test.rb‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,33 @@ def test_parses_errors_with_debug_string
214214
assert_equal([[], []], errors.map(&:files_from_error_sections).map(&:to_a))
215215
end
216216

217+
def test_parse_result_preserves_warnings
218+
result = Spoom::Sorbet::Errors::Parser.parse_result(<<~ERR)
219+
Option `--enable-experimental-rbs-signatures` has been combined into another option.
220+
Please add `--parser=prism` to your Sorbet config.
221+
222+
a.rb:80: unexpected token "end" https://srb.help/2001
223+
80 |end
224+
^^^
225+
Errors: 1
226+
ERR
227+
228+
assert_equal(
229+
[
230+
"Option `--enable-experimental-rbs-signatures` has been combined into another option.\n",
231+
"Please add `--parser=prism` to your Sorbet config.\n",
232+
],
233+
result.warnings,
234+
)
235+
assert_equal(1, result.errors.size)
236+
237+
error = T.must(result.errors.first)
238+
assert_equal("a.rb", error.file)
239+
assert_equal(80, error.line)
240+
assert_equal("unexpected token \"end\"", error.message)
241+
assert_equal(2001, error.code)
242+
end
243+
217244
def test_parses_errors_with_multiple_blank_lines
218245
errors = Spoom::Sorbet::Errors::Parser.parse_string(<<~ERR)
219246
lib/a.rb:54: Method `foo` does not exist on `String` https://srb.help/7003

0 commit comments

Comments
 (0)