Auto merge of #2708 - micbou:omni-completer-omnifunc, r=vheon

[READY] Do not cache omnifunc on FileReadyToParse event

We currently cache the omnifunc when the file is ready to be parsed i.e. on the `FileType`, `BufEnter` events and possibly on `InsertLeave` and `TextChanged`. That's problematic if the omnifunc is modified after these events and the user is requesting completions from it. This was the cause of issue https://github.com/Valloric/YouCompleteMe/issues/1027 but is not relevant anymore because we now always parse the file on the `BufEnter` event and thus the omnifunc is properly updated after switching buffers. Still, it's more correct to cache the omnifunc inside the `ShouldUseNow` method since `ComputeCandidatesInner` is called immediately after. Performance is not a concern: it takes less than 10 μs to get the omnifunc.

Fixes https://github.com/Valloric/YouCompleteMe/issues/1027.

<!-- 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/2708)
<!-- Reviewable:end -->

python/ycm/omni_completer.py

@@ -48,17 +48,15 @@ class OmniCompleter( Completer ):
 
 
   def ShouldUseNow( self, request_data ):
+    self._omnifunc = utils.ToUnicode( vim.eval( '&omnifunc' ) )
     if not self._omnifunc:
       return False
-
     if self.ShouldUseCache():
       return super( OmniCompleter, self ).ShouldUseNow( request_data )
     return self.ShouldUseNowInner( request_data )
 
 
   def ShouldUseNowInner( self, request_data ):
-    if not self._omnifunc:
-      return False
     if request_data.get( 'force_semantic', False ):
       return True
     return super( OmniCompleter, self ).ShouldUseNowInner( request_data )
@@ -67,10 +65,9 @@ class OmniCompleter( Completer ):
   def ComputeCandidates( self, request_data ):
     if self.ShouldUseCache():
       return super( OmniCompleter, self ).ComputeCandidates( request_data )
-    else:
-      if self.ShouldUseNowInner( request_data ):
-        return self.ComputeCandidatesInner( request_data )
-      return []
+    if self.ShouldUseNowInner( request_data ):
+      return self.ComputeCandidatesInner( request_data )
+    return []
 
 
   def ComputeCandidatesInner( self, request_data ):
@@ -78,7 +75,7 @@ class OmniCompleter( Completer ):
       return []
 
     try:
-      return_value = int( vim.eval( self._omnifunc + '(1,"")' ) )
+      return_value = vimsupport.GetIntValue( self._omnifunc + '(1,"")' )
       if return_value < 0:
         # FIXME: Technically, if the return is -1 we should raise an error
         return []
@@ -120,10 +117,6 @@ class OmniCompleter( Completer ):
       return []
 
 
-  def OnFileReadyToParse( self, request_data ):
-    self._omnifunc = utils.ToUnicode( vim.eval( '&omnifunc' ) )
-
-
   def FilterAndSortCandidatesInner( self, candidates, sort_property, query ):
     request_data = {
       'candidates': candidates,

python/ycm/tests/omni_completer_test.py

@@ -47,8 +47,6 @@ def OmniCompleter_GetCompletions_Cache_List_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 5 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -72,8 +70,6 @@ def OmniCompleter_GetCompletions_Cache_ListFilter_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 6 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -97,8 +93,6 @@ def OmniCompleter_GetCompletions_NoCache_List_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 5 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -122,8 +116,6 @@ def OmniCompleter_GetCompletions_NoCache_ListFilter_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 6 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     # Actual result is that the results are not filtered, as we expect the
     # omnifunc or vim itself to do this filtering.
@@ -149,8 +141,6 @@ def OmniCompleter_GetCompletions_NoCache_UseFindStart_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 6 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     # Actual result is that the results are not filtered, as we expect the
     # omnifunc or vim itself to do this filtering.
@@ -176,8 +166,6 @@ def OmniCompleter_GetCompletions_Cache_UseFindStart_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 6 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     # There are no results because the query 'test.t' doesn't match any
     # candidate (and cache_omnifunc=1, so we FilterAndSortCandidates).
@@ -203,8 +191,6 @@ def OmniCompleter_GetCompletions_Cache_Object_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 6 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -243,8 +229,6 @@ def OmniCompleter_GetCompletions_Cache_ObjectList_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 7 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -289,8 +273,6 @@ def OmniCompleter_GetCompletions_NoCache_ObjectList_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 7 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     # We don't filter the result - we expect the omnifunc to do that
     # based on the query we supplied (Note: that means no fuzzy matching!).
@@ -343,8 +325,6 @@ def OmniCompleter_GetCompletions_Cache_ObjectListObject_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 7 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -389,8 +369,6 @@ def OmniCompleter_GetCompletions_NoCache_ObjectListObject_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 7 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     # No FilterAndSortCandidates for cache_omnifunc=0 (we expect the omnifunc
     # to do the filtering?)
@@ -428,8 +406,6 @@ def OmniCompleter_GetCompletions_Cache_List_Unicode_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 12 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -455,8 +431,6 @@ def OmniCompleter_GetCompletions_NoCache_List_Unicode_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 12 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -484,8 +458,6 @@ def OmniCompleter_GetCompletions_Cache_List_Filter_Unicode_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 17 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -509,8 +481,6 @@ def OmniCompleter_GetCompletions_NoCache_List_Filter_Unicode_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 17 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -551,8 +521,6 @@ def OmniCompleter_GetCompletions_Cache_ObjectList_Unicode_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 17 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -606,8 +574,6 @@ def OmniCompleter_GetCompletions_Cache_ObjectListObject_Unicode_test( ycm ):
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 13 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       ycm.GetCompletionResponse(),
@@ -644,8 +610,6 @@ def OmniCompleter_GetCompletions_RestoreCursorPositionAfterOmnifuncCall_test(
                               omnifunc = Omnifunc )
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 3, 5 ) ):
-    # Make sure there is an omnifunc set up.
-    ycm.OnFileReadyToParse()
     ycm.SendCompletionRequest()
     assert_that(
       vimsupport.CurrentLineAndColumn(),
@@ -658,3 +622,49 @@ def OmniCompleter_GetCompletions_RestoreCursorPositionAfterOmnifuncCall_test(
         'completion_start_column': 6
       } )
     )
+
+
+@YouCompleteMeInstance( { 'cache_omnifunc': 0 } )
+def OmniCompleter_GetCompletions_NoCache_NoSemanticTrigger_test( ycm ):
+  def Omnifunc( findstart, base ):
+    if findstart:
+      return 0
+    return [ 'test' ]
+
+  current_buffer = VimBuffer( 'buffer',
+                              contents = [ 'te' ],
+                              filetype = 'java',
+                              omnifunc = Omnifunc )
+
+  with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 3 ) ):
+    ycm.SendCompletionRequest()
+    assert_that(
+      ycm.GetCompletionResponse(),
+      has_entries( {
+        'completions': empty(),
+        'completion_start_column': 1
+      } )
+    )
+
+
+@YouCompleteMeInstance( { 'cache_omnifunc': 0 } )
+def OmniCompleter_GetCompletions_NoCache_ForceSemantic_test( ycm ):
+  def Omnifunc( findstart, base ):
+    if findstart:
+      return 0
+    return [ 'test' ]
+
+  current_buffer = VimBuffer( 'buffer',
+                              contents = [ 'te' ],
+                              filetype = 'java',
+                              omnifunc = Omnifunc )
+
+  with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 3 ) ):
+    ycm.SendCompletionRequest( force_semantic = True )
+    assert_that(
+      ycm.GetCompletionResponse(),
+      has_entries( {
+        'completions': ToBytesOnPY2( [ 'test' ] ),
+        'completion_start_column': 1
+      } )
+    )

python/ycm/youcompleteme.py

@@ -365,8 +365,6 @@ class YouCompleteMe( object ):
     if not self.IsServerReady():
       return
 
-    self._omnicomp.OnFileReadyToParse( None )
-
     extra_data = {}
     self._AddTagsFilesIfNeeded( extra_data )
     self._AddSyntaxDataIfNeeded( extra_data )