Missing "Unused parameter" warning ("Parameter 'foo' is never used")

Consider the sample below.
As you can see, I'm getting the warning for the "args" parmeter, but it doesn't show for either of the methods in MyClass.
ParamNeverUsed.png

8 comments
Comment actions Permalink

This is because ReSharper knows that the Main method is only called by the runtime, so it knows that if it's not used in the body of Main, it can safely be deleted.

ReSharper doesn't know if anyone else is calling Foo or Bar. If someone else is calling them, removing the parameters would cause compile time errors. If no-one is calling them, then yes, it's safe to delete.

You can get ReSharper to display the warning by turning on Solution Wide Analysis. This will allow ReSharper to know if anyone else is calling the method, and if not, the warnings will appear.

0
Comment actions Permalink

I see, thanks for the clarification Matt.

Personally I disagree with this approach.
First, turning on solution-wide analysis when you have hundreds of projects and thousands of files seems like a bad idea.
Second, some classes may be "infrastructure" classes so the fact that no one calls them from the solution doesn't necessarily mean they won't be called , so I disaggree with this apparent assumption. (Granted, test code should call most, but maybe my solution doesn't include test code, or coverage isn't 100%).

It would have been nice to have more fine grained rules, something like "unused parameter in private methods" which will default to say a warning , and "unused parameter in public methods" which will default to say a suggestion or a hint.

At the end of the day, I'd wager that the vast majority of unused parameters are unintetional. I mean, why else would they be there ? Sure, you could be forced by an interface / superclass (another possible refinement that the rules can take into account), but even then chances are there's a good reason for each of the parameters to be in the signature. Worst case I could manually suppress it, or if it's a hint - no big deal anyway.

0
Comment actions Permalink

ReSharper already does take several of these things into account. Parameters on private methods are analysed without needing SWA enabled (the file has been analysed, after all), and parameters in methods implementing interfaces are always marked as in-use, since you can't remove them without changing the interface. Public methods default to always in-use, because ReSharper doesn't analyse all files unless SWA is enabled - since it doesn't know who's calling, it can't say if the parameter is in use in a call site.

If SWA is enabled, ReSharper can look at all of the files in the solution to see if something is in use, but that's all it can do. It can't know if you have an external solution calling some methods but not others. So, ReSharper does the best it can - if it's not used in the solution, it's marked as being not in use. It's shown as a suggestion, so you're free to ignore it, or you can get it marked as in-use, either by including tests, as you say, or by helping ReSharper's analysis with the [PublicAPI] annotation attribute. If you provide this, ReSharper will assume that it is being used externally, and suppress any warnings about unused code.

And yes, enabling SWA on a solution with hundreds of projects will have a performance impact, although it is workable. We can run SWA on the ReSharper solution, which has over 300 projects. However (and we should probably take this advice, too), a solution of that size is itself a code smell, and should really be split - you're rarely changing code that affects more than a handful of projects at a time.

Matt

0
Comment actions Permalink
parameters in methods implementing interfaces are always marked as in-use, since you can't remove them without changing the interface. Public methods default to always in-use, because ReSharper doesn't analyse all files unless SWA is enabled - since it doesn't know who's calling, it can't say if the parameter is in use in a call site.

All I'm saying is that it would be nice to be able to tell R# to default to the opposite (possibly by refining the rules as I suggested above). Instead of assuming the method can be called externally etc, assume I forgot to use the parameter. In other words, I would like to be told about unused parameters even for public methods implementing public interfaces (especially when those interfaces are in my code, if that's something R# knows without SWA). Most of the time I wrote these interfaces myself, all the more reason for unused parameters to be suspect.

1
Comment actions Permalink

I'm with Yosi. If i declare a function which takes a parameter and i don't use the parameter inside the function i expect that R# give me a hint about this.

Regards
Klaus

1
Comment actions Permalink

I'm getting confused. ReSharper already does tell you about unused parameters in public methods, as long as SWA is enabled, and as long as it's not an interface. If the method is being called externally, ReSharper doesn't know, so it does mark it as unused, as you requested.

What exactly do you want ReSharper to do that it isn't already doing? As far as I can make out, the only thing it isn't doing is warning you of an unused parameter when the method is implementing an interface that is defined in your codebase. Is that right? If so, then this would be a new feature and should be raised as a feature request at http://youtrack.jetbrains.com/dashboard#newissue=yes

0
Comment actions Permalink

I want a warning to be issued for unused parameters even when SWA is turned off (which is the case in my solution), regardless of whether they are public or implement an interface. My assumption is that regardless of the circumstances, in most cases unused parameters are unintentional (again, this could default to a hint). I'd wager this will cause less "false positives" than existing warnings, such as "IDictionary<X,Y> could be of type IEnumerable<KeyValuePair<X,Y>" and "Use implicit variable declaration", to name a few.

1
Comment actions Permalink

@Matt Ellis You seem to be confusing two entirely unrelated concepts:

- The concept of warnings.  Which I must see.  Because they reveal bugs.

- The concept of refactoring.  Which I may and may not perform after I have seen a warning.  And which may and may not break my code.  Which can be broken anyway by a multitude of other refactorings, so one more would not be making any difference.

So, long story short:

ReSharper should be telling me that I have a function which accepts a parameter which is never used within the function.  It can do so easily, and it can do so irrespective of whether the function is public or private, whether the function is used by any other code out there, whether I have solution-wide-analysis enabled, and so on.  That's why I bought ReSharper: to help me find bugs.  Refactoring is a nice extra.  It amuses me how much wrong thinking has been put behind NOT providing a much needed feature.

 

0

Please sign in to leave a comment.