Auto merge of #3104 - micbou:omnifunc-convert-strings-dictionaries, r=bstaletic

[READY] Convert strings from omnifunc into dictionaries

Vim allows each item of the list returned by an omnifunc to be either a string
or a dictionary (see `:h complete-items`) but ycmd only supports lists where items are all strings or all
dictionaries. Convert all strings into dictionaries.

Fixes https://github.com/Valloric/YouCompleteMe/issues/3103.
Fixes https://github.com/Valloric/YouCompleteMe/issues/3179

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

python/ycm/base.py

@@ -111,22 +111,16 @@ def AdjustCandidateInsertionText( candidates ):
 
   new_candidates = []
   for candidate in candidates:
-    if isinstance( candidate, dict ):
+    new_candidate = candidate.copy()
-      new_candidate = candidate.copy()
 
-      if 'abbr' not in new_candidate:
+    if 'abbr' not in new_candidate:
-        new_candidate[ 'abbr' ] = new_candidate[ 'word' ]
+      new_candidate[ 'abbr' ] = new_candidate[ 'word' ]
 
-      new_candidate[ 'word' ] = NewCandidateInsertionText(
+    new_candidate[ 'word' ] = NewCandidateInsertionText(
-        new_candidate[ 'word' ],
+      new_candidate[ 'word' ],
-        text_after_cursor )
+      text_after_cursor )
 
-      new_candidates.append( new_candidate )
+    new_candidates.append( new_candidate )
-
-    elif isinstance( candidate, str ) or isinstance( candidate, bytes ):
-      new_candidates.append(
-        { 'abbr': candidate,
-          'word': NewCandidateInsertionText( candidate, text_after_cursor ) } )
   return new_candidates
 
 

python/ycm/omni_completer.py

@@ -123,7 +123,14 @@ class OmniCompleter( Completer ):
       if not hasattr( items, '__iter__' ):
         raise TypeError( OMNIFUNC_NOT_LIST )
 
-      return list( filter( bool, items ) )
+      # Vim allows each item of the list to be either a string or a dictionary
+      # but ycmd only supports lists where items are all strings or all
+      # dictionaries. Convert all strings into dictionaries.
+      for index, item in enumerate( items ):
+        if not isinstance( item, dict ):
+          items[ index ] = { 'word': item }
+
+      return items
 
     except ( TypeError, ValueError, vim.error ) as error:
       vimsupport.PostVimMessage(

python/ycm/tests/base_test.py

@@ -57,49 +57,49 @@ def MockTextAfterCursor( text ):
 def AdjustCandidateInsertionText_Basic_test():
   with MockTextAfterCursor( 'bar' ):
     eq_( [ { 'abbr': 'foobar', 'word': 'foo' } ],
-         base.AdjustCandidateInsertionText( [ 'foobar' ] ) )
+         base.AdjustCandidateInsertionText( [ { 'word': 'foobar' } ] ) )
 
 
 def AdjustCandidateInsertionText_ParenInTextAfterCursor_test():
   with MockTextAfterCursor( 'bar(zoo' ):
     eq_( [ { 'abbr': 'foobar', 'word': 'foo' } ],
-         base.AdjustCandidateInsertionText( [ 'foobar' ] ) )
+         base.AdjustCandidateInsertionText( [ { 'word': 'foobar' } ] ) )
 
 
 def AdjustCandidateInsertionText_PlusInTextAfterCursor_test():
   with MockTextAfterCursor( 'bar+zoo' ):
     eq_( [ { 'abbr': 'foobar', 'word': 'foo' } ],
-         base.AdjustCandidateInsertionText( [ 'foobar' ] ) )
+         base.AdjustCandidateInsertionText( [ { 'word': 'foobar' } ] ) )
 
 
 def AdjustCandidateInsertionText_WhitespaceInTextAfterCursor_test():
   with MockTextAfterCursor( 'bar zoo' ):
     eq_( [ { 'abbr': 'foobar', 'word': 'foo' } ],
-         base.AdjustCandidateInsertionText( [ 'foobar' ] ) )
+         base.AdjustCandidateInsertionText( [ { 'word': 'foobar' } ] ) )
 
 
 def AdjustCandidateInsertionText_MoreThanWordMatchingAfterCursor_test():
   with MockTextAfterCursor( 'bar.h' ):
     eq_( [ { 'abbr': 'foobar.h', 'word': 'foo' } ],
-         base.AdjustCandidateInsertionText( [ 'foobar.h' ] ) )
+         base.AdjustCandidateInsertionText( [ { 'word': 'foobar.h' } ] ) )
 
   with MockTextAfterCursor( 'bar(zoo' ):
     eq_( [ { 'abbr': 'foobar(zoo', 'word': 'foo' } ],
-         base.AdjustCandidateInsertionText( [ 'foobar(zoo' ] ) )
+         base.AdjustCandidateInsertionText( [ { 'word': 'foobar(zoo' } ] ) )
 
 
 def AdjustCandidateInsertionText_NotSuffix_test():
   with MockTextAfterCursor( 'bar' ):
     eq_( [ { 'abbr': 'foofoo', 'word': 'foofoo' } ],
-         base.AdjustCandidateInsertionText( [ 'foofoo' ] ) )
+         base.AdjustCandidateInsertionText( [ { 'word': 'foofoo' } ] ) )
 
 
 def AdjustCandidateInsertionText_NothingAfterCursor_test():
   with MockTextAfterCursor( '' ):
-    eq_( [ 'foofoo',
+    eq_( [ { 'word': 'foofoo' },
-           'zobar' ],
+           { 'word': 'zobar' } ],
-         base.AdjustCandidateInsertionText( [ 'foofoo',
+         base.AdjustCandidateInsertionText( [ { 'word': 'foofoo' },
-                                              'zobar' ] ) )
+                                              { 'word': 'zobar' } ] ) )
 
 
 def AdjustCandidateInsertionText_MultipleStrings_test():
@@ -108,17 +108,16 @@ def AdjustCandidateInsertionText_MultipleStrings_test():
            { 'abbr': 'zobar', 'word': 'zo' },
            { 'abbr': 'qbar', 'word': 'q' },
            { 'abbr': 'bar', 'word': '' }, ],
-         base.AdjustCandidateInsertionText( [ 'foobar',
+         base.AdjustCandidateInsertionText( [ { 'word': 'foobar' },
-                                              'zobar',
+                                              { 'word': 'zobar' },
-                                              'qbar',
+                                              { 'word': 'qbar' },
-                                              'bar' ] ) )
+                                              { 'word': 'bar' } ] ) )
 
 
 def AdjustCandidateInsertionText_DictInput_test():
   with MockTextAfterCursor( 'bar' ):
     eq_( [ { 'abbr': 'foobar', 'word': 'foo' } ],
-         base.AdjustCandidateInsertionText(
+         base.AdjustCandidateInsertionText( [ { 'word': 'foobar' } ] ) )
-           [ { 'word': 'foobar' } ] ) )
 
 
 def AdjustCandidateInsertionText_DontTouchAbbr_test():

python/ycm/tests/omni_completer_test.py

@@ -57,7 +57,11 @@ def OmniCompleter_GetCompletions_Cache_List_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ 'a', 'b', 'cdef' ] ),
+        'completions': ToBytesOnPY2( [
+          { 'word': 'a' },
+          { 'word': 'b' },
+          { 'word': 'cdef' }
+        ] ),
         'completion_start_column': 6
       } )
     )
@@ -105,7 +109,11 @@ def OmniCompleter_GetCompletions_NoCache_List_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ 'a', 'b', 'cdef' ] ),
+        'completions': ToBytesOnPY2( [
+          { 'word': 'a' },
+          { 'word': 'b' },
+          { 'word': 'cdef' }
+        ] ),
         'completion_start_column': 6
       } )
     )
@@ -131,7 +139,11 @@ def OmniCompleter_GetCompletions_NoCache_ListFilter_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ 'a', 'b', 'cdef' ] ),
+        'completions': ToBytesOnPY2( [
+          { 'word': 'a' },
+          { 'word': 'b' },
+          { 'word': 'cdef' }
+        ] ),
         'completion_start_column': 6
       } )
     )
@@ -157,7 +169,11 @@ def OmniCompleter_GetCompletions_NoCache_UseFindStart_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ 'a', 'b', 'cdef' ] ),
+        'completions': ToBytesOnPY2( [
+          { 'word': 'a' },
+          { 'word': 'b' },
+          { 'word': 'cdef' }
+        ] ),
         'completion_start_column': 1
       } )
     )
@@ -207,7 +223,7 @@ def OmniCompleter_GetCompletions_Cache_Object_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': [ 'CDtEF' ],
+        'completions': [ { 'word': 'CDtEF' } ],
         'completion_start_column': 6
       } )
     )
@@ -427,9 +443,11 @@ def OmniCompleter_GetCompletions_Cache_List_Unicode_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': [ 'å_unicode_identifier',
+        'completions': [
-                         'πππππππ yummy πie',
+          { 'word': 'å_unicode_identifier' },
-                         '†est' ],
+          { 'word': 'πππππππ yummy πie' },
+          { 'word': '†est' }
+        ],
         'completion_start_column': 13
       } )
     )
@@ -453,9 +471,11 @@ def OmniCompleter_GetCompletions_NoCache_List_Unicode_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ '†est',
+        'completions': ToBytesOnPY2( [
-                                       'å_unicode_identifier',
+          { 'word': '†est' },
-                                       'πππππππ yummy πie' ] ),
+          { 'word': 'å_unicode_identifier' },
+          { 'word': 'πππππππ yummy πie' }
+        ] ),
         'completion_start_column': 13
       } )
     )
@@ -479,7 +499,7 @@ def OmniCompleter_GetCompletions_Cache_List_Filter_Unicode_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': [ 'πππππππ yummy πie' ],
+        'completions': [ { 'word': 'πππππππ yummy πie' } ],
         'completion_start_column': 13
       } )
     )
@@ -503,7 +523,7 @@ def OmniCompleter_GetCompletions_NoCache_List_Filter_Unicode_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ 'πππππππ yummy πie' ] ),
+        'completions': ToBytesOnPY2( [ { 'word': 'πππππππ yummy πie' } ] ),
         'completion_start_column': 13
       } )
     )
@@ -643,7 +663,7 @@ def OmniCompleter_GetCompletions_RestoreCursorPositionAfterOmnifuncCall_test(
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ 'length' ] ),
+        'completions': ToBytesOnPY2( [ { 'word': 'length' } ] ),
         'completion_start_column': 6
       } )
     )
@@ -678,7 +698,7 @@ def OmniCompleter_GetCompletions_MoveCursorPositionAtStartColumn_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ 'length' ] ),
+        'completions': ToBytesOnPY2( [ { 'word': 'length' } ] ),
         'completion_start_column': 6
       } )
     )
@@ -711,14 +731,14 @@ def StartColumnCompliance( ycm,
 
 
 def OmniCompleter_GetCompletions_StartColumnCompliance_test():
-  yield StartColumnCompliance, -4, [ 'foo' ], 3
+  yield StartColumnCompliance, -4, [ { 'word': 'foo' } ], 3
   yield StartColumnCompliance, -3, [],        1
   yield StartColumnCompliance, -2, [],        1
-  yield StartColumnCompliance, -1, [ 'foo' ], 3
+  yield StartColumnCompliance, -1, [ { 'word': 'foo' } ], 3
-  yield StartColumnCompliance,  0, [ 'foo' ], 1
+  yield StartColumnCompliance,  0, [ { 'word': 'foo' } ], 1
-  yield StartColumnCompliance,  1, [ 'foo' ], 2
+  yield StartColumnCompliance,  1, [ { 'word': 'foo' } ], 2
-  yield StartColumnCompliance,  2, [ 'foo' ], 3
+  yield StartColumnCompliance,  2, [ { 'word': 'foo' } ], 3
-  yield StartColumnCompliance,  3, [ 'foo' ], 3
+  yield StartColumnCompliance,  3, [ { 'word': 'foo' } ], 3
 
 
 @YouCompleteMeInstance( { 'g:ycm_cache_omnifunc': 0,
@@ -763,12 +783,42 @@ def OmniCompleter_GetCompletions_NoCache_ForceSemantic_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ 'test' ] ),
+        'completions': ToBytesOnPY2( [ { 'word': 'test' } ] ),
         'completion_start_column': 1
       } )
     )
 
 
+@YouCompleteMeInstance( { 'g:ycm_cache_omnifunc': 1,
+                          'g:ycm_semantic_triggers': TRIGGERS } )
+def OmniCompleter_GetCompletions_ConvertStringsToDictionaries_test( ycm ):
+  def Omnifunc( findstart, base ):
+    if findstart:
+      return 5
+    return [
+      { 'word': 'a' },
+      'b'
+    ]
+
+  current_buffer = VimBuffer( 'buffer',
+                              contents = [ 'test.' ],
+                              filetype = FILETYPE,
+                              omnifunc = Omnifunc )
+
+  with MockVimBuffers( [ current_buffer ], [ current_buffer ], ( 1, 7 ) ):
+    ycm.SendCompletionRequest()
+    assert_that(
+      ycm.GetCompletionResponse(),
+      has_entries( {
+        'completions': ToBytesOnPY2( [
+          { 'word': 'a' },
+          { 'word': 'b' }
+        ] ),
+        'completion_start_column': 6
+      } )
+    )
+
+
 @YouCompleteMeInstance( {
   'g:ycm_cache_omnifunc': 0,
   'g:ycm_filetype_specific_completion_to_disable': { FILETYPE: 1 },
@@ -843,7 +893,11 @@ def OmniCompleter_GetCompletions_FiletypeDisabled_ForceSemantic_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ 'a', 'b', 'cdef' ] ),
+        'completions': ToBytesOnPY2( [
+          { 'word': 'a' },
+          { 'word': 'b' },
+          { 'word': 'cdef' }
+        ] ),
         'completion_start_column': 6
       } )
     )
@@ -869,7 +923,11 @@ def OmniCompleter_GetCompletions_AllFiletypesDisabled_ForceSemantic_test( ycm ):
     assert_that(
       ycm.GetCompletionResponse(),
       has_entries( {
-        'completions': ToBytesOnPY2( [ 'a', 'b', 'cdef' ] ),
+        'completions': ToBytesOnPY2( [
+          { 'word': 'a' },
+          { 'word': 'b' },
+          { 'word': 'cdef' }
+        ] ),
         'completion_start_column': 6
       } )
     )