Make object path resolution more robust (fix #957)fix-957

This fixes #957 and should make object path resolution more predictable
in general. Instead of splitting the path on "/" before doing any
parsing, we use `StringScanner` and `complete_expression?` to scan
through the string looking for complete slash-delimited Ruby
expressions.

It also turned out that separating the code for handling "-" from the
path-resolution code simplified things a lot. It doesn't really make
sense for "-" to be in there anyway, since paths like "foo/-/bar" don't
mean anything.

4 files changed, 74 insertions, 62 deletions

diff --git a/lib/pry/commands/cd.rb b/lib/pry/commands/cd.rb
index 8e8cddb3..667453a0 100644
--- a/lib/pry/commands/cd.rb
+++ b/lib/pry/commands/cd.rb
@@ -21,9 +21,19 @@ class Pry
 
     def process
       state.old_stack ||= []
-      stack, state.old_stack =
-        ObjectPath.new(arg_string, _pry_.binding_stack, state.old_stack).resolve
-      _pry_.binding_stack = stack if stack
+
+      if arg_string.strip == "-"
+        unless state.old_stack.empty?
+          _pry_.binding_stack, state.old_stack = state.old_stack, _pry_.binding_stack
+        end
+      else
+        stack = ObjectPath.new(arg_string, _pry_.binding_stack).resolve
+
+        if stack && stack != _pry_.binding_stack
+          state.old_stack = _pry_.binding_stack
+          _pry_.binding_stack = stack
+        end
+      end
     end
   end
 
diff --git a/lib/pry/input_completer.rb b/lib/pry/input_completer.rb
index 86f9abf0..d310f042 100644
--- a/lib/pry/input_completer.rb
+++ b/lib/pry/input_completer.rb
@@ -60,8 +60,7 @@ class Pry::InputCompleter
     if path.call.empty?
       target = options[:target]
     else
-      target, _ = Pry::ObjectPath.new(path.call, @pry.binding_stack).resolve
-      target = target.last
+      target = Pry::ObjectPath.new(path.call, @pry.binding_stack).resolve.last
     end
 
     begin
diff --git a/lib/pry/object_path.rb b/lib/pry/object_path.rb
index b669d3ec..5bffb02c 100644
--- a/lib/pry/object_path.rb
+++ b/lib/pry/object_path.rb
@@ -11,79 +11,72 @@ class Pry
   # Object paths are mostly relevant in the context of the `cd` command.
   # @see https://github.com/pry/pry/wiki/State-navigation
   class ObjectPath
+    SPECIAL_TERMS = ["", "::", ".", ".."]
+
     # @param [String] path_string The object path expressed as a string.
     # @param [Array<Binding>] current_stack The current state of the binding
     #   stack.
-    # @param [Array<Binding>] old_stack The previous state of the binding
-    #   stack, if applicable.
-    def initialize(path_string, current_stack, old_stack=[])
+    def initialize(path_string, current_stack)
       @path_string   = path_string
       @current_stack = current_stack
-      @old_stack     = old_stack
     end
 
-    # @return [Array(Array<Binding>, Array<Binding>)] an array
-    #   containing two elements, the new binding stack and the old binding
-    #   stack.
+    # @return [Array<Binding>] a new stack resulting from applying the given
+    #   path to the current stack.
     def resolve
-      # Extract command arguments. Delete blank arguments like " ", but
-      # don't delete empty strings like "".
-      path      = @path_string.split(/\//).delete_if { |a| a =~ /\A\s+\z/ }
-      stack     = @current_stack.dup
-      state_old_stack = @old_stack
+      scanner = StringScanner.new(@path_string.strip)
+      stack   = @current_stack.dup
+
+      begin
+        next_segment  = ""
 
-      # Special case when we only get a single "/", return to root.
-      if path.empty?
-        state_old_stack = stack.dup unless @old_stack.empty?
-        stack = [stack.first]
-      end
+        loop do
+          # Scan for as long as we don't see a slash
+          next_segment << scanner.scan(/[^\/]*/)
 
-      path.each_with_index do |context, i|
-        begin
-          case context.chomp
-          when ""
-            state_old_stack = stack.dup
-            stack = [stack.first]
-          when "::"
-            state_old_stack = stack.dup
-            stack.push(TOPLEVEL_BINDING)
-          when "."
-            next
-          when ".."
-            unless stack.size == 1
-              # Don't rewrite old_stack if we're in complex expression
-              # (e.g.: `cd 1/2/3/../4).
-              state_old_stack = stack.dup if path.first == ".."
-              stack.pop
-            end
-          when "-"
-            unless @old_stack.empty?
-              # Interchange current stack and old stack with each other.
-              stack, state_old_stack = state_old_stack, stack
-            end
+          if complete?(next_segment) || scanner.eos?
+            scanner.getch # consume the slash
+            break
           else
-            state_old_stack = stack.dup if i == 0
-            stack.push(Pry.binding_for(stack.last.eval(context)))
+            next_segment << scanner.getch # append the slash
           end
+        end
 
-        rescue RescuableException => e
-          # Restore old stack to its initial values.
-          state_old_stack = @old_stack
+        case next_segment.chomp
+        when ""
+          stack = [stack.first]
+        when "::"
+          stack.push(TOPLEVEL_BINDING)
+        when "."
+          next
+        when ".."
+          stack.pop unless stack.size == 1
+        else
+          stack.push(Pry.binding_for(stack.last.eval(next_segment)))
+        end
+      rescue RescuableException => e
+        return handle_failure(next_segment, e)
+      end until scanner.eos?
 
-          msg = [
-            "Bad object path: #{@path_string}.",
-            "Failed trying to resolve: #{context}.",
-            e.inspect
-          ].join(' ')
+      stack
+    end
 
-          CommandError.new(msg).tap do |err|
-            err.set_backtrace e.backtrace
-            raise err
-          end
-        end
-      end
+    private
+
+    def complete?(segment)
+      SPECIAL_TERMS.include?(segment) || Pry::Code.complete_expression?(segment)
+    end
+
+    def handle_failure(context, err)
+      msg = [
+        "Bad object path: #{@path_string.inspect}",
+        "Failed trying to resolve: #{context.inspect}",
+        "Exception: #{err.inspect}"
+      ].join("\n")
 
-      [stack, state_old_stack]
+      raise CommandError.new(msg).tap { |e|
+        e.set_backtrace err.backtrace
+      }
     end
   end
 end
diff --git a/spec/commands/cd_spec.rb b/spec/commands/cd_spec.rb
index a156dd9d..b8b7a31d 100644
--- a/spec/commands/cd_spec.rb
+++ b/spec/commands/cd_spec.rb
@@ -240,6 +240,16 @@ describe 'cd' do
     @t.assert_binding_stack [@o]
   end
 
+  it 'can cd into an expression containing a string with slashes in it' do
+    @t.eval 'cd ["http://google.com"]'
+    @t.eval('self').should == ["http://google.com"]
+  end
+
+  it 'can cd into an expression with division in it' do
+    @t.eval 'cd (10/2)/even?'
+    @t.eval('self').should == false
+  end
+
   # Regression test for ticket #516.
   # FIXME: This is actually broken.
   # it 'should be able to cd into the Object BasicObject' do