From de1bab9f1e2c2af664204178e85e3d180d5a13f6 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Thu, 19 Sep 2019 11:28:40 -0700 Subject: [PATCH] Fix Intellisense not working for saved files (#867) * Fix tools service to store the corrected file path * Use ClientFilePath for key * Further fixes * Undo spacing changes * Fix tests * Trigger CI rebuild --- .../Connection/ConnectionService.cs | 66 +++++++++---------- .../LanguageServices/DiagnosticsHelper.cs | 2 +- .../LanguageServices/LanguageService.cs | 34 +++++----- .../Utility/FileUtils.cs | 13 +++- .../Utility/ResolvedFile.cs | 10 ++- .../Workspace/Contracts/ScriptFile.cs | 22 +++---- .../Workspace/Workspace.cs | 47 +++++++++---- .../Utility/LiveConnectionHelper.cs | 4 +- .../LanguageServer/LanguageServiceTests.cs | 24 +++---- .../LanguageServer/PeekDefinitionTests.cs | 14 ++-- .../Utility/LiveConnectionHelper.cs | 4 +- .../LanguageServer/AutocompleteTests.cs | 40 +++++++---- .../LanguageServer/LanguageServiceTestBase.cs | 4 +- 13 files changed, 168 insertions(+), 116 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs b/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs index 308a2cd1..14700415 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs @@ -29,7 +29,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection /// Main class for the Connection Management services /// public class ConnectionService - { + { public const string AdminConnectionPrefix = "ADMIN:"; internal const string PasswordPlaceholder = "******"; private const string SqlAzureEdition = "SQL Azure"; @@ -37,7 +37,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection /// /// Singleton service instance /// - private static readonly Lazy instance + private static readonly Lazy instance = new Lazy(() => new ConnectionService()); /// @@ -56,7 +56,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection /// A map containing all CancellationTokenSource objects that are associated with a given URI/ConnectionType pair. /// Entries in this map correspond to DbConnection instances that are in the process of connecting. /// - private readonly ConcurrentDictionary cancelTupleToCancellationTokenSourceMap = + private readonly ConcurrentDictionary cancelTupleToCancellationTokenSourceMap = new ConcurrentDictionary(); private readonly object cancellationTokenSourceLock = new object(); @@ -92,7 +92,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection /// Service host object for sending/receiving requests/events. /// Internal for testing purposes. /// - internal IProtocolEndpoint ServiceHost { get; set;} + internal IProtocolEndpoint ServiceHost { get; set; } /// /// Gets the connection queue @@ -465,16 +465,16 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection } if (SqlAzureEdition.Equals(serverEdition, StringComparison.OrdinalIgnoreCase)) { - switch(serverInfo.EngineEditionId) + switch (serverInfo.EngineEditionId) { - case (int) DatabaseEngineEdition.SqlDataWarehouse: + case (int)DatabaseEngineEdition.SqlDataWarehouse: serverEdition = SR.AzureSqlDwEdition; break; - case (int) DatabaseEngineEdition.SqlStretchDatabase: + case (int)DatabaseEngineEdition.SqlStretchDatabase: serverEdition = SR.AzureSqlStretchEdition; break; default: - serverEdition = SR.AzureSqlDbEdition; + serverEdition = SR.AzureSqlDbEdition; break; } } @@ -516,7 +516,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection } cancelTupleToCancellationTokenSourceMap[cancelKey] = source; } - + // Open the connection await connection.OpenAsync(source.Token); } @@ -599,7 +599,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection throw new InvalidOperationException(SR.ConnectionServiceDbErrorDefaultNotConnected(ownerUri)); } - if(IsDedicatedAdminConnection(connectionInfo.ConnectionDetails)) + if (IsDedicatedAdminConnection(connectionInfo.ConnectionDetails)) { // Since this is a dedicated connection only 1 is allowed at any time. Return the default connection for use in the requested action connection = defaultConnection; @@ -618,7 +618,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection return connection; } - private async Task TryOpenConnectionForConnectionType(string ownerUri, string connectionType, + private async Task TryOpenConnectionForConnectionType(string ownerUri, string connectionType, bool alwaysPersistSecurity, ConnectionInfo connectionInfo) { // If the DbConnection does not exist and is not the default connection, create one. @@ -727,7 +727,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection } } - return false; + return false; } /// @@ -888,14 +888,14 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection connectionDetails.DatabaseName = "master"; var connection = this.ConnectionFactory.CreateSqlConnection(BuildConnectionString(connectionDetails), connectionDetails.AzureAccountToken); connection.Open(); - + List results = new List(); - var systemDatabases = new[] {"master", "model", "msdb", "tempdb"}; + var systemDatabases = new[] { "master", "model", "msdb", "tempdb" }; using (DbCommand command = connection.CreateCommand()) { command.CommandText = @"SELECT name FROM sys.databases WHERE state_desc='ONLINE' ORDER BY name ASC"; command.CommandTimeout = 15; - command.CommandType = CommandType.Text; + command.CommandType = CommandType.Text; using (var reader = command.ExecuteReader()) { @@ -907,7 +907,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection } // Put system databases at the top of the list - results = + results = results.Where(s => systemDatabases.Any(s.Equals)).Concat( results.Where(s => systemDatabases.All(x => !s.Equals(x)))).ToList(); @@ -937,9 +937,9 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection /// Add a new method to be called when the onconnection request is submitted /// /// - public void RegisterOnConnectionTask(OnConnectionHandler activity) - { - onConnectionActivities.Add(activity); + public void RegisterOnConnectionTask(OnConnectionHandler activity) + { + onConnectionActivities.Add(activity); } /// @@ -976,7 +976,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection private void RunConnectRequestHandlerTask(ConnectParams connectParams) { // create a task to connect asynchronously so that other requests are not blocked in the meantime - Task.Run(async () => + Task.Run(async () => { try { @@ -1017,7 +1017,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection bool result = CancelConnect(cancelParams); await requestContext.SendResult(result); } - catch(Exception ex) + catch (Exception ex) { await requestContext.SendError(ex.ToString()); } @@ -1037,7 +1037,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection bool result = Instance.Disconnect(disconnectParams); await requestContext.SendResult(result); } - catch(Exception ex) + catch (Exception ex) { await requestContext.SendError(ex.ToString()); } @@ -1058,7 +1058,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection ListDatabasesResponse result = ListDatabases(listDatabasesParams); await requestContext.SendResult(result); } - catch(Exception ex) + catch (Exception ex) { await requestContext.SendError(ex.ToString()); } @@ -1075,7 +1075,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection string serverName = builder.DataSource; return serverName != null && serverName.StartsWith(AdminConnectionPrefix, StringComparison.OrdinalIgnoreCase); } - + /// /// Build a connection string from a connection details instance /// @@ -1099,7 +1099,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection { connectionBuilder = new SqlConnectionStringBuilder(connectionDetails.ConnectionString); } - else + else { // add alternate port to data source property if provided string dataSource = !connectionDetails.Port.HasValue @@ -1121,7 +1121,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection } if (!string.IsNullOrEmpty(connectionDetails.AuthenticationType)) { - switch(connectionDetails.AuthenticationType) + switch (connectionDetails.AuthenticationType) { case "Integrated": connectionBuilder.IntegratedSecurity = true; @@ -1364,7 +1364,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection conn.Close(); conn.Dispose(); info.RemoveConnection(key); - + string connectionString = BuildConnectionString(info.ConnectionDetails); // create a sql connection instance @@ -1377,7 +1377,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection conn.ChangeDatabase(newDatabaseName); } } - + } // Fire a connection changed event @@ -1393,9 +1393,9 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection Logger.Write( TraceEventType.Error, string.Format( - "Exception caught while trying to change database context to [{0}] for OwnerUri [{1}]. Exception:{2}", - newDatabaseName, - ownerUri, + "Exception caught while trying to change database context to [{0}] for OwnerUri [{1}]. Exception:{2}", + newDatabaseName, + ownerUri, e.ToString()) ); } @@ -1511,12 +1511,12 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection } catch (Exception ex) { - string error = string.Format(CultureInfo.InvariantCulture, + string error = string.Format(CultureInfo.InvariantCulture, "Failed opening a SqlConnection: error:{0} inner:{1} stacktrace:{2}", ex.Message, ex.InnerException != null ? ex.InnerException.Message : string.Empty, ex.StackTrace); Logger.Write(TraceEventType.Error, error); } - + return null; } diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/DiagnosticsHelper.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/DiagnosticsHelper.cs index 0ae47a03..a3ab9276 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/DiagnosticsHelper.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/DiagnosticsHelper.cs @@ -41,7 +41,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices PublishDiagnosticsNotification.Type, new PublishDiagnosticsNotification { - Uri = scriptFile.ClientFilePath, + Uri = scriptFile.ClientUri, Diagnostics = allMarkers .Select(GetDiagnosticFromMarker) diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs index 850fb6b6..2e12b38b 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs @@ -452,7 +452,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices ConnectionInfo connInfo; ConnectionServiceInstance.TryFindConnection( - scriptFile.ClientFilePath, + scriptFile.ClientUri, out connInfo); var completionItems = await GetCompletionItems( @@ -514,7 +514,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices DefinitionResult definitionResult = null; if (scriptFile != null) { - isConnected = ConnectionServiceInstance.TryFindConnection(scriptFile.ClientFilePath, out connInfo); + isConnected = ConnectionServiceInstance.TryFindConnection(scriptFile.ClientUri, out connInfo); definitionResult = GetDefinition(textDocumentPosition, scriptFile, connInfo); } @@ -723,7 +723,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices ConnectionInfo connInfo; ConnectionServiceInstance.TryFindConnection( - scriptFile.ClientFilePath, + scriptFile.ClientUri, out connInfo); // check that there is an active connection for the current editor @@ -809,7 +809,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices foreach (var scriptFile in CurrentWorkspace.GetOpenedFiles()) { - await DiagnosticsHelper.ClearScriptDiagnostics(scriptFile.ClientFilePath, eventContext); + await DiagnosticsHelper.ClearScriptDiagnostics(scriptFile.ClientUri, eventContext); } } // otherwise rerun diagnostic analysis on all opened SQL files @@ -898,7 +898,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices public ParseResult ParseAndBind(ScriptFile scriptFile, ConnectionInfo connInfo) { // get or create the current parse info object - ScriptParseInfo parseInfo = GetScriptParseInfo(scriptFile.ClientFilePath, createIfNotExists: true); + ScriptParseInfo parseInfo = GetScriptParseInfo(scriptFile.ClientUri, createIfNotExists: true); if (Monitor.TryEnter(parseInfo.BuildingMetadataLock, LanguageService.BindingTimeout)) { @@ -1130,7 +1130,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices private bool ShouldSkipNonMssqlFile(ScriptFile scriptFile) { - return ShouldSkipNonMssqlFile(scriptFile.ClientFilePath); + return ShouldSkipNonMssqlFile(scriptFile.ClientUri); } /// @@ -1335,7 +1335,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices internal DefinitionResult GetDefinition(TextDocumentPosition textDocumentPosition, ScriptFile scriptFile, ConnectionInfo connInfo) { // Parse sql - ScriptParseInfo scriptParseInfo = GetScriptParseInfo(textDocumentPosition.TextDocument.Uri); + ScriptParseInfo scriptParseInfo = GetScriptParseInfo(scriptFile.ClientUri); if (scriptParseInfo == null) { return null; @@ -1450,7 +1450,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices textDocumentPosition.Position.Character); int endColumn = textDocumentPosition.Position.Character; - ScriptParseInfo scriptParseInfo = GetScriptParseInfo(textDocumentPosition.TextDocument.Uri); + ScriptParseInfo scriptParseInfo = GetScriptParseInfo(scriptFile.ClientUri); if (scriptParseInfo != null && scriptParseInfo.ParseResult != null) { if (Monitor.TryEnter(scriptParseInfo.BuildingMetadataLock)) @@ -1499,7 +1499,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices int startLine = textDocumentPosition.Position.Line; int endColumn = textDocumentPosition.Position.Character; - ScriptParseInfo scriptParseInfo = GetScriptParseInfo(textDocumentPosition.TextDocument.Uri); + ScriptParseInfo scriptParseInfo = GetScriptParseInfo(scriptFile.ClientUri); if (scriptParseInfo == null) { @@ -1509,7 +1509,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices ConnectionInfo connInfo; ConnectionServiceInstance.TryFindConnection( - scriptFile.ClientFilePath, + scriptFile.ClientUri, out connInfo); // reparse and bind the SQL statement if needed @@ -1587,7 +1587,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices bool useLowerCaseSuggestions = this.CurrentWorkspaceSettings.SqlTools.IntelliSense.LowerCaseSuggestions.Value; // get the current script parse info object - ScriptParseInfo scriptParseInfo = GetScriptParseInfo(textDocumentPosition.TextDocument.Uri); + ScriptParseInfo scriptParseInfo = GetScriptParseInfo(scriptFile.ClientUri); if (scriptParseInfo == null) { @@ -1672,7 +1672,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices { ConnectionInfo connInfo; ConnectionServiceInstance.TryFindConnection( - scriptFile.ClientFilePath, + scriptFile.ClientUri, out connInfo); var parseResult = ParseAndBind(scriptFile, connInfo); @@ -1797,10 +1797,10 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices { continue; } - else if (ShouldSkipNonMssqlFile(scriptFile.ClientFilePath)) + else if (ShouldSkipNonMssqlFile(scriptFile.ClientUri)) { // Clear out any existing markers in case file type was changed - await DiagnosticsHelper.ClearScriptDiagnostics(scriptFile.ClientFilePath, eventContext); + await DiagnosticsHelper.ClearScriptDiagnostics(scriptFile.ClientUri, eventContext); continue; } @@ -1884,9 +1884,9 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices /// internal bool IsPreviewWindow(ScriptFile scriptFile) { - if (scriptFile != null && !string.IsNullOrWhiteSpace(scriptFile.ClientFilePath)) + if (scriptFile != null && !string.IsNullOrWhiteSpace(scriptFile.ClientUri)) { - return scriptFile.ClientFilePath.StartsWith("tsqloutput:"); + return scriptFile.ClientUri.StartsWith("tsqloutput:"); } else { @@ -1971,4 +1971,4 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices } } } -} \ No newline at end of file +} diff --git a/src/Microsoft.SqlTools.ServiceLayer/Utility/FileUtils.cs b/src/Microsoft.SqlTools.ServiceLayer/Utility/FileUtils.cs index 1a54128a..9623bcf6 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Utility/FileUtils.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Utility/FileUtils.cs @@ -138,16 +138,23 @@ namespace Microsoft.SqlTools.ServiceLayer.Utility } } - internal static ResolvedFile TryGetFullPath(string filePath) + /// + /// Attempts to resolve the given filePath to an absolute path to a file on disk, + /// defaulting to the original filePath if that fails. + /// + /// The file path to resolve + /// The full file path URI used by the client + /// + internal static ResolvedFile TryGetFullPath(string filePath, string clientUri) { try { - return new ResolvedFile(Path.GetFullPath(filePath), true); + return new ResolvedFile(Path.GetFullPath(filePath), clientUri, true); } catch(NotSupportedException) { // This is not a standard path. - return new ResolvedFile(filePath, false); + return new ResolvedFile(filePath, clientUri, false); } } } diff --git a/src/Microsoft.SqlTools.ServiceLayer/Utility/ResolvedFile.cs b/src/Microsoft.SqlTools.ServiceLayer/Utility/ResolvedFile.cs index 44f8c08f..7c9280f7 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Utility/ResolvedFile.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Utility/ResolvedFile.cs @@ -19,20 +19,24 @@ namespace Microsoft.SqlTools.ServiceLayer.Utility /// internal class ResolvedFile { - public ResolvedFile(string filePath, bool canReadFromDisk) + public ResolvedFile(string filePath, string clientUri, bool canReadFromDisk) { FilePath = filePath; + ClientUri = clientUri; CanReadFromDisk = canReadFromDisk; } public string FilePath { get; private set; } + + public string ClientUri { get; private set; } + public bool CanReadFromDisk { get; private set; } - public string LowercaseFilePath + public string LowercaseClientUri { get { - return FilePath?.ToLower(); + return ClientUri?.ToLower(); } } } diff --git a/src/Microsoft.SqlTools.ServiceLayer/Workspace/Contracts/ScriptFile.cs b/src/Microsoft.SqlTools.ServiceLayer/Workspace/Contracts/ScriptFile.cs index fc7dc1b3..41a8fced 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Workspace/Contracts/ScriptFile.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Workspace/Contracts/ScriptFile.cs @@ -21,11 +21,11 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace.Contracts /// /// Gets a unique string that identifies this file. At this time, /// this property returns a normalized version of the value stored - /// in the FilePath property. + /// in the ClientUri property. /// public string Id { - get { return this.FilePath.ToLower(); } + get { return this.ClientUri.ToLower(); } } /// @@ -34,11 +34,11 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace.Contracts public string FilePath { get; private set; } /// - /// Gets or sets the path which the editor client uses to identify this file. + /// Gets or sets the URI which the editor client uses to identify this file. /// Setter for testing purposes only /// virtual to allow mocking. /// - public virtual string ClientFilePath { get; internal set; } + public virtual string ClientUri { get; internal set; } /// /// Gets or sets a boolean that determines whether @@ -103,7 +103,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace.Contracts /// public ScriptFile() { - ClientFilePath = "test.sql"; + ClientUri = "test.sql"; } /// @@ -111,15 +111,15 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace.Contracts /// the given TextReader. /// /// The path at which the script file resides. - /// The path which the client uses to identify the file. + /// The URI which the client uses to identify the file. /// The TextReader to use for reading the file's contents. public ScriptFile( string filePath, - string clientFilePath, + string clientUri, TextReader textReader) { FilePath = filePath; - ClientFilePath = clientFilePath; + ClientUri = clientUri; IsAnalysisEnabled = true; IsInMemory = Workspace.IsPathInMemoryOrNonFileUri(filePath); @@ -130,15 +130,15 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace.Contracts /// Creates a new ScriptFile instance with the specified file contents. /// /// The path at which the script file resides. - /// The path which the client uses to identify the file. + /// The path which the client uses to identify the file. /// The initial contents of the script file. public ScriptFile( string filePath, - string clientFilePath, + string clientUri, string initialBuffer) { FilePath = filePath; - ClientFilePath = clientFilePath; + ClientUri = clientUri; IsAnalysisEnabled = true; SetFileContents(initialBuffer); diff --git a/src/Microsoft.SqlTools.ServiceLayer/Workspace/Workspace.cs b/src/Microsoft.SqlTools.ServiceLayer/Workspace/Workspace.cs index 29de3f91..b86e47bb 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Workspace/Workspace.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Workspace/Workspace.cs @@ -70,7 +70,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace // Resolve the full file path ResolvedFile resolvedFile = this.ResolveFilePath(filePath); - string keyName = resolvedFile.LowercaseFilePath; + string keyName = resolvedFile.LowercaseClientUri; ScriptFile scriptFile = null; return this.workspaceFiles.TryGetValue(keyName, out scriptFile); @@ -98,7 +98,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace // Resolve the full file path ResolvedFile resolvedFile = this.ResolveFilePath(filePath); - string keyName = resolvedFile.LowercaseFilePath; + string keyName = resolvedFile.LowercaseClientUri; // Make sure the file isn't already loaded into the workspace ScriptFile scriptFile = null; @@ -117,7 +117,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace using (FileStream fileStream = new FileStream(resolvedFile.FilePath, FileMode.Open, FileAccess.Read)) using (StreamReader streamReader = new StreamReader(fileStream, Encoding.UTF8)) { - scriptFile = new ScriptFile(resolvedFile.FilePath, filePath,streamReader); + scriptFile = new ScriptFile(resolvedFile.FilePath, resolvedFile.ClientUri,streamReader); this.workspaceFiles.Add(keyName, scriptFile); } @@ -128,20 +128,26 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace return scriptFile; } - private ResolvedFile ResolveFilePath(string filePath) + /// + /// Resolves a URI identifier into an actual file on disk if it exists. + /// + /// The URI identifying the file + /// + private ResolvedFile ResolveFilePath(string clientUri) { bool canReadFromDisk = false; - if (!IsPathInMemoryOrNonFileUri(filePath)) + string filePath = clientUri; + if (!IsPathInMemoryOrNonFileUri(clientUri)) { - if (filePath.StartsWith(@"file://")) + if (clientUri.StartsWith(@"file://")) { // VS Code encodes the ':' character in the drive name, which can lead to problems parsing // the URI, so unencode it if present. See https://github.com/Microsoft/vscode/issues/2990 - filePath = filePath.Replace("%3A/", ":/", StringComparison.OrdinalIgnoreCase); + clientUri = clientUri.Replace("%3A/", ":/", StringComparison.OrdinalIgnoreCase); // Client sent the path in URI format, extract the local path and trim // any extraneous slashes - Uri fileUri = new Uri(filePath); + Uri fileUri = new Uri(clientUri); filePath = fileUri.LocalPath; if (filePath.StartsWith("//") || filePath.StartsWith("\\\\") || filePath.StartsWith("/")) { @@ -154,21 +160,36 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace // into the SqlTools engine. filePath = UnescapePath(filePath); + // Client paths are handled a bit differently because of how we currently identifiers in + // ADS. The URI is passed around as an identifier - but for things we control like connecting + // an editor the URI we pass in is NOT escaped fully. This is a problem for certain functionality + // which is handled by VS Code - such as Intellise Completion - as the URI passed in there is + // the fully escaped URI. That means we need to do some extra work to make sure that the URI values + // are consistent. + // So to solve that we'll make sure to unescape ALL uri's that are passed in and store that value for + // use as an identifier (filePath will be the actual file path on disk). + // # and ? are still always escaped though by ADS so we need to escape those again to get them to actually + // match + clientUri = Uri.UnescapeDataString(UnescapePath(clientUri)); + clientUri = clientUri.Replace("#", "%23"); + clientUri = clientUri.Replace("?", "%3F"); + // switch to unix path separators on non-Windows platforms if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { filePath = filePath.Replace('\\', '/'); + clientUri = clientUri.Replace('\\', '/'); } // Get the absolute file path - ResolvedFile resolvedFile = FileUtilities.TryGetFullPath(filePath); + ResolvedFile resolvedFile = FileUtilities.TryGetFullPath(filePath, clientUri); filePath = resolvedFile.FilePath; canReadFromDisk = resolvedFile.CanReadFromDisk; } - Logger.Write(TraceEventType.Verbose, "Resolved path: " + filePath); + Logger.Write(TraceEventType.Verbose, "Resolved path: " + clientUri); - return new ResolvedFile(filePath, canReadFromDisk); + return new ResolvedFile(filePath, clientUri, canReadFromDisk); } /// @@ -204,13 +225,13 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace // Resolve the full file path ResolvedFile resolvedFile = this.ResolveFilePath(filePath); - string keyName = resolvedFile.LowercaseFilePath; + string keyName = resolvedFile.LowercaseClientUri; // Make sure the file isn't already loaded into the workspace ScriptFile scriptFile = null; if (!this.workspaceFiles.TryGetValue(keyName, out scriptFile)) { - scriptFile = new ScriptFile(resolvedFile.FilePath, filePath, initialBuffer); + scriptFile = new ScriptFile(resolvedFile.FilePath, resolvedFile.ClientUri, initialBuffer); this.workspaceFiles.Add(keyName, scriptFile); diff --git a/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/Utility/LiveConnectionHelper.cs b/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/Utility/LiveConnectionHelper.cs index e5a1599a..a83beea8 100644 --- a/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/Utility/LiveConnectionHelper.cs +++ b/test/Microsoft.SqlTools.ManagedBatchParser.IntegrationTests/Utility/LiveConnectionHelper.cs @@ -42,7 +42,7 @@ namespace Microsoft.SqlTools.ManagedBatchParser.IntegrationTests.Utility { ownerUri = GetTestSqlFile(); scriptFile = TestServiceProvider.Instance.WorkspaceService.Workspace.GetFile(ownerUri); - ownerUri = scriptFile.ClientFilePath; + ownerUri = scriptFile.ClientUri; } var connectionService = GetLiveTestConnectionService(); var connectionResult = @@ -68,7 +68,7 @@ namespace Microsoft.SqlTools.ManagedBatchParser.IntegrationTests.Utility { ownerUri = GetTestSqlFile(); scriptFile = TestServiceProvider.Instance.WorkspaceService.Workspace.GetFile(ownerUri); - ownerUri = scriptFile.ClientFilePath; + ownerUri = scriptFile.ClientUri; } ConnectParams connectParams = TestServiceProvider.Instance.ConnectionProfileService.GetConnectionParameters(TestServerType.OnPrem, databaseName); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/LanguageServer/LanguageServiceTests.cs b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/LanguageServer/LanguageServiceTests.cs index c5c220cb..7ad16dea 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/LanguageServer/LanguageServiceTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/LanguageServer/LanguageServiceTests.cs @@ -231,7 +231,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.LanguageServer { TextDocument = new TextDocumentIdentifier { - Uri = result.ScriptFile.ClientFilePath + Uri = result.ScriptFile.ClientUri }, Position = new Position { @@ -246,7 +246,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.LanguageServer Thread.Sleep(2000); // We should get back a non-null ScriptParseInfo - ScriptParseInfo parseInfo = service.GetScriptParseInfo(result.ScriptFile.ClientFilePath); + ScriptParseInfo parseInfo = service.GetScriptParseInfo(result.ScriptFile.ClientUri); Assert.NotNull(parseInfo); // And we should get back a non-null SignatureHelp @@ -298,7 +298,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.LanguageServer { TextDocument = new TextDocumentIdentifier { - Uri = connectionInfoResult.ScriptFile.ClientFilePath + Uri = connectionInfoResult.ScriptFile.ClientUri }, Position = new Position { @@ -318,7 +318,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.LanguageServer // And refresh the cache await langService.HandleRebuildIntelliSenseNotification( - new RebuildIntelliSenseParams() { OwnerUri = connectionInfoResult.ScriptFile.ClientFilePath }, + new RebuildIntelliSenseParams() { OwnerUri = connectionInfoResult.ScriptFile.ClientUri }, new TestEventContext()); // Now we should expect to see the item show up in the completion list @@ -342,24 +342,24 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.LanguageServer public async Task HandleRequestToChangeToSqlcmdFile() { - var scriptFile = new ScriptFile() { ClientFilePath = "HandleRequestToChangeToSqlcmdFile_" + DateTime.Now.ToLongDateString() + "_.sql" }; + var scriptFile = new ScriptFile() { ClientUri = "HandleRequestToChangeToSqlcmdFile_" + DateTime.Now.ToLongDateString() + "_.sql" }; try { // Prepare a script file scriptFile.SetFileContents("koko wants a bananas"); - File.WriteAllText(scriptFile.ClientFilePath, scriptFile.Contents); + File.WriteAllText(scriptFile.ClientUri, scriptFile.Contents); // Create a workspace and add file to it so that its found for intellense building var workspace = new ServiceLayer.Workspace.Workspace(); var workspaceService = new WorkspaceService { Workspace = workspace }; var langService = new LanguageService() { WorkspaceServiceInstance = workspaceService }; - langService.CurrentWorkspace.GetFile(scriptFile.ClientFilePath); + langService.CurrentWorkspace.GetFile(scriptFile.ClientUri); langService.CurrentWorkspaceSettings.SqlTools.IntelliSense.EnableIntellisense = true; // Add a connection to ensure the intellisense building works ConnectionInfo connectionInfo = GetLiveAutoCompleteTestObjects().ConnectionInfo; - langService.ConnectionServiceInstance.OwnerToConnectionMap.Add(scriptFile.ClientFilePath, connectionInfo); + langService.ConnectionServiceInstance.OwnerToConnectionMap.Add(scriptFile.ClientUri, connectionInfo); // Test SQL int countOfValidationCalls = 0; @@ -367,7 +367,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.LanguageServer eventContextSql.Setup(x => x.SendEvent(PublishDiagnosticsNotification.Type, It.Is((notif) => ValidateNotification(notif, 2, ref countOfValidationCalls)))).Returns(Task.FromResult(new object())); await langService.HandleDidChangeLanguageFlavorNotification(new LanguageFlavorChangeParams { - Uri = scriptFile.ClientFilePath, + Uri = scriptFile.ClientUri, Language = LanguageService.SQL_LANG.ToLower(), Flavor = "MSSQL" }, eventContextSql.Object); @@ -378,7 +378,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.LanguageServer eventContextSqlCmd.Setup(x => x.SendEvent(PublishDiagnosticsNotification.Type, It.Is((notif) => ValidateNotification(notif, 0, ref countOfValidationCalls)))).Returns(Task.FromResult(new object())); await langService.HandleDidChangeLanguageFlavorNotification(new LanguageFlavorChangeParams { - Uri = scriptFile.ClientFilePath, + Uri = scriptFile.ClientUri, Language = LanguageService.SQL_CMD_LANG.ToLower(), Flavor = "MSSQL" }, eventContextSqlCmd.Object); @@ -388,9 +388,9 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.LanguageServer } finally { - if (File.Exists(scriptFile.ClientFilePath)) + if (File.Exists(scriptFile.ClientUri)) { - File.Delete(scriptFile.ClientFilePath); + File.Delete(scriptFile.ClientUri); } } } diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/LanguageServer/PeekDefinitionTests.cs b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/LanguageServer/PeekDefinitionTests.cs index 8312e68a..c3bd6041 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/LanguageServer/PeekDefinitionTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/LanguageServer/PeekDefinitionTests.cs @@ -235,7 +235,11 @@ GO"; .Callback, Func, Func, int?, int?>( (key, bindOperation, timeoutOperation, errHandler, t1, t2) => { - timeoutResult = (DefinitionResult)timeoutOperation((IBindingContext)null); + if(timeoutOperation != null) + { + timeoutResult = (DefinitionResult)timeoutOperation(null); + } + itemMock.Object.Result = timeoutResult; }) .Returns(() => itemMock.Object); @@ -251,14 +255,14 @@ GO"; }; LiveConnectionHelper.TestConnectionResult connectionResult = LiveConnectionHelper.InitLiveConnectionInfo(); ScriptFile scriptFile = connectionResult.ScriptFile; - ConnectionInfo connInfo = connectionResult.ConnectionInfo; scriptFile.Contents = "select * from dbo.func ()"; ScriptParseInfo scriptInfo = new ScriptParseInfo { IsConnected = true }; - languageService.ScriptParseInfoMap.Add(OwnerUri, scriptInfo); + languageService.ScriptParseInfoMap.Add(scriptFile.ClientUri, scriptInfo); - // When I call the language service - var result = languageService.GetDefinition(textDocument, scriptFile, connInfo); + // Pass in null connection info to force doing a local parse since that hits the BindingQueue timeout + // before we want it to (this is testing the timeout trying to fetch the definitions after the parse) + var result = languageService.GetDefinition(textDocument, scriptFile, null); // Then I expect null locations and an error to be reported Assert.NotNull(result); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Utility/LiveConnectionHelper.cs b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Utility/LiveConnectionHelper.cs index 35ff5d69..d5764e85 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Utility/LiveConnectionHelper.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Utility/LiveConnectionHelper.cs @@ -44,7 +44,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.Utility { ownerUri = GetTestSqlFile(); scriptFile = TestServiceProvider.Instance.WorkspaceService.Workspace.GetFile(ownerUri); - ownerUri = scriptFile.ClientFilePath; + ownerUri = scriptFile.ClientUri; } var connectionService = GetLiveTestConnectionService(); var connectionResult = @@ -70,7 +70,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.Utility { ownerUri = GetTestSqlFile(); scriptFile = TestServiceProvider.Instance.WorkspaceService.Workspace.GetFile(ownerUri); - ownerUri = scriptFile.ClientFilePath; + ownerUri = scriptFile.ClientUri; } ConnectParams connectParams = TestServiceProvider.Instance.ConnectionProfileService.GetConnectionParameters(TestServerType.OnPrem, databaseName); diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/AutocompleteTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/AutocompleteTests.cs index d0a1c9ca..f8102e4e 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/AutocompleteTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/AutocompleteTests.cs @@ -3,23 +3,16 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. // -using System.Collections.Generic; using System.Threading.Tasks; -using Microsoft.SqlServer.Management.SqlParser.Binder; -using Microsoft.SqlServer.Management.SqlParser.MetadataProvider; using Microsoft.SqlServer.Management.SqlParser.Parser; using Microsoft.SqlTools.Hosting.Protocol; -using Microsoft.SqlTools.ServiceLayer.Connection; using Microsoft.SqlTools.ServiceLayer.LanguageServices; using Microsoft.SqlTools.ServiceLayer.LanguageServices.Contracts; -using Microsoft.SqlTools.ServiceLayer.SqlContext; -using Microsoft.SqlTools.ServiceLayer.UnitTests.Utility; -using Microsoft.SqlTools.ServiceLayer.Workspace; using Microsoft.SqlTools.ServiceLayer.Workspace.Contracts; -using GlobalCommon = Microsoft.SqlTools.ServiceLayer.Test.Common; using Moq; using Xunit; using Microsoft.SqlTools.ServiceLayer.Connection.Contracts; +using Location = Microsoft.SqlTools.ServiceLayer.Workspace.Contracts.Location; namespace Microsoft.SqlTools.ServiceLayer.UnitTests.LanguageServer { @@ -91,11 +84,24 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.LanguageServer } [Fact] - public void GetDefinitionInvalidTextDocument() + public async void HandleDefinitionRequest_InvalidTextDocument_SendsEmptyListResponse() { InitializeTestObjects(); textDocument.TextDocument.Uri = "invaliduri"; - Assert.Null(langService.GetDefinition(textDocument, null, null)); + + // setup the mock for SendResult + var definitionRequestContext = new Mock>(); + Location[] result = null; + definitionRequestContext.Setup(rc => rc.SendResult(It.IsAny())) + .Returns((resultDetails) => { + result = resultDetails; + return Task.FromResult(0); + }); + + await langService.HandleDefinitionRequest(textDocument, definitionRequestContext.Object); + // Should get an empty array when passed + Assert.NotNull(result); + Assert.True(result.Length == 0, $"Unexpected values passed to SendResult : [{ string.Join(",", (object[])result)}]"); } [Fact] @@ -113,11 +119,21 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.LanguageServer } [Fact] - public void GetCompletionItemsInvalidTextDocument() + public async void HandleCompletionRequest_InvalidTextDocument_SendsNullResult() { InitializeTestObjects(); + // setup the mock for SendResult to capture the items + CompletionItem[] completionItems = null; + requestContext.Setup(x => x.SendResult(It.IsAny())) + .Returns((resultDetails) => { + completionItems = resultDetails; + return Task.FromResult(0); + }); + textDocument.TextDocument.Uri = "somethinggoeshere"; - Assert.True(langService.GetCompletionItems(textDocument, scriptFile.Object, null).Result.Length > 0); + await langService.HandleCompletionRequest(textDocument, requestContext.Object); + requestContext.Verify(m => m.SendResult(It.IsAny()), Times.Once()); + Assert.Null(completionItems); } [Fact] diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/LanguageServiceTestBase.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/LanguageServiceTestBase.cs index 2a27ef41..fda18ca5 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/LanguageServiceTestBase.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/LanguageServiceTestBase.cs @@ -69,12 +69,12 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.LanguageServer // set up file for returning the query scriptFile = new Mock(); scriptFile.SetupGet(file => file.Contents).Returns(GlobalCommon.Constants.StandardQuery); - scriptFile.SetupGet(file => file.ClientFilePath).Returns(this.testScriptUri); + scriptFile.SetupGet(file => file.ClientUri).Returns(this.testScriptUri); // set up workspace mock workspaceService = new Mock>(); workspaceService.Setup(service => service.Workspace.GetFile(It.IsAny())) - .Returns(scriptFile.Object); + .Returns((string filePath) => { return filePath == this.testScriptUri ? scriptFile.Object : null; }); // setup binding queue mock bindingQueue = new Mock();