Support FixIt commands across buffers

We simply apply the changes to each file in turn. The existing replacement
logic is unchanged, except that it now no longer implicitly assumes we are
talking about the current buffer.

If a buffer is not visible for the requested file name, we open it in
a horizontal split, make the edits, then hide the window. Because this
can cause UI flickering, and leave hidden, modified buffers around, we
issue a warning to the user stating the number of files for which we are
going to do this. We pop up the quickfix list at the end of applying
the edits to allow the user to see what we changed.

If the user opts to abort due to, say, the file being open in another
window, we simply raise an error and give up, as undoing the changes
is too complex to do programatically, but trivial to do manually in such
a rare case.

python/ycm/client/command_request.py

@@ -80,8 +80,8 @@ class CommandRequest( BaseRequest ):
 
   def _HandleGotoResponse( self ):
     if isinstance( self._response, list ):
-      defs = [ _BuildQfListItem( x ) for x in self._response ]
-      vim.eval( 'setqflist( %s )' % repr( defs ) )
+      vimsupport.SetQuickFixList(
+              [ _BuildQfListItem( x ) for x in self._response ] )
       vim.eval( 'youcompleteme#OpenGoToList()' )
     else:
       vimsupport.JumpToLocation( self._response[ 'filepath' ],
@@ -94,12 +94,10 @@ class CommandRequest( BaseRequest ):
       vimsupport.EchoText( "No fixits found for current line" )
     else:
       chunks = self._response[ 'fixits' ][ 0 ][ 'chunks' ]
-
-      vimsupport.ReplaceChunksList( chunks )
-
-      vimsupport.EchoTextVimWidth( "FixIt applied "
-                                   + str( len( chunks ) )
-                                   + " changes" )
+      try:
+        vimsupport.ReplaceChunks( chunks )
+      except RuntimeError as e:
+        vimsupport.PostMultiLineNotice( e.message )
 
 
   def _HandleBasicResponse( self ):

python/ycm/client/tests/command_request_test.py

@@ -18,7 +18,9 @@
 from ycm.test_utils import MockVimModule
 MockVimModule()
 
+import json
 from mock import patch, call
+from nose.tools import ok_
 from ycm.client.command_request import CommandRequest
 
 
@@ -85,6 +87,177 @@ class GoToResponse_QuickFix_test:
     self._request.RunPostCommandActionsIfNeeded()
 
     vim_eval.assert_has_calls( [
-      call( 'setqflist( {0} )'.format( repr( expected_qf_list ) ) ),
+      call( 'setqflist( {0} )'.format( json.dumps( expected_qf_list ) ) ),
       call( 'youcompleteme#OpenGoToList()' ),
     ] )
+
+
+class Response_Detection_test:
+
+  def BasicResponse_test( self ):
+    def _BasicResponseTest( command, response ):
+      with patch( 'vim.command' ) as vim_command:
+        request = CommandRequest( [ command ] )
+        request._response = response
+        request.RunPostCommandActionsIfNeeded()
+        vim_command.assert_called_with( "echom '{0}'".format( response ) )
+
+    tests = [
+      [ 'AnythingYouLike',        True ],
+      [ 'GoToEvenWorks',          10 ],
+      [ 'FixItWorks',             'String!' ],
+      [ 'and8434fd andy garbag!', 10.3 ],
+    ]
+
+    for test in tests:
+      yield _BasicResponseTest, test[ 0 ], test[ 1 ]
+
+
+  def FixIt_Response_Empty_test( self ):
+    # Ensures we recognise and handle fixit responses which indicate that there
+    # are no fixits available
+    def EmptyFixItTest( command ):
+      with patch( 'ycm.vimsupport.ReplaceChunks' ) as replace_chunks:
+        with patch( 'ycm.vimsupport.EchoText' ) as echo_text:
+          request = CommandRequest( [ command ] )
+          request._response = {
+            'fixits': []
+          }
+          request.RunPostCommandActionsIfNeeded()
+
+          echo_text.assert_called_with( 'No fixits found for current line' )
+          replace_chunks.assert_not_called()
+
+    for test in [ 'FixIt', 'Refactor', 'GoToHell', 'any_old_garbade!!!21' ]:
+      yield EmptyFixItTest, test
+
+
+  def FixIt_Response_test( self ):
+    # Ensures we recognise and handle fixit responses with some dummy chunk data
+    def FixItTest( command, response, chunks ):
+      with patch( 'ycm.vimsupport.ReplaceChunks' ) as replace_chunks:
+        with patch( 'ycm.vimsupport.EchoText' ) as echo_text:
+          request = CommandRequest( [ command ] )
+          request._response = response
+          request.RunPostCommandActionsIfNeeded()
+
+          replace_chunks.assert_called_with( chunks )
+          echo_text.assert_not_called()
+
+    basic_fixit = {
+      'fixits': [ {
+        'chunks': [ {
+          'dummy chunk contents': True
+        } ]
+      } ]
+    }
+    basic_fixit_chunks = basic_fixit[ 'fixits' ][ 0 ][ 'chunks' ]
+
+    multi_fixit = {
+      'fixits': [ {
+        'chunks': [ {
+          'dummy chunk contents': True
+        } ]
+      }, {
+        'additional fixits are ignored currently': True
+      } ]
+    }
+    multi_fixit_first_chunks = multi_fixit[ 'fixits' ][ 0 ][ 'chunks' ]
+
+    tests = [
+      [ 'AnythingYouLike',        basic_fixit, basic_fixit_chunks ],
+      [ 'GoToEvenWorks',          basic_fixit, basic_fixit_chunks ],
+      [ 'FixItWorks',             basic_fixit, basic_fixit_chunks ],
+      [ 'and8434fd andy garbag!', basic_fixit, basic_fixit_chunks ],
+      [ 'additional fixits ignored', multi_fixit, multi_fixit_first_chunks ],
+    ]
+
+    for test in tests:
+      yield FixItTest, test[ 0 ], test[ 1 ], test[ 2 ]
+
+
+  def Message_Response_test( self ):
+    # Ensures we correctly recognise and handle responses with a message to show
+    # to the user
+
+    def MessageTest( command, message ):
+      with patch( 'ycm.vimsupport.EchoText' ) as echo_text:
+        request = CommandRequest( [ command ] )
+        request._response = { 'message': message }
+        request.RunPostCommandActionsIfNeeded()
+        echo_text.assert_called_with( message )
+
+    tests = [
+      [ '___________', 'This is a message' ],
+      [ '',            'this is also a message' ],
+      [ 'GetType',     'std::string' ],
+    ]
+
+    for test in tests:
+      yield MessageTest, test[ 0 ], test[ 1 ]
+
+
+  def Detailed_Info_test( self ):
+    # Ensures we correctly detect and handle detailed_info responses which are
+    # used to display information in the preview window
+
+    def DetailedInfoTest( command, info ):
+      with patch( 'ycm.vimsupport.WriteToPreviewWindow' ) as write_to_preview:
+        request = CommandRequest( [ command ] )
+        request._response = { 'detailed_info': info }
+        request.RunPostCommandActionsIfNeeded()
+        write_to_preview.assert_called_with( info )
+
+    tests = [
+      [ '___________', 'This is a message' ],
+      [ '',            'this is also a message' ],
+      [ 'GetDoc',      'std::string\netc\netc' ],
+    ]
+
+    for test in tests:
+      yield DetailedInfoTest, test[ 0 ], test[ 1 ]
+
+
+  def GoTo_Single_test( self ):
+    # Ensures we handle any unknown type of response as a GoTo response
+
+    def GoToTest( command, response ):
+      with patch( 'ycm.vimsupport.JumpToLocation' ) as jump_to_location:
+        request = CommandRequest( [ command ] )
+        request._response = response
+        request.RunPostCommandActionsIfNeeded()
+        jump_to_location.assert_called_with(
+            response[ 'filepath' ],
+            response[ 'line_num' ],
+            response[ 'column_num' ] )
+
+    def GoToListTest( command, response ):
+      # Note: the detail of these called are tested by
+      # GoToResponse_QuickFix_test, so here we just check that the right call is
+      # made
+      with patch( 'ycm.vimsupport.SetQuickFixList' ) as set_qf_list:
+        with patch( 'vim.eval' ) as vim_eval:
+          request = CommandRequest( [ command ] )
+          request._response = response
+          request.RunPostCommandActionsIfNeeded()
+          ok_( set_qf_list.called )
+          ok_( vim_eval.called )
+
+    basic_goto = {
+      'filepath': 'test',
+      'line_num': 10,
+      'column_num': 100,
+    }
+
+    tests = [
+      [ GoToTest,     'AnythingYouLike', basic_goto ],
+      [ GoToTest,     'GoTo',            basic_goto ],
+      [ GoToTest,     'FindAThing',      basic_goto ],
+      [ GoToTest,     'FixItGoto',       basic_goto ],
+      [ GoToListTest, 'AnythingYouLike', [ basic_goto ] ],
+      [ GoToListTest, 'GoTo',            []  ],
+      [ GoToListTest, 'FixItGoto',       [ basic_goto, basic_goto ] ],
+    ]
+
+    for test in tests:
+      yield test[ 0 ], test[ 1 ], test[ 2 ]

python/ycm/tests/vimsupport_test.py

@@ -15,7 +15,7 @@
 # You should have received a copy of the GNU General Public License
 # along with YouCompleteMe.  If not, see <http://www.gnu.org/licenses/>.
 
-from ycm.test_utils import MockVimModule, MockVimCommand
+from ycm.test_utils import ExtendedMock, MockVimModule, MockVimCommand
 MockVimModule()
 
 from ycm import vimsupport
@@ -23,6 +23,7 @@ from nose.tools import eq_
 from hamcrest import assert_that, calling, raises, none
 from mock import MagicMock, call, patch
 import os
+import json
 
 
 def ReplaceChunk_SingleLine_Repl_1_test():
@@ -239,9 +240,9 @@ def ReplaceChunk_SingleToMultipleLines_test():
 
   # now make another change to the "2nd" line
   start, end = _BuildLocations( 2, 3, 2, 4 )
-  ( new_line_offset, new_char_offset ) = vimsupport.ReplaceChunk( 
-                                                           start, 
-                                                           end, 
+  ( new_line_offset, new_char_offset ) = vimsupport.ReplaceChunk(
+                                                           start,
+                                                           end,
                                                            'cccc',
                                                            line_offset,
                                                            char_offset,
@@ -343,7 +344,6 @@ def ReplaceChunk_SingleToMultipleLinesReplace_2_test():
   eq_( char_offset, 4 )
 
 
-
 def ReplaceChunk_MultipleLinesToSingleLine_test():
   result_buffer = [ "aAa", "aBa", "aCaaaa" ]
   start, end = _BuildLocations( 2, 2, 3, 2 )
@@ -539,41 +539,517 @@ def _BuildLocations( start_line, start_column, end_line, end_column ):
   }
 
 
-def ReplaceChunksList_SortedChunks_test():
+def ReplaceChunksInBuffer_SortedChunks_test():
   chunks = [
     _BuildChunk( 1, 4, 1, 4, '('),
     _BuildChunk( 1, 11, 1, 11, ')' )
   ]
 
   result_buffer = [ "CT<10 >> 2> ct" ]
-  vimsupport.ReplaceChunksList( chunks, result_buffer )
+  vimsupport.ReplaceChunksInBuffer( chunks, result_buffer, None )
 
   expected_buffer = [ "CT<(10 >> 2)> ct" ]
   eq_( expected_buffer, result_buffer )
 
 
-def ReplaceChunksList_UnsortedChunks_test():
+def ReplaceChunksInBuffer_UnsortedChunks_test():
   chunks = [
     _BuildChunk( 1, 11, 1, 11, ')'),
     _BuildChunk( 1, 4, 1, 4, '(' )
   ]
 
   result_buffer = [ "CT<10 >> 2> ct" ]
-  vimsupport.ReplaceChunksList( chunks, result_buffer )
+  vimsupport.ReplaceChunksInBuffer( chunks, result_buffer, None )
 
   expected_buffer = [ "CT<(10 >> 2)> ct" ]
   eq_( expected_buffer, result_buffer )
 
 
-def _BuildChunk( start_line, start_column, end_line, end_column,
-                 replacement_text ):
+class MockBuffer( ):
+  """An object that looks like a vim.buffer object, enough for ReplaceChunk to
+  generate a location list"""
+
+  def __init__( self, lines, name, number ):
+    self.lines = lines
+    self.name = name
+    self.number = number
+
+
+  def __getitem__( self, index ):
+    return self.lines[ index ]
+
+
+  def __len__( self ):
+    return len( self.lines )
+
+
+  def __setitem__( self, key, value ):
+    return self.lines.__setitem__( key, value )
+
+
+@patch( 'ycm.vimsupport.GetBufferNumberForFilename',
+        return_value=1,
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.BufferIsVisible',
+        return_value=True,
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.OpenFilename' )
+@patch( 'ycm.vimsupport.EchoTextVimWidth', new_callable=ExtendedMock )
+@patch( 'vim.eval', new_callable=ExtendedMock )
+@patch( 'vim.command', new_callable=ExtendedMock )
+def ReplaceChunks_SingleFile_Open_test( vim_command,
+                                        vim_eval,
+                                        echo_text_vim_width,
+                                        open_filename,
+                                        buffer_is_visible,
+                                        get_buffer_number_for_filename ):
+
+  chunks = [
+    _BuildChunk( 1, 1, 2, 1, 'replacement', 'single_file' )
+  ]
+
+  result_buffer = MockBuffer( [
+    'line1',
+    'line2',
+    'line3',
+  ], 'single_file', 1 )
+
+  with patch( 'vim.buffers', [ None, result_buffer, None ] ):
+    vimsupport.ReplaceChunks( chunks )
+
+  # Ensure that we applied the replacement correctly
+  eq_( result_buffer.lines, [
+    'replacementline2',
+    'line3',
+  ] )
+
+  # GetBufferNumberForFilename is called twice:
+  #  - once to the check if we would require opening the file (so that we can
+  #    raise a warning)
+  #  - once whilst applying the changes
+  get_buffer_number_for_filename.assert_has_exact_calls( [
+      call( 'single_file', False ),
+      call( 'single_file', False ),
+  ] )
+
+  # BufferIsVisible is called twice for the same reasons as above
+  buffer_is_visible.assert_has_exact_calls( [
+      call( 1 ),
+      call( 1 ),
+  ] )
+
+  # we don't attempt to open any files
+  open_filename.assert_not_called()
+
+  # But we do set the quickfix list
+  vim_eval.assert_has_exact_calls( [
+      call( 'setqflist( {0} )'.format( json.dumps( [ {
+        'bufnr': 1,
+        'filename': 'single_file',
+        'lnum': 1,
+        'col': 1,
+        'text': 'replacement',
+        'type': 'F'
+      } ] ) ) ),
+  ] )
+  vim_command.assert_has_calls( [
+      call( 'copen 1' )
+  ] )
+
+  # And it is ReplaceChunks that prints the message showing the number of
+  # changes
+  echo_text_vim_width.assert_has_exact_calls( [
+      call( 'Applied 1 changes' ),
+  ] )
+
+
+@patch( 'ycm.vimsupport.GetBufferNumberForFilename',
+        side_effect=[ -1, -1, 1 ],
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.BufferIsVisible',
+        side_effect=[ False, False, True ],
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.OpenFilename',
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.EchoTextVimWidth', new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.Confirm',
+        return_value=True,
+        new_callable=ExtendedMock )
+@patch( 'vim.eval', return_value=10, new_callable=ExtendedMock )
+@patch( 'vim.command', new_callable=ExtendedMock )
+def ReplaceChunks_SingleFile_NotOpen_test( vim_command,
+                                           vim_eval,
+                                           confirm,
+                                           echo_text_vim_width,
+                                           open_filename,
+                                           buffer_is_visible,
+                                           get_buffer_number_for_filename ):
+
+  chunks = [
+    _BuildChunk( 1, 1, 2, 1, 'replacement', 'single_file' )
+  ]
+
+  result_buffer = MockBuffer( [
+    'line1',
+    'line2',
+    'line3',
+  ], 'single_file', 1 )
+
+  with patch( 'vim.buffers', [ None, result_buffer, None ] ):
+    vimsupport.ReplaceChunks( chunks )
+
+  # We checked if it was OK to open the file
+  confirm.assert_has_exact_calls( [
+    call( vimsupport.FIXIT_OPENING_BUFFERS_MESSAGE_FORMAT.format( 1 ) )
+  ] )
+
+  # Ensure that we applied the replacement correctly
+  eq_( result_buffer.lines, [
+    'replacementline2',
+    'line3',
+  ] )
+
+  # GetBufferNumberForFilename is called 3 times. The return values are set in
+  # the @patch call above:
+  #  - once to the check if we would require opening the file (so that we can
+  #    raise a warning) (-1 return)
+  #  - once whilst applying the changes (-1 return)
+  #  - finally after calling OpenFilename (1 return)
+  get_buffer_number_for_filename.assert_has_exact_calls( [
+      call( 'single_file', False ),
+      call( 'single_file', False ),
+      call( 'single_file', False ),
+  ] )
+
+  # BufferIsVisible is called 3 times for the same reasons as above, with the
+  # return of each one
+  buffer_is_visible.assert_has_exact_calls( [
+    call( -1 ),
+    call( -1 ),
+    call( 1 ),
+  ] )
+
+  # We open 'single_file' as expected.
+  open_filename.assert_called_with( 'single_file', {
+    'focus': True,
+    'fix': True,
+    'size': 10
+  } )
+
+  # And close it again, then show the preview window (note, we don't check exact
+  # calls because there are other calls which are checked elsewhere)
+  vim_command.assert_has_calls( [
+    call( 'lclose' ),
+    call( 'hide' ),
+    call( 'copen 1' ),
+  ] )
+
+  # And update the quickfix list
+  vim_eval.assert_has_exact_calls( [
+    call( '&previewheight' ),
+    call( 'setqflist( {0} )'.format( json.dumps( [ {
+      'bufnr': 1,
+      'filename': 'single_file',
+      'lnum': 1,
+      'col': 1,
+      'text': 'replacement',
+      'type': 'F'
+    } ] ) ) ),
+  ] )
+
+  # And it is ReplaceChunks that prints the message showing the number of
+  # changes
+  echo_text_vim_width.assert_has_exact_calls( [
+    call( 'Applied 1 changes' ),
+  ] )
+
+
+@patch( 'ycm.vimsupport.GetBufferNumberForFilename',
+        side_effect=[ -1, -1, 1 ],
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.BufferIsVisible',
+        side_effect=[ False, False, True ],
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.OpenFilename',
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.EchoTextVimWidth',
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.Confirm',
+        return_value=False,
+        new_callable=ExtendedMock )
+@patch( 'vim.eval',
+        return_value=10,
+        new_callable=ExtendedMock )
+@patch( 'vim.command', new_callable=ExtendedMock )
+def ReplaceChunks_User_Declines_To_Open_File_test(
+                                           vim_command,
+                                           vim_eval,
+                                           confirm,
+                                           echo_text_vim_width,
+                                           open_filename,
+                                           buffer_is_visible,
+                                           get_buffer_number_for_filename ):
+
+  # Same as above, except the user selects Cancel when asked if they should
+  # allow us to open lots of (ahem, 1) file.
+
+  chunks = [
+    _BuildChunk( 1, 1, 2, 1, 'replacement', 'single_file' )
+  ]
+
+  result_buffer = MockBuffer( [
+    'line1',
+    'line2',
+    'line3',
+  ], 'single_file', 1 )
+
+  with patch( 'vim.buffers', [ None, result_buffer, None ] ):
+    vimsupport.ReplaceChunks( chunks )
+
+  # We checked if it was OK to open the file
+  confirm.assert_has_exact_calls( [
+    call( vimsupport.FIXIT_OPENING_BUFFERS_MESSAGE_FORMAT.format( 1 ) )
+  ] )
+
+  # Ensure that buffer is not changed
+  eq_( result_buffer.lines, [
+    'line1',
+    'line2',
+    'line3',
+  ] )
+
+  # GetBufferNumberForFilename is called once. The return values are set in
+  # the @patch call above:
+  #  - once to the check if we would require opening the file (so that we can
+  #    raise a warning) (-1 return)
+  get_buffer_number_for_filename.assert_has_exact_calls( [
+      call( 'single_file', False ),
+  ] )
+
+  # BufferIsVisible is called once for the above file, which wasn't visible.
+  buffer_is_visible.assert_has_exact_calls( [
+    call( -1 ),
+  ] )
+
+  # We don't attempt to open any files or update any quickfix list or anything
+  # like that
+  open_filename.assert_not_called()
+  vim_eval.assert_not_called()
+  vim_command.assert_not_called()
+  echo_text_vim_width.assert_not_called()
+
+
+@patch( 'ycm.vimsupport.GetBufferNumberForFilename',
+        side_effect=[ -1, -1, 1 ],
+        new_callable=ExtendedMock )
+# Key difference is here: In the final check, BufferIsVisible returns False
+@patch( 'ycm.vimsupport.BufferIsVisible',
+        side_effect=[ False, False, False ],
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.OpenFilename',
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.EchoTextVimWidth',
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.Confirm',
+        return_value=True,
+        new_callable=ExtendedMock )
+@patch( 'vim.eval',
+        return_value=10,
+        new_callable=ExtendedMock )
+@patch( 'vim.command',
+        new_callable=ExtendedMock )
+def ReplaceChunks_User_Aborts_Opening_File_test(
+                                           vim_command,
+                                           vim_eval,
+                                           confirm,
+                                           echo_text_vim_width,
+                                           open_filename,
+                                           buffer_is_visible,
+                                           get_buffer_number_for_filename ):
+
+  # Same as above, except the user selects Abort or Quick during the
+  # "swap-file-found" dialog
+
+  chunks = [
+    _BuildChunk( 1, 1, 2, 1, 'replacement', 'single_file' )
+  ]
+
+  result_buffer = MockBuffer( [
+    'line1',
+    'line2',
+    'line3',
+  ], 'single_file', 1 )
+
+  with patch( 'vim.buffers', [ None, result_buffer, None ] ):
+    assert_that( calling( vimsupport.ReplaceChunks ).with_args( chunks ),
+                 raises( RuntimeError,
+                  'Unable to open file: single_file\nFixIt/Refactor operation '
+                  'aborted prior to completion. Your files have not been '
+                  'fully updated. Please use undo commands to revert the '
+                  'applied changes.' ) )
+
+  # We checked if it was OK to open the file
+  confirm.assert_has_exact_calls( [
+    call( vimsupport.FIXIT_OPENING_BUFFERS_MESSAGE_FORMAT.format( 1 ) )
+  ] )
+
+  # Ensure that buffer is not changed
+  eq_( result_buffer.lines, [
+    'line1',
+    'line2',
+    'line3',
+  ] )
+
+  # We tried to open this file
+  open_filename.assert_called_with( "single_file", {
+    'focus': True,
+    'fix': True,
+    'size': 10
+  } )
+  vim_eval.assert_called_with( "&previewheight" )
+
+  # But raised an exception before issuing the message at the end
+  echo_text_vim_width.assert_not_called()
+
+
+@patch( 'ycm.vimsupport.GetBufferNumberForFilename', side_effect=[
+          22, # first_file (check)
+          -1, # another_file (check)
+          22, # first_file (apply)
+          -1, # another_file (apply)
+          19, # another_file (check after open)
+        ],
+        new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.BufferIsVisible', side_effect=[
+          True,  # first_file (check)
+          False, # second_file (check)
+          True,  # first_file (apply)
+          False, # second_file (apply)
+          True,  # side_effect (check after open)
+        ],
+        new_callable=ExtendedMock)
+@patch( 'ycm.vimsupport.OpenFilename',
+        new_callable=ExtendedMock)
+@patch( 'ycm.vimsupport.EchoTextVimWidth',
+        new_callable=ExtendedMock)
+@patch( 'ycm.vimsupport.Confirm', return_value=True,
+        new_callable=ExtendedMock)
+@patch( 'vim.eval', return_value=10,
+        new_callable=ExtendedMock)
+@patch( 'vim.command',
+        new_callable=ExtendedMock)
+def ReplaceChunks_MultiFile_Open_test( vim_command,
+                                       vim_eval,
+                                       confirm,
+                                       echo_text_vim_width,
+                                       open_filename,
+                                       buffer_is_visible,
+                                       get_buffer_number_for_filename ):
+
+  # Chunks are split across 2 files, one is already open, one isn't
+
+  chunks = [
+    _BuildChunk( 1, 1, 2, 1, 'first_file_replacement ', '1_first_file' ),
+    _BuildChunk( 2, 1, 2, 1, 'second_file_replacement ', '2_another_file' ),
+  ]
+
+  first_file = MockBuffer( [
+    'line1',
+    'line2',
+    'line3',
+  ], '1_first_file', 22 )
+  another_file = MockBuffer( [
+    'another line1',
+    'ACME line2',
+  ], '2_another_file', 19 )
+
+  vim_buffers = [ None ] * 23
+  vim_buffers[ 22 ] = first_file
+  vim_buffers[ 19 ] = another_file
+
+  with patch( 'vim.buffers', vim_buffers ):
+    vimsupport.ReplaceChunks( chunks )
+
+  # We checked for the right file names
+  get_buffer_number_for_filename.assert_has_exact_calls( [
+    call( '1_first_file', False ),
+    call( '2_another_file', False ),
+    call( '1_first_file', False ),
+    call( '2_another_file', False ),
+    call( '2_another_file', False ),
+  ] )
+
+  # We checked if it was OK to open the file
+  confirm.assert_has_exact_calls( [
+    call( vimsupport.FIXIT_OPENING_BUFFERS_MESSAGE_FORMAT.format( 1 ) )
+  ] )
+
+  # Ensure that buffers are updated
+  eq_( another_file.lines, [
+    'another line1',
+    'second_file_replacement ACME line2',
+  ] )
+  eq_( first_file.lines, [
+    'first_file_replacement line2',
+    'line3',
+  ] )
+
+  # We open '2_another_file' as expected.
+  open_filename.assert_called_with( '2_another_file', {
+    'focus': True,
+    'fix': True,
+    'size': 10
+  } )
+
+  # And close it again, then show the preview window (note, we don't check exact
+  # calls because there are other calls which are checked elsewhere)
+  vim_command.assert_has_calls( [
+    call( 'lclose' ),
+    call( 'hide' ),
+    call( 'copen 2' ),
+  ] )
+
+  # And update the quickfix list with each entry
+  vim_eval.assert_has_exact_calls( [
+    call( '&previewheight' ),
+    call( 'setqflist( {0} )'.format( json.dumps( [ {
+      'bufnr': 22,
+      'filename': '1_first_file',
+      'lnum': 1,
+      'col': 1,
+      'text': 'first_file_replacement ',
+      'type': 'F'
+    }, {
+      'bufnr': 19,
+      'filename': '2_another_file',
+      'lnum': 2,
+      'col': 1,
+      'text': 'second_file_replacement ',
+      'type': 'F'
+    } ] ) ) ),
+  ] )
+
+  # And it is ReplaceChunks that prints the message showing the number of
+  # changes
+  echo_text_vim_width.assert_has_exact_calls( [
+    call( 'Applied 2 changes' ),
+  ] )
+
+
+def _BuildChunk( start_line,
+                 start_column,
+                 end_line,
+                 end_column,
+                 replacement_text, filepath='test_file_name' ):
   return {
     'range': {
       'start': {
+        'filepath': filepath,
         'line_num': start_line,
         'column_num': start_column,
       },
       'end': {
+        'filepath': filepath,
         'line_num': end_line,
         'column_num': end_column,
       },
@@ -582,14 +1058,14 @@ def _BuildChunk( start_line, start_column, end_line, end_column,
   }
 
 
-@patch( 'vim.command' )
-@patch( 'vim.current' )
+@patch( 'vim.command', new_callable=ExtendedMock )
+@patch( 'vim.current', new_callable=ExtendedMock)
 def WriteToPreviewWindow_test( vim_current, vim_command ):
   vim_current.window.options.__getitem__ = MagicMock( return_value = True )
 
   vimsupport.WriteToPreviewWindow( "test" )
 
-  vim_command.assert_has_calls( [
+  vim_command.assert_has_exact_calls( [
     call( 'silent! pclose!' ),
     call( 'silent! pedit! _TEMP_FILE_' ),
     call( 'silent! wincmd P' ),
@@ -598,7 +1074,9 @@ def WriteToPreviewWindow_test( vim_current, vim_command ):
   vim_current.buffer.__setitem__.assert_called_with(
       slice( None, None, None ), [ 'test' ] )
 
-  vim_current.buffer.options.__setitem__.assert_has_calls( [
+  vim_current.buffer.options.__setitem__.assert_has_exact_calls( [
+    call( 'modifiable', True ),
+    call( 'readonly', False ),
     call( 'buftype', 'nofile' ),
     call( 'swapfile', False ),
     call( 'modifiable', False ),
@@ -616,14 +1094,14 @@ def WriteToPreviewWindow_MultiLine_test( vim_current ):
       slice( None, None, None ), [ 'test', 'test2' ] )
 
 
-@patch( 'vim.command' )
-@patch( 'vim.current' )
+@patch( 'vim.command', new_callable=ExtendedMock )
+@patch( 'vim.current', new_callable=ExtendedMock )
 def WriteToPreviewWindow_JumpFail_test( vim_current, vim_command ):
   vim_current.window.options.__getitem__ = MagicMock( return_value = False )
 
   vimsupport.WriteToPreviewWindow( "test" )
 
-  vim_command.assert_has_calls( [
+  vim_command.assert_has_exact_calls( [
     call( 'silent! pclose!' ),
     call( 'silent! pedit! _TEMP_FILE_' ),
     call( 'silent! wincmd P' ),
@@ -634,15 +1112,15 @@ def WriteToPreviewWindow_JumpFail_test( vim_current, vim_command ):
   vim_current.buffer.options.__setitem__.assert_not_called()
 
 
-@patch( 'vim.command' )
-@patch( 'vim.current' )
+@patch( 'vim.command', new_callable=ExtendedMock )
+@patch( 'vim.current', new_callable=ExtendedMock )
 def WriteToPreviewWindow_JumpFail_MultiLine_test( vim_current, vim_command ):
 
   vim_current.window.options.__getitem__ = MagicMock( return_value = False )
 
   vimsupport.WriteToPreviewWindow( "test\ntest2" )
 
-  vim_command.assert_has_calls( [
+  vim_command.assert_has_exact_calls( [
     call( 'silent! pclose!' ),
     call( 'silent! pedit! _TEMP_FILE_' ),
     call( 'silent! wincmd P' ),
@@ -689,7 +1167,9 @@ def BufferIsVisibleForFilename_test():
     eq_( vimsupport.BufferIsVisibleForFilename( 'another_filename' ), False )
 
 
-@patch( 'vim.command', side_effect = MockVimCommand )
+@patch( 'vim.command',
+        side_effect = MockVimCommand,
+        new_callable=ExtendedMock )
 def CloseBuffersForFilename_test( vim_command ):
   buffers = [
     {
@@ -709,14 +1189,14 @@ def CloseBuffersForFilename_test( vim_command ):
   with patch( 'vim.buffers', buffers ):
     vimsupport.CloseBuffersForFilename( 'some_filename' )
 
-  vim_command.assert_has_calls( [
+  vim_command.assert_has_exact_calls( [
     call( 'silent! bwipeout! 2' ),
     call( 'silent! bwipeout! 5' )
   ], any_order = True )
 
 
-@patch( 'vim.command' )
-@patch( 'vim.current' )
+@patch( 'vim.command', new_callable=ExtendedMock )
+@patch( 'vim.current', new_callable=ExtendedMock )
 def OpenFilename_test( vim_current, vim_command ):
   # Options used to open a logfile
   options = {
@@ -728,18 +1208,18 @@ def OpenFilename_test( vim_current, vim_command ):
 
   vimsupport.OpenFilename( __file__, options )
 
-  vim_command.assert_has_calls( [
-    call( 'silent! 12split {0}'.format( __file__ ) ),
+  vim_command.assert_has_exact_calls( [
+    call( '12split {0}'.format( __file__ ) ),
     call( "exec "
           "'au BufEnter <buffer> :silent! checktime {0}'".format( __file__ ) ),
     call( 'silent! normal G zz' ),
     call( 'silent! wincmd p' )
   ] )
 
-  vim_current.buffer.options.__setitem__.assert_has_calls( [
+  vim_current.buffer.options.__setitem__.assert_has_exact_calls( [
     call( 'autoread', True ),
   ] )
 
-  vim_current.window.options.__setitem__.assert_has_calls( [
+  vim_current.window.options.__setitem__.assert_has_exact_calls( [
     call( 'winfixheight', True )
   ] )

python/ycm/vimsupport.py

@@ -20,6 +20,7 @@ import os
 import tempfile
 import json
 import re
+from collections import defaultdict
 from ycmd.utils import ToUtf8IfNeeded
 from ycmd import user_options_store
 
@@ -28,6 +29,13 @@ BUFFER_COMMAND_MAP = { 'same-buffer'      : 'edit',
                        'vertical-split'   : 'vsplit',
                        'new-tab'          : 'tabedit' }
 
+FIXIT_OPENING_BUFFERS_MESSAGE_FORMAT = (
+    'The requested operation will apply changes to {0} files which are not '
+    'currently open. This will therefore open {0} new files in the hidden '
+    'buffers. The quickfix list can then be used to review the changes. No '
+    'files will be written to disk. Do you wish to continue?' )
+
+
 def CurrentLineAndColumn():
   """Returns the 0-based current line and 0-based current column."""
   # See the comment in CurrentColumn about the calculation for the line and
@@ -236,6 +244,15 @@ def SetLocationList( diagnostics ):
   vim.eval( 'setloclist( 0, {0} )'.format( json.dumps( diagnostics ) ) )
 
 
+def SetQuickFixList( quickfix_list, display=False ):
+  """list should be in qflist format: see ":h setqflist" for details"""
+  vim.eval( 'setqflist( {0} )'.format( json.dumps( quickfix_list ) ) )
+
+  if display:
+    vim.command( 'copen {0}'.format( len( quickfix_list ) ) )
+    JumpToPreviousWindow()
+
+
 def ConvertDiagnosticsToQfList( diagnostics ):
   def ConvertDiagnosticToQfFormat( diagnostic ):
     # See :h getqflist for a description of the dictionary fields.
@@ -428,6 +445,8 @@ def PresentDialog( message, choices, default_choice_index = 0 ):
 
 
 def Confirm( message ):
+  """Display |message| with Ok/Cancel operations. Returns True if the user
+  selects Ok"""
   return bool( PresentDialog( message, [ "Ok", "Cancel" ] ) == 0 )
 
 
@@ -449,6 +468,7 @@ def EchoTextVimWidth( text ):
   old_ruler = GetIntValue( '&ruler' )
   old_showcmd = GetIntValue( '&showcmd' )
   vim.command( 'set noruler noshowcmd' )
+  vim.command( 'redraw' )
 
   EchoText( truncated_text, False )
 
@@ -490,9 +510,145 @@ def GetIntValue( variable ):
   return int( vim.eval( variable ) )
 
 
-def ReplaceChunksList( chunks, vim_buffer = None ):
-  if vim_buffer is None:
-    vim_buffer = vim.current.buffer
+def _SortChunksByFile( chunks ):
+  """Sort the members of the list |chunks| (which must be a list of dictionaries
+  conforming to ycmd.responses.FixItChunk) by their filepath. Returns a new
+  list in arbitrary order."""
+
+  chunks_by_file = defaultdict( list )
+
+  for chunk in chunks:
+    filepath = chunk[ 'range' ][ 'start' ][ 'filepath' ]
+    chunks_by_file[ filepath ].append( chunk )
+
+  return chunks_by_file
+
+
+def _GetNumNonVisibleFiles( file_list ):
+  """Returns the number of file in the iterable list of files |file_list| which
+  are not curerntly open in visible windows"""
+  return len(
+      [ f for f in file_list
+        if not BufferIsVisible( GetBufferNumberForFilename( f, False ) ) ] )
+
+
+def _OpenFileInSplitIfNeeded( filepath ):
+  """Ensure that the supplied filepath is open in a visible window, opening a
+  new split if required. Returns the buffer number of the file and an indication
+  of whether or not a new split was opened.
+
+  If the supplied filename is already open in a visible window, return just
+  return its buffer number. If the supplied file is not visible in a window
+  in the current tab, opens it in a new vertical split.
+
+  Returns a tuple of ( buffer_num, split_was_opened ) indicating the buffer
+  number and whether or not this method created a new split. If the user opts
+  not to open a file, or if opening fails, this method raises RuntimeError,
+  otherwise, guarantees to return a visible buffer number in buffer_num."""
+
+  buffer_num = GetBufferNumberForFilename( filepath, False )
+
+  # We only apply changes in the current tab page (i.e. "visible" windows).
+  # Applying changes in tabs does not lead to a better user experience, as the
+  # quickfix list no longer works as you might expect (doesn't jump into other
+  # tabs), and the complexity of choosing where to apply edits is significant.
+  if BufferIsVisible( buffer_num ):
+    # file is already open and visible, just return that buffer number (and an
+    # idicator that we *didn't* open a split)
+    return ( buffer_num, False )
+
+  # The file is not open in a visible window, so we open it in a split.
+  # We open the file with a small, fixed height. This means that we don't
+  # make the current buffer the smallest after a series of splits.
+  OpenFilename( filepath, {
+    'focus': True,
+    'fix': True,
+    'size': GetIntValue( '&previewheight' ),
+  } )
+
+  # OpenFilename returns us to the original cursor location. This is what we
+  # want, because we don't want to disorientate the user, but we do need to
+  # know the (now open) buffer number for the filename
+  buffer_num = GetBufferNumberForFilename( filepath, False )
+  if not BufferIsVisible( buffer_num ):
+    # This happens, for example, if there is a swap file and the user
+    # selects the "Quit" or "Abort" options. We just raise an exception to
+    # make it clear to the user that the abort has left potentially
+    # partially-applied changes.
+    raise RuntimeError(
+        'Unable to open file: {0}\nFixIt/Refactor operation '
+        'aborted prior to completion. Your files have not been '
+        'fully updated. Please use undo commands to revert the '
+        'applied changes.'.format( filepath ) )
+
+  # We opened this file in a split
+  return ( buffer_num, True )
+
+
+def ReplaceChunks( chunks ):
+  """Apply the source file deltas supplied in |chunks| to arbitrary files.
+  |chunks| is a list of changes defined by ycmd.responses.FixItChunk,
+  which may apply arbitrary modifications to arbitrary files.
+
+  If a file specified in a particular chunk is not currently open in a visible
+  buffer (i.e., one in a window visible in the current tab), we:
+    - issue a warning to the user that we're going to open new files (and offer
+      her the option to abort cleanly)
+    - open the file in a new split, make the changes, then hide the buffer.
+
+  If for some reason a file could not be opened or changed, raises RuntimeError.
+  Otherwise, returns no meaningful value."""
+
+  # We apply the edits file-wise for efficiency, and because we must track the
+  # file-wise offset deltas (caused by the modifications to the text).
+  chunks_by_file = _SortChunksByFile( chunks )
+
+  # We sort the file list simply to enable repeatable testing
+  sorted_file_list = sorted( chunks_by_file.iterkeys() )
+
+  # Make sure the user is prepared to have her screen mutilated by the new
+  # buffers
+  num_files_to_open = _GetNumNonVisibleFiles( sorted_file_list )
+
+  if num_files_to_open > 0:
+    if not Confirm(
+            FIXIT_OPENING_BUFFERS_MESSAGE_FORMAT.format( num_files_to_open ) ):
+      return
+
+  # Store the list of locations where we applied changes. We use this to display
+  # the quickfix window showing the user where we applied changes.
+  locations = []
+
+  for filepath in sorted_file_list:
+    ( buffer_num, close_window ) = _OpenFileInSplitIfNeeded( filepath )
+
+    ReplaceChunksInBuffer( chunks_by_file[ filepath ],
+                           vim.buffers[ buffer_num ],
+                           locations )
+
+    # When opening tons of files, we don't want to have a split for each new
+    # file, as this simply does not scale, so we open the window, make the
+    # edits, then hide the window.
+    if close_window:
+      # Some plugins (I'm looking at you, syntastic) might open a location list
+      # for the window we just opened. We don't want that location list hanging
+      # around, so we close it. lclose is a no-op if there is no location list.
+      vim.command( 'lclose' )
+
+      # Note that this doesn't lose our changes. It simply "hides" the buffer,
+      # which can later be re-accessed via the quickfix list or `:ls`
+      vim.command( 'hide' )
+
+  # Open the quickfix list, populated with entries for each location we changed.
+  if locations:
+    SetQuickFixList( locations, True )
+
+  EchoTextVimWidth( "Applied " + str( len( chunks ) ) + " changes" )
+
+
+def ReplaceChunksInBuffer( chunks, vim_buffer, locations ):
+  """Apply changes in |chunks| to the buffer-like object |buffer|. Append each
+  chunk's start to the list |locations|"""
 
   # We need to track the difference in length, but ensuring we apply fixes
   # in ascending order of insertion point.
@@ -519,7 +675,8 @@ def ReplaceChunksList( chunks, vim_buffer = None ):
       chunk[ 'range' ][ 'end' ],
       chunk[ 'replacement_text' ],
       line_delta, char_delta,
-      vim_buffer )
+      vim_buffer,
+      locations )
     line_delta += new_line_delta
     char_delta += new_char_delta
 
@@ -534,11 +691,12 @@ def ReplaceChunksList( chunks, vim_buffer = None ):
 # returns the delta (in lines and characters) that any position after the end
 # needs to be adjusted by.
 def ReplaceChunk( start, end, replacement_text, line_delta, char_delta,
-                  vim_buffer ):
+                  vim_buffer, locations = None ):
   # ycmd's results are all 1-based, but vim's/python's are all 0-based
   # (so we do -1 on all of the values)
   start_line = start[ 'line_num' ] - 1 + line_delta
   end_line = end[ 'line_num' ] - 1 + line_delta
+
   source_lines_count = end_line - start_line + 1
   start_column = start[ 'column_num' ] - 1 + char_delta
   end_column = end[ 'column_num' ] - 1
@@ -563,6 +721,17 @@ def ReplaceChunk( start, end, replacement_text, line_delta, char_delta,
 
   vim_buffer[ start_line : end_line + 1 ] = replacement_lines[:]
 
+  if locations is not None:
+    locations.append( {
+      'bufnr': vim_buffer.number,
+      'filename': vim_buffer.name,
+      # line and column numbers are 1-based in qflist
+      'lnum': start_line + 1,
+      'col': start_column + 1,
+      'text': replacement_text,
+      'type': 'F',
+    } )
+
   new_line_delta = replacement_lines_count - source_lines_count
   return ( new_line_delta, new_char_delta )
 
@@ -710,22 +879,58 @@ def OpenFilename( filename, options = {} ):
   size = ( options.get( 'size', '' ) if command in [ 'split', 'vsplit' ] else
            '' )
   focus = options.get( 'focus', False )
-  watch = options.get( 'watch', False )
-  position = options.get( 'position', 'start' )
 
   # There is no command in Vim to return to the previous tab so we need to
   # remember the current tab if needed.
   if not focus and command is 'tabedit':
     previous_tab = GetIntValue( 'tabpagenr()' )
+  else:
+    previous_tab = None
 
   # Open the file
   CheckFilename( filename )
-  vim.command( 'silent! {0}{1} {2}'.format( size, command, filename ) )
+  try:
+    vim.command( '{0}{1} {2}'.format( size, command, filename ) )
+  # When the file we are trying to jump to has a swap file,
+  # Vim opens swap-exists-choices dialog and throws vim.error with E325 error,
+  # or KeyboardInterrupt after user selects one of the options which actually
+  # opens the file (Open read-only/Edit anyway).
+  except vim.error as e:
+    if 'E325' not in str( e ):
+      raise
+
+    # Otherwise, the user might have chosen Quit. This is detectable by the
+    # current file not being the target file
+    if filename != GetCurrentBufferFilepath():
+      return
+  except KeyboardInterrupt:
+    # Raised when the user selects "Abort" after swap-exists-choices
+    return
+
+  _SetUpLoadedBuffer( command,
+                      filename,
+                      options.get( 'fix', False ),
+                      options.get( 'position', 'start' ),
+                      options.get( 'watch', False ) )
+
+  # Vim automatically set the focus to the opened file so we need to get the
+  # focus back (if the focus option is disabled) when opening a new tab or
+  # window.
+  if not focus:
+    if command is 'tabedit':
+      JumpToTab( previous_tab )
+    if command in [ 'split', 'vsplit' ]:
+      JumpToPreviousWindow()
+
+
+def _SetUpLoadedBuffer( command, filename, fix, position, watch ):
+  """After opening a buffer, configure it according to the supplied options,
+  which are as defined by the OpenFilename method."""
 
   if command is 'split':
-    vim.current.window.options[ 'winfixheight' ] = options.get( 'fix', False )
+    vim.current.window.options[ 'winfixheight' ] = fix
   if command is 'vsplit':
-    vim.current.window.options[ 'winfixwidth' ] = options.get( 'fix', False )
+    vim.current.window.options[ 'winfixwidth' ] = fix
 
   if watch:
     vim.current.buffer.options[ 'autoread' ] = True
@@ -735,11 +940,3 @@ def OpenFilename( filename, options = {} ):
   if position is 'end':
     vim.command( 'silent! normal G zz' )
 
-  # Vim automatically set the focus to the opened file so we need to get the
-  # focus back (if the focus option is disabled) when opening a new tab or
-  # window.
-  if not focus:
-    if command is 'tabedit':
-      JumpToTab( previous_tab )
-    if command in [ 'split', 'vsplit' ]:
-      JumpToPreviousWindow()