From 16510b8a3a1e12ba5d9884acd91992a6b9d6d9d5 Mon Sep 17 00:00:00 2001 From: Collin Alpert Date: Fri, 24 May 2024 18:22:35 +0200 Subject: [PATCH] Handle BinaryOperation -return in UseConcreteTypeAnalyzer --- .../UseConcreteTypeAnalyzer.Collector.cs | 8 + .../UseConcreteTypeTests.Disqualification.cs | 382 +++++++++--------- 2 files changed, 191 insertions(+), 199 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs index 2c5c6a5f64..67b859d948 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs @@ -442,6 +442,14 @@ private void GetValueTypes(List values, IOperation? op) } + case OperationKind.Binary: + { + var binaryOperation = (IBinaryOperation)op; + GetValueTypes(values, binaryOperation.LeftOperand); + GetValueTypes(values, binaryOperation.RightOperand); + return; + } + case OperationKind.Literal: { if (op.HasNullConstantValue()) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeTests.Disqualification.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeTests.Disqualification.cs index 255d819bdb..3bcf3e095b 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeTests.Disqualification.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeTests.Disqualification.cs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. -using System.Collections.Generic; using System.Threading.Tasks; +using Test.Utilities; using Xunit; namespace Microsoft.NetCore.Analyzers.Performance.UnitTests @@ -10,277 +10,261 @@ public static partial class UseConcreteTypeTests { [Theory] [MemberData(nameof(DisqualifiedSources))] - public static async Task Disqualication(string insert) + [WorkItem(7317, "https://github.com/dotnet/roslyn-analyzers/issues/7317")] + public static async Task Disqualification(string insert) { - string source = @$" - #pragma warning disable CS8019 - #nullable enable - - using System; - using System.Threading.Tasks; - - namespace Example - {{ - public interface IFoo - {{ - void Bar(); - }} - - public class Foo : IFoo - {{ - public void Bar() - {{ - }} - }} - - {insert} - }}"; + string source = $$""" + #pragma warning disable CS8019 + #nullable enable + + using System; + using System.Threading.Tasks; + + namespace Example + { + public interface IFoo + { + void Bar(); + } + + public class Foo : IFoo + { + public void Bar() + { + } + } + + {{insert}} + } + """; await TestCSAsync(source); } - public static IEnumerable DisqualifiedSources => new List + public static TheoryData DisqualifiedSources => new() { - new[] + """ + public class TestLocalFunction { - @" - public class TestLocalFunction + private IFoo SyncMethod(int x) { - private IFoo SyncMethod(int x) + switch (x) { - switch (x) - { - case 0: return MakeFoo(); - default: return new Foo(); - } - - static IFoo MakeFoo() => new Foo() as IFoo; - } - - private async Task AsyncMethod(Task stuff, int x) - { - await stuff; - - switch (x) - { - case 0: return MakeFoo(); - default: return new Foo(); - } - - static IFoo MakeFoo() => new Foo() as IFoo; + case 0: return MakeFoo(); + default: return new Foo(); } - private async Task TryCatchAsyncMethod(Task stuff, int x) - { - try - { - await stuff; - return new Foo(); - } - catch - { - return new Foo(); - } - } + static IFoo MakeFoo() => new Foo() as IFoo; } - ", - }, - new[] - { - @" - public class TestArray + private async Task AsyncMethod(Task stuff, int x) { - private readonly IFoo[] _foos = new IFoo[3]; + await stuff; - private IFoo Method(int x) + switch (x) { - switch (x) - { - case 0: return _foos[0]; - default: return new Foo(); - } + case 0: return MakeFoo(); + default: return new Foo(); } + + static IFoo MakeFoo() => new Foo() as IFoo; } - ", - }, - new[] - { - @" - public class TestLambda + private async Task TryCatchAsyncMethod(Task stuff, int x) { - private IFoo MethodWithExpression(int x) + try { - switch (x) - { - case 0: return Stub(() => new Foo()); - default: return new Foo(); - } + await stuff; + return new Foo(); } - - private IFoo MethodWithBlock(int x) + catch { - switch (x) - { - case 0: - return Stub(() => - { - return new Foo(); - }); - default: return new Foo(); - } + return new Foo(); } + } + } + """, + """ + public class TestArray + { + private readonly IFoo[] _foos = new IFoo[3]; - public IFoo Stub(Func func) + private IFoo Method(int x) + { + switch (x) { - return func(); + case 0: return _foos[0]; + default: return new Foo(); } } - ", - }, - - new[] + } + """, + """ + public class TestLambda { - @" - public class TestByRef + private IFoo MethodWithExpression(int x) { - private IFoo MethodUsingByRef(int x) - { - switch (x) - { - case 0: - { - IFoo localRef = new Foo(); - RefMethod(ref localRef); - return localRef; - } - - case 1: - { - OutMethod(out var localOut); - return localOut; - } - - default: - return new Foo(); - } - } - - public void RefMethod(ref IFoo foo) + switch (x) { - foo = new Foo(); + case 0: return Stub(() => new Foo()); + default: return new Foo(); } + } - public void OutMethod(out IFoo foo) + private IFoo MethodWithBlock(int x) + { + switch (x) { - foo = new Foo(); + case 0: + return Stub(() => + { + return new Foo(); + }); + default: return new Foo(); } } - ", - }, - new[] + public IFoo Stub(Func func) + { + return func(); + } + } + """, + """ + public class TestByRef { - @" - public class TestTuples + private IFoo MethodUsingByRef(int x) { - private IFoo MethodTuple(int x) + switch (x) { - switch (x) + case 0: { - case 0: - var (l, m) = MakeTuple(); - return l; + IFoo localRef = new Foo(); + RefMethod(ref localRef); + return localRef; + } - default: return new Foo(); + case 1: + { + OutMethod(out var localOut); + return localOut; } - } - public (IFoo, IFoo) MakeTuple() - { - return (new Foo(), new Foo()); + default: + return new Foo(); } } - ", - }, - new[] - { - @" - public interface IBase + public void RefMethod(ref IFoo foo) { - IFoo Method(); + foo = new Foo(); } - public class TestInterfaceMethod : IBase + public void OutMethod(out IFoo foo) { - public IFoo Method() - { - return new Foo(); - } + foo = new Foo(); } - ", - }, - - new[] + } + """, + """ + public class TestTuples { - @" - public class Base1 + private IFoo MethodTuple(int x) { - public virtual IFoo Method() + switch (x) { - return new Foo(); + case 0: + var (l, m) = MakeTuple(); + return l; + + default: return new Foo(); } } - public class Base2 : Base1 + public (IFoo, IFoo) MakeTuple() { + return (new Foo(), new Foo()); } + } + """, + """ + public interface IBase + { + IFoo Method(); + } - public class TestOverrideMethod : Base2 + public class TestInterfaceMethod : IBase + { + public IFoo Method() { - public override IFoo Method() - { - return new Foo(); - } + return new Foo(); } - ", - }, - - new[] + } + """, + """ + public class Base1 { - @" - public abstract class TestConstraints + public virtual IFoo Method() { - public virtual IFoo VirtualMethod() =>new Foo(); - public abstract IFoo AbstractMethod(); - public IFoo PublicMethod() => new Foo(); - public IFoo InternalMethod() => new Foo(); + return new Foo(); } - ", - }, + } - new[] + public class Base2 : Base1 { - @" - public class TestConflictingLocals + } + + public class TestOverrideMethod : Base2 + { + public override IFoo Method() { - public void Test(int x) - { - IFoo l; + return new Foo(); + } + } + """, + """ + public abstract class TestConstraints + { + public virtual IFoo VirtualMethod() =>new Foo(); + public abstract IFoo AbstractMethod(); + public IFoo PublicMethod() => new Foo(); + public IFoo InternalMethod() => new Foo(); + } + """, + """ + public class TestConflictingLocals + { + public void Test(int x) + { + IFoo l; - switch (x) - { - case 0: l = new Foo(); break; - case 1: l = MakeFoo(); break; - } + switch (x) + { + case 0: l = new Foo(); break; + case 1: l = MakeFoo(); break; } + } - public IFoo MakeFoo() => new Foo(); + public IFoo MakeFoo() => new Foo(); + } + """, + """ + public abstract class Base + { + public static Base operator *(Base left, int right) => default!; + } + + public class A : Base; + public class B : Base; + + public class Test + { + private static Base M(Base input) + { + return input is A ? new B() : input * 4; } - ", - }, + } + """ }; } }