From 858d83c95eabc6b8d85cb8978b64c52fab224240 Mon Sep 17 00:00:00 2001 From: David Wessman Date: Mon, 29 Apr 2024 07:36:06 +0200 Subject: [PATCH] Adds handling for column index in error reporting (#85) --- CHANGELOG.md | 2 + README.md | 14 +++- lib/syntax_tree/erb/format.rb | 3 +- lib/syntax_tree/erb/nodes.rb | 53 +++--------- lib/syntax_tree/erb/parser.rb | 152 ++++++++++++++++++++-------------- test/erb_test.rb | 65 ++++++++++++--- test/html_test.rb | 24 +++--- 7 files changed, 179 insertions(+), 134 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd356c5..1602314 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a ## [Unreleased] - Support Ruby 3.3 by handling yield in ERB specifically +- Use SyntaxTree own class for ParseError to get better error feedback +- Adds handling for keeping track of column index, to support better error messages ## [0.11.0] - 2024-04-23 diff --git a/README.md b/README.md index 420075d..ad25a2f 100644 --- a/README.md +++ b/README.md @@ -30,18 +30,26 @@ Currently handles Add this line to your application's Gemfile: ```ruby -gem "w_syntax_tree-erb", "~> 0.10", require: false +gem "w_syntax_tree-erb", "~> 0.11", require: false ``` > I added the `w_` prefix to avoid conflicts if there will ever be an official `syntax_tree-erb` gem. ## Usage +### Parsing + +```sh +bundle exec stree ast --plugins=erb "./**/*.html.erb" +``` + +### Format + ```sh -bundle exec stree --plugins=erb "./**/*.html.erb" +bundle exec stree write --plugins=erb "./**/*.html.erb" ``` -From code: +### In code ```ruby require "syntax_tree/erb" diff --git a/lib/syntax_tree/erb/format.rb b/lib/syntax_tree/erb/format.rb index 56f27e1..66dd397 100644 --- a/lib/syntax_tree/erb/format.rb +++ b/lib/syntax_tree/erb/format.rb @@ -20,7 +20,8 @@ def visit_token(node) # Visit a Document node. def visit_document(node) - child_nodes = node.child_nodes.sort_by(&:location) + child_nodes = + node.child_nodes.sort_by { |node| node.location.start_char } handle_child_nodes(child_nodes) diff --git a/lib/syntax_tree/erb/nodes.rb b/lib/syntax_tree/erb/nodes.rb index 9806814..570f0ec 100644 --- a/lib/syntax_tree/erb/nodes.rb +++ b/lib/syntax_tree/erb/nodes.rb @@ -2,48 +2,6 @@ module SyntaxTree module ERB - # A Location represents a position for a node in the source file. - class Location - attr_reader :start_char, :end_char, :start_line, :end_line - - def initialize(start_char:, end_char:, start_line:, end_line:) - @start_char = start_char - @end_char = end_char - @start_line = start_line - @end_line = end_line - end - - def deconstruct_keys(keys) - { - start_char: start_char, - end_char: end_char, - start_line: start_line, - end_line: end_line - } - end - - def to(other) - Location.new( - start_char: start_char, - start_line: start_line, - end_char: other.end_char, - end_line: other.end_line - ) - end - - def <=>(other) - start_char <=> other.start_char - end - - def to_s - if start_line == end_line - "line #{start_line}, char #{start_char}..#{end_char}" - else - "line #{start_line},char #{start_char} to line #{end_line}, char #{end_char}" - end - end - end - # A parent node that contains a bit of shared functionality. class Node def format(q) @@ -322,6 +280,7 @@ def initialize( ) end + @content = content @content = prepare_content(content) @closing_tag = closing_tag end @@ -376,11 +335,19 @@ def prepare_content(content) result rescue SyntaxTree::Parser::ParseError => error + opening_location = opening_tag.location + content_location = content.first&.location || opening_location raise( SyntaxTree::Parser::ParseError.new( "Could not parse ERB-tag: #{error.message}", @opening_tag.location.start_line + error.lineno - 1, - error.column + ( + if opening_location.start_line == error.lineno + opening_location.start_column + error.column - 1 + else + content_location.start_column + error.column - 1 + end + ) ) ) end diff --git a/lib/syntax_tree/erb/parser.rb b/lib/syntax_tree/erb/parser.rb index 55fb961..eca23bf 100644 --- a/lib/syntax_tree/erb/parser.rb +++ b/lib/syntax_tree/erb/parser.rb @@ -54,7 +54,7 @@ def parse_any_tag SyntaxTree::Parser::ParseError.new( "Duplicate doctype declaration", tag.location.start_line, - 0 + tag.location.start_column ) ) else @@ -73,6 +73,7 @@ def parse_any_tag def make_tokens Enumerator.new do |enum| index = 0 + column_index = 0 line = 1 state = %i[outside] @@ -82,56 +83,56 @@ def make_tokens case source[index..] when /\A\n{2,}/ # two or more newlines should be ONE blank line - enum.yield :blank_line, $&, index, line + enum.yield(:blank_line, $&, index, line, column_index) line += $&.count("\n") when /\A\n/ # newlines - enum.yield :new_line, $&, index, line + enum.yield(:new_line, $&, index, line, column_index) line += 1 when /\A/m # comments # - enum.yield :html_comment, $&, index, line + enum.yield(:html_comment, $&, index, line, column_index) line += $&.count("\n") when /\A/ # An ERB-comment # <%# this is an ERB comment %> - enum.yield :erb_comment, $&, index, line + enum.yield(:erb_comment, $&, index, line, column_index) when /\A<%={1,2}/, /\A<%-/, /\A<%/ # the beginning of an ERB tag # <% # <%=, <%== - enum.yield :erb_open, $&, index, line + enum.yield(:erb_open, $&, index, line, column_index) state << :erb_start line += $&.count("\n") when %r{\A/ - enum.yield :erb_do_close, $&, index, line + enum.yield(:erb_do_close, $&, index, line, column_index) state.pop when /\A-?%>/ - enum.yield :erb_close, $&, index, line + enum.yield(:erb_close, $&, index, line, column_index) state.pop when /\Ayield\b/ - enum.yield :erb_yield, $&, index, line + enum.yield(:erb_yield, $&, index, line, column_index) when /\A[\p{L}\w]*\b/ # Split by word boundary while parsing the code # This allows us to separate what_to_do vs do - enum.yield :erb_code, $&, index, line + enum.yield(:erb_code, $&, index, line, column_index) else - enum.yield :erb_code, source[index], index, line + enum.yield(:erb_code, source[index], index, line, column_index) index += 1 + column_index += 1 next end in :string_single_quote case source[index..] when /\A(?: |\t|\n|\r\n)+/m - # whitespace - enum.yield :whitespace, $&, index, line + enum.yield(:whitespace, $&, index, line, column_index) line += $&.count("\n") when /\A\'/ # the end of a quoted string - enum.yield :string_close_single_quote, $&, index, line + enum.yield( + :string_close_single_quote, + $&, + index, + line, + column_index + ) state.pop when /\A<%[=]?/ # the beginning of an ERB tag # <% - enum.yield :erb_open, $&, index, line + enum.yield(:erb_open, $&, index, line, column_index) state << :erb_start when /\A[^<']+/ # plain text content # abc - enum.yield :text, $&, index, line + enum.yield(:text, $&, index, line, column_index) else raise( SyntaxTree::Parser::ParseError.new( "Unexpected character, #{source[index]}, when looking for closing single quote", line, - 0 + column_index ) ) end in :string_double_quote case source[index..] when /\A(?: |\t|\n|\r\n)+/m - # whitespace - enum.yield :whitespace, $&, index, line + enum.yield(:whitespace, $&, index, line, column_index) line += $&.count("\n") when /\A\"/ - # the end of a quoted string - enum.yield :string_close_double_quote, $&, index, line + enum.yield( + :string_close_double_quote, + $&, + index, + line, + column_index + ) state.pop when /\A<%[=]?/ # the beginning of an ERB tag # <% - enum.yield :erb_open, $&, index, line + enum.yield(:erb_open, $&, index, line, column_index) state << :erb_start when /\A[^<"]+/ # plain text content # abc - enum.yield :text, $&, index, line + enum.yield(:text, $&, index, line, column_index) else raise( SyntaxTree::Parser::ParseError.new( "Unexpected character, #{source[index]}, when looking for closing double quote", line, - 0 + column_index ) ) end @@ -260,63 +271,76 @@ def make_tokens when /\A-?%>/ # the end of an ERB tag # -%> or %> - enum.yield :erb_close, $&, index, line + enum.yield(:erb_close, $&, index, line, column_index) state.pop when /\A>/ # the end of a tag # > - enum.yield :close, $&, index, line + enum.yield(:close, $&, index, line, column_index) state.pop when /\A\?>/ # the end of a tag # ?> - enum.yield :special_close, $&, index, line + enum.yield(:special_close, $&, index, line, column_index) state.pop when %r{\A/>} # the end of a self-closing tag - enum.yield :slash_close, $&, index, line + enum.yield(:slash_close, $&, index, line, column_index) state.pop when %r{\A/} # a forward slash # / - enum.yield :slash, $&, index, line + enum.yield :slash, $&, index, line, column_index when /\A=/ # an equals sign # = - enum.yield :equals, $&, index, line + enum.yield :equals, $&, index, line, column_index when /\A[@#]*[:\w\.\-\_]+\b/ # a name for an element or an attribute # strong, vue-component-kebab, VueComponentPascal # abc, #abc, @abc, :abc - enum.yield :name, $&, index, line + enum.yield :name, $&, index, line, column_index when /\A<%/ # the beginning of an ERB tag # <% - enum.yield :erb_open, $&, index, line + enum.yield :erb_open, $&, index, line, column_index state << :erb_start when /\A"/ # the beginning of a string - enum.yield :string_open_double_quote, $&, index, line + enum.yield( + :string_open_double_quote, + $&, + index, + line, + column_index + ) state << :string_double_quote when /\A'/ # the beginning of a string - enum.yield :string_open_single_quote, $&, index, line + enum.yield( + :string_open_single_quote, + $&, + index, + line, + column_index + ) state << :string_single_quote else raise( SyntaxTree::Parser::ParseError.new( "Unexpected character, #{source[index]}, when parsing HTML- or ERB-tag", line, - 0 + column_index ) ) end end index += $&.length + column_index = $&.rindex("\n") || column_index + $&.length end - enum.yield :EOF, nil, index, line + enum.yield(:EOF, nil, index, line, column_index) end end @@ -325,7 +349,7 @@ def make_tokens # return the new Token. Otherwise we're going to raise a # MissingTokenError. def consume(expected) - type, value, index, line = tokens.peek + type, value, index, line, column = tokens.peek if expected != type raise( @@ -339,6 +363,8 @@ def consume(expected) tokens.next + rindex = value.rindex("\n") + Token.new( type: type, value: value, @@ -347,7 +373,9 @@ def consume(expected) start_char: index, end_char: index + value.length, start_line: line, - end_line: line + value.count("\n") + end_line: line + value.count("\n"), + start_column: column, + end_column: rindex ? value.length - rindex : column + value.length ) ) end @@ -408,7 +436,7 @@ def parse_html_opening_tag SyntaxTree::Parser::ParseError.new( "Invalid HTML-tag name #{name.value}", name.location.start_line, - 0 + name.location.start_column ) ) end @@ -472,7 +500,7 @@ def parse_html_element SyntaxTree::Parser::ParseError.new( "Missing closing tag for <#{opening.name.value}>", opening.location.start_line, - 0 + opening.location.start_column ) ) end @@ -482,7 +510,7 @@ def parse_html_element SyntaxTree::Parser::ParseError.new( "Expected closing tag for <#{opening.name.value}> but got <#{closing.name.value}>", closing.location.start_line, - 0 + closing.location.start_column ) ) end @@ -510,7 +538,7 @@ def parse_erb_case(erb_node) SyntaxTree::Parser::ParseError.new( "No matching ERB-tag for the <% #{erb_node.keyword.value} %>", location.start_line, - 0 + location.start_column ) ) end @@ -535,7 +563,7 @@ def parse_erb_case(erb_node) SyntaxTree::Parser::ParseError.new( "No matching when- or else-tag for the case-tag", erb_node.location.start_line, - 0 + erb_node.location.start_column ) ) end @@ -555,7 +583,7 @@ def parse_erb_if(erb_node) SyntaxTree::Parser::ParseError.new( "No matching ERB-tag for the <% if %>", erb_node.location.start_line, - 0 + erb_node.location.start_column ) ) end @@ -587,7 +615,7 @@ def parse_erb_if(erb_node) SyntaxTree::Parser::ParseError.new( "No matching <% elsif %> or <% else %> for the <% if %>", erb_node.location.start_line, - 0 + erb_node.location.start_column ) ) end @@ -603,7 +631,7 @@ def parse_erb_else(erb_node) SyntaxTree::Parser::ParseError.new( "No matching <% end %> for the <% else %>", erb_node.location.start_line, - 0 + erb_node.location.start_column ) ) end @@ -645,7 +673,7 @@ def parse_erb_tag SyntaxTree::Parser::ParseError.new( "No matching closing tag for the <% #{keyword.value} %>", closing_tag.location.start_line, - 0 + closing_tag.location.start_column ) ) end @@ -681,7 +709,7 @@ def parse_erb_tag SyntaxTree::Parser::ParseError.new( "No matching <% end %> for the <% do %>", erb_node.location.start_line, - 0 + erb_node.location.start_column ) ) end diff --git a/test/erb_test.rb b/test/erb_test.rb index 08e87a4..9b11021 100644 --- a/test/erb_test.rb +++ b/test/erb_test.rb @@ -12,7 +12,7 @@ def test_empty_file end def test_missing_erb_end_tag - example = <<-HTML + example = <<~HTML