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 on reporting unused parameter diagnostic for special paramet… #32883

Merged
merged 1 commit into from
Jan 29, 2019

Conversation

mavasani
Copy link
Contributor

…er names

We ignore parameter names that start with an underscore and are optionally followed by an integer, such as _, _1, _2, etc.
This allows bailing out on unused parameters for methods that need a specific signature and are forced to have these parameters. This also serves as a convenient way to suppress instances of unused parameter diagnostic without disabling the diagnostic completely.

Fixes #32851
Fixes #32228

…er names

We ignore parameter names that start with an underscore and are optionally followed by an integer, such as '_', '_1', '_2', etc.
This allows bailing out on unused parameters for methods that need a specific signature and are forced to have these parameters. This also serves as a convenient way to suppress instances of unused parameter diagnostic without disabling the diagnostic completely.

Fixes dotnet#32851
Fixes dotnet#32228
@mavasani mavasani requested review from CyrusNajmabadi and a team January 28, 2019 20:14
@mavasani
Copy link
Contributor Author

Tagging @bartdesmet @YairHalberstadt

@YairHalberstadt
Copy link
Contributor

Looks great!
It might be worth adding this as a codefix to the analyser, but if course that could be a seperate pr.

@mavasani
Copy link
Contributor Author

Tagging @jinujoseph @vatsalyaagrawal for approval and @dotnet/roslyn-ide for further review(s).

@genlu
Copy link
Member

genlu commented Jan 29, 2019

So we are creating a naming convention in IDE layer to mark discard parameters w/o compiler support for analysis purposes? Is this something we intend to support officially, or just as a temporary workaround until dotnet/csharplang#2180 is finalized?

@mavasani
Copy link
Contributor Author

Looks like a reasonable long term solution to me given we don’t expect code bases with wide usages of _1, _2, etc as parameter names.

@mavasani
Copy link
Contributor Author

@genlu Any further comments? Thanks!

@genlu
Copy link
Member

genlu commented Jan 29, 2019

I'm fine with this approach, just two suggestions:

  1. consider reporting a info level diagnostic for unused parameter of this pattern
  2. maybe change the text of the warning to let people aware of this

Also, we will need to mention this in the VS document when it's up

@mavasani
Copy link
Contributor Author

consider reporting a info level diagnostic for unused parameter of this pattern

The existing diagnostic is already an info diagnostic, and it seems likely this will just be noise. Lets do this only if we find real customer code bases where such parameter naming is used widely enough for used parameter names.

maybe change the text of the warning to let people aware of this

I think this case is extremely rare, and we see more folks filing such issues, we can change the message. I don't want to make the diagnostic message unnecessarily complex for a rare case. Hopefully, language will also be modified soon enough to allow real discard parameters and we wouldn't need to do anything here.

@jinujoseph jinujoseph added this to the 16.0.P4 milestone Jan 29, 2019
@mavasani mavasani merged commit 6663cce into dotnet:master Jan 29, 2019
@mavasani mavasani deleted the SpecialParameterNames branch January 29, 2019 18:36
xoofx pushed a commit to stark-lang/stark-roslyn that referenced this pull request Apr 16, 2019
Bail out on reporting unused parameter diagnostic for special paramet…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants