Is there an automated way to catch property self-assignment?

Potentially you could use a tool such as FxCop and write a custom rule using VisitAssignmentStatement:
Some examples:
Example1
Example2


Jay,

A great question. I believe most of the points which are relevant have been covered in various responses, but just to summarise:

  1. There is no way to handle this implicitly. The .Net compiler allows recursive properties, which is at the heart of this issue (in terms of not being able to 'capture' it). Note that, as per this question, that's not likely to change.
  2. As such, if you want to enforce the capture of this 'bug', you would need to do so explicitly via a 3rd party tool. I'm a ReSharper fan like many others, and that allows us to highlight property recursion where necessary. Also as others have mentioned, using FXCop along with effective Unit Testing patterns will also help you prevent such bugs from arising.
  3. Naming conventions are king. By providing an example as you have with deliberately 'useless' (!) variable and property naming, you've highlighted exactly WHY we should all be using an effective naming convention. The issue you highlight is one which has caught us all out, and becomes more pervasive as the code base grows.

Whilst potentially not the answer you were after, I think #3 is the most important point here. Naming items properly is the single most effective solution. Not only that but people might not have the luxury of relying on ReSharper and other such tools.

To that end I'd also recommend StyleCop. It can be a little invasive, but it is a great tool for helping a developer, or team of developers, adhere to a set of syntax conventions which you'll find will pretty quickly eradicate bugs such as the one you've highlighted.

Happy coding!


This isn't cast-iron, but if you installed ReSharper and set its inspection for 'Unused parameter' to 'Error', and further turned its Solution-Wide Analysis on, you'd see this in the code window:

enter image description here

One of these red indicators over in the right margin:

enter image description here

And this down in the status bar:

enter image description here

Which together make a combo that as a ReSharper user you'd soon become unable to ignore :)

Wouldn't stop you compiling, though.


You said A = A and B = B is the same, but this is not true! You can make changes in a property's getter and setter so A = A can change the variable like in the example below:

public Int32 A
{
    get { return _A++; }
    set { _A = value; }
}

So the compiler doesn't know if it is a mistake or not. Of course I would avoid such situations because it's not so easy to work with code like this (e.g. if you just have an assembly and don't know why A is changing each time) and I would avoid exposing an explicit setter of such a property and prefer something like below:

public UInt32 UniqueID { get { _UniqueID++; } }

public void Reset()
{
    _UniqueID = 0;
}

Conclusion

A compile time error isn't making any sense here because the compiler doesn't know what happens in the property (Remember: A property is just a simplification of two methods, set_MyProperty and get_MyProperty), also if the property changes (e.g. by being virtual) the behaviour may change too.

(EDIT) Avoid misleading naming

I write property and parameters in the same way you are doing it. So as an example of how a simple class might look:

public class MyClass
{
    public Int32 Sample { get; private set; }

    public MyClass(Int32 sample)
    {
        Sample = sample;
    }
}

I'm falling into the same trap like you every week around ~1 time so I never thought about changing the naming. But there are some suggestions of what you could use:

  • Use p(arameter) as prefix, however this is something which I would not recommend because it makes the code unreadable IMHO.
  • Use value as postfix, this seems ok for me. So instead of sample you would have sampleValue which is different to Sample (the name of the property) and it should be easier to detect if the property name is used instead of the parameter name
  • Use _ as prefix. I wouldn't use it because I already use _ as prefix for members to enable fast access to them and makes intellisense look strange IMHO.

I think this depends totaly on you personal or company coding style but i personally would use Value as postfix.

Tags:

C#