dotnet / dotnet/roslyn

Tag synthesized helper nodes (type-inference helpers, etc.) with IsSynthesizedHelper for uniform decl/impl filtering

Open
#83,891 0 comments 1 reaction 2 assignees Claimed by @chsienki View on GitHub
Area-Razor
Dominant language
C#
Stars
20.7k
Forks
4.3k
PR merge metrics
PR metrics pending

Description

Follow-up from review feedback on #83688.

While reviewing the decl/impl split, @davidwengier suggested that synthesized helper nodes -- specifically the type-inference helpers represented by `NamespaceDeclarationIntermediateNode { IsGenericTyped: true }` -- should be tagged with the new general-purpose `IntermediateNode.IsSynthesizedHelper` flag, so the decl-lowering filter can treat them uniformly with the rest of the compiler plumbing instead of carrying a special case.

### Today

`DefaultRazorDeclCSharpLoweringPhase` filters synthesized helpers from the decl tree at the namespace and class levels via `IsSynthesizedHelper`, but at the document level it falls back to a node-type check:

```csharp
// src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorDeclCSharpLoweringPhase.cs
foreach (var docChild in documentNode.Children)
{
if (docChild is NamespaceDeclarationIntermediateNode { IsGenericTyped: true })
{
continue;
}

declDocNode.Children.Add(docChild == primaryNamespace ? declNamespace : docChild);
}
```

This inconsistency was flagged by Copilot review as well -- if other document-level synthesized helper nodes are ever introduced, they will silently leak into the decl document.

### Proposal

Set `IsSynthesizedHelper = true` on the type-inference helper namespace nodes (and any other synthesized helper nodes that don't yet set it, e.g. anywhere the codegen pipeline injects compiler-only plumbing) when they are constructed, then collapse the document-level loop in `DefaultRazorDeclCSharpLoweringPhase` to use the same `IsSynthesizedHelper` check the other two levels already use.

### Why

- Uniform filter at all three levels of the decl spine (document / namespace / class).
- New synthesized helper nodes added in future automatically participate in the split as long as authors set the flag, with no need to extend the special-case list in the lowering phase.
- Removes an asymmetry that is currently easy to miss.

### Related

- PR #83688 (Sonic 3.1: Generate Decl Files) -- introduced `IsSynthesizedHelper`
- Review thread: https://github.com/dotnet/roslyn/pull/83688#discussion_r... (Wengier suggestion at line 32 of `IntermediateNode.cs`, follow-up exchange at line 90 of `DefaultRazorDeclCSharpLoweringPhase.cs`)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.