Unused method parameters not greyed

This is not new in 4.0, but this might be the place to have it fixed (or explained!)

If I write this method:

void myMethod(int used, int unused)
{
Debug.Print(used.ToString();
}

Then 'unused' will be greyed-out, because it's not used
Also, if myMethod is not called then 'myMethod' will also be grey

If I make this method 'public', then the greying is removed from both myMethod (this makes sense to me - you can't tell if it's called except possibly via whole-sln checking)

BUT

the greying is ALSO removed from 'unused', just because I made the method public.

This doesn't make sense to me - the greying of method parameters is surely to help the author of the method, not its callers. Is this a bug in that the 'supress greying for public methods' behaviour is being applied to both the method name AND its parameters?

Or is there some deep reason for this which I've failed to understand?

8 comments
Comment actions Permalink

It's probably due to not doing whole sln checking. It's not safe to delete the parameter since other methods could be calling you. You'd need to change the call signature of all callers to remove the unused parameter.

That's ok, but generally the greyed out code means it's pretty safe to just delete... that wouldn't be the case here.

0
Comment actions Permalink

We can't gray out parameters of public methods because:

1) This method could be assigned to delegate, and this parameters is
necessary to match the signature
2) It could be quasi-implementation of some interface method

--
Eugene Pasynkov
Developer
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"
"Will Dean" <resharper@indcomp.co.uk> wrote in message
news:27571582.1203426277783.JavaMail.itn@is.intellij.net...

This is not new in 4.0, but this might be the place to have it fixed (or
explained!)

>

If I write this method:

>

void myMethod(int used, int unused)
{
Debug.Print(used.ToString();
}

>

Then 'unused' will be greyed-out, because it's not used
Also, if myMethod is not called then 'myMethod' will also be grey

>

If I make this method 'public', then the greying is removed from both
myMethod (this makes sense to me - you can't tell if it's called except
possibly via whole-sln checking)

>

BUT

>

the greying is ALSO removed from 'unused', just because I made the method
public.

>

This doesn't make sense to me - the greying of method parameters is surely
to help the author of the method, not its callers. Is this a bug in that
the 'supress greying for public methods' behaviour is being applied to
both the method name AND its parameters?

>

Or is there some deep reason for this which I've failed to understand?



0
Comment actions Permalink

ok, that makes some sense - I was thinking of grey as meaning 'unused', rather than 'completely safe to delete with no side effects'.

There is a subtle distinction here.

Clearly no method with any callers is ever going to allow straightforard deletion of any of its parameters - I do think there's an opportunity here to usefully highlight method parameters which are unused within a method - though perhaps greying is not the right way.

Perhaps that's because I've just had to find a bug in a bit of code which would have been more obvious it it was clear that that a method parameter hadn't been used.

0
Comment actions Permalink

Hello Will,

Consider so called quasi-implementation. Removing unused would break code.

interface IFoo
{
void Bar(int used, int unused);
}

class Foo
{
public void Bar(int used, int unused) { ... } // here we cannot know that
Bar will implement IFoo in some other place, but in fact it does
}

class Foo2 : Foo, IFoo {}

Sincerely,
Ilya Ryzhenkov

JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"


WD> This is not new in 4.0, but this might be the place to have it fixed
WD> (or explained!)
WD>
WD> If I write this method:
WD>
WD> void myMethod(int used, int unused)
WD> {
WD> Debug.Print(used.ToString();
WD> }
WD> Then 'unused' will be greyed-out, because it's not used Also, if
WD> myMethod is not called then 'myMethod' will also be grey
WD>
WD> If I make this method 'public', then the greying is removed from
WD> both myMethod (this makes sense to me - you can't tell if it's
WD> called except possibly via whole-sln checking)
WD>
WD> BUT
WD>
WD> the greying is ALSO removed from 'unused', just because I made the
WD> method public.
WD>
WD> This doesn't make sense to me - the greying of method parameters is
WD> surely to help the author of the method, not its callers. Is this a
WD> bug in that the 'supress greying for public methods' behaviour is
WD> being applied to both the method name AND its parameters?
WD>
WD> Or is there some deep reason for this which I've failed to
WD> understand?
WD>


0
Comment actions Permalink

We can't gray out parameters of public methods because:


We actually can do this but this requires collecting information from the
whole solution. We plan to add this functionality (as well as lot of other
warnings for public entities, like unused public method) into our Solution-Wide
Analysis feature. This work is already in progress but it not planned to
be included into 4.0 release.

Valentin Kipiatkov
CTO and Chief Scientist
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"

We can't gray out parameters of public methods because:

1) This method could be assigned to delegate, and this parameters is
necessary to match the signature
2) It could be quasi-implementation of some interface method



0
Comment actions Permalink

OK, I understand where you guys are coming from - you're saying 'grey' == 'can be deleted', and so there are lots of circumstances where 'can be deleted' is not true. I see this now.

But I'm coming at this from another direction, which is that unused parameters are often (NOT always) lurking bugs, and it would be useful to me to have them drawn to my attention in some way (which I can now see shouldn't be grey.)

To me this would be just as useful as being nudged about checking that objects are not null, and WAY more useful than, for example, having every local variable declaration turned into a 'var', which 4.0 seems to think (by default) might be desirable.

0
Comment actions Permalink

Just be sure this is an option.

Our product is so large, that different teams work on different parts.
There is no one solution for the "whole thing", but lots of solutions for
logical pieces. So anything public is potentially callable, and thus we
almost never want public members, methods, whatever, to be flagged as
unused.

It would certainly be useful for smaller projects/solutions, I'm sure, but
it's not a super high priority for me or anyone on my team.


"Valentin Kipiatkov" <valentin@jetbrains.com> wrote in message
news:a3b000aa10988ca4132b2dcd5b5@news.intellij.net...
>> We can't gray out parameters of public methods because:
>

We actually can do this but this requires collecting information from the
whole solution. We plan to add this functionality (as well as lot of other
warnings for public entities, like unused public method) into our
Solution-Wide Analysis feature. This work is already in progress but it
not planned to be included into 4.0 release.

>

Valentin Kipiatkov CTO and Chief Scientist JetBrains, Inc
http://www.jetbrains.com "Develop with pleasure!"

>
>> We can't gray out parameters of public methods because:
>>
>> 1) This method could be assigned to delegate, and this parameters is
>> necessary to match the signature
>> 2) It could be quasi-implementation of some interface method
>


0
Comment actions Permalink

"So anything public is potentially callable, and thus we
almost never want public members, methods, whatever, to be flagged as
unused."

It's not about flagging public members or methods. It's about flagging method parameters which are unused by the method.

But of course it should be an option, like everything else is in R#...

0

Please sign in to leave a comment.