Skip to content

Add an analyzer that warns about non-literal sequence numbers #35805

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

Merged
merged 6 commits into from
Sep 4, 2021

Conversation

MackinnonBuck
Copy link
Member

@MackinnonBuck MackinnonBuck commented Aug 26, 2021

Add an analyzer that warns about non-literal sequence numbers
The analyzer warns when using anything other than integer literals for sequence numbers in RenderTreeBuilder methods.

Potential improvements to consider
There are definitely situations where it's appropriate for developers to use non-literals as the sequence number (e.g. RenderTreeBuilder extension methods that accept a sequence and pass it to a RenderTreeBuilder method). In such cases, should we expect developers to suppress the warning, or should we attempt something smarter? For example, if the argument passed as the sequence number is itself a parameter of the containing method, should we perform the same analysis on the method containing the RenderTreeBuilder method invocation? Should we go even further and ensure that the parameter is not reassigned anywhere in the containing method?

Fixes #14579

@MackinnonBuck MackinnonBuck requested a review from pranavkm August 26, 2021 23:12
@MackinnonBuck MackinnonBuck requested a review from Pilchie as a code owner August 26, 2021 23:12
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Aug 26, 2021
Copy link
Contributor

@pranavkm pranavkm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look great. I'd prefer if @SteveSandersonMS to sign off on this to see if there was more / less we should be doing here.

@SteveSandersonMS SteveSandersonMS self-requested a review September 3, 2021 14:34
Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks brilliant - thanks!

@MackinnonBuck MackinnonBuck enabled auto-merge (squash) September 4, 2021 00:11
@MackinnonBuck MackinnonBuck merged commit 8f674d0 into main Sep 4, 2021
@MackinnonBuck MackinnonBuck deleted the t-mbuck/sequence-number-analyzer branch September 4, 2021 00:47
@ghost ghost added this to the 7.0-preview1 milestone Sep 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Analyzer to detect nonconstant sequence number in RenderTreeBuilder calls
3 participants