Skip to content

No IDE0060 for unused parameter in partial method implementation#63530

Merged
mavasani merged 2 commits intodotnet:mainfrom
louis-z:issue-57814
Sep 22, 2022
Merged

No IDE0060 for unused parameter in partial method implementation#63530
mavasani merged 2 commits intodotnet:mainfrom
louis-z:issue-57814

Conversation

@louis-z
Copy link
Contributor

@louis-z louis-z commented Aug 22, 2022

If a partial method implementation does not use a parameter, do not generate an IDE0060 diagnostic.

Also, fix minor typos ("Unsued" vs "Unused")

Fixes #57814

@louis-z louis-z requested a review from a team as a code owner August 22, 2022 23:53
@ghost ghost added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Aug 22, 2022
@louis-z louis-z changed the title No IDE0060 diagnostic for partial method implementation No IDE0060 for unused parameter in partial method implementation Aug 22, 2022
Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

Consider adding a test with partial definition without implementation.

@louis-z
Copy link
Contributor Author

louis-z commented Aug 23, 2022

Consider adding a test with partial definition without implementation.

Just did.

@louis-z louis-z requested a review from Youssef1313 August 24, 2022 11:30
@louis-z louis-z force-pushed the issue-57814 branch 3 times, most recently from 1f76643 to 06d2f0b Compare August 27, 2022 15:39
method.IsVirtual ||
method.IsOverride ||
method.PartialImplementationPart != null ||
method.PartialDefinitionPart != null ||
Copy link
Member

Choose a reason for hiding this comment

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

I don't know, but I feel the check is too strong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that

  • a partial method definition might be generated by some tool, with a parameter list as part of its signature
  • the corresponding implementation must have the same signature

I believe it's pointless to run the IDE0060 rule on the implementation (i.e. when method.PartialDefinitionPart != null ).

After all, if any of the parameters are unused by the implementation, what can the developer do? They probably have no control over the autogenerated definition and the implementation's signature must match the definition's...

Copy link
Member

Choose a reason for hiding this comment

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

I think we should specifically check for generated code. This check can probably be easier when #63447 is merged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know about this upcoming IsGeneratedCode flag. You're right, waiting for it sounds like a good idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if IsGeneratedCode check helps here. The callback would only be made for non-generated code for IDE0060 analyzer, and you can't check whether or not the other partial symbol is generated or not from this callback.

@mavasani
Copy link
Contributor

@louis-z Can you please resolve merge conflicts?

If a partial method implementation does not use a parameter, do not generate an IDE0060 diagnostic.

Also, fix minor typos (Unsued vs Unused)
@mavasani
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@mavasani mavasani merged commit 121d9b5 into dotnet:main Sep 22, 2022
@ghost ghost added this to the Next milestone Sep 22, 2022
@Cosifne Cosifne modified the milestones: Next, 17.4 P3 Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False positive IDE0060 for partial method with SG

4 participants