Retire the PROTOTYPE(sonic) markers under src/Razor - #84743
Merged
chsienki merged 4 commits intoAug 4, 2026
Conversation
Neither is a Sonic regression, so the PROTOTYPE(sonic) skips shouldn't have been there. RenderMode_GenericComponent_CSharp10 hits the known @typeparam + @rendermode limitation (dotnet/razor#9683) -- the file-scoped attribute class handles it on C# 11+, but C# 10 still surfaces the raw C# errors. LanguageVersion_BreakingChange_8_0 is just that Razor < 11 doesn't map the @rendermode expression, which we can't change retroactively without a newer compiler diverging from the shipped VS tooling and breaking hot reload. Un-skipped both to assert the current behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8188d9c-18d2-4e12-8b25-4c8d99114193
This was never a Sonic regression. On main, ImplementInterface already interleaves the new IDerived members with the existing IBase ones, and the test expects exactly that. "Fix test output" (8acbfb4) changed the expected to append-at-end back when @implements components were split with a render-only impl -- ImplementInterface saw no existing IBase members in the impl it edited, so it appended cleanly at the end. The classify-first reorder (dotnet#84605) then made @implements/@inherits/@typeparam fall back to a single unsplit document again, so the impl carries the IBase members and Roslyn interleaves, exactly like main. The append-at-end baseline was left behind, and the test got skipped as a Sonic bug it never was. Restored main's interleaved baseline and un-skipped. Closes dotnet#84609. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8188d9c-18d2-4e12-8b25-4c8d99114193
…delete the dead ones The SONICDEV guardrail marks the impl/decl-specific RazorCodeDocument getters [Obsolete] to push callers onto the GetCSharpDocument(bool)/GetRequiredCSharpDocument(bool) overloads, so a caller has to say out loud which half it wants. Building the product with SONICDEV defined, the only warnings left were five call sites in the source generator, so they were still reaching for GetRequiredImplCSharpDocument()/GetDeclCSharpDocument(). Moved them to the bool overloads (behavior-identical -- impl is declarationDocument: false, decl is declarationDocument: true), and the product now builds clean with SONICDEV on. The impl/decl getters stay -- they're the API the tests use all over. But four convenience wrappers had zero callers anywhere, so removed them outright rather than leave dead obsolete surface: * RazorCodeDocument.TryGetImplCSharpDocument * RazorCodeDocumentExtensions.GetCSharpSourceText * ProjectExtensions.TryGetSourceGeneratedDocumentForRazorDocumentAsync (the no-bool overload) * IRazorEditServiceExtensions.MapCSharpEditsAsync (the no-bool overload) All four live in internal types, so nothing to update in PublicAPI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8188d9c-18d2-4e12-8b25-4c8d99114193
…ICDEV scaffolding The impl/decl-named RazorCodeDocument getters (GetImplCSharpDocument, GetRequiredImplCSharpDocument, GetDeclCSharpDocument) have no production callers -- every use is a test or a benchmark. Test-only methods sitting on the production type, kept behind #if SONICDEV obsolete markers, are a smell, so they move to a RazorCodeDocumentTestExtensions in the Microsoft.AspNetCore.Razor.Test.Common friend assembly, wrapping the internal GetCSharpDocument(bool)/GetRequiredCSharpDocument(bool) overloads. The extension sits in the Microsoft.AspNetCore.Razor.Language namespace so the call sites resolve against the same names with no edits; the one perf benchmark, which doesn't reference Test.Common, calls the bool overload directly. With the last #if SONICDEV markers gone, nothing reads the SONICDEV constant, so the commented-out build toggle comes out too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8188d9c-18d2-4e12-8b25-4c8d99114193
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Retires the remaining PROTOTYPE(sonic)/SONICDEV guardrails under src/Razor by removing now-inert wrappers/obsoletions, updating call sites to the explicit GetCSharpDocument(bool) APIs, and re-enabling tests whose expectations now match current behavior.
Changes:
- Re-enable previously skipped cohost and rendermode integration tests, updating expected output/diagnostics.
- Remove
SONICDEV-gated convenience APIs/wrappers and update callers to pass the decl/impl choice explicitly. - Move test-only decl/impl getters to a new
RazorCodeDocumentTestExtensionshelper in the Test.Common friend assembly.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Razor/src/Shared/Microsoft.AspNetCore.Razor.Test.Common/Language/RazorCodeDocumentTestExtensions.cs | Adds test-only extension helpers for decl/impl C# document access. |
| src/Razor/src/Razor/test/Microsoft.CodeAnalysis.Razor.CohostingShared.UnitTests/CodeActions/ImplementInterfaceTests.cs | Unskips and updates expected member ordering for ImplementInterface cohost test. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/DocumentMapping/IRazorEditServiceExtensions.cs | Removes the SONICDEV-guarded overload and keeps the explicit decl/impl overload. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/RazorCodeDocumentExtensions.cs | Removes SONICDEV-guarded GetCSharpSourceText wrapper; callers use explicit document selection. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/ProjectExtensions.cs | Deletes SONICDEV-guarded convenience wrapper for impl S.G. document retrieval. |
| src/Razor/src/Compiler/perf/Microbenchmarks/CodeGenerationBenchmark.cs | Updates benchmark to use GetRequiredCSharpDocument(declarationDocument: false). |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/RazorSourceGenerator.cs | Updates generator to use explicit decl/impl selection APIs. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorCodeDocument.cs | Removes SONICDEV-guarded impl/decl convenience getters from the production type. |
| src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentRenderModeDirectiveIntegrationTests.cs | Unskips and adjusts diagnostic expectations/comments for rendermode tests. |
| src/Razor/Directory.Build.props | Removes the now-inert SONICDEV define toggle comment block. |
Comment on lines
107
to
111
| { | ||
| RazorSourceGeneratorEventSource.Log.GenerateDeclarationCodeStart(sourceItem.FilePath); | ||
| var declEngine = GetDeclarationProjectEngine(sourceItem, imports, razorSourceGeneratorOptions); | ||
| fallbackDecl = declEngine.Process(sourceItem, cancellationToken).GetRequiredImplCSharpDocument(); | ||
| fallbackDecl = declEngine.Process(sourceItem, cancellationToken).GetRequiredCSharpDocument(declarationDocument: false); | ||
| fallbackTypeName = typeName; |
Member
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Retires the
PROTOTYPE(sonic)markers undersrc/Razor, in four independent commits.Un-skip two rendermode tests
RenderMode_GenericComponent_CSharp10andLanguageVersion_BreakingChange_8_0were skipped as Sonic bugs, but neither is a regression -- the behavior matchesmain. The generic case is the known@typeparam+@rendermodelimitation (dotnet/razor#9683); the pre-11 case is the intentional "don't map the expression, to avoid breaking hot reload" behavior. Un-skipped, asserting current behavior with a comment explaining why.Un-skip the ImplementInterface cohost test -- Closes #84609
On
main, ImplementInterface already interleaves the new members with the existing ones, and the test expects that. A mid-branch baseline change (during the declaration-engine era, when@implementssplit with a render-only impl) flipped the expected order to append-at-end; the classify-first reorder (#84605) then made@implementsfall back to unsplit, matchingmainagain, so the stale baseline started failing and got skipped. Restoredmain's baseline.Retire the SONICDEV document-getter guardrails
The
#if SONICDEV[Obsolete]markers nudged callers ontoGetCSharpDocument(bool)/GetRequiredCSharpDocument(bool). The only product callers left were in the source generator, so they move to the bool overloads. Four convenience wrappers had zero callers and are deleted. The three impl/decl getters were test-only, so they move to aRazorCodeDocumentTestExtensionsin theMicrosoft.AspNetCore.Razor.Test.Commonfriend assembly; with the last markers gone, the now-inert SONICDEV build toggle comes out too.Not addressed
The fallback-component nested-delegate metadata loss (#84646) stays skipped as a known issue -- it's narrow and fails loudly at compile time. Both fix options are sized in a comment on that issue.
Microsoft Reviewers: Open in CodeFlow