diff --git a/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs b/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs index 20ad4ab6bd..0ebc54df4a 100644 --- a/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs +++ b/src/Components/Analyzers/src/ComponentParameterAnalyzer.cs @@ -18,7 +18,8 @@ namespace Microsoft.AspNetCore.Components.Analyzers { SupportedDiagnostics = ImmutableArray.Create(new[] { - DiagnosticDescriptors.ComponentParametersShouldNotBePublic, + DiagnosticDescriptors.ComponentParametersShouldBePublic, + DiagnosticDescriptors.ComponentParameterSettersShouldBePublic, DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesMustBeUnique, DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesHasWrongType, }); @@ -61,14 +62,27 @@ namespace Microsoft.AspNetCore.Components.Analyzers { var captureUnmatchedValuesParameters = new List(); - // Per-property validations - foreach (var property in properties) + // Per-property validations + foreach (var property in properties) { - if (property.SetMethod?.DeclaredAccessibility == Accessibility.Public) + var propertyLocation = property.Locations.FirstOrDefault(); + if (propertyLocation == null) + { + continue; + } + + if (property.DeclaredAccessibility != Accessibility.Public) { context.ReportDiagnostic(Diagnostic.Create( - DiagnosticDescriptors.ComponentParametersShouldNotBePublic, - property.Locations[0], + DiagnosticDescriptors.ComponentParametersShouldBePublic, + propertyLocation, + property.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); + } + else if (property.SetMethod?.DeclaredAccessibility != Accessibility.Public) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.ComponentParameterSettersShouldBePublic, + propertyLocation, property.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); } @@ -76,13 +90,13 @@ namespace Microsoft.AspNetCore.Components.Analyzers { captureUnmatchedValuesParameters.Add(property); - // Check the type, we need to be able to assign a Dictionary - var conversion = context.Compilation.ClassifyConversion(symbols.ParameterCaptureUnmatchedValuesRuntimeType, property.Type); + // Check the type, we need to be able to assign a Dictionary + var conversion = context.Compilation.ClassifyConversion(symbols.ParameterCaptureUnmatchedValuesRuntimeType, property.Type); if (!conversion.Exists || conversion.IsExplicit) { context.ReportDiagnostic(Diagnostic.Create( DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesHasWrongType, - property.Locations[0], + propertyLocation, property.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat), property.Type.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat), symbols.ParameterCaptureUnmatchedValuesRuntimeType.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); @@ -90,9 +104,9 @@ namespace Microsoft.AspNetCore.Components.Analyzers } } - // Check if the type defines multiple CaptureUnmatchedValues parameters. Doing this outside the loop means we place the - // errors on the type. - if (captureUnmatchedValuesParameters.Count > 1) + // Check if the type defines multiple CaptureUnmatchedValues parameters. Doing this outside the loop means we place the + // errors on the type. + if (captureUnmatchedValuesParameters.Count > 1) { context.ReportDiagnostic(Diagnostic.Create( DiagnosticDescriptors.ComponentParameterCaptureUnmatchedValuesMustBeUnique, diff --git a/src/Components/Analyzers/src/ComponentParametersShouldNotBePublicCodeFixProvider.cs b/src/Components/Analyzers/src/ComponentParametersShouldBePublicCodeFixProvider.cs similarity index 71% rename from src/Components/Analyzers/src/ComponentParametersShouldNotBePublicCodeFixProvider.cs rename to src/Components/Analyzers/src/ComponentParametersShouldBePublicCodeFixProvider.cs index 84555969b9..173633f0e9 100644 --- a/src/Components/Analyzers/src/ComponentParametersShouldNotBePublicCodeFixProvider.cs +++ b/src/Components/Analyzers/src/ComponentParametersShouldBePublicCodeFixProvider.cs @@ -13,13 +13,13 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; namespace Microsoft.AspNetCore.Components.Analyzers { - [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ComponentParametersShouldNotBePublicCodeFixProvider)), Shared] - public class ComponentParametersShouldNotBePublicCodeFixProvider : CodeFixProvider + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ComponentParametersShouldBePublicCodeFixProvider)), Shared] + public class ComponentParametersShouldBePublicCodeFixProvider : CodeFixProvider { - private static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_FixTitle), Resources.ResourceManager, typeof(Resources)); + private static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.ComponentParametersShouldBePublic_FixTitle), Resources.ResourceManager, typeof(Resources)); public override ImmutableArray FixableDiagnosticIds - => ImmutableArray.Create(DiagnosticDescriptors.ComponentParametersShouldNotBePublic.Id); + => ImmutableArray.Create(DiagnosticDescriptors.ComponentParametersShouldBePublic.Id); public sealed override FixAllProvider GetFixAllProvider() { @@ -64,9 +64,24 @@ namespace Microsoft.AspNetCore.Components.Analyzers return null; } - var publicModifier = node.Modifiers.FirstOrDefault(m => m.IsKind(SyntaxKind.PublicKeyword)); - node = node.WithModifiers( - node.Modifiers.Remove(publicModifier)); + var newModifiers = node.Modifiers; + for (var i = 0; i < node.Modifiers.Count; i++) + { + var modifier = node.Modifiers[i]; + if (modifier.IsKind(SyntaxKind.PrivateKeyword) || + modifier.IsKind(SyntaxKind.ProtectedKeyword) || + modifier.IsKind(SyntaxKind.InternalKeyword) || + + // We also remove public in case the user has written something totally backwards such as private public protected Foo + modifier.IsKind(SyntaxKind.PublicKeyword)) + { + newModifiers = newModifiers.Remove(modifier); + } + } + + var publicModifier = SyntaxFactory.Token(SyntaxKind.PublicKeyword); + newModifiers = newModifiers.Insert(0, publicModifier); + node = node.WithModifiers(newModifiers); return node; } } diff --git a/src/Components/Analyzers/src/DiagnosticDescriptors.cs b/src/Components/Analyzers/src/DiagnosticDescriptors.cs index cb380e5646..74632adc6f 100644 --- a/src/Components/Analyzers/src/DiagnosticDescriptors.cs +++ b/src/Components/Analyzers/src/DiagnosticDescriptors.cs @@ -11,14 +11,14 @@ namespace Microsoft.AspNetCore.Components.Analyzers // no change of clashing between that and the BL prefix used here. // // Tracking https://github.com/aspnet/AspNetCore/issues/10382 to rationalize this - public static readonly DiagnosticDescriptor ComponentParametersShouldNotBePublic = new DiagnosticDescriptor( + public static readonly DiagnosticDescriptor ComponentParameterSettersShouldBePublic = new DiagnosticDescriptor( "BL0001", - new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_Title), Resources.ResourceManager, typeof(Resources)), - new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_Format), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.ComponentParameterSettersShouldBePublic_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.ComponentParameterSettersShouldBePublic_Format), Resources.ResourceManager, typeof(Resources)), "Encapsulation", DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: new LocalizableResourceString(nameof(Resources.ComponentParametersShouldNotBePublic_Description), Resources.ResourceManager, typeof(Resources))); + description: new LocalizableResourceString(nameof(Resources.ComponentParameterSettersShouldBePublic_Description), Resources.ResourceManager, typeof(Resources))); public static readonly DiagnosticDescriptor ComponentParameterCaptureUnmatchedValuesMustBeUnique = new DiagnosticDescriptor( "BL0002", @@ -37,5 +37,14 @@ namespace Microsoft.AspNetCore.Components.Analyzers DiagnosticSeverity.Warning, isEnabledByDefault: true, description: new LocalizableResourceString(nameof(Resources.ComponentParameterCaptureUnmatchedValuesHasWrongType_Description), Resources.ResourceManager, typeof(Resources))); + + public static readonly DiagnosticDescriptor ComponentParametersShouldBePublic = new DiagnosticDescriptor( + "BL0004", + new LocalizableResourceString(nameof(Resources.ComponentParameterShouldBePublic_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.ComponentParameterShouldBePublic_Format), Resources.ResourceManager, typeof(Resources)), + "Encapsulation", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: new LocalizableResourceString(nameof(Resources.ComponentParametersShouldBePublic_Description), Resources.ResourceManager, typeof(Resources))); } } diff --git a/src/Components/Analyzers/src/Resources.Designer.cs b/src/Components/Analyzers/src/Resources.Designer.cs index e7f866fe08..13c55946a3 100644 --- a/src/Components/Analyzers/src/Resources.Designer.cs +++ b/src/Components/Analyzers/src/Resources.Designer.cs @@ -61,7 +61,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers { } /// - /// Looks up a localized string similar to Component parameters with CaptureUnmatchedValuess must be a correct type.. + /// Looks up a localized string similar to Component parameters with CaptureUnmatchedValues must be a correct type.. /// internal static string ComponentParameterCaptureUnmatchedValuesHasWrongType_Description { get { @@ -70,7 +70,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers { } /// - /// Looks up a localized string similar to Component parameter '{0}' defines CaptureUnmatchedValuess but has an unsupported type '{1}'. Use a type assignable from '{2}'.. + /// Looks up a localized string similar to Component parameter '{0}' defines CaptureUnmatchedValues but has an unsupported type '{1}'. Use a type assignable from '{2}'.. /// internal static string ComponentParameterCaptureUnmatchedValuesHasWrongType_Format { get { @@ -115,38 +115,65 @@ namespace Microsoft.AspNetCore.Components.Analyzers { } /// - /// Looks up a localized string similar to Component parameters should not have public setters.. + /// Looks up a localized string similar to Component parameters should have public setters.. /// - internal static string ComponentParametersShouldNotBePublic_Description { + internal static string ComponentParameterSettersShouldBePublic_Description { get { - return ResourceManager.GetString("ComponentParametersShouldNotBePublic_Description", resourceCulture); + return ResourceManager.GetString("ComponentParameterSettersShouldBePublic_Description", resourceCulture); } } /// - /// Looks up a localized string similar to Make component parameter private. + /// Looks up a localized string similar to Component parameter '{0}' should have a public setter.. /// - internal static string ComponentParametersShouldNotBePublic_FixTitle { + internal static string ComponentParameterSettersShouldBePublic_Format { get { - return ResourceManager.GetString("ComponentParametersShouldNotBePublic_FixTitle", resourceCulture); + return ResourceManager.GetString("ComponentParameterSettersShouldBePublic_Format", resourceCulture); } } /// - /// Looks up a localized string similar to Component parameter '{0}' has a public setter, but component parameters should not be publicly settable.. + /// Looks up a localized string similar to Component parameter should have public setters.. /// - internal static string ComponentParametersShouldNotBePublic_Format { + internal static string ComponentParameterSettersShouldBePublic_Title { get { - return ResourceManager.GetString("ComponentParametersShouldNotBePublic_Format", resourceCulture); + return ResourceManager.GetString("ComponentParameterSettersShouldBePublic_Title", resourceCulture); } } /// - /// Looks up a localized string similar to Component parameter has public setter. + /// Looks up a localized string similar to Component parameter '{0}' should be public.. /// - internal static string ComponentParametersShouldNotBePublic_Title { + internal static string ComponentParameterShouldBePublic_Format { get { - return ResourceManager.GetString("ComponentParametersShouldNotBePublic_Title", resourceCulture); + return ResourceManager.GetString("ComponentParameterShouldBePublic_Format", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Component parameter should be public.. + /// + internal static string ComponentParameterShouldBePublic_Title { + get { + return ResourceManager.GetString("ComponentParameterShouldBePublic_Title", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Component parameters should be public.. + /// + internal static string ComponentParametersShouldBePublic_Description { + get { + return ResourceManager.GetString("ComponentParametersShouldBePublic_Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Make component parameters public.. + /// + internal static string ComponentParametersShouldBePublic_FixTitle { + get { + return ResourceManager.GetString("ComponentParametersShouldBePublic_FixTitle", resourceCulture); } } } diff --git a/src/Components/Analyzers/src/Resources.resx b/src/Components/Analyzers/src/Resources.resx index 2996ec3082..1c4319cb5b 100644 --- a/src/Components/Analyzers/src/Resources.resx +++ b/src/Components/Analyzers/src/Resources.resx @@ -1,17 +1,17 @@ - @@ -117,17 +117,14 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - Component parameters should not have public setters. + + Component parameters should have public setters. - - Make component parameter private + + Component parameter '{0}' should have a public setter. - - Component parameter '{0}' has a public setter, but component parameters should not be publicly settable. - - - Component parameter has public setter + + Component parameter should have public setters. Components may only define a single parameter with CaptureUnmatchedValues. @@ -147,4 +144,16 @@ Component parameter with CaptureUnmatchedValues has the wrong type - + + Component parameter '{0}' should be public. + + + Component parameter should be public. + + + Component parameters should be public. + + + Make component parameters public. + + \ No newline at end of file diff --git a/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesHasWrongTypeTest.cs b/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesHasWrongTypeTest.cs index ba547ff85b..e4673c3747 100644 --- a/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesHasWrongTypeTest.cs +++ b/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesHasWrongTypeTest.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test using {typeof(ParameterAttribute).Namespace}; class TypeName {{ - [Parameter(CaptureUnmatchedValues = true)] {propertyType} MyProperty {{ get; set; }} + [Parameter(CaptureUnmatchedValues = true)] public {propertyType} MyProperty {{ get; set; }} }} }}" + ComponentsTestDeclarations.Source; @@ -39,7 +39,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test using {typeof(ParameterAttribute).Namespace}; class TypeName {{ - [Parameter(CaptureUnmatchedValues = false)] string MyProperty {{ get; set; }} + [Parameter(CaptureUnmatchedValues = false)] public string MyProperty {{ get; set; }} }} }}" + ComponentsTestDeclarations.Source; @@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test using {typeof(ParameterAttribute).Namespace}; class TypeName {{ - [Parameter(CaptureUnmatchedValues = true)] string MyProperty {{ get; set; }} + [Parameter(CaptureUnmatchedValues = true)] public string MyProperty {{ get; set; }} }} }}" + ComponentsTestDeclarations.Source; @@ -67,7 +67,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test Severity = DiagnosticSeverity.Warning, Locations = new[] { - new DiagnosticResultLocation("Test0.cs", 7, 63) + new DiagnosticResultLocation("Test0.cs", 7, 70) } }); } diff --git a/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesMustBeUniqueTest.cs b/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesMustBeUniqueTest.cs index 7e2c919287..f7b055caa7 100644 --- a/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesMustBeUniqueTest.cs +++ b/src/Components/Analyzers/test/ComponentParameterCaptureUnmatchedValuesMustBeUniqueTest.cs @@ -21,8 +21,8 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test using {typeof(ParameterAttribute).Namespace}; class TypeName {{ - [Parameter(CaptureUnmatchedValues = false)] string MyProperty {{ get; set; }} - [Parameter(CaptureUnmatchedValues = true)] Dictionary MyOtherProperty {{ get; set; }} + [Parameter(CaptureUnmatchedValues = false)] public string MyProperty {{ get; set; }} + [Parameter(CaptureUnmatchedValues = true)] public Dictionary MyOtherProperty {{ get; set; }} }} }}" + ComponentsTestDeclarations.Source; @@ -39,8 +39,8 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test using {typeof(ParameterAttribute).Namespace}; class TypeName {{ - [Parameter(CaptureUnmatchedValues = true)] Dictionary MyProperty {{ get; set; }} - [Parameter(CaptureUnmatchedValues = true)] Dictionary MyOtherProperty {{ get; set; }} + [Parameter(CaptureUnmatchedValues = true)] public Dictionary MyProperty {{ get; set; }} + [Parameter(CaptureUnmatchedValues = true)] public Dictionary MyOtherProperty {{ get; set; }} }} }}" + ComponentsTestDeclarations.Source; diff --git a/src/Components/Analyzers/test/ComponentParameterSettersShouldBePublicTest.cs b/src/Components/Analyzers/test/ComponentParameterSettersShouldBePublicTest.cs new file mode 100644 index 0000000000..d58d1cfd14 --- /dev/null +++ b/src/Components/Analyzers/test/ComponentParameterSettersShouldBePublicTest.cs @@ -0,0 +1,95 @@ +// 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.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using TestHelper; +using Xunit; + +namespace Microsoft.AspNetCore.Components.Analyzers +{ + public class ComponentParameterSettersShouldBePublicTest : DiagnosticVerifier + { + [Fact] + public void IgnoresPublicSettersProperties() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + [Parameter] public string MyProperty {{ get; set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresPrivateSettersNonParameterProperties() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + private string MyProperty {{ get; private set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void ErrorsForNonPublicSetterParameters() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + [Parameter] public string MyProperty1 {{ get; private set; }} + [Parameter] public string MyProperty2 {{ get; protected set; }} + [Parameter] public string MyProperty3 {{ get; internal set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParameterSettersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty1' should have a public setter.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 7, 39) + } + }, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParameterSettersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty2' should have a public setter.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 8, 39) + } + }, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParameterSettersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty3' should have a public setter.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 9, 39) + } + }); + } + + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() => new ComponentParameterAnalyzer(); + } +} diff --git a/src/Components/Analyzers/test/ComponentParametersShouldNotBePublicTest.cs b/src/Components/Analyzers/test/ComponentParametersShouldBePublicCodeFixProviderTest.cs similarity index 55% rename from src/Components/Analyzers/test/ComponentParametersShouldNotBePublicTest.cs rename to src/Components/Analyzers/test/ComponentParametersShouldBePublicCodeFixProviderTest.cs index 8d5df9053f..4cf55844aa 100644 --- a/src/Components/Analyzers/test/ComponentParametersShouldNotBePublicTest.cs +++ b/src/Components/Analyzers/test/ComponentParametersShouldBePublicCodeFixProviderTest.cs @@ -9,17 +9,17 @@ using Xunit; namespace Microsoft.AspNetCore.Components.Analyzers.Test { - public class ComponentParametersShouldNotBePublic : CodeFixVerifier + public class ComponentParametersShouldBePublicCodeFixProviderTest : CodeFixVerifier { [Fact] - public void IgnoresPublicPropertiesWithoutParameterAttribute() + public void IgnoresPrivatePropertiesWithoutParameterAttribute() { var test = @" namespace ConsoleApplication1 { class TypeName { - public string MyProperty { get; set; } + private string MyProperty { get; set; } } }" + ComponentsTestDeclarations.Source; @@ -27,7 +27,7 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test } [Fact] - public void IgnoresNonpublicPropertiesWithParameterAttribute() + public void AddsDiagnosticAndFixForPrivatePropertiesWithParameterAttribute() { var test = @" namespace ConsoleApplication1 @@ -36,50 +36,30 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test class TypeName { - [Parameter] string MyPropertyNoModifer { get; set; } - [Parameter] private string MyPropertyPrivate { get; set; } - [CascadingParameter] protected string MyPropertyProtected { get; set; } - [CascadingParameter] internal string MyPropertyInternal { get; set; } - } - }" + ComponentsTestDeclarations.Source; - - VerifyCSharpDiagnostic(test); - } - - [Fact] - public void AddsDiagnosticAndFixForPublicPropertiesWithParameterAttribute() - { - var test = @" - namespace ConsoleApplication1 - { - using " + typeof(ParameterAttribute).Namespace + @"; - - class TypeName - { - [Parameter] public string BadProperty1 { get; set; } - [CascadingParameter] public object BadProperty2 { get; set; } + [Parameter] private string BadProperty1 { get; set; } + [CascadingParameter] private object BadProperty2 { get; set; } } }" + ComponentsTestDeclarations.Source; VerifyCSharpDiagnostic(test, new DiagnosticResult { - Id = DiagnosticDescriptors.ComponentParametersShouldNotBePublic.Id, - Message = "Component parameter 'ConsoleApplication1.TypeName.BadProperty1' has a public setter, but component parameters should not be publicly settable.", + Id = DiagnosticDescriptors.ComponentParametersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.BadProperty1' should be public.", Severity = DiagnosticSeverity.Warning, Locations = new[] { - new DiagnosticResultLocation("Test0.cs", 8, 39) + new DiagnosticResultLocation("Test0.cs", 8, 40) } }, new DiagnosticResult { - Id = DiagnosticDescriptors.ComponentParametersShouldNotBePublic.Id, - Message = "Component parameter 'ConsoleApplication1.TypeName.BadProperty2' has a public setter, but component parameters should not be publicly settable.", + Id = DiagnosticDescriptors.ComponentParametersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.BadProperty2' should be public.", Severity = DiagnosticSeverity.Warning, Locations = new[] { - new DiagnosticResultLocation("Test0.cs", 9, 48) + new DiagnosticResultLocation("Test0.cs", 9, 49) } }); @@ -90,8 +70,8 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test class TypeName { - [Parameter] string BadProperty1 { get; set; } - [CascadingParameter] object BadProperty2 { get; set; } + [Parameter] public string BadProperty1 { get; set; } + [CascadingParameter] public object BadProperty2 { get; set; } } }" + ComponentsTestDeclarations.Source); } @@ -108,16 +88,46 @@ namespace Microsoft.AspNetCore.Components.Analyzers.Test { [Parameter] public string MyProperty1 { get; private set; } [Parameter] public object MyProperty2 { get; protected set; } - [Parameter] public object MyProperty2 { get; internal set; } + [Parameter] public object MyProperty3 { get; internal set; } } }" + ComponentsTestDeclarations.Source; - VerifyCSharpDiagnostic(test); + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParameterSettersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty1' should have a public setter.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 8, 39) + } + }, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParameterSettersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty2' should have a public setter.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 9, 39) + } + }, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParameterSettersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty3' should have a public setter.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 10, 39) + } + }); } protected override CodeFixProvider GetCSharpCodeFixProvider() { - return new ComponentParametersShouldNotBePublicCodeFixProvider(); + return new ComponentParametersShouldBePublicCodeFixProvider(); } protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() diff --git a/src/Components/Analyzers/test/ComponentParametersShouldBePublicTest.cs b/src/Components/Analyzers/test/ComponentParametersShouldBePublicTest.cs new file mode 100644 index 0000000000..2f03b9ea31 --- /dev/null +++ b/src/Components/Analyzers/test/ComponentParametersShouldBePublicTest.cs @@ -0,0 +1,106 @@ +// 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.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using TestHelper; +using Xunit; + +namespace Microsoft.AspNetCore.Components.Analyzers +{ + public class ComponentParametersShouldBePublicTest : DiagnosticVerifier + { + [Fact] + public void IgnoresPublicProperties() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + [Parameter] public string MyProperty {{ get; set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void IgnoresPrivateNonParameterProperties() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + private string MyProperty {{ get; set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + VerifyCSharpDiagnostic(test); + } + + [Fact] + public void ErrorsForNonPublicParameters() + { + var test = $@" + namespace ConsoleApplication1 + {{ + using {typeof(ParameterAttribute).Namespace}; + class TypeName + {{ + [Parameter] string MyProperty1 {{ get; set; }} + [Parameter] private string MyProperty2 {{ get; set; }} + [Parameter] protected string MyProperty3 {{ get; set; }} + [Parameter] internal string MyProperty4 {{ get; set; }} + }} + }}" + ComponentsTestDeclarations.Source; + + VerifyCSharpDiagnostic(test, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParametersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty1' should be public.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 7, 32) + } + }, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParametersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty2' should be public.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 8, 40) + } + }, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParametersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty3' should be public.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 9, 42) + } + }, + new DiagnosticResult + { + Id = DiagnosticDescriptors.ComponentParametersShouldBePublic.Id, + Message = "Component parameter 'ConsoleApplication1.TypeName.MyProperty4' should be public.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] + { + new DiagnosticResultLocation("Test0.cs", 10, 41) + } + }); + } + + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() => new ComponentParameterAnalyzer(); + } +}