From 608f53ecb082c04eb9eb97590fced810c2013524 Mon Sep 17 00:00:00 2001 From: Ben Jackson Date: Tue, 3 Jan 2017 17:41:13 +0000 Subject: [PATCH] Allow completers to set the start column where our interpretation doesn't agree with the completion engine --- ycmd/completers/javascript/tern_completer.py | 31 ++++++- ycmd/handlers.py | 2 +- ycmd/request_wrap.py | 60 +++++++++++--- ycmd/tests/javascript/__init__.py | 37 ++++++--- ycmd/tests/javascript/get_completions_test.py | 31 ++++++- .../javascript/testdata/node/.tern-project | 5 ++ .../javascript/testdata/node/node_test.js | 2 + ycmd/tests/request_wrap_test.py | 82 +++++++++++++++++++ 8 files changed, 220 insertions(+), 30 deletions(-) create mode 100644 ycmd/tests/javascript/testdata/node/.tern-project create mode 100644 ycmd/tests/javascript/testdata/node/node_test.js diff --git a/ycmd/completers/javascript/tern_completer.py b/ycmd/completers/javascript/tern_completer.py index 2c1d449e28..07478afb3e 100644 --- a/ycmd/completers/javascript/tern_completer.py +++ b/ycmd/completers/javascript/tern_completer.py @@ -198,9 +198,34 @@ def ComputeCandidatesInner( self, request_data ): 'omitObjectPrototype': False } - completions = self._GetResponse( query, - request_data[ 'start_codepoint' ], - request_data ).get( 'completions', [] ) + response = self._GetResponse( query, + request_data[ 'start_codepoint' ], + request_data ) + + completions = response.get( 'completions', [] ) + tern_start_codepoint = response[ 'start' ][ 'ch' ] + + # Tern returns the range of the word in the file which it is replacing. This + # may not be the same range that our "completion start column" calculation + # decided (i.e. it might not strictly be an identifier according to our + # rules). For example, when completing: + # + # require( '| + # + # with the cursor on |, tern returns something like 'test' (i.e. including + # the single-quotes). Single-quotes are not a JavaScript identifier, so + # should not normally be considered an identifier character, but by using + # our own start_codepoint calculation, the inserted string would be: + # + # require( ''test' + # + # which is clearly incorrect. It should be: + # + # require( 'test' + # + # So, we use the start position that tern tells us to use. + # We add 1 because tern offsets are 0-based and ycmd offsets are 1-based + request_data[ 'start_codepoint' ] = tern_start_codepoint + 1 def BuildDoc( completion ): doc = completion.get( 'type', 'Unknown type' ) diff --git a/ycmd/handlers.py b/ycmd/handlers.py index d54e35a6eb..a6a6105966 100644 --- a/ycmd/handlers.py +++ b/ycmd/handlers.py @@ -121,7 +121,7 @@ def GetCompletions(): return _JsonResponse( BuildCompletionResponse( completions if completions else [], - request_data.CompletionStartColumn(), + request_data[ 'start_column' ], errors = errors ) ) diff --git a/ycmd/request_wrap.py b/ycmd/request_wrap.py index d5b09ea3f9..f81c30dbd3 100644 --- a/ycmd/request_wrap.py +++ b/ycmd/request_wrap.py @@ -40,36 +40,45 @@ def __init__( self, request, validate = True ): if validate: EnsureRequestValid( request ) self._request = request + + # Maps the keys returned by this objects __getitem__ to a # tuple of + # ( getter_method, setter_method ). Values computed by getter_method (or set + # by setter_method) are cached in _cached_computed. setter_method may be + # None for read-only items. self._computed_key = { # Unicode string representation of the current line - 'line_value': self._CurrentLine, + 'line_value': ( self._CurrentLine, None ), # The calculated start column, as a codepoint offset into the # unicode string line_value - 'start_codepoint': self.CompletionStartCodepoint, + 'start_codepoint': ( self._GetCompletionStartCodepoint, + self._SetCompletionStartCodepoint ), # The 'column_num' as a unicode codepoint offset - 'column_codepoint': (lambda: - ByteOffsetToCodepointOffset( self[ 'line_bytes' ], - self[ 'column_num' ] ) ), + 'column_codepoint': ( lambda: ByteOffsetToCodepointOffset( + self[ 'line_bytes' ], + self[ 'column_num' ] ), + None ), # Bytes string representation of the current line - 'line_bytes': lambda: ToBytes( self[ 'line_value' ] ), + 'line_bytes': ( lambda: ToBytes( self[ 'line_value' ] ), + None ), # The calculated start column, as a byte offset into the UTF-8 encoded # bytes returned by line_bytes - 'start_column': self.CompletionStartColumn, + 'start_column': ( self._GetCompletionStartColumn, + self._SetCompletionStartColumn ), # Note: column_num is the byte offset into the UTF-8 encoded bytes # returned by line_bytes # unicode string representation of the 'query' after the beginning # of the identifier to be completed - 'query': self._Query, + 'query': ( self._Query, None ), - 'filetypes': self._Filetypes, + 'filetypes': ( self._Filetypes, None ), - 'first_filetype': self._FirstFiletype, + 'first_filetype': ( self._FirstFiletype, None ), } self._cached_computed = {} @@ -78,12 +87,23 @@ def __getitem__( self, key ): if key in self._cached_computed: return self._cached_computed[ key ] if key in self._computed_key: - value = self._computed_key[ key ]() + getter, _ = self._computed_key[ key ] + value = getter() self._cached_computed[ key ] = value return value return self._request[ key ] + def __setitem__( self, key, value ): + if key in self._computed_key: + _, setter = self._computed_key[ key ] + if setter: + setter( value ) + return + + raise ValueError( 'Key "{0}" is read-only'.format( key ) ) + + def __contains__( self, key ): return key in self._computed_key or key in self._request @@ -102,18 +122,32 @@ def _CurrentLine( self ): return SplitLines( contents )[ self._request[ 'line_num' ] - 1 ] - def CompletionStartColumn( self ): + def _GetCompletionStartColumn( self ): return CompletionStartColumn( self[ 'line_value' ], self[ 'column_num' ], self[ 'first_filetype' ] ) - def CompletionStartCodepoint( self ): + def _SetCompletionStartColumn( self, column_num ): + self._cached_computed[ 'start_column' ] = column_num + self._cached_computed[ 'start_codepoint' ] = ByteOffsetToCodepointOffset( + self[ 'line_value' ], + column_num ) + + + def _GetCompletionStartCodepoint( self ): return CompletionStartCodepoint( self[ 'line_value' ], self[ 'column_num' ], self[ 'first_filetype' ] ) + def _SetCompletionStartCodepoint( self, codepoint_offset ): + self._cached_computed[ 'start_codepoint' ] = codepoint_offset + self._cached_computed[ 'start_column' ] = CodepointOffsetToByteOffset( + self[ 'line_value' ], + codepoint_offset ) + + def _Query( self ): return self[ 'line_value' ][ self[ 'start_codepoint' ] - 1 : self[ 'column_codepoint' ] - 1 diff --git a/ycmd/tests/javascript/__init__.py b/ycmd/tests/javascript/__init__.py index e18c75314f..5f3bf97e67 100644 --- a/ycmd/tests/javascript/__init__.py +++ b/ycmd/tests/javascript/__init__.py @@ -86,14 +86,29 @@ def IsolatedYcmd( test ): started, no .ycm_extra_conf.py loaded, etc). Do NOT attach it to test generators but directly to the yielded tests.""" - @functools.wraps( test ) - def Wrapper( *args, **kwargs ): - old_server_state = handlers._server_state - app = SetUpApp() - try: - with CurrentWorkingDirectory( PathToTestFile() ): - test( app, *args, **kwargs ) - finally: - StopCompleterServer( app, 'javascript' ) - handlers._server_state = old_server_state - return Wrapper + return IsolatedYcmdInDirectory( PathToTestFile() ) + + +def IsolatedYcmdInDirectory( directory ): + """Defines a decorator to be attached to tests of this package. This decorator + passes a unique ycmd application as a parameter running in the directory + supplied. It should be used on tests that change the server state in a + irreversible way (ex: a semantic subserver is stopped or restarted) or expect + a clean state (ex: no semantic subserver started, no .ycm_extra_conf.py + loaded, etc). + + Do NOT attach it to test generators but directly to the yielded tests.""" + def Decorator( test ): + @functools.wraps( test ) + def Wrapper( *args, **kwargs ): + old_server_state = handlers._server_state + app = SetUpApp() + try: + with CurrentWorkingDirectory( directory ): + test( app, *args, **kwargs ) + finally: + StopCompleterServer( app, 'javascript' ) + handlers._server_state = old_server_state + return Wrapper + + return Decorator diff --git a/ycmd/tests/javascript/get_completions_test.py b/ycmd/tests/javascript/get_completions_test.py index f53e6fcdb9..71080e52a8 100644 --- a/ycmd/tests/javascript/get_completions_test.py +++ b/ycmd/tests/javascript/get_completions_test.py @@ -28,8 +28,10 @@ from pprint import pformat import requests -from ycmd.tests.javascript import PathToTestFile, SharedYcmd -from ycmd.tests.test_utils import BuildRequest, CompletionEntryMatcher +from ycmd.tests.javascript import ( PathToTestFile, SharedYcmd, + IsolatedYcmdInDirectory ) +from ycmd.tests.test_utils import ( BuildRequest, CompletionEntryMatcher, + WaitUntilCompleterServerReady ) from ycmd.utils import ReadFile # The following properties/methods are in Object.prototype, so are present @@ -480,3 +482,28 @@ def GetCompletions_Unicode_InFile_test( app ): } ) }, } ) + + +@IsolatedYcmdInDirectory( PathToTestFile( 'node' ) ) +def GetCompletions_ChangeStartColumn_test( app ): + WaitUntilCompleterServerReady( app, 'javascript' ) + RunTest( app, { + 'description': 'the completion_start_column is updated by tern', + 'request': { + 'filetype' : 'javascript', + 'filepath' : PathToTestFile( 'node', 'node_test.js' ), + 'line_num' : 1, + 'column_num' : 17, + 'force_semantic': True, + }, + 'expect': { + 'response': requests.codes.ok, + 'data': has_entries( { + 'completions': contains( + CompletionEntryMatcher( '"path"', 'path' ) + ), + 'completion_start_column': 14, + 'errors': empty(), + } ) + }, + } ) diff --git a/ycmd/tests/javascript/testdata/node/.tern-project b/ycmd/tests/javascript/testdata/node/.tern-project new file mode 100644 index 0000000000..f71dc8690f --- /dev/null +++ b/ycmd/tests/javascript/testdata/node/.tern-project @@ -0,0 +1,5 @@ +{ + "plugins": { + "node": {} + } +} diff --git a/ycmd/tests/javascript/testdata/node/node_test.js b/ycmd/tests/javascript/testdata/node/node_test.js new file mode 100644 index 0000000000..b8d63533cf --- /dev/null +++ b/ycmd/tests/javascript/testdata/node/node_test.js @@ -0,0 +1,2 @@ +p = require( "path" ) +p.jo diff --git a/ycmd/tests/request_wrap_test.py b/ycmd/tests/request_wrap_test.py index 26b6c8bd75..9f6c6c7027 100644 --- a/ycmd/tests/request_wrap_test.py +++ b/ycmd/tests/request_wrap_test.py @@ -24,6 +24,7 @@ # Not installing aliases from python-future; it's unreliable and slow. from builtins import * # noqa +from hamcrest import assert_that, calling, equal_to, raises from nose.tools import eq_ from ycmd.utils import ToBytes @@ -228,3 +229,84 @@ def Query_UnicodeSinglecharExclusive_test(): eq_( '', RequestWrap( PrepareJson( column_num = 5, contents = 'abc.ø' ) )[ 'query' ] ) + + +def StartColumn_Set_test(): + wrap = RequestWrap( PrepareJson( column_num = 11, + contents = 'this \'test', + filetype = 'javascript' ) ) + eq_( wrap[ 'start_column' ], 7 ) + eq_( wrap[ 'start_codepoint' ], 7 ) + + wrap[ 'start_column' ] = 6 + eq_( wrap[ 'start_column' ], 6 ) + eq_( wrap[ 'start_codepoint' ], 6 ) + + +def StartColumn_SetUnicode_test(): + wrap = RequestWrap( PrepareJson( column_num = 14, + contents = '†e߆ \'test', + filetype = 'javascript' ) ) + eq_( 7, wrap[ 'start_codepoint' ] ) + eq_( 12, wrap[ 'start_column' ] ) + + wrap[ 'start_column' ] = 11 + eq_( wrap[ 'start_column' ], 11 ) + eq_( wrap[ 'start_codepoint' ], 6 ) + + +def StartCodepoint_Set_test(): + wrap = RequestWrap( PrepareJson( column_num = 11, + contents = 'this \'test', + filetype = 'javascript' ) ) + eq_( wrap[ 'start_column' ], 7 ) + eq_( wrap[ 'start_codepoint' ], 7 ) + + wrap[ 'start_codepoint' ] = 6 + eq_( wrap[ 'start_column' ], 6 ) + eq_( wrap[ 'start_codepoint' ], 6 ) + + +def StartCodepoint_SetUnicode_test(): + wrap = RequestWrap( PrepareJson( column_num = 14, + contents = '†e߆ \'test', + filetype = 'javascript' ) ) + eq_( 7, wrap[ 'start_codepoint' ] ) + eq_( 12, wrap[ 'start_column' ] ) + + wrap[ 'start_codepoint' ] = 6 + eq_( wrap[ 'start_column' ], 11 ) + eq_( wrap[ 'start_codepoint' ], 6 ) + + +def Calculated_SetMethod_test(): + assert_that( + calling( RequestWrap( PrepareJson( ) ).__setitem__).with_args( + 'line_value', '' ), + raises( ValueError, 'Key "line_value" is read-only' ) ) + + +def Calculated_SetOperator_test(): + # Differs from the above in that it use [] operator rather than __setitem__ + # directly. And it uses a different property for extra credit. + wrap = RequestWrap( PrepareJson() ) + try: + wrap[ 'query' ] = 'test' + except ValueError as error: + assert_that( str( error ), + equal_to( 'Key "query" is read-only' ) ) + else: + raise AssertionError( 'Expected setting "query" to fail' ) + + +def NonCalculated_Set_test(): + # Differs from the above in that it use [] operator rather than __setitem__ + # directly. And it uses a different property for extra credit. + wrap = RequestWrap( PrepareJson() ) + try: + wrap[ 'column_num' ] = 10 + except ValueError as error: + assert_that( str( error ), + equal_to( 'Key "column_num" is read-only' ) ) + else: + raise AssertionError( 'Expected setting "column_num" to fail' )