Follow-up: Additional improvements needed for .NET Framework instrumentation type import (PR #1824)

Summary

PR #1824 addresses the core issue of ensuring type/method references use the target module's core library (mscorlib vs System.Runtime). However, the Copilot code review identified several additional improvements that should be implemented in a follow-up to ensure complete and robust handling of all type reference scenarios.

Background

This issue tracks the unresolved review comments from PR #1824 which fixes issue #1818. The current implementation correctly handles basic type imports but has some edge cases that need additional work.

Unresolved Items

1. Return Type Not Imported When Cloning ModuleTrackerTemplate Methods

Location: Instrumenter.cs lines 359-364

When cloning ModuleTrackerTemplate methods, the return type is copied from the template module (methodDef.ReturnType) without being imported into the target module's type system. This can leave return type references scoped to the coverlet assembly's core library (e.g., System.Runtime) when instrumenting .NET Framework targets.

Required Fix: Import/redirect the method return type using the same core-library helper used for parameters/locals.

2. Return Type Not Redirected for Updated Method References

Location: Instrumenter.cs lines 389-394

For method references moved onto the custom tracker type, updatedMethodReference uses methodReference.ReturnType directly (not imported/redirected). This can keep System.Runtime-scoped return types in the injected IL for .NET Framework targets.

Required Fix: The return type should be imported/redirected consistently with parameters (use the core-library import helper for the return type as well).

3. Generic Type Arguments Not Handled in ImportMethodToCoreLibrary

Location: Instrumenter.cs lines 462-485

ImportMethodToCoreLibrary only rewrites DeclaringType, parameter types, and return type element scopes. This misses nested type references for generic instance methods/types (e.g., Interlocked.Exchange<T> used in ModuleTrackerTemplate), where the generic arguments can still remain scoped to System.Runtime.

Required Fix: Consider recursively redirecting scopes for all nested TypeReferences:

• Generic arguments
• Modifiers
• Arrays/byref/pointers
• Avoid trying to set Scope on GenericParameter types

4. Code Duplication with CoreLibMetadataImporterProvider

Location: Instrumenter.cs lines 439-456

The new ImportToCoreLibrary/ImportMethodToCoreLibrary logic largely duplicates the existing core-library redirection logic in CoreLibMetadataImporterProvider later in the same file.

Required Fix: Refactor to share a single implementation (e.g., a common helper that both paths call) to reduce the risk of future divergence.

Acceptance Criteria

• Method return types are properly imported/redirected when cloning ModuleTrackerTemplate methods
• Method return types are properly redirected for updated method references
• Generic type arguments are recursively handled for complete scope redirection
• GenericParameter types are properly handled (scope should not be set on them)
• Duplicated core-library redirection logic is consolidated into a shared helper
• Unit tests verify all edge cases including generic methods like Interlocked.Exchange<T>

Related

• Fixes for issue [BUG] TypeInitializationException when targeting .NET Framework #1818
• PR Ensure type/method refs use target module's core library #1824

Labels

• enhancement
• coverlet-core
• netfx