Fix trimming analyzer missing diagnostics for method calls in field access expressions

[Copilot]

Description

The trimming analyzer fails to report diagnostics when method calls are used in field access expressions. For example, type.GetInterfaceMap(@interface).TargetMethods does not warn, while type.GetInterfaceMap(@interface) correctly reports IL2072.

Root cause: TrimAnalysisVisitor.VisitFieldReference does not visit the field's instance operation before processing the field itself.

Fix: Visit the instance before processing the field, consistent with how other visitor methods handle instances (property references, event references). The fix applies to both field reads and field writes.

Changes Made

1. TrimAnalysisVisitor.cs: Added Visit(fieldRef.Instance, state); at the start of VisitFieldReference() to ensure method calls used as field instances are properly analyzed for diagnostics
2. LocalDataFlowVisitor.cs: Extended the fix to field assignments by adding instance visiting in ProcessSingleTargetAssignment for IFieldReferenceOperation cases, ensuring both field reads and writes analyze method calls in the instance expression
3. GetInterfaceDataFlow.cs: Added comprehensive test cases for the GetInterfaceMap scenario
   • Tests cover direct calls, field access, and nested field access patterns
   • Uses typeof(TestType) instead of o.GetType() to avoid unrelated IL2075 warnings
   • Uses array indexing [0] instead of LINQ for test framework compatibility
4. FieldDataFlow.cs: Updated expected warnings in AccessReturnedInstanceField test to remove tool-specific exclusions, as the fix now ensures consistent warning reporting across all tools (Analyzer, Trimmer, and NativeAot)

Testing

• ✅ Code review - no issues
• ✅ Security scan (CodeQL) - no vulnerabilities
• ✅ Test compilation - successful
• ✅ Test warnings - focused on IL2072 only, no unexpected IL2075 warnings
• ⏳ Full test suite - will be verified in CI

Risk Assessment

Low risk. This is a targeted fix that only affects field reference analysis. The change ensures that:

1. Method calls are analyzed regardless of how their return values are used (both reads and writes)
2. Behavior is consistent across all usage patterns and all tools
3. No false negatives in trimming diagnostics

This resolves the remaining cases from dotnet/linker#2832.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Type.GetInterfaceMap(Type) is treated oddly by the trimming analyzers</issue_title>
<issue_description>### Description

The second usage of GetInterfaceMap causes

IL2072: 'interfaceType' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.NonPublicMethods' in call to 'System.Type.GetInterfaceMap(Type)'. The return value of method 'System.Linq.Enumerable.First<TSource>(IEnumerable<TSource>)' 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.

while the first usage doesn't cause any warning.

I am not sure whether it's how it's supposed to be or it's an actual bug.

Reproduction Steps

object o = new();

var type = o.GetType();

var @interface = type.GetInterfaces().First();

_ = type.GetInterfaceMap(@interface).TargetMethods;

_ = type.GetInterfaceMap(@interface);

Expected behavior

I am not sure

Actual behavior

The warning is reported or not depending on whether some field is referenced or not

Regression?

No response

Known Workarounds

No response

Configuration

.NET 8

Other information

No response</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes Type.GetInterfaceMap(Type) is treated oddly by the trimming analyzers #108114

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[sbomer]

@copilot fix error:

'/__w/1/s/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/DynamicallyAccessedMembersAttribute.cs'.\n/__w/1/s/artifacts/bin/ILLink.testcases/linker_tests/DataFlow/GetInterfaceDataFlow/GetInterfaceDataFlow.cs(197,55): error CS1061: 'Type[]' does not contain a definition for 'First' and no accessible extension method 'First' accepting a first argument of type 'Type[]' could be found (are you missing a using directive or an assembly reference?)\n/__w/1/s/artifacts/bin/ILLink.testcases/linker_tests/DataFlow/GetInterfaceDataFlow/GetInterfaceDataFlow.cs(209,55): error CS1061: 'Type[]' does not contain a definition for 'First' and no accessible extension method 'First' accepting a first argument of type 'Type[]' could be found (are you missing a using directive or an assembly reference?)\n/__w/1/s/artifacts/bin/ILLink.testcases/linker_tests/DataFlow/GetInterfaceDataFlow/GetInterfaceDataFlow.cs(219,55): error CS1061: 'Type[]' does not contain a definition for 'First' and no accessible extension method 'First' accepting a first argument of type 'Type[]' could be found (are you missing a using directive or an assembly reference?)\n)\n---- System.Exception : Roslyn compilation

[Copilot]

@copilot fix error:

'/__w/1/s/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/DynamicallyAccessedMembersAttribute.cs'.\n/__w/1/s/artifacts/bin/ILLink.testcases/linker_tests/DataF...

Fixed in 8b353ad. Replaced .First() with [0] array indexing since the test framework doesn't include LINQ assembly references. This pattern is consistent with other dataflow tests (e.g., ArrayDataFlow.cs).

[sbomer]

@copilot fix this error:

Unexpected warning found: ILLink: Trim analysis warning IL2075: Mono.Linker.Tests.Cases.DataFlow.GetInterfaceDataFlow.GetInterfaceMap_DataFlow.TestDirectCall(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.Interfaces' in call to 'System.Type.GetInterfaces()'. The return value of method 'System.Object.GetType()' 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.
Unexpected warning found: ILLink: Trim analysis warning IL2075: Mono.Linker.Tests.Cases.DataFlow.GetInterfaceDataFlow.GetInterfaceMap_DataFlow.TestFieldAccess(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.Interfaces' in call to 'System.Type.GetInterfaces()'. The return value of method 'System.Object.GetType()' 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.
Unexpected warning found: ILLink: Trim analysis warning IL2075: Mono.Linker.Tests.Cases.DataFlow.GetInterfaceDataFlow.GetInterfaceMap_DataFlow.TestMultipleFieldAccess(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.Interfaces' in call to 'System.Type.GetInterfaces()'. The return value of method 'System.Object.GetType()' 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.

Be sure to run this test before committing.

[Copilot]

@copilot fix this error:

Unexpected warning found: ILLink: Trim analysis warning IL2075: Mono.Linker.Tests.Cases.DataFlow.GetInterfaceDataFlow.GetInterfaceMap_DataFlow.TestDirectCall(): 'this' argu...

Fixed in 025264e. Replaced o.GetType() with typeof(TestType) to avoid the unrelated IL2075 warnings. The tests now focus only on the IL2072 warnings for GetInterfaceMap, which is what we're testing with this fix.

[Copilot]

Pull request overview

This PR fixes a trimming analyzer bug where diagnostics were not reported when method calls appear in field access expressions. For example, type.GetInterfaceMap(@interface).TargetMethods was not warning even though @interface lacks required annotations, while type.GetInterfaceMap(@interface) correctly reports IL2072.

Changes:

• Added instance visiting in VisitFieldReference to ensure method calls in field access chains are analyzed
• Updated field assignment handling to explicitly visit instances before processing field values
• Added comprehensive test coverage for the GetInterfaceMap scenario with field access patterns

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
TrimAnalysisVisitor.cs	Added Visit(fieldRef.Instance, state) in VisitFieldReference to analyze method calls used as field instances, matching the pattern used by property and event references
LocalDataFlowVisitor.cs	Refactored assignment handling to explicitly visit field reference instances, ensuring consistency for both read and write operations
GetInterfaceDataFlow.cs	Added test class GetInterfaceMap_DataFlow with comprehensive test cases covering direct calls, field access, multiple field access, and properly annotated scenarios
FieldDataFlow.cs	Updated test expectations in AccessReturnedInstanceField to expect IL2072 warnings for all tools (removed tool-specific exclusions)

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @dotnet/illink
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[github-actions]

🤖 Copilot Code Review — PR #123040

Note

This review was generated by Copilot using multi-model analysis (Claude Opus 4.6 primary, Claude Sonnet 4 and GPT-5.3-codex sub-agents).

Holistic Assessment

Motivation: The PR fixes a confirmed bug (#108114) where the trimming analyzer silently drops diagnostics when a method call's return value is used as the instance of a field access (e.g., type.GetInterfaceMap(@interface).TargetMethods produces no IL2072 warning while type.GetInterfaceMap(@interface) does). The problem is real and the root cause is correctly identified.

Approach: The fix adds Visit(fieldRef.Instance, state) in two places — TrimAnalysisVisitor.VisitFieldReference (field reads) and LocalDataFlowVisitor.ProcessSingleTargetAssignment (field writes). This is consistent with how the codebase already handles instance visiting for property references, event references, and indexer references.

Summary: ✅ LGTM. The change is correct, targeted, and well-tested. All three models independently verified the same conclusion with no blocking or advisory issues found.

---

Detailed Findings

✅ Correctness — Fix addresses root cause

TrimAnalysisVisitor overrides VisitFieldReference, which bypasses the base DefaultVisit → VisitChildOperations path that would normally visit child operations including the instance. The fix restores instance visiting that was lost by the override. For the assignment path, ProcessSingleTargetAssignment already visited instances for properties (line 287), events (line 337), and indexers (line 343), but was missing it for fields. Both gaps are now closed.

✅ Null safety — Static fields handled correctly

For static fields, fieldRef.Instance is null. OperationWalker.Visit(null, state) returns default safely (verified in OperationWalker.cs:20-36). No risk of NRE.

✅ State ordering — Instance visited before state capture

Visit(fieldRef.Instance, state) is called before state.Current is read, which is the correct evaluation order. Any state effects from evaluating the instance (e.g., diagnostics, local updates) are reflected when building the field target value. This matches the property reference pattern.

✅ Double-visiting — Safe by design

In compound operations (e.g., ??=), the instance may be visited through both the read path (VisitFieldReference) and the write path (ProcessSingleTargetAssignment). This is explicitly accepted in the codebase — comments at lines 481-485 and 505-508 document that double-visiting is tolerated and warning deduplication prevents duplicate diagnostics. Flagged by all 3 models, all confirmed safe.

✅ Test coverage — Comprehensive

• FieldDataFlow.cs: Existing AccessReturnedInstanceField tests (read, write, ??=) updated to expect warnings from all tools, not just Trimmer/NativeAot.
• GetInterfaceDataFlow.cs: New GetInterfaceMap_DataFlow class covers: direct call (baseline), single field access (core bug), nested field access (.TargetMethods.Length), and annotated parameter (negative test confirming no false positive).

Generated by Code Review for issue #123040 · ◷

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.