diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/ImplicitExpressionEditHandler.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/ImplicitExpressionEditHandler.cs index ababd6c7f7..a55c2a8949 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/ImplicitExpressionEditHandler.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/ImplicitExpressionEditHandler.cs @@ -109,11 +109,21 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return HandleInsertion(target, lastChar.Value, change); } + if (IsAcceptableInsertionInBalancedParenthesis(target, change)) + { + return PartialParseResultInternal.Accepted; + } + if (IsAcceptableDeletion(target, change)) { return HandleDeletion(target, lastChar.Value, change); } + if (IsAcceptableDeletionInBalancedParenthesis(target, change)) + { + return PartialParseResultInternal.Accepted; + } + return PartialParseResultInternal.Rejected; } @@ -216,8 +226,196 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy private static bool IsAcceptableInsertion(Span target, SourceChange change) { return change.IsInsert && - (IsAcceptableEndInsertion(target, change) || - IsAcceptableInnerInsertion(target, change)); + (IsAcceptableEndInsertion(target, change) || + IsAcceptableInnerInsertion(target, change)); + } + + // Internal for testing + internal static bool IsAcceptableDeletionInBalancedParenthesis(Span target, SourceChange change) + { + if (!change.IsDelete) + { + return false; + } + + var changeStart = change.Span.AbsoluteIndex; + var changeLength = change.Span.Length; + var changeEnd = changeStart + changeLength; + var tokens = target.Symbols.Cast().ToArray(); + if (!IsInsideParenthesis(changeStart, tokens) || !IsInsideParenthesis(changeEnd, tokens)) + { + // Either the start or end of the delete does not fall inside of parenthesis, unacceptable inner deletion. + return false; + } + + var relativePosition = changeStart - target.Start.AbsoluteIndex; + var deletionContent = target.Content.Substring(relativePosition, changeLength); + + if (deletionContent.IndexOfAny(new[] { '(', ')' }) >= 0) + { + // Change deleted some parenthesis + return false; + } + + return true; + } + + // Internal for testing + internal static bool IsAcceptableInsertionInBalancedParenthesis(Span target, SourceChange change) + { + if (!change.IsInsert) + { + return false; + } + + if (change.NewText.IndexOfAny(new[] { '(', ')' }) >= 0) + { + // Insertions of parenthesis aren't handled by us. If someone else wants to accept it, they can. + return false; + } + + var tokens = target.Symbols.Cast().ToArray(); + if (IsInsideParenthesis(change.Span.AbsoluteIndex, tokens)) + { + return true; + } + + return false; + } + + // Internal for testing + internal static bool IsInsideParenthesis(int position, IReadOnlyList tokens) + { + var balanceCount = 0; + var foundInsertionPoint = false; + for (var i = 0; i < tokens.Count; i++) + { + var currentToken = tokens[i]; + if (ContainsPosition(position, currentToken)) + { + if (balanceCount == 0) + { + // Insertion point is outside of parenthesis, i.e. inserting at the pipe: @Foo|Baz() + return false; + } + + foundInsertionPoint = true; + } + + if (!TryUpdateBalanceCount(currentToken, ref balanceCount)) + { + // Couldn't update the count. This usually occurrs when we run into a ')' outside of any parenthesis. + return false; + } + + if (foundInsertionPoint && balanceCount == 0) + { + // Once parenthesis become balanced after the insertion point we return true, no need to go further. + // If they get unbalanced down the line the expression was already unbalanced to begin with and this + // change happens prior to any ambiguity. + return true; + } + } + + // Unbalanced parenthesis + return false; + } + + // Internal for testing + internal static bool ContainsPosition(int position, CSharpSymbol currentToken) + { + var tokenStart = currentToken.Start.AbsoluteIndex; + if (tokenStart == position) + { + // Token is exactly at the insertion point. + return true; + } + + var tokenEnd = tokenStart + currentToken.Content.Length; + if (tokenStart < position && tokenEnd > position) + { + // Insertion point falls in the middle of the current token. + return true; + } + + return false; + } + + // Internal for testing + internal static bool TryUpdateBalanceCount(CSharpSymbol token, ref int count) + { + var updatedCount = count; + if (token.Type == CSharpSymbolType.LeftParenthesis) + { + updatedCount++; + } + else if (token.Type == CSharpSymbolType.RightParenthesis) + { + if (updatedCount == 0) + { + return false; + } + + updatedCount--; + } + else if (token.Type == CSharpSymbolType.StringLiteral) + { + var content = token.Content; + if (content.Length > 0 && content[content.Length - 1] != '"') + { + // Incomplete string literal may have consumed some of our parenthesis and usually occurr during auto-completion of '"' => '""'. + if (!TryUpdateCountFromContent(content, ref updatedCount)) + { + return false; + } + } + } + else if (token.Type == CSharpSymbolType.CharacterLiteral) + { + var content = token.Content; + if (content.Length > 0 && content[content.Length - 1] != '\'') + { + // Incomplete character literal may have consumed some of our parenthesis and usually occurr during auto-completion of "'" => "''". + if (!TryUpdateCountFromContent(content, ref updatedCount)) + { + return false; + } + } + } + + if (updatedCount < 0) + { + return false; + } + + count = updatedCount; + return true; + } + + // Internal for testing + internal static bool TryUpdateCountFromContent(string content, ref int count) + { + var updatedCount = count; + for (var i = 0; i < content.Length; i++) + { + if (content[i] == '(') + { + updatedCount++; + } + else if (content[i] == ')') + { + if (updatedCount == 0) + { + // Unbalanced parenthesis, i.e. @Foo) + return false; + } + + updatedCount--; + } + } + + count = updatedCount; + return true; } // Accepts character insertions at the end of spans. AKA: '@foo' -> '@fooo' or '@foo' -> '@foo ' etc. @@ -291,10 +489,16 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { return TryAcceptChange(target, change); } - else + else if (previousChar == '(') { - return PartialParseResultInternal.Rejected; + var changeRelativePosition = change.Span.AbsoluteIndex - target.Start.AbsoluteIndex; + if (target.Content[changeRelativePosition] == ')') + { + return PartialParseResultInternal.Accepted | PartialParseResultInternal.Provisional; + } } + + return PartialParseResultInternal.Rejected; } private PartialParseResultInternal HandleInsertion(Span target, char previousChar, SourceChange change) @@ -308,6 +512,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { return HandleInsertionAfterIdPart(target, change); } + else if (previousChar == '(') + { + return HandleInsertionAfterOpenParenthesis(target, change); + } else { return PartialParseResultInternal.Rejected; @@ -321,6 +529,12 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { return TryAcceptChange(target, change); } + else if (IsDoubleParenthesisInsertion(change) || IsOpenParenthesisInsertion(change)) + { + // Allow inserting parens after an identifier - this is needed to support signature + // help intellisense in VS. + return TryAcceptChange(target, change); + } else if (EndsWithDot(change.NewText)) { // Accept it, possibly provisionally @@ -337,6 +551,40 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy } } + private PartialParseResultInternal HandleInsertionAfterOpenParenthesis(Span target, SourceChange change) + { + if (IsCloseParenthesisInsertion(change)) + { + return TryAcceptChange(target, change); + } + + return PartialParseResultInternal.Rejected; + } + + private static bool IsDoubleParenthesisInsertion(SourceChange change) + { + return + change.IsInsert && + change.NewText.Length == 2 && + change.NewText == "()"; + } + + private static bool IsOpenParenthesisInsertion(SourceChange change) + { + return + change.IsInsert && + change.NewText.Length == 1 && + change.NewText == "("; + } + + private static bool IsCloseParenthesisInsertion(SourceChange change) + { + return + change.IsInsert && + change.NewText.Length == 1 && + change.NewText == ")"; + } + private static bool EndsWithDot(string content) { return (content.Length == 1 && content[0] == '.') || diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/ImplicitExpressionEditHandlerTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/ImplicitExpressionEditHandlerTest.cs new file mode 100644 index 0000000000..14436d244c --- /dev/null +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/ImplicitExpressionEditHandlerTest.cs @@ -0,0 +1,463 @@ +// 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.Collections.Generic; +using System.Linq; +using Xunit; + +namespace Microsoft.AspNetCore.Razor.Language.Legacy +{ + public class ImplicitExpressionEditHandlerTest + { + [Fact] + public void IsAcceptableDeletionInBalancedParenthesis_DeletionStartNotInBalancedParenthesis_ReturnsFalse() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "(Hell)(o)"); + var change = new SourceChange(new SourceSpan(6, 1), string.Empty); + + // Act + var result = ImplicitExpressionEditHandler.IsAcceptableDeletionInBalancedParenthesis(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsAcceptableDeletionInBalancedParenthesis_DeletionEndNotInBalancedParenthesis_ReturnsFalse() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "(Hell)(o)"); + var change = new SourceChange(new SourceSpan(5, 1), string.Empty); + + // Act + var result = ImplicitExpressionEditHandler.IsAcceptableDeletionInBalancedParenthesis(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsAcceptableDeletionInBalancedParenthesis_DeletionOverlapsBalancedParenthesis_ReturnsFalse() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "(Hell)(o)"); + var change = new SourceChange(new SourceSpan(5, 2), string.Empty); + + // Act + var result = ImplicitExpressionEditHandler.IsAcceptableDeletionInBalancedParenthesis(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsAcceptableDeletionInBalancedParenthesis_DeletionDoesNotImpactBalancedParenthesis_ReturnsTrue() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "(H(ell)o)"); + var change = new SourceChange(new SourceSpan(3, 3), string.Empty); + + // Act + var result = ImplicitExpressionEditHandler.IsAcceptableDeletionInBalancedParenthesis(span, change); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData("(")] + [InlineData(")")] + public void IsAcceptableInsertionInBalancedParenthesis_ReturnsFalseIfChangeIsParenthesis(string changeText) + { + // Arrange + var change = new SourceChange(0, 1, changeText); + + // Act + var result = ImplicitExpressionEditHandler.IsAcceptableInsertionInBalancedParenthesis(null, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void TryUpdateCountFromContent_SingleLeftParenthesis_CountsCorrectly() + { + // Arrange + var content = "("; + var count = 0; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateCountFromContent(content, ref count); + + // Assert + Assert.True(result); + Assert.Equal(1, count); + } + + [Fact] + public void TryUpdateCountFromContent_SingleRightParenthesis_CountsCorrectly() + { + // Arrange + var content = ")"; + var count = 2; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateCountFromContent(content, ref count); + + // Assert + Assert.True(result); + Assert.Equal(1, count); + } + + [Fact] + public void TryUpdateCountFromContent_CorrectCount_ReturnsTrue() + { + // Arrange + var content = "\"(()("; + var count = 0; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateCountFromContent(content, ref count); + + // Assert + Assert.True(result); + Assert.Equal(2, count); + } + + [Fact] + public void TryUpdateCountFromContent_ExistingCountAndNonParenthesisContent_ReturnsTrue() + { + // Arrange + var content = "'(abc)de)fg"; + var count = 1; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateCountFromContent(content, ref count); + + // Assert + Assert.True(result); + Assert.Equal(0, count); + } + + [Fact] + public void TryUpdateCountFromContent_InvalidParenthesis_ReturnsFalse() + { + // Arrange + var content = "'())))))"; + var count = 4; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateCountFromContent(content, ref count); + + // Assert + Assert.False(result); + Assert.Equal(4, count); + } + + [Fact] + public void TryUpdateBalanceCount_SingleLeftParenthesis_CountsCorrectly() + { + // Arrange + var token = new CSharpSymbol("(", CSharpSymbolType.LeftParenthesis); + var count = 0; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateBalanceCount(token, ref count); + + // Assert + Assert.True(result); + Assert.Equal(1, count); + } + + [Fact] + public void TryUpdateBalanceCount_SingleRightParenthesis_CountsCorrectly() + { + // Arrange + var token = new CSharpSymbol(")", CSharpSymbolType.RightParenthesis); + var count = 2; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateBalanceCount(token, ref count); + + // Assert + Assert.True(result); + Assert.Equal(1, count); + } + + [Fact] + public void TryUpdateBalanceCount_IncompleteStringLiteral_CountsCorrectly() + { + // Arrange + var token = new CSharpSymbol("\"((", CSharpSymbolType.StringLiteral); + var count = 2; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateBalanceCount(token, ref count); + + // Assert + Assert.True(result); + Assert.Equal(4, count); + } + + [Fact] + public void TryUpdateBalanceCount_IncompleteCharacterLiteral_CountsCorrectly() + { + // Arrange + var token = new CSharpSymbol("'((", CSharpSymbolType.CharacterLiteral); + var count = 2; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateBalanceCount(token, ref count); + + // Assert + Assert.True(result); + Assert.Equal(4, count); + } + + [Fact] + public void TryUpdateBalanceCount_CompleteStringLiteral_CountsCorrectly() + { + // Arrange + var token = new CSharpSymbol("\"((\"", CSharpSymbolType.StringLiteral); + var count = 2; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateBalanceCount(token, ref count); + + // Assert + Assert.True(result); + Assert.Equal(2, count); + } + + [Fact] + public void TryUpdateBalanceCount_CompleteCharacterLiteral_CountsCorrectly() + { + // Arrange + var token = new CSharpSymbol("'('", CSharpSymbolType.CharacterLiteral); + var count = 2; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateBalanceCount(token, ref count); + + // Assert + Assert.True(result); + Assert.Equal(2, count); + } + + [Fact] + public void TryUpdateBalanceCount_InvalidParenthesis_ReturnsFalse() + { + // Arrange + var token = new CSharpSymbol(")", CSharpSymbolType.RightParenthesis); + var count = 0; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateBalanceCount(token, ref count); + + // Assert + Assert.False(result); + Assert.Equal(0, count); + } + + [Fact] + public void TryUpdateBalanceCount_InvalidParenthesisStringLiteral_ReturnsFalse() + { + // Arrange + var token = new CSharpSymbol("\")", CSharpSymbolType.StringLiteral); + var count = 0; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateBalanceCount(token, ref count); + + // Assert + Assert.False(result); + Assert.Equal(0, count); + } + + [Fact] + public void TryUpdateBalanceCount_InvalidParenthesisCharacterLiteral_ReturnsFalse() + { + // Arrange + var token = new CSharpSymbol("')", CSharpSymbolType.CharacterLiteral); + var count = 0; + + // Act + var result = ImplicitExpressionEditHandler.TryUpdateBalanceCount(token, ref count); + + // Assert + Assert.False(result); + Assert.Equal(0, count); + } + + [Fact] + public void ContainsPosition_AtStartOfToken_ReturnsTrue() + { + // Arrange + var token = GetTokens(new SourceLocation(4, 1, 2), "hello").Single(); + + // Act + var result = ImplicitExpressionEditHandler.ContainsPosition(4, token); + + // Assert + Assert.True(result); + } + + [Fact] + public void ContainsPosition_InsideOfToken_ReturnsTrue() + { + // Arrange + var token = GetTokens(new SourceLocation(4, 1, 2), "hello").Single(); + + // Act + var result = ImplicitExpressionEditHandler.ContainsPosition(6, token); + + // Assert + Assert.True(result); + } + + [Fact] + public void ContainsPosition_AtEndOfToken_ReturnsFalse() + { + // Arrange + var token = GetTokens(new SourceLocation(4, 1, 2), "hello").Single(); + + // Act + var result = ImplicitExpressionEditHandler.ContainsPosition(9, token); + + // Assert + Assert.False(result); + } + + [Theory] + [InlineData(10)] + [InlineData(2)] + public void ContainsPosition_OutsideOfToken_ReturnsFalse(int position) + { + // Arrange + var token = GetTokens(new SourceLocation(4, 1, 2), "hello").Single(); + + // Act + var result = ImplicitExpressionEditHandler.ContainsPosition(position, token); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsInsideParenthesis_SurroundedByCompleteParenthesis_ReturnsFalse() + { + // Arrange + var tokens = GetTokens(SourceLocation.Zero, "(hello)point(world)"); + + // Act + var result = ImplicitExpressionEditHandler.IsInsideParenthesis(9, tokens); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsInsideParenthesis_InvalidParenthesis_ReturnsFalse() + { + // Arrange + var tokens = GetTokens(SourceLocation.Zero, "(hello))point)"); + + // Act + var result = ImplicitExpressionEditHandler.IsInsideParenthesis(10, tokens); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsInsideParenthesis_NoParenthesis_ReturnsFalse() + { + // Arrange + var tokens = GetTokens(SourceLocation.Zero, "Hello World"); + + // Act + var result = ImplicitExpressionEditHandler.IsInsideParenthesis(3, tokens); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsInsideParenthesis_InBalancedParenthesis_ReturnsTrue() + { + // Arrange + var tokens = GetTokens(SourceLocation.Zero, "Foo(GetValue(), DoSomething(point))"); + + // Act + var result = ImplicitExpressionEditHandler.IsInsideParenthesis(30, tokens); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData("(")] + [InlineData(")")] + public void IsAcceptableInsertionInBalancedParenthesis_InsertingParenthesis_ReturnsFalse(string text) + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "(Hello World)"); + var change = new SourceChange(new SourceSpan(3, 0), text); + + // Act + var result = ImplicitExpressionEditHandler.IsAcceptableInsertionInBalancedParenthesis(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsAcceptableInsertionInBalancedParenthesis_UnbalancedParenthesis_ReturnsFalse() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "(Hello"); + var change = new SourceChange(new SourceSpan(6, 0), " World"); + + // Act + var result = ImplicitExpressionEditHandler.IsAcceptableInsertionInBalancedParenthesis(span, change); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsAcceptableInsertionInBalancedParenthesis_BalancedParenthesis_ReturnsTrue() + { + // Arrange + var span = GetSpan(SourceLocation.Zero, "(Hello)"); + var change = new SourceChange(new SourceSpan(6, 0), " World"); + + // Act + var result = ImplicitExpressionEditHandler.IsAcceptableInsertionInBalancedParenthesis(span, change); + + // Assert + Assert.True(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; + } + + private static IReadOnlyList GetTokens(SourceLocation start, string content) + { + var parent = GetSpan(start, content); + var tokens = parent.Symbols.Cast().ToArray(); + return tokens; + } + } +} diff --git a/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserIntegrationTest.cs b/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserIntegrationTest.cs index f253aec895..22bad6622c 100644 --- a/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserIntegrationTest.cs +++ b/test/Microsoft.VisualStudio.Editor.Razor.Test/DefaultVisualStudioRazorParserIntegrationTest.cs @@ -428,6 +428,67 @@ namespace Microsoft.VisualStudio.Editor.Razor } } + [ForegroundFact] + public async Task ImplicitExpression_AcceptsParenthesisAtEnd_SingleEdit() + { + // Arrange + var factory = new SpanFactory(); + var edit = new TestEdit(8, 0, new StringTextSnapshot("foo @foo bar"), 2, new StringTextSnapshot("foo @foo() bar"), "()"); + + using (var manager = CreateParserManager(edit.OldSnapshot)) + { + await manager.InitializeWithDocumentAsync(edit.OldSnapshot); + + // Apply the () edit + manager.ApplyEdit(edit); + + // Assert + Assert.Equal(1, manager.ParseCount); + ParserTestBase.EvaluateParseTree( + manager.PartialParsingSyntaxTreeRoot, + new MarkupBlock( + factory.Markup("foo "), + new ExpressionBlock( + factory.CodeTransition(), + factory.Code("foo()") + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), + factory.Markup(" bar"))); + } + } + + [ForegroundFact] + public async Task ImplicitExpression_AcceptsParenthesisAtEnd_TwoEdits() + { + // Arrange + var factory = new SpanFactory(); + var edit1 = new TestEdit(8, 0, new StringTextSnapshot("foo @foo bar"), 1, new StringTextSnapshot("foo @foo( bar"), "("); + var edit2 = new TestEdit(9, 0, new StringTextSnapshot("foo @foo( bar"), 1, new StringTextSnapshot("foo @foo() bar"), ")"); + using (var manager = CreateParserManager(edit1.OldSnapshot)) + { + await manager.InitializeWithDocumentAsync(edit1.OldSnapshot); + + // Apply the ( edit + manager.ApplyEdit(edit1); + + // Apply the ) edit + manager.ApplyEdit(edit2); + + // Assert + Assert.Equal(1, manager.ParseCount); + ParserTestBase.EvaluateParseTree( + manager.PartialParsingSyntaxTreeRoot, + new MarkupBlock( + factory.Markup("foo "), + new ExpressionBlock( + factory.CodeTransition(), + factory.Code("foo()") + .AsImplicitExpression(CSharpCodeParser.DefaultKeywords) + .Accepts(AcceptedCharactersInternal.NonWhiteSpace)), + factory.Markup(" bar"))); + } + } + [ForegroundFact] public async Task ImplicitExpressionCorrectlyTriggersReparseIfIfKeywordTyped() {