From dd3abdc8cbb938557f9ca05804fee9e09f3c49f9 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Sat, 27 Apr 2019 14:07:32 +0200 Subject: [PATCH 1/2] Removes "global" diagnostics, lookup via registry instead of imported functions --- plugin/code_actions.py | 2 +- plugin/core/diagnostics.py | 144 +++++++++++--------------------- plugin/core/registry.py | 4 +- plugin/core/test_diagnostics.py | 60 +++++++++++++ plugin/core/test_windows.py | 32 +++---- plugin/core/windows.py | 39 ++++----- plugin/diagnostics.py | 37 +++++++- plugin/hover.py | 2 +- 8 files changed, 176 insertions(+), 144 deletions(-) create mode 100644 plugin/core/test_diagnostics.py diff --git a/plugin/code_actions.py b/plugin/code_actions.py index e00a230dc..be233608f 100644 --- a/plugin/code_actions.py +++ b/plugin/code_actions.py @@ -9,7 +9,7 @@ from .core.registry import client_for_view, LspTextCommand from .core.protocol import Request -from .core.diagnostics import get_point_diagnostics +from .diagnostics import get_point_diagnostics from .core.url import filename_to_uri from .core.views import region_to_range from .core.registry import session_for_view diff --git a/plugin/core/diagnostics.py b/plugin/core/diagnostics.py index cfaded43e..8f1f3f339 100644 --- a/plugin/core/diagnostics.py +++ b/plugin/core/diagnostics.py @@ -1,48 +1,19 @@ -import sublime - from .logging import debug from .url import uri_to_filename from .protocol import Diagnostic -from .events import global_events -from .views import range_to_region -from .windows import WindowLike, ViewLike - assert Diagnostic try: + import sublime from typing import Any, List, Dict, Tuple, Callable, Optional + assert sublime assert Any and List and Dict and Tuple and Callable and Optional - assert ViewLike and WindowLike except ImportError: pass -global_diagnostics = dict( -) # type: Dict[int, Dict[str, Dict[str, List[Diagnostic]]]] - - -def update_file_diagnostics(window: sublime.Window, file_path: str, source: str, - diagnostics: 'List[Diagnostic]') -> bool: - updated = False - if diagnostics: - file_diagnostics = global_diagnostics.setdefault(window.id(), dict()).setdefault( - file_path, dict()) - file_diagnostics[source] = diagnostics - updated = True - else: - if window.id() in global_diagnostics: - window_diagnostics = global_diagnostics[window.id()] - if file_path in window_diagnostics: - if source in window_diagnostics[file_path]: - updated = True - del window_diagnostics[file_path][source] - if not window_diagnostics[file_path]: - del window_diagnostics[file_path] - return updated - - class DiagnosticsUpdate(object): - def __init__(self, window: sublime.Window, client_name: str, + def __init__(self, window, client_name: str, file_path: str, diagnostics: 'List[Diagnostic]') -> 'None': self.window = window self.client_name = client_name @@ -50,72 +21,53 @@ def __init__(self, window: sublime.Window, client_name: str, self.diagnostics = diagnostics -def handle_client_diagnostics(window: sublime.Window, client_name: str, update: dict): - maybe_file_uri = update.get('uri') - if maybe_file_uri is not None: - file_path = uri_to_filename(maybe_file_uri) - - diagnostics = list( - Diagnostic.from_lsp(item) for item in update.get('diagnostics', [])) - - if update_file_diagnostics(window, file_path, client_name, diagnostics): - global_events.publish("document.diagnostics", - DiagnosticsUpdate(window, client_name, file_path, diagnostics)) - else: - debug('missing uri in diagnostics update') -# TODO: expose updates to features - +class WindowDiagnostics(object): -def remove_diagnostics(view: sublime.View, client_name: str): - """Removes diagnostics for a file - """ - window = view.window() or sublime.active_window() + def __init__(self): + self._diagnostics = {} # type: Dict[str, Dict[str, List[Diagnostic]]] + self._on_updated = None # type: Optional[Callable] - file_path = view.file_name() - if file_path: - if update_file_diagnostics(window, file_path, client_name, []): - global_events.publish("document.diagnostics", DiagnosticsUpdate(window, client_name, file_path, [])) + def get(self) -> 'Dict[str, Dict[str, List[Diagnostic]]]': + return self._diagnostics + def set_on_updated(self, update_handler: 'Callable'): + self._on_updated = update_handler -class GlobalDiagnostics(object): - def update(self, window: 'Any', client_name: str, update: dict): - handle_client_diagnostics(window, client_name, update) + def get_by_path(self, file_path: str) -> 'List[Diagnostic]': + view_diagnostics = [] + if file_path in self._diagnostics: + for origin in self._diagnostics[file_path]: + view_diagnostics.extend(self._diagnostics[file_path][origin]) + return view_diagnostics - def remove(self, view: 'Any', client_name: str): - """Removes diagnostics for a file if no views exist for it - """ - remove_diagnostics(view, client_name) - - -def get_line_diagnostics(view, point): - row, _ = view.rowcol(point) - diagnostics = get_diagnostics_for_view(view) - return tuple( - diagnostic for diagnostic in diagnostics - if diagnostic.range.start.row <= row <= diagnostic.range.end.row - ) - - -def get_point_diagnostics(view, point): - diagnostics = get_diagnostics_for_view(view) - return tuple( - diagnostic for diagnostic in diagnostics - if range_to_region(diagnostic.range, view).contains(point) - ) - - -def get_window_diagnostics(window: sublime.Window) -> 'Optional[Dict[str, Dict[str, List[Diagnostic]]]]': - return global_diagnostics.get(window.id()) - - -def get_diagnostics_for_view(view: sublime.View) -> 'List[Diagnostic]': - view_diagnostics = [] - window = view.window() - file_path = view.file_name() - if file_path and window: - if window.id() in global_diagnostics: - file_diagnostics = global_diagnostics[window.id()] - if file_path in file_diagnostics: - for origin in file_diagnostics[file_path]: - view_diagnostics.extend(file_diagnostics[file_path][origin]) - return view_diagnostics + def update(self, file_path: str, client_name: str, diagnostics: 'List[Diagnostic]') -> bool: + updated = False + if diagnostics: + file_diagnostics = self._diagnostics.setdefault(file_path, dict()) + file_diagnostics[client_name] = diagnostics + updated = True + else: + if file_path in self._diagnostics: + if client_name in self._diagnostics[file_path]: + updated = True + del self._diagnostics[file_path][client_name] + if not self._diagnostics[file_path]: + del self._diagnostics[file_path] + return updated + + def handle_client_diagnostics(self, client_name: str, update: dict): + maybe_file_uri = update.get('uri') + if maybe_file_uri is not None: + file_path = uri_to_filename(maybe_file_uri) + + diagnostics = list( + Diagnostic.from_lsp(item) for item in update.get('diagnostics', [])) + + if self.update(file_path, client_name, diagnostics): + if self._on_updated: + self._on_updated(file_path, client_name, diagnostics) + else: + debug('missing uri in diagnostics update') + + def remove(self, file_path: str, client_name: str): + self.update(file_path, client_name, []) diff --git a/plugin/core/registry.py b/plugin/core/registry.py index 84ca8df10..ed37da376 100644 --- a/plugin/core/registry.py +++ b/plugin/core/registry.py @@ -1,6 +1,5 @@ import sublime import sublime_plugin -from .diagnostics import GlobalDiagnostics from .windows import WindowRegistry, DocumentHandlerFactory from .configurations import ( ConfigManager @@ -102,10 +101,9 @@ def unload_sessions(): configs = ConfigManager(client_configs.all) -diagnostics = GlobalDiagnostics() documents = DocumentHandlerFactory(sublime, settings) handlers_dispatcher = LanguageHandlerDispatcher() -windows = WindowRegistry(configs, documents, diagnostics, start_window_config, sublime, handlers_dispatcher) +windows = WindowRegistry(configs, documents, start_window_config, sublime, handlers_dispatcher) def config_for_scope(view: 'Any', point=None) -> 'Optional[ClientConfig]': diff --git a/plugin/core/test_diagnostics.py b/plugin/core/test_diagnostics.py new file mode 100644 index 000000000..3de72398d --- /dev/null +++ b/plugin/core/test_diagnostics.py @@ -0,0 +1,60 @@ +import unittest +from .diagnostics import WindowDiagnostics +from .protocol import Diagnostic, Range, Point +# from .configurations import WindowConfigManager, _merge_dicts, ConfigManager, is_supported_syntax +# from .test_session import test_config, test_language +from .test_protocol import LSP_MINIMAL_DIAGNOSTIC + + +class WindowDiagnosticsTest(unittest.TestCase): + + def test_empty_diagnostics(self): + wd = WindowDiagnostics() + self.assertEqual(wd.get_by_path(__file__), []) + + # todo: remove + + def test_updated_diagnostics(self): + wd = WindowDiagnostics() + + test_file_path = "test.py" + diag = Diagnostic('message', Range(Point(0, 0), Point(1, 1)), 1, None, dict()) + + wd.update(test_file_path, "test_server", [diag]) + view_diags = wd.get_by_path(test_file_path) + self.assertEqual(len(view_diags), 1) + self.assertEqual(view_diags[0], diag) + + wd.update(test_file_path, "test_server", []) + view_diags = wd.get_by_path(test_file_path) + self.assertEqual(len(view_diags), 0) + + def test_handle_diagnostics_update(self): + wd = WindowDiagnostics() + + test_file_path = "/test.py" + update = { + 'uri': 'file:///test.py', + 'diagnostics': [LSP_MINIMAL_DIAGNOSTIC] + } + + wd.handle_client_diagnostics("test_server", update) + + view_diags = wd.get_by_path(test_file_path) + self.assertEqual(len(view_diags), 1) + + def test_remove_diagnostics(self): + wd = WindowDiagnostics() + + test_file_path = "test.py" + diag = Diagnostic('message', Range(Point(0, 0), Point(1, 1)), 1, None, dict()) + + wd.update(test_file_path, "test_server", [diag]) + view_diags = wd.get_by_path(test_file_path) + self.assertEqual(len(view_diags), 1) + + wd.remove(test_file_path, "test_server") + + view_diags = wd.get_by_path(test_file_path) + self.assertEqual(len(view_diags), 0) + diff --git a/plugin/core/test_windows.py b/plugin/core/test_windows.py index c18a813e3..b8847a871 100644 --- a/plugin/core/test_windows.py +++ b/plugin/core/test_windows.py @@ -1,4 +1,5 @@ -from .windows import WindowManager, WindowRegistry, WindowLike, ViewLike +from .windows import WindowManager, WindowRegistry, ViewLike +from .diagnostics import WindowDiagnostics from .sessions import create_session, Session from .test_session import MockClient, test_config, test_language from .test_rpc import MockSettings @@ -219,17 +220,6 @@ def for_window(self, window, configs): return MockDocuments() -class MockDiagnostics(object): - def __init__(self): - pass - - def update(self, window: WindowLike, client_name: str, update: dict) -> None: - pass - - def remove(self, view: ViewLike, client_name: str) -> None: - pass - - def mock_start_session(window, project_path, config, on_created: 'Callable', on_ended: 'Callable'): return create_session(test_config, project_path, dict(), MockSettings(), bootstrap_client=MockClient(), @@ -241,7 +231,7 @@ class WindowRegistryTests(unittest.TestCase): def test_can_get_window_state(self): windows = WindowRegistry(TestGlobalConfigs(), TestDocumentHandlerFactory(), - MockDiagnostics(), mock_start_session, + mock_start_session, test_sublime, MockHandlerDispatcher()) test_window = MockWindow() wm = windows.lookup(test_window) @@ -251,7 +241,7 @@ def test_removes_window_state(self): global_events.reset() test_window = MockWindow([[MockView(__file__)]]) windows = WindowRegistry(TestGlobalConfigs(), TestDocumentHandlerFactory(), - MockDiagnostics(), mock_start_session, + mock_start_session, test_sublime, MockHandlerDispatcher()) wm = windows.lookup(test_window) wm.start_active_views() @@ -271,7 +261,7 @@ class WindowManagerTests(unittest.TestCase): def test_can_start_active_views(self): docs = MockDocuments() wm = WindowManager(MockWindow([[MockView(__file__)]]), MockConfigs(), docs, - MockDiagnostics(), mock_start_session, test_sublime, MockHandlerDispatcher()) + WindowDiagnostics(), mock_start_session, test_sublime, MockHandlerDispatcher()) wm.start_active_views() # session must be started (todo: verify session is ready) @@ -281,7 +271,7 @@ def test_can_start_active_views(self): def test_can_open_supported_view(self): docs = MockDocuments() window = MockWindow([[]]) - wm = WindowManager(window, MockConfigs(), docs, MockDiagnostics(), mock_start_session, test_sublime, + wm = WindowManager(window, MockConfigs(), docs, WindowDiagnostics(), mock_start_session, test_sublime, MockHandlerDispatcher()) wm.start_active_views() @@ -298,7 +288,7 @@ def test_can_open_supported_view(self): def test_can_restart_sessions(self): docs = MockDocuments() wm = WindowManager(MockWindow([[MockView(__file__)]]), MockConfigs(), docs, - MockDiagnostics(), mock_start_session, test_sublime, MockHandlerDispatcher()) + WindowDiagnostics(), mock_start_session, test_sublime, MockHandlerDispatcher()) wm.start_active_views() # session must be started (todo: verify session is ready) @@ -320,7 +310,7 @@ def test_ends_sessions_when_closed(self): docs = MockDocuments() test_window = MockWindow([[MockView(__file__)]]) wm = WindowManager(test_window, MockConfigs(), docs, - MockDiagnostics(), mock_start_session, test_sublime, MockHandlerDispatcher()) + WindowDiagnostics(), mock_start_session, test_sublime, MockHandlerDispatcher()) wm.start_active_views() # session must be started (todo: verify session is ready) @@ -341,7 +331,7 @@ def test_ends_sessions_when_quick_switching(self): docs = MockDocuments() test_window = MockWindow([[MockView(__file__)]]) wm = WindowManager(test_window, MockConfigs(), docs, - MockDiagnostics(), mock_start_session, test_sublime, MockHandlerDispatcher()) + WindowDiagnostics(), mock_start_session, test_sublime, MockHandlerDispatcher()) wm.start_active_views() # session must be started (todo: verify session is ready) @@ -367,7 +357,7 @@ def test_ends_sessions_when_quick_switching(self): def test_offers_restart_on_crash(self): docs = MockDocuments() wm = WindowManager(MockWindow([[MockView(__file__)]]), MockConfigs(), docs, - MockDiagnostics(), mock_start_session, test_sublime, + WindowDiagnostics(), mock_start_session, test_sublime, MockHandlerDispatcher()) wm.start_active_views() @@ -389,7 +379,7 @@ def test_invokes_language_handler(self): docs = MockDocuments() dispatcher = MockHandlerDispatcher() wm = WindowManager(MockWindow([[MockView(__file__)]]), MockConfigs(), docs, - MockDiagnostics(), mock_start_session, test_sublime, + WindowDiagnostics(), mock_start_session, test_sublime, dispatcher) wm.start_active_views() diff --git a/plugin/core/windows.py b/plugin/core/windows.py index f81b32b16..ee7d7494f 100644 --- a/plugin/core/windows.py +++ b/plugin/core/windows.py @@ -1,3 +1,4 @@ +from .diagnostics import WindowDiagnostics, DiagnosticsUpdate from .events import global_events from .logging import debug, server_log from .types import ClientStates, ClientConfig, WindowLike, ViewLike, LanguageConfig, config_supports_syntax @@ -48,14 +49,6 @@ def for_window(self, window: WindowLike) -> ConfigRegistry: ... -class DiagnosticsHandler(Protocol): - def update(self, window: WindowLike, client_name: str, update: dict) -> None: - ... - - def remove(self, view: ViewLike, client_name: str) -> None: - ... - - class DocumentHandler(Protocol): def add_session(self, session) -> None: ... @@ -290,7 +283,7 @@ def notify_did_change(self, view: ViewLike): class WindowManager(object): def __init__(self, window: WindowLike, configs: ConfigRegistry, documents: DocumentHandler, - diagnostics: DiagnosticsHandler, session_starter: 'Callable', sublime: 'Any', + diagnostics: WindowDiagnostics, session_starter: 'Callable', sublime: 'Any', handler_dispatcher, on_closed: 'Optional[Callable]' = None) -> None: # to move here: @@ -305,6 +298,10 @@ def __init__(self, window: WindowLike, configs: ConfigRegistry, documents: Docum self._handlers = handler_dispatcher self._restarting = False self._project_path = get_project_path(self._window) + self._diagnostics.set_on_updated( + lambda file_path, client_name, diagnostics: + global_events.publish("document.diagnostics", + DiagnosticsUpdate(self._window, client_name, file_path, diagnostics))) self._on_closed = on_closed self._is_closing = False @@ -446,7 +443,7 @@ def _handle_session_started(self, session, project_path, config): client.on_notification( "textDocument/publishDiagnostics", - lambda params: self._diagnostics.update(self._window, config.name, params)) + lambda params: self._diagnostics.handle_client_diagnostics(config.name, params)) client.on_notification( "window/showMessage", @@ -475,14 +472,15 @@ def _handle_session_started(self, session, project_path, config): self._window.status_message("{} initialized".format(config.name)) def _handle_view_closed(self, view, session): - self._diagnostics.remove(view, session.config.name) - if not self._is_closing: - if not self._window.is_valid(): - # try to detect close synchronously (for quitting) - self._handle_window_closed() - else: - # in case the window is invalidated after the last view is closed - self._sublime.set_timeout_async(lambda: self._check_window_closed(), 100) + if view.file_name(): + self._diagnostics.remove(view.file_name(), session.config.name) + if not self._is_closing: + if not self._window.is_valid(): + # try to detect close synchronously (for quitting) + self._handle_window_closed() + else: + # in case the window is invalidated after the last view is closed + self._sublime.set_timeout_async(lambda: self._check_window_closed(), 100) def _check_window_closed(self): # debug('window {} check window closed closing={}, valid={}'.format( @@ -525,11 +523,10 @@ def _handle_server_crash(self, config: ClientConfig): class WindowRegistry(object): - def __init__(self, configs: GlobalConfigs, documents: 'Any', diagnostics: DiagnosticsHandler, + def __init__(self, configs: GlobalConfigs, documents: 'Any', session_starter: 'Callable', sublime: 'Any', handler_dispatcher) -> None: self._windows = {} # type: Dict[int, WindowManager] self._configs = configs - self._diagnostics = diagnostics self._documents = documents self._session_starter = session_starter self._sublime = sublime @@ -540,7 +537,7 @@ def lookup(self, window: 'Any') -> WindowManager: if state is None: window_configs = self._configs.for_window(window) window_documents = self._documents.for_window(window, window_configs) - state = WindowManager(window, window_configs, window_documents, self._diagnostics, self._session_starter, + state = WindowManager(window, window_configs, window_documents, WindowDiagnostics(), self._session_starter, self._sublime, self._handler_dispatcher, lambda: self._on_closed(window)) self._windows[window.id()] = state return state diff --git a/plugin/diagnostics.py b/plugin/diagnostics.py index e89a9644e..2ef890aea 100644 --- a/plugin/diagnostics.py +++ b/plugin/diagnostics.py @@ -10,7 +10,9 @@ pass from .core.configurations import is_supported_syntax -from .core.diagnostics import DiagnosticsUpdate, get_window_diagnostics, get_line_diagnostics +from .core.diagnostics import ( + DiagnosticsUpdate +) from .core.events import global_events from .core.logging import debug from .core.panels import ensure_panel @@ -18,6 +20,7 @@ from .core.settings import settings, PLUGIN_NAME, client_configs from .core.views import range_to_region from .core.workspace import get_project_path +from .core.registry import windows diagnostic_severity_names = { @@ -144,6 +147,14 @@ def update_diagnostics_phantoms(view: sublime.View, diagnostics: 'List[Diagnosti phantom_sets_by_buffer.pop(buffer_id, None) +def get_point_diagnostics(view, point): + diagnostics = get_view_diagnostics(view) + return tuple( + diagnostic for diagnostic in diagnostics + if range_to_region(diagnostic.range, view).contains(point) + ) + + def update_diagnostics_regions(view: sublime.View, diagnostics: 'List[Diagnostic]', severity: int): region_name = "lsp_" + format_severity(severity) if settings.show_diagnostics_phantoms and not view.is_dirty(): @@ -169,6 +180,27 @@ def update_diagnostics_in_view(view: sublime.View, diagnostics: 'List[Diagnostic update_diagnostics_regions(view, diagnostics, severity) +def get_view_diagnostics(view): + if view.window(): + if view.file_name(): + return windows.lookup(view.window())._diagnostics.get_by_path(view.file_name()) + else: + return [] + + +def get_line_diagnostics(view, point): + row, _ = view.rowcol(point) + diagnostics = get_view_diagnostics(view) + return tuple( + diagnostic for diagnostic in diagnostics + if diagnostic.range.start.row <= row <= diagnostic.range.end.row + ) + + +def get_window_diagnostics(window): + return windows.lookup(window)._diagnostics.get() + + def update_diagnostics_in_status_bar(view: sublime.View): errors = 0 warnings = 0 @@ -207,12 +239,15 @@ def update_count_in_status_bar(view): def handle_diagnostics(update: DiagnosticsUpdate): + debug('got update for', update.file_path, update.window.id()) window = update.window view = window.find_open_file(update.file_path) if view: update_diagnostics_in_view(view, update.diagnostics) if settings.show_diagnostics_count_in_view_status: update_diagnostics_in_status_bar(view) + else: + debug('view not found') update_diagnostics_panel(window) diff --git a/plugin/hover.py b/plugin/hover.py index 9992803a3..c7c402015 100644 --- a/plugin/hover.py +++ b/plugin/hover.py @@ -10,7 +10,7 @@ pass from .core.configurations import is_supported_syntax -from .core.diagnostics import get_point_diagnostics +from .diagnostics import get_point_diagnostics from .core.registry import session_for_view, LspTextCommand from .core.protocol import Request, DiagnosticSeverity from .core.documents import get_document_position From d2c9c9a2491d2f2ffabe3a550553030c227d6cad Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Sat, 27 Apr 2019 14:14:20 +0200 Subject: [PATCH 2/2] lint fix --- plugin/core/test_diagnostics.py | 1 - 1 file changed, 1 deletion(-) diff --git a/plugin/core/test_diagnostics.py b/plugin/core/test_diagnostics.py index 3de72398d..13376bb9b 100644 --- a/plugin/core/test_diagnostics.py +++ b/plugin/core/test_diagnostics.py @@ -57,4 +57,3 @@ def test_remove_diagnostics(self): view_diags = wd.get_by_path(test_file_path) self.assertEqual(len(view_diags), 0) -