Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR: Improve skipping imported symbols (Outline) #16255

Merged
3 changes: 3 additions & 0 deletions .github/scripts/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ else
pip uninstall spyder-kernels -q -y
pip uninstall python-lsp-server -q -y
pip uninstall qdarkstyle -q -y

# Install jupyter-client 6 until we release spyder-kernels 2.1.1
pip install jupyter-client==6.1.12
fi

# Install subrepos in development mode
Expand Down
4 changes: 2 additions & 2 deletions external-deps/python-lsp-server/.gitrepo
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
[subrepo]
remote = https://github.com/python-lsp/python-lsp-server.git
branch = develop
commit = e802f2889ae33d45166c6cf3efbeb3a87a39c23b
parent = 1ffd2053d8c97c4573bb6633590b12291a3c7704
commit = b5b2ff02703209e800633ead5b4764ca4459e274
parent = f1612a3d40e43b49241b9df0241cd45d515aa3b4
method = merge
cmdver = 0.4.3
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ def _parse_pylint_stdio_result(document, stdout):
'C': lsp.DiagnosticSeverity.Information,
'E': lsp.DiagnosticSeverity.Error,
'F': lsp.DiagnosticSeverity.Error,
'I': lsp.DiagnosticSeverity.Information,
'R': lsp.DiagnosticSeverity.Hint,
'W': lsp.DiagnosticSeverity.Warning,
}
Expand Down
49 changes: 36 additions & 13 deletions external-deps/python-lsp-server/pylsp/plugins/symbols.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@ def pylsp_document_symbols(config, document):
# pylint: disable=too-many-nested-blocks
# pylint: disable=too-many-locals
# pylint: disable=too-many-branches
# pylint: disable=too-many-statements

symbols_settings = config.plugin_settings('jedi_symbols')
all_scopes = symbols_settings.get('all_scopes', True)
add_import_symbols = symbols_settings.get('include_import_symbols', True)
definitions = document.jedi_names(all_scopes=all_scopes)
symbols = []
exclude = set({})
redefinitions = {}

while definitions != []:
d = definitions.pop(0)

Expand All @@ -33,27 +36,47 @@ def pylsp_document_symbols(config, document):
if ' import ' in code or 'import ' in code:
continue

# Skip comparing module names.
# Skip imported symbols comparing module names.
sym_full_name = d.full_name
module_name = document.dot_path
document_dot_path = document.dot_path
if sym_full_name is not None:
# module_name returns where the symbol is imported, whereas
# full_name says where it really comes from. So if the parent
# modules in full_name are not in module_name, it means the
# symbol was not defined there.
# Note: The last element of sym_full_name is the symbol itself,
# so we don't need to use it below.
# We assume a symbol is imported from another module to start
# with.
imported_symbol = True
for mod in sym_full_name.split('.')[:-1]:
if mod in module_name:
imported_symbol = False

# The last element of sym_full_name is the symbol itself, so
# we need to discard it to do module comparisons below.
if '.' in sym_full_name:
sym_module_name = sym_full_name.rpartition('.')[0]

# This is necessary to display symbols in init files (the checks
# below fail without it).
if document_dot_path.endswith('__init__'):
document_dot_path = document_dot_path.rpartition('.')[0]

# document_dot_path is the module where the symbol is imported,
# whereas sym_module_name is the one where it was declared.
if sym_module_name.startswith(document_dot_path):
# If sym_module_name starts with the same string as document_dot_path,
# we can safely assume it was declared in the document.
imported_symbol = False
elif sym_module_name.split('.')[0] in document_dot_path.split('.'):
# If the first module in sym_module_name is one of the modules in
# document_dot_path, we need to check if sym_module_name starts
# with the modules in document_dot_path.
document_mods = document_dot_path.split('.')
for i in range(1, len(document_mods) + 1):
submod = '.'.join(document_mods[-i:])
if sym_module_name.startswith(submod):
imported_symbol = False
break

# When there's no __init__.py next to a file or in one of its
# parents, the check above fails. However, Jedi has a nice way
# parents, the checks above fail. However, Jedi has a nice way
# to tell if the symbol was declared in the same file: if
# full_name starts by __main__.
if imported_symbol:
if not sym_full_name.startswith('__main__'):
if not sym_module_name.startswith('__main__'):
continue

try:
Expand Down
2 changes: 2 additions & 0 deletions external-deps/python-lsp-server/test/test_language_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ def test_exit_with_parent_process_died(client_exited_server): # pylint: disable
assert not client_exited_server.client_thread.is_alive()


@flaky(max_runs=10, min_passes=1)
@pytest.mark.skipif(sys.platform.startswith('linux'), reason='Fails on linux')
def test_not_exit_without_check_parent_process_flag(client_server): # pylint: disable=redefined-outer-name
response = client_server._endpoint.request('initialize', {
Expand All @@ -112,6 +113,7 @@ def test_not_exit_without_check_parent_process_flag(client_server): # pylint: d
assert 'capabilities' in response


@flaky(max_runs=10, min_passes=1)
@pytest.mark.skipif(RUNNING_IN_CI, reason='This test is hanging on CI')
def test_missing_message(client_server): # pylint: disable=redefined-outer-name
with pytest.raises(JsonRpcMethodNotFound):
Expand Down
1 change: 1 addition & 0 deletions pytest.ini
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ markers =
change_directory
use_startup_wdir: Test startup workingdir CONF
preload_project: Preload a project on the main window
preload_complex_project: Preload a complex project on the main window
no_xvfb
external_interpreter
test_environment_interpreter
15 changes: 15 additions & 0 deletions spyder/app/tests/script_outline_1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from math import cos
from numpy import (
linspace)

def foo(x):
return x

class MyClass:
C = 1

def one(self):
return 1

def two(self):
return 2
18 changes: 18 additions & 0 deletions spyder/app/tests/script_outline_2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from math import cos
from numpy import (
linspace)

from file1 import (
foo)

def bar(x):
return x

class MyOtherClass:
D = 1

def three(self):
return 3

def four(self):
return 4
18 changes: 18 additions & 0 deletions spyder/app/tests/script_outline_3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from math import cos
from numpy import (
linspace)

from subdir.a import (
bar)

def baz(x):
return x

class AnotherClass:
E = 1

def five(self):
return 5

def six(self):
return 4
101 changes: 67 additions & 34 deletions spyder/app/tests/test_mainwindow.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
from spyder.plugins.layout.layouts import DefaultLayouts
from spyder.plugins.projects.api import EmptyProject
from spyder.py3compat import PY2, to_text_string
from spyder.utils import encoding
from spyder.utils.misc import remove_backslashes
from spyder.utils.clipboard_helper import CLIPBOARD_HELPER
from spyder.widgets.dock import DockTitleBar
Expand Down Expand Up @@ -183,10 +184,14 @@ def remove_fake_entrypoints():
pass


def read_asset_file(filename):
"""Read contents of an asset file."""
return encoding.read(osp.join(LOCATION, filename))[0]


# =============================================================================
# ---- Fixtures
# =============================================================================

@pytest.fixture
def main_window(request, tmpdir):
"""Main Window fixture"""
Expand Down Expand Up @@ -223,13 +228,34 @@ def main_window(request, tmpdir):
else:
CONF.set('main', 'single_instance', False)

# Check if we need to preload a project in a give test
# Check if we need to load a simple project to the interface
preload_project = request.node.get_closest_marker('preload_project')

if preload_project:
# Create project directory
project = tmpdir.mkdir('test_project')
project_path = str(project)

# Create Spyder project
spy_project = EmptyProject(project_path)
CONF.set('project_explorer', 'current_project_path', project_path)

# Add a file to the project
file = project.join('file.py')
file.write(read_asset_file('script_outline_1.py'))
spy_project.set_recent_files([str(file)])
else:
CONF.set('project_explorer', 'current_project_path', None)

# Check if we need to preload a complex project in a give test
preload_complex_project = request.node.get_closest_marker(
'preload_complex_project')

if preload_complex_project:
# Create project
project = tmpdir.mkdir('test_project')
project_subdir = project.mkdir('subdir')
project_sub_subdir = project_subdir.mkdir('sub_subdir')

# Create directories out of the project
out_of_project_1 = tmpdir.mkdir('out_of_project_1')
Expand All @@ -242,40 +268,38 @@ def main_window(request, tmpdir):
CONF.set('project_explorer', 'current_project_path', project_path)

# Add some files to project. This is necessary to test that we get
# symgbols for all these files.
# symbols for all these files.
abs_filenames = []
filenames_to_create = {
project: ['file1.py', 'file2.py', 'file3.txt', '__init__.py'],
project_subdir: ['a.py', '__init__.py'],
out_of_project_1: ['b.py'],
out_of_project_2: ['c.py', '__init__.py'],
out_of_project_1_subdir: ['d.py', '__init__.py'],
out_of_project_2_subdir: ['e.py']
project_sub_subdir: ['b.py', '__init__.py'],
out_of_project_1: ['c.py'],
out_of_project_2: ['d.py', '__init__.py'],
out_of_project_1_subdir: ['e.py', '__init__.py'],
out_of_project_2_subdir: ['f.py']
}

for path in filenames_to_create.keys():
filenames = filenames_to_create[path]
for filename in filenames:
f = path.join(filename)
abs_filenames.append(str(f))
file = path.join(filename)
abs_filenames.append(str(file))
if osp.splitext(filename)[1] == '.py':
code = dedent(
"""
from math import cos
from numpy import (
linspace)

def f(x):
return x
"""
)
f.write(code)
if path == project_subdir:
code = read_asset_file('script_outline_2.py')
elif path == project_sub_subdir:
code = read_asset_file('script_outline_3.py')
else:
code = read_asset_file('script_outline_1.py')
file.write(code)
else:
f.write("Hello world!")
file.write("Hello world!")

spy_project.set_recent_files(abs_filenames)
else:
CONF.set('project_explorer', 'current_project_path', None)
if not preload_project:
CONF.set('project_explorer', 'current_project_path', None)

# Get config values passed in parametrize and apply them
try:
Expand Down Expand Up @@ -3718,6 +3742,7 @@ def hello():
@flaky(max_runs=3)
@pytest.mark.use_introspection
@pytest.mark.preload_project
@pytest.mark.skipif(os.name == 'nt', reason='Times out on Windows')
def test_ordering_lsp_requests_at_startup(main_window, qtbot):
"""
Test the ordering of requests we send to the LSP at startup when a
Expand Down Expand Up @@ -3822,7 +3847,7 @@ def test_tour_message(main_window, qtbot):
@pytest.mark.slow
@flaky(max_runs=3)
@pytest.mark.use_introspection
@pytest.mark.preload_project
@pytest.mark.preload_complex_project
@pytest.mark.skipif(not sys.platform.startswith('linux'),
reason="Only works on Linux")
def test_update_outline(main_window, qtbot, tmpdir):
Expand All @@ -3842,12 +3867,12 @@ def test_update_outline(main_window, qtbot, tmpdir):
]

# Wait a bit for trees to be filled
qtbot.wait(20000)
qtbot.wait(25000)

# Assert all Python editors are filled
assert all(
[
len(treewidget.editor_tree_cache[editor.get_id()]) == 1
len(treewidget.editor_tree_cache[editor.get_id()]) == 4
for editor in editors_py
]
)
Expand All @@ -3872,21 +3897,29 @@ def test_update_outline(main_window, qtbot, tmpdir):
# Assert spinner is not shown
assert not outline_explorer.get_widget()._spinner.isSpinning()

# Set one file as session without projects
prev_file = tmpdir.join("foo.py")
prev_file.write("def zz(x):\n"
" return x**2\n")
CONF.set('editor', 'filenames', [str(prev_file)])
# Set some files as session without projects
prev_filenames = ["prev_file_1.py", "prev_file_2.py"]
prev_paths = []
for fname in prev_filenames:
file = tmpdir.join(fname)
file.write(read_asset_file("script_outline_1.py"))
prev_paths.append(str(file))

CONF.set('editor', 'filenames', prev_paths)

# Close project to open that file automatically
main_window.projects.close_project()

# Wait a bit for its tree to be filled
qtbot.wait(1000)
qtbot.wait(3000)

# Assert the editor was filled
editor = list(treewidget.editor_ids.keys())[0]
assert len(treewidget.editor_tree_cache[editor.get_id()]) > 0
# Assert the editors were filled
assert all(
[
len(treewidget.editor_tree_cache[editor.get_id()]) == 4
for editor in treewidget.editor_ids.keys()
]
)

# Remove test file from session
CONF.set('editor', 'filenames', [])
Expand Down
6 changes: 6 additions & 0 deletions spyder/plugins/editor/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ class Editor(SpyderPluginWidget):
:py:meth:spyder.plugins.editor.widgets.editor.EditorStack.send_to_help
"""

sig_open_files_finished = Signal()
"""
This signal is emitted when the editor finished to open files.
"""

def __init__(self, parent, ignore_last_opened_files=False):
SpyderPluginWidget.__init__(self, parent)

Expand Down Expand Up @@ -3175,6 +3180,7 @@ def setup_open_files(self, close_previous_files=True):
else:
self.__load_temp_file()
self.set_create_new_file_if_empty(True)
self.sig_open_files_finished.emit()

def save_open_files(self):
"""Save the list of open files"""
Expand Down
Loading