Auto merge of #2963 - micbou:refactor-completedone-logic, r=puremourning

[READY] Refactor CompleteDone logic

Since the `CompleteDone` code only depends on the completion request, it makes sense to move its logic to the `CompletionRequest` class. This is also going to help fixing the issue where a FixIt is applied twice when selecting a completion:

![completedone-bug](https://user-images.githubusercontent.com/10026824/38029281-bc09d224-3295-11e8-9976-d4dd031f2cca.gif)

Finally, this reduces the time it takes to run the tests because the ycmd server is not started anymore in the `postcomplete` tests.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2963)
<!-- Reviewable:end -->

python/ycm/client/completion_request.py

@@ -22,10 +22,12 @@ from __future__ import absolute_import
 # Not installing aliases from python-future; it's unreliable and slow.
 from builtins import *  # noqa
 
+from future.utils import iteritems
 from ycmd.utils import ToUnicode
 from ycm.client.base_request import ( BaseRequest, JsonFromFuture,
                                       HandleServerException,
                                       MakeServerException )
+from ycm import vimsupport
 
 
 class CompletionRequest( BaseRequest ):
@@ -34,6 +36,10 @@ class CompletionRequest( BaseRequest ):
     self.request_data = request_data
     self._response_future = None
     self._response = { 'completions': [], 'completion_start_column': -1 }
+    self._complete_done_hooks = {
+      'cs': self._OnCompleteDone_Csharp,
+      'java': self._OnCompleteDone_Java,
+    }
 
 
   def Start( self ):
@@ -69,7 +75,155 @@ class CompletionRequest( BaseRequest ):
     return response
 
 
-def ConvertCompletionDataToVimData( completion_identifier, completion_data ):
+  def OnCompleteDone( self ):
+    if not self.Done():
+      return
+
+    complete_done_actions = self._GetCompleteDoneHooks()
+    for action in complete_done_actions:
+      action()
+
+
+  def _GetCompleteDoneHooks( self ):
+    filetypes = vimsupport.CurrentFiletypes()
+    for key, value in iteritems( self._complete_done_hooks ):
+      if key in filetypes:
+        yield value
+
+
+  def _GetCompletionsUserMayHaveCompleted( self ):
+    completed_item = vimsupport.GetVariableValue( 'v:completed_item' )
+    completions = self.RawResponse()[ 'completions' ]
+
+    if 'user_data' in completed_item and completed_item[ 'user_data' ]:
+      # Vim supports user_data (8.0.1493) or later, so we actually know the
+      # _exact_ element that was selected, having put its index in the
+      # user_data field.
+      return [ completions[ int( completed_item[ 'user_data' ] ) ] ]
+
+    # Otherwise, we have to guess by matching the values in the completed item
+    # and the list of completions. Sometimes this returns multiple
+    # possibilities, which is essentially unresolvable.
+
+    result = _FilterToMatchingCompletions( completed_item, completions, True )
+    result = list( result )
+
+    if result:
+      return result
+
+    if _HasCompletionsThatCouldBeCompletedWithMoreText( completed_item,
+                                                        completions ):
+      # Since the way that YCM works leads to CompleteDone called on every
+      # character, return blank if the completion might not be done. This won't
+      # match if the completion is ended with typing a non-keyword character.
+      return []
+
+    result = _FilterToMatchingCompletions( completed_item, completions, False )
+
+    return list( result )
+
+
+  def _OnCompleteDone_Csharp( self ):
+    completions = self._GetCompletionsUserMayHaveCompleted()
+    namespaces = [ _GetRequiredNamespaceImport( c ) for c in completions ]
+    namespaces = [ n for n in namespaces if n ]
+    if not namespaces:
+      return
+
+    if len( namespaces ) > 1:
+      choices = [ "{0} {1}".format( i + 1, n )
+                  for i, n in enumerate( namespaces ) ]
+      choice = vimsupport.PresentDialog( "Insert which namespace:", choices )
+      if choice < 0:
+        return
+      namespace = namespaces[ choice ]
+    else:
+      namespace = namespaces[ 0 ]
+
+    vimsupport.InsertNamespace( namespace )
+
+
+  def _OnCompleteDone_Java( self ):
+    completions = self._GetCompletionsUserMayHaveCompleted()
+    fixit_completions = [ _GetFixItCompletion( c ) for c in completions ]
+    fixit_completions = [ f for f in fixit_completions if f ]
+    if not fixit_completions:
+      return
+
+    # If we have user_data in completions (8.0.1493 or later), then we would
+    # only ever return max. 1 completion here. However, if we had to guess, it
+    # is possible that we matched multiple completion items (e.g. for overloads,
+    # or similar classes in multiple packages). In any case, rather than
+    # prompting the user and disturbing her workflow, we just apply the first
+    # one. This might be wrong, but the solution is to use a (very) new version
+    # of Vim which supports user_data on completion items
+    fixit_completion = fixit_completions[ 0 ]
+
+    for fixit in fixit_completion:
+      vimsupport.ReplaceChunks( fixit[ 'chunks' ], silent=True )
+
+
+def _GetRequiredNamespaceImport( completion ):
+  if ( 'extra_data' not in completion
+       or 'required_namespace_import' not in completion[ 'extra_data' ] ):
+    return None
+  return completion[ 'extra_data' ][ 'required_namespace_import' ]
+
+
+def _GetFixItCompletion( completion ):
+  if ( 'extra_data' not in completion
+       or 'fixits' not in completion[ 'extra_data' ] ):
+    return None
+
+  return completion[ 'extra_data' ][ 'fixits' ]
+
+
+def _FilterToMatchingCompletions( completed_item,
+                                  completions,
+                                  full_match_only ):
+  """Filter to completions matching the item Vim said was completed"""
+  match_keys = ( [ 'word', 'abbr', 'menu', 'info' ] if full_match_only
+                  else [ 'word' ] )
+
+  for index, completion in enumerate( completions ):
+    item = _ConvertCompletionDataToVimData( index, completion )
+
+    def matcher( key ):
+      return ( ToUnicode( completed_item.get( key, "" ) ) ==
+               ToUnicode( item.get( key, "" ) ) )
+
+    if all( [ matcher( i ) for i in match_keys ] ):
+      yield completion
+
+
+def _HasCompletionsThatCouldBeCompletedWithMoreText( completed_item,
+                                                     completions ):
+  if not completed_item:
+    return False
+
+  completed_word = ToUnicode( completed_item[ 'word' ] )
+  if not completed_word:
+    return False
+
+  # Sometimes CompleteDone is called after the next character is inserted.
+  # If so, use inserted character to filter possible completions further.
+  text = vimsupport.TextBeforeCursor()
+  reject_exact_match = True
+  if text and text[ -1 ] != completed_word[ -1 ]:
+    reject_exact_match = False
+    completed_word += text[ -1 ]
+
+  for index, completion in enumerate( completions ):
+    word = ToUnicode(
+        _ConvertCompletionDataToVimData( index, completion )[ 'word' ] )
+    if reject_exact_match and word == completed_word:
+      continue
+    if word.startswith( completed_word ):
+      return True
+  return False
+
+
+def _ConvertCompletionDataToVimData( completion_identifier, completion_data ):
   # see :h complete-items for a description of the dictionary fields
   vim_data = {
     'word'  : '',
@@ -114,5 +268,5 @@ def ConvertCompletionDataToVimData( completion_identifier, completion_data ):
 
 
 def _ConvertCompletionDatasToVimDatas( response_data ):
-  return [ ConvertCompletionDataToVimData( i, x )
+  return [ _ConvertCompletionDataToVimData( i, x )
            for i, x in enumerate( response_data ) ]

python/ycm/tests/client/completion_request_test.py

@@ -34,7 +34,7 @@ class ConvertCompletionResponseToVimDatas_test( object ):
       completion_request._ConvertCompletionResponseToVimDatas method """
 
   def _Check( self, completion_id, completion_data, expected_vim_data ):
-    vim_data = completion_request.ConvertCompletionDataToVimData(
+    vim_data = completion_request._ConvertCompletionDataToVimData(
         completion_id,
         completion_data )
 

python/ycm/tests/postcomplete_test.py

@@ -28,16 +28,16 @@ from ycm.tests.test_utils import MockVimModule
 MockVimModule()
 
 import contextlib
-from hamcrest import assert_that, empty
 from mock import MagicMock, DEFAULT, patch
 from nose.tools import eq_, ok_
 
 from ycm import vimsupport
-from ycm.tests import YouCompleteMeInstance
 from ycmd.utils import ToBytes
-
+from ycm.client.completion_request import (
-from ycm.youcompleteme import _CompleteDoneHook_CSharp
+  CompletionRequest,
-from ycm.youcompleteme import _CompleteDoneHook_Java
+  _FilterToMatchingCompletions,
+  _GetRequiredNamespaceImport,
+  _HasCompletionsThatCouldBeCompletedWithMoreText )
 
 
 def CompleteItemIs( word, abbr = None, menu = None,
@@ -114,390 +114,356 @@ def BuildCompletionFixIt( fixits,
 
 
 @contextlib.contextmanager
-def _SetupForCsharpCompletionDone( ycm, completions ):
+def _SetupForCsharpCompletionDone( completions ):
   with patch( 'ycm.vimsupport.InsertNamespace' ):
-    with _SetUpCompleteDone( ycm, completions ):
+    with _SetUpCompleteDone( completions ) as request:
-      yield
+      yield request
 
 
 @contextlib.contextmanager
-def _SetUpCompleteDone( ycm, completions ):
+def _SetUpCompleteDone( completions ):
   with patch( 'ycm.vimsupport.TextBeforeCursor', return_value = '   Test' ):
-    request = MagicMock()
+    request = CompletionRequest( None )
     request.Done = MagicMock( return_value = True )
     request.RawResponse = MagicMock( return_value = {
       'completions': completions
     } )
-    ycm._latest_completion_request = request
+    yield request
-    yield
 
 
 @patch( 'ycm.vimsupport.CurrentFiletypes', return_value = [ 'cs' ] )
-@YouCompleteMeInstance()
+def GetCompleteDoneHooks_ResultOnCsharp_test( *args ):
-def GetCompleteDoneHooks_ResultOnCsharp_test( ycm, *args ):
+  request = CompletionRequest( None )
-  result = list( ycm.GetCompleteDoneHooks() )
+  result = list( request._GetCompleteDoneHooks() )
-  eq_( [ _CompleteDoneHook_CSharp ], result )
+  eq_( result, [ request._OnCompleteDone_Csharp ] )
 
 
 @patch( 'ycm.vimsupport.CurrentFiletypes', return_value = [ 'java' ] )
-@YouCompleteMeInstance()
+def GetCompleteDoneHooks_ResultOnJava_test( *args ):
-def GetCompleteDoneHooks_ResultOnJava_test( ycm, *args ):
+  request = CompletionRequest( None )
-  result = list( ycm.GetCompleteDoneHooks() )
+  result = list( request._GetCompleteDoneHooks() )
-  eq_( [ _CompleteDoneHook_Java ], result )
+  eq_( result, [ request._OnCompleteDone_Java ] )
 
 
 @patch( 'ycm.vimsupport.CurrentFiletypes', return_value = [ 'ycmtest' ] )
-@YouCompleteMeInstance()
+def GetCompleteDoneHooks_EmptyOnOtherFiletype_test( *args ):
-def GetCompleteDoneHooks_EmptyOnOtherFiletype_test( ycm, *args ):
+  request = CompletionRequest( None )
-  result = ycm.GetCompleteDoneHooks()
+  result = request._GetCompleteDoneHooks()
-  eq_( 0, len( list( result ) ) )
+  eq_( len( list( result ) ), 0 )
 
 
 @patch( 'ycm.vimsupport.CurrentFiletypes', return_value = [ 'ycmtest' ] )
-@YouCompleteMeInstance()
+def OnCompleteDone_WithActionCallsIt_test( *args ):
-def OnCompleteDone_WithActionCallsIt_test( ycm, *args ):
+  request = CompletionRequest( None )
+  request.Done = MagicMock( return_value = True )
   action = MagicMock()
-  ycm._complete_done_hooks[ 'ycmtest' ] = action
+  request._complete_done_hooks[ 'ycmtest' ] = action
-  ycm.OnCompleteDone()
+  request.OnCompleteDone()
   ok_( action.called )
 
 
 @patch( 'ycm.vimsupport.CurrentFiletypes', return_value = [ 'ycmtest' ] )
-@YouCompleteMeInstance()
+def OnCompleteDone_NoActionNoError_test( *args ):
-def OnCompleteDone_NoActionNoError_test( ycm, *args ):
+  request = CompletionRequest( None )
-  with patch.object( ycm, '_OnCompleteDone_Csharp' ) as csharp:
+  request.Done = MagicMock( return_value = True )
-    with patch.object( ycm, '_OnCompleteDone_Java' ) as java:
+  request._OnCompleteDone_Csharp = MagicMock()
-      ycm.OnCompleteDone()
+  request._OnCompleteDone_Java = MagicMock()
-      csharp.assert_not_called()
+  request.OnCompleteDone()
-      java.assert_not_called()
+  request._OnCompleteDone_Csharp.assert_not_called()
+  request._OnCompleteDone_Java.assert_not_called()
 
 
-@YouCompleteMeInstance()
+@patch( 'ycm.vimsupport.CurrentFiletypes', return_value = [ 'ycmtest' ] )
-def FilterToCompletedCompletions_MatchIsReturned_test( ycm, *args ):
+def OnCompleteDone_NoActionIfNotDone_test( *args ):
+  request = CompletionRequest( None )
+  request.Done = MagicMock( return_value = False )
+  action = MagicMock()
+  request._complete_done_hooks[ 'ycmtest' ] = action
+  request.OnCompleteDone()
+  action.assert_not_called()
+
+
+def FilterToCompletedCompletions_MatchIsReturned_test():
   completions = [ BuildCompletion( insertion_text = 'Test' ) ]
-  result = ycm._FilterToMatchingCompletions( CompleteItemIs( 'Test' ),
+  result = _FilterToMatchingCompletions( CompleteItemIs( 'Test' ),
                                          completions,
                                          False )
   eq_( list( result ), completions )
 
 
-@YouCompleteMeInstance()
+def FilterToCompletedCompletions_ShortTextDoesntRaise_test():
-def FilterToCompletedCompletions_ShortTextDoesntRaise_test( ycm, *args ):
   completions = [ BuildCompletion( insertion_text = 'AAA' ) ]
-  ycm._FilterToMatchingCompletions( CompleteItemIs( 'A' ),
+  result = _FilterToMatchingCompletions( CompleteItemIs( 'A' ),
                                          completions,
                                          False )
+  eq_( list( result ), [] )
 
 
-@YouCompleteMeInstance()
+def FilterToCompletedCompletions_ExactMatchIsReturned_test():
-def FilterToCompletedCompletions_ExactMatchIsReturned_test( ycm, *args ):
   completions = [ BuildCompletion( insertion_text = 'Test' ) ]
-  result = ycm._FilterToMatchingCompletions( CompleteItemIs( 'Test' ),
+  result = _FilterToMatchingCompletions( CompleteItemIs( 'Test' ),
                                          completions,
                                          False )
   eq_( list( result ), completions )
 
 
-@YouCompleteMeInstance()
+def FilterToCompletedCompletions_NonMatchIsntReturned_test():
-def FilterToCompletedCompletions_NonMatchIsntReturned_test( ycm, *args ):
   completions = [ BuildCompletion( insertion_text = 'A' ) ]
-  result = ycm._FilterToMatchingCompletions( CompleteItemIs( '   Quote' ),
+  result = _FilterToMatchingCompletions( CompleteItemIs( '   Quote' ),
                                          completions,
                                          False )
-  assert_that( list( result ), empty() )
+  eq_( list( result ), [] )
 
 
-@YouCompleteMeInstance()
+def FilterToCompletedCompletions_Unicode_test():
-def FilterToCompletedCompletions_Unicode_test( ycm, *args ):
   completions = [ BuildCompletion( insertion_text = '†es†' ) ]
-  result = ycm._FilterToMatchingCompletions( CompleteItemIs( '†es†' ),
+  result = _FilterToMatchingCompletions( CompleteItemIs( '†es†' ),
                                          completions,
                                          False )
   eq_( list( result ), completions )
 
 
 @patch( 'ycm.vimsupport.TextBeforeCursor', return_value = '   Quote' )
-@YouCompleteMeInstance()
 def HasCompletionsThatCouldBeCompletedWithMoreText_MatchIsReturned_test(
-    ycm, *args ):
+    *args ):
   completions = [ BuildCompletion( insertion_text = 'Test' ) ]
-  result = ycm._HasCompletionsThatCouldBeCompletedWithMoreText(
+  ok_( _HasCompletionsThatCouldBeCompletedWithMoreText( CompleteItemIs( 'Te' ),
-    CompleteItemIs( 'Te' ),
+                                                        completions ) )
-    completions )
-  eq_( result, True )
 
 
 @patch( 'ycm.vimsupport.TextBeforeCursor', return_value = '   Quote' )
-@YouCompleteMeInstance()
 def HasCompletionsThatCouldBeCompletedWithMoreText_ShortTextDoesntRaise_test(
-    ycm, *args ):
+    *args ):
   completions = [ BuildCompletion( insertion_text = 'AAA' ) ]
-  ycm._HasCompletionsThatCouldBeCompletedWithMoreText( CompleteItemIs( 'X' ),
+  ok_( not _HasCompletionsThatCouldBeCompletedWithMoreText(
-                                                       completions )
+    CompleteItemIs( 'X' ),
+    completions ) )
 
 
 @patch( 'ycm.vimsupport.TextBeforeCursor', return_value = '   Quote' )
-@YouCompleteMeInstance()
 def HasCompletionsThatCouldBeCompletedWithMoreText_ExactMatchIsntReturned_test(
-    ycm, *args ):
+    *args ):
   completions = [ BuildCompletion( insertion_text = 'Test' ) ]
-  result = ycm._HasCompletionsThatCouldBeCompletedWithMoreText(
+  ok_( not _HasCompletionsThatCouldBeCompletedWithMoreText(
     CompleteItemIs( 'Test' ),
-    completions )
+    completions ) )
-  eq_( result, False )
 
 
 @patch( 'ycm.vimsupport.TextBeforeCursor', return_value = '   Quote' )
-@YouCompleteMeInstance()
 def HasCompletionsThatCouldBeCompletedWithMoreText_NonMatchIsntReturned_test(
-    ycm, *args ):
+    *args ):
   completions = [ BuildCompletion( insertion_text = "A" ) ]
-  result = ycm._HasCompletionsThatCouldBeCompletedWithMoreText(
+  ok_( not _HasCompletionsThatCouldBeCompletedWithMoreText(
     CompleteItemIs( '   Quote' ),
-    completions )
+    completions ) )
-  eq_( result, False )
 
 
 @patch( 'ycm.vimsupport.TextBeforeCursor', return_value = 'Uniç' )
-@YouCompleteMeInstance()
 def HasCompletionsThatCouldBeCompletedWithMoreText_Unicode_test(
-    ycm, *args ):
+    *args ):
   completions = [ BuildCompletion( insertion_text = 'Uniçø∂¢' ) ]
-  result = ycm._HasCompletionsThatCouldBeCompletedWithMoreText(
+  ok_( _HasCompletionsThatCouldBeCompletedWithMoreText(
     CompleteItemIs( 'Uniç' ),
-    completions )
+    completions ) )
-  eq_( result, True )
 
 
-@YouCompleteMeInstance()
+def GetRequiredNamespaceImport_ReturnNoneForNoExtraData_test():
-def GetRequiredNamespaceImport_ReturnNoneForNoExtraData_test( ycm ):
+  eq_( _GetRequiredNamespaceImport( {} ), None )
-  eq_( None, ycm._GetRequiredNamespaceImport( {} ) )
 
 
-@YouCompleteMeInstance()
+def GetRequiredNamespaceImport_ReturnNamespaceFromExtraData_test():
-def GetRequiredNamespaceImport_ReturnNamespaceFromExtraData_test( ycm ):
   namespace = 'A_NAMESPACE'
-  eq_( namespace, ycm._GetRequiredNamespaceImport(
+  eq_( _GetRequiredNamespaceImport( BuildCompletionNamespace( namespace ) ),
-    BuildCompletionNamespace( namespace )
+       namespace )
-  ) )
-
-
-@YouCompleteMeInstance()
-def GetCompletionsUserMayHaveCompleted_ReturnEmptyIfNotDone_test( ycm ):
-  with _SetupForCsharpCompletionDone( ycm, [] ):
-    ycm._latest_completion_request.Done = MagicMock( return_value = False )
-    eq_( [], ycm.GetCompletionsUserMayHaveCompleted() )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Te' ) )
-@YouCompleteMeInstance()
 def GetCompletionsUserMayHaveCompleted_ReturnEmptyIfPendingMatches_test(
-    ycm, *args ):
+    *args ):
   completions = [ BuildCompletionNamespace( None ) ]
-  with _SetupForCsharpCompletionDone( ycm, completions ):
+  with _SetupForCsharpCompletionDone( completions ) as request:
-    eq_( [], ycm.GetCompletionsUserMayHaveCompleted() )
+    eq_( request._GetCompletionsUserMayHaveCompleted(), [] )
 
 
-@YouCompleteMeInstance()
+def GetCompletionsUserMayHaveCompleted_ReturnMatchIfExactMatches_test( *args ):
-def GetCompletionsUserMayHaveCompleted_ReturnMatchIfExactMatches_test(
-    ycm, *args ):
   info = [ 'NS', 'Test', 'Abbr', 'Menu', 'Info', 'Kind' ]
   completions = [ BuildCompletionNamespace( *info ) ]
-  with _SetupForCsharpCompletionDone( ycm, completions ):
+  with _SetupForCsharpCompletionDone( completions ) as request:
     with patch( 'ycm.vimsupport.GetVariableValue',
                 GetVariableValue_CompleteItemIs( *info[ 1: ] ) ):
-      eq_( completions, ycm.GetCompletionsUserMayHaveCompleted() )
+      eq_( request._GetCompletionsUserMayHaveCompleted(), completions )
 
 
-@YouCompleteMeInstance()
+def GetCompletionsUserMayHaveCompleted_ReturnMatchIfExactMatchesEvenIfPartial_test(): # noqa
-def GetCompletionsUserMayHaveCompleted_ReturnMatchIfExactMatchesEvenIfPartial_test( # noqa
-    ycm, *args ):
   info = [ 'NS', 'Test', 'Abbr', 'Menu', 'Info', 'Kind' ]
   completions = [ BuildCompletionNamespace( *info ),
                   BuildCompletion( insertion_text = 'TestTest' ) ]
-  with _SetupForCsharpCompletionDone( ycm, completions ):
+  with _SetupForCsharpCompletionDone( completions ) as request:
     with patch( 'ycm.vimsupport.GetVariableValue',
                 GetVariableValue_CompleteItemIs( *info[ 1: ] ) ):
-      eq_( [ completions[ 0 ] ], ycm.GetCompletionsUserMayHaveCompleted() )
+      eq_( request._GetCompletionsUserMayHaveCompleted(), [ completions[ 0 ] ] )
 
 
-@YouCompleteMeInstance()
+def GetCompletionsUserMayHaveCompleted_DontReturnMatchIfNoExactMatchesAndPartial_test(): # noqa
-def GetCompletionsUserMayHaveCompleted_DontReturnMatchIfNoExactMatchesAndPartial_test( # noqa
-    ycm, *args ):
   info = [ 'NS', 'Test', 'Abbr', 'Menu', 'Info', 'Kind' ]
   completions = [ BuildCompletion( insertion_text = info[ 0 ] ),
                   BuildCompletion( insertion_text = 'TestTest' ) ]
-  with _SetupForCsharpCompletionDone( ycm, completions ):
+  with _SetupForCsharpCompletionDone( completions ) as request:
     with patch( 'ycm.vimsupport.GetVariableValue',
                 GetVariableValue_CompleteItemIs( *info[ 1: ] ) ):
-      eq_( [], ycm.GetCompletionsUserMayHaveCompleted() )
+      eq_( request._GetCompletionsUserMayHaveCompleted(), [] )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test' ) )
-@YouCompleteMeInstance()
+def GetCompletionsUserMayHaveCompleted_ReturnMatchIfMatches_test( *args ):
-def GetCompletionsUserMayHaveCompleted_ReturnMatchIfMatches_test( ycm, *args ):
   completions = [ BuildCompletionNamespace( None ) ]
-  with _SetupForCsharpCompletionDone( ycm, completions ):
+  with _SetupForCsharpCompletionDone( completions ) as request:
-    eq_( completions, ycm.GetCompletionsUserMayHaveCompleted() )
+    eq_( request._GetCompletionsUserMayHaveCompleted(), completions )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test', user_data='0' ) )
-@YouCompleteMeInstance()
+def GetCompletionsUserMayHaveCompleted_UseUserData0_test( *args ):
-def GetCompletionsUserMayHaveCompleted_UseUserData0_test( ycm, *args ):
+  # Identical completions but we specify the first one via user_data.
-  # identical completions but we specify the first one via user_data
   completions = [
     BuildCompletionNamespace( 'namespace1' ),
     BuildCompletionNamespace( 'namespace2' )
   ]
 
-  with _SetupForCsharpCompletionDone( ycm, completions ):
+  with _SetupForCsharpCompletionDone( completions ) as request:
-    eq_( [ BuildCompletionNamespace( 'namespace1' ) ],
+    eq_( request._GetCompletionsUserMayHaveCompleted(),
-         ycm.GetCompletionsUserMayHaveCompleted() )
+         [ BuildCompletionNamespace( 'namespace1' ) ] )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test', user_data='1' ) )
-@YouCompleteMeInstance()
+def GetCompletionsUserMayHaveCompleted_UseUserData1_test( *args ):
-def GetCompletionsUserMayHaveCompleted_UseUserData1_test( ycm, *args ):
+  # Identical completions but we specify the second one via user_data.
-  # identical completions but we specify the second one via user_data
   completions = [
     BuildCompletionNamespace( 'namespace1' ),
     BuildCompletionNamespace( 'namespace2' )
   ]
 
-  with _SetupForCsharpCompletionDone( ycm, completions ):
+  with _SetupForCsharpCompletionDone( completions ) as request:
-    eq_( [ BuildCompletionNamespace( 'namespace2' ) ],
+    eq_( request._GetCompletionsUserMayHaveCompleted(),
-         ycm.GetCompletionsUserMayHaveCompleted() )
+         [ BuildCompletionNamespace( 'namespace2' ) ] )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test' ) )
-@YouCompleteMeInstance()
+def PostCompleteCsharp_EmptyDoesntInsertNamespace_test( *args ):
-def PostCompleteCsharp_EmptyDoesntInsertNamespace_test( ycm, *args ):
+  with _SetupForCsharpCompletionDone( [] ) as request:
-  with _SetupForCsharpCompletionDone( ycm, [] ):
+    request._OnCompleteDone_Csharp()
-    ycm._OnCompleteDone_Csharp()
     ok_( not vimsupport.InsertNamespace.called )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test' ) )
-@YouCompleteMeInstance()
 def PostCompleteCsharp_ExistingWithoutNamespaceDoesntInsertNamespace_test(
-    ycm, *args ):
+    *args ):
   completions = [ BuildCompletionNamespace( None ) ]
-  with _SetupForCsharpCompletionDone( ycm, completions ):
+  with _SetupForCsharpCompletionDone( completions ) as request:
-    ycm._OnCompleteDone_Csharp()
+    request._OnCompleteDone_Csharp()
     ok_( not vimsupport.InsertNamespace.called )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test' ) )
-@YouCompleteMeInstance()
+def PostCompleteCsharp_ValueDoesInsertNamespace_test( *args ):
-def PostCompleteCsharp_ValueDoesInsertNamespace_test( ycm, *args ):
   namespace = 'A_NAMESPACE'
   completions = [ BuildCompletionNamespace( namespace ) ]
-  with _SetupForCsharpCompletionDone( ycm, completions ):
+  with _SetupForCsharpCompletionDone( completions ) as request:
-    ycm._OnCompleteDone_Csharp()
+    request._OnCompleteDone_Csharp()
     vimsupport.InsertNamespace.assert_called_once_with( namespace )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test' ) )
 @patch( 'ycm.vimsupport.PresentDialog', return_value = 1 )
-@YouCompleteMeInstance()
+def PostCompleteCsharp_InsertSecondNamespaceIfSelected_test( *args ):
-def PostCompleteCsharp_InsertSecondNamespaceIfSelected_test( ycm, *args ):
   namespace = 'A_NAMESPACE'
   namespace2 = 'ANOTHER_NAMESPACE'
   completions = [
     BuildCompletionNamespace( namespace ),
     BuildCompletionNamespace( namespace2 ),
   ]
-  with _SetupForCsharpCompletionDone( ycm, completions ):
+  with _SetupForCsharpCompletionDone( completions ) as request:
-    ycm._OnCompleteDone_Csharp()
+    request._OnCompleteDone_Csharp()
     vimsupport.InsertNamespace.assert_called_once_with( namespace2 )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test' ) )
 @patch( 'ycm.vimsupport.ReplaceChunks' )
-@YouCompleteMeInstance()
+def PostCompleteJava_ApplyFixIt_NoFixIts_test( replace_chunks, *args ):
-def PostCompleteJava_ApplyFixIt_NoFixIts_test( ycm, replace_chunks, *args ):
   completions = [
     BuildCompletionFixIt( [] )
   ]
-  with _SetUpCompleteDone( ycm, completions ):
+  with _SetUpCompleteDone( completions ) as request:
-    ycm._OnCompleteDone_Java()
+    request._OnCompleteDone_Java()
     replace_chunks.assert_not_called()
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test' ) )
 @patch( 'ycm.vimsupport.ReplaceChunks' )
-@YouCompleteMeInstance()
+def PostCompleteJava_ApplyFixIt_EmptyFixIt_test( replace_chunks, *args ):
-def PostCompleteJava_ApplyFixIt_EmptyFixIt_test( ycm, replace_chunks, *args ):
   completions = [
     BuildCompletionFixIt( [ { 'chunks': [] } ] )
   ]
-  with _SetUpCompleteDone( ycm, completions ):
+  with _SetUpCompleteDone( completions ) as request:
-    ycm._OnCompleteDone_Java()
+    request._OnCompleteDone_Java()
     replace_chunks.assert_called_once_with( [], silent = True )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test' ) )
 @patch( 'ycm.vimsupport.ReplaceChunks' )
-@YouCompleteMeInstance()
+def PostCompleteJava_ApplyFixIt_NoFixIt_test( replace_chunks, *args ):
-def PostCompleteJava_ApplyFixIt_NoFixIt_test( ycm, replace_chunks, *args ):
   completions = [
     BuildCompletion( )
   ]
-  with _SetUpCompleteDone( ycm, completions ):
+  with _SetUpCompleteDone( completions ) as request:
-    ycm._OnCompleteDone_Java()
+    request._OnCompleteDone_Java()
     replace_chunks.assert_not_called()
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test' ) )
 @patch( 'ycm.vimsupport.ReplaceChunks' )
-@YouCompleteMeInstance()
+def PostCompleteJava_ApplyFixIt_PickFirst_test( replace_chunks, *args ):
-def PostCompleteJava_ApplyFixIt_PickFirst_test( ycm, replace_chunks, *args ):
   completions = [
     BuildCompletionFixIt( [ { 'chunks': 'one' } ] ),
     BuildCompletionFixIt( [ { 'chunks': 'two' } ] ),
   ]
-  with _SetUpCompleteDone( ycm, completions ):
+  with _SetUpCompleteDone( completions ) as request:
-    ycm._OnCompleteDone_Java()
+    request._OnCompleteDone_Java()
     replace_chunks.assert_called_once_with( 'one', silent = True )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test', user_data='0' ) )
 @patch( 'ycm.vimsupport.ReplaceChunks' )
-@YouCompleteMeInstance()
+def PostCompleteJava_ApplyFixIt_PickFirstUserData_test( replace_chunks, *args ):
-def PostCompleteJava_ApplyFixIt_PickFirstUserData_test( ycm,
-                                                        replace_chunks,
-                                                        *args ):
   completions = [
     BuildCompletionFixIt( [ { 'chunks': 'one' } ] ),
     BuildCompletionFixIt( [ { 'chunks': 'two' } ] ),
   ]
-  with _SetUpCompleteDone( ycm, completions ):
+  with _SetUpCompleteDone( completions ) as request:
-    ycm._OnCompleteDone_Java()
+    request._OnCompleteDone_Java()
     replace_chunks.assert_called_once_with( 'one', silent = True )
 
 
 @patch( 'ycm.vimsupport.GetVariableValue',
         GetVariableValue_CompleteItemIs( 'Test', user_data='1' ) )
 @patch( 'ycm.vimsupport.ReplaceChunks' )
-@YouCompleteMeInstance()
+def PostCompleteJava_ApplyFixIt_PickSecond_test( replace_chunks, *args ):
-def PostCompleteJava_ApplyFixIt_PickSecond_test( ycm, replace_chunks, *args ):
   completions = [
     BuildCompletionFixIt( [ { 'chunks': 'one' } ] ),
     BuildCompletionFixIt( [ { 'chunks': 'two' } ] ),
   ]
-  with _SetUpCompleteDone( ycm, completions ):
+  with _SetUpCompleteDone( completions ) as request:
-    ycm._OnCompleteDone_Java()
+    request._OnCompleteDone_Java()
     replace_chunks.assert_called_once_with( 'two', silent = True )

python/ycm/tests/youcompleteme_test.py

@@ -1,4 +1,4 @@
-# Copyright (C) 2016-2017 YouCompleteMe contributors
+# Copyright (C) 2016-2018 YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -1039,3 +1039,22 @@ def YouCompleteMe_OnPeriodicTick_ValidResponse_test( ycm,
     mock_future.result.assert_called()
     post_data_to_handler_async.assert_called() # Poll again!
     assert_that( ycm._message_poll_request is not None )
+
+
+@YouCompleteMeInstance()
+@patch( 'ycm.client.completion_request.CompletionRequest.OnCompleteDone' )
+def YouCompleteMe_OnCompleteDone_CompletionRequest_test( ycm,
+                                                         on_complete_done ):
+  current_buffer = VimBuffer( 'current_buffer' )
+  with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 1 ) ):
+    ycm.SendCompletionRequest()
+  ycm.OnCompleteDone()
+  on_complete_done.assert_called()
+
+
+@YouCompleteMeInstance()
+@patch( 'ycm.client.completion_request.CompletionRequest.OnCompleteDone' )
+def YouCompleteMe_OnCompleteDone_NoCompletionRequest_test( ycm,
+                                                           on_complete_done ):
+  ycm.OnCompleteDone()
+  on_complete_done.assert_not_called()

python/ycm/youcompleteme.py

@@ -45,8 +45,7 @@ from ycm.client.base_request import ( BaseRequest, BuildRequestData,
                                       HandleServerException )
 from ycm.client.completer_available_request import SendCompleterAvailableRequest
 from ycm.client.command_request import SendCommandRequest
-from ycm.client.completion_request import ( CompletionRequest,
+from ycm.client.completion_request import CompletionRequest
-                                            ConvertCompletionDataToVimData )
 from ycm.client.debug_info_request import ( SendDebugInfoRequest,
                                             FormatDebugInfoResponse )
 from ycm.client.omni_completion_request import OmniCompletionRequest
@@ -108,15 +107,6 @@ SERVER_LOGFILE_FORMAT = 'ycmd_{port}_{std}_'
 HANDLE_FLAG_INHERIT = 0x00000001
 
 
-# The following two methods exist for testability only
-def _CompleteDoneHook_CSharp( ycm ):
-  ycm._OnCompleteDone_Csharp()
-
-
-def _CompleteDoneHook_Java( ycm ):
-  ycm._OnCompleteDone_Java()
-
-
 class YouCompleteMe( object ):
   def __init__( self, user_options ):
     self._available_completers = {}
@@ -136,10 +126,6 @@ class YouCompleteMe( object ):
     self._SetUpLogging()
     self._SetUpServer()
     self._ycmd_keepalive.Start()
-    self._complete_done_hooks = {
-      'cs': _CompleteDoneHook_CSharp,
-      'java': _CompleteDoneHook_Java,
-    }
 
 
   def _SetUpServer( self ):
@@ -500,160 +486,11 @@ class YouCompleteMe( object ):
 
 
   def OnCompleteDone( self ):
-    complete_done_actions = self.GetCompleteDoneHooks()
+    completion_request = self.GetCurrentCompletionRequest()
-    for action in complete_done_actions:
+    if completion_request:
-      action(self)
+      completion_request.OnCompleteDone()
 
 
-  def GetCompleteDoneHooks( self ):
-    filetypes = vimsupport.CurrentFiletypes()
-    for key, value in iteritems( self._complete_done_hooks ):
-      if key in filetypes:
-        yield value
-
-
-  def GetCompletionsUserMayHaveCompleted( self ):
-    latest_completion_request = self.GetCurrentCompletionRequest()
-    if not latest_completion_request or not latest_completion_request.Done():
-      return []
-
-    completed_item = vimsupport.GetVariableValue( 'v:completed_item' )
-    completions = latest_completion_request.RawResponse()[ 'completions' ]
-
-    if 'user_data' in completed_item and completed_item[ 'user_data' ] != '':
-      # Vim supports user_data (8.0.1493) or later, so we actually know the
-      # _exact_ element that was selected, having put its index in the user_data
-      # field.
-      return [ completions[ int( completed_item[ 'user_data' ] ) ] ]
-
-    # Otherwise, we have to guess by matching the values in the completed item
-    # and the list of completions. Sometimes this returns multiple
-    # possibilities, which is essentially unresolvable.
-
-    result = self._FilterToMatchingCompletions( completed_item,
-                                                completions,
-                                                True )
-    result = list( result )
-
-    if result:
-      return result
-
-    if self._HasCompletionsThatCouldBeCompletedWithMoreText( completed_item,
-                                                             completions ):
-      # Since the way that YCM works leads to CompleteDone called on every
-      # character, return blank if the completion might not be done. This won't
-      # match if the completion is ended with typing a non-keyword character.
-      return []
-
-    result = self._FilterToMatchingCompletions( completed_item,
-                                                completions,
-                                                False )
-
-    return list( result )
-
-
-  def _FilterToMatchingCompletions( self,
-                                    completed_item,
-                                    completions,
-                                    full_match_only ):
-    """Filter to completions matching the item Vim said was completed"""
-    match_keys = ( [ "word", "abbr", "menu", "info" ] if full_match_only
-                    else [ 'word' ] )
-
-    for index, completion in enumerate( completions ):
-      item = ConvertCompletionDataToVimData( index, completion )
-
-      def matcher( key ):
-        return ( utils.ToUnicode( completed_item.get( key, "" ) ) ==
-                 utils.ToUnicode( item.get( key, "" ) ) )
-
-      if all( [ matcher( i ) for i in match_keys ] ):
-        yield completion
-
-
-  def _HasCompletionsThatCouldBeCompletedWithMoreText( self,
-                                                       completed_item,
-                                                       completions ):
-    if not completed_item:
-      return False
-
-    completed_word = utils.ToUnicode( completed_item[ 'word' ] )
-    if not completed_word:
-      return False
-
-    # Sometimes CompleteDone is called after the next character is inserted.
-    # If so, use inserted character to filter possible completions further.
-    text = vimsupport.TextBeforeCursor()
-    reject_exact_match = True
-    if text and text[ -1 ] != completed_word[ -1 ]:
-      reject_exact_match = False
-      completed_word += text[ -1 ]
-
-    for index, completion in enumerate( completions ):
-      word = utils.ToUnicode(
-          ConvertCompletionDataToVimData( index, completion )[ 'word' ] )
-      if reject_exact_match and word == completed_word:
-        continue
-      if word.startswith( completed_word ):
-        return True
-    return False
-
-
-  def _OnCompleteDone_Csharp( self ):
-    completions = self.GetCompletionsUserMayHaveCompleted()
-    namespaces = [ self._GetRequiredNamespaceImport( c )
-                   for c in completions ]
-    namespaces = [ n for n in namespaces if n ]
-    if not namespaces:
-      return
-
-    if len( namespaces ) > 1:
-      choices = [ "{0} {1}".format( i + 1, n )
-                  for i, n in enumerate( namespaces ) ]
-      choice = vimsupport.PresentDialog( "Insert which namespace:", choices )
-      if choice < 0:
-        return
-      namespace = namespaces[ choice ]
-    else:
-      namespace = namespaces[ 0 ]
-
-    vimsupport.InsertNamespace( namespace )
-
-
-  def _GetRequiredNamespaceImport( self, completion ):
-    if ( "extra_data" not in completion
-         or "required_namespace_import" not in completion[ "extra_data" ] ):
-      return None
-    return completion[ "extra_data" ][ "required_namespace_import" ]
-
-
-  def _OnCompleteDone_Java( self ):
-    completions = self.GetCompletionsUserMayHaveCompleted()
-    fixit_completions = [ self._GetFixItCompletion( c ) for c in completions ]
-    fixit_completions = [ f for f in fixit_completions if f ]
-    if not fixit_completions:
-      return
-
-    # If we have user_data in completions (8.0.1493 or later), then we would
-    # only ever return max. 1 completion here. However, if we had to guess, it
-    # is possible that we matched multiple completion items (e.g. for overloads,
-    # or similar classes in multiple packages). In any case, rather than
-    # prompting the user and disturbing her workflow, we just apply the first
-    # one. This might be wrong, but the solution is to use a (very) new version
-    # of Vim which supports user_data on completion items
-    fixit_completion = fixit_completions[ 0 ]
-
-    for fixit in fixit_completion:
-      vimsupport.ReplaceChunks( fixit[ 'chunks' ], silent=True )
-
-
-  def _GetFixItCompletion( self, completion ):
-    if ( "extra_data" not in completion
-         or "fixits" not in completion[ "extra_data" ] ):
-      return None
-
-    return completion[ "extra_data" ][ "fixits" ]
-
   def GetErrorCount( self ):
     return self.CurrentBuffer().GetErrorCount()