From aee6d0af18f042faf2734586f26730e91bc84bb4 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 9 Aug 2023 19:39:22 +0000 Subject: [PATCH] [analysis_server] Restore use of pathContext.fromUri() for parsing file URIs in the LSP server This reverts be4189f04701d86abeac29504a97b3ebb6897ae8 plus adds an additional test to verify pkg:path to behaviour (to catch future regressions or if pkg:path has to be reverted, this will need reverting too). This relies on the fix made at https://github.com/dart-lang/path/issues/148 which rolled into the SDK in https://github.com/dart-lang/sdk/commit/f1de897762973de062fc56739de418847b9c9fe0. Change-Id: I1dea45e2017f7505bc4aca97f6c07c1a6e445a5e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319523 Commit-Queue: Brian Wilkerson Reviewed-by: Brian Wilkerson --- .../lib/src/lsp/handlers/handler_call_hierarchy.dart | 2 +- .../lsp/handlers/handler_change_workspace_folders.dart | 2 +- .../src/lsp/handlers/handler_completion_resolve.dart | 2 +- .../lib/src/lsp/handlers/handler_initialize.dart | 4 ++-- .../services/refactoring/move_top_level_to_file.dart | 3 ++- pkg/analysis_server/test/lsp/server_abstract.dart | 10 ++++++---- pkg/analysis_server/test/lsp/server_test.dart | 8 ++++++++ 7 files changed, 21 insertions(+), 10 deletions(-) diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_call_hierarchy.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_call_hierarchy.dart index 121dbcf9a60..85d1bc0d3fa 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_call_hierarchy.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_call_hierarchy.dart @@ -396,7 +396,7 @@ mixin _CallHierarchyUtils on HandlerHelperMixin { displayName: item.name, containerName: item.detail, kind: fromSymbolKind(item.kind), - file: item.uri.toFilePath(), + file: pathContext.fromUri(item.uri), nameRange: nameRange.result, codeRange: codeRange.result, ); diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_change_workspace_folders.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_change_workspace_folders.dart index 7bb6bc521b1..5c0d7c90a43 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_change_workspace_folders.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_change_workspace_folders.dart @@ -43,6 +43,6 @@ class WorkspaceFoldersHandler /// Return the result of converting the list of workspace [folders] to file /// paths. List _convertWorkspaceFolders(List folders) { - return folders.map((wf) => wf.uri.toFilePath()).toList(); + return folders.map((wf) => pathContext.fromUri(wf.uri)).toList(); } } diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart index 1df1606d5bd..76d4645d6fe 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart @@ -149,7 +149,7 @@ class CompletionResolveHandler // Compute the relative path and then put into a URI so the display // always uses forward slashes (as a URI) regardless of platform. ? pathContext.toUri(pathContext.relative( - libraryUri.toFilePath(), + pathContext.fromUri(libraryUri), from: pathContext.dirname(file), )) : libraryUri; diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_initialize.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_initialize.dart index 5d3655b6a10..14ca11c401b 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_initialize.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_initialize.dart @@ -44,13 +44,13 @@ class InitializeMessageHandler // Only file URIs are supported, but there's no way to signal this to // the LSP client (and certainly not before initialization). if (uri.isScheme('file')) { - workspacePaths.add(uri.toFilePath()); + workspacePaths.add(pathContext.fromUri(uri)); } } } if (rootUri != null) { if (rootUri.isScheme('file')) { - workspacePaths.add(rootUri.toFilePath()); + workspacePaths.add(pathContext.fromUri(rootUri)); } } else if (rootPath != null) { workspacePaths.add(rootPath); diff --git a/pkg/analysis_server/lib/src/services/refactoring/move_top_level_to_file.dart b/pkg/analysis_server/lib/src/services/refactoring/move_top_level_to_file.dart index 5201784388a..8e7294777aa 100644 --- a/pkg/analysis_server/lib/src/services/refactoring/move_top_level_to_file.dart +++ b/pkg/analysis_server/lib/src/services/refactoring/move_top_level_to_file.dart @@ -68,11 +68,12 @@ class MoveTopLevelToFile extends RefactoringProducer { return; } _initializeFromMembers(members); + var pathContext = refactoringContext.server.resourceProvider.pathContext; var sourcePath = members.containingFile; // TODO(dantup): Add refactor-specific validation for incoming arguments. // Argument is a String URI. var destinationUri = Uri.parse(commandArguments[0] as String); - var destinationFilePath = destinationUri.toFilePath(); + var destinationFilePath = pathContext.fromUri(destinationUri); var destinationImportUri = unitResult.session.uriConverter.pathToUri(destinationFilePath); diff --git a/pkg/analysis_server/test/lsp/server_abstract.dart b/pkg/analysis_server/test/lsp/server_abstract.dart index e1190b5d1bf..836dd2fb751 100644 --- a/pkg/analysis_server/test/lsp/server_abstract.dart +++ b/pkg/analysis_server/test/lsp/server_abstract.dart @@ -183,7 +183,7 @@ abstract class AbstractLspAnalysisServerTest String? getCurrentFileContent(Uri uri) { try { return server.resourceProvider - .getFile(uri.toFilePath()) + .getFile(pathContext.fromUri(uri)) .readAsStringSync(); } catch (_) { return null; @@ -1367,7 +1367,8 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin return configurationParams.items.map( (requestedConfig) { final uri = requestedConfig.scopeUri; - final path = uri != null ? Uri.parse(uri).toFilePath() : null; + final path = + uri != null ? pathContext.fromUri(Uri.parse(uri)) : null; // Use the config the test provided for this path, or fall back to // global. return (folders != null ? folders[path] : null) ?? global; @@ -1472,7 +1473,7 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin /// Formats a path relative to the project root always using forward slashes. /// /// This is used in the text format for comparing edits. - String relativeUri(Uri uri) => relativePath(uri.toFilePath()); + String relativeUri(Uri uri) => relativePath(pathContext.fromUri(uri)); Future rename( Uri uri, @@ -1574,7 +1575,8 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin .map((notification) => PublishDiagnosticsParams.fromJson( notification.params as Map)) .listen((diagnostics) { - latestDiagnostics[diagnostics.uri.toFilePath()] = diagnostics.diagnostics; + latestDiagnostics[pathContext.fromUri(diagnostics.uri)] = + diagnostics.diagnostics; }); } diff --git a/pkg/analysis_server/test/lsp/server_test.dart b/pkg/analysis_server/test/lsp/server_test.dart index 4ec18106dd2..3a65d03be81 100644 --- a/pkg/analysis_server/test/lsp/server_test.dart +++ b/pkg/analysis_server/test/lsp/server_test.dart @@ -236,6 +236,14 @@ class ServerTest extends AbstractLspAnalysisServerTest { ); } + /// The LSP server relies on pathContext.fromUri() handling encoded colons + /// in paths, so verify that works as expected. + Future test_pathContext_fromUri_windows() async { + expect(path.windows.fromUri('file:///C:/foo'), r'C:\foo'); + expect(path.windows.fromUri('file:///C%3a/foo'), r'C:\foo'); + expect(path.windows.fromUri('file:///C%3A/foo'), r'C:\foo'); + } + Future test_shutdown_initialized() async { await initialize(); final response = await sendShutdown();