Improve usability when using uncaptured iteration variables in Blazor bindings

[sfiruch]

Is your feature request related to a problem? Please describe.

When using the for-loop iterator variable as part of value binding in Blazor, you get an IndexOutOfRange exception. Consider this scenario:

@for(var i = 0; i < stringArray.Length; i++)
{
    <input type="text" @bind="stringArray[i]" />
}

This appears to work at a first glance. However, as soon as you try to modify the bound string, you'll get a difficult to understand IndexOutOfRange exception:

Error: System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at YourProject.Pages.Index.<>c__DisplayClass0_0.<BuildRenderTree>b__0(String __value)
   at Microsoft.AspNetCore.Components.EventCallbackFactoryBinderExtensions.<>c__DisplayClass22_0`1.<CreateBinderCore>b__0(ChangeEventArgs e)
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.AspNetCore.Components.ComponentBase.CallStateHasChangedOnAsyncCompletion(Task task)
   at Microsoft.AspNetCore.Components.RenderTree.Renderer.GetErrorHandledTask(Task taskToHandle)

The problem is that the closure in the generated code doesn't use the i at iteration time, but the value of i long after the for-loop has ended, i.e. i is not captured. The solution is to introduce a new local variable with a narrower scope:

@for(var i = 0; i < stringArray.Length; i++)
{
    var copyOfI = i;
    <input type="text" @bind="stringArray[copyOfI]" />
}

This is how C# works, but it is not very developer-friendly. It is quite difficult to debug this problem in Blazor, because the debugger will not let you look at the value of i. Instead you'll see that an exception was generated by some generated code, without line numbers and without context. Many people have struggled with problem before (e.g. #24460, #16815, #15633, #16144, #16073, #16809, #12539, #16276, many posts on StackOverflow and forums).

Describe the solution you'd like

Given that it's not feasible to change the semantics of the language or include compiler warnings, the following alternatives might alleviate the problem:

• A Roslyn Analyzer to flag uncaptured local variables.
• Improve the debugging experience, where one can inspect the value of i leading to the exception.
• Include the indexer value in the IndexOutOfRangeException

Perhaps there are other, much better/simpler solutions?

[ghost]

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[pranavkm]

We had a a contributor provide an analyzer for this, but we think this is better suited as part of Roslyn analyzers that isn't Blazor specific. Parking this in next sprint planning to figure out who gets to drive this work for the next release.

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[TanayParikh]

We had a a contributor provide an analyzer for this, but we think this is better suited as part of Roslyn analyzers that isn't Blazor specific. Parking this in next sprint planning to figure out who gets to drive this work for the next release.

Marking as external as we have to work with dotnet/roslyn to realize this feature.

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[cro-sotax]

From my duplicate issue:

Thank you for your suggestion @cro-sotax. In general Blazor does not attempt to rewrite any of the user's C# code (C# blocks are typically opaque to the compiler). We document this gotcha as part of our docs and have an issue to possibly add a C# compiler diagnostic for it - #25252.

Having looked a bit further into the concept, I see that my suggestion would cause more trouble than it is worth.

That being said, I think the ChildContent case is a bit of a special case: For a reader of the code, it is not that obvious that the closure is happening. If I exchanged the ParentComponent for a div or vice-versa, one could easily miss that this changes the counter variable behavior.

We had a a contributor provide an analyzer for this, but we think this is better suited as part of Roslyn analyzers that isn't Blazor specific. Parking this in next sprint planning to figure out who gets to drive this work for the next release.

If I write a closure in C# code, it is much easier to notice this effect. Most C# developers are probably aware of the difference between for and foreach here. Even if not, the first thing they do in case of a resulting bug might be to place a breakpoint into the lambda. But in this case, it doesn't "look" like code that could fail, more like the closure is an implementation detail of the source generator.

Couldn't both the contributor's analyzer and the non-Blazor analyzer be integrated? The latter sounds like a different kind of warning to me, because "I wrote the closure, I know what I'm doing".

[mkArtakMSFT]

This is going to be addressed as part of #44690.
Still thinking about how to handle what @pranavkm has suggested above:

We had a a contributor provide an analyzer for this, but we think this is better suited as part of Roslyn analyzers that isn't Blazor specific. Parking this in next sprint planning to figure out who gets to drive this work for the next release.