Skip to content

Commit 55e1646

Browse files
davidwessmandavidwessman
committed
Errors: Changes reporting and handling
- Relies on the parsing errors raised in ErbContent instead of rescuing them in the parser, this allows us to keep the line number of the error. - Removes location from all error messages - Adds more tests that actually check the error messages and line number
1 parent 2922893 commit 55e1646

File tree

4 files changed

+201
-82
lines changed

4 files changed

+201
-82
lines changed

‎lib/syntax_tree/erb/nodes.rb‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,10 +370,20 @@ def prepare_content(content)
370370
end
371371
rescue SyntaxTree::Parser::ParseError
372372
# Try to add the keyword to see if it parses
373-
result = ErbContent.new(value: [keyword, *content])
374-
@keyword = nil
375-
376-
result
373+
begin
374+
result = ErbContent.new(value: [keyword, *content])
375+
@keyword = nil
376+
377+
result
378+
rescue SyntaxTree::Parser::ParseError => error
379+
raise(
380+
SyntaxTree::Parser::ParseError.new(
381+
"Could not parse ERB-tag: #{error.message}",
382+
@opening_tag.location.start_line + error.lineno - 1,
383+
error.column
384+
)
385+
)
386+
end
377387
end
378388
end
379389

‎lib/syntax_tree/erb/parser.rb‎

Lines changed: 18 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def parse_any_tag
5252
if @found_doctype
5353
raise(
5454
SyntaxTree::Parser::ParseError.new(
55-
"Only one doctype element is allowed",
55+
"Duplicate doctype declaration",
5656
tag.location.start_line,
5757
0
5858
)
@@ -129,7 +129,7 @@ def make_tokens
129129
else
130130
raise(
131131
SyntaxTree::Parser::ParseError.new(
132-
"Unexpected character at #{index}: #{source[index]}",
132+
"Unexpected character: #{source[index]}",
133133
line,
134134
0
135135
)
@@ -218,7 +218,7 @@ def make_tokens
218218
else
219219
raise(
220220
SyntaxTree::Parser::ParseError.new(
221-
"Unexpected character in string at #{index}: #{source[index]}",
221+
"Unexpected character, #{source[index]}, when looking for closing single quote",
222222
line,
223223
0
224224
)
@@ -246,7 +246,7 @@ def make_tokens
246246
else
247247
raise(
248248
SyntaxTree::Parser::ParseError.new(
249-
"Unexpected character in string at #{index}: #{source[index]}",
249+
"Unexpected character, #{source[index]}, when looking for closing double quote",
250250
line,
251251
0
252252
)
@@ -305,7 +305,7 @@ def make_tokens
305305
else
306306
raise(
307307
SyntaxTree::Parser::ParseError.new(
308-
"Unexpected character at #{index}: #{source[index]}",
308+
"Unexpected character, #{source[index]}, when parsing HTML- or ERB-tag",
309309
line,
310310
0
311311
)
@@ -406,7 +406,7 @@ def parse_html_opening_tag
406406
if name.value =~ /\A[@:#]/
407407
raise(
408408
SyntaxTree::Parser::ParseError.new(
409-
"Invalid html-tag name #{name}",
409+
"Invalid HTML-tag name #{name.value}",
410410
name.location.start_line,
411411
0
412412
)
@@ -470,7 +470,7 @@ def parse_html_element
470470
if closing.nil?
471471
raise(
472472
SyntaxTree::Parser::ParseError.new(
473-
"Missing closing tag for <#{opening.name.value}> at #{opening.location}",
473+
"Missing closing tag for <#{opening.name.value}>",
474474
opening.location.start_line,
475475
0
476476
)
@@ -480,7 +480,7 @@ def parse_html_element
480480
if closing.name.value != opening.name.value
481481
raise(
482482
SyntaxTree::Parser::ParseError.new(
483-
"Expected closing tag for <#{opening.name.value}> but got <#{closing.name.value}> at #{closing.location}",
483+
"Expected closing tag for <#{opening.name.value}> but got <#{closing.name.value}>",
484484
closing.location.start_line,
485485
0
486486
)
@@ -505,10 +505,11 @@ def parse_erb_case(erb_node)
505505

506506
unless erb_tag.is_a?(ErbCaseWhen) || erb_tag.is_a?(ErbElse) ||
507507
erb_tag.is_a?(ErbEnd)
508+
location = erb_tag&.location || erb_node.location
508509
raise(
509510
SyntaxTree::Parser::ParseError.new(
510-
"Found no matching erb-tag to the if-tag at #{erb_node.location}",
511-
erb_node.location.start_line,
511+
"No matching ERB-tag for the <% #{erb_node.keyword.value} %>-statement",
512+
location.start_line,
512513
0
513514
)
514515
)
@@ -532,7 +533,7 @@ def parse_erb_case(erb_node)
532533
else
533534
raise(
534535
SyntaxTree::Parser::ParseError.new(
535-
"Found no matching when- or else-tag to the case-tag at #{erb_node.location}",
536+
"No matching when- or else-tag for the case-tag",
536537
erb_node.location.start_line,
537538
0
538539
)
@@ -552,7 +553,7 @@ def parse_erb_if(erb_node)
552553
unless erb_tag.is_a?(ErbControl) || erb_tag.is_a?(ErbEnd)
553554
raise(
554555
SyntaxTree::Parser::ParseError.new(
555-
"Found no matching erb-tag to the if-tag at #{erb_node.location}",
556+
"Found no matching ERB-tag for the <% if %>-statement",
556557
erb_node.location.start_line,
557558
0
558559
)
@@ -584,7 +585,7 @@ def parse_erb_if(erb_node)
584585
else
585586
raise(
586587
SyntaxTree::Parser::ParseError.new(
587-
"Found no matching elsif- or else-tag to the if-tag at #{erb_node.location}",
588+
"Found no matching elsif- or else-tag for the if-tag",
588589
erb_node.location.start_line,
589590
0
590591
)
@@ -600,7 +601,7 @@ def parse_erb_else(erb_node)
600601
unless erb_end.is_a?(ErbEnd)
601602
raise(
602603
SyntaxTree::Parser::ParseError.new(
603-
"Found no matching end-tag for the else-tag at #{erb_node.location}",
604+
"Found no matching end-tag for the else-tag",
604605
erb_node.location.start_line,
605606
0
606607
)
@@ -642,8 +643,8 @@ def parse_erb_tag
642643
if !closing_tag.is_a?(ErbClose)
643644
raise(
644645
SyntaxTree::Parser::ParseError.new(
645-
"Found no matching closing tag for the erb-tag at #{opening_tag.location}",
646-
opening_tag.location.start_line,
646+
"Found no matching closing tag for the ERB-tag",
647+
closing_tag.location.start_line,
647648
0
648649
)
649650
)
@@ -678,7 +679,7 @@ def parse_erb_tag
678679
unless erb_end.is_a?(ErbEnd)
679680
raise(
680681
SyntaxTree::Parser::ParseError.new(
681-
"Found no matching end-tag for the do-tag at #{erb_node.location}",
682+
"No matching <% end %> for the <% do %>-statement",
682683
erb_node.location.start_line,
683684
0
684685
)
@@ -695,27 +696,6 @@ def parse_erb_tag
695696
erb_node
696697
end
697698
end
698-
rescue SyntaxTree::Parser::ParseError => error
699-
# If we have parsed tokens that we cannot process after we parsed <%, we should throw a ParseError
700-
# and not let it be handled by a `maybe`.
701-
if opening_tag
702-
message =
703-
if error.message.include?("Could not parse ERB-tag")
704-
error.message
705-
else
706-
"Could not parse ERB-tag: #{error.message}"
707-
end
708-
709-
raise(
710-
SyntaxTree::Parser::ParseError.new(
711-
message,
712-
opening_tag.location.start_line,
713-
0
714-
)
715-
)
716-
else
717-
raise(error)
718-
end
719699
end
720700

721701
def parse_until_erb_close

‎test/erb_test.rb‎

Lines changed: 77 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,56 @@ def test_empty_file
1212
end
1313

1414
def test_missing_erb_end_tag
15-
assert_raises(SyntaxTree::Parser::ParseError) do
16-
ERB.parse("<% if no_end_tag %>")
17-
end
15+
example = <<-HTML
16+
<ul>
17+
<% if condition %>
18+
<li>A</li>
19+
<li>B</li>
20+
<li><%= "C" %></li>
21+
</ul>
22+
HTML
23+
ERB.parse(example)
24+
rescue SyntaxTree::Parser::ParseError => error
25+
assert_equal(2, error.lineno)
26+
assert_equal(0, error.column)
27+
assert_match(
28+
/Found no matching ERB-tag for the <% if %>-statement/,
29+
error.message
30+
)
1831
end
1932

2033
def test_missing_erb_block_end_tag
21-
assert_raises(SyntaxTree::Parser::ParseError) do
22-
ERB.parse("<% no_end_tag do %>")
23-
end
34+
example = <<-HTML
35+
<% no_end_tag do %>
36+
<h1>What</h1>
37+
HTML
38+
ERB.parse(example)
39+
rescue SyntaxTree::Parser::ParseError => error
40+
assert_equal(1, error.lineno)
41+
assert_equal(0, error.column)
42+
assert_match(
43+
/No matching <% end %> for the <% do %>-statement/,
44+
error.message
45+
)
2446
end
2547

2648
def test_missing_erb_case_end_tag
27-
assert_raises(SyntaxTree::Parser::ParseError) do
28-
ERB.parse("<% case variabel %>\n<% when 1>\n Hello\n")
29-
end
49+
example = <<-HTML
50+
<% case variabel %>
51+
<% when 1 %>
52+
Hello
53+
<% when 2 %>
54+
World
55+
<h1>What</h1>
56+
HTML
57+
ERB.parse(example)
58+
rescue SyntaxTree::Parser::ParseError => error
59+
assert_equal(4, error.lineno)
60+
assert_equal(0, error.column)
61+
assert_match(
62+
/No matching ERB-tag for the <% when %>-statement/,
63+
error.message
64+
)
3065
end
3166

3267
def test_erb_code_with_non_ascii
@@ -35,19 +70,42 @@ def test_erb_code_with_non_ascii
3570
assert_instance_of(SyntaxTree::ERB::ErbNode, parsed.elements.first)
3671
end
3772

38-
def test_erb_errors
73+
def test_erb_syntax_error
3974
example = <<-HTML
40-
<ul>
41-
<% if @items.each do |i|%>
42-
<li><%= i %></li>
43-
<% end.blank? %>
44-
<li>No items</li>
45-
<% end %>
46-
</ul>
47-
HTML
75+
<ul>
76+
<% if @items.each do |i|%>
77+
<li><%= i %></li>
78+
<% end.blank? %>
79+
<li>No items</li>
80+
<% end %>
81+
</ul>
82+
HTML
4883
ERB.parse(example)
4984
rescue SyntaxTree::Parser::ParseError => error
50-
assert_equal(2, error.lineno)
85+
assert_equal(4, error.lineno)
86+
assert_equal(0, error.column)
87+
assert_match(/Could not parse ERB-tag/, error.message)
88+
end
89+
90+
def test_erb_syntax_error2
91+
example = <<-HTML
92+
<%= content_tag :header do %>
93+
<div class="flex-1 min-w-0">
94+
<h2>
95+
<%= yield :page_header %>
96+
</h2>
97+
<%= content_tag :div do %>
98+
<%= yield :page_subheader %>
99+
<% end if content_for?(:page_subheader) %>
100+
</div>
101+
<%= content_tag :div do %>
102+
<%= yield :page_actions %>
103+
<% end if content_for?(:page_actions) %>
104+
<% end if content_for?(:page_header) %>
105+
HTML
106+
ERB.parse(example)
107+
rescue SyntaxTree::Parser::ParseError => error
108+
assert_equal(8, error.lineno)
51109
assert_equal(0, error.column)
52110
assert_match(/Could not parse ERB-tag/, error.message)
53111
end

0 commit comments

Comments
 (0)