Enhance "Unused Parameter" Detection

[vsfeedback]

Parameters are often on methods to maintain patterns even if they're not used by the method. This relates to encapsulation and it not being the caller's business whether the parameter is being used or not.

It might be there to possibly be used in the future. It might be there for backward compatibility. It might be there for pattern consistency.

A classic example would be maintaining a pattern that all async methods have a CancellationToken parameter. The current method implementation might be such that it makes no sense to use it, but for future flexibility it's none of the caller's business whether it actually gets used or not.

Indeed the method might even be documented as warning the caller it might potentially throw an OperationCanceledException, but in reality the current implementation never will. The CancellationToken and the exception documentation are saying "it's none of the caller's business whether the method actually does any of this" because the ability to do it (or not) in the future is being retained.

Likewise a method might have used the token and thrown the exception in the past, but no longer does.

This leads to the undesirable condition that the otherwise great "Unused Parameter" warning in the editor gets ignored because of false'ish (for the developer's situation) indicators. It's the same reason why code that is posting a bunch of "warnings we can ignore" when compiled is sloppy and shouldn't be tolerated. It leads to easily missing a warning or two that shouldn't be ignored.

As for using "_", it conflicts with the caller being able to name the parameters on the call.

Finding a way for the developer to flag a parameter to indicate "it being unused is correct" would help support reliable coding by allowing the legitimate "parameter unused" warnings to avoid getting lost in false'ish reports.

I'm wondering if one "clean" way to handle this would be through the XML documentation (///) support. Scattering a bunch of [attributes] in the parameter list would be pretty ugly. Having the XML documentation above the method signature provide the necessary hint would seem to be rather "clean", and even potentially appropriate. Although the hint in the XML documentation shouldn't be something that will carry through to the published documentation. That would be violating encapsulation and "it's none of the caller's business".

Another approach, depending upon whether this would be a nightmare for the parser, would be to combine discarded parameters (_) and parameter names. For example, "public void MyMethod(object _ myIgnoredParm)". This allows the developer to declare it is okay the parm isn't being used, while also providing a name for the caller to use -- maintaining encapsulation and the "it's none of the caller's business whether the parm is used or not."

Indeed "object _ myIgnoredParm" could even be used by the compiler to come back and say "hey, you said myIgnoredParm was to be discarded, but then you tried to use it."

This issue has been moved from https://developercommunity.visualstudio.com/content/idea/582741/enhance-unused-parameter-detection.html
VSTS ticketId: 899334
These are the original issue comments:

Jane Wu [MSFT] on 5/27/2019, 01:06 AM (63 days ago):

Thank you for taking the time to provide your suggestion. We will do some preliminary checks to make sure we can proceed further. We'll provide an update once the issue has been triaged by the product team.

[yaakov-h]

Scattering a bunch of [attributes] in the parameter list would be pretty ugly.

Would it really, though?

There are already attributes used on parameters, both ones inserted by the compiler (such as [Nullable]) as well as ones added by humans (e.g. [CallerMemberName]). It's not a new concept, it has heavy precedent.

Attributes seem like the lowest-effort solution to this problem, would not require a change to the language spec, as the Analyzer that detects unused parameters could simply ignore any parameter with a well-known attribute defined.

[YairHalberstadt]

You could even name the attribute _Attribute for [_] object myIgnoredParam 😀

[HaloFour]

I'm a little confused as to why this was moved to the C# repo. AFAICT the compiler doesn't care whether or not you use the parameters. If you wanted warnings about parameters that are unused, or warnings about parameters that should be unused that seems like a perfect use case for analyzers.

[bbarry]

I think there is an implication that there might be some sort of syntactic solution to describe the parameter as being unused so that the compiler may give better UX in the 2 identifiable bad cases.

I think an analyzer and a [SuppressMessage(...)] is a fine enough solution though.

[TonyValenti]

I use this:

        public static void Ignore<T>(this T This) {
            This.Equals(This);
        }

[TonyValenti]

Then I just Parameter.Ignore() at the top of my method.

[yaakov-h]

@TonyValenti Can I suggest adding [Conditional("...")] to that method? 😉

[TonyValenti]

@yaakov-h Great Call! Done!