Fix #911 handle perforce files (#400)

- Additional handling of document events with "perforce:" or other SCM strings
- Much of the handling had been added already, but adding in additional validation and ensuring that everywhere `Workspace.GetFile` is called we add a not-null check to avoid null reference errors
This commit is contained in:
Kevin Cunnane
2017-07-05 13:25:05 -07:00
committed by GitHub
parent 2a5ae06f12
commit 3aba287759
16 changed files with 152 additions and 40 deletions

View File

@@ -38,7 +38,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Formatter
public override void InitializeService(IProtocolEndpoint serviceHost)
{
{
Logger.Write(LogLevel.Verbose, "TSqlFormatter initialized");
serviceHost.SetRequestHandler(DocumentFormattingRequest.Type, HandleDocFormatRequest);
serviceHost.SetRequestHandler(DocumentRangeFormattingRequest.Type, HandleDocRangeFormatRequest);
@@ -107,6 +107,10 @@ namespace Microsoft.SqlTools.ServiceLayer.Formatter
{
var range = docFormatParams.Range;
ScriptFile scriptFile = GetFile(docFormatParams);
if (scriptFile == null)
{
return new TextEdit[0];
}
TextEdit textEdit = new TextEdit { Range = range };
string text = scriptFile.GetTextInRange(range.ToBufferRange());
return DoFormat(docFormatParams, textEdit, text);
@@ -118,7 +122,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Formatter
return await Task.Factory.StartNew(() =>
{
var scriptFile = GetFile(docFormatParams);
if (scriptFile.FileLines.Count == 0)
if (scriptFile == null
|| scriptFile.FileLines.Count == 0)
{
return new TextEdit[0];
}

View File

@@ -24,6 +24,7 @@ using Microsoft.SqlTools.ServiceLayer.LanguageServices.Contracts;
using Microsoft.SqlTools.ServiceLayer.QueryExecution;
using Microsoft.SqlTools.ServiceLayer.Scripting;
using Microsoft.SqlTools.ServiceLayer.SqlContext;
using Microsoft.SqlTools.ServiceLayer.Utility;
using Microsoft.SqlTools.ServiceLayer.Workspace;
using Microsoft.SqlTools.ServiceLayer.Workspace.Contracts;
using Microsoft.SqlTools.Utility;
@@ -297,6 +298,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
// get the current list of completion items and return to client
var scriptFile = CurrentWorkspace.GetFile(
textDocumentPosition.TextDocument.Uri);
if (scriptFile == null)
{
await requestContext.SendResult(null);
return;
}
ConnectionInfo connInfo;
ConnectionServiceInstance.TryFindConnection(
@@ -343,9 +349,15 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
// Retrieve document and connection
ConnectionInfo connInfo;
var scriptFile = CurrentWorkspace.GetFile(textDocumentPosition.TextDocument.Uri);
bool isConnected = ConnectionServiceInstance.TryFindConnection(scriptFile.ClientFilePath, out connInfo);
bool isConnected = false;
bool succeeded = false;
DefinitionResult definitionResult = GetDefinition(textDocumentPosition, scriptFile, connInfo);
DefinitionResult definitionResult = null;
if (scriptFile != null)
{
isConnected = ConnectionServiceInstance.TryFindConnection(scriptFile.ClientFilePath, out connInfo);
definitionResult = GetDefinition(textDocumentPosition, scriptFile, connInfo);
}
if (definitionResult != null)
{
if (definitionResult.IsErrorResult)
@@ -413,8 +425,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
{
ScriptFile scriptFile = CurrentWorkspace.GetFile(
textDocumentPosition.TextDocument.Uri);
SignatureHelp help = GetSignatureHelp(textDocumentPosition, scriptFile);
SignatureHelp help = null;
if (scriptFile != null)
{
help = GetSignatureHelp(textDocumentPosition, scriptFile);
}
if (help != null)
{
await requestContext.SendResult(help);
@@ -437,7 +452,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
var scriptFile = CurrentWorkspace.GetFile(
textDocumentPosition.TextDocument.Uri);
var hover = GetHoverItem(textDocumentPosition, scriptFile);
Hover hover = null;
if (scriptFile != null)
{
hover = GetHoverItem(textDocumentPosition, scriptFile);
}
if (hover != null)
{
await requestContext.SendResult(hover);

View File

@@ -7,6 +7,7 @@ using System;
using System.IO;
using Microsoft.SqlTools.ServiceLayer.QueryExecution.Contracts;
using Microsoft.SqlTools.ServiceLayer.SqlContext;
using Microsoft.SqlTools.ServiceLayer.Utility;
namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage
{

View File

@@ -7,6 +7,7 @@ using System;
using System.IO;
using Microsoft.SqlTools.ServiceLayer.QueryExecution.Contracts;
using Microsoft.SqlTools.ServiceLayer.SqlContext;
using Microsoft.SqlTools.ServiceLayer.Utility;
namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage
{

View File

@@ -7,6 +7,7 @@ using System;
using System.IO;
using Microsoft.SqlTools.ServiceLayer.QueryExecution.Contracts;
using Microsoft.SqlTools.ServiceLayer.SqlContext;
using Microsoft.SqlTools.ServiceLayer.Utility;
namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage
{

View File

@@ -5,6 +5,7 @@
using System.IO;
using Microsoft.SqlTools.ServiceLayer.SqlContext;
using Microsoft.SqlTools.ServiceLayer.Utility;
namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage
{

View File

@@ -11,6 +11,7 @@ using System.IO;
using System.Text;
using Microsoft.SqlTools.ServiceLayer.QueryExecution.Contracts;
using Microsoft.SqlTools.ServiceLayer.SqlContext;
using Microsoft.SqlTools.ServiceLayer.Utility;
using Microsoft.SqlTools.Utility;
namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage

View File

@@ -693,7 +693,10 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution
{
// Get the document from the parameters
ScriptFile queryFile = WorkspaceService.Workspace.GetFile(docRequest.OwnerUri);
if (queryFile == null)
{
return string.Empty;
}
// If a selection was not provided, use the entire document
if (docRequest.QuerySelection == null)
{

View File

@@ -17,7 +17,7 @@ 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.QueryExecution;
using Microsoft.SqlTools.ServiceLayer.Utility;
using Microsoft.SqlTools.ServiceLayer.Workspace.Contracts;
using Microsoft.SqlTools.Utility;
using ConnectionType = Microsoft.SqlTools.ServiceLayer.Connection.ConnectionType;

View File

@@ -4,7 +4,7 @@
//
using System;
using System.IO;
namespace Microsoft.SqlTools.ServiceLayer.QueryExecution
namespace Microsoft.SqlTools.ServiceLayer.Utility
{
internal static class FileUtilities
{
@@ -136,8 +136,18 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution
File.SetAttributes(fullFilePath, FileAttributes.Normal);
}
}
internal static ResolvedFile TryGetFullPath(string filePath)
{
try
{
return new ResolvedFile(Path.GetFullPath(filePath), true);
}
catch(NotSupportedException)
{
// This is not a standard path.
return new ResolvedFile(filePath, false);
}
}
}
}

View File

@@ -0,0 +1,39 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//
namespace Microsoft.SqlTools.ServiceLayer.Utility
{
/// <summary>
/// Utility object holding a result of a file resolve action.
///
/// Workspace APIs support reading from disk if a file hasn't been
/// officially opened via VSCode APIs with a buffer. This is problematic
/// in the case where it's not a file on disk as any attempt will cause
/// an exception to be thrown.
///
/// To mitigate this a ResolvedFile object has an additional flag indicating
/// if the file can be read from disk.
/// </summary>
internal class ResolvedFile
{
public ResolvedFile(string filePath, bool canReadFromDisk)
{
FilePath = filePath;
CanReadFromDisk = canReadFromDisk;
}
public string FilePath { get; private set; }
public bool CanReadFromDisk { get; private set; }
public string LowercaseFilePath
{
get
{
return FilePath?.ToLower();
}
}
}
}

View File

@@ -12,6 +12,7 @@ using System.Linq;
using Microsoft.SqlTools.Utility;
using Microsoft.SqlTools.ServiceLayer.Workspace.Contracts;
using System.Runtime.InteropServices;
using Microsoft.SqlTools.ServiceLayer.Utility;
namespace Microsoft.SqlTools.ServiceLayer.Workspace
{
@@ -67,8 +68,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
Validate.IsNotNullOrWhitespaceString("filePath", filePath);
// Resolve the full file path
string resolvedFilePath = this.ResolveFilePath(filePath);
string keyName = resolvedFilePath.ToLower();
ResolvedFile resolvedFile = this.ResolveFilePath(filePath);
string keyName = resolvedFile.LowercaseFilePath;
ScriptFile scriptFile = null;
return this.workspaceFiles.TryGetValue(keyName, out scriptFile);
@@ -95,36 +96,39 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
}
// Resolve the full file path
string resolvedFilePath = this.ResolveFilePath(filePath);
string keyName = resolvedFilePath.ToLower();
ResolvedFile resolvedFile = this.ResolveFilePath(filePath);
string keyName = resolvedFile.LowercaseFilePath;
// Make sure the file isn't already loaded into the workspace
ScriptFile scriptFile = null;
if (!this.workspaceFiles.TryGetValue(keyName, out scriptFile))
{
if (IsUntitled(resolvedFilePath))
if (IsUntitled(resolvedFile.FilePath)
|| !resolvedFile.CanReadFromDisk)
{
// It's not a registered untitled file, so any attempt to read from disk will fail as it's in memory
// It's either not a registered untitled file, or not a valid file on disk
// so any attempt to read from disk will fail.
return null;
}
// This method allows FileNotFoundException to bubble up
// if the file isn't found.
using (FileStream fileStream = new FileStream(resolvedFilePath, FileMode.Open, FileAccess.Read))
using (FileStream fileStream = new FileStream(resolvedFile.FilePath, FileMode.Open, FileAccess.Read))
using (StreamReader streamReader = new StreamReader(fileStream, Encoding.UTF8))
{
scriptFile = new ScriptFile(resolvedFilePath, filePath,streamReader);
scriptFile = new ScriptFile(resolvedFile.FilePath, filePath,streamReader);
this.workspaceFiles.Add(keyName, scriptFile);
}
Logger.Write(LogLevel.Verbose, "Opened file on disk: " + resolvedFilePath);
Logger.Write(LogLevel.Verbose, "Opened file on disk: " + resolvedFile.FilePath);
}
return scriptFile;
}
private string ResolveFilePath(string filePath)
private ResolvedFile ResolveFilePath(string filePath)
{
bool canReadFromDisk = false;
if (!IsPathInMemoryOrNonFileUri(filePath))
{
if (filePath.StartsWith(@"file://"))
@@ -151,12 +155,14 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
}
// Get the absolute file path
filePath = Path.GetFullPath(filePath);
ResolvedFile resolvedFile = FileUtilities.TryGetFullPath(filePath);
filePath = resolvedFile.FilePath;
canReadFromDisk = resolvedFile.CanReadFromDisk;
}
Logger.Write(LogLevel.Verbose, "Resolved path: " + filePath);
return filePath;
return new ResolvedFile(filePath, canReadFromDisk);
}
/// <summary>
@@ -191,18 +197,18 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
}
// Resolve the full file path
string resolvedFilePath = this.ResolveFilePath(filePath);
string keyName = resolvedFilePath.ToLower();
ResolvedFile resolvedFile = this.ResolveFilePath(filePath);
string keyName = resolvedFile.LowercaseFilePath;
// Make sure the file isn't already loaded into the workspace
ScriptFile scriptFile = null;
if (!this.workspaceFiles.TryGetValue(keyName, out scriptFile))
{
scriptFile = new ScriptFile(resolvedFilePath, filePath, initialBuffer);
scriptFile = new ScriptFile(resolvedFile.FilePath, filePath, initialBuffer);
this.workspaceFiles.Add(keyName, scriptFile);
Logger.Write(LogLevel.Verbose, "Opened file as in-memory buffer: " + resolvedFilePath);
Logger.Write(LogLevel.Verbose, "Opened file as in-memory buffer: " + resolvedFile.FilePath);
}
return scriptFile;

View File

@@ -216,13 +216,15 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
msg.AppendLine(string.Format(" File: {0}", fileUri));
ScriptFile changedFile = Workspace.GetFile(fileUri);
if (changedFile != null)
{
changedFile.ApplyChange(
GetFileChangeDetails(
textChange.Range.Value,
textChange.Text));
changedFile.ApplyChange(
GetFileChangeDetails(
textChange.Range.Value,
textChange.Text));
changedFiles.Add(changedFile);
changedFiles.Add(changedFile);
}
}
Logger.Write(LogLevel.Verbose, msg.ToString());
@@ -244,14 +246,17 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
{
Logger.Write(LogLevel.Verbose, "HandleDidOpenTextDocumentNotification");
if (IsScmEvent(openParams.TextDocument.Uri))
if (IsScmEvent(openParams.TextDocument.Uri))
{
return;
}
// read the SQL file contents into the ScriptFile
ScriptFile openedFile = Workspace.GetFileBuffer(openParams.TextDocument.Uri, openParams.TextDocument.Text);
if (openedFile == null)
{
return;
}
// Propagate the changes to the event handlers
var textDocOpenTasks = TextDocOpenCallbacks.Select(
t => t(openedFile, eventContext));

View File

@@ -12,6 +12,7 @@ using Microsoft.SqlTools.ServiceLayer.Test.Common;
using Microsoft.SqlTools.ServiceLayer.Test.Common.Baselined;
using Xunit;
using Microsoft.SqlTools.ServiceLayer.QueryExecution;
using Microsoft.SqlTools.ServiceLayer.Utility;
namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.BatchParser
{

View File

@@ -17,10 +17,10 @@ using Microsoft.SqlTools.Hosting.Protocol.Contracts;
using Microsoft.SqlTools.ServiceLayer.Connection;
using Microsoft.SqlTools.ServiceLayer.LanguageServices;
using Microsoft.SqlTools.ServiceLayer.LanguageServices.Contracts;
using Microsoft.SqlTools.ServiceLayer.QueryExecution;
using Microsoft.SqlTools.ServiceLayer.Scripting;
using Microsoft.SqlTools.ServiceLayer.SqlContext;
using Microsoft.SqlTools.ServiceLayer.UnitTests.Utility;
using Microsoft.SqlTools.ServiceLayer.Utility;
using Microsoft.SqlTools.ServiceLayer.Workspace;
using Microsoft.SqlTools.ServiceLayer.Workspace.Contracts;
using Moq;

View File

@@ -149,12 +149,22 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Workspace
[Fact]
public async Task DontProcessGitFileEvents()
{
// setup test workspace
await VerifyFileIsNotAddedOnDocOpened("git:/myfile.sql");
}
[Fact]
public async Task DontProcessPerforceFileEvents()
{
await VerifyFileIsNotAddedOnDocOpened("perforce:/myfile.sql");
}
private async Task VerifyFileIsNotAddedOnDocOpened(string filePath)
{
// setup test workspace
var workspace = new ServiceLayer.Workspace.Workspace();
var workspaceService = new WorkspaceService<SqlToolsSettings> {Workspace = workspace};
// send a document open event with git:/ prefix URI
string filePath = "git:/myfile.sql";
var openParams = new DidOpenTextDocumentNotification
{
TextDocument = new TextDocumentItem { Uri = filePath }
@@ -178,6 +188,15 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Workspace
Assert.False(workspaceService.Workspace.ContainsFile(filePath));
}
[Fact]
public void GetFileReturnsNullForPerforceFile()
{
// when I ask for a non-file object in the workspace, it should return null
var workspace = new ServiceLayer.Workspace.Workspace();
ScriptFile file = workspace.GetFile("perforce:myfile.sql");
Assert.Null(file);
}
[Fact]
public async Task WorkspaceContainsFile()
{