diff --git a/src/Microsoft.VisualStudio.Editor.Razor/DefaultVisualStudioRazorParser.cs b/src/Microsoft.VisualStudio.Editor.Razor/DefaultVisualStudioRazorParser.cs index d8e1955ce0..dedcd3e3ce 100644 --- a/src/Microsoft.VisualStudio.Editor.Razor/DefaultVisualStudioRazorParser.cs +++ b/src/Microsoft.VisualStudio.Editor.Razor/DefaultVisualStudioRazorParser.cs @@ -11,7 +11,6 @@ using Microsoft.AspNetCore.Razor.Language; using Microsoft.AspNetCore.Razor.Language.Legacy; using Microsoft.CodeAnalysis.Razor; using Microsoft.CodeAnalysis.Razor.Editor; -using Microsoft.CodeAnalysis.Razor.ProjectSystem; using Microsoft.VisualStudio.Language.Intellisense; using Microsoft.VisualStudio.Text; using ITextBuffer = Microsoft.VisualStudio.Text.ITextBuffer; @@ -27,6 +26,7 @@ namespace Microsoft.VisualStudio.Editor.Razor internal TimeSpan IdleDelay = TimeSpan.FromSeconds(3); internal Timer _idleTimer; internal BackgroundParser _parser; + internal ChangeReference _latestChangeReference; private readonly object IdleLock = new object(); private readonly ICompletionBroker _completionBroker; @@ -96,6 +96,8 @@ namespace Microsoft.VisualStudio.Editor.Razor public override ITextBuffer TextBuffer => _documentTracker.TextBuffer; + public override bool HasPendingChanges => _latestChangeReference != null; + // Used in unit tests to ensure we can be notified when idle starts. internal ManualResetEventSlim NotifyForegroundIdleStart { get; set; } @@ -253,7 +255,7 @@ namespace Microsoft.VisualStudio.Editor.Razor // If partial parsing failed or there were outstanding parser tasks, start a full reparse if ((result & PartialParseResultInternal.Rejected) == PartialParseResultInternal.Rejected) { - _parser.QueueChange(change, snapshot); + QueueChange(change, snapshot); } if ((result & PartialParseResultInternal.Provisional) == PartialParseResultInternal.Provisional) @@ -292,7 +294,15 @@ namespace Microsoft.VisualStudio.Editor.Razor } var snapshot = TextBuffer.CurrentSnapshot; - _parser.QueueChange(null, snapshot); + QueueChange(null, snapshot); + } + + private void QueueChange(SourceChange change, ITextSnapshot snapshot) + { + _dispatcher.AssertForegroundThread(); + + _latestChangeReference = new ChangeReference(change, snapshot); + _parser.QueueChange(change, snapshot); } private void OnNotifyForegroundIdle() @@ -311,7 +321,7 @@ namespace Microsoft.VisualStudio.Editor.Razor } } - private async void Timer_Tick(object state) + private void Timer_Tick(object state) { try { @@ -322,12 +332,12 @@ namespace Microsoft.VisualStudio.Editor.Razor StopIdleTimer(); // We need to get back to the UI thread to properly check if a completion is active. - await Task.Factory.StartNew(OnIdle, null, CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler); + Task.Factory.StartNew(OnIdle, null, CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler); } catch (Exception ex) { // This is something totally unexpected, let's just send it over to the workspace. - await Task.Factory.StartNew(() => _errorReporter.ReportError(ex), CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler); + Task.Factory.StartNew(() => _errorReporter.ReportError(ex), CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler); } } @@ -335,19 +345,29 @@ namespace Microsoft.VisualStudio.Editor.Razor { _dispatcher.AssertBackgroundThread(); - if (DocumentStructureChanged != null) - { - if (args.Snapshot != TextBuffer.CurrentSnapshot) - { - // A different text change is being parsed. - return; - } + // Jump back to UI thread to notify structure changes. + Task.Factory.StartNew(OnDocumentStructureChanged, args, CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler); + } - _codeDocument = args.CodeDocument; - _snapshot = args.Snapshot; - _partialParser = new RazorSyntaxTreePartialParser(CodeDocument.GetSyntaxTree()); - DocumentStructureChanged(this, args); + // Internal for testing + internal void OnDocumentStructureChanged(object state) + { + _dispatcher.AssertForegroundThread(); + + var args = (DocumentStructureChangedEventArgs)state; + if (_latestChangeReference == null || // extra hardening + !_latestChangeReference.IsAssociatedWith(args)) + { + // In the middle of parsing a newer change. + return; } + + _latestChangeReference = null; + _codeDocument = args.CodeDocument; + _snapshot = args.Snapshot; + _partialParser = new RazorSyntaxTreePartialParser(CodeDocument.GetSyntaxTree()); + + DocumentStructureChanged?.Invoke(this, args); } private void ConfigureTemplateEngine(IRazorEngineBuilder builder) @@ -399,5 +419,25 @@ namespace Microsoft.VisualStudio.Editor.Razor return Array.Empty(); } } + + // Internal for testing + internal class ChangeReference + { + public ChangeReference(SourceChange change, ITextSnapshot snapshot) + { + Change = change; + Snapshot = snapshot; + } + + public SourceChange Change { get; } + + public ITextSnapshot Snapshot { get; } + + public bool IsAssociatedWith(DocumentStructureChangedEventArgs other) + { + return Change == other.SourceChange && + Snapshot == other.Snapshot; + } + } } } diff --git a/src/Microsoft.VisualStudio.Editor.Razor/VisualStudioRazorParser.cs b/src/Microsoft.VisualStudio.Editor.Razor/VisualStudioRazorParser.cs index 1a512faedf..e8dd96cf90 100644 --- a/src/Microsoft.VisualStudio.Editor.Razor/VisualStudioRazorParser.cs +++ b/src/Microsoft.VisualStudio.Editor.Razor/VisualStudioRazorParser.cs @@ -21,6 +21,8 @@ namespace Microsoft.VisualStudio.Editor.Razor public abstract ITextBuffer TextBuffer { get; } + public abstract bool HasPendingChanges { get; } + public abstract void QueueReparse(); } } diff --git a/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserIntegrationTest.cs b/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserIntegrationTest.cs index fcca740062..3117f5b585 100644 --- a/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserIntegrationTest.cs +++ b/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserIntegrationTest.cs @@ -27,44 +27,30 @@ namespace Microsoft.VisualStudio.Editor.Razor private const string TestProjectPath = "C:\\This\\Path\\Is\\Just\\For\\Project.csproj"; [ForegroundFact] - public void BufferChangeStartsFullReparseIfChangeOverlapsMultipleSpans() + public async Task BufferChangeStartsFullReparseIfChangeOverlapsMultipleSpans() { // Arrange var original = new StringTextSnapshot("Foo @bar Baz"); var testBuffer = new TestTextBuffer(original); var documentTracker = CreateDocumentTracker(testBuffer); - using (var parser = new DefaultVisualStudioRazorParser( - Dispatcher, - documentTracker, - CreateTemplateEngineFactory(), - new DefaultErrorReporter(), - new TestCompletionBroker())) + using (var manager = CreateParserManager(original)) { - parser.DocumentTracker_ContextChanged(null, null); var changed = new StringTextSnapshot("Foo @bap Daz"); var edit = new TestEdit(7, 3, original, 3, changed, "p D"); - var parseComplete = new ManualResetEventSlim(); - var parseCount = 0; - parser.DocumentStructureChanged += (s, a) => - { - Interlocked.Increment(ref parseCount); - parseComplete.Set(); - }; + + await manager.InitializeWithDocumentAsync(edit.OldSnapshot); // Act - 1 - testBuffer.ApplyEdit(edit); - DoWithTimeoutIfNotDebugging(parseComplete.Wait); // Wait for the parse to finish + await manager.ApplyEditAndWaitForParseAsync(edit); // Assert - 1 - Assert.Equal(1, parseCount); - parseComplete.Reset(); + Assert.Equal(2, manager.ParseCount); // Act - 2 - testBuffer.ApplyEdit(edit); + await manager.ApplyEditAndWaitForParseAsync(edit); // Assert - 2 - DoWithTimeoutIfNotDebugging(parseComplete.Wait); - Assert.Equal(2, parseCount); + Assert.Equal(3, manager.ParseCount); } } @@ -78,7 +64,7 @@ namespace Microsoft.VisualStudio.Editor.Razor var factory = new SpanFactory(); var changed = new StringTextSnapshot("foo @await Html. baz"); var edit = new TestEdit(15, 0, original, 1, changed, "."); - manager.InitializeWithDocument(edit.OldSnapshot); + await manager.InitializeWithDocumentAsync(edit.OldSnapshot); // Act await manager.ApplyEditAndWaitForReparseAsync(edit); @@ -95,7 +81,7 @@ namespace Microsoft.VisualStudio.Editor.Razor } [ForegroundFact] - public void ImplicitExpressionAcceptsDotlessCommitInsertionsInStatementBlockAfterIdentifiers() + public async Task ImplicitExpressionAcceptsDotlessCommitInsertionsInStatementBlockAfterIdentifiers() { var factory = new SpanFactory(); var changed = new StringTextSnapshot("@{" + Environment.NewLine @@ -130,7 +116,7 @@ namespace Microsoft.VisualStudio.Editor.Razor factory.EmptyHtml())); }; - manager.InitializeWithDocument(edit.OldSnapshot); + await manager.InitializeWithDocumentAsync(edit.OldSnapshot); // This is the process of a dotless commit when doing "." insertions to commit intellisense changes. ApplyAndVerifyPartialChange(edit, "DateTime."); @@ -154,7 +140,7 @@ namespace Microsoft.VisualStudio.Editor.Razor } [ForegroundFact] - public void ImplicitExpressionAcceptsDotlessCommitInsertionsInStatementBlock() + public async Task ImplicitExpressionAcceptsDotlessCommitInsertionsInStatementBlock() { var factory = new SpanFactory(); var changed = new StringTextSnapshot("@{" + Environment.NewLine @@ -189,7 +175,7 @@ namespace Microsoft.VisualStudio.Editor.Razor factory.EmptyHtml())); }; - manager.InitializeWithDocument(edit.OldSnapshot); + await manager.InitializeWithDocumentAsync(edit.OldSnapshot); // This is the process of a dotless commit when doing "." insertions to commit intellisense changes. ApplyAndVerifyPartialChange(edit, "DateT."); @@ -226,7 +212,7 @@ namespace Microsoft.VisualStudio.Editor.Razor factory.Markup(" baz"))); }; - manager.InitializeWithDocument(edit.OldSnapshot); + await manager.InitializeWithDocumentAsync(edit.OldSnapshot); // This is the process of a dotless commit when doing "." insertions to commit intellisense changes. ApplyAndVerifyPartialChange(edit, "DateT."); @@ -272,7 +258,7 @@ namespace Microsoft.VisualStudio.Editor.Razor factory.Markup(" baz"))); }; - manager.InitializeWithDocument(edit.OldSnapshot); + await manager.InitializeWithDocumentAsync(edit.OldSnapshot); // This is the process of a dotless commit when doing "." insertions to commit intellisense changes. ApplyAndVerifyPartialChange(edit, "DateTime."); @@ -324,7 +310,7 @@ namespace Microsoft.VisualStudio.Editor.Razor factory.Markup(" baz"))); }; - manager.InitializeWithDocument(edit.OldSnapshot); + await manager.InitializeWithDocumentAsync(edit.OldSnapshot); // This is the process of a dotless commit when doing "." insertions to commit intellisense changes. @@ -374,13 +360,13 @@ namespace Microsoft.VisualStudio.Editor.Razor var charTyped = new TestEdit(14, 0, new StringTextSnapshot("foo @foo. @bar"), 1, new StringTextSnapshot("foo @foo. @barb"), "b"); using (var manager = CreateParserManager(dotTyped.OldSnapshot)) { - manager.InitializeWithDocument(dotTyped.OldSnapshot); + await manager.InitializeWithDocumentAsync(dotTyped.OldSnapshot); // Apply the dot change await manager.ApplyEditAndWaitForReparseAsync(dotTyped); // Act (apply the identifier start char change) - manager.ApplyEditAndWaitForParse(charTyped); + await manager.ApplyEditAndWaitForParseAsync(charTyped); // Assert Assert.Equal(2, manager.ParseCount); @@ -403,7 +389,7 @@ namespace Microsoft.VisualStudio.Editor.Razor } [ForegroundFact] - public void ImplicitExpressionAcceptsIdentifierTypedAfterDotIfLastChangeWasProvisionalAcceptanceOfDot() + public async Task ImplicitExpressionAcceptsIdentifierTypedAfterDotIfLastChangeWasProvisionalAcceptanceOfDot() { // Arrange var factory = new SpanFactory(); @@ -411,7 +397,7 @@ namespace Microsoft.VisualStudio.Editor.Razor var charTyped = new TestEdit(9, 0, new StringTextSnapshot("foo @foo. bar"), 1, new StringTextSnapshot("foo @foo.b bar"), "b"); using (var manager = CreateParserManager(dotTyped.OldSnapshot)) { - manager.InitializeWithDocument(dotTyped.OldSnapshot); + await manager.InitializeWithDocumentAsync(dotTyped.OldSnapshot); // Apply the dot change manager.ApplyEdit(dotTyped); @@ -434,87 +420,87 @@ namespace Microsoft.VisualStudio.Editor.Razor } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfIfKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfIfKeywordTyped() { - RunTypeKeywordTest("if"); + await RunTypeKeywordTestAsync("if"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfDoKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfDoKeywordTyped() { - RunTypeKeywordTest("do"); + await RunTypeKeywordTestAsync("do"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfTryKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfTryKeywordTyped() { - RunTypeKeywordTest("try"); + await RunTypeKeywordTestAsync("try"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfForKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfForKeywordTyped() { - RunTypeKeywordTest("for"); + await RunTypeKeywordTestAsync("for"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfForEachKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfForEachKeywordTyped() { - RunTypeKeywordTest("foreach"); + await RunTypeKeywordTestAsync("foreach"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfWhileKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfWhileKeywordTyped() { - RunTypeKeywordTest("while"); + await RunTypeKeywordTestAsync("while"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfSwitchKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfSwitchKeywordTyped() { - RunTypeKeywordTest("switch"); + await RunTypeKeywordTestAsync("switch"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfLockKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfLockKeywordTyped() { - RunTypeKeywordTest("lock"); + await RunTypeKeywordTestAsync("lock"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfUsingKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfUsingKeywordTyped() { - RunTypeKeywordTest("using"); + await RunTypeKeywordTestAsync("using"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfSectionKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfSectionKeywordTyped() { - RunTypeKeywordTest("section"); + await RunTypeKeywordTestAsync("section"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfInheritsKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfInheritsKeywordTyped() { - RunTypeKeywordTest("inherits"); + await RunTypeKeywordTestAsync("inherits"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfFunctionsKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfFunctionsKeywordTyped() { - RunTypeKeywordTest("functions"); + await RunTypeKeywordTestAsync("functions"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfNamespaceKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfNamespaceKeywordTyped() { - RunTypeKeywordTest("namespace"); + await RunTypeKeywordTestAsync("namespace"); } [ForegroundFact] - public void ImplicitExpressionCorrectlyTriggersReparseIfClassKeywordTyped() + public async Task ImplicitExpressionCorrectlyTriggersReparseIfClassKeywordTyped() { - RunTypeKeywordTest("class"); + await RunTypeKeywordTestAsync("class"); } private TestParserManager CreateParserManager(ITextSnapshot originalSnapshot) @@ -570,7 +556,7 @@ namespace Microsoft.VisualStudio.Editor.Razor return templateEngineFactory; } - private void RunTypeKeywordTest(string keyword) + private async Task RunTypeKeywordTestAsync(string keyword) { // Arrange var before = "@" + keyword.Substring(0, keyword.Length - 1); @@ -581,10 +567,10 @@ namespace Microsoft.VisualStudio.Editor.Razor var edit = new TestEdit(change, old, changed); using (var manager = CreateParserManager(old)) { - manager.InitializeWithDocument(edit.OldSnapshot); + await manager.InitializeWithDocumentAsync(edit.OldSnapshot); // Act - manager.ApplyEditAndWaitForParse(edit); + await manager.ApplyEditAndWaitForParseAsync(edit); // Assert Assert.Equal(2, manager.ParseCount); @@ -657,12 +643,12 @@ namespace Microsoft.VisualStudio.Editor.Razor public RazorSyntaxTree CurrentSyntaxTree { get; private set; } - public void InitializeWithDocument(ITextSnapshot snapshot) + public async Task InitializeWithDocumentAsync(ITextSnapshot snapshot) { var old = new StringTextSnapshot(string.Empty); var initialChange = new SourceChange(0, 0, snapshot.GetText()); var edit = new TestEdit(initialChange, old, snapshot); - ApplyEditAndWaitForParse(edit); + await ApplyEditAndWaitForParseAsync(edit); } public void ApplyEdit(TestEdit edit) @@ -670,10 +656,10 @@ namespace Microsoft.VisualStudio.Editor.Razor _testBuffer.ApplyEdit(edit); } - public void ApplyEditAndWaitForParse(TestEdit edit) + public async Task ApplyEditAndWaitForParseAsync(TestEdit edit) { ApplyEdit(edit); - WaitForParse(); + await WaitForParseAsync(); } public async Task ApplyEditAndWaitForReparseAsync(TestEdit edit) @@ -682,9 +668,14 @@ namespace Microsoft.VisualStudio.Editor.Razor await WaitForReparseAsync(); } - public void WaitForParse() + public async Task WaitForParseAsync() { - DoWithTimeoutIfNotDebugging(_parserComplete.Wait); // Wait for the parse to finish + // Get off of the foreground thread so we can wait for the document structure changed event to fire + await Task.Run(() => + { + DoWithTimeoutIfNotDebugging(_parserComplete.Wait); + }); + _parserComplete.Reset(); } @@ -703,7 +694,11 @@ namespace Microsoft.VisualStudio.Editor.Razor Assert.Null(_parser._idleTimer); - DoWithTimeoutIfNotDebugging(_reparseComplete.Wait); + // Get off of the foreground thread so we can wait for the document structure changed event to fire for reparse + await Task.Run(() => + { + DoWithTimeoutIfNotDebugging(_reparseComplete.Wait); + }); _reparseComplete.Reset(); _parser.BlockBackgroundIdleWork.Reset(); diff --git a/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserTest.cs b/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserTest.cs index ed1e89f1fe..0ef27cad70 100644 --- a/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserTest.cs +++ b/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserTest.cs @@ -4,6 +4,7 @@ using System; using System.Linq; using System.Threading; +using Microsoft.AspNetCore.Razor.Language; using Microsoft.CodeAnalysis.Razor; using Microsoft.VisualStudio.Language.Intellisense; using Microsoft.VisualStudio.Test; @@ -26,6 +27,63 @@ namespace Microsoft.VisualStudio.Editor.Razor return documentTracker; } + [ForegroundFact] + public void OnDocumentStructureChanged_IgnoresEditsThatAreOld() + { + // Arrange + using (var parser = new DefaultVisualStudioRazorParser( + Dispatcher, + CreateDocumentTracker(), + Mock.Of(), + new DefaultErrorReporter(), + Mock.Of())) + { + var called = false; + parser.DocumentStructureChanged += (sender, e) => called = true; + parser._latestChangeReference = new DefaultVisualStudioRazorParser.ChangeReference(null, new StringTextSnapshot(string.Empty)); + var args = new DocumentStructureChangedEventArgs( + new SourceChange(0, 0, string.Empty), + new StringTextSnapshot(string.Empty), + TestRazorCodeDocument.CreateEmpty()); + + // Act + parser.OnDocumentStructureChanged(args); + + // Assert + Assert.False(called); + } + } + + [ForegroundFact] + public void OnDocumentStructureChanged_FiresForLatestTextBufferEdit() + { + // Arrange + var documentTracker = CreateDocumentTracker(); + using (var parser = new DefaultVisualStudioRazorParser( + Dispatcher, + documentTracker, + Mock.Of(), + new DefaultErrorReporter(), + Mock.Of())) + { + var called = false; + parser.DocumentStructureChanged += (sender, e) => called = true; + var latestChange = new SourceChange(0, 0, string.Empty); + var latestSnapshot = documentTracker.TextBuffer.CurrentSnapshot; + parser._latestChangeReference = new DefaultVisualStudioRazorParser.ChangeReference(latestChange, latestSnapshot); + var args = new DocumentStructureChangedEventArgs( + latestChange, + latestSnapshot, + TestRazorCodeDocument.CreateEmpty()); + + // Act + parser.OnDocumentStructureChanged(args); + + // Assert + Assert.True(called); + } + } + [ForegroundFact] public void StartIdleTimer_DoesNotRestartTimerWhenAlreadyRunning() {