Skip to content

Commit

Permalink
Remove ItemCollection from CodeRenderingContext (#10764)
Browse files Browse the repository at this point in the history
Because of a dare from @333fred, I'm currently on a crusade to remove
`ItemCollection` from the Razor Compiler completely. Previously, I
removed `ItemCollection` from `TagHelperDescriptorProviderContext`
(#10720). This time, I'm removing it from `CodeRenderingContext`.

It turns out that every `CodeRenderingContext` greedily creates an
`ItemCollection`, though there are only ever two things stored there:

1. A string to use for new lines in `CodeWriter`.
2. A string representing "suppress IDs", which is already specified in
`RazorCodeGenerationOptions`.

These two items were really present as a hook that compiler tests could
set. However, it's much easier and less wasteful to elevate both items
to `RazorCodeGenerationOptions` and make tests set the options directly.

I made a few other refactorings as part of this change:

- I merged several abstract base classes with their single default
concrete type:
  - `CodeRenderingContext` and `DefaultCodeRenderingContext`
  - `RazorCSharpDocument` and `DefaultRazorCSharpDocument`,
  - `RazorCodeGenerationOptions` and `DefaultRazorCodeGenerationOptions`
- `RazorCodeGenerationOptionsBuilder` and
`DefaultRazorCodeGenerationOptionsBuilder`.
- Reworked `RazorCodeGenerationOptions` and introduced
`RazorCodeGenerationOptionsFlags` to track its boolean fields.
- Cleaned up `RazorCSharpDocument` and unified its collections on
`ImmutableArray<>` rather than `IReadOnlyList<>`.
- Enabled nullability annotations for several types.
- Used more pooled collections in `CodeRenderingContext`.

Note: I was careful with my commit history, and it should be clean
enough to review commit-by-commit if that's your preference.
  • Loading branch information
DustinCampbell authored Aug 22, 2024
2 parents fc8332f + 934fea8 commit 3423142
Show file tree
Hide file tree
Showing 60 changed files with 1,538 additions and 1,256 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public class MyService<TModel>
AssertSourceMappingsMatchBaseline(compiled.CodeDocument);

// We expect this test to generate a bunch of errors.
Assert.True(compiled.CodeDocument.GetCSharpDocument().Diagnostics.Count > 0);
Assert.NotEmpty(compiled.CodeDocument.GetCSharpDocument().Diagnostics);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public void Execute_NoOps_IfNamespaceNodeIsMissing()
// Arrange
var irDocument = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};

var pass = new AssemblyAttributeInjectionPass
Expand All @@ -40,7 +40,7 @@ public void Execute_NoOps_IfNamespaceNodeHasEmptyContent()
// Arrange
var irDocument = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};
var builder = IntermediateNodeBuilder.Create(irDocument);
var @namespace = new NamespaceDeclarationIntermediateNode() { Content = string.Empty };
Expand All @@ -66,7 +66,7 @@ public void Execute_NoOps_IfClassNameNodeIsMissing()
// Arrange
var irDocument = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};

var builder = IntermediateNodeBuilder.Create(irDocument);
Expand All @@ -93,7 +93,7 @@ public void Execute_NoOps_IfClassNameIsEmpty()
// Arrange
var irDocument = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};
var builder = IntermediateNodeBuilder.Create(irDocument);
var @namespace = new NamespaceDeclarationIntermediateNode
Expand Down Expand Up @@ -134,7 +134,7 @@ public void Execute_NoOps_IfDocumentIsNotViewOrPage()
var irDocument = new DocumentIntermediateNode
{
DocumentKind = "Default",
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};
var builder = IntermediateNodeBuilder.Create(irDocument);
var @namespace = new NamespaceDeclarationIntermediateNode() { Content = "SomeNamespace" };
Expand Down Expand Up @@ -170,7 +170,7 @@ public void Execute_NoOps_ForDesignTime()
var irDocument = new DocumentIntermediateNode
{
DocumentKind = MvcViewDocumentClassifierPass.MvcViewDocumentKind,
Options = RazorCodeGenerationOptions.CreateDesignTimeDefault(),
Options = RazorCodeGenerationOptions.DesignTimeDefault,
};
var builder = IntermediateNodeBuilder.Create(irDocument);
var @namespace = new NamespaceDeclarationIntermediateNode
Expand Down Expand Up @@ -217,7 +217,7 @@ public void Execute_AddsRazorViewAttribute_ToViews()
var irDocument = new DocumentIntermediateNode
{
DocumentKind = MvcViewDocumentClassifierPass.MvcViewDocumentKind,
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};
var builder = IntermediateNodeBuilder.Create(irDocument);
var @namespace = new NamespaceDeclarationIntermediateNode
Expand Down Expand Up @@ -270,7 +270,7 @@ public void Execute_EscapesViewPathWhenAddingAttributeToViews()
var irDocument = new DocumentIntermediateNode
{
DocumentKind = MvcViewDocumentClassifierPass.MvcViewDocumentKind,
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};
var builder = IntermediateNodeBuilder.Create(irDocument);
var @namespace = new NamespaceDeclarationIntermediateNode
Expand Down Expand Up @@ -323,7 +323,7 @@ public void Execute_AddsRazorPagettribute_ToPage()
var irDocument = new DocumentIntermediateNode
{
DocumentKind = RazorPageDocumentClassifierPass.RazorPageDocumentKind,
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};
var builder = IntermediateNodeBuilder.Create(irDocument);
var pageDirective = new DirectiveIntermediateNode
Expand Down Expand Up @@ -383,7 +383,7 @@ public void Execute_EscapesViewPathAndRouteWhenAddingAttributeToPage()
var irDocument = new DocumentIntermediateNode
{
DocumentKind = MvcViewDocumentClassifierPass.MvcViewDocumentKind,
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};
var builder = IntermediateNodeBuilder.Create(irDocument);
var @namespace = new NamespaceDeclarationIntermediateNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void InstrumentationPass_NoOps_ForDesignTime()
// Arrange
var document = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDesignTimeDefault(),
Options = RazorCodeGenerationOptions.DesignTimeDefault,
};

var builder = IntermediateNodeBuilder.Create(document);
Expand Down Expand Up @@ -50,7 +50,7 @@ public void InstrumentationPass_InstrumentsHtml()
// Arrange
var document = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};

var builder = IntermediateNodeBuilder.Create(document);
Expand Down Expand Up @@ -89,7 +89,7 @@ public void InstrumentationPass_SkipsHtml_WithoutLocation()
// Arrange
var document = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};

var builder = IntermediateNodeBuilder.Create(document);
Expand Down Expand Up @@ -121,7 +121,7 @@ public void InstrumentationPass_InstrumentsCSharpExpression()
// Arrange
var document = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};

var builder = IntermediateNodeBuilder.Create(document);
Expand Down Expand Up @@ -157,7 +157,7 @@ public void InstrumentationPass_SkipsCSharpExpression_WithoutLocation()
// Arrange
var document = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};

var builder = IntermediateNodeBuilder.Create(document);
Expand Down Expand Up @@ -188,7 +188,7 @@ public void InstrumentationPass_SkipsCSharpExpression_InsideTagHelperAttribute()
// Arrange
var document = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};

var builder = IntermediateNodeBuilder.Create(document);
Expand Down Expand Up @@ -239,7 +239,7 @@ public void InstrumentationPass_SkipsCSharpExpression_InsideTagHelperProperty()
// Arrange
var document = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};

var builder = IntermediateNodeBuilder.Create(document);
Expand Down Expand Up @@ -290,7 +290,7 @@ public void InstrumentationPass_InstrumentsTagHelper()
// Arrange
var document = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};

var builder = IntermediateNodeBuilder.Create(document);
Expand Down Expand Up @@ -321,7 +321,7 @@ public void InstrumentationPass_SkipsTagHelper_WithoutLocation()
// Arrange
var document = new DocumentIntermediateNode()
{
Options = RazorCodeGenerationOptions.CreateDefault(),
Options = RazorCodeGenerationOptions.Default,
};

var builder = IntermediateNodeBuilder.Create(document);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public class MyService<TModel>
AssertCSharpDocumentMatchesBaseline(compiled.CodeDocument.GetCSharpDocument());

// We expect this test to generate a bunch of errors.
Assert.True(compiled.CodeDocument.GetCSharpDocument().Diagnostics.Count > 0);
Assert.NotEmpty(compiled.CodeDocument.GetCSharpDocument().Diagnostics);
}

[Fact]
Expand Down Expand Up @@ -536,7 +536,7 @@ public class MyService<TModel>
AssertSourceMappingsMatchBaseline(compiled.CodeDocument);

// We expect this test to generate a bunch of errors.
Assert.True(compiled.CodeDocument.GetCSharpDocument().Diagnostics.Count > 0);
Assert.NotEmpty(compiled.CodeDocument.GetCSharpDocument().Diagnostics);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public class MyService<TModel>
AssertLinePragmas(compiled.CodeDocument, designTime: false);

// We expect this test to generate a bunch of errors.
Assert.True(compiled.CodeDocument.GetCSharpDocument().Diagnostics.Count > 0);
Assert.NotEmpty(compiled.CodeDocument.GetCSharpDocument().Diagnostics);
}

[Fact]
Expand Down Expand Up @@ -949,7 +949,7 @@ public class MyService<TModel>
AssertSourceMappingsMatchBaseline(compiled.CodeDocument);

// We expect this test to generate a bunch of errors.
Assert.True(compiled.CodeDocument.GetCSharpDocument().Diagnostics.Count > 0);
Assert.NotEmpty(compiled.CodeDocument.GetCSharpDocument().Diagnostics);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ public void CSharpCodeWriter_RespectTabSetting()
o.IndentSize = 4;
});

using var writer = new CodeWriter(Environment.NewLine, options);
using var writer = new CodeWriter(options);

// Act
writer.BuildClassDeclaration(Array.Empty<string>(), "C", "", Array.Empty<string>(), Array.Empty<TypeParameter>(), context: null);
Expand All @@ -428,7 +428,7 @@ public void CSharpCodeWriter_RespectSpaceSetting()
o.IndentSize = 4;
});

using var writer = new CodeWriter(Environment.NewLine, options);
using var writer = new CodeWriter(options);

// Act
writer.BuildClassDeclaration(Array.Empty<string>(), "C", "", Array.Empty<string>(), Array.Empty<TypeParameter>(), context: null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public void CreateDefault_CreatesDefaultCodeTarget()
{
// Arrange
var codeDocument = TestRazorCodeDocument.CreateEmpty();
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

// Act
var target = CodeTarget.CreateDefault(codeDocument, options);
Expand All @@ -32,7 +32,7 @@ public void CreateDefault_CallsDelegate()
Action<CodeTargetBuilder> @delegate = (b) => { wasCalled = true; };

var codeDocument = TestRazorCodeDocument.CreateEmpty();
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

// Act
CodeTarget.CreateDefault(codeDocument, options, @delegate);
Expand All @@ -46,7 +46,7 @@ public void CreateDefault_AllowsNullDelegate()
{
// Arrange
var codeDocument = TestRazorCodeDocument.CreateEmpty();
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

// Act
CodeTarget.CreateDefault(codeDocument, options, configure: null);
Expand All @@ -59,7 +59,7 @@ public void CreateEmpty_AllowsNullDelegate()
{
// Arrange
var codeDocument = TestRazorCodeDocument.CreateEmpty();
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

// Act
CodeTarget.CreateDefault(codeDocument, options, configure: null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public void Build_CreatesDefaultCodeTarget()
{
// Arrange
var codeDocument = TestRazorCodeDocument.CreateEmpty();
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

var builder = new DefaultCodeTargetBuilder(codeDocument, options);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class DefaultCodeTargetTest
public void Constructor_CreatesDefensiveCopy()
{
// Arrange
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

var extensions = new ICodeTargetExtension[]
{
Expand All @@ -33,7 +33,7 @@ public void Constructor_CreatesDefensiveCopy()
public void CreateWriter_DesignTime_CreatesDesignTimeNodeWriter()
{
// Arrange
var options = RazorCodeGenerationOptions.CreateDesignTimeDefault();
var options = RazorCodeGenerationOptions.DesignTimeDefault;
var target = new DefaultCodeTarget(options, Enumerable.Empty<ICodeTargetExtension>());

// Act
Expand All @@ -47,7 +47,7 @@ public void CreateWriter_DesignTime_CreatesDesignTimeNodeWriter()
public void CreateWriter_Runtime_CreatesRuntimeNodeWriter()
{
// Arrange
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;
var target = new DefaultCodeTarget(options, Enumerable.Empty<ICodeTargetExtension>());

// Act
Expand All @@ -61,7 +61,7 @@ public void CreateWriter_Runtime_CreatesRuntimeNodeWriter()
public void HasExtension_ReturnsTrue_WhenExtensionFound()
{
// Arrange
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

var extensions = new ICodeTargetExtension[]
{
Expand All @@ -82,7 +82,7 @@ public void HasExtension_ReturnsTrue_WhenExtensionFound()
public void HasExtension_ReturnsFalse_WhenExtensionNotFound()
{
// Arrange
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

var extensions = new ICodeTargetExtension[]
{
Expand All @@ -103,7 +103,7 @@ public void HasExtension_ReturnsFalse_WhenExtensionNotFound()
public void GetExtension_ReturnsExtension_WhenExtensionFound()
{
// Arrange
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

var extensions = new ICodeTargetExtension[]
{
Expand All @@ -124,7 +124,7 @@ public void GetExtension_ReturnsExtension_WhenExtensionFound()
public void GetExtension_ReturnsFirstMatch_WhenExtensionFound()
{
// Arrange
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

var extensions = new ICodeTargetExtension[]
{
Expand All @@ -148,7 +148,7 @@ public void GetExtension_ReturnsFirstMatch_WhenExtensionFound()
public void GetExtension_ReturnsNull_WhenExtensionNotFound()
{
// Arrange
var options = RazorCodeGenerationOptions.CreateDefault();
var options = RazorCodeGenerationOptions.Default;

var extensions = new ICodeTargetExtension[]
{
Expand Down
Loading

0 comments on commit 3423142

Please sign in to comment.