Add null checks for reflection calls instead of null-forgiving operators

[thomhurst]

Summary

Multiple reflection-based method/property lookups use null-forgiving operators (!) without validation, risking NullReferenceException if signatures change.

Severity: CRITICAL

These are time bombs - they work until a refactoring changes a method name or signature, then crash at runtime.

---

Affected Locations

1. ModuleExecutor.cs - Reflection Chain

File: src/ModularPipelines/Engine/ModuleExecutor.cs:488-496

var executeMethod = typeof(IModuleExecutionPipeline)
    .GetMethod(nameof(IModuleExecutionPipeline.ExecuteAsync))!  // Null forgiving
    .MakeGenericMethod(resultType);
    
var task = (Task) executeMethod.Invoke(_executionPipeline, new object[] { ... })!;  // Null forgiving
await task;

var resultProperty = task.GetType().GetProperty("Result")!;  // Null forgiving
return (IModuleResult) resultProperty.GetValue(task)!;  // Null forgiving

Problem: Four ! operators in a chain. If any reflection call returns null, we get NullReferenceException with no context.

2. OptionsProvider.cs - Property Access

File: src/ModularPipelines/Engine/OptionsProvider.cs:40-42

yield return option!
    .GetType()
    .GetProperty("Value", BindingFlags.Public | BindingFlags.Instance)!
    .GetValue(option);  // Chained null-forgiving

Problem: Assumes IOptions<T> always has a Value property accessible via reflection.

3. AttributeEventInvoker.cs - Method Invocation

File: src/ModularPipelines/Attributes/AttributeEventInvoker.cs (similar pattern)

---

Proposed Fix

Add Explicit Null Checks with Clear Errors

// Before
var executeMethod = typeof(IModuleExecutionPipeline)
    .GetMethod(nameof(IModuleExecutionPipeline.ExecuteAsync))!;

// After
var executeMethod = typeof(IModuleExecutionPipeline)
    .GetMethod(nameof(IModuleExecutionPipeline.ExecuteAsync))
    ?? throw new InvalidOperationException(
        $"Method {nameof(IModuleExecutionPipeline.ExecuteAsync)} not found on {nameof(IModuleExecutionPipeline)}");

Consider Compile-Time Alternatives

Where possible, replace reflection with compile-time generics:

// Instead of runtime reflection for Task<T>.Result
if (task is Task<TResult> typedTask)
{
    return await typedTask;
}

---

Acceptance Criteria

• All reflection calls have explicit null checks
• Error messages identify which method/property was not found
• Consider caching reflection results for performance
• Add unit tests that verify reflection assumptions
• Document why reflection is necessary in each case

Labels

bug, robustness, critical