Why doesn't ReSharper flag erroneous use of structs?

Consider this:

   public struct NiceStruct
   {
      public float Floating;
   }


   public class JustTesting
   {
      public void DoSomething(NiceStruct niceStruct)
      {
         niceStruct.Floating = 42;
         return;
      }
   }

Why isn't ReSharper flagging the fact that setting the struct member Floating to 42 is futile? The value is discarded when the method returns.

(I encountered something like this in a program posted on Code Project. No wonder it didn't work! But it took me a while to find the error, and it would have helped immensely if ReSharper had pointed it out to me.)

In fact, this is such an obvious error I'm surprised that Visual Studio is not flagging it.

9 comments
Comment actions Permalink

No comments? Have I misunderstood something about how C# works?

Can someone from JetBrains please comment on this.

Thanks.

0
Comment actions Permalink

Well, the reason is pretty obvious because this will never produce any side-effects, structs are always passed and copied by value, so when you pass a struct to the method it actually creates a copy of the struct so why should R# warn you about something that will never impact anything else but its own scope?

0
Comment actions Permalink

Not sure I agree with that. Consider the following code:

public void DoSomething(float x)
{
   x = 42;
   return;
}


This will be flagged with the "value assigned is not used" warning. Why shouldn't the same warning appear if the value is assigned to a field of a struct passed by value?

0
Comment actions Permalink

One of the wonderful things about ReSharper is that it flags potential / probable programming errors.

This is a very probable programming error. I found an example of it in a Code Project program, and the program didn't work the way the programmer intended - he thought the value was getting changed in the caller's struct. It took me a while to find it, and it would have been a great help if ReSharper had flagged it for me.

It's also a bit subtle, as this does work if the passed object is a class and not a struct. Maybe that's what happened, maybe the programmer had originally defined NiceStruct as a class (named NiceClass?) and then later on while trying to optimize things he said to himself, "hey, structs are more efficient than classes, I'll just change NiceClass into a struct!" Or maybe a second programmer who was in love with structs changed the first programmer's classes to structs.

And as Richard has pointed out, ReSharper does warn about a similar situation involving simple variables.

0
Comment actions Permalink

Well, if the parameter isn't used then you should see that R# marks it as dead code, at least it does for me.

Go to Inspection Severity and make sure that Unused Parameter is set to anything on the list except to 'Do Not Show'.

But yeah, it might be a bug or something because it doesn't annotate the parameter like it does in Richard's example.

However, the examples don't really describe your case, because you're saying the parameter was actually used inside the method so you expect R# to tell you that the value is overwritten, right? which makes more sense to me because it doesn't issue any warning for it and I think it should.

0
Comment actions Permalink

Eyal, thanks for your interest.

Here's an updated sample program that better shows how insidious this problem is:

   public class ObjectA
   {
      public float TheAnswer;
   }


   public struct ObjectB
   {
      public float TheAnswer;
   }


   public class JustTesting
   {
      public static void Main()
      {
         ObjectA objectA = new ObjectA();
         ObjectB objectB = new ObjectB();

         DoSomething(objectA, objectB);

         Console.WriteLine("Answer A = " + objectA.TheAnswer +
                                         ", answer B = " + objectB.TheAnswer);
      }


      public static void DoSomething(ObjectA objectA, ObjectB objectB)
      {
         // Many complicated and inscrutable calculations

         // Now return the ultimate answer to the caller
         objectA.TheAnswer = 42;
         objectB.TheAnswer = 42;
      }
   }

The output of this program is:

Answer A = 42, answer B = 0

In the Code Project program I'm talking about the programmer was just using a struct (like ObjectB), and he apparently thought he could pass data back to the caller via the struct's members. Which doesn't work, because that's the way structs are.

What I'm saying is that the last statement in DoSomething() looks very much like a programming error, and that it would be nice if ReSharper flagged it as such.

0
Comment actions Permalink

Do you want RS to give warning in the below scenario as well? I.e. should RS treat this code as "bad practice" and issue a warning? I think RS only safely can display a warning if foo.Bar is assigned a value, but foo.Bar is never read, but isn't it already doing that?

bool Foo(SomeStruct foo)
{
  while (foo.Bar>0)
  {
     foo.Bar = GetSomethingCalculated();
     if (foo.Bar <= 0) return true;
  }
  return false;
}

0
Comment actions Permalink

> I think RS only safely can display a warning if foo.Bar is assigned a  value, but foo.Bar is never read,

Agreed.

> ... but isn't it already doing that?

I'm not getting any warning for the sample program I posted. Maybe I have a wrong option set?

I'm also a bit disappointed that nobody from JetBrains has replied on this thread.

0
Comment actions Permalink

Not sure why they aren't saying anything, I'm waiting for their answer to two topics myself and it seems like they either really busy or ignoring them, go figure. :/

0

Please sign in to leave a comment.