Skip to content

Add analyzer to check for closing over loop variable #10476

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
danroth27 opened this issue May 22, 2019 · 7 comments
Closed

Add analyzer to check for closing over loop variable #10476

danroth27 opened this issue May 22, 2019 · 7 comments
Labels
affected-few This issue impacts only small number of customers analyzer Indicates an issue which is related to analyzer experience area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-razor.language severity-minor This label is used by an internal tool
Milestone

Comments

@danroth27
Copy link
Member

A lot of users are stumbling over the C# behavior of closing a delegate over a loop variable:

@for (var i = 1; i < 4; i++)
{
    <button class="btn btn-primary"
            onclick="@(e => UpdateHeading(e, i))"> // OOPS!
        Button #@i
    </button>
}

We should consider providing an analyzer that detects this case and warns.

@danroth27 danroth27 added area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one labels May 22, 2019
@mkArtakMSFT mkArtakMSFT added this to the 3.1.0 milestone May 23, 2019
@mkArtakMSFT mkArtakMSFT modified the milestones: 3.1.0, 5.0.0 Aug 12, 2019
@mkArtakMSFT mkArtakMSFT modified the milestones: 5.0.0, Backlog Aug 19, 2019
@pranavkm pranavkm added the help wanted Up for grabs. We would accept a PR to help resolve this issue label Jul 6, 2020
@SQL-MisterMagoo
Copy link
Contributor

I'd like to have a try at this, but would need a bit of guidance.

Would this need to go into the Razor folder with the "RZ" diagnostics in ComponentDiagnosticFactory ?
https://github.com/dotnet/aspnetcore/tree/master/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Components

Or is it actually a more general diagnostic that should move up a folder into the RazorDiagnosticFactory as it could be useful in Razor Pages as well as Components?

Thanks

@SQL-MisterMagoo
Copy link
Contributor

@danroth27 Realised I should probably tag someone 😄

@pranavkm
Copy link
Contributor

@SQL-MisterMagoo
Copy link
Contributor

@pranavkm Thanks - I'll concentrate on that area

@SQL-MisterMagoo
Copy link
Contributor

@pranavkm @danroth27 Would you want a code fix along with the analyzer?

@SQL-MisterMagoo
Copy link
Contributor

@pranavkm Do you have any tips on how I can test a new analyzer in a project (not unit tests/e2e tests, but manually to see if everything is working like the error list in VS looks correct etc...)?

@captainsafia captainsafia removed the help wanted Up for grabs. We would accept a PR to help resolve this issue label Sep 9, 2020
@mkArtakMSFT mkArtakMSFT added affected-few This issue impacts only small number of customers severity-minor This label is used by an internal tool labels Oct 12, 2020 — with ASP.NET Core Issue Ranking
@javiercn javiercn added analyzer Indicates an issue which is related to analyzer experience feature-razor.language labels Apr 19, 2021
@pranavkm
Copy link
Contributor

Dupe of #25252.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affected-few This issue impacts only small number of customers analyzer Indicates an issue which is related to analyzer experience area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-razor.language severity-minor This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

6 participants