Make SyntaxTree mutations not leak.

- Prior to this when the partial parser would successfully parse a change we'd mutate the returned syntax tree so any data inquired about the tree would then be wrong. We now isolate mutations to copied versions of the syntax tree.
- Added copy tests to ensure that we were appropriately copying all the various syntax node types.

#1793
This commit is contained in:
N. Taylor Mullen 2017-12-15 14:40:13 -08:00
parent c0eecc87e7
commit 463e11b739
13 changed files with 243 additions and 14 deletions

View File

@ -214,6 +214,20 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
visitor.VisitBlock(this); visitor.VisitBlock(this);
} }
public override SyntaxTreeNode Clone()
{
var blockBuilder = new BlockBuilder(this);
blockBuilder.Children.Clear();
for (var i = 0; i < Children.Count; i++)
{
var clonedChild = Children[i].Clone();
blockBuilder.Children.Add(clonedChild);
}
return blockBuilder.Build();
}
private class EquivalenceComparer : IEqualityComparer<SyntaxTreeNode> private class EquivalenceComparer : IEqualityComparer<SyntaxTreeNode>
{ {
public static readonly EquivalenceComparer Default = new EquivalenceComparer(); public static readonly EquivalenceComparer Default = new EquivalenceComparer();

View File

@ -151,5 +151,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
{ {
visitor.VisitSpan(this); visitor.VisitSpan(this);
} }
public override SyntaxTreeNode Clone()
{
var spanBuilder = new SpanBuilder(this);
return spanBuilder.Build();
}
} }
} }

View File

@ -43,5 +43,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
public abstract int GetEquivalenceHash(); public abstract int GetEquivalenceHash();
public abstract void Accept(ParserVisitor visitor); public abstract void Accept(ParserVisitor visitor);
public abstract SyntaxTreeNode Clone();
} }
} }

View File

@ -159,6 +159,41 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
return null; return null;
} }
public override SyntaxTreeNode Clone()
{
var tagHelperBlockBuilder = new TagHelperBlockBuilder(this);
tagHelperBlockBuilder.Children.Clear();
for (var i = 0; i < Children.Count; i++)
{
var clonedChild = Children[i].Clone();
tagHelperBlockBuilder.Children.Add(clonedChild);
}
tagHelperBlockBuilder.Attributes.Clear();
for (var i = 0; i < Attributes.Count; i++)
{
var existingAttribute = Attributes[i];
var clonedValue = existingAttribute.Value != null ? existingAttribute.Value.Clone() : null;
tagHelperBlockBuilder.Attributes.Add(
new TagHelperAttributeNode(existingAttribute.Name, clonedValue, existingAttribute.AttributeStructure));
}
if (SourceStartTag != null)
{
var clonedStartTag = (Block)SourceStartTag.Clone();
tagHelperBlockBuilder.SourceStartTag = clonedStartTag;
}
if (SourceEndTag != null)
{
var clonedEndTag = (Block)SourceEndTag.Clone();
tagHelperBlockBuilder.SourceEndTag = clonedEndTag;
}
return tagHelperBlockBuilder.Build();
}
/// <inheritdoc /> /// <inheritdoc />
public override string ToString() public override string ToString()
{ {

View File

@ -18,9 +18,12 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
public TagHelperBlockBuilder(TagHelperBlock original) public TagHelperBlockBuilder(TagHelperBlock original)
: base(original) : base(original)
{ {
TagName = original.TagName; SourceStartTag = original.SourceStartTag;
SourceEndTag = original.SourceEndTag;
TagMode = original.TagMode;
BindingResult = original.Binding; BindingResult = original.Binding;
Attributes = new List<TagHelperAttributeNode>(original.Attributes); Attributes = new List<TagHelperAttributeNode>(original.Attributes);
TagName = original.TagName;
} }
/// <summary> /// <summary>

View File

@ -25,6 +25,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
internal Timer _idleTimer; internal Timer _idleTimer;
internal BackgroundParser _parser; internal BackgroundParser _parser;
internal ChangeReference _latestChangeReference; internal ChangeReference _latestChangeReference;
internal RazorSyntaxTreePartialParser _partialParser;
private readonly object IdleLock = new object(); private readonly object IdleLock = new object();
private readonly VisualStudioCompletionBroker _completionBroker; private readonly VisualStudioCompletionBroker _completionBroker;
@ -32,7 +33,6 @@ namespace Microsoft.VisualStudio.Editor.Razor
private readonly ForegroundDispatcher _dispatcher; private readonly ForegroundDispatcher _dispatcher;
private readonly RazorTemplateEngineFactoryService _templateEngineFactory; private readonly RazorTemplateEngineFactoryService _templateEngineFactory;
private readonly ErrorReporter _errorReporter; private readonly ErrorReporter _errorReporter;
private RazorSyntaxTreePartialParser _partialParser;
private RazorTemplateEngine _templateEngine; private RazorTemplateEngine _templateEngine;
private RazorCodeDocument _codeDocument; private RazorCodeDocument _codeDocument;
private ITextSnapshot _snapshot; private ITextSnapshot _snapshot;

View File

@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved. // 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. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using Microsoft.AspNetCore.Razor.Language; using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Legacy; using Microsoft.AspNetCore.Razor.Language.Legacy;
using Span = Microsoft.AspNetCore.Razor.Language.Legacy.Span; using Span = Microsoft.AspNetCore.Razor.Language.Legacy.Span;
@ -9,15 +10,24 @@ namespace Microsoft.VisualStudio.Editor.Razor
{ {
internal class RazorSyntaxTreePartialParser internal class RazorSyntaxTreePartialParser
{ {
private readonly RazorSyntaxTree _syntaxTree;
private Span _lastChangeOwner; private Span _lastChangeOwner;
private bool _lastResultProvisional; private bool _lastResultProvisional;
public RazorSyntaxTreePartialParser(RazorSyntaxTree syntaxTree) public RazorSyntaxTreePartialParser(RazorSyntaxTree syntaxTree)
{ {
_syntaxTree = syntaxTree; if (syntaxTree == null)
{
throw new ArgumentNullException(nameof(syntaxTree));
}
// We mutate the existing syntax tree so we need to clone the one passed in so our mutations don't
// impact external state.
SyntaxTreeRoot = (Block)syntaxTree.Root.Clone();
} }
// Internal for testing
internal Block SyntaxTreeRoot { get; }
public PartialParseResultInternal Parse(SourceChange change) public PartialParseResultInternal Parse(SourceChange change)
{ {
var result = GetPartialParseResult(change); var result = GetPartialParseResult(change);
@ -46,7 +56,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
} }
// Locate the span responsible for this change // Locate the span responsible for this change
_lastChangeOwner = _syntaxTree.Root.LocateOwner(change); _lastChangeOwner = SyntaxTreeRoot.LocateOwner(change);
if (_lastResultProvisional) if (_lastResultProvisional)
{ {

View File

@ -8,6 +8,26 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
{ {
public class BlockTest public class BlockTest
{ {
[Fact]
public void Clone_ClonesBlock()
{
// Arrange
var blockBuilder = new BlockBuilder()
{
ChunkGenerator = new DynamicAttributeBlockChunkGenerator(new LocationTagged<string>("class=\"", SourceLocation.Zero), 0, 0, 0),
Type = BlockKindInternal.Expression,
};
blockBuilder.Children.Add(new SpanBuilder(new SourceLocation(1, 2, 3)).Build());
var block = blockBuilder.Build();
// Act
var copy = (Block)block.Clone();
// Assert
ParserTestBase.EvaluateParseTree(copy, block);
Assert.NotSame(block, copy);
}
[Fact] [Fact]
public void ConstructorWithBlockBuilderSetsParent() public void ConstructorWithBlockBuilderSetsParent()
{ {

View File

@ -0,0 +1,31 @@
// 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 Xunit;
namespace Microsoft.AspNetCore.Razor.Language.Legacy
{
public class SpanTest
{
[Fact]
public void Clone_ClonesSpan()
{
// Arrange
var spanBuilder = new SpanBuilder(new SourceLocation(1, 2, 3))
{
EditHandler = new SpanEditHandler(CSharpLanguageCharacteristics.Instance.TokenizeString),
Kind = SpanKindInternal.Transition,
ChunkGenerator = new ExpressionChunkGenerator(),
};
spanBuilder.Accept(new CSharpSymbol("@", CSharpSymbolType.Transition));
var span = spanBuilder.Build();
// Act
var copy = (Span)span.Clone();
// Assert
Assert.Equal(span, copy);
Assert.NotSame(span, copy);
}
}
}

View File

@ -1,12 +1,114 @@
// Copyright (c) .NET Foundation. All rights reserved. // 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. // 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; using Xunit;
namespace Microsoft.AspNetCore.Razor.Language.Legacy namespace Microsoft.AspNetCore.Razor.Language.Legacy
{ {
public class TagHelperBlockTest public class TagHelperBlockTest
{ {
[Fact]
public void Clone_ClonesTagHelperChildren()
{
// Arrange
var tagHelper = new TagHelperBlockBuilder(
"p",
TagMode.StartTagAndEndTag,
attributes: new List<TagHelperAttributeNode>(),
children: new[]
{
new SpanBuilder(SourceLocation.Zero).Build(),
new SpanBuilder(new SourceLocation(0, 1, 2)).Build(),
}).Build();
// Act
var copy = (TagHelperBlock)tagHelper.Clone();
// Assert
ParserTestBase.EvaluateParseTree(copy, tagHelper);
Assert.Collection(
copy.Children,
child => Assert.NotSame(tagHelper.Children[0], child),
child => Assert.NotSame(tagHelper.Children[1], child));
}
[Fact]
public void Clone_ClonesTagHelperAttributes()
{
// Arrange
var tagHelper = (TagHelperBlock)new TagHelperBlockBuilder(
"p",
TagMode.StartTagAndEndTag,
attributes: new List<TagHelperAttributeNode>()
{
new TagHelperAttributeNode("class", new SpanBuilder(SourceLocation.Zero).Build(), AttributeStructure.NoQuotes),
new TagHelperAttributeNode("checked", new SpanBuilder(SourceLocation.Undefined).Build(), AttributeStructure.NoQuotes)
},
children: Enumerable.Empty<SyntaxTreeNode>()).Build();
// Act
var copy = (TagHelperBlock)tagHelper.Clone();
// Assert
ParserTestBase.EvaluateParseTree(copy, tagHelper);
Assert.Collection(
copy.Attributes,
attribute => Assert.NotSame(tagHelper.Attributes[0], attribute),
attribute => Assert.NotSame(tagHelper.Attributes[1], attribute));
}
[Fact]
public void Clone_ClonesTagHelperSourceStartTag()
{
// Arrange
var tagHelper = (TagHelperBlock)new TagHelperBlockBuilder(
"p",
TagMode.StartTagAndEndTag,
attributes: new List<TagHelperAttributeNode>(),
children: Enumerable.Empty<SyntaxTreeNode>())
{
SourceStartTag = new BlockBuilder()
{
Type = BlockKindInternal.Comment,
ChunkGenerator = new RazorCommentChunkGenerator()
}.Build()
}.Build();
// Act
var copy = (TagHelperBlock)tagHelper.Clone();
// Assert
ParserTestBase.EvaluateParseTree(copy, tagHelper);
Assert.NotSame(tagHelper.SourceStartTag, copy.SourceStartTag);
}
[Fact]
public void Clone_ClonesTagHelperSourceEndTag()
{
// Arrange
var tagHelper = (TagHelperBlock)new TagHelperBlockBuilder(
"p",
TagMode.StartTagAndEndTag,
attributes: new List<TagHelperAttributeNode>(),
children: Enumerable.Empty<SyntaxTreeNode>())
{
SourceEndTag = new BlockBuilder()
{
Type = BlockKindInternal.Comment,
ChunkGenerator = new RazorCommentChunkGenerator()
}.Build()
}.Build();
// Act
var copy = (TagHelperBlock)tagHelper.Clone();
// Assert
ParserTestBase.EvaluateParseTree(copy, tagHelper);
Assert.NotSame(tagHelper.SourceEndTag, copy.SourceEndTag);
}
[Fact] [Fact]
public void FlattenFlattensSelfClosingTagHelpers() public void FlattenFlattensSelfClosingTagHelpers()
{ {

View File

@ -97,7 +97,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
{ {
manager.ApplyEdit(testEdit); manager.ApplyEdit(testEdit);
Assert.Equal(1, manager.ParseCount); Assert.Equal(1, manager.ParseCount);
ParserTestBase.EvaluateParseTree(manager.CurrentSyntaxTree.Root, new MarkupBlock( ParserTestBase.EvaluateParseTree(manager.PartialParsingSyntaxTreeRoot, new MarkupBlock(
factory.EmptyHtml(), factory.EmptyHtml(),
new StatementBlock( new StatementBlock(
factory.CodeTransition(), factory.CodeTransition(),
@ -156,7 +156,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
{ {
manager.ApplyEdit(testEdit); manager.ApplyEdit(testEdit);
Assert.Equal(1, manager.ParseCount); Assert.Equal(1, manager.ParseCount);
ParserTestBase.EvaluateParseTree(manager.CurrentSyntaxTree.Root, new MarkupBlock( ParserTestBase.EvaluateParseTree(manager.PartialParsingSyntaxTreeRoot, new MarkupBlock(
factory.EmptyHtml(), factory.EmptyHtml(),
new StatementBlock( new StatementBlock(
factory.CodeTransition(), factory.CodeTransition(),
@ -203,7 +203,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
manager.ApplyEdit(testEdit); manager.ApplyEdit(testEdit);
Assert.Equal(1, manager.ParseCount); Assert.Equal(1, manager.ParseCount);
ParserTestBase.EvaluateParseTree(manager.CurrentSyntaxTree.Root, new MarkupBlock( ParserTestBase.EvaluateParseTree(manager.PartialParsingSyntaxTreeRoot, new MarkupBlock(
factory.Markup("foo "), factory.Markup("foo "),
new ExpressionBlock( new ExpressionBlock(
factory.CodeTransition(), factory.CodeTransition(),
@ -249,7 +249,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
manager.ApplyEdit(testEdit); manager.ApplyEdit(testEdit);
Assert.Equal(1, manager.ParseCount); Assert.Equal(1, manager.ParseCount);
ParserTestBase.EvaluateParseTree(manager.CurrentSyntaxTree.Root, new MarkupBlock( ParserTestBase.EvaluateParseTree(manager.PartialParsingSyntaxTreeRoot, new MarkupBlock(
factory.Markup("foo "), factory.Markup("foo "),
new ExpressionBlock( new ExpressionBlock(
factory.CodeTransition(), factory.CodeTransition(),
@ -301,7 +301,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
applyEdit(); applyEdit();
Assert.Equal(1, manager.ParseCount); Assert.Equal(1, manager.ParseCount);
ParserTestBase.EvaluateParseTree(manager.CurrentSyntaxTree.Root, new MarkupBlock( ParserTestBase.EvaluateParseTree(manager.PartialParsingSyntaxTreeRoot, new MarkupBlock(
factory.Markup("foo "), factory.Markup("foo "),
new ExpressionBlock( new ExpressionBlock(
factory.CodeTransition(), factory.CodeTransition(),
@ -369,7 +369,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
// Assert // Assert
Assert.Equal(2, manager.ParseCount); Assert.Equal(2, manager.ParseCount);
ParserTestBase.EvaluateParseTree(manager.CurrentSyntaxTree.Root, ParserTestBase.EvaluateParseTree(manager.PartialParsingSyntaxTreeRoot,
new MarkupBlock( new MarkupBlock(
factory.Markup("foo "), factory.Markup("foo "),
new ExpressionBlock( new ExpressionBlock(
@ -406,7 +406,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
// Assert // Assert
Assert.Equal(1, manager.ParseCount); Assert.Equal(1, manager.ParseCount);
ParserTestBase.EvaluateParseTree(manager.CurrentSyntaxTree.Root, ParserTestBase.EvaluateParseTree(manager.PartialParsingSyntaxTreeRoot,
new MarkupBlock( new MarkupBlock(
factory.Markup("foo "), factory.Markup("foo "),
new ExpressionBlock( new ExpressionBlock(
@ -642,6 +642,8 @@ namespace Microsoft.VisualStudio.Editor.Razor
public RazorSyntaxTree CurrentSyntaxTree { get; private set; } public RazorSyntaxTree CurrentSyntaxTree { get; private set; }
public Block PartialParsingSyntaxTreeRoot => _parser._partialParser.SyntaxTreeRoot;
public async Task InitializeWithDocumentAsync(ITextSnapshot snapshot) public async Task InitializeWithDocumentAsync(ITextSnapshot snapshot)
{ {
var old = new StringTextSnapshot(string.Empty); var old = new StringTextSnapshot(string.Empty);

View File

@ -117,10 +117,12 @@ namespace Microsoft.VisualStudio.Editor.Razor
var latestChange = new SourceChange(0, 0, string.Empty); var latestChange = new SourceChange(0, 0, string.Empty);
var latestSnapshot = documentTracker.TextBuffer.CurrentSnapshot; var latestSnapshot = documentTracker.TextBuffer.CurrentSnapshot;
parser._latestChangeReference = new DefaultVisualStudioRazorParser.ChangeReference(latestChange, latestSnapshot); parser._latestChangeReference = new DefaultVisualStudioRazorParser.ChangeReference(latestChange, latestSnapshot);
var codeDocument = TestRazorCodeDocument.CreateEmpty();
codeDocument.SetSyntaxTree(RazorSyntaxTree.Parse(TestRazorSourceDocument.Create()));
var args = new DocumentStructureChangedEventArgs( var args = new DocumentStructureChangedEventArgs(
latestChange, latestChange,
latestSnapshot, latestSnapshot,
TestRazorCodeDocument.CreateEmpty()); codeDocument);
// Act // Act
parser.OnDocumentStructureChanged(args); parser.OnDocumentStructureChanged(args);

View File

@ -3,12 +3,14 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNetCore.Mvc.Razor.Extensions; using Microsoft.AspNetCore.Mvc.Razor.Extensions;
using Microsoft.AspNetCore.Razor.Language; using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Legacy; using Microsoft.AspNetCore.Razor.Language.Legacy;
using Microsoft.VisualStudio.Test; using Microsoft.VisualStudio.Test;
using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text;
using Xunit; using Xunit;
using Span = Microsoft.AspNetCore.Razor.Language.Legacy.Span;
namespace Microsoft.VisualStudio.Editor.Razor namespace Microsoft.VisualStudio.Editor.Razor
{ {
@ -566,7 +568,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
var result = parser.Parse(edit.Change); var result = parser.Parse(edit.Change);
Assert.Equal(PartialParseResultInternal.Accepted | additionalFlags, result); Assert.Equal(PartialParseResultInternal.Accepted | additionalFlags, result);
ParserTestBase.EvaluateParseTree(expectedTree, syntaxTree.Root); ParserTestBase.EvaluateParseTree(parser.SyntaxTreeRoot, expectedTree);
} }
private static TestEdit CreateInsertionChange(string initialText, int insertionLocation, string insertionText) private static TestEdit CreateInsertionChange(string initialText, int insertionLocation, string insertionText)