Skip to content

Guard DAM code fix locations by compilation #131910

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.

Sign up for GitHub

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

Open
agocke wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Guard DAM code fix locations by compilation #131910

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
990f56b
Guard DAM code fix locations by compilation
agocke Aug 5, 2026
c039869
Require compilation for diagnostic contexts
agocke Aug 5, 2026
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
21 changes: 13 additions & 8 deletions src/tools/illink/src/ILLink.CodeFix/DynamicallyAccessedMembersCodeFixProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,21 +94,26 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var document = context.Document;
var diagnostic = context.Diagnostics[0];
var codeFixTitle = CodeFixTitle.ToString();

if (await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root)
return;
if (diagnostic.AdditionalLocations.Count == 0)
return;
if (root.FindNode(diagnostic.AdditionalLocations[0].SourceSpan, getInnermostNodeForTie: true) is not SyntaxNode targetNode)
if (diagnostic.AdditionalLocations[0].SourceTree is not { } targetTree)
return;
if (document.Project.Solution.GetDocument(targetTree) is not { } targetDocument)

@sbomer sbomer Aug 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This means the target document could be a different file right? Do we have test coverage for that case?

return;
if (await targetDocument.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } targetRoot)
return;
if (targetRoot.FindNode(diagnostic.AdditionalLocations[0].SourceSpan, getInnermostNodeForTie: true) is not SyntaxNode targetNode)
return;
if (diagnostic.Properties["attributeArgument"] is not string stringArgs || stringArgs.Contains(","))
if (!diagnostic.Properties.TryGetValue(DynamicallyAccessedMembersAnalyzer.attributeArgument, out string? stringArgs)
|| stringArgs is null
|| stringArgs.Contains(","))
return;

string codeFixTitle = CodeFixTitle.ToString();
context.RegisterCodeFix(CodeAction.Create(
title: CodeFixTitle.ToString(),
title: codeFixTitle,
createChangedDocument: ct => AddAttributeAsync(
document,
targetDocument,
targetNode,
stringArgs,
addAsReturnAttribute: AttributeOnReturn.Contains(diagnostic.Id),
Expand Down
97 changes: 40 additions & 57 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,22 +216,22 @@ private static void VerifyDamOnMethodsMatch(SymbolAnalysisContext context, IMeth
var baseMethodReturnAnnotation = FlowAnnotations.GetMethodReturnValueAnnotation(baseMethod);
if (overrideMethodReturnAnnotation != baseMethodReturnAnnotation)
{
Location[]? additionalLocations = null;
ImmutableDictionary<string, string?>? properties = null;
if (overrideMethodReturnAnnotation == DynamicallyAccessedMemberTypes.None
&& !overrideMethod.TryGetReturnAttribute(DynamicallyAccessedMembersAttribute, out _))
{
(additionalLocations, properties) = CreateCodeFixArguments(
context.Compilation,
GetPrimaryLocation(overrideMethod.Locations),
baseMethodReturnAnnotation);
}

(IMethodSymbol attributableMethod, DynamicallyAccessedMemberTypes missingAttribute) = GetTargetAndRequirements(overrideMethod,
baseMethod, overrideMethodReturnAnnotation, baseMethodReturnAnnotation);

Location attributableSymbolLocation = GetPrimaryLocation(attributableMethod.Locations);

// code fix does not support merging multiple attributes. If an attribute is present or the method is not in source, do not provide args for code fix.
(Location[]? sourceLocation, Dictionary<string, string?>? DAMArgs) = (!attributableSymbolLocation.IsInSource
|| (overrideMethod.TryGetReturnAttribute(DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var _)
&& baseMethod.TryGetReturnAttribute(DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var _))
) ? (null, null) : CreateArguments(attributableSymbolLocation, missingAttribute);

var returnOrigin = origin ??= overrideMethod;
var returnOrigin = origin ?? overrideMethod;
context.ReportDiagnostic(Diagnostic.Create(
DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.DynamicallyAccessedMembersMismatchOnMethodReturnValueBetweenOverrides),
GetPrimaryLocation(returnOrigin.Locations), sourceLocation, DAMArgs?.ToImmutableDictionary(), overrideMethod.GetDisplayName(), baseMethod.GetDisplayName()));
GetPrimaryLocation(returnOrigin.Locations), additionalLocations, properties,
overrideMethod.GetDisplayName(), baseMethod.GetDisplayName()));
}

foreach (var overrideParam in overrideMethod.GetMetadataParameters())
Expand All @@ -241,21 +241,21 @@ private static void VerifyDamOnMethodsMatch(SymbolAnalysisContext context, IMeth
var overrideParameterAnnotation = FlowAnnotations.GetMethodParameterAnnotation(overrideParam);
if (overrideParameterAnnotation != baseParameterAnnotation)
{
(IMethodSymbol attributableMethod, DynamicallyAccessedMemberTypes missingAttribute) = GetTargetAndRequirements(overrideMethod,
baseMethod, overrideParameterAnnotation, baseParameterAnnotation);

Location attributableSymbolLocation = attributableMethod.GetParameter(overrideParam.Index).Location!;

// code fix does not support merging multiple attributes. If an attribute is present or the method is not in source, do not provide args for code fix.
(Location[]? sourceLocation, Dictionary<string, string?>? DAMArgs) = (!attributableSymbolLocation.IsInSource
|| (overrideParam.ParameterSymbol!.TryGetAttribute(DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var _)
&& baseParam.ParameterSymbol!.TryGetAttribute(DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var _))
) ? (null, null) : CreateArguments(attributableSymbolLocation, missingAttribute);
Location[]? additionalLocations = null;
ImmutableDictionary<string, string?>? properties = null;
if (overrideParameterAnnotation == DynamicallyAccessedMemberTypes.None
&& !overrideParam.ParameterSymbol!.TryGetAttribute(DynamicallyAccessedMembersAttribute, out _))
{
(additionalLocations, properties) = CreateCodeFixArguments(

@sbomer sbomer Aug 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this still offer a code fix on a base method when the base method implements an interface? Might be worth adding a test like this:

interface I
{
    void M(
        [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
        Type type);
}

class Base
{
    public void M(Type type) { }
}

class Derived : Base, I
{
}

context.Compilation,
overrideParam.Location!,
baseParameterAnnotation);
}

var parameterOrigin = origin ?? overrideParam.ParameterSymbol;
context.ReportDiagnostic(Diagnostic.Create(
DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.DynamicallyAccessedMembersMismatchOnMethodParameterBetweenOverrides),
GetPrimaryLocation(parameterOrigin?.Locations), sourceLocation, DAMArgs?.ToImmutableDictionary(),
GetPrimaryLocation(parameterOrigin?.Locations), additionalLocations, properties,
overrideParam.GetDisplayName(), overrideMethod.GetDisplayName(), baseParam.GetDisplayName(), baseMethod.GetDisplayName()));
}
}
Expand All @@ -266,22 +266,10 @@ private static void VerifyDamOnMethodsMatch(SymbolAnalysisContext context, IMeth
var overriddenMethodTypeParameterAnnotation = baseMethod.TypeParameters[i].GetDynamicallyAccessedMemberTypes();
if (methodTypeParameterAnnotation != overriddenMethodTypeParameterAnnotation)
{

(IMethodSymbol attributableMethod, DynamicallyAccessedMemberTypes missingAttribute) = GetTargetAndRequirements(overrideMethod, baseMethod, methodTypeParameterAnnotation, overriddenMethodTypeParameterAnnotation);

var attributableSymbol = attributableMethod.TypeParameters[i];
Location attributableSymbolLocation = GetPrimaryLocation(attributableSymbol.Locations);

// code fix does not support merging multiple attributes. If an attribute is present or the method is not in source, do not provide args for code fix.
(Location[]? sourceLocation, Dictionary<string, string?>? DAMArgs) = (!attributableSymbolLocation.IsInSource
|| (overrideMethod.TypeParameters[i].TryGetAttribute(DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var _)
&& baseMethod.TypeParameters[i].TryGetAttribute(DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var _))
) ? (null, null) : CreateArguments(attributableSymbolLocation, missingAttribute);

var typeParameterOrigin = origin ?? overrideMethod.TypeParameters[i];
context.ReportDiagnostic(Diagnostic.Create(
DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.DynamicallyAccessedMembersMismatchOnGenericParameterBetweenOverrides),
GetPrimaryLocation(typeParameterOrigin.Locations), sourceLocation, DAMArgs?.ToImmutableDictionary(),
GetPrimaryLocation(typeParameterOrigin.Locations),
overrideMethod.TypeParameters[i].GetDisplayName(), overrideMethod.GetDisplayName(),
baseMethod.TypeParameters[i].GetDisplayName(), baseMethod.GetDisplayName()));
}
Expand All @@ -308,7 +296,7 @@ private static void VerifyDamOnInterfaceAndImplementationMethodsMatch(SymbolAnal
{
if (implementationMember is IMethodSymbol implementationMethod && interfaceMember is IMethodSymbol interfaceMethod)
{
ISymbol origin = implementationMethod;
ISymbol? origin = null;
INamedTypeSymbol implementationType = implementationMethod.ContainingType;

// If this type implements an interface method through a base class, the origin of the warning is this type,
Expand Down Expand Up @@ -351,29 +339,24 @@ private static void VerifyDamOnPropertyAndAccessorMatch(SymbolAnalysisContext co
}
}

private static (IMethodSymbol Method, DynamicallyAccessedMemberTypes Requirements) GetTargetAndRequirements(IMethodSymbol method, IMethodSymbol overriddenMethod, DynamicallyAccessedMemberTypes methodAnnotation, DynamicallyAccessedMemberTypes overriddenMethodAnnotation)
private static (Location[]?, ImmutableDictionary<string, string?>?) CreateCodeFixArguments(
Compilation compilation,
Location targetLocation,
DynamicallyAccessedMemberTypes annotation)
{
DynamicallyAccessedMemberTypes mismatchedArgument;
IMethodSymbol paramNeedsAttributes;
if (methodAnnotation == DynamicallyAccessedMemberTypes.None)
{
mismatchedArgument = overriddenMethodAnnotation;
paramNeedsAttributes = method;
}
else
if (targetLocation.SourceTree is not { } syntaxTree
|| !compilation.ContainsSyntaxTree(syntaxTree))
{
mismatchedArgument = methodAnnotation;
paramNeedsAttributes = overriddenMethod;
return default;
}
return (paramNeedsAttributes, mismatchedArgument);
}

private static (Location[]?, Dictionary<string, string?>?) CreateArguments(Location attributableSymbolLocation, DynamicallyAccessedMemberTypes mismatchedArgument)
{
Dictionary<string, string?>? DAMArgument = new();
Location[]? sourceLocation = new Location[] { attributableSymbolLocation };
DAMArgument.Add(DynamicallyAccessedMembersAnalyzer.attributeArgument, mismatchedArgument.ToString());
return (sourceLocation, DAMArgument);
return (
[targetLocation],
new Dictionary<string, string?>
{
[attributeArgument] = annotation.ToString()
}.ToImmutableDictionary());
Comment on lines +354 to +358
}

}
}
8 changes: 5 additions & 3 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/DAM-code-fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ The DAM warning pattern can be annotated in a way that makes the reflection usag
Once initialized, the analyzer walks the compiler-generated AST of the program to determine coherent use of DAM attributes and where they may be necessary. This is achieved by considering uses of annotated fields, methods, and parameters. If an inconsistent use is detected, the analyzer will trigger a warning and report a diagnostic.

### How information passes from Analyzer to Code Fix
The DAM Analyzer reports diagnostics that contain information about the specific warning, including the warning ID (`descriptor`), the location of the warning (`location`), the location where a code fix may be applied (`additionalLocations`), the argument to be included in the DAM attribute to be applied (`properties`), and any additional arguments (`messageArgs`). These diagnostics are then unpacked by the Code Fixer.
The DAM Analyzer reports diagnostics that carry the warning ID (`descriptor`), a primary source location (`location`), and message arguments (`messageArgs`).
Data-flow diagnostics also carry the propagated DAM requirement in `properties["attributeArgument"]` and the declaration of the symbol that needs the attribute as an additional location. The declaration location is included only when its syntax tree belongs to the current compilation. This prevents diagnostics from containing source locations from a referenced compilation.
Override and interface diagnostics use the same guarded location and property format when the local implementation is missing an attribute that is present on the related contract. They do not offer fixes that remove or replace an existing attribute.

### How the Code Fix changes the file
The Code Fix uses `SyntaxGenerator` to create the DAM attribute to add from the DAM argument passed through the `properties` dictionary. The Syntax Node that the attribute is applied to is found from the `additionalLocations` of the diagnostic. `SyntaxEditor` applies the attribute to the location specified and update the original document.
For data-flow diagnostics, the Code Fix resolves the document containing the additional location. `SyntaxGenerator` builds the DAM attribute from `properties["attributeArgument"]`, and `SyntaxEditor` applies it to that declaration. If the analyzer did not provide a local declaration location, no fix is offered.

## Future Work
1. **Multiple Arguments:** The Code Fix does not support the case where there are multiple arguments present on a node (i.e. `DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.PublicFields)`).
1. **Multiple Arguments:** The Code Fix does not support adding an attribute with multiple arguments (i.e. `DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.PublicFields`).
2. **Merging Arguments:** When there are two differing DAM attributes on nodes that should have the same attribute, we do not provide a Code Fix. However, we could read which attributes are present, merge them, and replace the attributes in both locations.
3. **Replace Checks in `DAMCodeFixProvider.AddAttributeAsync()`:** Changes to `AddAttribute()` and `AddReturnAttribute()` were made that should be updated in the `DAMCodeFixProvider` once the new Roslyn package is published and the repo uses the new package. We can remove the `addGenericParameterAttribute` check from `DAMCodeFixProvider.AddReturnAttribute()` entirely as the API will support adding a generic parameter using `AddAttribute()`. Additionally, we can replace the lambda function in the return attribute check with `AddReturnAttribute()`.
Loading
Loading