diff --git a/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/ComponentParametersShouldNotBePublicAnalyzer.cs b/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/ComponentParametersShouldNotBePublicAnalyzer.cs index 95a3881d72..a9ef40f12c 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/ComponentParametersShouldNotBePublicAnalyzer.cs +++ b/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/ComponentParametersShouldNotBePublicAnalyzer.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using Microsoft.AspNetCore.Components.Shared; @@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers .Where(attr => semanticModel.GetTypeInfo(attr).Type?.ToDisplayString() == ComponentsApi.ParameterAttribute.FullTypeName) .FirstOrDefault(); - if (parameterAttribute != null && IsPublic(declaration)) + if (parameterAttribute != null && IsPubliclySettable(declaration)) { var identifierText = declaration.Identifier.Text; if (!string.IsNullOrEmpty(identifierText)) @@ -60,7 +60,17 @@ namespace Microsoft.AspNetCore.Components.Analyzers } } - private static bool IsPublic(PropertyDeclarationSyntax declaration) - => declaration.Modifiers.Any(m => m.IsKind(SyntaxKind.PublicKeyword)); + private static bool IsPubliclySettable(PropertyDeclarationSyntax declaration) + { + // If the property has a setter explicitly marked private/protected/internal, then it's not public + var setter = declaration.AccessorList?.Accessors.SingleOrDefault(x => x.Keyword.IsKind(SyntaxKind.SetKeyword)); + if (setter != null && setter.Modifiers.Any(x => x.IsKind(SyntaxKind.PrivateKeyword) || x.IsKind(SyntaxKind.ProtectedKeyword) || x.IsKind(SyntaxKind.InternalKeyword))) + { + return false; + } + + // Otherwise fallback to the property declaration modifiers + return declaration.Modifiers.Any(x => x.IsKind(SyntaxKind.PublicKeyword)); + } } } diff --git a/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/Resources.Designer.cs b/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/Resources.Designer.cs index 6c62a9818f..df4ae809b0 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/Resources.Designer.cs +++ b/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/Resources.Designer.cs @@ -62,7 +62,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers { } /// - /// Looks up a localized string similar to Component parameters should not be public.. + /// Looks up a localized string similar to Component parameters should not have public setters.. /// internal static string ComponentParametersShouldNotBePublic_Description { get { @@ -80,7 +80,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers { } /// - /// Looks up a localized string similar to Component parameter '{0}' is marked public, but component parameters should not be public.. + /// Looks up a localized string similar to Component parameter '{0}' has a public setter, but component parameters should not be publicly settable.. /// internal static string ComponentParametersShouldNotBePublic_Format { get { @@ -89,7 +89,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers { } /// - /// Looks up a localized string similar to Component parameter is marked public. + /// Looks up a localized string similar to Component parameter has public setter. /// internal static string ComponentParametersShouldNotBePublic_Title { get { diff --git a/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/Resources.resx b/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/Resources.resx index 64ed4fa4a5..ec781527eb 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/Resources.resx +++ b/src/Components/src/Microsoft.AspNetCore.Components.Analyzers/Resources.resx @@ -118,15 +118,15 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - Component parameters should not be public. + Component parameters should not have public setters. Make component parameter private - Component parameter '{0}' is marked public, but component parameters should not be public. + Component parameter '{0}' has a public setter, but component parameters should not be publicly settable. - Component parameter is marked public + Component parameter has public setter \ No newline at end of file diff --git a/src/Components/test/Microsoft.AspNetCore.Components.Analyzers.Test/ComponentParametersShouldNotBePublicTest.cs b/src/Components/test/Microsoft.AspNetCore.Components.Analyzers.Test/ComponentParametersShouldNotBePublicTest.cs index 832c37b315..e79bcf12aa 100644 --- a/src/Components/test/Microsoft.AspNetCore.Components.Analyzers.Test/ComponentParametersShouldNotBePublicTest.cs +++ b/src/Components/test/Microsoft.AspNetCore.Components.Analyzers.Test/ComponentParametersShouldNotBePublicTest.cs @@ -75,7 +75,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test new DiagnosticResult { Id = "BL9993", - Message = "Component parameter 'BadProperty1' is marked public, but component parameters should not be public.", + Message = "Component parameter 'BadProperty1' has a public setter, but component parameters should not be publicly settable.", Severity = DiagnosticSeverity.Warning, Locations = new[] { @@ -85,7 +85,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test new DiagnosticResult { Id = "BL9993", - Message = "Component parameter 'BadProperty2' is marked public, but component parameters should not be public.", + Message = "Component parameter 'BadProperty2' has a public setter, but component parameters should not be publicly settable.", Severity = DiagnosticSeverity.Warning, Locations = new[] { @@ -106,6 +106,25 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test }" + BlazorParameterSource); } + [Fact] + public void IgnoresPublicPropertiesWithNonPublicSetterWithParameterAttribute() + { + var test = @" + namespace ConsoleApplication1 + { + using " + typeof(ParameterAttribute).Namespace + @"; + + class TypeName + { + [Parameter] public string MyProperty1 { get; private set; } + [Parameter] public object MyProperty2 { get; protected set; } + [Parameter] public object MyProperty2 { get; internal set; } + } + }" + BlazorParameterSource; + + VerifyCSharpDiagnostic(test); + } + protected override CodeFixProvider GetCSharpCodeFixProvider() { return new ComponentParametersShouldNotBePublicCodeFixProvider();