Skip to content

Commit

Permalink
Revert ycm-core#1275
Browse files Browse the repository at this point in the history
PR ycm-core#1275 broke GoTo in python in more ways than one. See:

- ycm-core/YouCompleteMe#3458
- ycm-core#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 ycm-core#1341
for details.
  • Loading branch information
bstaletic authored and ObiWahn committed Dec 2, 2019
1 parent 0fa6a12 commit f27e460
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 109 deletions.
59 changes: 0 additions & 59 deletions ycmd/completers/python/python_completer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
54 changes: 4 additions & 50 deletions ycmd/tests/python/subcommands_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 ) },
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions ycmd/tests/python/testdata/goto/child.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
from parent import P
class C( P ):
def c( self ):
self.from_base()
2 changes: 2 additions & 0 deletions ycmd/tests/python/testdata/goto/parent.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class P:
def from_base( self ): pass

0 comments on commit f27e460

Please sign in to comment.