diff --git a/src/Components/Analyzers/src/ComponentFacts.cs b/src/Components/Analyzers/src/ComponentFacts.cs index f7f976dce6dc..debd00db09ee 100644 --- a/src/Components/Analyzers/src/ComponentFacts.cs +++ b/src/Components/Analyzers/src/ComponentFacts.cs @@ -107,6 +107,26 @@ public static bool IsSupplyParameterFromForm(ComponentSymbols symbols, IProperty return property.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, symbols.SupplyParameterFromFormAttribute)); } + public static bool IsPersistentState(ComponentSymbols symbols, IPropertySymbol property) + { + if (symbols == null) + { + throw new ArgumentNullException(nameof(symbols)); + } + + if (property == null) + { + throw new ArgumentNullException(nameof(property)); + } + + if (symbols.PersistentStateAttribute == null) + { + return false; + } + + return property.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, symbols.PersistentStateAttribute)); + } + public static bool IsComponentBase(ComponentSymbols symbols, INamedTypeSymbol type) { if (symbols is null) diff --git a/src/Components/Analyzers/src/ComponentSymbols.cs b/src/Components/Analyzers/src/ComponentSymbols.cs index e28da74f28d8..52b9c175ea27 100644 --- a/src/Components/Analyzers/src/ComponentSymbols.cs +++ b/src/Components/Analyzers/src/ComponentSymbols.cs @@ -47,14 +47,16 @@ public static bool TryCreate(Compilation compilation, out ComponentSymbols symbo var parameterCaptureUnmatchedValuesRuntimeType = dictionary.Construct(@string, @object); - // Try to get optional symbols for SupplyParameterFromForm analyzer + // Try to get optional symbols for SupplyParameterFromForm and PersistentState analyzers var supplyParameterFromFormAttribute = compilation.GetTypeByMetadataName(ComponentsApi.SupplyParameterFromFormAttribute.MetadataName); + var persistentStateAttribute = compilation.GetTypeByMetadataName(ComponentsApi.PersistentStateAttribute.MetadataName); var componentBaseType = compilation.GetTypeByMetadataName(ComponentsApi.ComponentBase.MetadataName); symbols = new ComponentSymbols( parameterAttribute, cascadingParameterAttribute, supplyParameterFromFormAttribute, + persistentStateAttribute, componentBaseType, parameterCaptureUnmatchedValuesRuntimeType, icomponentType); @@ -65,6 +67,7 @@ private ComponentSymbols( INamedTypeSymbol parameterAttribute, INamedTypeSymbol cascadingParameterAttribute, INamedTypeSymbol supplyParameterFromFormAttribute, + INamedTypeSymbol persistentStateAttribute, INamedTypeSymbol componentBaseType, INamedTypeSymbol parameterCaptureUnmatchedValuesRuntimeType, INamedTypeSymbol icomponentType) @@ -72,6 +75,7 @@ private ComponentSymbols( ParameterAttribute = parameterAttribute; CascadingParameterAttribute = cascadingParameterAttribute; SupplyParameterFromFormAttribute = supplyParameterFromFormAttribute; // Can be null + PersistentStateAttribute = persistentStateAttribute; // Can be null ComponentBaseType = componentBaseType; // Can be null ParameterCaptureUnmatchedValuesRuntimeType = parameterCaptureUnmatchedValuesRuntimeType; IComponentType = icomponentType; @@ -86,6 +90,8 @@ private ComponentSymbols( public INamedTypeSymbol SupplyParameterFromFormAttribute { get; } // Can be null if not available + public INamedTypeSymbol PersistentStateAttribute { get; } // Can be null if not available + public INamedTypeSymbol ComponentBaseType { 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..a2dc5b852c6d 100644 --- a/src/Components/Analyzers/src/ComponentsApi.cs +++ b/src/Components/Analyzers/src/ComponentsApi.cs @@ -35,6 +35,12 @@ public static class ComponentBase public const string MetadataName = FullTypeName; } + public static class PersistentStateAttribute + { + public const string FullTypeName = "Microsoft.AspNetCore.Components.PersistentStateAttribute"; + public const string MetadataName = FullTypeName; + } + public static class IComponent { public const string FullTypeName = "Microsoft.AspNetCore.Components.IComponent"; diff --git a/src/Components/Analyzers/src/DiagnosticDescriptors.cs b/src/Components/Analyzers/src/DiagnosticDescriptors.cs index afba1e3acd50..5f67edaf8447 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 PersistentStateShouldNotHavePropertyInitializer = new( + "BL0009", + CreateLocalizableResourceString(nameof(Resources.PersistentStateShouldNotHavePropertyInitializer_Title)), + CreateLocalizableResourceString(nameof(Resources.PersistentStateShouldNotHavePropertyInitializer_Format)), + Usage, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: CreateLocalizableResourceString(nameof(Resources.PersistentStateShouldNotHavePropertyInitializer_Description))); } diff --git a/src/Components/Analyzers/src/PersistentStateAnalyzer.cs b/src/Components/Analyzers/src/PersistentStateAnalyzer.cs new file mode 100644 index 000000000000..8cd361302d3c --- /dev/null +++ b/src/Components/Analyzers/src/PersistentStateAnalyzer.cs @@ -0,0 +1,101 @@ +// 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 System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +#nullable enable + +namespace Microsoft.AspNetCore.Components.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class PersistentStateAnalyzer : DiagnosticAnalyzer +{ + public PersistentStateAnalyzer() + { + SupportedDiagnostics = ImmutableArray.Create( + DiagnosticDescriptors.PersistentStateShouldNotHavePropertyInitializer); + } + + 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; + } + + context.RegisterSyntaxNodeAction(context => + { + var propertyDeclaration = (PropertyDeclarationSyntax)context.Node; + + // Check if property has an initializer + if (propertyDeclaration.Initializer == null) + { + return; + } + + // Ignore initializers that set to default values (null, default, etc.) + if (IsDefaultValueInitializer(propertyDeclaration.Initializer.Value)) + { + return; + } + + var propertySymbol = context.SemanticModel.GetDeclaredSymbol(propertyDeclaration); + if (propertySymbol == null) + { + return; + } + + // Check if property has [PersistentState] attribute + if (!ComponentFacts.IsPersistentState(symbols, propertySymbol)) + { + return; + } + + // Check if the containing type inherits from ComponentBase + var containingType = propertySymbol.ContainingType; + if (!ComponentFacts.IsComponentBase(symbols, containingType)) + { + return; + } + + var propertyLocation = propertySymbol.Locations.FirstOrDefault(); + if (propertyLocation != null) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.PersistentStateShouldNotHavePropertyInitializer, + propertyLocation, + propertySymbol.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); + } + }, SyntaxKind.PropertyDeclaration); + }); + } + + private static bool IsDefaultValueInitializer(ExpressionSyntax expression) + { + return expression switch + { + // null + LiteralExpressionSyntax { Token.ValueText: "null" } => true, + // null! + PostfixUnaryExpressionSyntax { Operand: LiteralExpressionSyntax { Token.ValueText: "null" }, OperatorToken.ValueText: "!" } => true, + // default + LiteralExpressionSyntax literal when literal.Token.IsKind(SyntaxKind.DefaultKeyword) => true, + // default! + PostfixUnaryExpressionSyntax { Operand: LiteralExpressionSyntax literal, OperatorToken.ValueText: "!" } + when literal.Token.IsKind(SyntaxKind.DefaultKeyword) => true, + _ => 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..6a23211094aa 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 + + The value of a property decorated with [PersistentState] and initialized with a property initializer can be overwritten when the component receives parameters. To ensure the initialized value is not overwritten, move the initialization to a component lifecycle method like OnInitialized or OnInitializedAsync + + + Property '{0}' has [PersistentState] and a property initializer. This can be overwritten during parameter binding. + + + Property with [PersistentState] should not have initializer + \ No newline at end of file diff --git a/src/Components/Analyzers/test/PersistentStateAnalyzerTest.cs b/src/Components/Analyzers/test/PersistentStateAnalyzerTest.cs new file mode 100644 index 000000000000..f95df939f0e5 --- /dev/null +++ b/src/Components/Analyzers/test/PersistentStateAnalyzerTest.cs @@ -0,0 +1,282 @@ +// 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.Test; + +public class PersistentStateAnalyzerTest : DiagnosticVerifier +{ + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() => new PersistentStateAnalyzer(); + + 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 abstract class CascadingParameterAttributeBase : System.Attribute + {{ + }} + + public class PersistentStateAttribute : CascadingParameterAttributeBase + {{ + public RestoreBehavior RestoreBehavior {{ get; set; }} + public bool AllowUpdates {{ get; set; }} + }} + + public enum RestoreBehavior + {{ + Default, + SkipInitialValue, + SkipLastSnapshot + }} + + public interface {typeof(IComponent).Name} + {{ + }} + + public abstract class ComponentBase : {typeof(IComponent).Name} + {{ + }} + }} +"; + + [Fact] + public void IgnoresPropertiesWithoutPersistentStateAttribute() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + public string MyProperty {{ get; set; }} = ""initial-value""; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresPersistentStateWithoutInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [PersistentState] public string MyProperty {{ get; set; }} + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresNonComponentBaseClasses() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class NotAComponent + {{ + [PersistentState] public string MyProperty {{ get; set; }} = ""initial-value""; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void ReportsWarningForPersistentStateWithInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [PersistentState] public string MyProperty {{ get; set; }} = ""initial-value""; + }} + }}" + TestDeclarations; + + var expected = new DiagnosticResult + { + Id = "BL0009", + Message = "Property 'ConsoleApplication1.TestComponent.MyProperty' has [PersistentState] and a property initializer. This can be overwritten during parameter binding.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 7, 45) + } + }; + + VerifyCSharpDiagnostic(test, expected); + } + + [Fact] + public void ReportsWarningForPersistentStateWithObjectInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [PersistentState] public InputModel Input {{ get; set; }} = new InputModel(); + }} + + class InputModel + {{ + public string Value {{ get; set; }} = """"; + }} + }}" + TestDeclarations; + + var expected = new DiagnosticResult + { + Id = "BL0009", + Message = "Property 'ConsoleApplication1.TestComponent.Input' has [PersistentState] and a property initializer. This can be overwritten during parameter binding.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 7, 49) + } + }; + + VerifyCSharpDiagnostic(test, expected); + } + + [Fact] + public void IgnoresPersistentStateWithNullInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [PersistentState] public string MyProperty {{ get; set; }} = null; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresPersistentStateWithNullForgivingInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [PersistentState] public string MyProperty {{ get; set; }} = null!; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresPersistentStateWithDefaultInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [PersistentState] public string MyProperty {{ get; set; }} = default; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresPersistentStateWithDefaultForgivingInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [PersistentState] public string MyProperty {{ get; set; }} = default!; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void WorksWithInheritedComponentBase() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class BaseComponent : ComponentBase + {{ + }} + + class TestComponent : BaseComponent + {{ + [PersistentState] public string MyProperty {{ get; set; }} = ""initial-value""; + }} + }}" + TestDeclarations; + + var expected = new DiagnosticResult + { + Id = "BL0009", + Message = "Property 'ConsoleApplication1.TestComponent.MyProperty' has [PersistentState] and a property initializer. This can be overwritten during parameter binding.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 11, 45) + } + }; + + VerifyCSharpDiagnostic(test, expected); + } + + [Fact] + public void WorksWithPersistentStateAttributeWithParameters() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [PersistentState(RestoreBehavior = RestoreBehavior.SkipInitialValue, AllowUpdates = true)] + public string MyProperty {{ get; set; }} = ""initial-value""; + }} + }}" + TestDeclarations; + + var expected = new DiagnosticResult + { + Id = "BL0009", + Message = "Property 'ConsoleApplication1.TestComponent.MyProperty' has [PersistentState] and a property initializer. This can be overwritten during parameter binding.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 8, 27) + } + }; + + VerifyCSharpDiagnostic(test, expected); + } +} \ No newline at end of file