From cc8e4b065e3e4e37c90761d30afc8beb7dc2c22f Mon Sep 17 00:00:00 2001
From: ydah <13041216+ydah@users.noreply.github.com>
Date: Tue, 24 Oct 2023 22:02:02 +0900
Subject: [PATCH] Improve an error message for ParseError

---
 lib/lrama/command.rb               |  2 +-
 lib/lrama/parser.rb                | 11 +++++--
 parser.y                           | 11 +++++--
 spec/lrama/context_spec.rb         |  9 +++---
 spec/lrama/counterexamples_spec.rb |  8 ++---
 spec/lrama/output_spec.rb          |  2 +-
 spec/lrama/parser_spec.rb          | 50 +++++++++++++++++-------------
 spec/lrama/states_spec.rb          | 40 +++++++++++++-----------
 8 files changed, 77 insertions(+), 56 deletions(-)

diff --git a/lib/lrama/command.rb b/lib/lrama/command.rb
index 5d8957da..afaecda5 100644
--- a/lib/lrama/command.rb
+++ b/lib/lrama/command.rb
@@ -8,7 +8,7 @@ def run(argv)
       warning = Lrama::Warning.new
       text = options.y.read
       options.y.close if options.y != STDIN
-      grammar = Lrama::Parser.new(text).parse
+      grammar = Lrama::Parser.new(text, options.grammar_file).parse
       states = Lrama::States.new(grammar, warning, trace_state: (options.trace_opts[:automaton] || options.trace_opts[:closure]))
       states.compute
       context = Lrama::Context.new(states)
diff --git a/lib/lrama/parser.rb b/lib/lrama/parser.rb
index 82bd90c3..6edbd93d 100644
--- a/lib/lrama/parser.rb
+++ b/lib/lrama/parser.rb
@@ -672,8 +672,9 @@ class Parser < Racc::Parser
 
 include Lrama::Report::Duration
 
-def initialize(text)
+def initialize(text, path)
   @text = text
+  @path = path
 end
 
 def parse
@@ -696,8 +697,12 @@ def next_token
 end
 
 def on_error(error_token_id, error_value, value_stack)
-  raise ParseError, sprintf("\n%d:%d: parse error on value %s (%s)",
-                            @lexer.line, @lexer.column, error_value.inspect, token_to_str(error_token_id) || '?')
+  source = @text.split("\n")[error_value.line - 1]
+  raise ParseError, <<~ERROR
+    #{@path}:#{@lexer.line}:#{@lexer.column}: parse error on value #{error_value.inspect} (#{token_to_str(error_token_id) || '?'})
+    #{source}
+    #{' ' * @lexer.column}^
+  ERROR
 end
 ...end parser.y/module_eval...
 ##### State transition tables begin ###
diff --git a/parser.y b/parser.y
index 968af9fd..adeb3804 100644
--- a/parser.y
+++ b/parser.y
@@ -387,8 +387,9 @@ end
 
 include Lrama::Report::Duration
 
-def initialize(text)
+def initialize(text, path)
   @text = text
+  @path = path
 end
 
 def parse
@@ -411,6 +412,10 @@ def next_token
 end
 
 def on_error(error_token_id, error_value, value_stack)
-  raise ParseError, sprintf("\n%d:%d: parse error on value %s (%s)",
-                            @lexer.line, @lexer.column, error_value.inspect, token_to_str(error_token_id) || '?')
+  source = @text.split("\n")[error_value.line - 1]
+  raise ParseError, <<~ERROR
+    #{@path}:#{@lexer.line}:#{@lexer.column}: parse error on value #{error_value.inspect} (#{token_to_str(error_token_id) || '?'})
+    #{source}
+    #{' ' * @lexer.column}^
+  ERROR
 end
diff --git a/spec/lrama/context_spec.rb b/spec/lrama/context_spec.rb
index a8ff640d..c78a4ca8 100644
--- a/spec/lrama/context_spec.rb
+++ b/spec/lrama/context_spec.rb
@@ -4,8 +4,9 @@
 
   describe "basic" do
     it do
-      y = File.read(fixture_path("context/basic.y"))
-      grammar = Lrama::Parser.new(y).parse
+      path = "context/basic.y"
+      y = File.read(fixture_path(path))
+      grammar = Lrama::Parser.new(y, path).parse
       states = Lrama::States.new(grammar, warning)
       states.compute
       context = Lrama::Context.new(states)
@@ -181,7 +182,7 @@
 %%
         INPUT
 
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
         context = Lrama::Context.new(states)
@@ -230,7 +231,7 @@
 %%
         INPUT
 
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
         context = Lrama::Context.new(states)
diff --git a/spec/lrama/counterexamples_spec.rb b/spec/lrama/counterexamples_spec.rb
index f0cce980..d1f55ee3 100644
--- a/spec/lrama/counterexamples_spec.rb
+++ b/spec/lrama/counterexamples_spec.rb
@@ -50,7 +50,7 @@
       end
 
       it "build counterexamples of S/R conflicts" do
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
         counterexamples = Lrama::Counterexamples.new(states)
@@ -249,7 +249,7 @@
       end
 
       it "build counterexamples of R/R conflicts" do
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
         counterexamples = Lrama::Counterexamples.new(states)
@@ -326,7 +326,7 @@
       end
 
       it "build counterexamples of S/R conflicts" do
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
         counterexamples = Lrama::Counterexamples.new(states)
@@ -407,7 +407,7 @@
       end
 
       it "build counterexamples of S/R and R/R conflicts" do
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
         counterexamples = Lrama::Counterexamples.new(states)
diff --git a/spec/lrama/output_spec.rb b/spec/lrama/output_spec.rb
index f6a4f30b..f547787c 100644
--- a/spec/lrama/output_spec.rb
+++ b/spec/lrama/output_spec.rb
@@ -17,7 +17,7 @@
   let(:header_out) { StringIO.new }
   let(:warning) { Lrama::Warning.new(StringIO.new) } # suppress warnings
   let(:text) { File.read(grammar_file_path) }
-  let(:grammar) { Lrama::Parser.new(text).parse }
+  let(:grammar) { Lrama::Parser.new(text, grammar_file_path).parse }
   let(:states) { s = Lrama::States.new(grammar, warning); s.compute; s }
   let(:context) { Lrama::Context.new(states) }
   let(:grammar_file_path) { fixture_path("common/basic.y") }
diff --git a/spec/lrama/parser_spec.rb b/spec/lrama/parser_spec.rb
index 634e1402..635533e3 100644
--- a/spec/lrama/parser_spec.rb
+++ b/spec/lrama/parser_spec.rb
@@ -40,8 +40,9 @@
 
   describe '#parse' do
     it "basic" do
-      y = File.read(fixture_path("common/basic.y"))
-      grammar = Lrama::Parser.new(y).parse
+      path = "common/basic.y"
+      y = File.read(fixture_path(path))
+      grammar = Lrama::Parser.new(y, path).parse
 
       expect(grammar.union.code.s_value).to eq(<<-CODE.chomp)
 
@@ -407,8 +408,9 @@
     end
 
     it "nullable" do
-      y = File.read(fixture_path("common/nullable.y"))
-      grammar = Lrama::Parser.new(y).parse
+      path = "common/nullable.y"
+      y = File.read(fixture_path(path))
+      grammar = Lrama::Parser.new(y, path).parse
 
       expect(grammar.nterms.sort_by(&:number)).to eq([
         Sym.new(id: T.new(type: T::Ident, s_value: "$accept"),       alias_name: nil, number:  6, tag: nil, term: false, token_id: 0, nullable: false),
@@ -562,7 +564,7 @@ class : keyword_class tSTRING keyword_end { code 1 }
 %%
       INPUT
 
-      grammar = Lrama::Parser.new(y).parse
+      grammar = Lrama::Parser.new(y, "parse.y").parse
 
       expect(grammar._rules).to eq([
         [
@@ -605,7 +607,7 @@ class : keyword_class tSTRING keyword_end { code 1 }
 %%
 
       INPUT
-      grammar = Lrama::Parser.new(y).parse
+      grammar = Lrama::Parser.new(y, "parse.y").parse
 
       expect(grammar.terms.sort_by(&:number)).to eq([
         Sym.new(id: T.new(type: T::Ident, s_value: "EOI"),           alias_name: "\"EOI\"",           number:  0, tag: nil,                                   term: true, token_id:   0, nullable: false, precedence: nil),
@@ -661,7 +663,7 @@ class : keyword_class { code 1 } tSTRING { code 2 } keyword_end { code 3 }
 %%
 
       INPUT
-      grammar = Lrama::Parser.new(y).parse
+      grammar = Lrama::Parser.new(y, "parse.y").parse
 
       expect(grammar.nterms.sort_by(&:number)).to eq([
         Sym.new(id: T.new(type: T::Ident, s_value: "$accept"), alias_name: nil, number: 11, tag: nil,                                 term: false, token_id: 0, nullable: false),
@@ -756,7 +758,7 @@ class : keyword_class tSTRING %prec tPLUS keyword_end { code 1 }
 %%
 
         INPUT
-        parser = Lrama::Parser.new(y)
+        parser = Lrama::Parser.new(y, "parse.y")
 
         expect { parser.parse }.to raise_error("Ident after %prec")
       end
@@ -773,7 +775,7 @@ class : keyword_class { code 2 } tSTRING %prec "=" '!' keyword_end { code 3 }
 %%
 
         INPUT
-        parser = Lrama::Parser.new(y)
+        parser = Lrama::Parser.new(y, "parse.y")
 
         expect { parser.parse }.to raise_error("Char after %prec")
       end
@@ -790,7 +792,7 @@ class : keyword_class { code 4 } tSTRING '?' keyword_end %prec tEQ { code 5 } {
 %%
 
         INPUT
-        parser = Lrama::Parser.new(y)
+        parser = Lrama::Parser.new(y, "parse.y")
 
         expect { parser.parse }.to raise_error("Multiple User_code after %prec")
       end
@@ -811,7 +813,7 @@ class : keyword_class
 
 %%
         INPUT
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
         codes = grammar.rules.map(&:code).compact
 
         expect(codes.count).to eq(1)
@@ -838,7 +840,7 @@ class : keyword_class
 
 %%
         INPUT
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
         codes = grammar.rules.map(&:code).compact
 
         expect(codes.count).to eq(1)
@@ -883,7 +885,7 @@ class : keyword_class tSTRING keyword_end { code 1 }
 %%
 
         INPUT
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
 
         expect(grammar.terms.sort_by(&:number)).to eq([
           Sym.new(id: T.new(type: T::Ident, s_value: "EOI"),           alias_name: "\"EOI\"",           number:  0, tag: nil,                                   term: true, token_id:   0, nullable: false),
@@ -932,7 +934,7 @@ class : keyword_class tSTRING keyword_end { code 1 }
 %%
 
         INPUT
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
 
         expect(grammar.terms.sort_by(&:number)).to eq([
           Sym.new(id: T.new(type: T::Ident, s_value: "EOI"),           alias_name: "\"EOI\"",           number: 0, tag: nil,                                   term: true, token_id:   0, nullable: false, precedence: nil),
@@ -978,7 +980,7 @@ class : keyword_class tSTRING keyword_end { code 1 }
         ;
 %%
           INPUT
-          grammar = Lrama::Parser.new(y).parse
+          grammar = Lrama::Parser.new(y, "parse.y").parse
 
           expect(grammar.rules).to eq([
             Rule.new(
@@ -1086,7 +1088,7 @@ class : keyword_class tSTRING keyword_end { code 1 }
    ;
 %%
           INPUT
-          grammar = Lrama::Parser.new(y).parse
+          grammar = Lrama::Parser.new(y, "parse.y").parse
 
           expect(grammar.rules).to eq([
             Rule.new(
@@ -1177,7 +1179,7 @@ class : keyword_class tSTRING keyword_end { code 1 }
                 { $$ = $1 - $2; }
 ;
             INPUT
-            grammar = Lrama::Parser.new(y).parse
+            grammar = Lrama::Parser.new(y, "parse.y").parse
 
             expect(grammar.rules).to eq([
               Rule.new(
@@ -1311,7 +1313,7 @@ class : keyword_class tSTRING keyword_end { code 1 }
 ;
             INPUT
 
-            expect { Lrama::Parser.new(y).parse }.to raise_error("'results' is invalid name.")
+            expect { Lrama::Parser.new(y, "parse.y").parse }.to raise_error("'results' is invalid name.")
           end
         end
       end
@@ -1332,7 +1334,11 @@ class : keyword_class tSTRING keyword_end { code 1 }
        ;
         INPUT
 
-        expect { Lrama::Parser.new(y).parse }.to raise_error(/5:14: parse error/)
+        expect { Lrama::Parser.new(y, "error_messages/parse.y").parse }.to raise_error(<<~ERROR)
+          error_messages/parse.y:5:14: parse error on value #<struct Lrama::Lexer::Token type=#<struct Lrama::Lexer::Token::Type id=19, name="Ident">, s_value="invalid", alias=nil> (IDENTIFIER)
+          %expect invalid
+                        ^
+        ERROR
       end
     end
   end
@@ -1367,7 +1373,7 @@ class : keyword_class tSTRING keyword_end ;
 
 %%
       INPUT
-      grammar = Lrama::Parser.new(y).parse
+      grammar = Lrama::Parser.new(y, "parse.y").parse
       terms = grammar.terms.sort_by(&:number).map do |term|
         [term.id.s_value, term.token_id]
       end
@@ -1416,7 +1422,7 @@ class : keyword_class tSTRING keyword_end
 
 %%
         INPUT
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "parse.y").parse
         codes = grammar.rules.map(&:code)
 
         expect(codes.count).to eq(3)
@@ -1463,7 +1469,7 @@ class : keyword_class tSTRING keyword_end
 %%
         INPUT
 
-        expect { Lrama::Parser.new(y).parse }.to raise_error(RuntimeError) do |e|
+        expect { Lrama::Parser.new(y, "parse.y").parse }.to raise_error(RuntimeError) do |e|
           expect(e.message).to eq(<<~MSG.chomp)
             $$ of 'stmt' has no declared type
             $1 of 'stmt' has no declared type
diff --git a/spec/lrama/states_spec.rb b/spec/lrama/states_spec.rb
index e46c3759..07b0cf77 100644
--- a/spec/lrama/states_spec.rb
+++ b/spec/lrama/states_spec.rb
@@ -4,8 +4,9 @@
 
   describe '#compute' do
     it "basic" do
-      y = File.read(fixture_path("common/basic.y"))
-      grammar = Lrama::Parser.new(y).parse
+      path = "common/basic.y"
+      y = File.read(fixture_path(path))
+      grammar = Lrama::Parser.new(y, path).parse
       states = Lrama::States.new(grammar, warning)
       states.compute
 
@@ -297,8 +298,9 @@ class    go to state 5
     end
 
     it '#State#accessing_symbol' do
-      y = File.read(fixture_path("common/basic.y"))
-      grammar = Lrama::Parser.new(y).parse
+      path = "common/basic.y"
+      y = File.read(fixture_path(path))
+      grammar = Lrama::Parser.new(y, path).parse
       states = Lrama::States.new(grammar, warning)
       states.compute
 
@@ -337,8 +339,9 @@ class    go to state 5
 
   describe '#reads_relation' do
     it do
-      y = File.read(fixture_path("states/reads_relation.y"))
-      grammar = Lrama::Parser.new(y).parse
+      path = "states/reads_relation.y"
+      y = File.read(fixture_path(path))
+      grammar = Lrama::Parser.new(y, path).parse
       states = Lrama::States.new(grammar, warning)
       states.compute
 
@@ -604,8 +607,9 @@ class    go to state 5
 
   describe '#includes_relation' do
     it do
-      y = File.read(fixture_path("states/includes_relation.y"))
-      grammar = Lrama::Parser.new(y).parse
+      path = "states/includes_relation.y"
+      y = File.read(fixture_path(path))
+      grammar = Lrama::Parser.new(y, path).parse
       states = Lrama::States.new(grammar, warning)
       states.compute
 
@@ -904,7 +908,7 @@ class    go to state 5
 
 %%
         INPUT
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "states/compute_look_ahead_sets.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
 
@@ -973,7 +977,7 @@ class    go to state 5
 
 %%
         INPUT
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "states/compute_look_ahead_sets.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
 
@@ -1063,7 +1067,7 @@ class    go to state 5
 
 %%
         INPUT
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "states/compute_conflicts.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
 
@@ -1180,7 +1184,7 @@ class    go to state 5
 
 %%
         INPUT
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "states/compute_conflicts.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
 
@@ -1277,7 +1281,7 @@ class    go to state 5
 
 %%
         INPUT
-        grammar = Lrama::Parser.new(y).parse
+        grammar = Lrama::Parser.new(y, "states/compute_conflicts.y").parse
         states = Lrama::States.new(grammar, warning)
         states.compute
 
@@ -1426,7 +1430,7 @@ class    go to state 5
 
 %%
       INPUT
-      grammar = Lrama::Parser.new(y).parse
+      grammar = Lrama::Parser.new(y, "states/compute_default_reduction.y").parse
       states = Lrama::States.new(grammar, warning)
       states.compute
 
@@ -1598,7 +1602,7 @@ class    go to state 5
 
 %%
       INPUT
-      grammar = Lrama::Parser.new(y).parse
+      grammar = Lrama::Parser.new(y, "states/compute_default_reduction.y").parse
       states = Lrama::States.new(grammar, warning)
       states.compute
 
@@ -1816,7 +1820,7 @@ class : keyword_class tSTRING keyword_end %prec tPLUS
           end
 
           it "has errors for r/r conflicts" do
-            grammar = Lrama::Parser.new(header + y).parse
+            grammar = Lrama::Parser.new(header + y, "states/check_conflicts.y").parse
             states = Lrama::States.new(grammar, warning)
             states.compute
 
@@ -1840,7 +1844,7 @@ class : keyword_class tSTRING keyword_end %prec tPLUS
           end
 
           it "has errors for s/r conflicts and r/r conflicts" do
-            grammar = Lrama::Parser.new(header + y).parse
+            grammar = Lrama::Parser.new(header + y, "states/check_conflicts.y").parse
             states = Lrama::States.new(grammar, warning)
             states.compute
 
@@ -1864,7 +1868,7 @@ class : keyword_class tSTRING keyword_end %prec tPLUS
         end
 
         it "has warns for s/r conflicts and r/r conflicts" do
-          grammar = Lrama::Parser.new(header + y).parse
+          grammar = Lrama::Parser.new(header + y, "states/check_conflicts.y").parse
           states = Lrama::States.new(grammar, warning)
           states.compute