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

Make ReflectionAccessAnalyzer logic more similar to ILLink/ILC #105956

Merged
merged 2 commits into from
Aug 6, 2024
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 @@ -98,8 +98,7 @@ public override void Initialize (AnalysisContext context)
foreach (var operationBlock in context.OperationBlocks) {
TrimDataFlowAnalysis trimDataFlowAnalysis = new (context, dataFlowAnalyzerContext, operationBlock);
trimDataFlowAnalysis.InterproceduralAnalyze ();
foreach (var diagnostic in trimDataFlowAnalysis.CollectDiagnostics ())
context.ReportDiagnostic (diagnostic);
trimDataFlowAnalysis.ReportDiagnostics (context.ReportDiagnostic);
}
});

Expand All @@ -117,16 +116,12 @@ public override void Initialize (AnalysisContext context)
return;

var location = GetPrimaryLocation (type.Locations);
DiagnosticContext diagnosticContext = new (location);

if (type.BaseType is INamedTypeSymbol baseType)
GenericArgumentDataFlow.ProcessGenericArgumentDataFlow (diagnosticContext, baseType);
GenericArgumentDataFlow.ProcessGenericArgumentDataFlow (location, baseType, context.ReportDiagnostic);

foreach (var interfaceType in type.Interfaces)
GenericArgumentDataFlow.ProcessGenericArgumentDataFlow (diagnosticContext, interfaceType);

foreach (var diagnostic in diagnosticContext.Diagnostics)
context.ReportDiagnostic (diagnostic);
GenericArgumentDataFlow.ProcessGenericArgumentDataFlow (location, interfaceType, context.ReportDiagnostic);
}, SymbolKind.NamedType);
context.RegisterSymbolAction (context => {
VerifyMemberOnlyApplyToTypesOrStrings (context, context.Symbol);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,36 +12,35 @@ namespace ILLink.Shared.TrimAnalysis
{
public readonly partial struct DiagnosticContext
{
public List<Diagnostic> Diagnostics { get; } = new ();
public readonly Location Location { get; }

readonly Location? Location { get; init; }
readonly Action<Diagnostic>? _reportDiagnostic;

public DiagnosticContext (Location location)
public DiagnosticContext (Location location, Action<Diagnostic>? reportDiagnostic)
{
Location = location;
_reportDiagnostic = reportDiagnostic;
}

public static DiagnosticContext CreateDisabled () => new () { Location = null };

public Diagnostic CreateDiagnostic (DiagnosticId id, params string[] args)
private Diagnostic CreateDiagnostic (DiagnosticId id, params string[] args)
{
return Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (id), Location, args);
}

public partial void AddDiagnostic (DiagnosticId id, params string[] args)
{
if (Location == null)
if (_reportDiagnostic is null)
return;

Diagnostics.Add (CreateDiagnostic (id, args));
_reportDiagnostic (CreateDiagnostic (id, args));
}

public partial void AddDiagnostic (DiagnosticId id, ValueWithDynamicallyAccessedMembers actualValue, ValueWithDynamicallyAccessedMembers expectedAnnotationsValue, params string[] args)
{
if (Location == null)
if (_reportDiagnostic is null)
return;

Diagnostics.Add (CreateDiagnostic (id, actualValue, expectedAnnotationsValue, args));
_reportDiagnostic (CreateDiagnostic (id, actualValue, expectedAnnotationsValue, args));
}

private Diagnostic CreateDiagnostic (DiagnosticId id, ValueWithDynamicallyAccessedMembers actualValue, ValueWithDynamicallyAccessedMembers expectedAnnotationsValue, params string[] args)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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;
using System.Collections.Generic;
using ILLink.Shared;
using ILLink.Shared.DataFlow;
Expand Down Expand Up @@ -29,22 +30,22 @@ public FeatureCheckReturnValuePattern (
OwningSymbol = owningSymbol;
}

public IEnumerable<Diagnostic> CollectDiagnostics (DataFlowAnalyzerContext context)
public void ReportDiagnostics (DataFlowAnalyzerContext context, Action<Diagnostic> reportDiagnostic)
{
var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ());
var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation (), reportDiagnostic);
// For now, feature check validation is enabled only when trim analysis is enabled.
if (!context.EnableTrimAnalyzer)
return diagnosticContext.Diagnostics;
return;

if (!OwningSymbol.IsStatic || OwningSymbol.Type.SpecialType != SpecialType.System_Boolean || OwningSymbol.SetMethod != null) {
// Warn about invalid feature checks (non-static or non-bool properties or properties with setter)
diagnosticContext.AddDiagnostic (
DiagnosticId.InvalidFeatureGuard);
return diagnosticContext.Diagnostics;
return;
}

if (ReturnValue == FeatureChecksValue.All)
return diagnosticContext.Diagnostics;
return;

ValueSet<string> returnValueFeatures = ReturnValue.EnabledFeatures;
// For any analyzer-supported feature that this property is declared to guard,
Expand All @@ -63,8 +64,6 @@ public IEnumerable<Diagnostic> CollectDiagnostics (DataFlowAnalyzerContext conte
}
}
}

return diagnosticContext.Diagnostics;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,55 +2,54 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using ILLink.Shared.DataFlow;
using ILLink.Shared.TrimAnalysis;
using ILLink.Shared.TypeSystemProxy;

namespace ILLink.RoslynAnalyzer.TrimAnalysis
{
internal static class GenericArgumentDataFlow
{
public static void ProcessGenericArgumentDataFlow (DiagnosticContext diagnosticContext, INamedTypeSymbol type)
public static void ProcessGenericArgumentDataFlow (Location location, INamedTypeSymbol type, Action<Diagnostic> reportDiagnostic)
{
ProcessGenericArgumentDataFlow (diagnosticContext, type.TypeArguments, type.TypeParameters);
ProcessGenericArgumentDataFlow (location, type.TypeArguments, type.TypeParameters, reportDiagnostic);
}

public static void ProcessGenericArgumentDataFlow (DiagnosticContext diagnosticContext, IMethodSymbol method)
public static void ProcessGenericArgumentDataFlow (Location location, IMethodSymbol method, Action<Diagnostic> reportDiagnostic)
{
ProcessGenericArgumentDataFlow (diagnosticContext, method.TypeArguments, method.TypeParameters);
ProcessGenericArgumentDataFlow (location, method.TypeArguments, method.TypeParameters, reportDiagnostic);

ProcessGenericArgumentDataFlow (diagnosticContext, method.ContainingType);
ProcessGenericArgumentDataFlow (location, method.ContainingType, reportDiagnostic);
}

public static void ProcessGenericArgumentDataFlow (DiagnosticContext diagnosticContext, IFieldSymbol field)
public static void ProcessGenericArgumentDataFlow (Location location, IFieldSymbol field, Action<Diagnostic> reportDiagnostic)
{
ProcessGenericArgumentDataFlow (diagnosticContext, field.ContainingType);
ProcessGenericArgumentDataFlow (location, field.ContainingType, reportDiagnostic);
}

static void ProcessGenericArgumentDataFlow (
DiagnosticContext diagnosticContext,
Location location,
ImmutableArray<ITypeSymbol> typeArguments,
ImmutableArray<ITypeParameterSymbol> typeParameters)
ImmutableArray<ITypeParameterSymbol> typeParameters,
Action<Diagnostic> reportDiagnostic)
{
var diagnosticContext = new DiagnosticContext (location, reportDiagnostic);
for (int i = 0; i < typeArguments.Length; i++) {
var typeArgument = typeArguments[i];
// Apply annotations to the generic argument
var genericParameterValue = new GenericParameterValue (typeParameters[i]);
if (genericParameterValue.DynamicallyAccessedMemberTypes != DynamicallyAccessedMemberTypes.None) {
SingleValue genericArgumentValue = SingleValueExtensions.FromTypeSymbol (typeArgument)!;
var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction (diagnosticContext, default (ReflectionAccessAnalyzer));
var reflectionAccessAnalyzer = new ReflectionAccessAnalyzer (reportDiagnostic);
var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction (diagnosticContext, reflectionAccessAnalyzer);
requireDynamicallyAccessedMembersAction.Invoke (genericArgumentValue, genericParameterValue);
}

// Recursively process generic argument data flow on the generic argument if it itself is generic
if (typeArgument is INamedTypeSymbol namedTypeArgument && namedTypeArgument.IsGenericType) {
ProcessGenericArgumentDataFlow (diagnosticContext, namedTypeArgument);
}
if (typeArgument is INamedTypeSymbol namedTypeArgument && namedTypeArgument.IsGenericType)
ProcessGenericArgumentDataFlow (location, namedTypeArgument, reportDiagnostic);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
Expand All @@ -27,18 +28,19 @@ internal partial struct HandleCallAction
ValueSetLattice<SingleValue> _multiValueLattice;

public HandleCallAction (
in DiagnosticContext diagnosticContext,
Location location,
ISymbol owningSymbol,
IOperation operation,
ValueSetLattice<SingleValue> multiValueLattice)
ValueSetLattice<SingleValue> multiValueLattice,
Action<Diagnostic>? reportDiagnostic)
{
_owningSymbol = owningSymbol;
_operation = operation;
_isNewObj = operation.Kind == OperationKind.ObjectCreation;
_diagnosticContext = diagnosticContext;
_diagnosticContext = new DiagnosticContext (location, reportDiagnostic);
_annotations = FlowAnnotations.Instance;
_reflectionAccessAnalyzer = default;
_requireDynamicallyAccessedMembersAction = new (diagnosticContext, _reflectionAccessAnalyzer);
_reflectionAccessAnalyzer = new (reportDiagnostic);
_requireDynamicallyAccessedMembersAction = new (_diagnosticContext, _reflectionAccessAnalyzer);
_multiValueLattice = multiValueLattice;
}

Expand Down Expand Up @@ -201,25 +203,25 @@ private partial bool TryResolveTypeNameForCreateInstanceAndMark (in MethodProxy
}

private partial void MarkStaticConstructor (TypeProxy type)
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForConstructorsOnType (_diagnosticContext, type.Type, BindingFlags.Static, parameterCount: 0);
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForConstructorsOnType (_diagnosticContext.Location, type.Type, BindingFlags.Static, parameterCount: 0);

private partial void MarkEventsOnTypeHierarchy (TypeProxy type, string name, BindingFlags? bindingFlags)
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForEventsOnTypeHierarchy (_diagnosticContext, type.Type, name, bindingFlags);
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForEventsOnTypeHierarchy (_diagnosticContext.Location, type.Type, name, bindingFlags);

private partial void MarkFieldsOnTypeHierarchy (TypeProxy type, string name, BindingFlags? bindingFlags)
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForFieldsOnTypeHierarchy (_diagnosticContext, type.Type, name, bindingFlags);
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForFieldsOnTypeHierarchy (_diagnosticContext.Location, type.Type, name, bindingFlags);

private partial void MarkPropertiesOnTypeHierarchy (TypeProxy type, string name, BindingFlags? bindingFlags)
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForPropertiesOnTypeHierarchy (_diagnosticContext, type.Type, name, bindingFlags);
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForPropertiesOnTypeHierarchy (_diagnosticContext.Location, type.Type, name, bindingFlags);

private partial void MarkPublicParameterlessConstructorOnType (TypeProxy type)
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForPublicParameterlessConstructor (_diagnosticContext, type.Type);
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForPublicParameterlessConstructor (_diagnosticContext.Location, type.Type);

private partial void MarkConstructorsOnType (TypeProxy type, BindingFlags? bindingFlags, int? parameterCount)
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForConstructorsOnType (_diagnosticContext, type.Type, bindingFlags, parameterCount);
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForConstructorsOnType (_diagnosticContext.Location, type.Type, bindingFlags, parameterCount);

private partial void MarkMethod (MethodProxy method)
=> ReflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForMethod (_diagnosticContext, method.Method);
=> _reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForMethod (_diagnosticContext.Location, method.Method);

// TODO: Does the analyzer need to do something here?
private partial void MarkType (TypeProxy type) { }
Expand All @@ -229,7 +231,7 @@ private partial bool MarkAssociatedProperty (MethodProxy method)
if (method.Method.MethodKind == MethodKind.PropertyGet || method.Method.MethodKind == MethodKind.PropertySet) {
var property = (IPropertySymbol) method.Method.AssociatedSymbol!;
Debug.Assert (property != null);
ReflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForProperty (_diagnosticContext, property!);
_reflectionAccessAnalyzer.GetReflectionAccessDiagnosticsForProperty (_diagnosticContext.Location, property!);
return true;
}

Expand Down
Loading
Loading