Correctly handling ycm_extra_conf files now

The user is asked about loading unknown extra conf files, as they were before.

python/ycm/client/base_request.py

@@ -24,15 +24,11 @@ from retries import retries
 from requests_futures.sessions import FuturesSession
 from concurrent.futures import ThreadPoolExecutor
 from ycm import vimsupport
+from ycm.server.responses import ServerError, UnknownExtraConf
 
 HEADERS = {'content-type': 'application/json'}
 EXECUTOR = ThreadPoolExecutor( max_workers = 4 )
 
-class ServerError( Exception ):
-  def __init__( self, message ):
-    super( ServerError, self ).__init__( message )
-
-
 class BaseRequest( object ):
   def __init__( self ):
     pass
@@ -103,7 +99,7 @@ def BuildRequestData( start_column = None, query = None ):
 def JsonFromFuture( future ):
   response = future.result()
   if response.status_code == requests.codes.server_error:
-    raise ServerError( response.json()[ 'message' ] )
+    _RaiseExceptionForData( response.json() )
 
   # We let Requests handle the other status types, we only handle the 500
   # error code.
@@ -137,3 +133,10 @@ def _CheckServerIsHealthyWithCache():
   except:
     return False
 
+
+def _RaiseExceptionForData( data ):
+  if data[ 'exception' ][ 'TYPE' ] == UnknownExtraConf.__name__:
+    raise UnknownExtraConf( data[ 'exception' ][ 'extra_conf_file' ] )
+
+  raise ServerError( '{0}: {1}'.format( data[ 'exception' ][ 'TYPE' ],
+                                        data[ 'message' ] ) )

python/ycm/client/event_notification.py

@@ -18,9 +18,11 @@
 # along with YouCompleteMe.  If not, see <http://www.gnu.org/licenses/>.
 
 from ycm import vimsupport
+from ycm.server.responses import UnknownExtraConf
 from ycm.client.base_request import ( BaseRequest, BuildRequestData,
                                      JsonFromFuture )
 
+
 class EventNotification( BaseRequest ):
   def __init__( self, event_name, extra_data = None ):
     super( EventNotification, self ).__init__()
@@ -50,11 +52,14 @@ class EventNotification( BaseRequest ):
     if not self._response_future or self._event_name != 'FileReadyToParse':
       return []
 
+    try:
       try:
         self._cached_response = JsonFromFuture( self._response_future )
+      except UnknownExtraConf as e:
+          if vimsupport.Confirm( str( e ) ):
+            _LoadExtraConfFile( e.extra_conf_file )
     except Exception as e:
       vimsupport.PostVimMessage( str( e ) )
-      return []
 
     if not self._cached_response:
       return []
@@ -83,3 +88,6 @@ def SendEventNotificationAsync( event_name, extra_data = None ):
   event = EventNotification( event_name, extra_data )
   event.Start()
 
+def _LoadExtraConfFile( filepath ):
+  BaseRequest.PostDataToHandler( { 'filepath': filepath },
+                                 'load_extra_conf_file' )

python/ycm/completers/cpp/clang_completer.py

@@ -79,6 +79,9 @@ class ClangCompleter( Completer ):
 
     if self._completer.UpdatingTranslationUnit( ToUtf8IfNeeded( filename ) ):
       self._logger.info( PARSING_FILE_MESSAGE )
+      # TODO: For this exception and the NO_COMPILE_FLAGS one, use a special
+      # exception class so that the client can be more silent about these
+      # messages.
       raise RuntimeError( PARSING_FILE_MESSAGE )
 
     flags = self._FlagsForRequest( request_data )

python/ycm/completers/cpp/flags.py

@@ -37,6 +37,7 @@ class Flags( object ):
     # It's caches all the way down...
     self.flags_for_file = {}
     self.special_clang_flags = _SpecialClangIncludes()
+    self.no_extra_conf_file_warning_posted = False
 
 
   def FlagsForFile( self, filename, add_special_clang_flags = True ):
@@ -45,7 +46,10 @@ class Flags( object ):
     except KeyError:
       module = extra_conf_store.ModuleForSourceFile( filename )
       if not module:
+        if not self.no_extra_conf_file_warning_posted:
+          self.no_extra_conf_file_warning_posted = True
           raise RuntimeError( NO_EXTRA_CONF_FILENAME_MESSAGE )
+        return None
 
       results = module.FlagsForFile( filename )
 

python/ycm/extra_conf_store.py

@@ -24,27 +24,29 @@ import imp
 import random
 import string
 import sys
+from threading import Lock
 from ycm import user_options_store
+from ycm.server.responses import UnknownExtraConf
 from fnmatch import fnmatch
 
 # Constants
 YCM_EXTRA_CONF_FILENAME = '.ycm_extra_conf.py'
-CONFIRM_CONF_FILE_MESSAGE = ('Found {0}. Load? \n\n(Question can be turned '
-                             'off with options, see YCM docs)')
 
 # Singleton variables
 _module_for_module_file = {}
+_module_for_module_file_lock = Lock()
 _module_file_for_source_file = {}
+_module_file_for_source_file_lock = Lock()
 
-class UnknownExtraConf( Exception ):
+
-  def __init__( self, extra_conf_file ):
+def Reset():
-    message = CONFIRM_CONF_FILE_MESSAGE.format( extra_conf_file )
+  global _module_for_module_file, _module_file_for_source_file
-    super( UnknownExtraConf, self ).__init__( message )
+  _module_for_module_file = {}
-    self.extra_conf_file = extra_conf_file
+  _module_file_for_source_file = {}
 
 
 def ModuleForSourceFile( filename ):
-  return _Load( ModuleFileForSourceFile( filename ) )
+  return Load( ModuleFileForSourceFile( filename ) )
 
 
 def ModuleFileForSourceFile( filename ):
@@ -52,9 +54,10 @@ def ModuleFileForSourceFile( filename ):
   order and return the filename of the first module that was allowed to load.
   If no module was found or allowed to load, None is returned."""
 
+  with _module_file_for_source_file_lock:
     if not filename in _module_file_for_source_file:
       for module_file in _ExtraConfModuleSourceFilesForFile( filename ):
-      if _Load( module_file ):
+        if Load( module_file ):
           _module_file_for_source_file[ filename ] = module_file
           break
 
@@ -84,6 +87,7 @@ def _CallExtraConfMethod( function_name ):
 
 def _Disable( module_file ):
   """Disables the loading of a module for the current session."""
+  with _module_for_module_file_lock:
     _module_for_module_file[ module_file ] = None
 
 
@@ -102,10 +106,15 @@ def _ShouldLoad( module_file ):
     if _MatchesGlobPattern( module_file, glob.lstrip('!') ):
       return not is_blacklisted
 
+  # We disable the file if it's unknown so that we don't ask the user about it
+  # repeatedly. Raising UnknownExtraConf should result in the client sending
+  # another request to load the module file if the user explicitly chooses to do
+  # that.
+  _Disable( module_file )
   raise UnknownExtraConf( module_file )
 
 
-def _Load( module_file, force = False ):
+def Load( module_file, force = False ):
   """Load and return the module contained in a file.
   Using force = True the module will be loaded regardless
   of the criteria in _ShouldLoad.
@@ -115,11 +124,13 @@ def _Load( module_file, force = False ):
     return None
 
   if not force:
+    with _module_for_module_file_lock:
       if module_file in _module_for_module_file:
         return _module_for_module_file[ module_file ]
 
     if not _ShouldLoad( module_file ):
-      return _Disable( module_file )
+      _Disable( module_file )
+      return None
 
   # This has to be here because a long time ago, the ycm_extra_conf.py files
   # used to import clang_helpers.py from the cpp folder. This is not needed
@@ -129,6 +140,7 @@ def _Load( module_file, force = False ):
   module = imp.load_source( _RandomName(), module_file )
   del sys.path[ 0 ]
 
+  with _module_for_module_file_lock:
     _module_for_module_file[ module_file ] = module
   return module
 

python/ycm/server/responses.py

@@ -19,6 +19,21 @@
 
 import os
 
+CONFIRM_CONF_FILE_MESSAGE = ('Found {0}. Load? \n\n(Question can be turned '
+                             'off with options, see YCM docs)')
+
+class ServerError( Exception ):
+  def __init__( self, message ):
+    super( ServerError, self ).__init__( message )
+
+
+class UnknownExtraConf( ServerError ):
+  def __init__( self, extra_conf_file ):
+    message = CONFIRM_CONF_FILE_MESSAGE.format( extra_conf_file )
+    super( UnknownExtraConf, self ).__init__( message )
+    self.extra_conf_file = extra_conf_file
+
+
 
 def BuildGoToResponse( filepath, line_num, column_num, description = None ):
   response = {
@@ -80,9 +95,10 @@ def BuildDiagnosticData( filepath,
   }
 
 
-def BuildExceptionResponse( error_message, traceback ):
+def BuildExceptionResponse( exception, traceback ):
   return {
-    'message': error_message,
+    'exception': exception,
+    'message': str( exception ),
     'traceback': traceback
   }
 

python/ycm/server/tests/basic_test.py

@@ -17,9 +17,11 @@
 # You should have received a copy of the GNU General Public License
 # along with YouCompleteMe.  If not, see <http://www.gnu.org/licenses/>.
 
+import os
+import httplib
 from webtest import TestApp
 from .. import ycmd
-from ..responses import BuildCompletionData
+from ..responses import BuildCompletionData, UnknownExtraConf
 from nose.tools import ok_, eq_, with_setup
 from hamcrest import ( assert_that, has_items, has_entry, contains,
                        contains_string, has_entries )
@@ -73,6 +75,15 @@ def Setup():
   ycmd.SetServerStateToDefaults()
 
 
+def PathToTestDataDir():
+  dir_of_current_script = os.path.dirname( os.path.abspath( __file__ ) )
+  return os.path.join( dir_of_current_script, 'testdata' )
+
+
+def PathToTestFile( test_basename ):
+  return os.path.join( PathToTestDataDir(), test_basename )
+
+
 @with_setup( Setup )
 def GetCompletions_IdentifierCompleter_Works_test():
   app = TestApp( ycmd.app )
@@ -91,7 +102,7 @@ def GetCompletions_IdentifierCompleter_Works_test():
 
 
 @with_setup( Setup )
-def GetCompletions_ClangCompleter_Works_test():
+def GetCompletions_ClangCompleter_WorksWithExplicitFlags_test():
   app = TestApp( ycmd.app )
   contents = """
 struct Foo {
@@ -122,6 +133,45 @@ int main()
                                    CompletionEntryMatcher( 'y' ) ) )
 
 
+@with_setup( Setup )
+def GetCompletions_ClangCompleter_UnknownExtraConfException_test():
+  app = TestApp( ycmd.app )
+  filepath = PathToTestFile( 'basic.cpp' )
+  completion_data = BuildRequest( filepath = filepath,
+                                  filetype = 'cpp',
+                                  contents = open( filepath ).read(),
+                                  force_semantic = True )
+
+  response = app.post_json( '/completions',
+                            completion_data,
+                            expect_errors = True )
+
+  eq_( response.status_code, httplib.INTERNAL_SERVER_ERROR )
+  assert_that( response.json,
+               has_entry( 'exception',
+                          has_entry( 'TYPE', UnknownExtraConf.__name__ ) ) )
+
+
+@with_setup( Setup )
+def GetCompletions_ClangCompleter_WorksWhenExtraConfExplicitlyAllowed_test():
+  app = TestApp( ycmd.app )
+  app.post_json( '/load_extra_conf_file',
+                 { 'filepath': PathToTestFile( '.ycm_extra_conf.py' ) } )
+
+  filepath = PathToTestFile( 'basic.cpp' )
+  completion_data = BuildRequest( filepath = filepath,
+                                  filetype = 'cpp',
+                                  contents = open( filepath ).read(),
+                                  line_num = 10,
+                                  column_num = 6,
+                                  start_column = 6 )
+
+  results = app.post_json( '/completions', completion_data ).json
+  assert_that( results, has_items( CompletionEntryMatcher( 'c' ),
+                                   CompletionEntryMatcher( 'x' ),
+                                   CompletionEntryMatcher( 'y' ) ) )
+
+
 @with_setup( Setup )
 def GetCompletions_ForceSemantic_Works_test():
   app = TestApp( ycmd.app )

python/ycm/server/tests/testdata/basic.cpp

@@ -0,0 +1,13 @@
+struct Foo {
+  int x;
+  int y;
+  char c;
+};
+
+int main()
+{
+  Foo foo;
+  // The location after the dot is line 11, col 7
+  foo.
+}
+

python/ycm/server/ycmd.py

@@ -36,17 +36,19 @@ utils.AddThirdPartyFoldersToSysPath()
 import logging
 import json
 import bottle
+import argparse
+import httplib
 from bottle import run, request, response
 import server_state
 from ycm import user_options_store
 from ycm.server.responses import BuildExceptionResponse
-import argparse
+from ycm import extra_conf_store
-import httplib
 
 # num bytes for the request body buffer; request.json only works if the request
 # size is less than this
 bottle.Request.MEMFILE_MAX = 300 * 1024
 
+# TODO: rename these to _lower_case
 SERVER_STATE = None
 LOGGER = None
 app = bottle.Bottle()
@@ -72,7 +74,6 @@ def EventNotification():
     return _JsonResponse( response_data )
 
 
-
 @app.post( '/run_completer_command' )
 def RunCompleterCommand():
   LOGGER.info( 'Received command request' )
@@ -141,6 +142,13 @@ def GetDetailedDiagnostic():
   return _JsonResponse( completer.GetDetailedDiagnostic( request_data ) )
 
 
+@app.post( '/load_extra_conf_file' )
+def LoadExtraConfFile():
+  LOGGER.info( 'Received extra conf load request' )
+  request_data = request.json
+  extra_conf_store.Load( request_data[ 'filepath' ], force = True )
+
+
 @app.post( '/debug_info' )
 def DebugInfo():
   # This can't be at the top level because of possible extra conf preload
@@ -167,13 +175,19 @@ def DebugInfo():
 # The type of the param is Bottle.HTTPError
 @app.error( httplib.INTERNAL_SERVER_ERROR )
 def ErrorHandler( httperror ):
-  return _JsonResponse( BuildExceptionResponse( str( httperror.exception ),
+  return _JsonResponse( BuildExceptionResponse( httperror.exception,
                                                 httperror.traceback ) )
 
 
 def _JsonResponse( data ):
   response.set_header( 'Content-Type', 'application/json' )
-  return json.dumps( data )
+  return json.dumps( data, default = _UniversalSerialize )
+
+
+def _UniversalSerialize( obj ):
+  serialized = obj.__dict__.copy()
+  serialized[ 'TYPE' ] = type( obj ).__name__
+  return serialized
 
 
 def _GetCompleterForRequestData( request_data ):
@@ -205,6 +219,7 @@ def SetServerStateToDefaults():
   LOGGER = logging.getLogger( __name__ )
   user_options_store.LoadDefaults()
   SERVER_STATE = server_state.ServerState( user_options_store.GetAll() )
+  extra_conf_store.Reset()
 
 
 def Main():