diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs index e48dd9445bfc..21e7f7954394 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs @@ -248,4 +248,13 @@ internal static class DiagnosticDescriptors DiagnosticSeverity.Info, isEnabledByDefault: true, helpLinkUri: AnalyzersLink); + + internal static readonly DiagnosticDescriptor DoNotUseLocalFunctionsInMarkup = new( + "ASP0029", + CreateLocalizableResourceString(nameof(Resources.Analyzer_DoNotUseLocalFunctionsInMarkup_Title)), + CreateLocalizableResourceString(nameof(Resources.Analyzer_DoNotUseLocalFunctionsInMarkup_Message)), + Usage, + DiagnosticSeverity.Error, + isEnabledByDefault: true, + helpLinkUri: AnalyzersLink); } diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RenderTreeBuilder/RenderTreeBuilderAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RenderTreeBuilder/RenderTreeBuilderAnalyzer.cs index 76c41aa8f066..50ce0911eb45 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RenderTreeBuilder/RenderTreeBuilderAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RenderTreeBuilder/RenderTreeBuilderAnalyzer.cs @@ -2,9 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Immutable; +using System.Linq; using Microsoft.AspNetCore.App.Analyzers.Infrastructure; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; @@ -16,7 +18,9 @@ namespace Microsoft.AspNetCore.Analyzers.RenderTreeBuilder; public partial class RenderTreeBuilderAnalyzer : DiagnosticAnalyzer { private const int SequenceParameterOrdinal = 0; - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(DiagnosticDescriptors.DoNotUseNonLiteralSequenceNumbers); + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create( + DiagnosticDescriptors.DoNotUseNonLiteralSequenceNumbers, + DiagnosticDescriptors.DoNotUseLocalFunctionsInMarkup); public override void Initialize(AnalysisContext context) { @@ -53,6 +57,21 @@ public override void Initialize(AnalysisContext context) } }, OperationKind.Invocation); + + // Register syntax node action to detect local functions + context.RegisterSyntaxNodeAction(context => + { + var localFunction = (LocalFunctionStatementSyntax)context.Node; + + // Check if this local function contains any RenderTreeBuilder method calls + if (ContainsRenderTreeBuilderCalls(wellKnownTypes, localFunction, context.SemanticModel)) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.DoNotUseLocalFunctionsInMarkup, + localFunction.Identifier.GetLocation(), + localFunction.Identifier.ValueText)); + } + }, SyntaxKind.LocalFunctionStatement); }); } @@ -60,4 +79,61 @@ private static bool IsRenderTreeBuilderMethodWithSequenceParameter(WellKnownType => SymbolEqualityComparer.Default.Equals(wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Components_Rendering_RenderTreeBuilder), targetMethod.ContainingType) && targetMethod.Parameters.Length > SequenceParameterOrdinal && targetMethod.Parameters[SequenceParameterOrdinal].Name == "sequence"; + + private static bool ContainsRenderTreeBuilderCalls(WellKnownTypes wellKnownTypes, LocalFunctionStatementSyntax localFunction, SemanticModel semanticModel) + { + var renderTreeBuilderType = wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Components_Rendering_RenderTreeBuilder); + if (renderTreeBuilderType is null) + { + return false; + } + + // Static local functions cannot capture from enclosing scope, so they're safe + if (localFunction.Modifiers.Any(SyntaxKind.StaticKeyword)) + { + return false; + } + + // Walk through all invocation expressions in the local function + var invocations = localFunction.DescendantNodes().OfType(); + + foreach (var invocation in invocations) + { + var symbolInfo = semanticModel.GetSymbolInfo(invocation); + if (symbolInfo.Symbol is IMethodSymbol method && + SymbolEqualityComparer.Default.Equals(renderTreeBuilderType, method.ContainingType)) + { + // Check if this is a call on a captured variable (not a parameter) + if (IsCallOnCapturedRenderTreeBuilder(invocation, localFunction, semanticModel, renderTreeBuilderType)) + { + return true; + } + } + } + + return false; + } + + private static bool IsCallOnCapturedRenderTreeBuilder(InvocationExpressionSyntax invocation, LocalFunctionStatementSyntax localFunction, SemanticModel semanticModel, INamedTypeSymbol _) + { + // Get the expression that the method is being called on + var memberAccess = invocation.Expression as MemberAccessExpressionSyntax; + if (memberAccess is null) + { + return false; + } + + var targetSymbol = semanticModel.GetSymbolInfo(memberAccess.Expression).Symbol; + + // If it's a parameter of the local function, it's not captured + if (targetSymbol is IParameterSymbol parameter) + { + // Check if this parameter belongs to our local function + var localFunctionSymbol = semanticModel.GetDeclaredSymbol(localFunction); + return localFunctionSymbol is not null && !localFunctionSymbol.Parameters.Contains(parameter, SymbolEqualityComparer.Default); + } + + // If it's a local variable or field, it could be captured + return targetSymbol is IFieldSymbol or ILocalSymbol; + } } diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx index 8c9397f5be64..aa67b460dd15 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx @@ -333,4 +333,10 @@ If the server does not specifically reject IPv6, IPAddress.IPv6Any is preferred over IPAddress.Any usage for safety and performance reasons. See https://aka.ms/aspnetcore-warnings/ASP0028 for more details. + + Do not use local functions in markup + + + Local function '{0}' accesses RenderTreeBuilder from parent scope, which can cause incorrect rendering behavior. Consider making it a static method or regular instance method that takes RenderTreeBuilder as a parameter. + diff --git a/src/Framework/AspNetCoreAnalyzers/test/Components/DoNotUseLocalFunctionsInMarkupTest.cs b/src/Framework/AspNetCoreAnalyzers/test/Components/DoNotUseLocalFunctionsInMarkupTest.cs new file mode 100644 index 000000000000..9ac0c488b35c --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/Components/DoNotUseLocalFunctionsInMarkupTest.cs @@ -0,0 +1,197 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Globalization; +using System.Linq; +using Microsoft.AspNetCore.Analyzer.Testing; + +namespace Microsoft.AspNetCore.Analyzers.RenderTreeBuilder; + +public class DoNotUseLocalFunctionsInMarkupTest +{ + private TestDiagnosticAnalyzerRunner Runner { get; } = new(new RenderTreeBuilderAnalyzer()); + + [Fact] + public async Task LocalFunctionWithRenderTreeBuilderCall_ProducesDiagnostic() + { + // Arrange + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Components.Rendering; + +var builder = new RenderTreeBuilder(); + +void /*MM*/LocalFunction() +{ + builder.OpenElement(0, ""div""); + builder.CloseElement(); +} + +LocalFunction(); +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var analyzerDiagnostic = diagnostics.FirstOrDefault(d => d.Descriptor == DiagnosticDescriptors.DoNotUseLocalFunctionsInMarkup); + Assert.NotNull(analyzerDiagnostic); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, analyzerDiagnostic.Location); + Assert.StartsWith("Local function 'LocalFunction' accesses RenderTreeBuilder from parent scope", analyzerDiagnostic.GetMessage(CultureInfo.InvariantCulture)); + } + + [Fact] + public async Task LocalFunctionWithMultipleRenderTreeBuilderCalls_ProducesDiagnostic() + { + // Arrange + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Components.Rendering; + +var builder = new RenderTreeBuilder(); + +void /*MM*/LocalFunction() +{ + builder.OpenElement(0, ""div""); + builder.AddContent(1, ""text""); + builder.CloseElement(); +} + +LocalFunction(); +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var analyzerDiagnostic = diagnostics.FirstOrDefault(d => d.Descriptor == DiagnosticDescriptors.DoNotUseLocalFunctionsInMarkup); + Assert.NotNull(analyzerDiagnostic); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, analyzerDiagnostic.Location); + Assert.StartsWith("Local function 'LocalFunction' accesses RenderTreeBuilder from parent scope", analyzerDiagnostic.GetMessage(CultureInfo.InvariantCulture)); + } + + [Fact] + public async Task LocalFunctionWithoutRenderTreeBuilderCall_NoDiagnostic() + { + // Arrange + var source = @" +void LocalFunction() +{ + var x = 5; + System.Console.WriteLine(x); +} + +LocalFunction(); +"; + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source); + + // Assert + var analyzerDiagnostics = diagnostics.Where(d => d.Descriptor == DiagnosticDescriptors.DoNotUseLocalFunctionsInMarkup); + Assert.Empty(analyzerDiagnostics); + } + + [Fact] + public async Task LocalFunctionWithParameterRenderTreeBuilder_NoDiagnostic() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Components.Rendering; + +void LocalFunction(RenderTreeBuilder builder) +{ + builder.OpenElement(0, ""div""); + builder.CloseElement(); +} + +var builder = new RenderTreeBuilder(); +LocalFunction(builder); +"; + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source); + + // Assert + var analyzerDiagnostics = diagnostics.Where(d => d.Descriptor == DiagnosticDescriptors.DoNotUseLocalFunctionsInMarkup); + Assert.Empty(analyzerDiagnostics); + } + + [Fact] + public async Task NestedLocalFunctionWithRenderTreeBuilderCall_ProducesDiagnostic() + { + // Arrange + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Components.Rendering; + +var builder = new RenderTreeBuilder(); + +void OuterFunction() +{ + void /*MM*/InnerFunction() + { + builder.OpenElement(0, ""div""); + builder.CloseElement(); + } + + InnerFunction(); +} + +OuterFunction(); +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var analyzerDiagnostics = diagnostics.Where(d => d.Descriptor == DiagnosticDescriptors.DoNotUseLocalFunctionsInMarkup).ToList(); + var innerFunctionDiagnostic = analyzerDiagnostics.FirstOrDefault(d => d.GetMessage(CultureInfo.InvariantCulture).Contains("InnerFunction")); + Assert.NotNull(innerFunctionDiagnostic); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, innerFunctionDiagnostic.Location); + Assert.StartsWith("Local function 'InnerFunction' accesses RenderTreeBuilder from parent scope", innerFunctionDiagnostic.GetMessage(CultureInfo.InvariantCulture)); + } + + [Fact] + public async Task StaticLocalFunction_NoDiagnostic() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Components.Rendering; + +var builder = new RenderTreeBuilder(); + +static void LocalFunction(RenderTreeBuilder builderParam) +{ + builderParam.OpenElement(0, ""div""); + builderParam.CloseElement(); +} + +LocalFunction(builder); +"; + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source); + + // Assert + var analyzerDiagnostics = diagnostics.Where(d => d.Descriptor == DiagnosticDescriptors.DoNotUseLocalFunctionsInMarkup); + Assert.Empty(analyzerDiagnostics); + } + + [Fact] + public async Task LocalFunctionWithMethodInvocation_ProducesDiagnostic() + { + // Arrange + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Components.Rendering; + +var builder = new RenderTreeBuilder(); + +void /*MM*/LocalFunction() +{ + builder.AddMarkupContent(0, ""
Hello
""); +} + +LocalFunction(); +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var analyzerDiagnostic = diagnostics.FirstOrDefault(d => d.Descriptor == DiagnosticDescriptors.DoNotUseLocalFunctionsInMarkup); + Assert.NotNull(analyzerDiagnostic); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, analyzerDiagnostic.Location); + Assert.StartsWith("Local function 'LocalFunction' accesses RenderTreeBuilder from parent scope", analyzerDiagnostic.GetMessage(CultureInfo.InvariantCulture)); + } +} \ No newline at end of file