Skip to content
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

Bail out for primary constructor parameters in IDE0059 analyzer #69748

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

mavasani
Copy link
Contributor

Fixes #69643

We already did so for IDE0060 (unused parameters) in a prior PR. This adds a similar bailout check in the core dataflow analysis IOperation walker for IDE0059 (unused value assignment).

Verified that the added unit test fails with IDE0059 diagnostic prior to the product fix.

Fixes dotnet#69643

We already did so for IDE0060 (unused parameters) in a prior PR. This adds a similar bailout check in the core dataflow analysis IOperation walker for IDE0059 (unused value assignment)
@mavasani mavasani added Area-IDE IDE-CodeStyle Built-in analyzers, fixes, and refactorings Feature - IDE0059 Unused value labels Aug 29, 2023
@mavasani mavasani requested a review from akhera99 August 29, 2023 08:55
@mavasani mavasani requested a review from a team as a code owner August 29, 2023 08:55
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Aug 29, 2023
@CyrusNajmabadi
Copy link
Member

Do we want to truly bail out? Or actually just analyze the entire class to determine if the parameters are unused?

@mavasani
Copy link
Contributor Author

Do we want to truly bail out? Or actually just analyze the entire class to determine if the parameters are unused?

That would be a thing to consider for IDE0060 (unused parameters diagnostic). This PR is primarily for IDE0059 (unnecessary value assignment), which flags writes to locals and parameters in a method body for which there are no reachable reads. Given that PC parameters are basically fields, we don't know if any writes within a method body are unnecessary.

@mavasani
Copy link
Contributor Author

Or actually just analyze the entire class to determine if the parameters are unused?

I am going to merge this PR to avoid this false positive. I'll file a separate feature request to consider adding functionality for flagging unused PC parameters.

@mavasani mavasani merged commit e4a0d5b into dotnet:main Aug 30, 2023
24 checks passed
@ghost ghost added this to the Next milestone Aug 30, 2023
@mavasani mavasani deleted the Issue69643 branch August 30, 2023 04:29
@Cosifne Cosifne modified the milestones: Next, 17.8 P3 Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Feature - IDE0059 Unused value IDE-CodeStyle Built-in analyzers, fixes, and refactorings untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IDE0059 is unnecessary in Primary Constructors
4 participants