diff --git a/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs b/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs index 4cfb6df6f..dcbbc12bb 100644 --- a/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs +++ b/src/Analysis/Ast/Impl/Documents/DocumentBuffer.cs @@ -13,7 +13,6 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. -using System; using System.Collections.Generic; using System.Linq; using System.Text; @@ -47,23 +46,18 @@ public void Update(IEnumerable changes) { lock (_lock) { _sb = _sb ?? new StringBuilder(_content); - var lastStart = int.MaxValue; - var lineLoc = SplitLines(_sb).ToArray(); - foreach (var change in changes) { + // Every change may change where the lines end so in order + // to correctly determine line/offsets we must re-split buffer + // into lines after each change. + var lineLoc = GetNewLineLications().ToArray(); + if (change.ReplaceAllText) { _sb = new StringBuilder(change.InsertedText); - lastStart = int.MaxValue; - lineLoc = SplitLines(_sb).ToArray(); continue; } var start = NewLineLocation.LocationToIndex(lineLoc, change.ReplacedSpan.Start, _sb.Length); - if (start > lastStart) { - throw new InvalidOperationException("changes must be in reverse order of start location"); - } - lastStart = start; - var end = NewLineLocation.LocationToIndex(lineLoc, change.ReplacedSpan.End, _sb.Length); if (end > start) { _sb.Remove(start, end - start); @@ -78,20 +72,33 @@ public void Update(IEnumerable changes) { } } - private static IEnumerable SplitLines(StringBuilder text) { - NewLineLocation nextLine; - - // TODO: Avoid string allocation by operating directly on StringBuilder - var str = text.ToString(); + public IEnumerable GetNewLineLications() { + _sb = _sb ?? new StringBuilder(_content); // for tests - var lastLineEnd = 0; - while ((nextLine = NewLineLocation.FindNewLine(str, lastLineEnd)).EndIndex != lastLineEnd) { - yield return nextLine; - lastLineEnd = nextLine.EndIndex; + if (_sb.Length == 0) { + yield return new NewLineLocation(0, NewLineKind.None); } - if (lastLineEnd != str.Length) { - yield return nextLine; + for (var i = 0; i < _sb.Length; i++) { + var ch = _sb[i]; + var nextCh = i < _sb.Length - 1 ? _sb[i + 1] : '\0'; + switch (ch) { + case '\r' when nextCh == '\n': + i++; + yield return new NewLineLocation(i + 1, NewLineKind.CarriageReturnLineFeed); + break; + case '\n': + yield return new NewLineLocation(i + 1, NewLineKind.LineFeed); + break; + case '\r': + yield return new NewLineLocation(i + 1, NewLineKind.CarriageReturn); + break; + default: + if (i == _sb.Length - 1) { + yield return new NewLineLocation(i + 1, NewLineKind.None); + } + break; + } } } } diff --git a/src/Analysis/Ast/Test/DocumentBufferTests.cs b/src/Analysis/Ast/Test/DocumentBufferTests.cs index 58652eb0c..80b8d2b50 100644 --- a/src/Analysis/Ast/Test/DocumentBufferTests.cs +++ b/src/Analysis/Ast/Test/DocumentBufferTests.cs @@ -15,9 +15,12 @@ using System; using System.Collections.Generic; +using System.Linq; +using System.Reflection; using FluentAssertions; using Microsoft.Python.Analysis.Documents; using Microsoft.Python.Core.Text; +using Microsoft.Python.Parsing; using Microsoft.VisualStudio.TestTools.UnitTesting; namespace Microsoft.Python.Analysis.Tests { @@ -77,7 +80,7 @@ public void ResetDocumentBuffer() { doc.Reset(0, string.Empty); Assert.AreEqual(string.Empty, doc.Text); - doc.Update(new[] { + doc.Update(new[] { DocumentChange.Insert("text", SourceLocation.MinValue) }); @@ -127,5 +130,163 @@ public void ReplaceAllDocumentBuffer() { Assert.AreEqual(@"text1234", doc.Text); Assert.AreEqual(4, doc.Version); } + + [TestMethod, Priority(0)] + public void DeleteMultipleDisjoint() { + var doc = new DocumentBuffer(); + doc.Reset(0, @" +line1 +line2 +line3 +line4 +"); + doc.Update(new[] { + DocumentChange.Delete(new SourceSpan(5, 5, 5, 6)), + DocumentChange.Delete(new SourceSpan(4, 5, 4, 6)), + DocumentChange.Delete(new SourceSpan(3, 5, 3, 6)), + DocumentChange.Delete(new SourceSpan(2, 5, 2, 6)) + }); + Assert.AreEqual(@" +line +line +line +line +", doc.Text); + } + + [TestMethod, Priority(0)] + public void InsertMultipleDisjoint() { + var doc = new DocumentBuffer(); + doc.Reset(0, @" +line +line +line +line +"); + doc.Update(new[] { + DocumentChange.Insert("4", new SourceLocation(5, 5)), + DocumentChange.Insert("3", new SourceLocation(4, 5)), + DocumentChange.Insert("2", new SourceLocation(3, 5)), + DocumentChange.Insert("1", new SourceLocation(2, 5)), + }); + Assert.AreEqual(@" +line1 +line2 +line3 +line4 +", doc.Text); + } + + [TestMethod, Priority(0)] + public void DeleteAcrossLines() { + var doc = new DocumentBuffer(); + doc.Reset(0, @" +line1 +line2 +line3 +line4 +"); + doc.Update(new[] { + DocumentChange.Delete(new SourceSpan(4, 5, 5, 5)), + DocumentChange.Delete(new SourceSpan(2, 5, 3, 5)), + }); + Assert.AreEqual(@" +line2 +line4 +", doc.Text); + } + + [TestMethod, Priority(0)] + public void SequentialChanges() { + var doc = new DocumentBuffer(); + doc.Reset(0, @" +line1 +line2 +line3 +line4 +"); + doc.Update(new[] { + DocumentChange.Delete(new SourceSpan(2, 5, 3, 5)), + DocumentChange.Delete(new SourceSpan(3, 5, 4, 5)) + }); + Assert.AreEqual(@" +line2 +line4 +", doc.Text); + } + + [TestMethod, Priority(0)] + public void InsertTopToBottom() { + var doc = new DocumentBuffer(); + doc.Reset(0, @"linelinelineline"); + doc.Update(new[] { + DocumentChange.Insert("\n", new SourceLocation(1, 1)), + DocumentChange.Insert("1\n", new SourceLocation(2, 5)), + DocumentChange.Insert("2\r", new SourceLocation(3, 5)), + DocumentChange.Insert("3\r\n", new SourceLocation(4, 5)), + DocumentChange.Insert("4\r\n", new SourceLocation(5, 5)), + }); + Assert.AreEqual("\nline1\nline2\rline3\r\nline4\r\n", doc.Text); + } + + [NewLineTestData] + [DataTestMethod, Priority(0)] + public void NewLines(string s, NewLineLocation[] expected) { + var doc = new DocumentBuffer(); + doc.Reset(0, s); + var nls = doc.GetNewLineLications().ToArray(); + for (var i = 0; i < nls.Length; i++) { + Assert.AreEqual(nls[i].Kind, expected[i].Kind); + Assert.AreEqual(nls[i].EndIndex, expected[i].EndIndex); + } + } + + private sealed class NewLineTestDataAttribute : Attribute, ITestDataSource { + public IEnumerable GetData(MethodInfo methodInfo) => + new List { + new object[] { + string.Empty, + new NewLineLocation[] { + new NewLineLocation(0, NewLineKind.None), + } + }, + new object[] { + "\r\r", + new NewLineLocation[] { + new NewLineLocation(1, NewLineKind.CarriageReturn), + new NewLineLocation(2, NewLineKind.CarriageReturn) + } + }, + new object[] { + "\n\n", + new NewLineLocation[] { + new NewLineLocation(1, NewLineKind.LineFeed), + new NewLineLocation(2, NewLineKind.LineFeed) + } + }, + new object[] { + "\r\n\n\r", + new NewLineLocation[] { + new NewLineLocation(2, NewLineKind.CarriageReturnLineFeed), + new NewLineLocation(3, NewLineKind.LineFeed), + new NewLineLocation(4, NewLineKind.CarriageReturn) + } + }, + new object[] { + "a\r\nb\r\n c\r d\ne", + new NewLineLocation[] { + new NewLineLocation(3, NewLineKind.CarriageReturnLineFeed), + new NewLineLocation(6, NewLineKind.CarriageReturnLineFeed), + new NewLineLocation(9, NewLineKind.CarriageReturn), + new NewLineLocation(12, NewLineKind.LineFeed), + new NewLineLocation(13, NewLineKind.None) + } + } + }; + public string GetDisplayName(MethodInfo methodInfo, object[] data) + => data != null ? $"{methodInfo.Name} ({FormatName((string)data[0])})" : null; + + private static string FormatName(string s) => s.Replace("\n", "\\n").Replace("\r", "\\r"); + } } } diff --git a/src/LanguageServer/Impl/Implementation/Server.Documents.cs b/src/LanguageServer/Impl/Implementation/Server.Documents.cs index e7ef1a04e..43b7937ec 100644 --- a/src/LanguageServer/Impl/Implementation/Server.Documents.cs +++ b/src/LanguageServer/Impl/Implementation/Server.Documents.cs @@ -13,8 +13,10 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. +using System; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using Microsoft.Python.Analysis.Documents; using Microsoft.Python.Core; using Microsoft.Python.LanguageServer.Protocol; @@ -32,15 +34,10 @@ public void DidOpenTextDocument(DidOpenTextDocumentParams @params) { public void DidChangeTextDocument(DidChangeTextDocumentParams @params) { _disposableBag.ThrowIfDisposed(); - var uri = @params.textDocument.uri; - var doc = _rdt.GetDocument(uri); + var doc = _rdt.GetDocument(@params.textDocument.uri); if (doc != null) { - var changes = new List(); - foreach (var c in @params.contentChanges) { - var change = c.range.HasValue ? DocumentChange.Replace(c.range.Value, c.text) : DocumentChange.ReplaceAll(c.text); - changes.Add(change); - } - doc.Update(changes); + doc.Update(@params.contentChanges + .Select(c => c.range.HasValue ? DocumentChange.Replace(c.range.Value, c.text) : DocumentChange.ReplaceAll(c.text))); _indexManager.AddPendingDoc(doc); } else { _log?.Log(TraceEventType.Warning, $"Unable to find document for {@params.textDocument.uri}"); diff --git a/src/LanguageServer/Impl/LanguageServer.cs b/src/LanguageServer/Impl/LanguageServer.cs index 5831b91fa..ca4d1e962 100644 --- a/src/LanguageServer/Impl/LanguageServer.cs +++ b/src/LanguageServer/Impl/LanguageServer.cs @@ -129,57 +129,10 @@ public async Task DidChangeTextDocument(JToken token, CancellationToken cancella _idleTimeTracker?.NotifyUserActivity(); using (await _prioritizer.DocumentChangePriorityAsync(cancellationToken)) { var @params = ToObject(token); - var version = @params.textDocument.version; - if (version == null || @params.contentChanges.IsNullOrEmpty()) { - _server.DidChangeTextDocument(@params); - return; - } - - // _server.DidChangeTextDocument can handle change buckets with decreasing version and without overlapping - // Split change into buckets that will be properly handled - var changes = SplitDidChangeTextDocumentParams(@params, version.Value); - - foreach (var change in changes) { - _server.DidChangeTextDocument(change); - } - } - } - - private static IEnumerable SplitDidChangeTextDocumentParams(DidChangeTextDocumentParams @params, int version) { - var changes = new Stack(); - var contentChanges = new Stack(); - var previousRange = new Range(); - - for (var i = @params.contentChanges.Length - 1; i >= 0; i--) { - var contentChange = @params.contentChanges[i]; - var range = contentChange.range.GetValueOrDefault(); - if (previousRange.end > range.start) { - changes.Push(CreateDidChangeTextDocumentParams(@params, version, contentChanges)); - contentChanges = new Stack(); - version--; - } - - contentChanges.Push(contentChange); - previousRange = range; - } - - if (contentChanges.Count > 0) { - changes.Push(CreateDidChangeTextDocumentParams(@params, version, contentChanges)); + _server.DidChangeTextDocument(@params); } - - return changes; } - private static DidChangeTextDocumentParams CreateDidChangeTextDocumentParams(DidChangeTextDocumentParams @params, int version, Stack contentChanges) - => new DidChangeTextDocumentParams { - _enqueueForAnalysis = @params._enqueueForAnalysis, - contentChanges = contentChanges.ToArray(), - textDocument = new VersionedTextDocumentIdentifier { - uri = @params.textDocument.uri, - version = version - } - }; - [JsonRpcMethod("textDocument/willSave")] public void WillSaveTextDocument(JToken token) { } diff --git a/src/LanguageServer/Impl/Protocol/Messages.cs b/src/LanguageServer/Impl/Protocol/Messages.cs index 2fb1305bb..ab253a760 100644 --- a/src/LanguageServer/Impl/Protocol/Messages.cs +++ b/src/LanguageServer/Impl/Protocol/Messages.cs @@ -126,9 +126,6 @@ public sealed class DidOpenTextDocumentParams { public sealed class DidChangeTextDocumentParams { public VersionedTextDocumentIdentifier textDocument; public TextDocumentContentChangedEvent[] contentChanges; - - // Defaults to true, but can be set to false to suppress analysis - public bool? _enqueueForAnalysis; } [Serializable]