Merge pull request #3480 from bundler/seg-dsl-error

[DSL] Add prettier DSL error

4 files changed, 113 insertions, 17 deletions

diff --git a/lib/bundler/dsl.rb b/lib/bundler/dsl.rb
index e6c4b975ef..f9f09e8b00 100644
--- a/lib/bundler/dsl.rb
+++ b/lib/bundler/dsl.rb
@@ -30,14 +30,9 @@ module Bundler
     def eval_gemfile(gemfile, contents = nil)
       contents ||= Bundler.read_file(gemfile.to_s)
       instance_eval(contents, gemfile.to_s, 1)
-    rescue SyntaxError => e
-      syntax_msg = e.message.gsub("#{gemfile}:", 'on line ')
-      raise GemfileError, "Gemfile syntax error #{syntax_msg}"
-    rescue ScriptError, RegexpError, NameError, ArgumentError, RuntimeError => e
-      e.backtrace[0] = "#{e.backtrace[0]}: #{e.message} (#{e.class})"
-      Bundler.ui.warn e.backtrace.join("\n       ")
-      raise GemfileError, "There was an error in your Gemfile," \
-        " and Bundler cannot continue."
+    rescue Exception => e
+      message = "There was an error parsing `#{File.basename gemfile.to_s}`: #{e.message}"
+      raise DSLError.new(message, gemfile, e.backtrace, contents)
     end
 
     def gemspec(opts = nil)
@@ -184,9 +179,7 @@ module Bundler
     end
 
     def method_missing(name, *args)
-      location = caller[0].split(':')[0..1].join(':')
-      raise GemfileError, "Undefined local variable or method `#{name}' for Gemfile\n" \
-        "        from #{location}"
+      raise GemfileError, "Undefined local variable or method `#{name}' for Gemfile"
     end
 
   private
@@ -327,5 +320,106 @@ module Bundler
       end
     end
 
+    class DSLError < GemfileError
+      # @return [String] the description that should be presented to the user.
+      #
+      attr_reader :description
+
+      # @return [String] the path of the dsl file that raised the exception.
+      #
+      attr_reader :dsl_path
+
+      # @return [Exception] the backtrace of the exception raised by the
+      #         evaluation of the dsl file.
+      #
+      attr_reader :backtrace
+
+      # @param [Exception] backtrace @see backtrace
+      # @param [String]    dsl_path  @see dsl_path
+      #
+      def initialize(description, dsl_path, backtrace, contents = nil)
+        @status_code = $!.respond_to?(:status_code) && $!.status_code
+
+        @description = description
+        @dsl_path    = dsl_path
+        @backtrace   = backtrace
+        @contents    = contents
+      end
+
+      def status_code
+        @status_code || super
+      end
+
+      # @return [String] the contents of the DSL that cause the exception to
+      #         be raised.
+      #
+      def contents
+        @contents ||= begin
+          dsl_path && File.exist?(dsl_path) && File.read(dsl_path)
+        end
+      end
+
+      # The message of the exception reports the content of podspec for the
+      # line that generated the original exception.
+      #
+      # @example Output
+      #
+      #   Invalid podspec at `RestKit.podspec` - undefined method
+      #   `exclude_header_search_paths=' for #<Pod::Specification for
+      #   `RestKit/Network (0.9.3)`>
+      #
+      #       from spec-repos/master/RestKit/0.9.3/RestKit.podspec:36
+      #       -------------------------------------------
+      #           # because it would break: #import <CoreData/CoreData.h>
+      #    >      ns.exclude_header_search_paths = 'Code/RestKit.h'
+      #         end
+      #       -------------------------------------------
+      #
+      # @return [String] the message of the exception.
+      #
+      def message
+        @message ||= begin
+          trace_line, description = parse_line_number_from_description
+
+          m = "\n[!] "
+          m << description
+          m << ". Bundler cannot continue.\n"
+
+          return m unless backtrace && dsl_path && contents
+
+          trace_line = backtrace.find { |l| l.include?(dsl_path.to_s) } || trace_line
+          return m unless trace_line
+          line_numer = trace_line.split(':')[1].to_i - 1
+          return m unless line_numer
+
+          lines      = contents.lines.to_a
+          indent     = ' #  '
+          indicator  = indent.gsub('#', '>')
+          first_line = (line_numer.zero?)
+          last_line  = (line_numer == (lines.count - 1))
+
+          m << "\n"
+          m << "#{indent}from #{trace_line.gsub(/:in.*$/, '')}\n"
+          m << "#{indent}-------------------------------------------\n"
+          m << "#{indent}#{    lines[line_numer - 1] }" unless first_line
+          m << "#{indicator}#{ lines[line_numer] }"
+          m << "#{indent}#{    lines[line_numer + 1] }" unless last_line
+          m << "\n" unless m.end_with?("\n")
+          m << "#{indent}-------------------------------------------\n"
+        end
+      end
+
+      private
+
+      def parse_line_number_from_description
+        description = self.description
+        if dsl_path && description =~ /((#{Regexp.quote File.expand_path(dsl_path)}|#{Regexp.quote dsl_path.to_s}):\d+)/
+          trace_line = Regexp.last_match[1]
+          description = description.sub(/#{Regexp.quote trace_line}:\s*/, '').sub("\n", ' - ')
+        end
+        [trace_line, description]
+      end
+    end
+
   end
 end
diff --git a/lib/bundler/friendly_errors.rb b/lib/bundler/friendly_errors.rb
index 1431725d3d..4bd3e53381 100644
--- a/lib/bundler/friendly_errors.rb
+++ b/lib/bundler/friendly_errors.rb
@@ -5,6 +5,9 @@ require "bundler/vendored_thor"
 module Bundler
   def self.with_friendly_errors
     yield
+  rescue Bundler::Dsl::DSLError => e
+    Bundler.ui.error e.message
+    exit e.status_code
   rescue Bundler::BundlerError => e
     Bundler.ui.error e.message, :wrap => true
     Bundler.ui.trace e
diff --git a/spec/bundler/dsl_spec.rb b/spec/bundler/dsl_spec.rb
index c2dacedd08..5c6be9efca 100644
--- a/spec/bundler/dsl_spec.rb
+++ b/spec/bundler/dsl_spec.rb
@@ -69,8 +69,7 @@ describe Bundler::Dsl do
       expect(Bundler).to receive(:read_file).with("Gemfile").
         and_return("unknown")
 
-      error_msg = "Undefined local variable or method `unknown'" \
-        " for Gemfile\\s+from Gemfile:1"
+      error_msg = "There was an error parsing `Gemfile`: Undefined local variable or method `unknown' for Gemfile. Bundler cannot continue."
       expect { subject.eval_gemfile("Gemfile") }.
         to raise_error(Bundler::GemfileError, Regexp.new(error_msg))
     end
@@ -80,7 +79,7 @@ describe Bundler::Dsl do
     it "handles syntax errors with a useful message" do
       expect(Bundler).to receive(:read_file).with("Gemfile").and_return("}")
       expect { subject.eval_gemfile("Gemfile") }.
-        to raise_error(Bundler::GemfileError, /Gemfile syntax error/)
+        to raise_error(Bundler::GemfileError, /There was an error parsing `Gemfile`: (syntax error, unexpected tSTRING_DEND|(compile error - )?syntax error, unexpected '}'). Bundler cannot continue./)
     end
   end
 
@@ -177,7 +176,7 @@ describe Bundler::Dsl do
     it "will raise a Bundler::GemfileError" do
       gemfile "gem 'foo', :path => /unquoted/string/syntax/error"
       expect { Bundler::Dsl.evaluate(bundled_app("Gemfile"), nil, true) }.
-        to raise_error(Bundler::GemfileError, /Gemfile syntax error/)
+        to raise_error(Bundler::GemfileError, /There was an error parsing `Gemfile`:( compile error -)? unknown regexp options - trg. Bundler cannot continue./)
     end
   end
 
@@ -185,7 +184,7 @@ describe Bundler::Dsl do
     it "will raise a Bundler::GemfileError" do
       gemfile "s = 'foo'.freeze; s.strip!"
       expect { Bundler::Dsl.evaluate(bundled_app("Gemfile"), nil, true) }.
-        to raise_error(Bundler::GemfileError, /There was an error in your Gemfile/)
+        to raise_error(Bundler::GemfileError, /There was an error parsing `Gemfile`: can't modify frozen String. Bundler cannot continue./)
     end
   end
 end
diff --git a/spec/install/gems/simple_case_spec.rb b/spec/install/gems/simple_case_spec.rb
index 759de0884f..0f05588c4d 100644
--- a/spec/install/gems/simple_case_spec.rb
+++ b/spec/install/gems/simple_case_spec.rb
@@ -17,7 +17,7 @@ describe "bundle install with gem sources" do
       G
 
       expect(err).to eq ""
-      expect(out).to match(/StandardError: FAIL/)
+      expect(out).to match(/StandardError, "FAIL"/)
       expect(bundled_app("Gemfile.lock")).not_to exist
     end