Fixes https://github.com/dotnet/runtime/issues/86032.
This replaces the syntax-based logic with:
- ISymbol-based logic that looks at generic instantiations in
parameter types, return types, and base/interface types
- IOperation-based logic that looks at invocations
The syntax-based logic used to also warn for the types of local
variables, but `SymbolKind.Local` isn't supported for
`RegisterSymbolAction`. Since this falls into the category of
warnings that isn't strictly necessary (based on nativeaot logic
which only warns on generic instantiations that can actually lead
to code execution), I just left out local variables.
I'm slightly concerned that this could be missing some cases, but
at least it works on our existing testcases. This also adds tests
for delegate creation using generic methods, which was one such
missing case noticed during code review.
There are two slight differences in warning locations, which I
don't expect to cause issues for warning suppressions:
- For invocations, the warning location used to be the generic
method name, without the argument list. Using the
IOperation-based approach, we use the invocation's location
that includes the argument list.
- For properties that use arrow syntax (`RequireMethods<TFields>
Property => null`), the ISymbol-based approach warns on the
underlying getter method, whose location seems to be the body
of the accessor. We used to warn on the return type
specifically, but I couldn't find a better way to get the
location.
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
namespace ILLink.RoslynAnalyzer
{
if (context.OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _))
return;
-
foreach (var operationBlock in context.OperationBlocks) {
TrimDataFlowAnalysis trimDataFlowAnalysis = new (context, operationBlock);
trimDataFlowAnalysis.InterproceduralAnalyze ();
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);
});
}
- static void ProcessGenericParameters (SyntaxNodeAnalysisContext context)
+ static IEnumerable<Diagnostic> 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<Diagnostic> 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<ITypeParameterSymbol> typeParams = default;
ImmutableArray<ITypeSymbol> typeArgs = default;
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;
}
}
}
<Target Name="UseLocalILLinkAnalyzer" BeforeTargets="CoreCompile">
<ItemGroup>
<Analyzer Remove="@(Analyzer)" Condition="'%(Filename)' == 'ILLink.RoslynAnalyzer'" />
- <Analyzer Include="/path/to/linker/repo/artifacts/bin/ILLink.RoslynAnalyzer/Debug/netstandard2.0/ILLink.RoslynAnalyzer.dll" />
+ <Analyzer Include="path/to/runtime/artifacts/bin/ILLink.RoslynAnalyzer/Debug/netstandard2.0/ILLink.RoslynAnalyzer.dll" />
</ItemGroup>
</Target>
```
\ No newline at end of file
// 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<T>()", "S", "C.M2<S>()", "'DynamicallyAccessedMemberTypes.PublicMethods'"));
}
// 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<TInner>'. The generic parameter 'TOuter' of 'C<TOuter>' 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<TInner>", "TOuter", "C<TOuter>", "'DynamicallyAccessedMemberTypes.PublicMethods'"));
+ VerifyCS.Diagnostic (DiagnosticId.DynamicallyAccessedMembersMismatchTypeArgumentTargetsGenericParameter).WithSpan (11, 36, 11, 57).WithSpan (4, 9, 4, 15).WithArguments ("TInner", "CRequires<TInner>", "TOuter", "C<TOuter>", "'DynamicallyAccessedMemberTypes.PublicMethods'"));
}
}
}
// The generic parameter 'S' of 'C.M2<S>()' 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<T>()",
}
[Fact]
- public async Task CodeFix_IL2091_AttrbuteTurnsOffCodeFix ()
+ public async Task CodeFix_IL2091_AttributeTurnsOffCodeFix ()
{
var test = $$"""
using System.Diagnostics.CodeAnalysis;
// The generic parameter 'S' of 'C.M2<S>()' 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<T>()",
"S",
-// 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;
TestGenericParameterFlowsToField ();
TestGenericParameterFlowsToReturnValue ();
+ TestGenericParameterFlowsToDelegateMethod<TestType> ();
+ TestGenericParameterFlowsToDelegateMethodDeclaringType<TestType> ();
+ TestGenericParameterFlowsToDelegateMethodDeclaringTypeInstance<TestType> ();
+
TestNoWarningsInRUCMethod<TestType> ();
TestNoWarningsInRUCType<TestType, TestType> ();
}
[ExpectedWarning ("IL2109", nameof (BaseTypeWithOpenGenericDAMTAndRUC<T>))]
[ExpectedWarning ("IL2091", nameof (BaseTypeWithOpenGenericDAMTAndRUC<T>))]
- class DerivedTypeWithOpenGenericOnBaseWithRUCOnBase<T> : BaseTypeWithOpenGenericDAMTAndRUC<T>
+ [ExpectedWarning ("IL2091", nameof (IGenericInterfaceTypeWithRequirements<T>))]
+ class DerivedTypeWithOpenGenericOnBaseWithRUCOnBase<T> : BaseTypeWithOpenGenericDAMTAndRUC<T>, IGenericInterfaceTypeWithRequirements<T>
{
[ExpectedWarning ("IL2091", nameof (DerivedTypeWithOpenGenericOnBaseWithRUCOnBase<T>), ProducedBy = Tool.Trimmer | Tool.NativeAot)]
[ExpectedWarning ("IL2026", nameof (BaseTypeWithOpenGenericDAMTAndRUC<T>), ProducedBy = Tool.Trimmer | Tool.NativeAot)]
new DerivedTypeWithOpenGenericOnBaseWithRUCOnDerived<TestType> ();
}
[ExpectedWarning ("IL2091", nameof (BaseTypeWithOpenGenericDAMT<T>))]
+ [ExpectedWarning ("IL2091", nameof (IGenericInterfaceTypeWithRequirements<T>))]
[RequiresUnreferencedCode ("RUC")]
- class DerivedTypeWithOpenGenericOnBaseWithRUCOnDerived<T> : BaseTypeWithOpenGenericDAMT<T>
+ class DerivedTypeWithOpenGenericOnBaseWithRUCOnDerived<T> : BaseTypeWithOpenGenericDAMT<T>, IGenericInterfaceTypeWithRequirements<T>
{
}
instance.PublicFieldsProperty = null;
_ = instance.PublicMethodsProperty;
instance.PublicMethodsProperty = null;
+ _ = instance.PublicMethodsImplicitGetter;
instance.PublicFieldsMethodParameter (null);
instance.PublicMethodsMethodParameter (null);
set;
}
- [ExpectedWarning ("IL2091", nameof (TypeGenericRequirementsOnMembers<TOuter>), ProducedBy = Tool.Analyzer)]
public TypeRequiresPublicMethods<TOuter> PublicMethodsProperty {
- [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods<TOuter>), ProducedBy = Tool.Trimmer)] // NativeAOT_StorageSpaceType
+ [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods<TOuter>), ProducedBy = Tool.Trimmer | Tool.Analyzer)] // NativeAOT_StorageSpaceType
get => null;
- [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods<TOuter>), ProducedBy = Tool.Trimmer)] // NativeAOT_StorageSpaceType
+ [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods<TOuter>), ProducedBy = Tool.Trimmer | Tool.Analyzer)] // NativeAOT_StorageSpaceType
set { }
}
+ [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods<TOuter>), ProducedBy = Tool.Trimmer | Tool.Analyzer, CompilerGeneratedCode = true)] // NativeAOT_StorageSpaceType
+ public TypeRequiresPublicMethods<TOuter> PublicMethodsImplicitGetter => null;
+
public void PublicFieldsMethodParameter (TypeRequiresPublicFields<TOuter> param) { }
[ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods<TOuter>), ProducedBy = Tool.Trimmer | Tool.Analyzer)] // NativeAOT_StorageSpaceType
public void PublicMethodsMethodParameter (TypeRequiresPublicMethods<TOuter> param) { }
TypeRequiresPublicFields<TOuter> t = null;
}
- [ExpectedWarning ("IL2091", nameof (TypeRequiresPublicMethods<TOuter>), 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<TOuter>), ProducedBy = Tool.Trimmer)] // NativeAOT_StorageSpaceType
public void PublicMethodsMethodLocalVariable ()
{
TypeRequiresPublicMethods<TOuter> t = null;
TypeRequiresPublicFields<TestType>.TestFields ();
}
+ [ExpectedWarning ("IL2091", nameof (MethodRequiresPublicFields))]
+ static void TestGenericParameterFlowsToDelegateMethod<T> ()
+ {
+ Action a = MethodRequiresPublicFields<T>;
+ }
+
+ [ExpectedWarning ("IL2091", nameof (DelegateMethodTypeRequiresFields<T>), nameof (DelegateMethodTypeRequiresFields<T>.Method))]
+ static void TestGenericParameterFlowsToDelegateMethodDeclaringType<T> ()
+ {
+ Action a = DelegateMethodTypeRequiresFields<T>.Method;
+ }
+
+ [ExpectedWarning ("IL2091", nameof (DelegateMethodTypeRequiresFields<T>))]
+ // NativeAOT_StorageSpaceType: illink warns about the type of 'instance' local variable
+ [ExpectedWarning ("IL2091", nameof (DelegateMethodTypeRequiresFields<T>), ProducedBy = Tool.Trimmer)]
+ // NativeAOT_StorageSpaceType: illink warns about the declaring type of 'InstanceMethod' on ldftn
+ [ExpectedWarning ("IL2091", nameof (DelegateMethodTypeRequiresFields<T>), ProducedBy = Tool.Trimmer)]
+ static void TestGenericParameterFlowsToDelegateMethodDeclaringTypeInstance<T> ()
+ {
+ var instance = new DelegateMethodTypeRequiresFields<T> ();
+ Action a = instance.InstanceMethod;
+ }
+
+ class DelegateMethodTypeRequiresFields<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] T>
+ {
+ public static void Method ()
+ {
+ }
+
+ public void InstanceMethod ()
+ {
+ }
+ }
+
public class TestType
{
}
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 ()
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;
projects / platform / upstream / dotnet / runtime.git / commitdiff