fix: address code review feedback

thomhurst · thomhurst · commit 4f37d18a975a · 2026-03-22T15:57:40.000Z
- EmitRegistrationField now owns the trailing semicolon, callers pass
  expression only (prevents fragile implicit contract)
- Remove dead try/catch around RunClassConstructor — InfrastructureGenerator
  always emits both shell classes, so TypeLoadException cannot occur
diff --git a/TUnit.Core.SourceGenerator.Tests/DuplicateTypeNameAcrossAssembliesTests.InfrastructureGenerator_WithDuplicateTypeNames_CompilesSuccessfully.DotNet10_0.verified.txt b/TUnit.Core.SourceGenerator.Tests/DuplicateTypeNameAcrossAssembliesTests.InfrastructureGenerator_WithDuplicateTypeNames_CompilesSuccessfully.DotNet10_0.verified.txt
@@ -99,16 +99,8 @@ file static class TUnitInfrastructure
             }
             catch { /* TUnit.Core not available - skip logging */ }
         }
-        try
-        {
-            global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);
-        }
-        catch (global::System.TypeLoadException) { /* Type not emitted - no test registrations */ }
-        try
-        {
-            global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);
-        }
-        catch (global::System.TypeLoadException) { /* Type not emitted - no hook registrations */ }
+        global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);
+        global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);
         try
         {
             global::TUnit.Core.GlobalContext.Current.GlobalLogger.LogDebug("[ModuleInitializer:TestsBase`1] TUnit infrastructure initialized");
diff --git a/TUnit.Core.SourceGenerator.Tests/DuplicateTypeNameAcrossAssembliesTests.InfrastructureGenerator_WithDuplicateTypeNames_CompilesSuccessfully.DotNet8_0.verified.txt b/TUnit.Core.SourceGenerator.Tests/DuplicateTypeNameAcrossAssembliesTests.InfrastructureGenerator_WithDuplicateTypeNames_CompilesSuccessfully.DotNet8_0.verified.txt
@@ -99,16 +99,8 @@ file static class TUnitInfrastructure
             }
             catch { /* TUnit.Core not available - skip logging */ }
         }
-        try
-        {
-            global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);
-        }
-        catch (global::System.TypeLoadException) { /* Type not emitted - no test registrations */ }
-        try
-        {
-            global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);
-        }
-        catch (global::System.TypeLoadException) { /* Type not emitted - no hook registrations */ }
+        global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);
+        global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);
         try
         {
             global::TUnit.Core.GlobalContext.Current.GlobalLogger.LogDebug("[ModuleInitializer:TestsBase`1] TUnit infrastructure initialized");
diff --git a/TUnit.Core.SourceGenerator.Tests/DuplicateTypeNameAcrossAssembliesTests.InfrastructureGenerator_WithDuplicateTypeNames_CompilesSuccessfully.DotNet9_0.verified.txt b/TUnit.Core.SourceGenerator.Tests/DuplicateTypeNameAcrossAssembliesTests.InfrastructureGenerator_WithDuplicateTypeNames_CompilesSuccessfully.DotNet9_0.verified.txt
@@ -99,16 +99,8 @@ file static class TUnitInfrastructure
             }
             catch { /* TUnit.Core not available - skip logging */ }
         }
-        try
-        {
-            global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);
-        }
-        catch (global::System.TypeLoadException) { /* Type not emitted - no test registrations */ }
-        try
-        {
-            global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);
-        }
-        catch (global::System.TypeLoadException) { /* Type not emitted - no hook registrations */ }
+        global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);
+        global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);
         try
         {
             global::TUnit.Core.GlobalContext.Current.GlobalLogger.LogDebug("[ModuleInitializer:TestsBase`1] TUnit infrastructure initialized");
diff --git a/TUnit.Core.SourceGenerator.Tests/DuplicateTypeNameAcrossAssembliesTests.InfrastructureGenerator_WithDuplicateTypeNames_CompilesSuccessfully.Net4_7.verified.txt b/TUnit.Core.SourceGenerator.Tests/DuplicateTypeNameAcrossAssembliesTests.InfrastructureGenerator_WithDuplicateTypeNames_CompilesSuccessfully.Net4_7.verified.txt
@@ -99,16 +99,8 @@ file static class TUnitInfrastructure
             }
             catch { /* TUnit.Core not available - skip logging */ }
         }
-        try
-        {
-            global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);
-        }
-        catch (global::System.TypeLoadException) { /* Type not emitted - no test registrations */ }
-        try
-        {
-            global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);
-        }
-        catch (global::System.TypeLoadException) { /* Type not emitted - no hook registrations */ }
+        global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);
+        global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);
         try
         {
             global::TUnit.Core.GlobalContext.Current.GlobalLogger.LogDebug("[ModuleInitializer:TestsBase`1] TUnit infrastructure initialized");
diff --git a/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs b/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs
@@ -452,20 +452,9 @@ private static void GenerateCode(SourceProductionContext context, AssemblyInfoMo
                 // Per-class/per-method test sources and per-hook sources contribute static field
                 // initializers to shared partial classes. RunClassConstructor forces each .cctor
                 // to execute, performing all registrations in ONE JIT-compiled method per class.
-                sourceBuilder.AppendLine("try");
-                sourceBuilder.AppendLine("{");
-                sourceBuilder.Indent();
+                // No try/catch needed — InfrastructureGenerator always emits both shell classes.
                 sourceBuilder.AppendLine("global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);");
-                sourceBuilder.Unindent();
-                sourceBuilder.AppendLine("}");
-                sourceBuilder.AppendLine("catch (global::System.TypeLoadException) { /* Type not emitted - no test registrations */ }");
-                sourceBuilder.AppendLine("try");
-                sourceBuilder.AppendLine("{");
-                sourceBuilder.Indent();
                 sourceBuilder.AppendLine("global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);");
-                sourceBuilder.Unindent();
-                sourceBuilder.AppendLine("}");
-                sourceBuilder.AppendLine("catch (global::System.TypeLoadException) { /* Type not emitted - no hook registrations */ }");
                 sourceBuilder.AppendLine();
 
                 sourceBuilder.AppendLine("try");
diff --git a/TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs b/TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs
@@ -2407,7 +2407,7 @@ private static string EscapeString(string value)
     private static void GenerateTestRegistrationField(CodeWriter writer, TestMethodMetadata testMethod, string uniqueClassName)
     {
         EmitRegistrationField(writer, uniqueClassName,
-            $"global::TUnit.Core.SourceRegistrar.RegisterReturn({GenerateTypeReference(testMethod.TypeSymbol, testMethod.IsGenericType)}, new {uniqueClassName}());");
+            $"global::TUnit.Core.SourceRegistrar.RegisterReturn({GenerateTypeReference(testMethod.TypeSymbol, testMethod.IsGenericType)}, new {uniqueClassName}())");
     }
 
     /// <summary>
@@ -2420,7 +2420,7 @@ private static void EmitRegistrationField(CodeWriter writer, string fieldName, s
         writer.AppendLine("internal static partial class TUnit_TestRegistration");
         writer.AppendLine("{");
         writer.Indent();
-        writer.AppendLine($"static readonly int _r_{fieldName} = {registrarCall}");
+        writer.AppendLine($"static readonly int _r_{fieldName} = {registrarCall};");
         writer.Unindent();
         writer.AppendLine("}");
     }
@@ -3031,7 +3031,7 @@ private static void GeneratePerClassTestSource(SourceProductionContext context,
             writer.AppendLine("}");
 
             EmitRegistrationField(writer, classGroup.TestSourceName,
-                $"global::TUnit.Core.SourceRegistrar.RegisterReturn(typeof({classGroup.ClassFullyQualified}), {classGroup.TestSourceName}.GetTests, {classGroup.TestSourceName}.EnumerateTestDescriptors);");
+                $"global::TUnit.Core.SourceRegistrar.RegisterReturn(typeof({classGroup.ClassFullyQualified}), {classGroup.TestSourceName}.GetTests, {classGroup.TestSourceName}.EnumerateTestDescriptors)");
 
             context.AddSource($"{classGroup.TestSourceName}.g.cs", SourceText.From(writer.ToString(), Encoding.UTF8));
         }

Original file line number	Diff line number	Diff line change
`@@ -99,16 +99,8 @@ file static class TUnitInfrastructure`
`99`	`99`	`}`
`100`	`100`	`catch { /* TUnit.Core not available - skip logging */ }`
`101`	`101`	`}`
`102`		`- try`
`103`		`- {`
`104`		`- global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);`
`105`		`- }`
`106`		`- catch (global::System.TypeLoadException) { /* Type not emitted - no test registrations */ }`
`107`		`- try`
`108`		`- {`
`109`		`- global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);`
`110`		`- }`
`111`		`- catch (global::System.TypeLoadException) { /* Type not emitted - no hook registrations */ }`
	`102`	`+ global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);`
	`103`	`+ global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_HookRegistration).TypeHandle);`
`112`	`104`	`try`
`113`	`105`	`{`
`114`	`106`	global::TUnit.Core.GlobalContext.Current.GlobalLogger.LogDebug("[ModuleInitializer:TestsBase`1] TUnit infrastructure initialized");
Original file line number	Diff line number	Diff line change
`@@ -2407,7 +2407,7 @@ private static string EscapeString(string value)`
`2407`	`2407`	`private static void GenerateTestRegistrationField(CodeWriter writer, TestMethodMetadata testMethod, string uniqueClassName)`
`2408`	`2408`	`{`
`2409`	`2409`	`EmitRegistrationField(writer, uniqueClassName,`
`2410`		`- $"global::TUnit.Core.SourceRegistrar.RegisterReturn({GenerateTypeReference(testMethod.TypeSymbol, testMethod.IsGenericType)}, new {uniqueClassName}());");`
	`2410`	`+ $"global::TUnit.Core.SourceRegistrar.RegisterReturn({GenerateTypeReference(testMethod.TypeSymbol, testMethod.IsGenericType)}, new {uniqueClassName}())");`
`2411`	`2411`	`}`
`2412`	`2412`
`2413`	`2413`	`/// <summary>`
`@@ -2420,7 +2420,7 @@ private static void EmitRegistrationField(CodeWriter writer, string fieldName, s`
`2420`	`2420`	`writer.AppendLine("internal static partial class TUnit_TestRegistration");`
`2421`	`2421`	`writer.AppendLine("{");`
`2422`	`2422`	`writer.Indent();`
`2423`		`- writer.AppendLine($"static readonly int _r_{fieldName} = {registrarCall}");`
	`2423`	`+ writer.AppendLine($"static readonly int _r_{fieldName} = {registrarCall};");`
`2424`	`2424`	`writer.Unindent();`
`2425`	`2425`	`writer.AppendLine("}");`
`2426`	`2426`	`}`
`@@ -3031,7 +3031,7 @@ private static void GeneratePerClassTestSource(SourceProductionContext context,`
`3031`	`3031`	`writer.AppendLine("}");`
`3032`	`3032`
`3033`	`3033`	`EmitRegistrationField(writer, classGroup.TestSourceName,`
`3034`		`- $"global::TUnit.Core.SourceRegistrar.RegisterReturn(typeof({classGroup.ClassFullyQualified}), {classGroup.TestSourceName}.GetTests, {classGroup.TestSourceName}.EnumerateTestDescriptors);");`
	`3034`	`+ $"global::TUnit.Core.SourceRegistrar.RegisterReturn(typeof({classGroup.ClassFullyQualified}), {classGroup.TestSourceName}.GetTests, {classGroup.TestSourceName}.EnumerateTestDescriptors)");`
`3035`	`3035`
`3036`	`3036`	`context.AddSource($"{classGroup.TestSourceName}.g.cs", SourceText.From(writer.ToString(), Encoding.UTF8));`
`3037`	`3037`	`}`