diff --git a/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs b/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs index 59ca05fade26..ce082e363696 100644 --- a/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs +++ b/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs @@ -5,14 +5,19 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Threading; +using System.Threading.Tasks; 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 class ComponentParameterAnalyzer : DiagnosticAnalyzer +public sealed class ComponentParameterAnalyzer : DiagnosticAnalyzer { public ComponentParameterAnalyzer() { @@ -22,6 +27,7 @@ public ComponentParameterAnalyzer() DiagnosticDescriptors.ComponentParameterSettersShouldBePublic, DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesMustBeUnique, DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesHasWrongType, + DiagnosticDescriptors.ComponentParametersShouldBeAutoProperies, }); } @@ -104,6 +110,13 @@ public override void Initialize(AnalysisContext context) symbols.ParameterCaptureUnmatchedValuesRuntimeType.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); } } + if (!IsAutoProperty(property) && !IsSameSemanticAsAutoProperty(property, context.CancellationToken)) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.ComponentParametersShouldBeAutoProperies, + propertyLocation, + property.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); + } } // Check if the type defines multiple CaptureUnmatchedValues parameters. Doing this outside the loop means we place the @@ -123,4 +136,79 @@ public override void Initialize(AnalysisContext context) }, SymbolKind.NamedType); }); } + + /// + /// Check if a property is an auto-property. + /// TODO: Remove this helper when https://github.com/dotnet/roslyn/issues/46682 is handled. + /// + private static bool IsAutoProperty(IPropertySymbol propertySymbol) + => propertySymbol.ContainingType.GetMembers() + .OfType() + .Any(f => f.IsImplicitlyDeclared && SymbolEqualityComparer.Default.Equals(propertySymbol, f.AssociatedSymbol)); + + private static bool IsSameSemanticAsAutoProperty(IPropertySymbol symbol, CancellationToken cancellationToken) + { + if (symbol.DeclaringSyntaxReferences.Length == 1 && + symbol.DeclaringSyntaxReferences[0].GetSyntax(cancellationToken) is PropertyDeclarationSyntax syntax && + syntax.AccessorList?.Accessors.Count == 2) + { + var getterAccessor = syntax.AccessorList.Accessors[0]; + var setterAccessor = syntax.AccessorList.Accessors[1]; + if (getterAccessor.IsKind(SyntaxKind.SetAccessorDeclaration)) + { + // Swap if necessary. + (getterAccessor, setterAccessor) = (setterAccessor, getterAccessor); + } + + if (!getterAccessor.IsKind(SyntaxKind.GetAccessorDeclaration) || !setterAccessor.IsKind(SyntaxKind.SetAccessorDeclaration)) + { + return false; + } + + IdentifierNameSyntax? identifierUsedInGetter = GetIdentifierUsedInGetter(getterAccessor); + if (identifierUsedInGetter is null) + { + return false; + } + + IdentifierNameSyntax? identifierUsedInSetter = GetIdentifierUsedInSetter(setterAccessor); + return identifierUsedInGetter.Identifier.ValueText == identifierUsedInSetter?.Identifier.ValueText; + } + + return false; + } + + private static IdentifierNameSyntax? GetIdentifierUsedInGetter(AccessorDeclarationSyntax getter) + { + if (getter.Body is { Statements: { Count: 1 } } && getter.Body.Statements[0] is ReturnStatementSyntax returnStatement) + { + return returnStatement.Expression as IdentifierNameSyntax; + } + + return getter.ExpressionBody?.Expression as IdentifierNameSyntax; + } + + private static IdentifierNameSyntax? GetIdentifierUsedInSetter(AccessorDeclarationSyntax setter) + { + AssignmentExpressionSyntax? assignmentExpression = null; + if (setter.Body is not null) + { + if (setter.Body.Statements.Count == 1) + { + assignmentExpression = (setter.Body.Statements[0] as ExpressionStatementSyntax)?.Expression as AssignmentExpressionSyntax; + } + } + else + { + assignmentExpression = setter.ExpressionBody?.Expression as AssignmentExpressionSyntax; + } + + if (assignmentExpression is not null && assignmentExpression.Right is IdentifierNameSyntax right && + right.Identifier.ValueText == "value") + { + return assignmentExpression.Left as IdentifierNameSyntax; + } + + return null; + } } diff --git a/src/Components/Analyzers/src/DiagnosticDescriptors.cs b/src/Components/Analyzers/src/DiagnosticDescriptors.cs index 884fd2b2ee32..4a483cae764c 100644 --- a/src/Components/Analyzers/src/DiagnosticDescriptors.cs +++ b/src/Components/Analyzers/src/DiagnosticDescriptors.cs @@ -8,10 +8,6 @@ namespace Microsoft.AspNetCore.Components.Analyzers; [System.Diagnostics.CodeAnalysis.SuppressMessage("MicrosoftCodeAnalysisReleaseTracking", "RS2008:Enable analyzer release tracking")] internal static class DiagnosticDescriptors { - // Note: The Razor Compiler (including Components features) use the RZ prefix for diagnostics, so there's currently - // no change of clashing between that and the BL prefix used here. - // - // Tracking https://github.com/dotnet/aspnetcore/issues/10382 to rationalize this public static readonly DiagnosticDescriptor ComponentParameterSettersShouldBePublic = new DiagnosticDescriptor( "BL0001", new LocalizableResourceString(nameof(Resources.ComponentParameterSettersShouldBePublic_Title), Resources.ResourceManager, typeof(Resources)), @@ -65,4 +61,12 @@ internal static class DiagnosticDescriptors DiagnosticSeverity.Warning, isEnabledByDefault: true, description: new LocalizableResourceString(nameof(Resources.DoNotUseRenderTreeTypes_Description), Resources.ResourceManager, typeof(Resources))); + + public static readonly DiagnosticDescriptor ComponentParametersShouldBeAutoProperies = new( + "BL0007", + new LocalizableResourceString(nameof(Resources.ComponentParametersShouldBeAutoProperties_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.ComponentParametersShouldBeAutoProperties_Message), Resources.ResourceManager, typeof(Resources)), + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); } diff --git a/src/Components/Analyzers/src/Resources.resx b/src/Components/Analyzers/src/Resources.resx index ed4f36c53195..5ef9f54b53eb 100644 --- a/src/Components/Analyzers/src/Resources.resx +++ b/src/Components/Analyzers/src/Resources.resx @@ -174,4 +174,10 @@ Do not use RenderTree types + + Component parameter '{0}' should be auto property + + + Component parameters should be auto properties + \ No newline at end of file diff --git a/src/Components/Analyzers/test/ComponentParameterShouldBeAutoPropertiesTest.cs b/src/Components/Analyzers/test/ComponentParameterShouldBeAutoPropertiesTest.cs new file mode 100644 index 000000000000..e80d27ddada3 --- /dev/null +++ b/src/Components/Analyzers/test/ComponentParameterShouldBeAutoPropertiesTest.cs @@ -0,0 +1,152 @@ +// 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.Diagnostics; +using TestHelper; +using Xunit; + +namespace Microsoft.AspNetCore.Components.Analyzers.Tests; + +public class ComponentParameterShouldBeAutoPropertiesTest : DiagnosticVerifier +{ + [Fact] + public void IsAutoProperty_NoDiagnostic() + { + var source = @" +using Microsoft.AspNetCore.Components + +public class C +{ +[Parameter] +public string MyProp { get; set; } + +[Parameter] +public string MyProp2 { set; get; } +} +" + ComponentsTestDeclarations.Source; + VerifyCSharpDiagnostic(source); + } + + [Fact] + public void HaveSameSemanticAsAutoProperty_NoDiagnostic() + { + var source = @" +using Microsoft.AspNetCore.Components + +public class C +{ +private string _myProp; +private string _myProp2; +private string _myProp3; + +[Parameter] +public string MyProp +{ + get => _myProp; + set => _myProp = value; +} + +[Parameter] +public string MyProp2 +{ + set => _myProp2 = value; + get => _myProp2; +} + +[Parameter] +public string MyProp3 +{ + get + { + return _myProp3; + } + set + { + _myProp3 = value; + } +} +} +" + ComponentsTestDeclarations.Source; + VerifyCSharpDiagnostic(source); + } + + [Fact] + public void HaveLogicInSetter_Diagnostic() + { + var source = @" +using Microsoft.AspNetCore.Components + +public class C +{ +private string _myProp; + +[Parameter] +public string MyProp +{ + get + { + return _myProp; + } + set + { + DoSomething(); + _myProp = value; + } +} + +private void DoSomething() { } +} +" + ComponentsTestDeclarations.Source; + VerifyCSharpDiagnostic(source, new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParametersShouldBeAutoProperies.Id, + Message = "Component parameter 'C.MyProp' should be auto property", + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 9, 15), + }, + Severity = CodeAnalysis.DiagnosticSeverity.Warning, + }); + } + + [Fact] + public void HaveLogicInGetter_Diagnostic() + { + var source = @" +using Microsoft.AspNetCore.Components + +public class C +{ +private string _myProp; + +[Parameter] +public string MyProp +{ + get + { + DoSomething(); + return _myProp; + } + set + { + _myProp = value; + } +} + +private void DoSomething() { } +} +" + ComponentsTestDeclarations.Source; + VerifyCSharpDiagnostic(source, new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParametersShouldBeAutoProperies.Id, + Message = "Component parameter 'C.MyProp' should be auto property", + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 9, 15), + }, + Severity = CodeAnalysis.DiagnosticSeverity.Warning, + }); + } + + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() => new ComponentParameterAnalyzer(); +}