diff options
author | Vitek Karas <10670590+vitek-karas@users.noreply.github.com> | 2022-04-29 21:12:28 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-04-29 21:12:28 +0300 |
commit | 98ba0ce1b4bc5c3e094a912d3221fe96938d6d8d (patch) | |
tree | 6080480eced0e8e5ea3f4854cba089588d48d853 /test | |
parent | 848f9d48961945b9a0236ae7ff7b687c7d4abe42 (diff) |
Share Type.GetType and similar intrinsics (#2773)
This shared pretty much all of the remaining intrinsics except for `object.GetType` (type hierarchy marking).
The linker move is straightforward and the only functional changes are:
* `Type.GetType()` with unsupported case insensitive search will not produce warnings from its return value
* Removed intrinsics handling of `Activator.CreateInstance<T>` - it's not needed, annotations do the same thing, and avoid generating duplicate warnings. This fixes: https://github.com/dotnet/linker/issues/1483
In the analyzer there are two behavioral differences from linker:
* Searching for assembly name + type name is completely disabled (no warnings, ignored) - see the comment added, but there's no good way to do it in most cases and probably not common. We can improve if really necessary.
* `Type.GetType` doesn't actually resolve any types since we don't have a type resolver implementation in the analyzer. This change makes all the rest work as appropriate, so once we do have a type resolver, it should all just light up.
I added test to cover at least basic type resolver cases in a way that it shows that analyzer doesn't handle it.
Other small things:
* Analyzer's ability to handle boolean constants
* Minor cleanup
Diffstat (limited to 'test')
7 files changed, 101 insertions, 58 deletions
diff --git a/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs b/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs index adf5c9369..b581247d7 100644 --- a/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs +++ b/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs @@ -111,8 +111,7 @@ namespace ILLink.RoslynAnalyzer.Tests [Fact] public Task GetTypeDataFlow () { - // https://github.com/dotnet/linker/issues/2273 - return RunTest (allowMissingWarnings: true); + return RunTest (); } [Fact] diff --git a/test/ILLink.RoslynAnalyzer.Tests/ReflectionTests.cs b/test/ILLink.RoslynAnalyzer.Tests/ReflectionTests.cs index 245e582d4..560e3109d 100644 --- a/test/ILLink.RoslynAnalyzer.Tests/ReflectionTests.cs +++ b/test/ILLink.RoslynAnalyzer.Tests/ReflectionTests.cs @@ -13,7 +13,7 @@ namespace ILLink.RoslynAnalyzer.Tests [Fact] public Task ActivatorCreateInstance () { - return RunTest (allowMissingWarnings: true); + return RunTest (); } [Fact] @@ -67,7 +67,7 @@ namespace ILLink.RoslynAnalyzer.Tests [Fact] public Task ExpressionNewType () { - return RunTest (allowMissingWarnings: true); + return RunTest (); } [Fact] @@ -196,13 +196,43 @@ namespace ILLink.RoslynAnalyzer.Tests [Fact] public Task TypeUsedViaReflection () { - return RunTest (allowMissingWarnings: true); + return RunTest (); + } + + [Fact] + public Task TypeUsedViaReflectionAssemblyDoesntExist () + { + return RunTest (); + } + + [Fact] + public Task TypeUsedViaReflectionInDifferentAssembly () + { + return RunTest (); + } + + [Fact] + public Task TypeUsedViaReflectionLdstrIncomplete () + { + return RunTest (); + } + + [Fact] + public Task TypeUsedViaReflectionLdstrValidButChanged () + { + return RunTest (); } [Fact] public Task TypeUsedViaReflectionTypeDoesntExist () { - return RunTest (allowMissingWarnings: true); + return RunTest (); + } + + [Fact] + public Task TypeUsedViaReflectionTypeNameIsSymbol () + { + return RunTest (); } } } diff --git a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/ReflectionTests.g.cs b/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/ReflectionTests.g.cs index 379d1d3b4..23c3173c0 100644 --- a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/ReflectionTests.g.cs +++ b/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/ReflectionTests.g.cs @@ -62,36 +62,6 @@ namespace ILLink.RoslynAnalyzer.Tests } [Fact] - public Task TypeUsedViaReflectionAssemblyDoesntExist () - { - return RunTest (allowMissingWarnings: true); - } - - [Fact] - public Task TypeUsedViaReflectionInDifferentAssembly () - { - return RunTest (allowMissingWarnings: true); - } - - [Fact] - public Task TypeUsedViaReflectionLdstrIncomplete () - { - return RunTest (allowMissingWarnings: true); - } - - [Fact] - public Task TypeUsedViaReflectionLdstrValidButChanged () - { - return RunTest (allowMissingWarnings: true); - } - - [Fact] - public Task TypeUsedViaReflectionTypeNameIsSymbol () - { - return RunTest (allowMissingWarnings: true); - } - - [Fact] public Task UnderlyingSystemType () { return RunTest (allowMissingWarnings: true); diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/GetTypeDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/GetTypeDataFlow.cs index 3bfe3c07d..dff625716 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/GetTypeDataFlow.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/GetTypeDataFlow.cs @@ -33,6 +33,8 @@ namespace Mono.Linker.Tests.Cases.DataFlow TestStringEmpty (); + TypeWithWarnings.Test (); + // TODO: // Test multi-value returns // Type.GetType over a constant and a param @@ -71,19 +73,17 @@ namespace Mono.Linker.Tests.Cases.DataFlow type.RequiresNone (); } - [ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresAll) + "(Type)", nameof (Type.GetType) + "(String)")] static void TestNull () { - // Warns about the return value of GetType, even though this throws at runtime. + // GetType(null) throws at runtime, so we "give up" on analysis Type.GetType (null).RequiresAll (); } - [ExpectedWarning ("IL2072", nameof (DataFlowTypeExtensions.RequiresAll) + "(Type)", nameof (Type.GetType) + "(String)")] static void TestNoValue () { Type t = null; + // null.AssemblyQualifiedName throws at runtime, so we "give up" on analysis string noValue = t.AssemblyQualifiedName; - // Warns about the return value of GetType, even though AssemblyQualifiedName throws at runtime. Type.GetType (noValue).RequiresAll (); } @@ -161,6 +161,24 @@ namespace Mono.Linker.Tests.Cases.DataFlow Type.GetType (typeName).RequiresNonPublicConstructors (); } + class TypeWithWarnings + { + + [RequiresUnreferencedCode ("--Method1--")] + public void Method1 () { } + + [RequiresUnreferencedCode ("--Method2--")] + public void Method2 () { } + + // https://github.com/dotnet/linker/issues/2273 + [ExpectedWarning ("IL2026", "--Method1--", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2026", "--Method2--", ProducedBy = ProducedBy.Trimmer)] + public static void Test () + { + Type.GetType ("Mono.Linker.Tests.Cases.DataFlow." + nameof (GetTypeDataFlow) + "+" + nameof (TypeWithWarnings)).RequiresPublicMethods (); + } + } + [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] private static string GetStringTypeWithPublicParameterlessConstructor () { diff --git a/test/Mono.Linker.Tests.Cases/Reflection/ActivatorCreateInstance.cs b/test/Mono.Linker.Tests.Cases/Reflection/ActivatorCreateInstance.cs index 968d93471..204e03469 100644 --- a/test/Mono.Linker.Tests.Cases/Reflection/ActivatorCreateInstance.cs +++ b/test/Mono.Linker.Tests.Cases/Reflection/ActivatorCreateInstance.cs @@ -10,6 +10,7 @@ namespace Mono.Linker.Tests.Cases.Reflection [SetupCSharpCompilerToUse ("csc")] [ExpectedNoWarnings] [KeptMember (".ctor()")] + [KeptMember (".cctor()")] public class ActivatorCreateInstance { public static void Main () @@ -51,7 +52,7 @@ namespace Mono.Linker.Tests.Cases.Reflection TestCreateInstanceOfTWithNewConstraint<TestCreateInstanceOfTWithNewConstraintType> (); TestCreateInstanceOfTWithNoConstraint<TestCreateInstanceOfTWithNoConstraintType> (); - TestCreateInstanceOfTWithDataflow<TestCreateInstanceOfTWithDataflowType> (); + TestCreateInstanceOfTWithDataflowType.Test (); TestNullArgsOnKnownType (); TestNullArgsOnAnnotatedType (typeof (TestType)); @@ -169,7 +170,9 @@ namespace Mono.Linker.Tests.Cases.Reflection public FromParameterOnStaticMethodTypeB (int arg) { } } - [ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, Object[])")] + // Small formatting difference + [ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, Object[])", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, params Object[])", ProducedBy = ProducedBy.Analyzer)] [ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance), nameof (CultureInfo))] [Kept] private void FromParameterOnInstanceMethod ( @@ -193,7 +196,9 @@ namespace Mono.Linker.Tests.Cases.Reflection } [ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type)")] - [ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, Object[])")] + // Small formatting difference + [ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, Object[])", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, params Object[])", ProducedBy = ProducedBy.Analyzer)] [Kept] private static void FromParameterWithNonPublicConstructors ( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicConstructors)] @@ -369,14 +374,15 @@ namespace Mono.Linker.Tests.Cases.Reflection } [Kept] - [ExpectedWarning ("IL2061", nameof (Activator) + "." + nameof (Activator.CreateInstance), "NonExistingAssembly")] + // Analyzer doesn't handle assembly resolution + [ExpectedWarning ("IL2061", nameof (Activator) + "." + nameof (Activator.CreateInstance), "NonExistingAssembly", ProducedBy = ProducedBy.Trimmer)] private static void WithNonExistingAssemblyName () { Activator.CreateInstance ("NonExistingAssembly", "Mono.Linker.Tests.Cases.Reflection.ActivatorCreateInstance+WithAssemblyNameParameterless1"); } [Kept] - private static string _typeNameField; + private static string _typeNameField = "Unknown"; [Kept] [ExpectedWarning ("IL2032", nameof (Activator) + "." + nameof (Activator.CreateInstance), "typeName")] @@ -554,9 +560,6 @@ namespace Mono.Linker.Tests.Cases.Reflection [Kept] [ExpectedWarning ("IL2091", nameof (Activator), nameof (Activator.CreateInstance) + "<T>")] - [ExpectedWarning ("IL2091", nameof (Activator), nameof (Activator.CreateInstance) + "<T>")] - // Warnings are currently duplicated in NETCORE (annotation and intrinsics) - but they're not identical in this case. - // See https://github.com/dotnet/linker/issues/1483 private static void TestCreateInstanceOfTWithNoConstraint<T> () { Activator.CreateInstance<T> (); @@ -573,14 +576,30 @@ namespace Mono.Linker.Tests.Cases.Reflection public TestCreateInstanceOfTWithDataflowType (int i) { } - } - [Kept] - private static void TestCreateInstanceOfTWithDataflow< - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor), - KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] T> () - { - Activator.CreateInstance<T> (); + [Kept] + static void TestWithMatchingDataFlow< + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor), + KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] T> () + { + Activator.CreateInstance<T> (); + } + + [Kept] + [ExpectedWarning ("IL2091")] + static void TestWithMismatchDataFlow< + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields), + KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] T> () + { + Activator.CreateInstance<T> (); + } + + [Kept] + public static void Test () + { + TestWithMatchingDataFlow<TestCreateInstanceOfTWithDataflowType> (); + TestWithMismatchDataFlow<TestCreateInstanceOfTWithDataflowType> (); + } } [Kept] diff --git a/test/Mono.Linker.Tests.Cases/Reflection/ExpressionNewType.cs b/test/Mono.Linker.Tests.Cases/Reflection/ExpressionNewType.cs index f100129ff..06c4a9526 100644 --- a/test/Mono.Linker.Tests.Cases/Reflection/ExpressionNewType.cs +++ b/test/Mono.Linker.Tests.Cases/Reflection/ExpressionNewType.cs @@ -53,11 +53,12 @@ namespace Mono.Linker.Tests.Cases.Reflection Expression.New (T); } - [ExpectedWarning ("IL2072", nameof (Expression) + "." + nameof (Expression.New), "'System." + nameof (Type) + "." + nameof (Type.GetType))] [ExpectedWarning ("IL2072", nameof (Expression) + "." + nameof (Expression.New), nameof (ExpressionNewType) + "." + nameof (ExpressionNewType.GetType))] [Kept] static void Branch_UnrecognizedPatterns () { + // Note that "RemovedType" will not resolve here since the type declared below with the same name is nested type and so its real name is "ExpressionNewType+RemovedType" + // This should not warn - we choose to not warn if we can't resolve a type (and anything which it's used for) Expression.New (Type.GetType ("RemovedType")); Expression.New (GetType ()); } diff --git a/test/Mono.Linker.Tests.Cases/Reflection/TypeUsedViaReflection.cs b/test/Mono.Linker.Tests.Cases/Reflection/TypeUsedViaReflection.cs index 8cab53e83..906f56ab4 100644 --- a/test/Mono.Linker.Tests.Cases/Reflection/TypeUsedViaReflection.cs +++ b/test/Mono.Linker.Tests.Cases/Reflection/TypeUsedViaReflection.cs @@ -2,6 +2,7 @@ using System.Diagnostics.CodeAnalysis; using System.Reflection; using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; using Mono.Linker.Tests.Cases.Expectations.Metadata; namespace Mono.Linker.Tests.Cases.Reflection @@ -268,6 +269,7 @@ namespace Mono.Linker.Tests.Cases.Reflection { const string reflectionTypeKeptString = "mono.linker.tests.cases.reflection.TypeUsedViaReflection+CaseInsensitive, test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null"; var typeKept = Type.GetType (reflectionTypeKeptString, false, true); + typeKept.RequiresPublicMethods (); // Validate that we don't track the value anymore since the above already warned about the problem } public class CaseUnknown { } @@ -324,7 +326,9 @@ namespace Mono.Linker.Tests.Cases.Reflection public class OverloadWith5ParametersWithIgnoreCase { } [Kept] - [ExpectedWarning ("IL2096", "'System.Type.GetType(String, Func<AssemblyName,Assembly>, Func<Assembly,String,Boolean,Type>, Boolean, Boolean)'")] + // Small difference in formatting between analyzer/linker + [ExpectedWarning ("IL2096", "'System.Type.GetType(String, Func<AssemblyName,Assembly>, Func<Assembly,String,Boolean,Type>, Boolean, Boolean)'", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2096", "'System.Type.GetType(String, Func<AssemblyName, Assembly>, Func<Assembly, String, Boolean, Type>, Boolean, Boolean)'", ProducedBy = ProducedBy.Analyzer)] static void TestTypeOverloadWith5ParametersWithIgnoreCase () { const string reflectionTypeKeptString = "Mono.Linker.Tests.Cases.Reflection.TypeUsedViaReflection+OverloadWith5ParametersWithIgnoreCase"; @@ -374,7 +378,9 @@ namespace Mono.Linker.Tests.Cases.Reflection } [Kept] - [ExpectedWarning ("IL2096", "'System.Type.GetType(String, Func<AssemblyName,Assembly>, Func<Assembly,String,Boolean,Type>, Boolean, Boolean)'")] + // Small difference in formatting between analyzer/linker + [ExpectedWarning ("IL2096", "'System.Type.GetType(String, Func<AssemblyName,Assembly>, Func<Assembly,String,Boolean,Type>, Boolean, Boolean)'", ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2096", "'System.Type.GetType(String, Func<AssemblyName, Assembly>, Func<Assembly, String, Boolean, Type>, Boolean, Boolean)'", ProducedBy = ProducedBy.Analyzer)] static void TestUnkownIgnoreCase5Params (int num) { const string reflectionTypeKeptString = "mono.linker.tests.cases.reflection.TypeUsedViaReflection+CaseUnknown2, test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null"; |