Skip to content

Commit

Permalink
Implement BL0007: Component parameters should be auto property (#30102)
Browse files Browse the repository at this point in the history
* Implement ASP0002: Use 'ParameterAttribute' correctly


* Attempt to detect full-props that have same semantics as auto-props

Fixes #26230

* Address feedback

* Add tests

* Few fixes

* Address feedback

* Rename test file and fix failing test

* Add unintentionally deleted test file

* Fix trailing spaces

* Update ComponentParameterShouldBeAutoPropertiesTest.cs

* Update src/Components/Analyzers/test/ComponentParameterShouldBeAutoPropertiesTest.cs

* Switch to MIT license

* Update ComponentParameterAnalyzer.cs

* Update ComponentParameterShouldBeAutoPropertiesTest.cs

Co-authored-by: Pranav K <[email protected]>
  • Loading branch information
Youssef1313 and pranavkm authored Feb 3, 2022
1 parent 0d2e5aa commit 417f950
Show file tree
Hide file tree
Showing 4 changed files with 255 additions and 5 deletions.
90 changes: 89 additions & 1 deletion src/Components/Analyzers/src/ComponentParameterAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -22,6 +27,7 @@ public ComponentParameterAnalyzer()
DiagnosticDescriptors.ComponentParameterSettersShouldBePublic,
DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesMustBeUnique,
DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesHasWrongType,
DiagnosticDescriptors.ComponentParametersShouldBeAutoProperies,
});
}

Expand Down Expand Up @@ -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
Expand All @@ -123,4 +136,79 @@ public override void Initialize(AnalysisContext context)
}, SymbolKind.NamedType);
});
}

/// <summary>
/// Check if a property is an auto-property.
/// TODO: Remove this helper when https://github.com/dotnet/roslyn/issues/46682 is handled.
/// </summary>
private static bool IsAutoProperty(IPropertySymbol propertySymbol)
=> propertySymbol.ContainingType.GetMembers()
.OfType<IFieldSymbol>()
.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;
}
}
12 changes: 8 additions & 4 deletions src/Components/Analyzers/src/DiagnosticDescriptors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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);
}
6 changes: 6 additions & 0 deletions src/Components/Analyzers/src/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -174,4 +174,10 @@
<data name="DoNotUseRenderTreeTypes_Title" xml:space="preserve">
<value>Do not use RenderTree types</value>
</data>
<data name="ComponentParametersShouldBeAutoProperties_Message" xml:space="preserve">
<value>Component parameter '{0}' should be auto property</value>
</data>
<data name="ComponentParametersShouldBeAutoProperties_Title" xml:space="preserve">
<value>Component parameters should be auto properties</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -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();
}

0 comments on commit 417f950

Please sign in to comment.