Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 4, 2025

This PR fixes namespace collision issues in source-generated code by ensuring all type references use global qualification (global::). Previously, generated code could reference types without the global:: prefix, which could cause compilation errors when user code defines types with the same names as System types.

Problem

The source generators were emitting code with unqualified type references like:

  • Task.CompletedTask instead of global::System.Threading.Tasks.Task.CompletedTask
  • [System.Runtime.CompilerServices.ModuleInitializer] instead of [global::System.Runtime.CompilerServices.ModuleInitializer]
  • [System.CodeDom.Compiler.GeneratedCode(...)] instead of [global::System.CodeDom.Compiler.GeneratedCode(...)]

This could cause namespace collisions if user code defined their own Task, ModuleInitializer, or other similarly named types.

Solution

Updated all source generators to use globally qualified type names:

Fixed Type References

  • Task.CompletedTaskglobal::System.Threading.Tasks.Task.CompletedTask
  • System.Runtime.CompilerServices.ModuleInitializerglobal::System.Runtime.CompilerServices.ModuleInitializer
  • System.CodeDom.Compiler.GeneratedCodeglobal::System.CodeDom.Compiler.GeneratedCode
  • System.Runtime.CompilerServices.UnsafeAccessorglobal::System.Runtime.CompilerServices.UnsafeAccessor

Files Modified

  • TestMetadataGenerator.cs - Fixed Task.CompletedTask and ModuleInitializer references
  • PropertyInjectionSourceGenerator.cs - Fixed ModuleInitializer and UnsafeAccessor references
  • AotConverterGenerator.cs - Fixed ModuleInitializer references
  • AotModuleInitializerGenerator.cs - Fixed ModuleInitializer references
  • HookMetadataGenerator.cs - Fixed ModuleInitializer references
  • DisableReflectionScannerGenerator.cs - Fixed GeneratedCode references
  • AssemblyLoaderGenerator.cs - Fixed GeneratedCode references
  • DynamicTestsGenerator.cs - Fixed GeneratedCode references

Example Scenario

Before this fix, the following user code could cause compilation errors:

namespace MyTests
{
    // User defines their own Task class
    public class Task
    {
        public static Task CompletedTask => new Task();
    }
    
    public class TestClass
    {
        [Test]
        public void MyTest()
        {
            // Generated code would reference MyTests.Task instead of System.Threading.Tasks.Task
            // causing compilation errors
        }
    }
}

After this fix, the generated code correctly uses global::System.Threading.Tasks.Task.CompletedTask, avoiding the collision.

Testing

  • All source generator tests pass with updated verification files
  • Public API tests confirm no breaking changes
  • Build succeeds without new warnings or errors

Fixes #2752.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@thomhurst thomhurst marked this pull request as ready for review August 4, 2025 17:22
@thomhurst
Copy link
Owner

@claude Review this

Copilot AI changed the title [WIP] Make sure ALL types in source generated code are globally qualified to avoid namespace clashes Make sure ALL types in source generated code are globally qualified to avoid namespace clashes Aug 4, 2025
Copilot AI requested a review from thomhurst August 4, 2025 17:23
@claude
Copy link
Contributor

claude bot commented Aug 4, 2025

Claude finished @thomhurst's task —— View job


Code Review: Global Qualification Fix for Source Generators

Todo List:

  • Read and analyze source generator files
  • Review test verification file changes
  • Check for completeness and consistency
  • Provide comprehensive review feedback

🎯 Review Summary

This PR successfully addresses namespace collision issues in source-generated code by ensuring all System type references use global qualification. The implementation is comprehensive and well-executed.

Strengths

Complete Coverage

All identified System type references have been properly qualified:

  • Task.CompletedTaskglobal::System.Threading.Tasks.Task.CompletedTask
  • System.Runtime.CompilerServices.ModuleInitializerglobal::System.Runtime.CompilerServices.ModuleInitializer
  • System.CodeDom.Compiler.GeneratedCodeglobal::System.CodeDom.Compiler.GeneratedCode
  • System.Runtime.CompilerServices.UnsafeAccessorglobal::System.Runtime.CompilerServices.UnsafeAccessor

Systematic Implementation

  • 8 source generator files updated consistently across the codebase:
    • TestMetadataGenerator.cs:1545,1613,1637,1705 - Task.CompletedTask references ✅
    • PropertyInjectionSourceGenerator.cs:152,201 - ModuleInitializer and UnsafeAccessor ✅
    • AssemblyLoaderGenerator.cs:56,59 - GeneratedCode and ModuleInitializer ✅
    • DisableReflectionScannerGenerator.cs:22,25 - GeneratedCode and ModuleInitializer ✅
    • All other generators similarly updated ✅

Thorough Testing

  • 90 test verification files updated, demonstrating comprehensive test coverage
  • All generated code examples in verification files show proper global qualification
  • Changes are consistent across different test scenarios

🔍 Technical Analysis

Code Quality

  • Pattern Consistency: All changes follow the same global:: prefix pattern
  • No Partial Updates: No instances found where qualification was incomplete
  • Maintains Functionality: Generated code structure unchanged, only qualification added

Problem Resolution

The fix correctly addresses the reported issue where user-defined types could shadow System types:

// Before: Could cause conflicts
Task.CompletedTask
[System.Runtime.CompilerServices.ModuleInitializer]

// After: Collision-proof
global::System.Threading.Tasks.Task.CompletedTask
[global::System.Runtime.CompilerServices.ModuleInitializer]

📋 Minor Observations

  1. Consistency Check: Verified all generators use the same qualification pattern - ✅ consistent
  2. No Regression Risk: Changes only add qualification prefixes without altering logic
  3. Build Compatibility: No breaking changes to public APIs based on verification files

🎯 Verdict

LGTM - This is a well-implemented fix that:

  • ✅ Completely resolves the namespace collision issue
  • ✅ Applies changes systematically across all generators
  • ✅ Maintains code quality and consistency
  • ✅ Includes comprehensive test coverage
  • ✅ Poses minimal risk of regression

The PR successfully makes all generated code collision-proof while maintaining clean, readable implementation.


@thomhurst thomhurst merged commit b2b8fb2 into main Aug 4, 2025
1 check passed
@thomhurst thomhurst deleted the copilot/fix-2752 branch August 4, 2025 17:27
@thomhurst thomhurst linked an issue Aug 4, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make sure ALL types in source generated code are globally qualified to avoid namespace clashes The name 'Task' does not exist in the current context

2 participants