diff --git a/src/Components/Analyzers/src/ComponentFacts.cs b/src/Components/Analyzers/src/ComponentFacts.cs index 43561128b0a5..f7f976dce6dc 100644 --- a/src/Components/Analyzers/src/ComponentFacts.cs +++ b/src/Components/Analyzers/src/ComponentFacts.cs @@ -87,6 +87,57 @@ public static bool IsCascadingParameter(ComponentSymbols symbols, IPropertySymbo return property.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, symbols.CascadingParameterAttribute)); } + public static bool IsSupplyParameterFromForm(ComponentSymbols symbols, IPropertySymbol property) + { + if (symbols == null) + { + throw new ArgumentNullException(nameof(symbols)); + } + + if (property == null) + { + throw new ArgumentNullException(nameof(property)); + } + + if (symbols.SupplyParameterFromFormAttribute == null) + { + return false; + } + + return property.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, symbols.SupplyParameterFromFormAttribute)); + } + + public static bool IsComponentBase(ComponentSymbols symbols, INamedTypeSymbol type) + { + if (symbols is null) + { + throw new ArgumentNullException(nameof(symbols)); + } + + if (type is null) + { + throw new ArgumentNullException(nameof(type)); + } + + if (symbols.ComponentBaseType == null) + { + return false; + } + + // Check if the type inherits from ComponentBase + var current = type.BaseType; + while (current != null) + { + if (SymbolEqualityComparer.Default.Equals(current, symbols.ComponentBaseType)) + { + return true; + } + current = current.BaseType; + } + + return false; + } + public static bool IsComponent(ComponentSymbols symbols, Compilation compilation, INamedTypeSymbol type) { if (symbols is null) diff --git a/src/Components/Analyzers/src/ComponentSymbols.cs b/src/Components/Analyzers/src/ComponentSymbols.cs index ccdca61d9749..e28da74f28d8 100644 --- a/src/Components/Analyzers/src/ComponentSymbols.cs +++ b/src/Components/Analyzers/src/ComponentSymbols.cs @@ -47,9 +47,15 @@ public static bool TryCreate(Compilation compilation, out ComponentSymbols symbo var parameterCaptureUnmatchedValuesRuntimeType = dictionary.Construct(@string, @object); + // Try to get optional symbols for SupplyParameterFromForm analyzer + var supplyParameterFromFormAttribute = compilation.GetTypeByMetadataName(ComponentsApi.SupplyParameterFromFormAttribute.MetadataName); + var componentBaseType = compilation.GetTypeByMetadataName(ComponentsApi.ComponentBase.MetadataName); + symbols = new ComponentSymbols( parameterAttribute, cascadingParameterAttribute, + supplyParameterFromFormAttribute, + componentBaseType, parameterCaptureUnmatchedValuesRuntimeType, icomponentType); return true; @@ -58,11 +64,15 @@ public static bool TryCreate(Compilation compilation, out ComponentSymbols symbo private ComponentSymbols( INamedTypeSymbol parameterAttribute, INamedTypeSymbol cascadingParameterAttribute, + INamedTypeSymbol supplyParameterFromFormAttribute, + INamedTypeSymbol componentBaseType, INamedTypeSymbol parameterCaptureUnmatchedValuesRuntimeType, INamedTypeSymbol icomponentType) { ParameterAttribute = parameterAttribute; CascadingParameterAttribute = cascadingParameterAttribute; + SupplyParameterFromFormAttribute = supplyParameterFromFormAttribute; // Can be null + ComponentBaseType = componentBaseType; // Can be null ParameterCaptureUnmatchedValuesRuntimeType = parameterCaptureUnmatchedValuesRuntimeType; IComponentType = icomponentType; } @@ -74,5 +84,9 @@ private ComponentSymbols( public INamedTypeSymbol CascadingParameterAttribute { get; } + public INamedTypeSymbol SupplyParameterFromFormAttribute { 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 a62d070dedef..361e99b3e146 100644 --- a/src/Components/Analyzers/src/ComponentsApi.cs +++ b/src/Components/Analyzers/src/ComponentsApi.cs @@ -23,6 +23,18 @@ public static class CascadingParameterAttribute public const string MetadataName = FullTypeName; } + public static class SupplyParameterFromFormAttribute + { + public const string FullTypeName = "Microsoft.AspNetCore.Components.SupplyParameterFromFormAttribute"; + public const string MetadataName = FullTypeName; + } + + public static class ComponentBase + { + public const string FullTypeName = "Microsoft.AspNetCore.Components.ComponentBase"; + 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 ea3332c56fc7..afba1e3acd50 100644 --- a/src/Components/Analyzers/src/DiagnosticDescriptors.cs +++ b/src/Components/Analyzers/src/DiagnosticDescriptors.cs @@ -74,4 +74,13 @@ internal static class DiagnosticDescriptors Usage, DiagnosticSeverity.Warning, isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor SupplyParameterFromFormShouldNotHavePropertyInitializer = new( + "BL0008", + CreateLocalizableResourceString(nameof(Resources.SupplyParameterFromFormShouldNotHavePropertyInitializer_Title)), + CreateLocalizableResourceString(nameof(Resources.SupplyParameterFromFormShouldNotHavePropertyInitializer_Format)), + Usage, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: CreateLocalizableResourceString(nameof(Resources.SupplyParameterFromFormShouldNotHavePropertyInitializer_Description))); } diff --git a/src/Components/Analyzers/src/Resources.resx b/src/Components/Analyzers/src/Resources.resx index 5ef9f54b53eb..a6dc20636cc6 100644 --- a/src/Components/Analyzers/src/Resources.resx +++ b/src/Components/Analyzers/src/Resources.resx @@ -180,4 +180,13 @@ Component parameters should be auto properties + + The value of a property decorated with [SupplyParameterFromForm] and initialized with a property initializer can be overwritten with null 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 [SupplyParameterFromForm] and a property initializer. This can be overwritten with null during form posts. + + + Property with [SupplyParameterFromForm] should not have initializer + \ No newline at end of file diff --git a/src/Components/Analyzers/src/SupplyParameterFromFormAnalyzer.cs b/src/Components/Analyzers/src/SupplyParameterFromFormAnalyzer.cs new file mode 100644 index 000000000000..00bbbcaa1180 --- /dev/null +++ b/src/Components/Analyzers/src/SupplyParameterFromFormAnalyzer.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 SupplyParameterFromFormAnalyzer : DiagnosticAnalyzer +{ + public SupplyParameterFromFormAnalyzer() + { + SupportedDiagnostics = ImmutableArray.Create( + DiagnosticDescriptors.SupplyParameterFromFormShouldNotHavePropertyInitializer); + } + + 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 [SupplyParameterFromForm] attribute + if (!ComponentFacts.IsSupplyParameterFromForm(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.SupplyParameterFromFormShouldNotHavePropertyInitializer, + 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/test/SupplyParameterFromFormAnalyzerTest.cs b/src/Components/Analyzers/test/SupplyParameterFromFormAnalyzerTest.cs new file mode 100644 index 000000000000..549b3cf115ec --- /dev/null +++ b/src/Components/Analyzers/test/SupplyParameterFromFormAnalyzerTest.cs @@ -0,0 +1,243 @@ +// 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 SupplyParameterFromFormAnalyzerTest : DiagnosticVerifier +{ + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() => new SupplyParameterFromFormAnalyzer(); + + 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 class SupplyParameterFromFormAttribute : System.Attribute + {{ + public string Name {{ get; set; }} + public string FormName {{ get; set; }} + }} + + public interface {typeof(IComponent).Name} + {{ + }} + + public abstract class ComponentBase : {typeof(IComponent).Name} + {{ + }} + }} +"; + + [Fact] + public void IgnoresPropertiesWithoutSupplyParameterFromFormAttribute() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + public string MyProperty {{ get; set; }} = ""initial-value""; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresSupplyParameterFromFormWithoutInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [SupplyParameterFromForm] public string MyProperty {{ get; set; }} + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresNonComponentBaseClasses() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class NotAComponent + {{ + [SupplyParameterFromForm] public string MyProperty {{ get; set; }} = ""initial-value""; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void ReportsWarningForSupplyParameterFromFormWithInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [SupplyParameterFromForm] public string MyProperty {{ get; set; }} = ""initial-value""; + }} + }}" + TestDeclarations; + + var expected = new DiagnosticResult + { + Id = "BL0008", + Message = "Property 'ConsoleApplication1.TestComponent.MyProperty' has [SupplyParameterFromForm] and a property initializer. This can be overwritten with null during form posts.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 7, 53) + } + }; + + VerifyCSharpDiagnostic(test, expected); + } + + [Fact] + public void ReportsWarningForSupplyParameterFromFormWithObjectInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [SupplyParameterFromForm] public InputModel Input {{ get; set; }} = new InputModel(); + }} + + class InputModel + {{ + public string Value {{ get; set; }} = """"; + }} + }}" + TestDeclarations; + + var expected = new DiagnosticResult + { + Id = "BL0008", + Message = "Property 'ConsoleApplication1.TestComponent.Input' has [SupplyParameterFromForm] and a property initializer. This can be overwritten with null during form posts.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 7, 57) + } + }; + + VerifyCSharpDiagnostic(test, expected); + } + + [Fact] + public void IgnoresSupplyParameterFromFormWithNullInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [SupplyParameterFromForm] public string MyProperty {{ get; set; }} = null; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresSupplyParameterFromFormWithNullForgivingInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [SupplyParameterFromForm] public string MyProperty {{ get; set; }} = null!; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresSupplyParameterFromFormWithDefaultInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [SupplyParameterFromForm] public string MyProperty {{ get; set; }} = default; + }} + }}" + TestDeclarations; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresSupplyParameterFromFormWithDefaultForgivingInitializer() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TestComponent : ComponentBase + {{ + [SupplyParameterFromForm] 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 + {{ + [SupplyParameterFromForm] public string MyProperty {{ get; set; }} = ""initial-value""; + }} + }}" + TestDeclarations; + + var expected = new DiagnosticResult + { + Id = "BL0008", + Message = "Property 'ConsoleApplication1.TestComponent.MyProperty' has [SupplyParameterFromForm] and a property initializer. This can be overwritten with null during form posts.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 11, 53) + } + }; + + VerifyCSharpDiagnostic(test, expected); + } +} \ No newline at end of file