Skip to content

Commit

Permalink
Fix #1655 by using platform-correct path handling for relative imports
Browse files Browse the repository at this point in the history
This fixes an issue which showed up on Windows in 0.17.x where
the VSCode Python extension passes paths which have their drive
letter normalised to lower case. Since paths are (mostly) case
insensitive on Windows, that shouldn't matter. However Jedi's
handling was `==` on `str`s, meaning that completions based on
relative imports on windows didn't work.

On master this same issue had started appearing due to the partial
migration to use `Path` instead of `str` (which will avoid such
issues entirely once complete), as `Path` instances were being
compared to `str` instances without first converting.

We fix both by clarifying which is used here and then doing the
proper conversions.
  • Loading branch information
PeterJCLaw committed Aug 29, 2020
1 parent 216f976 commit cbb9954
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 7 deletions.
19 changes: 12 additions & 7 deletions jedi/inference/imports.py
Expand Up @@ -10,6 +10,7 @@
"""
import os
from pathlib import Path
from typing import List, Optional, Tuple

from parso.python import tree
from parso.tree import search_ancestor
Expand Down Expand Up @@ -123,31 +124,35 @@ def _add_error(value, name, message):
debug.warning('ImportError without origin: ' + message)


def _level_to_base_import_path(project_path, directory, level):
def _level_to_base_import_path(
project_path: Path,
directory: Path,
level: int,
) -> Tuple[Optional[List[str]], Optional[str]]:
"""
In case the level is outside of the currently known package (something like
import .....foo), we can still try our best to help the user for
completions.
"""
for i in range(level - 1):
old = directory
directory = os.path.dirname(directory)
directory = directory.parent
if old == directory:
return None, None

d = directory
level_import_paths = []
level_import_paths: List[str] = []
# Now that we are on the level that the user wants to be, calculate the
# import path for it.
while True:
if d == project_path:
return level_import_paths, d
return level_import_paths, str(d)
dir_name = os.path.basename(d)
if dir_name:
level_import_paths.insert(0, dir_name)
d = os.path.dirname(d)
d = d.parent
else:
return None, directory
return None, str(directory)


class Importer:
Expand Down Expand Up @@ -202,7 +207,7 @@ def __init__(self, inference_state, import_path, module_context, level=0):
directory = os.path.dirname(path)

base_import_path, base_directory = _level_to_base_import_path(
project_path, directory, level,
project_path, Path(directory), level,
)
if base_directory is None:
# Everything is lost, the relative import does point
Expand Down
Empty file.
2 changes: 2 additions & 0 deletions test/examples/issue1655/subdir/file3.py
@@ -0,0 +1,2 @@
def do_nothing_file3():
pass
118 changes: 118 additions & 0 deletions test/test_inference/test_imports.py
Expand Up @@ -393,6 +393,124 @@ def test_relative_imports_without_path(Script):
assert [c.name for c in script.complete()] == ['api', 'import', 'whatever']


def test_relative_import_using_paths(Script):
project_path = get_example_dir('issue1655')
project = Project(project_path, sys_path=[], smart_sys_path=False)
file_path = project_path / 'subdir' / 'other.py'

script = Script(
"from . ",
project=project,
path=file_path,
)
assert [c.name for c in script.complete()] == ['file3', 'import']

script = Script(
"from . import ",
project=project,
path=file_path,
)
assert [c.name for c in script.complete()] == [
'file3',
'__doc__',
'__file__',
'__name__',
'__package__',
]

script = Script(
"from . import file3\n\nfile3.",
project=project,
path=file_path,
)
assert [c.name for c in script.complete()] == [
'do_nothing_file3',
'__doc__',
'__file__',
'__name__',
'__package__',
]


def test_relative_import_using_strs(Script):
project_path = get_example_dir('issue1655')
project = Project(str(project_path), sys_path=[], smart_sys_path=False)
file_path = project_path / 'subdir' / 'other.py'

script = Script(
"from . ",
project=project,
path=str(file_path),
)
assert [c.name for c in script.complete()] == ['file3', 'import']

script = Script(
"from . import ",
project=project,
path=str(file_path),
)
assert [c.name for c in script.complete()] == [
'file3',
'__doc__',
'__file__',
'__name__',
'__package__',
]

script = Script(
"from . import file3\n\nfile3.",
project=project,
path=str(file_path),
)
assert [c.name for c in script.complete()] == [
'do_nothing_file3',
'__doc__',
'__file__',
'__name__',
'__package__',
]


@pytest.mark.skipif("sys.platform!='win32'")
def test_relative_import_case_insensitive_windows(Script):
project_path = get_example_dir('issue1655')
project = Project(str(project_path).lower(), sys_path=[], smart_sys_path=False)
file_path = project_path / 'subdir' / 'other.py'

script = Script(
"from . ",
project=project,
path=file_path,
)
assert [c.name for c in script.complete()] == ['file3', 'import']

script = Script(
"from . import ",
project=project,
path=file_path,
)
assert [c.name for c in script.complete()] == [
'file3',
'__doc__',
'__file__',
'__name__',
'__package__',
]

script = Script(
"from . import file3\n\nfile3.",
project=project,
path=file_path,
)
assert [c.name for c in script.complete()] == [
'do_nothing_file3',
'__doc__',
'__file__',
'__name__',
'__package__',
]


def test_relative_import_out_of_file_system(Script):
code = "from " + '.' * 100
assert not Script(code).complete()
Expand Down

0 comments on commit cbb9954

Please sign in to comment.