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

Report Requires diagnostics from dataflow analyzer #92724

Merged
merged 16 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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 @@ -11,16 +11,8 @@ internal static class AnalyzerOptionsExtensions
{
public static string? GetMSBuildPropertyValue (
this AnalyzerOptions options,
string optionName,
Compilation compilation)
string optionName)
{
// MSBuild property values should be set at compilation level, and cannot have different values per-tree.
// So, we default to first syntax tree.
var tree = compilation.SyntaxTrees.FirstOrDefault ();
if (tree is null) {
return null;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't seem necessary and required passing more context (Compilation) down into the warning logic, so I removed it.


return options.AnalyzerConfigOptionsProvider.GlobalOptions.TryGetValue (
$"build_property.{optionName}", out var value)
? value
Expand All @@ -29,10 +21,9 @@ internal static class AnalyzerOptionsExtensions

public static bool IsMSBuildPropertyValueTrue (
this AnalyzerOptions options,
string propertyName,
Compilation compilation)
string propertyName)
{
var propertyValue = GetMSBuildPropertyValue (options, propertyName, compilation);
var propertyValue = GetMSBuildPropertyValue (options, propertyName);
if (!string.Equals (propertyValue?.Trim (), "true", System.StringComparison.OrdinalIgnoreCase))
return false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public override void Initialize (AnalysisContext context)
context.ConfigureGeneratedCodeAnalysis (GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.RegisterCompilationStartAction (context => {
var compilation = context.Compilation;
if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer, compilation))
if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer))
return;

context.RegisterOperationAction (operationContext => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public CapturedReferenceValue (IOperation operation)
{
switch (operation.Kind) {
case OperationKind.PropertyReference:
case OperationKind.EventReference:
case OperationKind.LocalReference:
case OperationKind.FieldReference:
case OperationKind.ParameterReference:
Expand All @@ -25,7 +26,6 @@ public CapturedReferenceValue (IOperation operation)
case OperationKind.None:
case OperationKind.InstanceReference:
case OperationKind.Invocation:
case OperationKind.EventReference:
case OperationKind.Invalid:
// These will just be ignored when referenced later.
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public void InterproceduralAnalyze ()
return;
}


Debug.Assert (Context.OwningSymbol is not IMethodSymbol methodSymbol ||
methodSymbol.MethodKind is not (MethodKind.LambdaMethod or MethodKind.LocalFunction));
var startMethod = new MethodBodyValue (Context.OwningSymbol, Context.GetControlFlowGraph (OperationBlock));
Expand All @@ -68,9 +67,6 @@ public void InterproceduralAnalyze ()
oldInterproceduralState = interproceduralState.Clone ();

foreach (var method in oldInterproceduralState.Methods) {
if (method.OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _))
continue;

AnalyzeMethod (method, ref interproceduralState);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public void Transfer (BlockProxy block, LocalDataFlowState<TValue, TValueLattice
HandleReturnValue (branchValue, branchValueOperation);
}

public abstract TValue GetFieldTargetValue (IFieldSymbol field);
public abstract TValue GetFieldTargetValue (IFieldSymbol field, IFieldReferenceOperation fieldReferenceOperation);

public abstract TValue GetParameterTargetValue (IParameterSymbol parameter);

Expand Down Expand Up @@ -165,7 +165,7 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, ISimpleAssignm
case IFieldReferenceOperation:
case IParameterReferenceOperation: {
TValue targetValue = targetOperation switch {
IFieldReferenceOperation fieldRef => GetFieldTargetValue (fieldRef.Field),
IFieldReferenceOperation fieldRef => GetFieldTargetValue (fieldRef.Field, fieldRef),
IParameterReferenceOperation parameterRef => GetParameterTargetValue (parameterRef.Parameter),
_ => throw new InvalidOperationException ()
};
Expand Down Expand Up @@ -202,6 +202,13 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, ISimpleAssignm
// even though a property setter has no return value.
return value;
}
case IEventReferenceOperation eventRef: {
// Handles assignment to an event like 'Event = Handler;', which is a write to the underlying field,
// not a call to an event accessor method. There is no Roslyn API to access the field,
// so just visit the instance and the value. https://github.com/dotnet/roslyn/issues/40103
Visit (eventRef.Instance, state);
return Visit (operation.Value, state);
}
case IImplicitIndexerReferenceOperation indexerRef: {
// An implicit reference to an indexer where the argument is a System.Index
TValue instanceValue = Visit (indexerRef.Instance, state);
Expand Down Expand Up @@ -293,10 +300,6 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, ISimpleAssignm
// also produces warnings for ref params/locals/returns.
// https://github.com/dotnet/linker/issues/2632
// https://github.com/dotnet/linker/issues/2158
case IEventReferenceOperation:
// An event assignment is an assignment to the generated backing field for
// auto-implemented events. There is no Roslyn API to access the field, so
// skip this. https://github.com/dotnet/roslyn/issues/40103
Visit (targetOperation, state);
break;

Expand Down Expand Up @@ -362,6 +365,26 @@ public override TValue VisitSimpleAssignment (ISimpleAssignmentOperation operati
return value;
}

public override TValue VisitEventAssignment (IEventAssignmentOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
{
var eventReference = (IEventReferenceOperation) operation.EventReference;
TValue instanceValue = Visit (eventReference.Instance, state);
TValue value = Visit (operation.HandlerValue, state);
if (operation.Adds) {
IMethodSymbol? addMethod = eventReference.Event.AddMethod;
Debug.Assert (addMethod != null);
if (addMethod != null)
HandleMethodCall (addMethod, instanceValue, ImmutableArray.Create (value), operation);
return value;
} else {
IMethodSymbol? removeMethod = eventReference.Event.RemoveMethod;
Debug.Assert (removeMethod != null);
if (removeMethod != null)
HandleMethodCall (removeMethod, instanceValue, ImmutableArray.Create (value), operation);
return value;
}
}

TValue GetFlowCaptureValue (IFlowCaptureReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
{
if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read)) {
Expand Down Expand Up @@ -460,6 +483,8 @@ public override TValue VisitDelegateCreation (IDelegateCreationOperation operati

public override TValue VisitPropertyReference (IPropertyReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
{
// Writing to a property should not go through this path.
Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read));
if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read))
return TopValue;

Expand All @@ -476,6 +501,19 @@ public override TValue VisitPropertyReference (IPropertyReferenceOperation opera
return HandleMethodCall (getMethod!, instanceValue, arguments.ToImmutableArray (), operation);
}

public override TValue VisitEventReference (IEventReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
{
// Writing to an event should not go through this path.
Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read));
if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read))
return TopValue;

Visit (operation.Instance, state);
// Accessing event for reading retrieves the event delegate from the event's backing field,
// so there is no method call to handle.
return TopValue;
}

public override TValue VisitImplicitIndexerReference (IImplicitIndexerReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
{
if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ public class DynamicallyAccessedMembersAnalyzer : DiagnosticAnalyzer
internal const string DynamicallyAccessedMembersAttribute = nameof (DynamicallyAccessedMembersAttribute);
public const string attributeArgument = "attributeArgument";
public const string FullyQualifiedDynamicallyAccessedMembersAttribute = "System.Diagnostics.CodeAnalysis." + DynamicallyAccessedMembersAttribute;
public static Lazy<ImmutableArray<RequiresAnalyzerBase>> RequiresAnalyzers { get; } = new Lazy<ImmutableArray<RequiresAnalyzerBase>> (GetRequiresAnalyzers);
static ImmutableArray<RequiresAnalyzerBase> GetRequiresAnalyzers () =>
ImmutableArray.Create<RequiresAnalyzerBase> (
new RequiresAssemblyFilesAnalyzer (),
new RequiresUnreferencedCodeAnalyzer (),
new RequiresDynamicCodeAnalyzer ());

public static ImmutableArray<DiagnosticDescriptor> GetSupportedDiagnostics ()
{
Expand All @@ -49,6 +55,12 @@ public static ImmutableArray<DiagnosticDescriptor> GetSupportedDiagnostics ()
diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedTypeNameInTypeGetType));
diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedParameterInMethodCreateInstance));
diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ParametersOfAssemblyCreateInstanceCannotBeAnalyzed));

foreach (var requiresAnalyzer in RequiresAnalyzers.Value) {
foreach (var diagnosticDescriptor in requiresAnalyzer.SupportedDiagnostics)
diagDescriptorsArrayBuilder.Add (diagnosticDescriptor);
}

return diagDescriptorsArrayBuilder.ToImmutable ();

void AddRange (DiagnosticId first, DiagnosticId last)
Expand All @@ -72,17 +84,16 @@ public override void Initialize (AnalysisContext context)
context.EnableConcurrentExecution ();
context.ConfigureGeneratedCodeAnalysis (GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.RegisterCompilationStartAction (context => {
if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer, context.Compilation))
if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer) &&
!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableSingleFileAnalyzer) &&
!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableAotAnalyzer))
return;

context.RegisterOperationBlockAction (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 ())
foreach (var diagnostic in trimDataFlowAnalysis.CollectDiagnostics ())
context.ReportDiagnostic (diagnostic);
}
});
Expand Down
Loading
Loading