Jelajahi Sumber

Auto merge of #2899 - micbou:catch-extra-conf-data-exception, r=bstaletic

[READY] Avoid traceback when computing extra conf data

Evaluating the values of [the `g:ycm_extra_conf_vim_data` option](https://github.com/Valloric/YouCompleteMe#the-gycm_extra_conf_vim_data-option) may raise a Python exception (e.g. one of the values is not defined). Since that option is evaluated each time a request is sent, such exception can render the editor almost unusable as each key press will print a Python traceback to the user.

A simple way to reproduce the issue is to add the following line in vimrc:
```viml
let g:ycm_extra_conf_vim_data = [ 'undefined_value' ]
```

We fix that by catching and logging the exception.

<!-- 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/2899)
<!-- Reviewable:end -->
zzbot 7 tahun lalu
induk
melakukan
26a5e3cbc4

+ 19 - 2
python/ycm/tests/command_test.py

@@ -22,7 +22,7 @@ from __future__ import absolute_import
 # Not installing aliases from python-future; it's unreliable and slow.
 from builtins import *  # noqa
 
-from ycm.tests.test_utils import ( MockVimModule, MockVimBuffers, VimBuffer )
+from ycm.tests.test_utils import MockVimModule, MockVimBuffers, VimBuffer
 MockVimModule()
 
 from hamcrest import assert_that, equal_to
@@ -32,7 +32,7 @@ from ycm.tests import YouCompleteMeInstance
 
 
 @YouCompleteMeInstance( { 'extra_conf_vim_data': [ 'tempname()' ] } )
-def SendCommandRequest_test( ycm ):
+def SendCommandRequest_ExtraConfData_Works_test( ycm ):
   current_buffer = VimBuffer( 'buffer' )
   with MockVimBuffers( [ current_buffer ], current_buffer ):
     with patch( 'ycm.youcompleteme.SendCommandRequest' ) as send_request:
@@ -47,3 +47,20 @@ def SendCommandRequest_test( ycm ):
         ycm.SendCommandRequest( [ 'GoTo' ], 'python' ),
         equal_to( 'Some response' )
       )
+
+
+@YouCompleteMeInstance( { 'extra_conf_vim_data': [ 'undefined_value' ] } )
+def SendCommandRequest_ExtraConfData_UndefinedValue_test( ycm ):
+  current_buffer = VimBuffer( 'buffer' )
+  with MockVimBuffers( [ current_buffer ], current_buffer ):
+    with patch( 'ycm.youcompleteme.SendCommandRequest' ) as send_request:
+      ycm.SendCommandRequest( [ 'GoTo' ], 'python' )
+      send_request.assert_called_once_with(
+        [ 'GoTo' ], 'python', { 'extra_conf_data': {} }
+      )
+    with patch( 'ycm.client.base_request.JsonFromFuture',
+                return_value = 'Some response' ):
+      assert_that(
+        ycm.SendCommandRequest( [ 'GoTo' ], 'python' ),
+        equal_to( 'Some response' )
+      )

+ 2 - 1
python/ycm/tests/test_utils.py

@@ -215,7 +215,7 @@ def _MockVimEval( value ):
   if match:
     return match.group( 'filepath' )
 
-  raise ValueError( 'Unexpected evaluation: {0}'.format( value ) )
+  raise VimError( 'Unexpected evaluation: {0}'.format( value ) )
 
 
 def _MockWipeoutBuffer( buffer_number ):
@@ -381,6 +381,7 @@ def MockVimModule():
 
   VIM_MOCK.buffers = {}
   VIM_MOCK.eval = MagicMock( side_effect = _MockVimEval )
+  VIM_MOCK.error = VimError
   sys.modules[ 'vim' ] = VIM_MOCK
 
   return VIM_MOCK

+ 0 - 3
python/ycm/tests/vimsupport_test.py

@@ -1684,7 +1684,6 @@ def JumpToLocation_DifferentFile_SameBuffer_Modified_CanHide_test(
 
 @patch( 'ycmd.user_options_store._USER_OPTIONS',
         { 'goto_buffer_command': 'same-buffer' } )
-@patch( 'vim.error', VimError )
 @patch( 'vim.command',
         side_effect = [ None, VimError( 'Unknown code' ), None ] )
 def JumpToLocation_DifferentFile_SameBuffer_SwapFile_Unexpected_test(
@@ -1701,7 +1700,6 @@ def JumpToLocation_DifferentFile_SameBuffer_SwapFile_Unexpected_test(
 
 @patch( 'ycmd.user_options_store._USER_OPTIONS',
         { 'goto_buffer_command': 'same-buffer' } )
-@patch( 'vim.error', VimError )
 @patch( 'vim.command',
         new_callable = ExtendedMock,
         side_effect = [ None, VimError( 'E325' ), None ] )
@@ -1720,7 +1718,6 @@ def JumpToLocation_DifferentFile_SameBuffer_SwapFile_Quit_test( vim_command ):
 
 @patch( 'ycmd.user_options_store._USER_OPTIONS',
         { 'goto_buffer_command': 'same-buffer' } )
-@patch( 'vim.error', VimError )
 @patch( 'vim.command',
         new_callable = ExtendedMock,
         side_effect = [ None, KeyboardInterrupt, None ] )

+ 11 - 2
python/ycm/youcompleteme.py

@@ -750,8 +750,17 @@ class YouCompleteMe( object ):
 
   def _AddExtraConfDataIfNeeded( self, extra_data ):
     def BuildExtraConfData( extra_conf_vim_data ):
-      return dict( ( expr, vimsupport.VimExpressionToPythonType( expr ) )
-                   for expr in extra_conf_vim_data )
+      extra_conf_data = {}
+      for expr in extra_conf_vim_data:
+        try:
+          extra_conf_data[ expr ] = vimsupport.VimExpressionToPythonType( expr )
+        except vim.error:
+          message = (
+            "Error evaluating '{expr}' in the 'g:ycm_extra_conf_vim_data' "
+            "option.".format( expr = expr ) )
+          vimsupport.PostVimMessage( message, truncate = True )
+          self._logger.exception( message )
+      return extra_conf_data
 
     extra_conf_vim_data = self._user_options[ 'extra_conf_vim_data' ]
     if extra_conf_vim_data: