From 1aa15374b53fb80a6307d3494dde393e70f070a4 Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Fri, 15 Jun 2018 15:20:03 -0700 Subject: [PATCH] Add partial parsing of `@functions` directive body. - Added ability to understand valid inserts, deletes and replacements for the `@functions` directive (and any other directive that uses our extensible code block bits). - Added unit tests. - Updated existing tests. - Found an issue when completing some C# items the auto-completions would impact the underlying snapshot after we'd captured the change. Fixed this by forcing a reparse when we detect that our understanding of the latest snapshot and the actual latest snapshot diverge. #2408 --- .../Legacy/CSharpCodeParser.cs | 4 + .../Legacy/CodeBlockEditHandler.cs | 126 ++++++++++ .../DefaultVisualStudioRazorParser.cs | 13 +- .../Legacy/CSharpAutoCompleteTest.cs | 2 +- .../Legacy/CSharpDirectivesTest.cs | 6 +- .../Legacy/CSharpErrorTest.cs | 2 +- .../Legacy/CSharpSpecialBlockTest.cs | 4 +- .../Legacy/CodeBlockEditHandlerTest.cs | 221 ++++++++++++++++++ .../Language/Legacy/TestSpanBuilder.cs | 5 + 9 files changed, 374 insertions(+), 9 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Razor.Language/Legacy/CodeBlockEditHandler.cs create mode 100644 test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CodeBlockEditHandlerTest.cs diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/CSharpCodeParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/CSharpCodeParser.cs index 13d8352ba6..621bfff401 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/CSharpCodeParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/CSharpCodeParser.cs @@ -1818,7 +1818,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy NextToken(); Balance(BalancingModes.NoErrorOnFailure, CSharpSymbolType.LeftBrace, CSharpSymbolType.RightBrace, startingBraceLocation); Span.ChunkGenerator = new StatementChunkGenerator(); + var existingEditHandler = Span.EditHandler; + Span.EditHandler = new CodeBlockEditHandler(Language.TokenizeString); Output(SpanKindInternal.Code); + + Span.EditHandler = existingEditHandler; }); break; } diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/CodeBlockEditHandler.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/CodeBlockEditHandler.cs new file mode 100644 index 0000000000..0430388d81 --- /dev/null +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/CodeBlockEditHandler.cs @@ -0,0 +1,126 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Globalization; + +namespace Microsoft.AspNetCore.Razor.Language.Legacy +{ + internal class CodeBlockEditHandler : SpanEditHandler + { + public CodeBlockEditHandler(Func> tokenizer) : base(tokenizer) + { + } + + protected override PartialParseResultInternal CanAcceptChange(Span target, SourceChange change) + { + if (IsAcceptableDeletion(target, change)) + { + return PartialParseResultInternal.Accepted; + } + + if (IsAcceptableReplacement(target, change)) + { + return PartialParseResultInternal.Accepted; + } + + if (IsAcceptableInsertion(change)) + { + return PartialParseResultInternal.Accepted; + } + + return PartialParseResultInternal.Rejected; + } + + // Internal for testing + internal static bool IsAcceptableReplacement(Span target, SourceChange change) + { + if (!change.IsReplace) + { + return false; + } + + if (ContainsInvalidContent(change)) + { + return false; + } + + if (ModifiesInvalidContent(target, change)) + { + return false; + } + + return true; + } + + // Internal for testing + internal static bool IsAcceptableDeletion(Span target, SourceChange change) + { + if (!change.IsDelete) + { + return false; + } + + if (ModifiesInvalidContent(target, change)) + { + return false; + } + + return true; + } + + // Internal for testing + internal static bool ModifiesInvalidContent(Span target, SourceChange change) + { + var relativePosition = change.Span.AbsoluteIndex - target.Start.AbsoluteIndex; + + if (target.Content.IndexOfAny(new[] { '{', '}' }, relativePosition, change.Span.Length) >= 0) + { + return true; + } + + return false; + } + + // Internal for testing + internal static bool IsAcceptableInsertion(SourceChange change) + { + if (!change.IsInsert) + { + return false; + } + + if (ContainsInvalidContent(change)) + { + return false; + } + + return true; + } + + // Internal for testing + internal static bool ContainsInvalidContent(SourceChange change) + { + if (change.NewText.IndexOfAny(new[] { '{', '}' }) >= 0) + { + return true; + } + + return false; + } + + public override string ToString() + { + return string.Format(CultureInfo.InvariantCulture, "{0};CodeBlock", base.ToString()); + } + + public override bool Equals(object obj) + { + return obj is CodeBlockEditHandler other && + base.Equals(other); + } + + public override int GetHashCode() => base.GetHashCode(); + } +} diff --git a/src/Microsoft.VisualStudio.Editor.Razor/DefaultVisualStudioRazorParser.cs b/src/Microsoft.VisualStudio.Editor.Razor/DefaultVisualStudioRazorParser.cs index b34901659b..23ae8208c5 100644 --- a/src/Microsoft.VisualStudio.Editor.Razor/DefaultVisualStudioRazorParser.cs +++ b/src/Microsoft.VisualStudio.Editor.Razor/DefaultVisualStudioRazorParser.cs @@ -377,13 +377,22 @@ namespace Microsoft.VisualStudio.Editor.Razor var backgroundParserArgs = (BackgroundParserResultsReadyEventArgs)state; if (_latestChangeReference == null || // extra hardening - _latestChangeReference != backgroundParserArgs.ChangeReference || - backgroundParserArgs.ChangeReference.Snapshot != TextBuffer.CurrentSnapshot) + _latestChangeReference != backgroundParserArgs.ChangeReference) { // In the middle of parsing a newer change or about to parse a newer change. return; } + if (backgroundParserArgs.ChangeReference.Snapshot != TextBuffer.CurrentSnapshot) + { + // Changes have impacted the snapshot after our we recorded our last change reference. + // This can happen for a multitude of reasons, usually because of a user auto-completing + // C# statements (causes multiple edits in quick succession). This ensures that our latest + // parse corresponds to the current snapshot. + QueueReparse(); + return; + } + _latestChangeReference = null; _codeDocument = backgroundParserArgs.CodeDocument; _snapshot = backgroundParserArgs.ChangeReference.Snapshot; diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpAutoCompleteTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpAutoCompleteTest.cs index 13633efdd1..fba29ab569 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpAutoCompleteTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpAutoCompleteTest.cs @@ -86,7 +86,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.CodeTransition(), Factory.MetaCode("functions").Accepts(AcceptedCharactersInternal.None), Factory.MetaCode("{").AutoCompleteWith("}", atEndOfSpan: true).Accepts(AcceptedCharactersInternal.None), - Factory.Code(Environment.NewLine + "foo").AsStatement())); + Factory.Code(Environment.NewLine + "foo").AsCodeBlock())); } [Fact] diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpDirectivesTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpDirectivesTest.cs index 23cda39e22..f8a1359575 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpDirectivesTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpDirectivesTest.cs @@ -694,7 +694,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.MetaCode("{") .AutoCompleteWith(null, atEndOfSpan: true) .Accepts(AcceptedCharactersInternal.None), - Factory.Code(" foo(); bar(); ").AsStatement(), + Factory.Code(" foo(); bar(); ").AsCodeBlock(), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None))); } @@ -1561,7 +1561,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.MetaCode("functions").Accepts(AcceptedCharactersInternal.None), Factory.Span(SpanKindInternal.Markup, " ", CSharpSymbolType.WhiteSpace).Accepts(AcceptedCharactersInternal.AllWhiteSpace), Factory.MetaCode("{").AutoCompleteWith(null, atEndOfSpan: true).Accepts(AcceptedCharactersInternal.None), - Factory.Code(" foo(); bar(); ").AsStatement(), + Factory.Code(" foo(); bar(); ").AsCodeBlock(), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None))); } @@ -1576,7 +1576,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.MetaCode("functions").Accepts(AcceptedCharactersInternal.None), Factory.Span(SpanKindInternal.Markup, " ", CSharpSymbolType.WhiteSpace).Accepts(AcceptedCharactersInternal.AllWhiteSpace), Factory.MetaCode("{").AutoCompleteWith(null, atEndOfSpan: true).Accepts(AcceptedCharactersInternal.None), - Factory.Code(" ").AsStatement(), + Factory.Code(" ").AsCodeBlock(), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None))); } diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpErrorTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpErrorTest.cs index 4d792c1a31..257d8a202d 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpErrorTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpErrorTest.cs @@ -306,7 +306,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.MetaCode("functions").Accepts(AcceptedCharactersInternal.None), Factory.Span(SpanKindInternal.Markup, " ", CSharpSymbolType.WhiteSpace).Accepts(AcceptedCharactersInternal.AllWhiteSpace), Factory.MetaCode("{").AutoCompleteWith("}", atEndOfSpan: true).Accepts(AcceptedCharactersInternal.None), - Factory.Code(" var foo = bar; if(foo != null) { bar(); } ").AsStatement())); + Factory.Code(" var foo = bar; if(foo != null) { bar(); } ").AsCodeBlock())); } [Fact] diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpSpecialBlockTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpSpecialBlockTest.cs index ca472f33f5..f94734641e 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpSpecialBlockTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CSharpSpecialBlockTest.cs @@ -174,7 +174,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.MetaCode("functions").Accepts(AcceptedCharactersInternal.None), Factory.Span(SpanKindInternal.Markup, " ", CSharpSymbolType.WhiteSpace).Accepts(AcceptedCharactersInternal.AllWhiteSpace), Factory.MetaCode("{").AutoCompleteWith(null, atEndOfSpan: true).Accepts(AcceptedCharactersInternal.None), - Factory.Code(code).AsStatement(), + Factory.Code(code).AsCodeBlock(), Factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None))); } @@ -195,7 +195,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy Factory.MetaCode("functions").Accepts(AcceptedCharactersInternal.None), Factory.Span(SpanKindInternal.Markup, " ", CSharpSymbolType.WhiteSpace).Accepts(AcceptedCharactersInternal.AllWhiteSpace), Factory.MetaCode("{").AutoCompleteWith("}", atEndOfSpan: true).Accepts(AcceptedCharactersInternal.None), - Factory.Code(" { { { { } zoop").AsStatement())); + Factory.Code(" { { { { } zoop").AsCodeBlock())); } [Fact] diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CodeBlockEditHandlerTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CodeBlockEditHandlerTest.cs new file mode 100644 index 0000000000..df17deb584 --- /dev/null +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/CodeBlockEditHandlerTest.cs @@ -0,0 +1,221 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Linq; +using Microsoft.AspNetCore.Razor.Language.Legacy; +using Xunit; + +namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy +{ + public class CodeBlockEditHandlerTest + { + [Fact] + public void IsAcceptableReplacement_AcceptableReplacement_ReturnsTrue() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "Hello {world}."); + var change = new SourceChange(new SourceSpan(0, 5), "H3ll0"); + + // Act + var result = CodeBlockEditHandler.IsAcceptableReplacement(span, change); + + // Assert + Assert.True(result); + } + + [Fact] + public void IsAcceptableReplacement_ChangeModifiesInvalidContent_ReturnsFalse() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "Hello {world}."); + var change = new SourceChange(new SourceSpan(6, 1), "!"); + + // Act + var result = CodeBlockEditHandler.IsAcceptableReplacement(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsAcceptableReplacement_ChangeContainsInvalidContent_ReturnsFalse() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "Hello {world}."); + var change = new SourceChange(new SourceSpan(0, 0), "{"); + + // Act + var result = CodeBlockEditHandler.IsAcceptableReplacement(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsAcceptableReplacement_NotReplace_ReturnsFalse() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "Hello {world}."); + var change = new SourceChange(new SourceSpan(0, 5), string.Empty); + + // Act + var result = CodeBlockEditHandler.IsAcceptableReplacement(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsAcceptableDeletion_ValidChange_ReturnsTrue() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "Hello {world}."); + var change = new SourceChange(new SourceSpan(0, 5), string.Empty); + + // Act + var result = CodeBlockEditHandler.IsAcceptableDeletion(span, change); + + // Assert + Assert.True(result); + } + + [Fact] + public void IsAcceptableDeletion_InvalidChange_ReturnsFalse() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "Hello {world}."); + var change = new SourceChange(new SourceSpan(5, 3), string.Empty); + + // Act + var result = CodeBlockEditHandler.IsAcceptableDeletion(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsAcceptableDeletion_NotDelete_ReturnsFalse() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "world"); + var change = new SourceChange(new SourceSpan(0, 0), "hello"); + + // Act + var result = CodeBlockEditHandler.IsAcceptableDeletion(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void ModifiesInvalidContent_ValidContent_ReturnsFalse() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "Hello {world}."); + var change = new SourceChange(new SourceSpan(0, 5), string.Empty); + + // Act + var result = CodeBlockEditHandler.ModifiesInvalidContent(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void ModifiesInvalidContent_InvalidContent_ReturnsTrue() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "Hello {world}."); + var change = new SourceChange(new SourceSpan(5, 7), string.Empty); + + // Act + var result = CodeBlockEditHandler.ModifiesInvalidContent(span, change); + + // Assert + Assert.True(result); + } + + [Fact] + public void IsAcceptableInsertion_ValidChange_ReturnsTrue() + { + // Arrange + var change = new SourceChange(new SourceSpan(0, 0), "hello"); + + // Act + var result = CodeBlockEditHandler.IsAcceptableInsertion(change); + + // Assert + Assert.True(result); + } + + [Fact] + public void IsAcceptableInsertion_InvalidChange_ReturnsFalse() + { + // Arrange + var change = new SourceChange(new SourceSpan(0, 0), "{"); + + // Act + var result = CodeBlockEditHandler.IsAcceptableInsertion(change); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsAcceptableInsertion_NotInsert_ReturnsFalse() + { + // Arrange + var change = new SourceChange(new SourceSpan(0, 2), string.Empty); + + // Act + var result = CodeBlockEditHandler.IsAcceptableInsertion(change); + + // Assert + Assert.False(result); + } + + [Theory] + [InlineData("{")] + [InlineData("}")] + [InlineData("if (true) { }")] + public void ContainsInvalidContent_InvalidContent_ReturnsTrue(string content) + { + // Arrange + var change = new SourceChange(new SourceSpan(0, 0), content); + + // Act + var result = CodeBlockEditHandler.ContainsInvalidContent(change); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData("var x = true;")] + [InlineData("if (true) Console.WriteLine('!')")] + public void ContainsInvalidContent_ValidContent_ReturnsFalse(string content) + { + // Arrange + var change = new SourceChange(new SourceSpan(0, 0), content); + + // Act + var result = CodeBlockEditHandler.ContainsInvalidContent(change); + + // Assert + Assert.False(result); + } + + private static Span GetSpan(SourceLocation start, string content) + { + var spanBuilder = new SpanBuilder(start); + var tokens = CSharpLanguageCharacteristics.Instance.TokenizeString(content).ToArray(); + foreach (var token in tokens) + { + spanBuilder.Accept(token); + } + var span = spanBuilder.Build(); + + return span; + } + } +} diff --git a/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/TestSpanBuilder.cs b/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/TestSpanBuilder.cs index ed7b3aadde..adbe7e886e 100644 --- a/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/TestSpanBuilder.cs +++ b/test/Microsoft.AspNetCore.Razor.Test.Common/Language/Legacy/TestSpanBuilder.cs @@ -307,6 +307,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return _self.With(new StatementChunkGenerator()); } + public SpanConstructor AsCodeBlock() + { + return AsStatement().With(new CodeBlockEditHandler(CSharpLanguageCharacteristics.Instance.TokenizeString)); + } + public SpanConstructor AsExpression() { return _self.With(new ExpressionChunkGenerator());