diff --git a/src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs b/src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs index 2c5c6a5f64e8..c08ccab04956 100644 --- a/src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs +++ b/src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs @@ -121,7 +121,7 @@ public void HandlePropertyReference(IPropertyReferenceOperation op) case OperationKind.PropertyReference: { var propertyRef = (IPropertyReferenceOperation)instance; - if (CanUpgrade(propertyRef.Property, false)) + if (CanUpgrade(propertyRef.Property)) { RecordVirtualDispatch(propertyRef.Property, targetMethod); } @@ -182,7 +182,7 @@ public void HandleInvocation(IInvocationOperation op) case OperationKind.PropertyReference: { var propertyRef = (IPropertyReferenceOperation)instance; - if (CanUpgrade(propertyRef.Property, false)) + if (CanUpgrade(propertyRef.Property)) { RecordVirtualDispatch(propertyRef.Property, op.TargetMethod); } @@ -343,7 +343,7 @@ public void HandlePropertyInitializer(IPropertyInitializerOperation op) { foreach (var property in op.InitializedProperties) { - if (CanUpgrade(property, false)) + if (CanUpgrade(property)) { RecordAssignment(property, valueType); } @@ -399,7 +399,7 @@ public void HandleReturn(IReturnOperation op) { if (methodSym.AssociatedSymbol is IPropertySymbol propertySym) { - if (CanUpgrade(propertySym, false)) + if (CanUpgrade(propertySym)) { var valueTypes = GetValueTypes(op.ReturnedValue); foreach (var valueType in valueTypes) @@ -725,15 +725,16 @@ private bool CanUpgrade(IFieldSymbol fieldSym) /// /// Trivial reject for properties that can't be upgraded in order to avoid wasted work. /// - private bool CanUpgrade(IPropertySymbol propSym, bool setter) + private bool CanUpgrade(IPropertySymbol propSym) { - var m = setter ? propSym.SetMethod! : propSym.GetMethod!; + var m = propSym.GetMethod!; return _checkVisibility!(m) && !m.IsImplementationOfAnyInterfaceMember() && !m.IsOverride && !m.IsVirtual - && m.PartialDefinitionPart == null; + && m.PartialDefinitionPart == null && + m.DeclaredAccessibility >= (propSym.SetMethod?.DeclaredAccessibility ?? default); } private void RecordVirtualDispatch(IFieldSymbol field, IMethodSymbol target) => VirtualDispatchFields.GetOrAdd(field, _ => PooledConcurrentSet.GetInstance(SymbolEqualityComparer.Default)).Add(target); diff --git a/src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeTests.cs b/src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeTests.cs index 5e15e693402d..e9e8865cdcae 100644 --- a/src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeTests.cs +++ b/src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeTests.cs @@ -1,9 +1,8 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. -using System.Threading.Tasks; +using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis.Testing; using Test.Utilities; -using Xunit; using VerifyCS = Test.Utilities.CSharpCodeFixVerifier< Microsoft.NetCore.Analyzers.Performance.UseConcreteTypeAnalyzer, Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; @@ -427,6 +426,45 @@ public void M(C1 c1, IFoo f) await TestCSAsync(Source, $"dotnet_code_quality.CA1859.api_surface = public,private,internal"); } + [Fact] + [WorkItem(50362, "https://github.com/dotnet/roslyn-analyzers/issues/50362")] + public static async Task ShouldNotTrigger_PropertyWithSetterMoreAccessibleThanGetter() + { + await TestCSAsync(@" + using System; + using System.Threading.Tasks; + + public class Class1 + { + public I Prop { private get; set; } = new Impl1(); + + void M() + { + Prop.M(); + Prop.MyProperty = 1; + Prop[1] = 1; + Prop.Evt += (s, e) => { }; + } + } + + public interface I + { + int MyProperty { get; set; } + int this[int i] { get; set; } + event EventHandler Evt; + void M(); + } + + public class Impl1 : I + { + public int MyProperty { get; set; } + public int this[int i] { get => i; set { } } + public event EventHandler Evt; + public void M() { } + } + "); + } + [Theory] [InlineData("private", "", true)] [InlineData("private", "private", true)] @@ -1502,7 +1540,7 @@ private void M(ValueTuple vt) await TestCSAsync(Source); } - private static async Task TestCSAsync(string source, params DiagnosticResult[] diagnosticResults) + private static async Task TestCSAsync([StringSyntax("C#-test")] string source, params DiagnosticResult[] diagnosticResults) { var test = new VerifyCS.Test { @@ -1515,7 +1553,7 @@ private static async Task TestCSAsync(string source, params DiagnosticResult[] d await test.RunAsync(); } - private static async Task TestCSAsync(string source, string editorConfigText, params DiagnosticResult[] diagnosticResults) + private static async Task TestCSAsync([StringSyntax("C#-test")] string source, string editorConfigText, params DiagnosticResult[] diagnosticResults) { var test = new VerifyCS.Test {