Skip to content

add all analyzers that are configured via editorconfig to code cleanup#59106

Merged
jmarolf merged 8 commits intodotnet:mainfrom
jmarolf:feature/run-all-codefixes-in-code-cleanup
Feb 22, 2022
Merged

add all analyzers that are configured via editorconfig to code cleanup#59106
jmarolf merged 8 commits intodotnet:mainfrom
jmarolf:feature/run-all-codefixes-in-code-cleanup

Conversation

@jmarolf
Copy link
Copy Markdown
Contributor

@jmarolf jmarolf commented Jan 27, 2022

Fixes #58086
Fixes #50504
Fixes #43503

If a user can set a codestyle option to fail the build, and that option has a fix-all we now give the user the option to run it on code-cleanup if desired.

image

@jmarolf jmarolf requested a review from a team as a code owner January 27, 2022 07:05
@ghost ghost added the Area-IDE label Jan 27, 2022
@sharwell
Copy link
Copy Markdown
Contributor

How certain are we that all of these items will not produce change runtime behavior when applied? I ask because the result of this operation is likely to be difficult/impossible to review.

Copy link
Copy Markdown
Contributor

@sharwell sharwell left a comment

Choose a reason for hiding this comment

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

❓ Before moving forward with this, can you confirm the following behavior:

  1. Team configures a code style rule with severity hidden (or suggestion)
  2. User enables the code fix for (1) as part of the code cleanup profile
  3. User runs Code Cleanup on Solution

For the above scenario, the Code Cleanup operation should take no action, as the project is not configured to enforce consistency for the item in question.

@Youssef1313
Copy link
Copy Markdown
Member

Does this fix #43503 ?

@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Jan 30, 2022

❓ Before moving forward with this, can you confirm the following behavior:

  1. Team configures a code style rule with severity hidden (or suggestion)
  2. User enables the code fix for (1) as part of the code cleanup profile
  3. User runs Code Cleanup on Solution
    For the above scenario, the Code Cleanup operation should take no action, as the project is not configured to enforce consistency for the item in question.

Need to understand this perspective a bit better. If the team does this:

Team configures a code style rule with severity hidden (or suggestion)

I interpret that as saying "this is style isn't required but if you think it makes sense then apply it".

An individual developer then does this

User enables the code fix for (1) as part of the code cleanup profile

Saying "even though this is optional I am opting in to this being done".

In this case not applying the code style when code cleanup is run would both

  1. be inconsistent behavior with how code cleanup works today
  2. would give the user no recourse for applying code style options on code cleanup other than to set them as warnings

@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Jan 30, 2022

How certain are we that all of these items will not produce change runtime behavior when applied? I ask because the result of this operation is likely to be difficult/impossible to review.

Don't know how relevant that is. All code fixes change semantics in some way. The assumption here is that the default code cleanup profile will not include all of these code style options but users can choose to add them if they want to. This mirrors the behavior we have had with dotnet format for nearly two years.

@jmarolf jmarolf force-pushed the feature/run-all-codefixes-in-code-cleanup branch from dd7e51c to ba14eed Compare February 11, 2022 01:28
@jmarolf jmarolf force-pushed the feature/run-all-codefixes-in-code-cleanup branch from 724c0fb to e12d172 Compare February 16, 2022 00:40
@jmarolf jmarolf force-pushed the feature/run-all-codefixes-in-code-cleanup branch from e12d172 to 454552f Compare February 16, 2022 00:45
@jmarolf jmarolf dismissed sharwell’s stale review February 16, 2022 00:46

Answered questions

@jmarolf jmarolf requested review from ryzngard and sharwell February 17, 2022 01:09
Copy link
Copy Markdown
Contributor

@ryzngard ryzngard left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Copy Markdown
Member

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

Awesome!

@jmarolf jmarolf merged commit 9f6b192 into dotnet:main Feb 22, 2022
@jmarolf jmarolf deleted the feature/run-all-codefixes-in-code-cleanup branch February 22, 2022 19:07
@ghost ghost added this to the Next milestone Feb 22, 2022
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P2 Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code cleanup supports all code style rules Code Cleanup execution doesn't honor .editorconfig rules IDE0046 code profiling not taken in account

6 participants