Deferred tag helper lowering: separate resolution from lowering phase#12957
Deferred tag helper lowering: separate resolution from lowering phase#12957
Conversation
Introduce new intermediate representation (IR) node types that enable separating the lowering phase from tag helper resolution. Currently, the lowering phase must run AFTER the tag helper rewrite phase because it consumes rewritten syntax tree nodes (MarkupTagHelperElement etc.) to produce TagHelperIntermediateNodes directly. This tight coupling means lowering depends on both the syntax tree AND tag helper binding results simultaneously. These new "unresolved" node types break that dependency. They represent elements and attributes whose final form (plain HTML vs. bound tag helper) is not yet determined at lowering time: - ElementOrTagHelperIntermediateNode: an element that may or may not match a tag helper. Stores all syntax-tree-derived metadata (tag name, attributes, source spans, component hints) so the resolution phase never needs syntax tree access. - MarkupOrTagHelperAttributeIntermediateNode: an attribute with pre-lowered fallback forms for both the tag-helper-attribute path (AsTagHelperAttribute) and the markup-attribute path (AsMarkupAttribute). - MarkupOrTagHelperAttributeValueIntermediateNode: a literal attribute value that resolves to CSharp, HtmlContent, or HtmlAttributeValue depending on tag helper binding. - CSharpOrTagHelperExpressionAttributeValueIntermediateNode: a dynamic attribute value (e.g. Value="@expr") that resolves differently based on binding. Also adds FindDescendant<T> test helper for robust IR tree assertions that work regardless of tree shape changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Prepare the base LoweringVisitor class for the upcoming refactoring by adding shared helper methods and consolidating duplicated visitor methods. This commit adds infrastructure without changing any observable behavior -- the subclass visitor methods still override the new base class methods via hiding. **New helper methods in LoweringVisitor:** - InferAttributeStructure: determines quote style from attribute syntax - ComputeAttributeValueSourceSpan: computes source span of attribute values - ExtractAttributeData: extracts attribute name/value pairs for tag helper binding (avoids re-scanning the syntax tree in the resolution phase) - ComputeSourceSpanFromChildren: aggregates child source spans (replaces duplicated inline logic in CSharp expression visitors) **Consolidated visitor methods moved to base class:** - VisitMarkupDynamicAttributeValue, VisitMarkupLiteralAttributeValue, VisitCSharpTemplateBlock, VisitCSharpExpressionLiteral: were duplicated across LegacyFileKindVisitor and ComponentFileKindVisitor with minor differences. The new base versions are unified and include awareness of the _insideUnresolvedAttribute flag (used by later commits). - VisitAttributeValue, Combine: shared attribute value merging logic **Other changes:** - Added class-level documentation explaining the phase's role - Moved LooksLikeAComponentName to top-level class scope (needed by the resolution phase in a later commit) - Added _insideUnresolvedAttribute field to base class (set by visitor changes in the next commit) The subclass methods are marked with 'new' to make the intentional hiding explicit. They will be removed in the following commits when each visitor is refactored to use the base class implementations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
With the shared infrastructure in place, refactor all three visitor subclasses to produce unresolved IR nodes instead of resolved tag helper nodes. **Core architectural change:** Switch ExecuteCore to use the pre-tag-helper syntax tree. The lowering phase now runs before the tag helper rewrite phase, so elements that might be tag helpers appear as regular MarkupElementSyntax (not MarkupTagHelperElementSyntax). This means the VisitMarkupTagHelper* methods are no longer called and can be removed. Instead, ALL elements are lowered as unresolved ElementOrTagHelperIntermediateNode, with pre-computed data for the resolution phase. **LegacyFileKindVisitor changes:** - VisitMarkupElement now wraps all elements in ElementOrTagHelperIntermediateNode with pre-computed boundary indices (StartTagEndIndex, BodyEndIndex), extracted attribute data, and tag structure flags - VisitMarkupAttributeBlock creates MarkupOrTagHelperAttributeIntermediateNode with two pre-computed fallback forms: AsTagHelperAttribute (merged attribute value via base VisitAttributeValue) and AsMarkupAttribute (full attribute as HTML via new LowerAttributeAsHtml) - Removed 7 VisitMarkupTagHelper* overrides (dead code with pre-rewrite tree) - Removed duplicate VisitAttributeValue/Combine (now inherited from base) - VisitHtmlContent gains boundary awareness to prevent merging across regions **ComponentFileKindVisitor changes:** - Same unresolved pattern: VisitMarkupElement wraps in ElementOrTagHelperIntermediateNode with IsComponent=true - Attributes get fallback forms like Legacy - Removed 9 VisitMarkupTagHelper* overrides and duplicate methods - Removed LooksLikeAComponentName (moved to top-level in prior commit) **ComponentImportFileKindVisitor changes:** - Removed dead DefaultVisit and VisitMarkupTagHelperElement overrides - Uses ComputeSourceSpanFromChildren helper (added in prior commit) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6aeb8be to
3d8f3b7
Compare
… unwrapping Add DefaultTagHelperResolutionPhase with the algorithm's top-level structure and the 'unwrap' (non-match) path fully implemented. The two tag helper construction methods -- BuildLegacyTagHelper and BuildComponentTagHelper -- are stubbed and will be implemented in the following commits. The resolution phase algorithm works as follows: 1. **Tree walking** (ResolveElements/ResolveElement): Walk the IR tree looking for ElementOrTagHelperIntermediateNode. For each one, attempt to bind it to known tag helpers using the TagHelperBinder. 2. **Decision logic** (ResolveElement): Based on the binding result: no match -> unwrap the element back to plain HTML markup; directive-attribute-only -> unwrap but keep attrs available; legacy match -> call BuildLegacyTagHelper (stubbed); component match -> call BuildComponentTagHelper (stubbed). 3. **Element unwrapping** (ConvertToMarkupElement/UnwrapElement): When an element doesn't match a tag helper, its ElementOrTagHelperIntermediateNode wrapper must be removed and its children promoted to plain HTML nodes. This handles attribute structure inference, data-dash attributes, and merging adjacent HTML content nodes. 4. **Validation**: Tag mode determination, allowed children validation, consistent tag structure checks, and malformed end-tag diagnostics. NOTE: Tests will not pass until BuildLegacyTagHelper and BuildComponentTagHelper are implemented in the following commits. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add BuildLegacyTagHelper and all its helper methods, completing the legacy (.cshtml) tag helper construction path. This handles how the resolution phase converts unresolved elements that match legacy tag helpers into TagHelperIntermediateNode IR. The legacy construction algorithm processes each attribute on the element: - Minimized bound attributes (e.g., <input disabled>) -> create property node with empty children - Bound attributes with values -> create property node, then dispatch to shared value lowering pipeline (stubbed, implemented in following commit) - Unbound attributes -> convert pre-parsed HTML values back to tag helper form via ConvertValueChildren/PopulateUnboundAttributeValue The key complexity is ConvertValueChildren, which must reverse the lowering phase's pre-parsed HTML structure. During lowering, attribute values were expanded into HtmlAttributeValue and CSharpExpressionAttributeValue nodes. The legacy construction path must convert these back into flat token sequences that TagHelperPropertyIntermediateNode expects. Also adds ConvertUnresolvedValuesToBasicForm (shared between legacy and component paths) which converts unresolved attribute value IR nodes into standard HtmlContent/CSharpExpression shapes. NOTE: LowerUnresolvedAttributeValues is stubbed -- the shared value lowering pipeline that handles string vs non-string attribute value conversion will be implemented in a following commit. BuildComponentTagHelper remains stubbed as well. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add BuildComponentTagHelper and all its helper methods, completing the component (.razor) tag helper construction path. This handles how the resolution phase converts unresolved elements that match Blazor component tag helpers into TagHelperIntermediateNode IR. The component construction algorithm processes each attribute: - Directive attributes (@Bind, @OnClick, @ref) -> ConvertToUnresolvedDirectiveAttribute with parameter support (e.g., @Bind:after, @OnClick:preventDefault) - Bound properties (Value, Class) -> ConvertToUnresolvedBoundProperty - Unbound HTML attributes -> ConvertToUnresolvedUnboundAttribute Key differences from legacy construction: - Components support directive attributes with rich parameter binding - Component attributes undergo NormalizeBoundPropertyChildren to merge adjacent CSharp tokens and match the expected IR structure - Components use ConvertExpressionAttributeValuesToCSharpExpression to normalize expression attribute values into CSharpExpressionIntermediateNode Also adds shared component post-processing helpers: - ConvertComponentAttributeToTagHelper: handles special attribute types (HtmlAttribute, ComponentAttribute, SplatAttribute) - CopyAsTagHelperAttributeValues: copies attribute values into expected tag helper property structure - FlattenToDirectCSharpTokens/FlattenDirectiveChildrenToCSharpTokens: recursive token flattening for directive parameter matching NOTE: LowerUnresolvedAttributeValues remains stubbed -- the shared value lowering pipeline will be implemented in the following commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the LowerUnresolvedAttributeValues stub with the full shared value lowering pipeline. This is the final piece of the resolution phase -- it handles converting unresolved attribute value IR nodes into their final CSharp/HtmlContent form for both legacy and component tag helpers. The value lowering pipeline dispatches based on two axes: (1) string vs non-string attribute type (determines output structure), and (2) legacy vs component context (determines lowering strategy). **Non-string value lowering (produces CSharp tokens):** - Legacy: ClassifyValueContent examines the attribute's children to detect the pattern -- implicit expression, explicit expression, code block, or mixed content. Each has specialized handling: implicit expressions extract content from source text, explicit @(expr) emits structured @, (, content, ) tokens, code blocks preserve CSharpCode nodes directly, and mixed content with @@ escapes emits CSharpExpression for escapes. - Component: simpler -- flatten each child directly to CSharp tokens using FlattenToDirectCSharpTokens recursive helper. **String value lowering (produces HtmlContent/CSharp nodes):** - Legacy: groups adjacent literal tokens into 'pending parts' that are flushed as HtmlContent nodes, with CSharp expressions emitted inline. Adjacent literals are batched into single HtmlContent nodes. - Component: processes each child individually, converting expression attribute values inline and merging adjacent HtmlContent nodes. Also adds remaining utility methods: EmitExplicitExpressionTokens, EmitParenthesizedExpressionTokens, EmitEscapedAtCSharpExpression, CollectAllTokenContent, and CollectUnresolvedLiteralContent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Insert DefaultTagHelperResolutionPhase into the compiler pipeline between lowering and the tag helper rewrite phase. The new phase order is: Discovery -> Lowering -> Resolution -> Rewrite -> ... Lowering now runs first on the original syntax tree, and resolution handles tag helper binding on IR nodes. Downstream fixes required by the new pipeline: - SourceGeneratorProjectEngine: change split point and re-run from discovery+1 when tag helpers change (since lowering now produces unresolved nodes dependent on discovery results) - ComponentLoweringPass: transfer diagnostics from TagHelperIntermediateNode to ComponentChildContentIntermediateNode during child content rewriting - ComponentBindLoweringPass: guard against empty children in @Bind:event parameters (unresolved pipeline can produce zero-child parameter nodes) - ComponentTypeArgumentIntermediateNode: handle HtmlContent form of type argument values produced by the unresolved pipeline Note: existing tests are updated in the following commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adapt tests to the new compiler pipeline where resolution runs after lowering as a separate phase. Test logic changes: - NodeWriter tests: update ExecutePhasesThrough to target DefaultTagHelperResolutionPhase instead of the lowering phase, and use FindDescendant<T>() for robust IR traversal - DefaultRazorTagHelperBinderPhaseTest: refactor from syntax-tree-based to IR-based assertions, since the binder phase now feeds into lowering+resolution rather than rewriting the syntax tree directly - RazorProjectEngineTest: update phase ordering assertion - LoweringPhaseIntegrationTest: update phase endpoint IR baseline updates (499 files) -- systematic differences from the new pipeline: - Source span adjustments from different boundary computation - LazyIntermediateToken -> IntermediateToken type changes - HtmlContent span changes for whitespace between tag helper attributes No codegen, mapping, or diagnostic baselines change -- only .ir.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add end-to-end compilation tests exercising real-world Blazor component patterns through the source generator pipeline. These catch regressions that IR-level tests miss by verifying actual C# compilation succeeds: - Router with CascadingAuthenticationState (default Blazor template pattern) - @Bind with generic components - RenderFragment<T> implicit context variables in nested wrappers - AuthorizeView with nested ChildContent and RenderFragment - Various component nesting depths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3d8f3b7 to
4e7d84d
Compare
| namespace Microsoft.AspNetCore.Razor.Language.Intermediate; | ||
|
|
||
| /// <summary> | ||
| /// A unresolved intermediate node representing a dynamic/expression attribute value (e.g. the |
| /// unresolved-specific IR nodes. Set unconditionally (not just when true) to ensure correct | ||
| /// reset between consecutive attributes. | ||
| /// </summary> | ||
| #pragma warning disable CS0649 // Assigned in subclass visitor methods (next commit) |
There was a problem hiding this comment.
never mind, was removed in later commit
|
|
||
| namespace Microsoft.AspNetCore.Razor.Language; | ||
|
|
||
| /// <summary> |
...rosoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorIntermediateNodeLoweringPhase.cs
Show resolved
Hide resolved
|
|
||
| foreach (var child in descendantNodes) | ||
| { | ||
| if (child is CSharpImplicitExpressionSyntax || child is CSharpExplicitExpressionSyntax) |
| /// <see cref="CSharpCodeAttributeValueIntermediateNode"/> (for unbound/plain HTML attributes)</item> | ||
| /// </list> | ||
| /// </summary> | ||
| internal sealed class CSharpOrTagHelperExpressionAttributeValueIntermediateNode : IntermediateNode |
There was a problem hiding this comment.
Razor used to do all sorts of tree mutation, so properties (conceptually anyway) made sense, but we generally don't anymore. I'd love to do a pass to change it to make them properly immutable that takes all the data in ctors at some point.
| _builder.Push(unresolvedNode); | ||
| Visit(node.Value); | ||
| _builder.Pop(); | ||
| return; |
There was a problem hiding this comment.
I wonder if it might read a bit nicer if this weren't so separated from the other cases, eg:
var source = BuildSourceSpanFromNode(node);
var prefix = node.Prefix?.GetContent() ?? string.Empty;
IntermediateNode irNode;
if (_insideUnresolvedAttribute)
{
irNode = new CSharpOrTagHelperExpressionAttributeValueIntermediateNode()
{
Prefix = prefix,
ContainsExpression = containsExpression,
Source = source,
};
}
else
{
irNode = containsExpression
? new CSharpExpressionAttributeValueIntermediateNode()
{
Prefix = prefix,
Source = source,
}
: new CSharpCodeAttributeValueIntermediateNode()
{
Prefix = prefix,
Source = source,
};
}
_builder.Push(irNode);
Visit(node.Value);
_builder.Pop();
| return; | ||
| } | ||
|
|
||
| _builder.Push(new HtmlAttributeValueIntermediateNode() |
There was a problem hiding this comment.
Can you explain how this differs from below? (I think what I'm missing is what the Push/Pop does here)
var prefix = node.Prefix?.GetContent() ?? string.Empty;
var source = BuildSourceSpanFromNode(node);
var irNode = _insideUnresolvedAttribute
? new MarkupOrTagHelperAttributeValueIntermediateNode()
{
Prefix = prefix,
Source = source,
}
: (IntermediateNode)new HtmlAttributeValueIntermediateNode()
{
Prefix = prefix,
Source = source,
};
irNode.Children.Add(IntermediateNodeFactory.HtmlToken(
arg: node,
contentFactory: static node => node.Value?.GetContent() ?? string.Empty,
source: BuildSourceSpanFromNode(node.Value)));
| // If we are top level in a tag helper HTML attribute, we want to be rendered as markup. | ||
| // This case happens for duplicate non-string bound attributes. They would be initially be categorized as | ||
| // CSharp but since they are duplicate, they should just be markup. | ||
| var markupLiteral = SyntaxFactory.MarkupTextLiteral(node.LiteralTokens).Green.CreateRed(node.Parent, node.Position); |
| namespace Microsoft.AspNetCore.Razor.Language; | ||
|
|
||
| /// <summary> | ||
| /// Converts the Razor syntax tree into intermediate representation (IR) nodes. Runs before |
There was a problem hiding this comment.
I assume making removing the nullable directive above is non-trivial for this file? So much easier to reason about things at this level without it.
| position = children.Count > 0 ? children[0].Position : position; | ||
| } | ||
|
|
||
| if (children.TryCast<MarkupLiteralAttributeValueSyntax>(out var attributeLiteralArray)) |
There was a problem hiding this comment.
Feels like a lot of duplicated code here around the builder. Would something like the following be a bit simpler (and still work?)
using PooledArrayBuilder<SyntaxToken> builder = [];
if (children.TryCast<MarkupLiteralAttributeValueSyntax>(out var attributeLiteralArray))
{
AppendTokens(escapedAtTokens, attributeLiteralArray, static attributeLiteral => MergeAttributeValue(attributeLiteral).LiteralTokens);
var rewritten = SyntaxFactory.MarkupTextLiteral(builder.ToList()).Green.CreateRed(node.Parent, position);
Visit(rewritten);
}
else if (children.TryCast<MarkupTextLiteralSyntax>(out var markupLiteralArray))
{
AppendTokens(escapedAtTokens, markupLiteralArray, static markupLiteral => markupLiteral.LiteralTokens);
var rewritten = SyntaxFactory.MarkupTextLiteral(builder.ToList()).Green.CreateRed(node.Parent, position);
Visit(rewritten);
}
else if (children.TryCast<CSharpExpressionLiteralSyntax>(out var expressionLiteralArray))
{
AppendTokens(escapedAtTokens, expressionLiteralArray, static expressionLiteral => expressionLiteral.LiteralTokens);
var generator = expressionLiteralArray[^1].ChunkGenerator;
var editHandler = expressionLiteralArray[^1].EditHandler;
var rewritten = SyntaxFactory.CSharpExpressionLiteral(builder.ToList(), generator, editHandler).Green.CreateRed(node.Parent, position);
Visit(rewritten);
}
else
{
if (escapedAtTokens.Count > 0)
{
// If we have escaped @ tokens but no other content to merge with,
// create a MarkupTextLiteral just for the escaped @ tokens
var rewritten = SyntaxFactory.MarkupTextLiteral(escapedAtTokens).Green.CreateRed(node.Parent, position);
Visit(rewritten);
}
else
{
Visit(node);
}
}
static void AppendTokens<TSyntaxNode>(
SyntaxTokenList escapedAtTokens,
ImmutableArray<TSyntaxNode> attributeLiteralArray,
Func<TSyntaxNode, SyntaxTokenList> getTokensForNode)
{
using PooledArrayBuilder<SyntaxToken> builder = [];
if (escapedAtTokens.Count > 0)
{
builder.AddRange(escapedAtTokens);
}
foreach (var literal in attributeLiteralArray)
{
var tokens = getTokensForNode(literal);
builder.AddRange(tokens);
}
}
| source.CharacterIndex, | ||
| source.Length + item.Width, | ||
| source.LineCount, | ||
| source.EndCharacterIndex); |
There was a problem hiding this comment.
tangent: Not sure how common this is, but it might be nice if SourceSpan was marked as readonly and had some With* methods, so this could be something like:
node.Source = source.WithLength(source.Length + item.Width);
There was a problem hiding this comment.
Yeah, we do have some .With methods already actually, I'll add one, good idea.
There was a problem hiding this comment.
Agreed that would be nice. Definitely a tangent for a follow-up though.
| : node.Value.EndPosition; | ||
| var valueLength = valueEnd - valueStart; | ||
|
|
||
| if (valueLength > 0) |
There was a problem hiding this comment.
more duplicated code. Maybe something like below might get rid of it?
protected SourceSpan? ComputeAttributeValueSourceSpan(MarkupAttributeBlockSyntax node)
{
int valueStart;
int valueEnd;
if (node.Value != null)
{
valueStart = node.ValuePrefix != null
? node.ValuePrefix.EndPosition
: node.Value.Position;
valueEnd = node.ValueSuffix != null
? node.ValueSuffix.Position
: node.Value.EndPosition;
if (valueStart == valueEnd)
{
valueStart = node.Value.Position;
valueEnd = valueStart;
}
}
else if (node.EqualsToken.Kind != SyntaxKind.None)
{
valueStart = node.ValuePrefix != null
? node.ValuePrefix.EndPosition
: node.EqualsToken.EndPosition;
valueEnd = valueStart;
}
else
{
return null;
}
var valueLength = valueEnd - valueStart;
var location = SourceDocument.Text.Lines.GetLinePosition(valueStart);
var endLocation = valueLength == 0 ? location : SourceDocument.Text.Lines.GetLinePosition(valueEnd);
var lineCount = endLocation.Line - location.Line;
return new SourceSpan(
SourceDocument.FilePath,
valueStart,
location.Line,
location.Character,
valueLength,
lineCount,
endLocation.Character);
}
| sourceRangeStart.Value.CharacterIndex, | ||
| contentLength, | ||
| sourceRangeStart.Value.LineCount, | ||
| sourceRangeStart.Value.EndCharacterIndex); |
There was a problem hiding this comment.
It seems odd to me to have the lineCount and EndCharacterIndex just pulled directly from the first child source range
This probably already existed, but would something like GetUnresolvedSyntaxTree or something that doesn't explicitly call out tag helpers be more appropriate? Refers to: src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorIntermediateNodeLoweringPhase.cs:39 in d9ca32c. [](commit_id = d9ca32c, deletion_comment = False) |
| /// All syntax-tree-derived information is stored directly on this node during lowering, | ||
| /// so the resolution phase does not need access to the syntax tree. | ||
| /// </summary> | ||
| internal sealed class ElementOrTagHelperIntermediateNode : IntermediateNode |
There was a problem hiding this comment.
I also like this idea, but I'll do it in a follow up so as not to churn this PR any further.
Consolidate duplicate branches and simplify control flow: - ComponentTagHelperResolver: Merge three expression-flattening branches (CSharpExpression, CSharpExpressionAttributeValue, CSharpOrTagHelper) into single 'or' pattern match - ComponentTagHelperResolver: Merge HtmlContent and MarkupOrTagHelperAttributeValue branches (identical token conversion) - ComponentTagHelperResolver: Merge HtmlContent/HtmlAttributeValue branches in NormalizeBoundPropertyChildren - ComponentTagHelperResolver: Remove redundant CSharpIntermediateToken check (else branch handles it identically) - ComponentTagHelperResolver: Replace unconditional return with canMerge flag and use LINQ All() for inner token check - LegacyTagHelperResolver: Use list pattern for @@ escape detection - LegacyTagHelperResolver: Replace foreach+IndexOf with for loop to avoid redundant linear search Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add API helpers and reduce allocations per review suggestions: - Add IntermediateNodeCollection.AddRange(PooledArrayBuilder) overload to replace foreach+Add loops when copying from pooled builders - Add TagHelperMatchingConventions.HasAttributeMatches() for existence-only checks, avoiding PooledArrayBuilder allocation when the actual matches aren't needed - Replace PooledArrayBuilder<string> + string.Concat(ToArray()) with pooled StringBuilder at all 4 content-collection sites, eliminating intermediate array allocations - Update CollectAllTokenContent to take StringBuilder directly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review comments from davidwengier: - Use 'or' pattern match for type checks in component attribute handling (ComponentTagHelperResolver) - Use property pattern for HasMissingEndCloseAngle + EndTagSpan check (DefaultTagHelperResolutionPhase) - Remove leftover comments from deleted VisitMarkupDynamicAttributeValue method that were incorrectly left above VisitCSharpExplicitExpression (DefaultRazorIntermediateNodeLoweringPhase) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix three missed review nits: - Fix grammar: 'A unresolved' -> 'An unresolved' in XML doc comment - Use 'or' pattern match instead of || for expression type check - Add break after setting containsExpression = true since further iteration is unnecessary once an expression is found Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace HashSet<string> with PooledHashSet<string> for renderedBoundAttributeNames in both Component and Legacy resolvers, avoiding per-element HashSet allocation. Uses try/finally for disposal since PooledHashSet is a ref struct that cannot be passed by ref from a using declaration. - Change Children.Count > 0 to > 1 for merged source span computation since a single child's source span is already set directly and doesn't need merging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refactor ConvertComponentAttributeToTagHelper to take HtmlAttributeIntermediateNode directly instead of IntermediateNode with a type check. At the call site, split the type check so HtmlAttributeIntermediateNode calls the method while other already-resolved attribute types (ComponentAttribute, Splat, SetKey, ReferenceCapture) are added directly to the tag helper node. Suggested by ToddGrun. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fc6ffd3 to
4a7f192
Compare
Summary
This PR separates tag helper resolution from the lowering phase in the Razor compiler. Currently, the lowering phase must run AFTER the tag helper rewrite phase because it consumes rewritten syntax tree nodes to produce
TagHelperIntermediateNodedirectly. This tight coupling means lowering depends on both the syntax tree AND tag helper binding results simultaneously.This PR breaks that dependency by introducing a new
DefaultTagHelperResolutionPhasethat runs between lowering and the rewrite phase. The lowering phase now produces "unresolved" IR nodes (ElementOrTagHelperIntermediateNode,MarkupOrTagHelperAttributeIntermediateNode) that represent elements whose final form is not yet determined. The resolution phase then binds these against the available tag helpers and converts them into the final IR shape.New pipeline order: Discovery → Lowering → Resolution → Rewrite → ...
Commit walkthrough
Each commit is designed to be reviewed independently:
Add IR node types for unresolved tag helper elements — New IR node types that store syntax-tree-derived metadata so the resolution phase never needs syntax tree access.
Add shared infrastructure to base LoweringVisitor class — Helper methods and consolidated duplicate visitor methods in preparation for the refactoring. No behavior change.
Refactor visitor subclasses to produce unresolved tag helper nodes — Switch to the pre-tag-helper syntax tree and produce unresolved IR nodes instead of resolved tag helper nodes. Removes all
VisitMarkupTagHelper*overrides (dead code with the pre-rewrite tree).Add resolution phase skeleton — Top-level algorithm structure: tree walking, element binding, unwrapping non-matches. Tag helper construction methods are stubbed.
Implement legacy tag helper construction — The
.cshtmlpath: processes attributes, handles bound/unbound/minimized cases, converts pre-parsed HTML structure back to tag helper property form.Implement component tag helper construction — The
.razorpath: directive attributes, bound properties, parameter binding, expression normalization.Implement shared value lowering pipeline — Converts unresolved attribute values into CSharp/HtmlContent form, dispatching by string vs non-string type and legacy vs component context.
Wire up resolution phase and fix downstream consumers — Inserts the new phase into the pipeline and fixes downstream passes that needed adjustment.
Update tests and IR baselines — Adapts tests to the new pipeline. 499
.ir.txtbaseline updates; no codegen, mapping, or diagnostic baselines change.Add source generator regression tests — End-to-end compilation tests for real-world Blazor patterns.
Architecture
The resolution phase uses a strategy pattern with two resolver subclasses:
LegacyTagHelperResolver— handles.cshtmlfilesComponentTagHelperResolver— handles.razorfilesThe shared tree walker (
ResolveElement) handles binding, diagnostics, and recursion. Resolver-specific behavior (element conversion, matched-element diagnostics, attribute value lowering) is dispatched through virtual methods. The resolvers are fully self-contained with no back-reference to the parent phase.Testing