Add warnings for various unbound attributes in Razor - #84760
Add warnings for various unbound attributes in Razor#84760davidwengier wants to merge 10 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 32920976-48ef-42be-a7e7-f1e06044dd0d
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 32920976-48ef-42be-a7e7-f1e06044dd0d
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 32920976-48ef-42be-a7e7-f1e06044dd0d
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 32920976-48ef-42be-a7e7-f1e06044dd0d
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 32920976-48ef-42be-a7e7-f1e06044dd0d
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 32920976-48ef-42be-a7e7-f1e06044dd0d
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 32920976-48ef-42be-a7e7-f1e06044dd0d
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR introduces several new Razor warning-level 11 diagnostics for component authoring scenarios (unknown component parameters, malformed/unbound directive attributes, and invalid @bind-* usage), along with the necessary compiler plumbing to detect these cases reliably and new integration tests + documentation updates.
Changes:
- Adds new level-11 warnings (
RZ10025–RZ10028) and wires them into component attribute/bind resolution and lowering. - Tracks whether components accept unmatched attributes (via
CaptureUnmatchedValues) and uses that to suppress warnings where appropriate. - Adds integration tests for warning-level gating (10 vs 11) and documents the new warnings in the warning-level catalog.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/TagHelpers/Producers/ComponentTagHelperProducer.cs | Computes and records AcceptsUnmatchedAttributes in component tag helper metadata. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/UnresolvedAttributeIntermediateNode.cs | Tracks whether an unresolved attribute is a directive-attribute candidate. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/TagHelperHtmlAttributeIntermediateNode.cs | Carries attribute-name span and directive-candidate info into later phases. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultTagHelperResolutionPhase.ComponentTagHelperResolver.cs | Emits unbound directive-attribute warnings and preserves spans/candidate flags during conversion. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorIntermediateNodeLoweringPhase.cs | Detects directive-attribute candidates and properly handles escaped @@ attribute names. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/TagHelperDescriptorExtensions.cs | Adds helper to query whether a component accepts unmatched attributes. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentsApi.cs | Adds constants for CaptureUnmatchedValues and Dictionary<,> metadata lookup. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentMetadata.cs | Persists AcceptsUnmatchedAttributes in metadata + checksum. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentLoweringPass.cs | Reports unknown component-parameter warning when appropriate (level 11). |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentDiagnosticFactory.cs | Defines new warning descriptors RZ10025–RZ10028 at warning level 11. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentBindLoweringPass.cs | Reports new bind-related warnings (missing target/change parameter) at level 11. |
| src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentDiagnosticRazorIntegrationTest.cs | Adds integration coverage for directive-attribute + unknown-parameter warnings and level gating. |
| src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentBindIntegrationTest.cs | Adds integration coverage for bind warnings and level gating. |
| docs/razor/WarningWavesProposal.md | Updates proposal doc to point to the warning-level catalog. |
| docs/razor/warning-levels.md | Adds/updates the warning-level catalog entries for the new diagnostics. |
| .github/instructions/Razor.instructions.md | Documents the process expectation to track non-zero warning-level warnings in the catalog. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 32920976-48ef-42be-a7e7-f1e06044dd0d
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentsApi.cs:52
ComponentsApi.IDictionary.MetadataNameappears to have a typo:System.Collection.IDictionary2doesn’t exist (should beSystem.Collections.Generic.IDictionary2). Even if unused today, this constant is a footgun for future callers.
public static class IDictionary
{
public const string MetadataName = "System.Collection.IDictionary`2";
}
src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/ComponentDiagnosticRazorIntegrationTest.cs:762
- The new RZ10028 assertions don’t verify
diagnostic.WarningLevel == 11, so the tests would still pass if the warning level accidentally regressed to 0 (or another level). Since this PR is explicitly about level-11 warnings, it’s worth asserting the level here too.
private static void AssertUnboundDirectiveAttributeDiagnostic(RazorDiagnostic diagnostic, string attributeName)
{
Assert.Equal("RZ10028", diagnostic.Id);
Assert.Equal(RazorDiagnosticSeverity.Warning, diagnostic.Severity);
Assert.Equal(
$"The attribute '{attributeName}' could not be bound to any directive attribute.",
diagnostic.GetMessage(CultureInfo.CurrentCulture));
Assert.Equal(attributeName.Length, diagnostic.Span.Length);
}
|
@chsienki another one for re-approval after merging and fixing the main break (and feedback) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/UnresolvedAttributeIntermediateNode.cs:25
IsDirectiveAttributeCandidateis part of the node’s semantic state, but it isn’t copied inCloneNode(). This means any phase that clones the IR tree will drop the flag and may miss (or incorrectly produce) the unbound directive-attribute warning behavior.
/// <summary>
/// Whether the parser treated the leading <c>@</c> as a Razor transition rather than an escaped
/// literal. Semantic resolution determines whether the candidate binds to a directive attribute.
/// </summary>
public bool IsDirectiveAttributeCandidate { get; set; }
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 12a73d6b-94c2-4ad0-9825-d762b9a23b8c
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentsApi.cs:54
ComponentsApi.IDictionary.MetadataNameappears to have a typo (System.Collection...). The correct metadata name for the genericIDictionary<,>interface isSystem.Collections.Generic.IDictionary2(note the missings` and namespace). If this constant is used in the future it will fail type lookup.
{
public const string MetadataName = "System.Collection.IDictionary`2";
}
public static class Dictionary
|
Sorry @chsienki, needs another re-approval, I had to push another fix. I should have done the merge locally and checked for issues, but I was lazy and just pressed the button in GitHub to do it, so had to do a minor test update. |
Add new warnings under analysis level 11, for unknow attributes, directive attributes, and bind attributes, on components.
Fixes dotnet/razor#11114
Fixes dotnet/razor#13125
Fixes dotnet/razor#7650
Microsoft Reviewers: Open in CodeFlow