Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix trim analyzer warning for inferred type arguments #87156

Merged
merged 6 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace ILLink.RoslynAnalyzer
{
Expand Down Expand Up @@ -76,17 +77,94 @@ 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 ();
foreach (var diagnostic in trimDataFlowAnalysis.TrimAnalysisPatterns.CollectDiagnostics ())
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;
sbomer marked this conversation as resolved.
Show resolved Hide resolved

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);
Expand All @@ -104,36 +182,22 @@ public override void Initialize (AnalysisContext context)
});
}

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;
Expand All @@ -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;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/illink/src/ILLink.RoslynAnalyzer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ To use a local build of the analyzer in another project, modify the `Analyzer` I
<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>
```
Original file line number Diff line number Diff line change
Expand Up @@ -1112,7 +1112,7 @@ private static void M2<S>()
// 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'"));
}
Expand Down Expand Up @@ -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<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'"));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2529,7 +2529,7 @@ await VerifyDynamicallyAccessedMembersCodeFix (
// 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>()",
Expand All @@ -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;
Expand All @@ -2568,7 +2568,7 @@ public static void Main()
// 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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -42,6 +42,10 @@ public static void Main ()
TestGenericParameterFlowsToField ();
TestGenericParameterFlowsToReturnValue ();

TestGenericParameterFlowsToDelegateMethod<TestType> ();
TestGenericParameterFlowsToDelegateMethodDeclaringType<TestType> ();
TestGenericParameterFlowsToDelegateMethodDeclaringTypeInstance<TestType> ();

TestNoWarningsInRUCMethod<TestType> ();
TestNoWarningsInRUCType<TestType, TestType> ();
}
Expand Down Expand Up @@ -211,7 +215,8 @@ static void TestDerivedTypeWithOpenGenericOnBaseWithRUCOnBase ()

[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)]
Expand All @@ -228,8 +233,9 @@ static void TestDerivedTypeWithOpenGenericOnBaseWithRUCOnDerived ()
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>
{
}

Expand Down Expand Up @@ -386,6 +392,7 @@ static void TestTypeGenericRequirementsOnMembers ()
instance.PublicFieldsProperty = null;
_ = instance.PublicMethodsProperty;
instance.PublicMethodsProperty = null;
_ = instance.PublicMethodsImplicitGetter;

instance.PublicFieldsMethodParameter (null);
instance.PublicMethodsMethodParameter (null);
Expand All @@ -409,14 +416,16 @@ public TypeRequiresPublicFields<TOuter> PublicFieldsProperty {
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) { }
Expand All @@ -432,7 +441,8 @@ public void PublicFieldsMethodLocalVariable ()
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;
Expand Down Expand Up @@ -880,6 +890,40 @@ static void TestGenericParameterFlowsToField ()
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
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down