Merge pull request #1814 from opal/elia/opal-backtrace-for-compilatio…

…n-errors Add opal backtrace for compilation errors
opal · Sep 11, 2018 · b693abd · b693abd
2 parents d77bd3c + 311e10d
commit b693abd
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 49 deletions.
diff --git a/lib/opal/compiler.rb b/lib/opal/compiler.rb
@@ -172,7 +172,7 @@ def initialize(source, options = {})
     def compile
       parse
 
-      @fragments = process(@sexp).flatten
+      @fragments = re_raise_with_location { process(@sexp).flatten }
       @fragments << fragment("\n", nil, s(:newline)) unless @fragments.last.code.end_with?("\n")
 
       @result = @fragments.map(&:code).join('')
@@ -184,11 +184,7 @@ def parse
 
       @parser = Opal::Parser.default_parser
 
-      begin
-        sexp, comments, tokens = @parser.tokenize(@buffer)
-      rescue ::Opal::Error, ::Parser::SyntaxError => error
-        raise ::Opal::SyntaxError.with_opal_backtrace(error, file)
-      end
+      sexp, comments, tokens = re_raise_with_location { @parser.tokenize(@buffer) }
 
       @sexp = s(:top, sexp || s(:nil))
       @comments = ::Parser::Source::Comment.associate_locations(sexp, comments)
@@ -227,7 +223,21 @@ def method_calls
     # method simply appends the filename and curent line number onto
     # the message and raises it.
     def error(msg, line = nil)
-      raise ::Opal::SyntaxError, "#{msg} -- #{file}:#{line}"
+      error = ::Opal::SyntaxError.new(msg)
+      error.location = Opal::OpalBacktraceLocation.new(file, line)
+      raise error
+    end
+
+    def re_raise_with_location
+      yield
+    rescue StandardError, ::Opal::SyntaxError => error
+      opal_location = ::Opal.opal_location_from_error(error)
+      opal_location.path = file
+      opal_location.label ||= @source.lines[opal_location.line.to_i - 1].strip
+      new_error = ::Opal::SyntaxError.new(error.message)
+      new_error.set_backtrace error.backtrace
+      ::Opal.add_opal_location_to_error(opal_location, new_error)
+      raise new_error
     end
 
     # This is called when a parsing/processing warning occurs. This

diff --git a/lib/opal/errors.rb b/lib/opal/errors.rb
@@ -29,46 +29,28 @@ class RewritingError < ParsingError
 
   class SyntaxError < ::SyntaxError
     attr_accessor :location
+  end
 
-    # Not redefining #backtrace because of https://bugs.ruby-lang.org/issues/14693
-    def self.with_opal_backtrace(error, path)
-      new_error = new(error.message)
-      backtrace = error.backtrace.to_a
-      backtrace.unshift OpalBacktraceLocation.new(error, path).to_s
-      new_error.set_backtrace backtrace
-      new_error
-    end
+  def self.opal_location_from_error(error)
+    opal_location = OpalBacktraceLocation.new
+    opal_location.location = error.location if error.respond_to?(:location)
+    opal_location.diagnostic = error.diagnostic if error.respond_to?(:diagnostic)
+    opal_location
+  end
+
+  def self.add_opal_location_to_error(opal_location, error)
+    backtrace = error.backtrace.to_a
+    backtrace.unshift opal_location.to_s
+    error.set_backtrace backtrace
+    error
   end
 
   # Loosely compatible with Thread::Backtrace::Location
   class OpalBacktraceLocation
-    attr_reader :error, :path
-
-    def initialize(error, path)
-      @error = error
-      @path = path
-    end
-
-    def location
-      if error.respond_to? :location
-        error.location
-      elsif error.respond_to?(:diagnostic) && error.diagnostic.respond_to?(:location)
-        error.diagnostic.location
-      end
-    end
+    attr_accessor :path, :lineno, :label
 
-    def lineno
-      location.line if location
-    end
-
-    # Use source code as the label
-    def label
-      case
-      when location.respond_to?(:source_line)
-        location.source_line
-      when location.respond_to?(:expression)
-        location.expression.source_line
-      end
+    def initialize(path = nil, lineno = nil, label = nil)
+      @path, @lineno, @label = path, lineno, label
     end
 
     def to_s
@@ -82,5 +64,22 @@ def to_s
       end
       string
     end
+
+    alias line lineno
+
+    def diagnostic=(diagnostic)
+      return unless diagnostic
+      self.location = diagnostic.location
+    end
+
+    def location=(location)
+      return unless location
+      self.lineno = location.line
+      if location.respond_to?(:source_line)
+        self.label = location.source_line
+      elsif location.respond_to?(:expression)
+        self.label = location.expression.source_line
+      end
+    end
   end
 end
diff --git a/spec/lib/compiler_spec.rb b/spec/lib/compiler_spec.rb
@@ -557,15 +557,39 @@ def expect_number_of_warnings(code)
   end
 
   describe 'a compilation error' do
-    it 'adds the file and line to the backtrace' do
-      error = nil
-      begin
-        compiled('foo.JS[:bar] += :baz', file: 'foobar.js.rb')
-      rescue Opal::SyntaxError => syntax_error
-        error = syntax_error
+    context 'at compile time' do
+      it 'adds the file and line to the backtrace' do
+        error = nil
+        begin
+          compiled('BEGIN {}', file: 'foobar.js.rb')
+        rescue Opal::SyntaxError => syntax_error
+          error = syntax_error
+        end
+
+        expect(error.backtrace[0]).to eq("foobar.js.rb:in `BEGIN {}'")
+        expect(compiler_backtrace(error)[0]).to end_with(":in `error'")
+        expect(compiler_backtrace(error)[-3]).to end_with(":in `block in compile'")
+        expect(compiler_backtrace(error)[-1]).to end_with(":in `compile'")
+        expect(error.backtrace.size).to be > 1
+      end
+    end
+
+    context 'at parse time' do
+      it 'adds the file and line to the backtrace' do
+        error = nil
+        begin
+          parsed('def foo', file: 'foobar.js.rb')
+        rescue Opal::SyntaxError => syntax_error
+          error = syntax_error
+        end
+        expect(error.backtrace[0]).to eq("foobar.js.rb:1:in `def foo'")
+        expect(compiler_backtrace(error)[0]).to end_with(":in `block in parse'")
+        expect(error.backtrace.size).to be > 1
       end
-      expect(error.backtrace[0]).to eq("foobar.js.rb:1:in `foo.JS[:bar] += :baz'")
-      expect(error.backtrace.size).to be > 1
+    end
+
+    def compiler_backtrace(error)
+      error.backtrace.grep(%r{lib/opal/compiler\.rb})
     end
   end
 
@@ -599,6 +623,10 @@ def compiled(*args)
     Opal::Compiler.new(*args).compile
   end
 
+  def parsed(*args)
+    Opal::Compiler.new(*args).parse
+  end
+
   alias compile compiled
 
   def expect_compiled(*args)