From 03b4177d3dd741df18651e87e0283d55192a6d34 Mon Sep 17 00:00:00 2001 From: Sam Roeca Date: Thu, 28 Nov 2019 23:04:08 -0500 Subject: [PATCH 1/5] Fix: no longer shows folders recursively to root In the previous implementation, Jedi would's traverse_parents function traversed parent directories to the system root every time. This would inadvertently add every folder to the system root every time. Obviously, this is not the behavior desired for the import system. This pull request provides a new argument to the traverse_parents function, "root", which represents the root parent for the search. This argument defaults to None, thereby preserving the existing behavior of the function. I chose to duplicate some code for performance reasons. Since I'm trying to avoid too much path manipulation magic, we do: * a search to a valid specified root, OR * a simple upward search until hitting the system root when there is no valid root specified. --- jedi/api/project.py | 5 ++++- jedi/common/utils.py | 26 +++++++++++++++++++++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/jedi/api/project.py b/jedi/api/project.py index f39ba90f..d4ef879a 100644 --- a/jedi/api/project.py +++ b/jedi/api/project.py @@ -110,7 +110,10 @@ class Project(object): suffixed += discover_buildout_paths(inference_state, inference_state.script_path) if add_parent_paths: - traversed = list(traverse_parents(inference_state.script_path)) + traversed = list(traverse_parents( + inference_state.script_path, + root=self._path, + )) # AFAIK some libraries have imports like `foo.foo.bar`, which # leads to the conclusion to by default prefer longer paths diff --git a/jedi/common/utils.py b/jedi/common/utils.py index bc71cafd..2427f7e9 100644 --- a/jedi/common/utils.py +++ b/jedi/common/utils.py @@ -2,15 +2,31 @@ import os from contextlib import contextmanager -def traverse_parents(path, include_current=False): +def traverse_parents(path, root=None, include_current=False): + """Iterate directories from a path to search root + + :path: the path of the script/directory to check. + :root: the root of the upward search. Assumes the system root if root is + None. + :include_current: includes the current file / directory. + + If the root path is not a substring of the provided path, assume the root + search path as well. + """ if not include_current: path = os.path.dirname(path) previous = None - while previous != path: - yield path - previous = path - path = os.path.dirname(path) + if root is None or not path.startswith(root): + while previous != path: + yield path + previous = path + path = os.path.dirname(path) + else: + while previous != root: + yield path + previous = path + path = os.path.dirname(path) @contextmanager From 4bc4f167e93855a77c3a7264fa40815497dda135 Mon Sep 17 00:00:00 2001 From: Sam Roeca Date: Fri, 29 Nov 2019 20:11:23 -0500 Subject: [PATCH 2/5] Revert "Fix: no longer shows folders recursively to root" This reverts commit 03b4177d3dd741df18651e87e0283d55192a6d34. --- jedi/api/project.py | 5 +---- jedi/common/utils.py | 26 +++++--------------------- 2 files changed, 6 insertions(+), 25 deletions(-) diff --git a/jedi/api/project.py b/jedi/api/project.py index d4ef879a..f39ba90f 100644 --- a/jedi/api/project.py +++ b/jedi/api/project.py @@ -110,10 +110,7 @@ class Project(object): suffixed += discover_buildout_paths(inference_state, inference_state.script_path) if add_parent_paths: - traversed = list(traverse_parents( - inference_state.script_path, - root=self._path, - )) + traversed = list(traverse_parents(inference_state.script_path)) # AFAIK some libraries have imports like `foo.foo.bar`, which # leads to the conclusion to by default prefer longer paths diff --git a/jedi/common/utils.py b/jedi/common/utils.py index 2427f7e9..bc71cafd 100644 --- a/jedi/common/utils.py +++ b/jedi/common/utils.py @@ -2,31 +2,15 @@ import os from contextlib import contextmanager -def traverse_parents(path, root=None, include_current=False): - """Iterate directories from a path to search root - - :path: the path of the script/directory to check. - :root: the root of the upward search. Assumes the system root if root is - None. - :include_current: includes the current file / directory. - - If the root path is not a substring of the provided path, assume the root - search path as well. - """ +def traverse_parents(path, include_current=False): if not include_current: path = os.path.dirname(path) previous = None - if root is None or not path.startswith(root): - while previous != path: - yield path - previous = path - path = os.path.dirname(path) - else: - while previous != root: - yield path - previous = path - path = os.path.dirname(path) + while previous != path: + yield path + previous = path + path = os.path.dirname(path) @contextmanager From c2fd7b3104232bb65de8736658ec09ea742d437b Mon Sep 17 00:00:00 2001 From: Sam Roeca Date: Fri, 29 Nov 2019 21:12:12 -0500 Subject: [PATCH 3/5] Fix: upward search omits unnecessary paths In the previous implementation, Jedi's traverse_parents function traversed parent directories to the system root every time. This would inadvertently add every folder to the system root every time. Obviously, this is not the behavior desired for the import system. This commit collects directories in an upward search until we: 1. Hit any directory without an __init__.py, AND 2. Are above self._path. --- jedi/api/project.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/jedi/api/project.py b/jedi/api/project.py index f39ba90f..6ce05270 100644 --- a/jedi/api/project.py +++ b/jedi/api/project.py @@ -110,7 +110,17 @@ class Project(object): suffixed += discover_buildout_paths(inference_state, inference_state.script_path) if add_parent_paths: - traversed = list(traverse_parents(inference_state.script_path)) + # Collect directories in upward search until we: + # 1. Hit any directory without an __init__.py, AND + # 2. Are above self._path. + traversed = [] + no_init = False + for parent_path in traverse_parents(inference_state.script_path): + if not os.path.isfile(os.path.join(parent_path, "__init__.py")): + no_init = True + if no_init and not parent_path.startswith(self._path): + break + traversed.append(parent_path) # AFAIK some libraries have imports like `foo.foo.bar`, which # leads to the conclusion to by default prefer longer paths From 1ba83414a5edf3ff06606cf45b4bb20ca15a38a0 Mon Sep 17 00:00:00 2001 From: Sam Roeca Date: Sat, 30 Nov 2019 10:14:28 -0500 Subject: [PATCH 4/5] Change search strategy for adding parent paths: 1. skip dirs with __init__.py 2. Stop immediately when above self._path --- jedi/api/project.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/jedi/api/project.py b/jedi/api/project.py index 6ce05270..6c75cd79 100644 --- a/jedi/api/project.py +++ b/jedi/api/project.py @@ -110,16 +110,15 @@ class Project(object): suffixed += discover_buildout_paths(inference_state, inference_state.script_path) if add_parent_paths: - # Collect directories in upward search until we: - # 1. Hit any directory without an __init__.py, AND - # 2. Are above self._path. + # Collect directories in upward search by: + # 1. Skipping directories with __init__.py + # 2. Stopping immediately when above self._path traversed = [] - no_init = False for parent_path in traverse_parents(inference_state.script_path): - if not os.path.isfile(os.path.join(parent_path, "__init__.py")): - no_init = True - if no_init and not parent_path.startswith(self._path): + if not parent_path.startswith(self._path): break + if os.path.isfile(os.path.join(parent_path, "__init__.py")): + continue traversed.append(parent_path) # AFAIK some libraries have imports like `foo.foo.bar`, which From 86071dda542e88cf06d98e30b08d3077a6304fc3 Mon Sep 17 00:00:00 2001 From: Dave Halter Date: Sun, 1 Dec 2019 00:10:11 +0100 Subject: [PATCH 5/5] Use a different sys path for import completions and import type inference Fix tests of the #1451 pull request --- jedi/api/project.py | 6 ++++-- jedi/inference/imports.py | 13 ++++++++----- test/completion/on_import.py | 6 +++++- test/test_inference/test_imports.py | 2 ++ 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/jedi/api/project.py b/jedi/api/project.py index 6c75cd79..6b76c8dc 100644 --- a/jedi/api/project.py +++ b/jedi/api/project.py @@ -94,7 +94,8 @@ class Project(object): return sys_path @inference_state_as_method_param_cache() - def _get_sys_path(self, inference_state, environment=None, add_parent_paths=True): + def _get_sys_path(self, inference_state, environment=None, + add_parent_paths=True, add_init_paths=False): """ Keep this method private for all users of jedi. However internally this one is used like a public method. @@ -117,7 +118,8 @@ class Project(object): for parent_path in traverse_parents(inference_state.script_path): if not parent_path.startswith(self._path): break - if os.path.isfile(os.path.join(parent_path, "__init__.py")): + if not add_init_paths \ + and os.path.isfile(os.path.join(parent_path, "__init__.py")): continue traversed.append(parent_path) diff --git a/jedi/inference/imports.py b/jedi/inference/imports.py index 0292d45b..e6f359cf 100644 --- a/jedi/inference/imports.py +++ b/jedi/inference/imports.py @@ -272,12 +272,15 @@ class Importer(object): for name in self.import_path ) - def _sys_path_with_modifications(self): + def _sys_path_with_modifications(self, is_completion): if self._fixed_sys_path is not None: return self._fixed_sys_path sys_path_mod = ( - self._inference_state.get_sys_path() + # For import completions we don't want to see init paths, but for + # inference we want to show the user as much as possible. + # See GH #1446. + self._inference_state.get_sys_path(add_init_paths=not is_completion) + sys_path.check_sys_path_modifications(self._module_context) ) @@ -297,7 +300,7 @@ class Importer(object): force_unicode(i.value if isinstance(i, tree.Name) else i) for i in self.import_path ) - sys_path = self._sys_path_with_modifications() + sys_path = self._sys_path_with_modifications(is_completion=False) value_set = [None] for i, name in enumerate(self.import_path): @@ -326,7 +329,7 @@ class Importer(object): for name in self._inference_state.compiled_subprocess.get_builtin_module_names()] if search_path is None: - search_path = self._sys_path_with_modifications() + search_path = self._sys_path_with_modifications(is_completion=True) for name in iter_module_names(self._inference_state, search_path): if in_module is None: @@ -355,7 +358,7 @@ class Importer(object): extname = modname[len('flask_'):] names.append(ImportName(self._module_context, extname)) # Now the old style: ``flaskext.foo`` - for dir in self._sys_path_with_modifications(): + for dir in self._sys_path_with_modifications(is_completion=True): flaskext = os.path.join(dir, 'flaskext') if os.path.isdir(flaskext): names += self._get_module_names([flaskext]) diff --git a/test/completion/on_import.py b/test/completion/on_import.py index 733d741e..203b8b9b 100644 --- a/test/completion/on_import.py +++ b/test/completion/on_import.py @@ -20,8 +20,12 @@ def builtin_test(): #? ['sqlite3'] import sqlite3 -#? ['classes'] +# classes is a local module that has an __init__.py and can therefore not be +# found. test can be found. +#? [] import classes +#? ['test'] +import test #? ['timedelta'] from datetime import timedel diff --git a/test/test_inference/test_imports.py b/test/test_inference/test_imports.py index a79f37eb..6bab3a6e 100644 --- a/test/test_inference/test_imports.py +++ b/test/test_inference/test_imports.py @@ -115,6 +115,8 @@ def test_find_module_not_package_zipped(Script, inference_state, environment): def test_import_not_in_sys_path(Script): """ non-direct imports (not in sys.path) + + This is in the end just a fallback. """ a = Script(path='module.py', line=5).goto_definitions() assert a[0].name == 'int'