From f27e460a32302767b9435a4d47d74c7b20c6a6fd Mon Sep 17 00:00:00 2001 From: Boris Staletic Date: Sat, 19 Oct 2019 18:32:55 +0200 Subject: [PATCH] Revert #1275 PR #1275 broke GoTo in python in more ways than one. See: - https://github.com/ycm-core/YouCompleteMe/issues/3458 - https://github.com/ycm-core/ycmd/issues/1341 Changes in the behaviour: - GoTo, GoToDefinition and GoToDeclaration now do what GoToType used to do. - We don't expose jedi's `goto_assignments` which can be useful, but we do have GoToReferences that calls jedi's `usages()`. - This means that `GoTo` on `self.x` will now jump to the definition of the type of `x`, while `GoToReferences` will do what is expected. - The `goto_usages` for this case returns "weird" locations, see #1341 for details. --- ycmd/completers/python/python_completer.py | 59 ---------------------- ycmd/tests/python/subcommands_test.py | 54 ++------------------ ycmd/tests/python/testdata/goto/child.py | 4 ++ ycmd/tests/python/testdata/goto/parent.py | 2 + 4 files changed, 10 insertions(+), 109 deletions(-) create mode 100644 ycmd/tests/python/testdata/goto/child.py create mode 100644 ycmd/tests/python/testdata/goto/parent.py diff --git a/ycmd/completers/python/python_completer.py b/ycmd/completers/python/python_completer.py index ff89bdb84f..0affd41464 100644 --- a/ycmd/completers/python/python_completer.py +++ b/ycmd/completers/python/python_completer.py @@ -24,7 +24,6 @@ from ycmd import extra_conf_store, responses from ycmd.completers.completer import Completer -from ycmd.completers.completer_utils import GetFileContents from ycmd.utils import ExpandVariablesInPath, FindExecutable, LOGGER import os @@ -174,21 +173,6 @@ def _GetJediScript( self, request_data ): environment = environment ) - def _GetJediScriptForDefinition( self, request_data, definition ): - path = definition.module_path - source = GetFileContents( request_data, path ) - line = definition.line - column = definition.column - environment = self._EnvironmentForRequest( request_data ) - sys_path = self._SysPathForFile( request_data, environment ) - return jedi.Script( source, - line, - column, - path, - sys_path = sys_path, - environment = environment ) - - # This method must be called under Jedi's lock. def _GetExtraData( self, completion ): if completion.module_path and completion.line and completion.column: @@ -234,8 +218,6 @@ def GetSubcommandsMap( self ): self._GoToDefinition( request_data ) ), 'GoToDeclaration': ( lambda self, request_data, args: self._GoToDefinition( request_data ) ), - 'GoToType' : ( lambda self, request_data, args: - self._GoToType( request_data ) ), 'GoToReferences' : ( lambda self, request_data, args: self._GoToReferences( request_data ) ), 'GetType' : ( lambda self, request_data, args: @@ -262,47 +244,6 @@ def _BuildGoToResponse( self, definitions ): def _GoToDefinition( self, request_data ): - def _GoToDefinitionsWithSameName( definition ): - module_path = definition.module_path - if not module_path: - return [] - name = definition.name - line = definition.line - column = definition.column - - definitions = self._GetJediScriptForDefinition( - request_data, definition ).goto_assignments() - return [ d for d in definitions if d.name == name and - ( d.module_path != module_path or - d.line != line or - d.column != column ) ] - - with self._jedi_lock: - # Jedi's goto_assignments() does not jump to a different file unless the - # cursor is on an import statement or if the follow_imports parameter is - # set to True. Unfortunately, we can't use that option because it also - # jumps to the original name of aliases which is unexpected as an alias is - # a kind of definition. So, we need to traverse definitions by repeatedly - # calling goto_assignments() until we can't jump anymore. - definitions = self._GetJediScript( request_data ).goto_assignments() - keep_traversing = True - while keep_traversing: - keep_traversing = False - previous_definitions = definitions - definitions = [] - for definition in previous_definitions: - goto_definitions = _GoToDefinitionsWithSameName( definition ) - if goto_definitions: - definitions.extend( goto_definitions ) - keep_traversing = True - else: - definitions.append( definition ) - if definitions: - return self._BuildGoToResponse( definitions ) - raise RuntimeError( 'Can\'t jump to definition.' ) - - - def _GoToType( self, request_data ): with self._jedi_lock: definitions = self._GetJediScript( request_data ).goto_definitions() if definitions: diff --git a/ycmd/tests/python/subcommands_test.py b/ycmd/tests/python/subcommands_test.py index e2e7be97f4..97fe5c9949 100644 --- a/ycmd/tests/python/subcommands_test.py +++ b/ycmd/tests/python/subcommands_test.py @@ -102,55 +102,6 @@ def Subcommands_GoTo( app, test, command ): def Subcommands_GoTo_test(): - builtins_pyi = matches_regexp( 'typeshed' ) - tests = [ - # Nothing - { 'request': ( 'basic.py', 3, 5 ), 'response': 'Can\'t jump to ' - 'definition.' }, - # Keyword - { 'request': ( 'basic.py', 4, 3 ), 'response': 'Can\'t jump to ' - 'definition.' }, - # Builtin - { 'request': ( 'basic.py', 1, 4 ), 'response': ( 'basic.py', 1, 1 ) }, - { 'request': ( 'basic.py', 1, 12 ), 'response': ( builtins_pyi, 927, 7 ) }, - { 'request': ( 'basic.py', 2, 2 ), 'response': ( 'basic.py', 1, 1 ) }, - # Class - { 'request': ( 'basic.py', 4, 7 ), 'response': ( 'basic.py', 4, 7 ) }, - { 'request': ( 'basic.py', 4, 11 ), 'response': ( 'basic.py', 4, 7 ) }, - { 'request': ( 'basic.py', 7, 19 ), 'response': ( 'basic.py', 4, 7 ) }, - # Instance - { 'request': ( 'basic.py', 7, 1 ), 'response': ( 'basic.py', 7, 1 ) }, - { 'request': ( 'basic.py', 7, 11 ), 'response': ( 'basic.py', 7, 1 ) }, - { 'request': ( 'basic.py', 8, 23 ), 'response': ( 'basic.py', 7, 1 ) }, - # Instance reference - { 'request': ( 'basic.py', 8, 1 ), 'response': ( 'basic.py', 8, 1 ) }, - { 'request': ( 'basic.py', 8, 5 ), 'response': ( 'basic.py', 8, 1 ) }, - { 'request': ( 'basic.py', 9, 12 ), 'response': ( 'basic.py', 8, 1 ) }, - # Builtin from different file - { 'request': ( 'multifile1.py', 2, 30 ), - 'response': ( 'multifile3.py', 1, 1 ) }, - { 'request': ( 'multifile1.py', 4, 5 ), - 'response': ( 'multifile3.py', 1, 1 ) }, - # Function from different file - { 'request': ( 'multifile1.py', 1, 24 ), - 'response': ( 'multifile3.py', 3, 5 ) }, - { 'request': ( 'multifile1.py', 5, 4 ), - 'response': ( 'multifile3.py', 3, 5 ) }, - # Alias from different file - { 'request': ( 'multifile1.py', 2, 47 ), - 'response': ( 'multifile2.py', 1, 51 ) }, - { 'request': ( 'multifile1.py', 6, 14 ), - 'response': ( 'multifile2.py', 1, 51 ) }, - ] - - for test in tests: - # GoTo, GoToDefinition, and GoToDeclaration are identical. - yield Subcommands_GoTo, test, 'GoTo' - yield Subcommands_GoTo, test, 'GoToDefinition' - yield Subcommands_GoTo, test, 'GoToDeclaration' - - -def Subcommands_GoToType_test(): builtins_pyi = matches_regexp( 'typeshed' ) tests = [ # Nothing @@ -175,6 +126,8 @@ def Subcommands_GoToType_test(): { 'request': ( 'basic.py', 8, 1 ), 'response': ( 'basic.py', 4, 7 ) }, { 'request': ( 'basic.py', 8, 5 ), 'response': ( 'basic.py', 4, 7 ) }, { 'request': ( 'basic.py', 9, 12 ), 'response': ( 'basic.py', 4, 7 ) }, + # Member access + { 'request': ( 'child.py', 4, 12 ), 'response': ( 'parent.py', 2, 7 ) }, # Builtin from different file { 'request': ( 'multifile1.py', 2, 30 ), 'response': ( builtins_pyi, 130, 7 ) }, @@ -193,7 +146,8 @@ def Subcommands_GoToType_test(): ] for test in tests: - yield Subcommands_GoTo, test, 'GoToType' + for cmd in [ 'GoTo', 'GoToDefinition', 'GoToDeclaration' ]: + yield Subcommands_GoTo, test, cmd @SharedYcmd diff --git a/ycmd/tests/python/testdata/goto/child.py b/ycmd/tests/python/testdata/goto/child.py new file mode 100644 index 0000000000..d460c297a6 --- /dev/null +++ b/ycmd/tests/python/testdata/goto/child.py @@ -0,0 +1,4 @@ +from parent import P +class C( P ): + def c( self ): + self.from_base() diff --git a/ycmd/tests/python/testdata/goto/parent.py b/ycmd/tests/python/testdata/goto/parent.py new file mode 100644 index 0000000000..6022a8ef49 --- /dev/null +++ b/ycmd/tests/python/testdata/goto/parent.py @@ -0,0 +1,2 @@ +class P: + def from_base( self ): pass