diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs index cd9017cc5e03ff..53533e252c5dfa 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs @@ -14,6 +14,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; namespace ILLink.RoslynAnalyzer { @@ -76,7 +77,6 @@ public override void Initialize (AnalysisContext context) if (context.OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) return; - foreach (var operationBlock in context.OperationBlocks) { TrimDataFlowAnalysis trimDataFlowAnalysis = new (context, operationBlock); trimDataFlowAnalysis.InterproceduralAnalyze (); @@ -84,9 +84,87 @@ public override void Initialize (AnalysisContext context) context.ReportDiagnostic (diagnostic); } }); - context.RegisterSyntaxNodeAction (context => { - ProcessGenericParameters (context); - }, SyntaxKind.GenericName); + // Examine generic instantiations in base types and interface list + context.RegisterSymbolAction (context => { + var type = (INamedTypeSymbol) context.Symbol; + // RUC on type doesn't silence DAM warnings about generic base/interface types. + // This knowledge lives in IsInRequiresUnreferencedCodeAttributeScope, + // which we still call for consistency here, but it is expected to return false. + if (type.IsInRequiresUnreferencedCodeAttributeScope (out _)) + return; + + if (type.BaseType is INamedTypeSymbol baseType) { + foreach (var diagnostic in ProcessGenericParameters (baseType, type.Locations[0])) + context.ReportDiagnostic (diagnostic); + } + + foreach (var interfaceType in type.Interfaces) { + foreach (var diagnostic in ProcessGenericParameters (interfaceType, type.Locations[0])) + context.ReportDiagnostic (diagnostic); + } + }, SymbolKind.NamedType); + // Examine generic instantiations in method return type and parameters. + // This includes property getters and setters. + context.RegisterSymbolAction (context => { + var method = (IMethodSymbol) context.Symbol; + if (method.IsInRequiresUnreferencedCodeAttributeScope (out _)) + return; + + var returnType = method.ReturnType; + foreach (var diagnostic in ProcessGenericParameters (returnType, method.Locations[0])) + context.ReportDiagnostic (diagnostic); + + foreach (var parameter in method.Parameters) { + foreach (var diagnostic in ProcessGenericParameters (parameter.Type, parameter.Locations[0])) + context.ReportDiagnostic (diagnostic); + } + }, SymbolKind.Method); + // Examine generic instantiations in field type. + context.RegisterSymbolAction (context => { + var field = (IFieldSymbol) context.Symbol; + if (field.IsInRequiresUnreferencedCodeAttributeScope (out _)) + return; + + foreach (var diagnostic in ProcessGenericParameters (field.Type, field.Locations[0])) + context.ReportDiagnostic (diagnostic); + }, SymbolKind.Field); + // Examine generic instantiations in invocations of generically instantiated methods, + // or methods on generically instantiated types. + context.RegisterOperationAction (context => { + if (context.ContainingSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) + return; + + var invocation = (IInvocationOperation) context.Operation; + var methodSymbol = invocation.TargetMethod; + foreach (var diagnostic in ProcessMethodGenericParameters (methodSymbol, invocation.Syntax.GetLocation ())) + context.ReportDiagnostic (diagnostic); + }, OperationKind.Invocation); + // Examine generic instantiations in delegate creation of generically instantiated methods. + context.RegisterOperationAction (context => { + if (context.ContainingSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) + return; + + var delegateCreation = (IDelegateCreationOperation) context.Operation; + if (delegateCreation.Target is not IMethodReferenceOperation methodReference) + return; + + if (methodReference.Method is not IMethodSymbol methodSymbol) + return; + foreach (var diagnostic in ProcessMethodGenericParameters (methodSymbol, delegateCreation.Syntax.GetLocation())) + context.ReportDiagnostic (diagnostic); + }, OperationKind.DelegateCreation); + // Examine generic instantiations in object creation of generically instantiated types. + context.RegisterOperationAction (context => { + if (context.ContainingSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) + return; + + var objectCreation = (IObjectCreationOperation) context.Operation; + if (objectCreation.Type is not ITypeSymbol typeSymbol) + return; + + foreach (var diagnostic in ProcessGenericParameters (typeSymbol, objectCreation.Syntax.GetLocation())) + context.ReportDiagnostic (diagnostic); + }, OperationKind.ObjectCreation); context.RegisterSymbolAction (context => { VerifyMemberOnlyApplyToTypesOrStrings (context, context.Symbol); VerifyDamOnPropertyAndAccessorMatch (context, (IMethodSymbol) context.Symbol); @@ -104,36 +182,22 @@ public override void Initialize (AnalysisContext context) }); } - static void ProcessGenericParameters (SyntaxNodeAnalysisContext context) + static IEnumerable ProcessMethodGenericParameters (IMethodSymbol methodSymbol, Location location) { - // RUC on the containing symbol normally silences warnings, but when examining a generic base type, - // the containing symbol is the declared derived type. RUC on the derived type does not silence - // warnings about base type arguments. - if (context.ContainingSymbol is not null - && context.ContainingSymbol is not INamedTypeSymbol - && context.ContainingSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) - return; + foreach (var diagnostic in ProcessGenericParameters (methodSymbol, location)) + yield return diagnostic; - var symbol = context.SemanticModel.GetSymbolInfo (context.Node).Symbol; + if (methodSymbol.IsStatic && methodSymbol.ContainingType is not null) { + foreach (var diagnostic in ProcessGenericParameters (methodSymbol.ContainingType, location)) + yield return diagnostic; + } + } + static IEnumerable ProcessGenericParameters (ISymbol symbol, Location location) + { // Avoid unnecessary execution if not NamedType or Method if (symbol is not INamedTypeSymbol && symbol is not IMethodSymbol) - return; - - // Members inside nameof or cref comments, commonly used to access the string value of a variable, type, or a memeber, - // can generate diagnostics warnings, which can be noisy and unhelpful. - // Walking the node heirarchy to check if the member is inside a nameof/cref to not generate diagnostics - var parentNode = context.Node; - while (parentNode != null) { - if (parentNode is InvocationExpressionSyntax invocationExpression && - invocationExpression.Expression is IdentifierNameSyntax ident1 && - ident1.Identifier.ValueText.Equals ("nameof")) - return; - else if (parentNode is CrefSyntax) - return; - - parentNode = parentNode.Parent; - } + yield break; ImmutableArray typeParams = default; ImmutableArray typeArgs = default; @@ -158,8 +222,8 @@ invocationExpression.Expression is IdentifierNameSyntax ident1 && continue; var sourceValue = SingleValueExtensions.FromTypeSymbol (typeArgs[i])!; var targetValue = new GenericParameterValue (typeParams[i]); - foreach (var diagnostic in GetDynamicallyAccessedMembersDiagnostics (sourceValue, targetValue, context.Node.GetLocation ())) - context.ReportDiagnostic (diagnostic); + foreach (var diagnostic in GetDynamicallyAccessedMembersDiagnostics (sourceValue, targetValue, location)) + yield return diagnostic; } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/README.md b/src/tools/illink/src/ILLink.RoslynAnalyzer/README.md index 6a0f14dd88e4a3..e5e1a4144385bd 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/README.md +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/README.md @@ -8,7 +8,7 @@ To use a local build of the analyzer in another project, modify the `Analyzer` I - + ``` \ No newline at end of file diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersAnalyzerTests.cs index 7acce352b10610..2a1f1e713281f0 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersAnalyzerTests.cs @@ -1112,7 +1112,7 @@ private static void M2() // The source value must declare at least the same requirements as those declared on the target location it is assigned to. return VerifyDynamicallyAccessedMembersAnalyzer (TargetGenericParameterWithAnnotations, VerifyCS.Diagnostic (DiagnosticId.DynamicallyAccessedMembersMismatchTypeArgumentTargetsGenericParameter) - .WithSpan (16, 3, 16, 8) + .WithSpan (16, 3, 16, 10) .WithSpan (14, 25, 14, 26) .WithArguments ("T", "C.M1()", "S", "C.M2()", "'DynamicallyAccessedMemberTypes.PublicMethods'")); } @@ -1402,7 +1402,7 @@ class CRequires<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Publi // The actual usage (return value) should warn, about missing annotation, but the cref should not. return VerifyDynamicallyAccessedMembersAnalyzer (Source, // (11,9): warning IL2091: 'TInner' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in 'CRequires'. The generic parameter 'TOuter' of 'C' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. - VerifyCS.Diagnostic (DiagnosticId.DynamicallyAccessedMembersMismatchTypeArgumentTargetsGenericParameter).WithSpan (11, 9, 11, 26).WithSpan (4, 9, 4, 15).WithArguments ("TInner", "CRequires", "TOuter", "C", "'DynamicallyAccessedMemberTypes.PublicMethods'")); + VerifyCS.Diagnostic (DiagnosticId.DynamicallyAccessedMembersMismatchTypeArgumentTargetsGenericParameter).WithSpan (11, 36, 11, 57).WithSpan (4, 9, 4, 15).WithArguments ("TInner", "CRequires", "TOuter", "C", "'DynamicallyAccessedMemberTypes.PublicMethods'")); } } } diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersCodeFixTests.cs index 1197135d892328..8736e3837d6b7e 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersCodeFixTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersCodeFixTests.cs @@ -2529,7 +2529,7 @@ await VerifyDynamicallyAccessedMembersCodeFix ( // The generic parameter 'S' of 'C.M2()' does not have matching annotations. // The source value must declare at least the same requirements as those declared on the target location it is assigned to. VerifyCS.Diagnostic(DiagnosticId.DynamicallyAccessedMembersMismatchTypeArgumentTargetsGenericParameter) - .WithSpan(16, 3, 16, 8) + .WithSpan(16, 3, 16, 10) .WithSpan(14, 25, 14, 26) .WithArguments("T", "C.M1()", @@ -2541,7 +2541,7 @@ await VerifyDynamicallyAccessedMembersCodeFix ( } [Fact] - public async Task CodeFix_IL2091_AttrbuteTurnsOffCodeFix () + public async Task CodeFix_IL2091_AttributeTurnsOffCodeFix () { var test = $$""" using System.Diagnostics.CodeAnalysis; @@ -2568,7 +2568,7 @@ public static void Main() // The generic parameter 'S' of 'C.M2()' does not have matching annotations. // The source value must declare at least the same requirements as those declared on the target location it is assigned to. VerifyCS.Diagnostic(DiagnosticId.DynamicallyAccessedMembersMismatchTypeArgumentTargetsGenericParameter) - .WithSpan(16, 3, 16, 8) + .WithSpan(16, 3, 16, 10) .WithArguments("T", "C.M1()", "S", diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs index 651dbdd8fe9daf..f4d41085849c7d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; @@ -42,6 +42,10 @@ public static void Main () TestGenericParameterFlowsToField (); TestGenericParameterFlowsToReturnValue (); + TestGenericParameterFlowsToDelegateMethod (); + TestGenericParameterFlowsToDelegateMethodDeclaringType (); + TestGenericParameterFlowsToDelegateMethodDeclaringTypeInstance (); + TestNoWarningsInRUCMethod (); TestNoWarningsInRUCType (); } @@ -211,7 +215,8 @@ static void TestDerivedTypeWithOpenGenericOnBaseWithRUCOnBase () [ExpectedWarning ("IL2109", nameof (BaseTypeWithOpenGenericDAMTAndRUC))] [ExpectedWarning ("IL2091", nameof (BaseTypeWithOpenGenericDAMTAndRUC))] - class DerivedTypeWithOpenGenericOnBaseWithRUCOnBase : BaseTypeWithOpenGenericDAMTAndRUC + [ExpectedWarning ("IL2091", nameof (IGenericInterfaceTypeWithRequirements))] + class DerivedTypeWithOpenGenericOnBaseWithRUCOnBase : BaseTypeWithOpenGenericDAMTAndRUC, IGenericInterfaceTypeWithRequirements { [ExpectedWarning ("IL2091", nameof (DerivedTypeWithOpenGenericOnBaseWithRUCOnBase), ProducedBy = Tool.Trimmer | Tool.NativeAot)] [ExpectedWarning ("IL2026", nameof (BaseTypeWithOpenGenericDAMTAndRUC), ProducedBy = Tool.Trimmer | Tool.NativeAot)] @@ -228,8 +233,9 @@ static void TestDerivedTypeWithOpenGenericOnBaseWithRUCOnDerived () new DerivedTypeWithOpenGenericOnBaseWithRUCOnDerived (); } [ExpectedWarning ("IL2091", nameof (BaseTypeWithOpenGenericDAMT))] + [ExpectedWarning ("IL2091", nameof (IGenericInterfaceTypeWithRequirements))] [RequiresUnreferencedCode ("RUC")] - class DerivedTypeWithOpenGenericOnBaseWithRUCOnDerived : BaseTypeWithOpenGenericDAMT + class DerivedTypeWithOpenGenericOnBaseWithRUCOnDerived : BaseTypeWithOpenGenericDAMT, IGenericInterfaceTypeWithRequirements { } @@ -386,6 +392,7 @@ static void TestTypeGenericRequirementsOnMembers () instance.PublicFieldsProperty = null; _ = instance.PublicMethodsProperty; instance.PublicMethodsProperty = null; + _ = instance.PublicMethodsImplicitGetter; instance.PublicFieldsMethodParameter (null); instance.PublicMethodsMethodParameter (null); @@ -409,14 +416,16 @@ public TypeRequiresPublicFields PublicFieldsProperty { set; } - [ExpectedWarning ("IL2091", nameof (TypeGenericRequirementsOnMembers), ProducedBy = Tool.Analyzer)] public TypeRequiresPublicMethods PublicMethodsProperty { - [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods), ProducedBy = Tool.Trimmer)] // NativeAOT_StorageSpaceType + [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods), ProducedBy = Tool.Trimmer | Tool.Analyzer)] // NativeAOT_StorageSpaceType get => null; - [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods), ProducedBy = Tool.Trimmer)] // NativeAOT_StorageSpaceType + [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods), ProducedBy = Tool.Trimmer | Tool.Analyzer)] // NativeAOT_StorageSpaceType set { } } + [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods), ProducedBy = Tool.Trimmer | Tool.Analyzer, CompilerGeneratedCode = true)] // NativeAOT_StorageSpaceType + public TypeRequiresPublicMethods PublicMethodsImplicitGetter => null; + public void PublicFieldsMethodParameter (TypeRequiresPublicFields param) { } [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods), ProducedBy = Tool.Trimmer | Tool.Analyzer)] // NativeAOT_StorageSpaceType public void PublicMethodsMethodParameter (TypeRequiresPublicMethods param) { } @@ -432,7 +441,8 @@ public void PublicFieldsMethodLocalVariable () TypeRequiresPublicFields t = null; } - [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods), ProducedBy = Tool.Trimmer | Tool.Analyzer)] // NativeAOT_StorageSpaceType + // The analyzer matches NativeAot behavior for local variables - it doesn't warn on generic types of local variables. + [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods), ProducedBy = Tool.Trimmer)] // NativeAOT_StorageSpaceType public void PublicMethodsMethodLocalVariable () { TypeRequiresPublicMethods t = null; @@ -880,6 +890,40 @@ static void TestGenericParameterFlowsToField () TypeRequiresPublicFields.TestFields (); } + [ExpectedWarning ("IL2091", nameof (MethodRequiresPublicFields))] + static void TestGenericParameterFlowsToDelegateMethod () + { + Action a = MethodRequiresPublicFields; + } + + [ExpectedWarning ("IL2091", nameof (DelegateMethodTypeRequiresFields), nameof (DelegateMethodTypeRequiresFields.Method))] + static void TestGenericParameterFlowsToDelegateMethodDeclaringType () + { + Action a = DelegateMethodTypeRequiresFields.Method; + } + + [ExpectedWarning ("IL2091", nameof (DelegateMethodTypeRequiresFields))] + // NativeAOT_StorageSpaceType: illink warns about the type of 'instance' local variable + [ExpectedWarning ("IL2091", nameof (DelegateMethodTypeRequiresFields), ProducedBy = Tool.Trimmer)] + // NativeAOT_StorageSpaceType: illink warns about the declaring type of 'InstanceMethod' on ldftn + [ExpectedWarning ("IL2091", nameof (DelegateMethodTypeRequiresFields), ProducedBy = Tool.Trimmer)] + static void TestGenericParameterFlowsToDelegateMethodDeclaringTypeInstance () + { + var instance = new DelegateMethodTypeRequiresFields (); + Action a = instance.InstanceMethod; + } + + class DelegateMethodTypeRequiresFields<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] T> + { + public static void Method () + { + } + + public void InstanceMethod () + { + } + } + public class TestType { } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs index a88250de5683d8..c1f98abe51c21d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaDataflow.cs @@ -116,9 +116,7 @@ static void TestAccessOnGenericMethod () static void MethodWithPublicMethodsInference<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> (T instance) { } - // https://github.com/dotnet/runtime/issues/86032 - // IL2026 should be produced by the analyzer as well, but it has a bug around inferred generic arguments - [ExpectedWarning ("IL2026", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--")] [ExpectedWarning ("IL3002", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "--AccessedThroughGenericParameterAnnotation.TypeWithRequiresMethod.MethodWhichRequires--", ProducedBy = Tool.NativeAot)] static void TestAccessOnGenericMethodWithInferenceOnMethod () diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs index 2efb414c67d16d..816bff646401c4 100644 --- a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs +++ b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs @@ -823,7 +823,9 @@ void VerifyLoggedMessages (AssemblyDefinition original, LinkerTestLogger logger, string actualName = memberDefinition.DeclaringType.FullName + "." + memberDefinition.Name; if (actualName.StartsWith (expectedMember.DeclaringType.FullName) && - actualName.Contains ("<" + expectedMember.Name + ">")) { + (actualName.Contains ("<" + expectedMember.Name + ">") || + actualName.EndsWith ("get_" + expectedMember.Name) || + actualName.EndsWith ("set_" + expectedMember.Name))) { expectedWarningFound = true; loggedMessages.Remove (loggedMessage); break;