Skip to content

Commit

Permalink
Simplify DAM analyzer and add conversion test (#2297)
Browse files Browse the repository at this point in the history 
* Simplify DAM analyzer and add conversion test

* PR feedback
- Take Location in helper instead of originOperation
- Avoid optional parameters
- Add test with DAM on conversion return type

* Add success testcase
  • Loading branch information
@sbomer
sbomer authored Sep 29, 2021
1 parent f2cb5fe commit 8e7b310
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 104 deletions.
141 changes: 59 additions & 82 deletions src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using ILLink.Shared;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
Expand Down Expand Up @@ -51,25 +52,52 @@ public override void Initialize (AnalysisContext context)
}, OperationKind.Return);
}

static void ProcessAssignmentOperation (OperationAnalysisContext context, IAssignmentOperation assignmentOperation)
static void CheckAndReportDynamicallyAccessedMembers (IOperation sourceOperation, IOperation targetOperation, OperationAnalysisContext context, Location location)
{
if (context.ContainingSymbol.HasAttribute (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute))
if (TryGetSymbolFromOperation (targetOperation, context) is not ISymbol target ||
TryGetSymbolFromOperation (sourceOperation, context) is not ISymbol source)
return;

if (TryGetSymbolFromOperation (assignmentOperation.Target) is not ISymbol target ||
TryGetSymbolFromOperation (assignmentOperation.Value) is not ISymbol source)
return;
CheckAndReportDynamicallyAccessedMembers (source, target, context, location, targetIsMethodReturn: false);
}

if (!target.TryGetDynamicallyAccessedMemberTypes (out var damtOnTarget))
static void CheckAndReportDynamicallyAccessedMembers (IOperation sourceOperation, ISymbol target, OperationAnalysisContext context, Location location, bool targetIsMethodReturn)
{
if (TryGetSymbolFromOperation (sourceOperation, context) is not ISymbol source)
return;

source.TryGetDynamicallyAccessedMemberTypes (out var damtOnSource);
CheckAndReportDynamicallyAccessedMembers (source, target, context, location, targetIsMethodReturn);
}

static void CheckAndReportDynamicallyAccessedMembers (ISymbol source, ISymbol target, OperationAnalysisContext context, Location location, bool targetIsMethodReturn)
{
// For the target symbol, a method symbol may represent either a "this" parameter or a method return.
// The target symbol should never be a named type.
Debug.Assert (target.Kind is not SymbolKind.NamedType);
var damtOnTarget = targetIsMethodReturn
? ((IMethodSymbol) target).GetDynamicallyAccessedMemberTypesOnReturnType ()
: target.GetDynamicallyAccessedMemberTypes ();
// For the source symbol, a named type represents a "this" parameter and a method symbol represents a method return.
var damtOnSource = source.Kind switch {
SymbolKind.NamedType => context.ContainingSymbol.GetDynamicallyAccessedMemberTypes (),
SymbolKind.Method => ((IMethodSymbol) source).GetDynamicallyAccessedMemberTypesOnReturnType (),
_ => source.GetDynamicallyAccessedMemberTypes ()
};

if (Annotations.SourceHasRequiredAnnotations (damtOnSource, damtOnTarget, out var missingAnnotations))
return;

var diag = GetDiagnosticId (source.Kind, target.Kind);
var diag = GetDiagnosticId (source.Kind, target.Kind, targetIsMethodReturn);
var diagArgs = GetDiagnosticArguments (source.Kind == SymbolKind.NamedType ? context.ContainingSymbol : source, target, missingAnnotations);
context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (diag), assignmentOperation.Syntax.GetLocation (), diagArgs));
context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (diag), location, diagArgs));
}

static void ProcessAssignmentOperation (OperationAnalysisContext context, IAssignmentOperation assignmentOperation)
{
if (context.ContainingSymbol.HasAttribute (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute))
return;

CheckAndReportDynamicallyAccessedMembers (assignmentOperation.Value, assignmentOperation.Target, context, assignmentOperation.Syntax.GetLocation ());
}

static void ProcessInvocationOperation (OperationAnalysisContext context, IInvocationOperation invocationOperation)
Expand All @@ -79,68 +107,25 @@ static void ProcessInvocationOperation (OperationAnalysisContext context, IInvoc

ProcessTypeArguments (context, invocationOperation);
ProcessArguments (context, invocationOperation);
if (!invocationOperation.TargetMethod.TryGetDynamicallyAccessedMemberTypes (out var damtOnCalledMethod))
return;

if (TryGetSymbolFromOperation (invocationOperation.Instance) is ISymbol instance &&
!instance.HasAttribute (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute)) {
instance!.TryGetDynamicallyAccessedMemberTypes (out var damtOnCaller);
if (Annotations.SourceHasRequiredAnnotations (damtOnCaller, damtOnCalledMethod, out var missingAnnotations))
return;

var diag = GetDiagnosticId (instance!.Kind, invocationOperation.TargetMethod.Kind);
var diagArgs = GetDiagnosticArguments (instance.Kind == SymbolKind.NamedType ? context.ContainingSymbol : instance, invocationOperation.TargetMethod, missingAnnotations);
context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (diag), invocationOperation.Syntax.GetLocation (), diagArgs));
}
if (invocationOperation.Instance != null)
CheckAndReportDynamicallyAccessedMembers (invocationOperation.Instance, invocationOperation.TargetMethod, context, invocationOperation.Syntax.GetLocation (), targetIsMethodReturn: false);
}

static void ProcessReturnOperation (OperationAnalysisContext context, IReturnOperation returnOperation)
{
if (!context.ContainingSymbol.TryGetDynamicallyAccessedMemberTypesOnReturnType (out var damtOnReturnType) ||
context.ContainingSymbol.HasAttribute (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute))
return;

if (TryGetSymbolFromOperation (returnOperation) is not ISymbol returnedSymbol)
return;

returnedSymbol.TryGetDynamicallyAccessedMemberTypes (out var damtOnReturnedValue);
if (Annotations.SourceHasRequiredAnnotations (damtOnReturnedValue, damtOnReturnType, out var missingAnnotations))
if (context.ContainingSymbol.HasAttribute (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute))
return;

var diag = GetDiagnosticId (returnedSymbol.Kind, context.ContainingSymbol.Kind, true);
var diagArgs = GetDiagnosticArguments (returnedSymbol.Kind == SymbolKind.NamedType ? context.ContainingSymbol : returnedSymbol, context.ContainingSymbol, missingAnnotations);
context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (diag), returnOperation.Syntax.GetLocation (), diagArgs));
CheckAndReportDynamicallyAccessedMembers (returnOperation, context.ContainingSymbol, context, returnOperation.Syntax.GetLocation (), targetIsMethodReturn: true);
}

static void ProcessArguments (OperationAnalysisContext context, IInvocationOperation invocationOperation)
{
if (context.ContainingSymbol.HasAttribute (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute))
return;

foreach (var argument in invocationOperation.Arguments) {
var targetParameter = argument.Parameter;
if (targetParameter is null || !targetParameter.TryGetDynamicallyAccessedMemberTypes (out var damtOnParameter))
return;

bool sourceIsMethodReturnType = argument.Value.Kind != OperationKind.Conversion;
ISymbol sourceArgument = sourceIsMethodReturnType ? TryGetSymbolFromOperation (argument.Value)! : context.ContainingSymbol;
if (sourceArgument is null)
return;

sourceArgument.TryGetDynamicallyAccessedMemberTypes (out var damtOnArgument);
if (Annotations.SourceHasRequiredAnnotations (damtOnArgument, damtOnParameter, out var missingAnnotations))
return;

IMethodSymbol? callerMethod = null;
// This can only happen within methods of System.Type and derived types.
if (sourceArgument.Kind == SymbolKind.Method && !sourceIsMethodReturnType) {
callerMethod = (IMethodSymbol?) sourceArgument;
sourceArgument = sourceArgument.ContainingType;
}

var diag = GetDiagnosticId (sourceArgument.Kind, targetParameter.Kind);
var diagArgs = GetDiagnosticArguments (callerMethod ?? sourceArgument, targetParameter, missingAnnotations);
context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (diag), argument.Syntax.GetLocation (), diagArgs));
var sourceSymbol = TryGetSymbolFromOperation (argument.Value, context);
if (argument.Parameter == null || sourceSymbol == null)
continue;
CheckAndReportDynamicallyAccessedMembers (sourceSymbol, argument.Parameter, context, argument.Syntax.GetLocation (), targetIsMethodReturn: false);
}
}

Expand All @@ -151,18 +136,7 @@ static void ProcessTypeArguments (OperationAnalysisContext context, IInvocationO
return;

for (int i = 0; i < targetMethod.TypeParameters.Length; i++) {
var arg = targetMethod.TypeArguments[i];
var param = targetMethod.TypeParameters[i];
if (!param.TryGetDynamicallyAccessedMemberTypes (out var damtOnTypeParameter))
continue;

arg.TryGetDynamicallyAccessedMemberTypes (out var damtOnTypeArgument);
if (Annotations.SourceHasRequiredAnnotations (damtOnTypeArgument, damtOnTypeParameter, out var missingAnnotations))
continue;

var diag = GetDiagnosticId (arg.Kind, param.Kind);
var diagArgs = GetDiagnosticArguments (arg, param, missingAnnotations);
context.ReportDiagnostic (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (diag), invocationOperation.Syntax.GetLocation (), diagArgs));
CheckAndReportDynamicallyAccessedMembers (targetMethod.TypeArguments[i], targetMethod.TypeParameters[i], context, invocationOperation.Syntax.GetLocation (), targetIsMethodReturn: false);
}
}

Expand Down Expand Up @@ -221,21 +195,24 @@ static IEnumerable<string> GetDiagnosticArguments (ISymbol symbol)
return args;
}

static ISymbol? TryGetSymbolFromOperation (IOperation? operation) =>
static ISymbol? TryGetSymbolFromOperation (IOperation? operation, OperationAnalysisContext context) =>
operation switch {
IArgumentOperation argument => TryGetSymbolFromOperation (argument.Value),
IAssignmentOperation assignment => TryGetSymbolFromOperation (assignment.Value),
IConversionOperation conversion => TryGetSymbolFromOperation (conversion.Operand),
// Represents an implicit/explicit reference to an instance. We grab the result type of this operation.
// In cases where this is an implicit reference, this will result in returning the actual type of the
// instance that 'this' represents.
IInstanceReferenceOperation instanceReference => instanceReference.Type,
IArgumentOperation argument => TryGetSymbolFromOperation (argument.Value, context),
IAssignmentOperation assignment => TryGetSymbolFromOperation (assignment.Value, context),
IConversionOperation conversion => conversion.OperatorMethod ?? TryGetSymbolFromOperation (conversion.Operand, context),
IInstanceReferenceOperation instanceReference => instanceReference.ReferenceKind switch {
InstanceReferenceKind.ContainingTypeInstance =>
context.ContainingSymbol.Kind == SymbolKind.Method
? ((IMethodSymbol) context.ContainingSymbol).ContainingType
: null,
_ => null
},
// The target method of an invocation represents the called method. If this invocation is found within
// a return operation, this representes the returned value, which might be annotated.
IInvocationOperation invocation => invocation.TargetMethod,
IMemberReferenceOperation memberReference => memberReference.Member,
IParameterReferenceOperation parameterReference => parameterReference.Parameter,
IReturnOperation returnOp => TryGetSymbolFromOperation (returnOp.ReturnedValue),
IReturnOperation returnOp => TryGetSymbolFromOperation (returnOp.ReturnedValue, context),
ITypeOfOperation typeOf => typeOf.TypeOperand,
_ => null
};
Expand Down
19 changes: 6 additions & 13 deletions src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,16 @@ internal static bool TryGetAttribute (this ISymbol member, string attributeName,
return false;
}

internal static bool TryGetDynamicallyAccessedMemberTypes (this ISymbol symbol, out DynamicallyAccessedMemberTypes? dynamicallyAccessedMemberTypes)
internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes (this ISymbol symbol)
{
dynamicallyAccessedMemberTypes = null;
if (!TryGetAttribute (symbol, DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var dynamicallyAccessedMembers))
return false;
return DynamicallyAccessedMemberTypes.None;

dynamicallyAccessedMemberTypes = (DynamicallyAccessedMemberTypes) dynamicallyAccessedMembers!.ConstructorArguments[0].Value!;
return true;
return (DynamicallyAccessedMemberTypes) dynamicallyAccessedMembers!.ConstructorArguments[0].Value!;
}

internal static bool TryGetDynamicallyAccessedMemberTypesOnReturnType (this ISymbol symbol, out DynamicallyAccessedMemberTypes? dynamicallyAccessedMemberTypes)
internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesOnReturnType (this IMethodSymbol methodSymbol)
{
dynamicallyAccessedMemberTypes = null;
if (symbol is not IMethodSymbol methodSymbol)
return false;

AttributeData? dynamicallyAccessedMembers = null;
foreach (var returnTypeAttribute in methodSymbol.GetReturnTypeAttributes ())
if (returnTypeAttribute.AttributeClass is var attrClass && attrClass != null &&
Expand All @@ -61,10 +55,9 @@ internal static bool TryGetDynamicallyAccessedMemberTypesOnReturnType (this ISym
}

if (dynamicallyAccessedMembers == null)
return false;
return DynamicallyAccessedMemberTypes.None;

dynamicallyAccessedMemberTypes = (DynamicallyAccessedMemberTypes) dynamicallyAccessedMembers.ConstructorArguments[0].Value!;
return true;
return (DynamicallyAccessedMemberTypes) dynamicallyAccessedMembers.ConstructorArguments[0].Value!;
}

internal static bool TryGetOverriddenMember (this ISymbol? symbol, [NotNullWhen (returnValue: true)] out ISymbol? overridenMember)
Expand Down
15 changes: 6 additions & 9 deletions src/ILLink.Shared/Annotations.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,20 @@ namespace ILLink.Shared
internal static class Annotations
{
public static bool SourceHasRequiredAnnotations (
DynamicallyAccessedMemberTypes? sourceMemberTypes,
DynamicallyAccessedMemberTypes? targetMemberTypes,
DynamicallyAccessedMemberTypes sourceMemberTypes,
DynamicallyAccessedMemberTypes targetMemberTypes,
out string missingMemberTypesString)
{
missingMemberTypesString = string.Empty;
if (targetMemberTypes == null)
return true;

sourceMemberTypes ??= DynamicallyAccessedMemberTypes.None;
var missingMemberTypesList = Enum.GetValues (typeof (DynamicallyAccessedMemberTypes))
.Cast<DynamicallyAccessedMemberTypes> ()
.Where (damt => (damt & targetMemberTypes & ~sourceMemberTypes) == damt && damt != DynamicallyAccessedMemberTypes.None)
.ToList ();

if (targetMemberTypes.Value.HasFlag (DynamicallyAccessedMemberTypes.PublicConstructors) &&
sourceMemberTypes.Value.HasFlag (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor) &&
!sourceMemberTypes.Value.HasFlag (DynamicallyAccessedMemberTypes.PublicConstructors))
if (targetMemberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicConstructors) &&
sourceMemberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor) &&
!sourceMemberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicConstructors))
missingMemberTypesList.Add (DynamicallyAccessedMemberTypes.PublicConstructors);

if (missingMemberTypesList.Contains (DynamicallyAccessedMemberTypes.PublicConstructors) &&
Expand All @@ -37,7 +34,7 @@ public static bool SourceHasRequiredAnnotations (
if (missingMemberTypesList.Count == 0)
return true;

missingMemberTypesString = targetMemberTypes.Value == DynamicallyAccessedMemberTypes.All
missingMemberTypesString = targetMemberTypes == DynamicallyAccessedMemberTypes.All
? $"'{nameof (DynamicallyAccessedMemberTypes)}.{nameof (DynamicallyAccessedMemberTypes.All)}'"
: string.Join (", ", missingMemberTypesList.Select (mmt => $"'{nameof (DynamicallyAccessedMemberTypes)}.{mmt}'"));

Expand Down
Loading

0 comments on commit 8e7b310

Please sign in to comment.