Skip to content

Commit

Permalink
Merge pull request #7133 from geeknoid/main
Browse files Browse the repository at this point in the history 
Fix a number of issues in CA1859.
  • Loading branch information
@mavasani
mavasani authored Jan 17, 2024
2 parents 09c7836 + 37bcec6 commit 23ec029
Show file tree
Hide file tree
Showing 3 changed files with 204 additions and 12 deletions.
76 changes: 75 additions & 1 deletion ...alyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,70 @@ public static void ReturnInstance(Collector c, CancellationToken cancellationTok
}

/// <summary>
/// Identify fields/locals/params that are used as 'this' for a virtual dispatch.
/// Identify properties that are used as 'this' for a virtual dispatch.
/// </summary>
public void HandlePropertyReference(IPropertyReferenceOperation op)
{
if (op.Property.IsAbstract || op.Property.IsVirtual)
{
if (op.Instance != null)
{
var targetMethod = op.Property.GetMethod ?? op.Property.SetMethod!;

var instance = op.Instance;
if (instance.Kind == OperationKind.ConditionalAccessInstance)
{
var parent = ((IConditionalAccessInstanceOperation)instance).GetConditionalAccess() ?? instance;
if (parent != null)
{
instance = ((IConditionalAccessOperation)parent).Operation;
}
}

switch (instance.Kind)
{
case OperationKind.FieldReference:
{
var fieldRef = (IFieldReferenceOperation)instance;
if (CanUpgrade(fieldRef.Field))
{
RecordVirtualDispatch(fieldRef.Field, targetMethod);
}

break;
}

case OperationKind.PropertyReference:
{
var propertyRef = (IPropertyReferenceOperation)instance;
if (CanUpgrade(propertyRef.Property, false))
{
RecordVirtualDispatch(propertyRef.Property, targetMethod);
}

break;
}

case OperationKind.LocalReference:
{
var localRef = (ILocalReferenceOperation)instance;
RecordVirtualDispatch(localRef.Local, targetMethod);
break;
}

case OperationKind.ParameterReference:
{
var parameterRef = (IParameterReferenceOperation)instance;
RecordVirtualDispatch(parameterRef.Parameter, targetMethod);
break;
}
}
}
}
}

/// <summary>
/// Identify fields/properties/locals/params that are used as 'this' for a virtual dispatch.
/// </summary>
public void HandleInvocation(IInvocationOperation op)
{
Expand Down Expand Up @@ -368,6 +431,17 @@ private void GetValueTypes(List<ITypeSymbol> values, IOperation? op)
{
switch (op?.Kind)
{
case OperationKind.Await:
{
if (op.Type != null)
{
values.Add(op.Type);
}

return;

}

case OperationKind.Literal:
{
if (op.HasNullConstantValue())
Expand Down
5 changes: 3 additions & 2 deletions src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public override void Initialize(AnalysisContext context)
context.RegisterOperationAction(context => coll.HandleDeconstructionAssignment((IDeconstructionAssignmentOperation)context.Operation), OperationKind.DeconstructionAssignment);
context.RegisterOperationAction(context => coll.HandleFieldInitializer((IFieldInitializerOperation)context.Operation), OperationKind.FieldInitializer);
context.RegisterOperationAction(context => coll.HandlePropertyInitializer((IPropertyInitializerOperation)context.Operation), OperationKind.PropertyInitializer);
context.RegisterOperationAction(context => coll.HandlePropertyReference((IPropertyReferenceOperation)context.Operation), OperationKind.PropertyReference);
context.RegisterOperationAction(context => coll.HandleVariableDeclarator((IVariableDeclaratorOperation)context.Operation), OperationKind.VariableDeclarator);
context.RegisterOperationAction(context => coll.HandleDeclarationExpression((IDeclarationExpressionOperation)context.Operation), OperationKind.DeclarationExpression);
context.RegisterOperationAction(context => coll.HandleReturn((IReturnOperation)context.Operation), OperationKind.Return);
Expand Down Expand Up @@ -323,9 +324,9 @@ void Evaluate(ISymbol affectedSymbol, ITypeSymbol fromType, PooledConcurrentSet<
}
}

if (toType.TypeKind is not TypeKind.Class and not TypeKind.Array)
if (toType.TypeKind is not TypeKind.Class and not TypeKind.Array and not TypeKind.Struct)
{
// we only deal with classes or arrays
// we only deal with classes, arrays, or structs
return;
}

Expand Down
135 changes: 126 additions & 9 deletions src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,123 @@ namespace Microsoft.NetCore.Analyzers.Performance.UnitTests
{
public static partial class UseConcreteTypeTests
{
[Fact]
[WorkItem(6904, "https://github.com/dotnet/roslyn-analyzers/issues/6904")]
public static async Task AwaitBug()
{
await TestCSAsync(@"
using System.Threading.Tasks;
public class Class1
{
private I Prop { get; set; } = new Impl1(); //<-- CA1859
public async Task Init()
{
Prop = await Task.FromResult<I>(new Impl2());
Prop.M();
}
}
internal interface I
{
void M();
}
internal class Impl1 : I
{
public void M() { }
}
internal class Impl2 : I
{
public void M() { }
}
");
}

[Fact]
[WorkItem(7078, "https://github.com/dotnet/roslyn-analyzers/issues/7078")]
public static async Task IndexerBug()
{
await TestCSAsync(@"
using System.Collections.Generic;
public struct MailAddress { }
public class C
{
private IList<MailAddress> {|#1:_field|};
private IList<MailAddress> {|#2:Property|} { get; set; }
internal void ParseValue1(string addresses)
{
IList<MailAddress> {|#0:result|} = ParseMultipleAddresses(addresses);
var x = result[0];
}
internal void ParseValue2(string addresses)
{
IList<MailAddress> {|#3:result|} = ParseMultipleAddresses(addresses);
#nullable enable
var x = result?[0];
#nullable disable
}
internal void ParseValue3(string addresses)
{
_field = ParseMultipleAddresses(addresses);
var x = _field[0];
}
internal void ParseValue4(string addresses)
{
Property = ParseMultipleAddresses(addresses);
var x = Property[0];
}
internal static List<MailAddress> ParseMultipleAddresses(string data) => new();
}
",
VerifyCS.Diagnostic(UseConcreteTypeAnalyzer.UseConcreteTypeForLocal)
.WithLocation(0)
.WithArguments("result", "System.Collections.Generic.IList<MailAddress>", "System.Collections.Generic.List<MailAddress>"),
VerifyCS.Diagnostic(UseConcreteTypeAnalyzer.UseConcreteTypeForField)
.WithLocation(1)
.WithArguments("_field", "System.Collections.Generic.IList<MailAddress>", "System.Collections.Generic.List<MailAddress>"),
VerifyCS.Diagnostic(UseConcreteTypeAnalyzer.UseConcreteTypeForProperty)
.WithLocation(2)
.WithArguments("Property", "System.Collections.Generic.IList<MailAddress>", "System.Collections.Generic.List<MailAddress>"),
VerifyCS.Diagnostic(UseConcreteTypeAnalyzer.UseConcreteTypeForLocal)
.WithLocation(3)
.WithArguments("result", "System.Collections.Generic.IList<MailAddress>", "System.Collections.Generic.List<MailAddress>"));
}

[Fact]
[WorkItem(7127, "https://github.com/dotnet/roslyn-analyzers/issues/7127")]
public static async Task ImmutableArrayBug()
{
await TestCSAsync(@"
using System.Collections.Generic;
using System.Collections.Immutable;
public class Class1
{
private IEnumerable<int> {|#0:CreateImmutableArrayPrivately|}()
{
return new ImmutableArray<int>
{
1, 2, 3, 4
};
}
}
", VerifyCS.Diagnostic(UseConcreteTypeAnalyzer.UseConcreteTypeForMethodReturn)
.WithLocation(0)
.WithArguments("CreateImmutableArrayPrivately", "System.Collections.Generic.IEnumerable<int>", "System.Collections.Immutable.ImmutableArray<int>"));
}

[Fact]
[WorkItem(6751, "https://github.com/dotnet/roslyn-analyzers/issues/6751")]
public static async Task MultipleReturns()
Expand Down Expand Up @@ -268,7 +385,7 @@ private static object Test(int arg)
public static async Task ShouldNotTrigger_ValidatePublicSymbolUsage()
{
const string Source = @"
#nullable enable
#nullable enable
using System;
using System.Collections.Generic;
Expand Down Expand Up @@ -382,7 +499,7 @@ await TestCSAsync(source, $"dotnet_code_quality.CA1859.api_surface = {editorConf
public static async Task ShouldNotTrigger1()
{
const string Source = @"
#nullable enable
#nullable enable
using System;
using System.Collections.Generic;
Expand Down Expand Up @@ -416,7 +533,7 @@ public class Derived2 : BaseType
public static async Task ShouldNotTrigger2()
{
const string Source = @"
#nullable enable
#nullable enable
using System.Collections.Generic;
Expand Down Expand Up @@ -458,7 +575,7 @@ public static int Bar()
public static async Task ShouldNotTrigger3()
{
const string Source = @"
#nullable enable
#nullable enable
using System;
using System.IO;
Expand Down Expand Up @@ -489,7 +606,7 @@ public class MyStream : MemoryStream { }
public static async Task ShouldNotTrigger4()
{
const string Source = @"
#nullable enable
#nullable enable
using System;
using System.IO;
Expand Down Expand Up @@ -520,7 +637,7 @@ public class MyStream : MemoryStream { }
public static async Task ShouldNotTrigger5()
{
const string Source = @"
#nullable enable
#nullable enable
interface IFoo
{
Expand Down Expand Up @@ -652,7 +769,7 @@ await TestCSAsync(Source,
public static async Task ShouldTrigger3()
{
const string Source = @"
#nullable enable
#nullable enable
using System;
using System.IO;
Expand Down Expand Up @@ -683,7 +800,7 @@ await TestCSAsync(Source,
public static async Task ShouldTrigger4()
{
const string Source = @"
#nullable enable
#nullable enable
using System;
using System.IO;
Expand Down Expand Up @@ -757,7 +874,7 @@ await TestCSAsync(Source,
public static async Task Conditional()
{
const string Source = @"
#nullable enable
#nullable enable
namespace Example
{
public interface IFoo
Expand Down

0 comments on commit 23ec029

Please sign in to comment.