Skip to content

Commit 90b1855

Browse files
DustinCampbellDustinCampbell
authored
Change RazorSyntaxTree.Diagnostics from an IReadOnlyList<RazorDiagnostic> to an ImmutableArray<RazorDiagnostic> (#10797)
This pull request represents several changes with the ultimate goal of exposing `RazorSyntaxTree.Diagnostics` as an `ImmutableArray<RazorDiagnostic>` rather than an `IReadOnlyList<RazorDiagnostic>`: - Clean up `RazorSyntaxTree` and get rid of `DefaultRazorSyntaxTree`. - Add `(Drain)ToImmutableOrdered*` methods to `PooledArrayBuilder<T>`. Note that this change also includes a refactoring to the various unit tests for ordering to share test data that I've isolated to a single commit. - Clean up and improve `ErrorSink` to no longer greedily create a new `List<T>` before any errors are encountered. - Clean up `ParserContext` and make it used pooled collections. - Use pooled collections when computing and caching the result of `RazorSyntaxTree.Diagnostics`.
2 parents 4ec65fb + 5c0677a commit 90b1855

File tree

50 files changed

+2122
-1674
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+2122
-1674
lines changed

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/legacyTest/Legacy/TagHelperParseTreeRewriterTest.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public void GetAttributeNameValuePairs_ParsesPairsCorrectly(
5252
IEnumerable<KeyValuePair<string, string>> expectedPairs)
5353
{
5454
// Arrange
55-
var errorSink = new ErrorSink();
55+
using var errorSink = new ErrorSink();
5656
var parseResult = ParseDocument(documentContent);
5757
var document = parseResult.Root;
5858

@@ -61,7 +61,7 @@ public void GetAttributeNameValuePairs_ParsesPairsCorrectly(
6161
var rootMarkup = Assert.IsType<MarkupBlockSyntax>(rootBlock.Document);
6262
var childBlock = Assert.Single(rootMarkup.Children);
6363
var element = Assert.IsType<MarkupElementSyntax>(childBlock);
64-
Assert.Empty(errorSink.Errors);
64+
Assert.Empty(errorSink.GetErrorsAndClear());
6565

6666
// Act
6767
var pairs = TagHelperParseTreeRewriter.Rewriter.GetAttributeNameValuePairs(element.StartTag);

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/legacyTest/Legacy/WhiteSpaceRewriterTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public void Moves_Whitespace_Preceeding_ExpressionBlock_To_Parent_Block()
3232
var rewritten = rewriter.Visit(parsed.Root);
3333

3434
// Assert
35-
var rewrittenTree = RazorSyntaxTree.Create(rewritten, parsed.Source, parsed.Diagnostics, parsed.Options);
35+
var rewrittenTree = new RazorSyntaxTree(rewritten, parsed.Source, parsed.Diagnostics, parsed.Options);
3636
BaselineTest(rewrittenTree);
3737
}
3838
}

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ public void Execute_CombinesErrorsOnRewritingErrors()
604604
var expectedRewritingError = RazorDiagnosticFactory.CreateParsing_TagHelperFoundMalformedTagHelper(
605605
new SourceSpan(new SourceLocation((Environment.NewLine.Length * 2) + 30, 2, 1), contentLength: 4), "form");
606606

607-
var erroredOriginalTree = RazorSyntaxTree.Create(originalTree.Root, originalTree.Source, [initialError], originalTree.Options);
607+
var erroredOriginalTree = new RazorSyntaxTree(originalTree.Root, originalTree.Source, [initialError], originalTree.Options);
608608
codeDocument.SetSyntaxTree(erroredOriginalTree);
609609

610610
// Act
@@ -615,7 +615,7 @@ public void Execute_CombinesErrorsOnRewritingErrors()
615615
var outputTree = codeDocument.GetSyntaxTree();
616616
Assert.Empty(originalTree.Diagnostics);
617617
Assert.NotSame(erroredOriginalTree, outputTree);
618-
Assert.Equal([initialError, expectedRewritingError], outputTree.Diagnostics);
618+
Assert.Equal<RazorDiagnostic>([initialError, expectedRewritingError], outputTree.Diagnostics);
619619
}
620620

621621
private static string AssemblyA => "TestAssembly";

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/Legacy/CSharpCodeParserTest.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,15 @@ public void MapDirectives_HandlesDuplicates()
215215
// Arrange
216216
var source = TestRazorSourceDocument.Create();
217217
var options = RazorParserOptions.CreateDefault();
218-
var context = new ParserContext(source, options);
218+
using var context = new ParserContext(source, options);
219219

220220
// Act & Assert (Does not throw)
221-
var directiveDescriptors = new[] {
221+
var directiveDescriptors = new[]
222+
{
222223
DirectiveDescriptor.CreateDirective("test", DirectiveKind.SingleLine),
223224
DirectiveDescriptor.CreateDirective("test", DirectiveKind.SingleLine),
224225
};
226+
225227
_ = new CSharpCodeParser(directiveDescriptors, context);
226228
}
227229
}

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/Legacy/HtmlMarkupParserTests.cs‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public void IsHyphen_ReturnsTrueForADashToken()
5353
public void AcceptAllButLastDoubleHypens_ReturnsTheOnlyDoubleHyphenToken()
5454
{
5555
// Arrange
56-
var sut = CreateTestParserForContent("-->");
56+
using var sut = CreateTestParserForContent("-->");
5757

5858
// Act
5959
var token = sut.AcceptAllButLastDoubleHyphens();
@@ -68,7 +68,7 @@ public void AcceptAllButLastDoubleHypens_ReturnsTheOnlyDoubleHyphenToken()
6868
public void AcceptAllButLastDoubleHypens_ReturnsTheDoubleHyphenTokenAfterAcceptingTheDash()
6969
{
7070
// Arrange
71-
var sut = CreateTestParserForContent("--->");
71+
using var sut = CreateTestParserForContent("--->");
7272

7373
// Act
7474
var token = sut.AcceptAllButLastDoubleHyphens();
@@ -83,7 +83,7 @@ public void AcceptAllButLastDoubleHypens_ReturnsTheDoubleHyphenTokenAfterAccepti
8383
public void IsHtmlCommentAhead_ReturnsTrueForEmptyCommentTag()
8484
{
8585
// Arrange
86-
var sut = CreateTestParserForContent("<!---->");
86+
using var sut = CreateTestParserForContent("<!---->");
8787

8888
// Act & Assert
8989
Assert.True(sut.IsHtmlCommentAhead());
@@ -93,7 +93,7 @@ public void IsHtmlCommentAhead_ReturnsTrueForEmptyCommentTag()
9393
public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTag()
9494
{
9595
// Arrange
96-
var sut = CreateTestParserForContent("<!-- Some comment content in here -->");
96+
using var sut = CreateTestParserForContent("<!-- Some comment content in here -->");
9797

9898
// Act & Assert
9999
Assert.True(sut.IsHtmlCommentAhead());
@@ -103,7 +103,7 @@ public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTag()
103103
public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraDashesAtClosingTag()
104104
{
105105
// Arrange
106-
var sut = CreateTestParserForContent("<!-- Some comment content in here ----->");
106+
using var sut = CreateTestParserForContent("<!-- Some comment content in here ----->");
107107

108108
// Act & Assert
109109
Assert.True(sut.IsHtmlCommentAhead());
@@ -113,7 +113,7 @@ public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraDashesAtClo
113113
public void IsHtmlCommentAhead_ReturnsFalseForContentWithBadEndingAndExtraDash()
114114
{
115115
// Arrange
116-
var sut = CreateTestParserForContent("<!-- Some comment content in here <!--->");
116+
using var sut = CreateTestParserForContent("<!-- Some comment content in here <!--->");
117117

118118
// Act & Assert
119119
Assert.False(sut.IsHtmlCommentAhead());
@@ -123,7 +123,7 @@ public void IsHtmlCommentAhead_ReturnsFalseForContentWithBadEndingAndExtraDash()
123123
public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraInfoAfter()
124124
{
125125
// Arrange
126-
var sut = CreateTestParserForContent("<!-- comment --> the first part is a valid comment without the Open angle and bang tokens");
126+
using var sut = CreateTestParserForContent("<!-- comment --> the first part is a valid comment without the Open angle and bang tokens");
127127

128128
// Act & Assert
129129
Assert.True(sut.IsHtmlCommentAhead());
@@ -133,7 +133,7 @@ public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraInfoAfter()
133133
public void IsHtmlCommentAhead_ReturnsFalseForNotClosedComment()
134134
{
135135
// Arrange
136-
var sut = CreateTestParserForContent("<!-- not closed comment");
136+
using var sut = CreateTestParserForContent("<!-- not closed comment");
137137

138138
// Act & Assert
139139
Assert.False(sut.IsHtmlCommentAhead());
@@ -143,7 +143,7 @@ public void IsHtmlCommentAhead_ReturnsFalseForNotClosedComment()
143143
public void IsHtmlCommentAhead_ReturnsFalseForCommentWithoutLastClosingAngle()
144144
{
145145
// Arrange
146-
var sut = CreateTestParserForContent("<!-- not closed comment--");
146+
using var sut = CreateTestParserForContent("<!-- not closed comment--");
147147

148148
// Act & Assert
149149
Assert.False(sut.IsHtmlCommentAhead());
@@ -153,7 +153,7 @@ public void IsHtmlCommentAhead_ReturnsFalseForCommentWithoutLastClosingAngle()
153153
public void IsHtmlCommentAhead_ReturnsTrueForCommentWithCodeInside()
154154
{
155155
// Arrange
156-
var sut = CreateTestParserForContent("<!-- not closed @DateTime.Now comment-->");
156+
using var sut = CreateTestParserForContent("<!-- not closed @DateTime.Now comment-->");
157157

158158
// Act & Assert
159159
Assert.True(sut.IsHtmlCommentAhead());
@@ -195,8 +195,19 @@ public void IsCommentContentEndingInvalid_ReturnsFalseForEmptyContent()
195195
Assert.False(HtmlMarkupParser.IsCommentContentEndingInvalid(sequence));
196196
}
197197

198-
private class TestHtmlMarkupParser : HtmlMarkupParser
198+
private class TestHtmlMarkupParser : HtmlMarkupParser, IDisposable
199199
{
200+
public TestHtmlMarkupParser(ParserContext context)
201+
: base(context)
202+
{
203+
EnsureCurrent();
204+
}
205+
206+
public void Dispose()
207+
{
208+
Context.Dispose();
209+
}
210+
200211
public new SyntaxToken PreviousToken
201212
{
202213
get => base.PreviousToken;
@@ -207,11 +218,6 @@ private class TestHtmlMarkupParser : HtmlMarkupParser
207218
return base.IsHtmlCommentAhead();
208219
}
209220

210-
public TestHtmlMarkupParser(ParserContext context) : base(context)
211-
{
212-
EnsureCurrent();
213-
}
214-
215221
public new SyntaxToken AcceptAllButLastDoubleHyphens()
216222
{
217223
return base.AcceptAllButLastDoubleHyphens();

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/Legacy/TagHelperParseTreeRewriterTest.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public void GetAttributeNameValuePairs_ParsesPairsCorrectly(
5252
IEnumerable<KeyValuePair<string, string>> expectedPairs)
5353
{
5454
// Arrange
55-
var errorSink = new ErrorSink();
55+
using var errorSink = new ErrorSink();
5656
var parseResult = ParseDocument(documentContent);
5757
var document = parseResult.Root;
5858

@@ -61,7 +61,7 @@ public void GetAttributeNameValuePairs_ParsesPairsCorrectly(
6161
var rootMarkup = Assert.IsType<MarkupBlockSyntax>(rootBlock.Document);
6262
var childBlock = Assert.Single(rootMarkup.Children);
6363
var element = Assert.IsType<MarkupElementSyntax>(childBlock);
64-
Assert.Empty(errorSink.Errors);
64+
Assert.Empty(errorSink.GetErrorsAndClear());
6565

6666
// Act
6767
var pairs = TagHelperParseTreeRewriter.Rewriter.GetAttributeNameValuePairs(element.StartTag);

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/Legacy/TokenizerLookaheadTest.cs‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public void Lookahead_MaintainsExistingBufferWhenSuccessfulAndTakeIfMatchIsFalse
6363
public void LookaheadUntil_PassesThePreviousTokensInTheSameOrder()
6464
{
6565
// Arrange
66-
var tokenizer = CreateContentTokenizer("asdf--fvd--<");
66+
using var tokenizer = CreateContentTokenizer("asdf--fvd--<");
6767

6868
// Act
6969
var i = 3;
@@ -89,7 +89,7 @@ public void LookaheadUntil_PassesThePreviousTokensInTheSameOrder()
8989
public void LookaheadUntil_ReturnsFalseAfterIteratingOverAllTokensIfConditionIsNotMet()
9090
{
9191
// Arrange
92-
var tokenizer = CreateContentTokenizer("asdf--fvd");
92+
using var tokenizer = CreateContentTokenizer("asdf--fvd");
9393

9494
// Act
9595
var tokens = new Stack<SyntaxToken>();
@@ -111,7 +111,7 @@ public void LookaheadUntil_ReturnsFalseAfterIteratingOverAllTokensIfConditionIsN
111111
public void LookaheadUntil_ReturnsTrueAndBreaksIteration()
112112
{
113113
// Arrange
114-
var tokenizer = CreateContentTokenizer("asdf--fvd");
114+
using var tokenizer = CreateContentTokenizer("asdf--fvd");
115115

116116
// Act
117117
var tokens = new Stack<SyntaxToken>();
@@ -134,8 +134,7 @@ private static TestTokenizerBackedParser CreateContentTokenizer(string content)
134134
var options = RazorParserOptions.CreateDefault();
135135
var context = new ParserContext(source, options);
136136

137-
var tokenizer = new TestTokenizerBackedParser(HtmlLanguageCharacteristics.Instance, context);
138-
return tokenizer;
137+
return new TestTokenizerBackedParser(HtmlLanguageCharacteristics.Instance, context);
139138
}
140139

141140
private static void AssertTokenEqual(SyntaxToken expected, SyntaxToken actual)
@@ -204,12 +203,18 @@ protected override StateResult Dispatch()
204203
}
205204
}
206205

207-
private class TestTokenizerBackedParser : TokenizerBackedParser<HtmlTokenizer>
206+
private class TestTokenizerBackedParser : TokenizerBackedParser<HtmlTokenizer>, IDisposable
208207
{
209-
internal TestTokenizerBackedParser(LanguageCharacteristics<HtmlTokenizer> language, ParserContext context) : base(language, context)
208+
internal TestTokenizerBackedParser(LanguageCharacteristics<HtmlTokenizer> language, ParserContext context)
209+
: base(language, context)
210210
{
211211
}
212212

213+
public void Dispose()
214+
{
215+
Context.Dispose();
216+
}
217+
213218
internal new bool LookaheadUntil(Func<SyntaxToken, IEnumerable<SyntaxToken>, bool> condition)
214219
{
215220
return base.LookaheadUntil(condition);

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/Legacy/WhiteSpaceRewriterTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public void Moves_Whitespace_Preceeding_ExpressionBlock_To_Parent_Block()
3232
var rewritten = rewriter.Visit(parsed.Root);
3333

3434
// Assert
35-
var rewrittenTree = RazorSyntaxTree.Create(rewritten, parsed.Source, parsed.Diagnostics, parsed.Options);
35+
var rewrittenTree = new RazorSyntaxTree(rewritten, parsed.Source, parsed.Diagnostics, parsed.Options);
3636
BaselineTest(rewrittenTree);
3737
}
3838
}

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultDirectiveSyntaxTreePass.cs‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
#nullable disable
5-
64
using System;
7-
using System.Collections.Generic;
5+
using System.Collections.Immutable;
86
using Microsoft.AspNetCore.Razor.Language.Extensions;
97
using Microsoft.AspNetCore.Razor.Language.Syntax;
108

@@ -26,23 +24,19 @@ public RazorSyntaxTree Execute(RazorCodeDocument codeDocument, RazorSyntaxTree s
2624
return sectionVerifier.Verify();
2725
}
2826

29-
private class NestedSectionVerifier : SyntaxRewriter
27+
private sealed class NestedSectionVerifier(RazorSyntaxTree syntaxTree) : SyntaxRewriter
3028
{
31-
private int _nestedLevel;
32-
private readonly RazorSyntaxTree _syntaxTree;
33-
private readonly List<RazorDiagnostic> _diagnostics;
29+
private readonly RazorSyntaxTree _syntaxTree = syntaxTree;
3430

35-
public NestedSectionVerifier(RazorSyntaxTree syntaxTree)
36-
{
37-
_syntaxTree = syntaxTree;
38-
_diagnostics = new List<RazorDiagnostic>(syntaxTree.Diagnostics);
39-
}
31+
private ImmutableArray<RazorDiagnostic>.Builder? _diagnostics;
32+
private int _nestedLevel;
4033

4134
public RazorSyntaxTree Verify()
4235
{
4336
var root = Visit(_syntaxTree.Root);
44-
var rewrittenTree = new DefaultRazorSyntaxTree(root, _syntaxTree.Source, _diagnostics, _syntaxTree.Options);
45-
return rewrittenTree;
37+
var diagnostics = _diagnostics?.DrainToImmutable() ?? _syntaxTree.Diagnostics;
38+
39+
return new RazorSyntaxTree(root, _syntaxTree.Source, diagnostics, _syntaxTree.Options);
4640
}
4741

4842
public override SyntaxNode Visit(SyntaxNode node)
@@ -55,6 +49,7 @@ public override SyntaxNode Visit(SyntaxNode node)
5549
{
5650
// We're very close to reaching the stack limit. Let's not go any deeper.
5751
// It's okay to not show nested section errors in deeply nested cases instead of crashing.
52+
_diagnostics ??= _syntaxTree.Diagnostics.ToBuilder();
5853
_diagnostics.Add(RazorDiagnosticFactory.CreateRewriter_InsufficientStack());
5954

6055
return node;

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorIntermediateNodeLoweringPhase.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,18 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument)
130130

131131
// The document should contain all errors that currently exist in the system. This involves
132132
// adding the errors from the primary and imported syntax trees.
133-
for (var i = 0; i < syntaxTree.Diagnostics.Count; i++)
133+
foreach (var diagnostic in syntaxTree.Diagnostics)
134134
{
135-
document.Diagnostics.Add(syntaxTree.Diagnostics[i]);
135+
document.Diagnostics.Add(diagnostic);
136136
}
137137

138138
if (imports is { IsDefault: false } importsArray)
139139
{
140140
foreach (var import in importsArray)
141141
{
142-
for (var j = 0; j < import.Diagnostics.Count; j++)
142+
foreach (var diagnostic in import.Diagnostics)
143143
{
144-
document.Diagnostics.Add(import.Diagnostics[j]);
144+
document.Diagnostics.Add(diagnostic);
145145
}
146146
}
147147
}

0 commit comments

Comments
 (0)