From ec1c50b0ab274e4cf53f7447c6ad16fde59ac7b7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 Aug 2025 22:01:15 +0000 Subject: [PATCH 1/4] Initial plan From f983baff80c0e8047e67530ce71d43e380f84092 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 Aug 2025 22:21:26 +0000 Subject: [PATCH 2/4] Implement ElementReference usage analyzer with comprehensive tests Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com> --- .../Analyzers/src/ComponentSymbols.cs | 6 + src/Components/Analyzers/src/ComponentsApi.cs | 6 + .../Analyzers/src/DiagnosticDescriptors.cs | 9 + .../src/ElementReferenceUsageAnalyzer.cs | 123 +++++++++++ src/Components/Analyzers/src/Resources.resx | 9 + .../test/ElementReferenceUsageAnalyzerTest.cs | 196 ++++++++++++++++++ 6 files changed, 349 insertions(+) create mode 100644 src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs create mode 100644 src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs diff --git a/src/Components/Analyzers/src/ComponentSymbols.cs b/src/Components/Analyzers/src/ComponentSymbols.cs index e28da74f28d8..f2b3d516b129 100644 --- a/src/Components/Analyzers/src/ComponentSymbols.cs +++ b/src/Components/Analyzers/src/ComponentSymbols.cs @@ -50,12 +50,14 @@ public static bool TryCreate(Compilation compilation, out ComponentSymbols symbo // Try to get optional symbols for SupplyParameterFromForm analyzer var supplyParameterFromFormAttribute = compilation.GetTypeByMetadataName(ComponentsApi.SupplyParameterFromFormAttribute.MetadataName); var componentBaseType = compilation.GetTypeByMetadataName(ComponentsApi.ComponentBase.MetadataName); + var elementReferenceType = compilation.GetTypeByMetadataName(ComponentsApi.ElementReference.MetadataName); symbols = new ComponentSymbols( parameterAttribute, cascadingParameterAttribute, supplyParameterFromFormAttribute, componentBaseType, + elementReferenceType, parameterCaptureUnmatchedValuesRuntimeType, icomponentType); return true; @@ -66,6 +68,7 @@ private ComponentSymbols( INamedTypeSymbol cascadingParameterAttribute, INamedTypeSymbol supplyParameterFromFormAttribute, INamedTypeSymbol componentBaseType, + INamedTypeSymbol elementReferenceType, INamedTypeSymbol parameterCaptureUnmatchedValuesRuntimeType, INamedTypeSymbol icomponentType) { @@ -73,6 +76,7 @@ private ComponentSymbols( CascadingParameterAttribute = cascadingParameterAttribute; SupplyParameterFromFormAttribute = supplyParameterFromFormAttribute; // Can be null ComponentBaseType = componentBaseType; // Can be null + ElementReferenceType = elementReferenceType; // Can be null ParameterCaptureUnmatchedValuesRuntimeType = parameterCaptureUnmatchedValuesRuntimeType; IComponentType = icomponentType; } @@ -88,5 +92,7 @@ private ComponentSymbols( public INamedTypeSymbol ComponentBaseType { get; } // Can be null if not available + public INamedTypeSymbol ElementReferenceType { get; } // Can be null if not available + public INamedTypeSymbol IComponentType { get; } } diff --git a/src/Components/Analyzers/src/ComponentsApi.cs b/src/Components/Analyzers/src/ComponentsApi.cs index 361e99b3e146..6ed84d9e1a77 100644 --- a/src/Components/Analyzers/src/ComponentsApi.cs +++ b/src/Components/Analyzers/src/ComponentsApi.cs @@ -40,4 +40,10 @@ public static class IComponent public const string FullTypeName = "Microsoft.AspNetCore.Components.IComponent"; public const string MetadataName = FullTypeName; } + + public static class ElementReference + { + public const string FullTypeName = "Microsoft.AspNetCore.Components.ElementReference"; + public const string MetadataName = FullTypeName; + } } diff --git a/src/Components/Analyzers/src/DiagnosticDescriptors.cs b/src/Components/Analyzers/src/DiagnosticDescriptors.cs index afba1e3acd50..eed7b8e3c874 100644 --- a/src/Components/Analyzers/src/DiagnosticDescriptors.cs +++ b/src/Components/Analyzers/src/DiagnosticDescriptors.cs @@ -83,4 +83,13 @@ internal static class DiagnosticDescriptors DiagnosticSeverity.Warning, isEnabledByDefault: true, description: CreateLocalizableResourceString(nameof(Resources.SupplyParameterFromFormShouldNotHavePropertyInitializer_Description))); + + public static readonly DiagnosticDescriptor ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync = new( + "BL0009", + CreateLocalizableResourceString(nameof(Resources.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync_Title)), + CreateLocalizableResourceString(nameof(Resources.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync_Format)), + Usage, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: CreateLocalizableResourceString(nameof(Resources.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync_Description))); } diff --git a/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs b/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs new file mode 100644 index 000000000000..ae5a9a8a42e0 --- /dev/null +++ b/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs @@ -0,0 +1,123 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Components.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class ElementReferenceUsageAnalyzer : DiagnosticAnalyzer +{ + public ElementReferenceUsageAnalyzer() + { + SupportedDiagnostics = ImmutableArray.Create( + DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync); + } + + public override ImmutableArray SupportedDiagnostics { get; } + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics); + context.RegisterCompilationStartAction(context => + { + if (!ComponentSymbols.TryCreate(context.Compilation, out var symbols)) + { + // Types we need are not defined. + return; + } + + if (symbols.ElementReferenceType == null) + { + // ElementReference type not available. + return; + } + + context.RegisterOperationAction(operationContext => + { + AnalyzeElementReferenceUsage(operationContext, symbols); + }, + OperationKind.FieldReference, + OperationKind.PropertyReference); + }); + } + + private static void AnalyzeElementReferenceUsage(OperationAnalysisContext context, ComponentSymbols symbols) + { + var memberReference = context.Operation as IMemberReferenceOperation; + if (memberReference == null) + { + return; + } + + // Only analyze operations within component types + if (context.ContainingSymbol?.ContainingType is not INamedTypeSymbol containingType || + !ComponentFacts.IsComponent(symbols, context.Compilation, containingType)) + { + return; + } + + var memberSymbol = memberReference.Member; + var memberType = GetMemberType(memberSymbol); + + // Check if the member is of ElementReference type + if (memberType == null || !IsElementReferenceType(memberType, symbols)) + { + return; + } + + // Check if we're in an OnAfterRenderAsync or OnAfterRender method + if (IsInOnAfterRenderMethod(context.Operation)) + { + return; + } + + // Report diagnostic for ElementReference usage outside of OnAfterRenderAsync + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync, + memberReference.Syntax.GetLocation(), + memberSymbol.Name)); + } + + private static bool IsElementReferenceType(ITypeSymbol type, ComponentSymbols symbols) + { + return SymbolEqualityComparer.Default.Equals(type, symbols.ElementReferenceType); + } + + private static ITypeSymbol GetMemberType(ISymbol memberSymbol) + { + return memberSymbol switch + { + IFieldSymbol field => field.Type, + IPropertySymbol property => property.Type, + _ => null + }; + } + + private static bool IsInOnAfterRenderMethod(IOperation operation) + { + // Walk up the operation tree to find the containing method + var current = operation; + while (current != null) + { + // Look for the method symbol in different contexts + if (current.SemanticModel != null) + { + var symbol = current.SemanticModel.GetEnclosingSymbol(current.Syntax.SpanStart); + if (symbol is IMethodSymbol methodSymbol) + { + return methodSymbol.Name == "OnAfterRenderAsync" || methodSymbol.Name == "OnAfterRender"; + } + } + + current = current.Parent; + } + + return false; + } +} \ No newline at end of file diff --git a/src/Components/Analyzers/src/Resources.resx b/src/Components/Analyzers/src/Resources.resx index a6dc20636cc6..db75c5f6b789 100644 --- a/src/Components/Analyzers/src/Resources.resx +++ b/src/Components/Analyzers/src/Resources.resx @@ -189,4 +189,13 @@ Property with [SupplyParameterFromForm] should not have initializer + + ElementReference instances should only be accessed within OnAfterRenderAsync (or OnAfterRender) to avoid accessing stale references or non-existent elements. + + + ElementReference '{0}' should only be accessed within OnAfterRenderAsync or OnAfterRender + + + ElementReference should only be accessed in OnAfterRenderAsync + \ No newline at end of file diff --git a/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs b/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs new file mode 100644 index 000000000000..2aa2aec6c08d --- /dev/null +++ b/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs @@ -0,0 +1,196 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using TestHelper; + +namespace Microsoft.AspNetCore.Components.Analyzers; + +public class ElementReferenceUsageAnalyzerTest : DiagnosticVerifier +{ + private static readonly string TestDeclarations = $@" + namespace {typeof(ParameterAttribute).Namespace} + {{ + public class {typeof(ParameterAttribute).Name} : System.Attribute + {{ + public bool CaptureUnmatchedValues {{ get; set; }} + }} + + public class {typeof(CascadingParameterAttribute).Name} : System.Attribute + {{ + }} + + public interface {typeof(IComponent).Name} + {{ + }} + + public abstract class ComponentBase : {typeof(IComponent).Name} + {{ + protected virtual void OnInitialized() {{ }} + protected virtual System.Threading.Tasks.Task OnInitializedAsync() => System.Threading.Tasks.Task.CompletedTask; + protected virtual void OnAfterRender(bool firstRender) {{ }} + protected virtual System.Threading.Tasks.Task OnAfterRenderAsync(bool firstRender) => System.Threading.Tasks.Task.CompletedTask; + }} + + public struct ElementReference + {{ + public string Id {{ get; }} + public ElementReference(string id) {{ Id = id; }} + public System.Threading.Tasks.ValueTask FocusAsync() => new System.Threading.Tasks.ValueTask(); + }} + }} +"; + [Fact] + public void ElementReferenceUsageInOnAfterRenderAsync_DoesNotWarn() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + using System.Threading.Tasks; + + class TestComponent : ComponentBase + { + private ElementReference myElement; + + protected override async Task OnAfterRenderAsync(bool firstRender) + { + if (firstRender) + { + await myElement.FocusAsync(); + } + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void ElementReferenceUsageInOnAfterRender_DoesNotWarn() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + + class TestComponent : ComponentBase + { + private ElementReference myElement; + + protected override void OnAfterRender(bool firstRender) + { + if (firstRender) + { + var elementId = myElement.Id; + } + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void ElementReferenceUsageInOnInitialized_Warns() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + + class TestComponent : ComponentBase + { + private ElementReference myElement; + + protected override void OnInitialized() + { + var elementId = myElement.Id; + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync.Id, + Message = "ElementReference 'myElement' should only be accessed within OnAfterRenderAsync or OnAfterRender", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 12, 33) + } + }); + } + + [Fact] + public void ElementReferenceUsageInEventHandler_Warns() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + using System.Threading.Tasks; + + class TestComponent : ComponentBase + { + private ElementReference myElement; + + private async Task HandleClick() + { + await myElement.FocusAsync(); + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync.Id, + Message = "ElementReference 'myElement' should only be accessed within OnAfterRenderAsync or OnAfterRender", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 13, 23) + } + }); + } + + [Fact] + public void ElementReferencePropertyUsage_Warns() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + + class TestComponent : ComponentBase + { + public ElementReference MyElement { get; set; } + + protected override void OnInitialized() + { + var elementId = MyElement.Id; + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync.Id, + Message = "ElementReference 'MyElement' should only be accessed within OnAfterRenderAsync or OnAfterRender", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 12, 33) + } + }); + } + + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return new ElementReferenceUsageAnalyzer(); + } +} \ No newline at end of file From b766fdec9ff0f926b853a07f450107f2e08a1a0c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 Aug 2025 22:29:10 +0000 Subject: [PATCH 3/4] Complete ElementReference analyzer with core functionality and comprehensive tests Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com> --- .../src/ElementReferenceUsageAnalyzer.cs | 7 ---- .../test/ElementReferenceUsageAnalyzerTest.cs | 34 +++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs b/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs index ae5a9a8a42e0..f21c05b6452e 100644 --- a/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs +++ b/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs @@ -55,13 +55,6 @@ private static void AnalyzeElementReferenceUsage(OperationAnalysisContext contex return; } - // Only analyze operations within component types - if (context.ContainingSymbol?.ContainingType is not INamedTypeSymbol containingType || - !ComponentFacts.IsComponent(symbols, context.Compilation, containingType)) - { - return; - } - var memberSymbol = memberReference.Member; var memberType = GetMemberType(memberSymbol); diff --git a/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs b/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs index 2aa2aec6c08d..30e8a5ba7de0 100644 --- a/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs +++ b/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs @@ -29,6 +29,8 @@ public abstract class ComponentBase : {typeof(IComponent).Name} {{ protected virtual void OnInitialized() {{ }} protected virtual System.Threading.Tasks.Task OnInitializedAsync() => System.Threading.Tasks.Task.CompletedTask; + protected virtual void OnParametersSet() {{ }} + protected virtual System.Threading.Tasks.Task OnParametersSetAsync() => System.Threading.Tasks.Task.CompletedTask; protected virtual void OnAfterRender(bool firstRender) {{ }} protected virtual System.Threading.Tasks.Task OnAfterRenderAsync(bool firstRender) => System.Threading.Tasks.Task.CompletedTask; }} @@ -189,6 +191,38 @@ protected override void OnInitialized() }); } + [Fact] + public void ElementReferenceUsageInOnParametersSet_Warns() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + + class TestComponent : ComponentBase + { + private ElementReference myElement; + + protected override void OnParametersSet() + { + var elementId = myElement.Id; + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync.Id, + Message = "ElementReference 'myElement' should only be accessed within OnAfterRenderAsync or OnAfterRender", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 12, 33) + } + }); + } + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() { return new ElementReferenceUsageAnalyzer(); From 9b53c3e254070c572463d5e1d3882f25c7dd1722 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 Aug 2025 08:52:38 +0000 Subject: [PATCH 4/4] Implement improved ElementReference analyzer with helper method analysis and more efficient registration Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com> --- .../Analyzers/src/DiagnosticDescriptors.cs | 9 + .../src/ElementReferenceUsageAnalyzer.cs | 318 +++++++++++++++--- src/Components/Analyzers/src/Resources.resx | 9 + .../test/ElementReferenceUsageAnalyzerTest.cs | 183 ++++++++++ 4 files changed, 467 insertions(+), 52 deletions(-) diff --git a/src/Components/Analyzers/src/DiagnosticDescriptors.cs b/src/Components/Analyzers/src/DiagnosticDescriptors.cs index eed7b8e3c874..2cd28cd29f09 100644 --- a/src/Components/Analyzers/src/DiagnosticDescriptors.cs +++ b/src/Components/Analyzers/src/DiagnosticDescriptors.cs @@ -92,4 +92,13 @@ internal static class DiagnosticDescriptors DiagnosticSeverity.Warning, isEnabledByDefault: true, description: CreateLocalizableResourceString(nameof(Resources.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync_Description))); + + public static readonly DiagnosticDescriptor ElementReferenceUsageInMethodCalledOutsideOnAfterRender = new( + "BL0010", + CreateLocalizableResourceString(nameof(Resources.ElementReferenceUsageInMethodCalledOutsideOnAfterRender_Title)), + CreateLocalizableResourceString(nameof(Resources.ElementReferenceUsageInMethodCalledOutsideOnAfterRender_Format)), + Usage, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: CreateLocalizableResourceString(nameof(Resources.ElementReferenceUsageInMethodCalledOutsideOnAfterRender_Description))); } diff --git a/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs b/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs index f21c05b6452e..09fe5f09ed08 100644 --- a/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs +++ b/src/Components/Analyzers/src/ElementReferenceUsageAnalyzer.cs @@ -2,8 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Immutable; +using System.Collections.Generic; +using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; @@ -15,7 +18,8 @@ public class ElementReferenceUsageAnalyzer : DiagnosticAnalyzer public ElementReferenceUsageAnalyzer() { SupportedDiagnostics = ImmutableArray.Create( - DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync); + DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync, + DiagnosticDescriptors.ElementReferenceUsageInMethodCalledOutsideOnAfterRender); } public override ImmutableArray SupportedDiagnostics { get; } @@ -24,9 +28,10 @@ public override void Initialize(AnalysisContext context) { context.EnableConcurrentExecution(); context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics); - context.RegisterCompilationStartAction(context => + + context.RegisterCompilationStartAction(compilationContext => { - if (!ComponentSymbols.TryCreate(context.Compilation, out var symbols)) + if (!ComponentSymbols.TryCreate(compilationContext.Compilation, out var symbols)) { // Types we need are not defined. return; @@ -38,79 +43,288 @@ public override void Initialize(AnalysisContext context) return; } - context.RegisterOperationAction(operationContext => + // Register analysis for component types only + compilationContext.RegisterSymbolStartAction(symbolStartContext => { - AnalyzeElementReferenceUsage(operationContext, symbols); - }, - OperationKind.FieldReference, - OperationKind.PropertyReference); + var namedTypeSymbol = (INamedTypeSymbol)symbolStartContext.Symbol; + + // Only analyze types that implement IComponent + if (!IsComponentType(namedTypeSymbol, symbols)) + { + return; + } + + var analyzer = new ComponentAnalyzer(symbols); + symbolStartContext.RegisterOperationAction(analyzer.AnalyzeOperation, + OperationKind.FieldReference, + OperationKind.PropertyReference, + OperationKind.Invocation); + symbolStartContext.RegisterSymbolEndAction(analyzer.AnalyzeSymbolEnd); + + }, SymbolKind.NamedType); }); } - private static void AnalyzeElementReferenceUsage(OperationAnalysisContext context, ComponentSymbols symbols) + private static bool IsComponentType(INamedTypeSymbol namedTypeSymbol, ComponentSymbols symbols) { - var memberReference = context.Operation as IMemberReferenceOperation; - if (memberReference == null) + if (symbols.IComponentType == null) { - return; + return false; } - var memberSymbol = memberReference.Member; - var memberType = GetMemberType(memberSymbol); - - // Check if the member is of ElementReference type - if (memberType == null || !IsElementReferenceType(memberType, symbols)) + return namedTypeSymbol.AllInterfaces.Any(i => SymbolEqualityComparer.Default.Equals(i, symbols.IComponentType)); + } + + private class ComponentAnalyzer + { + private readonly ComponentSymbols _symbols; + private readonly HashSet _methodsWithElementReferenceAccess = new(); + private readonly HashSet _methodsCalledFromOnAfterRender = new(); + private readonly Dictionary> _elementReferenceAccesses = new(); + private readonly Dictionary> _methodInvocations = new(); + + public ComponentAnalyzer(ComponentSymbols symbols) { - return; + _symbols = symbols; } - // Check if we're in an OnAfterRenderAsync or OnAfterRender method - if (IsInOnAfterRenderMethod(context.Operation)) + public void AnalyzeOperation(OperationAnalysisContext context) { - return; + var containingMethod = GetContainingMethod(context.Operation); + if (containingMethod == null) + { + return; + } + + switch (context.Operation) + { + case IMemberReferenceOperation memberRef: + AnalyzeMemberReference(memberRef, containingMethod); + break; + case IInvocationOperation invocation: + AnalyzeMethodInvocation(invocation, containingMethod); + break; + } } - // Report diagnostic for ElementReference usage outside of OnAfterRenderAsync - context.ReportDiagnostic(Diagnostic.Create( - DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync, - memberReference.Syntax.GetLocation(), - memberSymbol.Name)); - } + private void AnalyzeMemberReference(IMemberReferenceOperation memberRef, IMethodSymbol containingMethod) + { + var memberSymbol = memberRef.Member; + var memberType = GetMemberType(memberSymbol); + + // Check if the member is of ElementReference type + if (memberType == null || !IsElementReferenceType(memberType)) + { + return; + } - private static bool IsElementReferenceType(ITypeSymbol type, ComponentSymbols symbols) - { - return SymbolEqualityComparer.Default.Equals(type, symbols.ElementReferenceType); - } + // Skip if this is part of AddElementReferenceCapture call (BuildRenderTree) + if (IsElementReferenceCaptureCall(memberRef)) + { + return; + } - private static ITypeSymbol GetMemberType(ISymbol memberSymbol) - { - return memberSymbol switch + // Track this method as having ElementReference access + _methodsWithElementReferenceAccess.Add(containingMethod); + + if (!_elementReferenceAccesses.TryGetValue(containingMethod, out var accesses)) + { + accesses = new List<(IOperation, ISymbol)>(); + _elementReferenceAccesses[containingMethod] = accesses; + } + accesses.Add((memberRef, memberSymbol)); + } + + private void AnalyzeMethodInvocation(IInvocationOperation invocation, IMethodSymbol containingMethod) { - IFieldSymbol field => field.Type, - IPropertySymbol property => property.Type, - _ => null - }; - } + var targetMethod = invocation.TargetMethod; + + // Only track method calls within the same type + if (!SymbolEqualityComparer.Default.Equals(targetMethod.ContainingType, containingMethod.ContainingType)) + { + return; + } - private static bool IsInOnAfterRenderMethod(IOperation operation) - { - // Walk up the operation tree to find the containing method - var current = operation; - while (current != null) + if (!_methodInvocations.TryGetValue(containingMethod, out var invocations)) + { + invocations = new List<(IOperation, IMethodSymbol)>(); + _methodInvocations[containingMethod] = invocations; + } + invocations.Add((invocation, targetMethod)); + } + + public void AnalyzeSymbolEnd(SymbolAnalysisContext context) + { + // First, identify methods called from OnAfterRender/OnAfterRenderAsync + PropagateOnAfterRenderContext(); + + // Now report diagnostics + foreach (var methodWithElementRef in _methodsWithElementReferenceAccess) + { + // Skip OnAfterRender methods themselves - they are always safe + if (IsOnAfterRenderMethod(methodWithElementRef)) + { + continue; + } + + // Check if this method is called from anywhere outside OnAfterRender + var isCalledFromOutsideOnAfterRender = IsMethodCalledFromOutsideOnAfterRender(methodWithElementRef); + + if (isCalledFromOutsideOnAfterRender) + { + // Report diagnostic for helper method called outside OnAfterRender + if (_elementReferenceAccesses.TryGetValue(methodWithElementRef, out var accesses)) + { + foreach (var (operation, elementRef) in accesses) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.ElementReferenceUsageInMethodCalledOutsideOnAfterRender, + operation.Syntax.GetLocation(), + methodWithElementRef.Name, + elementRef.Name)); + } + } + } + else + { + // This method is not called from outside OnAfterRender contexts. + // Check if it's actually called from anywhere, or if it's a standalone method. + var isCalledFromAnywhere = _methodInvocations.Values.Any(invocations => + invocations.Any(call => SymbolEqualityComparer.Default.Equals(call.calledMethod, methodWithElementRef))); + + if (!isCalledFromAnywhere) + { + // Method is not called from anywhere, so report standard diagnostic for direct access + if (_elementReferenceAccesses.TryGetValue(methodWithElementRef, out var accesses)) + { + foreach (var (operation, elementRef) in accesses) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync, + operation.Syntax.GetLocation(), + elementRef.Name)); + } + } + } + // If the method is called from somewhere but not from outside OnAfterRender, then it's safe + } + } + } + + private void PropagateOnAfterRenderContext() { - // Look for the method symbol in different contexts - if (current.SemanticModel != null) + var visited = new HashSet(); + var toVisit = new Queue(); + + // Start with OnAfterRender methods + foreach (var method in _methodInvocations.Keys.Concat(_methodsWithElementReferenceAccess)) { - var symbol = current.SemanticModel.GetEnclosingSymbol(current.Syntax.SpanStart); - if (symbol is IMethodSymbol methodSymbol) + if (IsOnAfterRenderMethod(method)) { - return methodSymbol.Name == "OnAfterRenderAsync" || methodSymbol.Name == "OnAfterRender"; + toVisit.Enqueue(method); + _methodsCalledFromOnAfterRender.Add(method); } } - - current = current.Parent; + + // Propagate to methods called from OnAfterRender + while (toVisit.Count > 0) + { + var currentMethod = toVisit.Dequeue(); + if (visited.Contains(currentMethod)) + { + continue; + } + visited.Add(currentMethod); + + if (_methodInvocations.TryGetValue(currentMethod, out var invocations)) + { + foreach (var (_, calledMethod) in invocations) + { + if (!_methodsCalledFromOnAfterRender.Contains(calledMethod)) + { + _methodsCalledFromOnAfterRender.Add(calledMethod); + toVisit.Enqueue(calledMethod); + } + } + } + } + } + + private bool IsMethodCalledFromOutsideOnAfterRender(IMethodSymbol method) + { + // Check if the method is called from any method that is not in the OnAfterRender context + foreach (var invocationKvp in _methodInvocations) + { + var callingMethod = invocationKvp.Key; + var calledMethods = invocationKvp.Value; + + if (calledMethods.Any(call => SymbolEqualityComparer.Default.Equals(call.calledMethod, method))) + { + // This method is called by callingMethod + if (!IsOnAfterRenderMethod(callingMethod) && !_methodsCalledFromOnAfterRender.Contains(callingMethod)) + { + return true; + } + } + } + return false; } - return false; + private static bool IsElementReferenceCaptureCall(IMemberReferenceOperation memberRef) + { + // Check if this member reference is part of an AddElementReferenceCapture call + var parent = memberRef.Parent; + while (parent != null) + { + if (parent is IInvocationOperation invocation) + { + var methodName = invocation.TargetMethod.Name; + if (methodName == "AddElementReferenceCapture") + { + return true; + } + } + parent = parent.Parent; + } + return false; + } + + private bool IsElementReferenceType(ITypeSymbol type) + { + return SymbolEqualityComparer.Default.Equals(type, _symbols.ElementReferenceType); + } + + private static ITypeSymbol GetMemberType(ISymbol memberSymbol) + { + return memberSymbol switch + { + IFieldSymbol field => field.Type, + IPropertySymbol property => property.Type, + _ => null + }; + } + + private static IMethodSymbol GetContainingMethod(IOperation operation) + { + var current = operation; + while (current != null) + { + if (current.SemanticModel != null) + { + var symbol = current.SemanticModel.GetEnclosingSymbol(current.Syntax.SpanStart); + if (symbol is IMethodSymbol methodSymbol) + { + return methodSymbol; + } + } + current = current.Parent; + } + return null; + } + + private static bool IsOnAfterRenderMethod(IMethodSymbol methodSymbol) + { + return methodSymbol.Name == "OnAfterRenderAsync" || methodSymbol.Name == "OnAfterRender"; + } } } \ No newline at end of file diff --git a/src/Components/Analyzers/src/Resources.resx b/src/Components/Analyzers/src/Resources.resx index db75c5f6b789..e191ee931c81 100644 --- a/src/Components/Analyzers/src/Resources.resx +++ b/src/Components/Analyzers/src/Resources.resx @@ -198,4 +198,13 @@ ElementReference should only be accessed in OnAfterRenderAsync + + Methods that access ElementReference instances should only be called from within OnAfterRenderAsync (or OnAfterRender) to avoid accessing stale references or non-existent elements. + + + Method '{0}' accesses ElementReference '{1}' and is being invoked outside of OnAfterRenderAsync or OnAfterRender + + + Method accessing ElementReference called outside OnAfterRenderAsync + \ No newline at end of file diff --git a/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs b/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs index 30e8a5ba7de0..d6371f46f8be 100644 --- a/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs +++ b/src/Components/Analyzers/test/ElementReferenceUsageAnalyzerTest.cs @@ -223,6 +223,189 @@ protected override void OnParametersSet() }); } + [Fact] + public void ElementReferenceUsageInHelperMethodCalledFromOnAfterRender_DoesNotWarn() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + using System.Threading.Tasks; + + class TestComponent : ComponentBase + { + private ElementReference myElement; + + protected override async Task OnAfterRenderAsync(bool firstRender) + { + if (firstRender) + { + await Helper(); + } + } + + private async Task Helper() + { + await myElement.FocusAsync(); + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void ElementReferenceUsageInHelperMethodCalledFromEventHandler_Warns() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + using System.Threading.Tasks; + + class TestComponent : ComponentBase + { + private ElementReference myElement; + + private async Task HandleClick() + { + await Helper(); + } + + private async Task Helper() + { + await myElement.FocusAsync(); + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ElementReferenceUsageInMethodCalledOutsideOnAfterRender.Id, + Message = "Method 'Helper' accesses ElementReference 'myElement' and is being invoked outside of OnAfterRenderAsync or OnAfterRender", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 18, 23) + } + }); + } + + [Fact] + public void ElementReferenceUsageInHelperMethodCalledFromBothSafeAndUnsafeContexts_Warns() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + using System.Threading.Tasks; + + class TestComponent : ComponentBase + { + private ElementReference myElement; + + protected override async Task OnAfterRenderAsync(bool firstRender) + { + if (firstRender) + { + await Helper(); // Safe call + } + } + + private async Task HandleClick() + { + await Helper(); // Unsafe call + } + + private async Task Helper() + { + await myElement.FocusAsync(); + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ElementReferenceUsageInMethodCalledOutsideOnAfterRender.Id, + Message = "Method 'Helper' accesses ElementReference 'myElement' and is being invoked outside of OnAfterRenderAsync or OnAfterRender", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 26, 23) + } + }); + } + + [Fact] + public void ElementReferenceUsageInChainedHelperMethods_DoesNotWarn() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + using System.Threading.Tasks; + + class TestComponent : ComponentBase + { + private ElementReference myElement; + + protected override async Task OnAfterRenderAsync(bool firstRender) + { + if (firstRender) + { + await HelperLevel1(); + } + } + + private async Task HelperLevel1() + { + await HelperLevel2(); + } + + private async Task HelperLevel2() + { + await myElement.FocusAsync(); + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void ElementReferenceUsageInRegularMethod_StillWarns() + { + var test = @" + namespace ConsoleApplication1 + { + using Microsoft.AspNetCore.Components; + + class TestComponent : ComponentBase + { + private ElementReference myElement; + + public void SomePublicMethod() + { + var elementId = myElement.Id; // Should warn with standard diagnostic + } + } + }" + TestDeclarations; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ElementReferenceShouldOnlyBeAccessedInOnAfterRenderAsync.Id, + Message = "ElementReference 'myElement' should only be accessed within OnAfterRenderAsync or OnAfterRender", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 12, 33) + } + }); + } + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() { return new ElementReferenceUsageAnalyzer();