Auto merge of #3208 - micbou:null-character-completion, r=micbou

[READY] Handle null characters in completion response

The `detailed_info` field may contain null characters e.g. `\x00` in Python docstrings (though it doesn't make sense to use such characters in a docstring). Since these characters cannot be evaluated through `pyeval` (and `py3eval`), we replace them with the symbol used by Vim to display them (`^@`).

This PR also rewrites the `_ConvertCompletionDataToVimData` function by relying on two things:
 - [the `insertion_text` field is always defined](600f54de20/ycmd/responses.py (L109));
 - [Vim ignores fields with an empty string](833e5dab14/src/edit.c (L2748-L2755)).

I did some measurements and the performance hit is negligible.

Fixes #3207.

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

python/ycm/client/completion_request.py

@@ -190,49 +190,43 @@ def _FilterToMatchingCompletions( completed_item, completions ):
   return matched_completions
 
 
+def _GetCompletionInfoField( completion_data ):
+  info = completion_data.get( 'detailed_info', '' )
+
+  if 'extra_data' in completion_data:
+    docstring = completion_data[ 'extra_data' ].get( 'doc_string', '' )
+    if docstring:
+      if info:
+        info += '\n' + docstring
+      else:
+        info = docstring
+
+  # This field may contain null characters e.g. \x00 in Python docstrings. Vim
+  # cannot evaluate such characters so they are removed.
+  return info.replace( '\x00', '' )
+
+
 def _ConvertCompletionDataToVimData( completion_identifier, completion_data ):
-  # see :h complete-items for a description of the dictionary fields
+  # See :h complete-items for a description of the dictionary fields.
-  vim_data = {
+  return {
-    'word'  : '',
+    'word'     : completion_data[ 'insertion_text' ],
-    'dup'   : 1,
+    'abbr'     : completion_data.get( 'menu_text', '' ),
-    'empty' : 1,
+    'menu'     : completion_data.get( 'extra_menu_info', '' ),
+    'info'     : _GetCompletionInfoField( completion_data ),
+    'kind'     : ToUnicode( completion_data.get( 'kind', '' ) )[ :1 ].lower(),
+    'dup'      : 1,
+    'empty'    : 1,
+    # We store the completion item index as a string in the completion
+    # user_data. This allows us to identify the _exact_ item that was completed
+    # in the CompleteDone handler, by inspecting this item from v:completed_item
+    #
+    # We convert to string because completion user data items must be strings.
+    #
+    # Note: Not all versions of Vim support this (added in 8.0.1483), but adding
+    # the item to the dictionary is harmless in earlier Vims.
+    'user_data': str( completion_identifier )
   }
 
-  if ( 'extra_data' in completion_data and
-       'doc_string' in completion_data[ 'extra_data' ] ):
-    doc_string = completion_data[ 'extra_data' ][ 'doc_string' ]
-  else:
-    doc_string = ""
-
-  if 'insertion_text' in completion_data:
-    vim_data[ 'word' ] = completion_data[ 'insertion_text' ]
-  if 'menu_text' in completion_data:
-    vim_data[ 'abbr' ] = completion_data[ 'menu_text' ]
-  if 'extra_menu_info' in completion_data:
-    vim_data[ 'menu' ] = completion_data[ 'extra_menu_info' ]
-  if 'kind' in completion_data:
-    kind = ToUnicode( completion_data[ 'kind' ] )
-    if kind:
-      vim_data[ 'kind' ] = kind[ 0 ].lower()
-  if 'detailed_info' in completion_data:
-    vim_data[ 'info' ] = completion_data[ 'detailed_info' ]
-    if doc_string:
-      vim_data[ 'info' ] += '\n' + doc_string
-  elif doc_string:
-    vim_data[ 'info' ] = doc_string
-
-  # We store the completion item index as a string in the completion user_data.
-  # This allows us to identify the _exact_ item that was completed in the
-  # CompleteDone handler, by inspecting this item from v:completed_item
-  #
-  # We convert to string because completion user data items must be strings.
-  #
-  # Note: Not all versions of Vim support this (added in 8.0.1483), but adding
-  # the item to the dictionary is harmless in earlier Vims.
-  vim_data[ 'user_data' ] = str( completion_identifier )
-
-  return vim_data
-
 
 def _ConvertCompletionDatasToVimDatas( response_data ):
   return [ _ConvertCompletionDataToVimData( i, x )

python/ycm/tests/client/completion_request_test.py

@@ -48,7 +48,7 @@ class ConvertCompletionResponseToVimDatas_test( object ):
       raise
 
 
-  def All_Fields_test( self ):
+  def AllFields_test( self ):
     self._Check( 0, {
       'insertion_text':  'INSERTION TEXT',
       'menu_text':       'MENU TEXT',
@@ -70,7 +70,22 @@ class ConvertCompletionResponseToVimDatas_test( object ):
     } )
 
 
-  def Just_Detailed_Info_test( self ):
+  def OnlyInsertionTextField_test( self ):
+    self._Check( 17, {
+      'insertion_text':  'INSERTION TEXT'
+    }, {
+      'word'     : 'INSERTION TEXT',
+      'abbr'     : '',
+      'menu'     : '',
+      'kind'     : '',
+      'info'     : '',
+      'dup'      : 1,
+      'empty'    : 1,
+      'user_data': '17',
+    } )
+
+
+  def JustDetailedInfo_test( self ):
     self._Check( 9999999999, {
       'insertion_text':  'INSERTION TEXT',
       'menu_text':       'MENU TEXT',
@@ -89,7 +104,7 @@ class ConvertCompletionResponseToVimDatas_test( object ):
     } )
 
 
-  def Just_Doc_String_test( self ):
+  def JustDocString_test( self ):
     self._Check( 'not_an_int', {
       'insertion_text':  'INSERTION TEXT',
       'menu_text':       'MENU TEXT',
@@ -110,7 +125,7 @@ class ConvertCompletionResponseToVimDatas_test( object ):
     } )
 
 
-  def Extra_Info_No_Doc_String_test( self ):
+  def ExtraInfoNoDocString_test( self ):
     self._Check( 0, {
       'insertion_text':  'INSERTION TEXT',
       'menu_text':       'MENU TEXT',
@@ -123,13 +138,36 @@ class ConvertCompletionResponseToVimDatas_test( object ):
       'abbr'     : 'MENU TEXT',
       'menu'     : 'EXTRA MENU INFO',
       'kind'     : 'k',
+      'info'     : '',
       'dup'      : 1,
       'empty'    : 1,
       'user_data': '0',
     } )
 
 
-  def Extra_Info_No_Doc_String_With_Detailed_Info_test( self ):
+  def NullCharactersInExtraInfoAndDocString_test( self ):
+    self._Check( '0', {
+      'insertion_text':  'INSERTION TEXT',
+      'menu_text':       'MENU TEXT',
+      'extra_menu_info': 'EXTRA MENU INFO',
+      'kind':            'K',
+      'detailed_info':   'DETAILED\x00INFO',
+      'extra_data': {
+        'doc_string': 'DOC\x00STRING'
+      },
+    }, {
+      'word'     : 'INSERTION TEXT',
+      'abbr'     : 'MENU TEXT',
+      'menu'     : 'EXTRA MENU INFO',
+      'kind'     : 'k',
+      'info'     : 'DETAILEDINFO\nDOCSTRING',
+      'dup'      : 1,
+      'empty'    : 1,
+      'user_data': '0',
+    } )
+
+
+  def ExtraInfoNoDocStringWithDetailedInfo_test( self ):
     self._Check( '0', {
       'insertion_text':  'INSERTION TEXT',
       'menu_text':       'MENU TEXT',
@@ -150,7 +188,7 @@ class ConvertCompletionResponseToVimDatas_test( object ):
     } )
 
 
-  def Empty_Insertion_Text_test( self ):
+  def EmptyInsertionText_test( self ):
     self._Check( 0, {
       'insertion_text':  '',
       'menu_text':       'MENU TEXT',
@@ -170,24 +208,3 @@ class ConvertCompletionResponseToVimDatas_test( object ):
       'empty'    : 1,
       'user_data': '0',
     } )
-
-
-  def No_Insertion_Text_test( self ):
-    self._Check( 0, {
-      'menu_text':       'MENU TEXT',
-      'extra_menu_info': 'EXTRA MENU INFO',
-      'kind':            'K',
-      'detailed_info':   'DETAILED INFO',
-      'extra_data': {
-        'doc_string':    'DOC STRING',
-      },
-    }, {
-      'word'     : '',
-      'abbr'     : 'MENU TEXT',
-      'menu'     : 'EXTRA MENU INFO',
-      'kind'     : 'k',
-      'info'     : 'DETAILED INFO\nDOC STRING',
-      'dup'      : 1,
-      'empty'    : 1,
-      'user_data': '0'
-    } )

python/ycm/tests/postcomplete_test.py

@@ -66,18 +66,22 @@ def BuildCompletion( insertion_text = 'Test',
                      detailed_info = None,
                      kind = None,
                      extra_data = None ):
-  if extra_data is None:
+  completion = {
-    extra_data = {}
+    'insertion_text': insertion_text
-
-  return {
-    'extra_data': extra_data,
-    'insertion_text': insertion_text,
-    'menu_text': menu_text,
-    'extra_menu_info': extra_menu_info,
-    'kind': kind,
-    'detailed_info': detailed_info,
   }
 
+  if extra_menu_info:
+    completion[ 'extra_menu_info' ] = extra_menu_info
+  if menu_text:
+    completion[ 'menu_text' ] = menu_text
+  if detailed_info:
+    completion[ 'detailed_info' ] = detailed_info
+  if kind:
+    completion[ 'kind' ] = kind
+  if extra_data:
+    completion[ 'extra_data' ] = extra_data
+  return completion
+
 
 def BuildCompletionNamespace( namespace = None,
                               insertion_text = 'Test',