Skip to content

Avoid reporting CA1859 for properties with setter more accessible than getter #50488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Alex-Sob wants to merge 3 commits into from
Closed

Avoid reporting CA1859 for properties with setter more accessible than getter #50488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c49619
Fix and add test
Alex-Sob Aug 26, 2025
0a7dbd2
Merge branch 'main' into dev/alex-sob/CA1859-fix
Alex-Sob Aug 28, 2025
b054382
Merge branch 'main' into dev/alex-sob/CA1859-fix
Alex-Sob Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions ...NetAnalyzers/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -725,15 +725,16 @@ private bool CanUpgrade(IFieldSymbol fieldSym)
/// <summary>
/// Trivial reject for properties that can't be upgraded in order to avoid wasted work.
/// </summary>
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<IMethodSymbol>.GetInstance(SymbolEqualityComparer.Default)).Add(target);
Expand Down
46 changes: 42 additions & 4 deletions ...is.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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>;
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -1502,7 +1540,7 @@ private void M(ValueTuple<Action> 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
{
Expand All @@ -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
{
Expand Down