New rule: NoAssignmentProofComparison

Apr 4, 2011 at 10:05 PM

Not sure if this was discussed before.

I would like to have a new rule which verifies that source code doesn't have C++ style assignment proof comparisons.

Examples:

false == b

5 == a

10 < i

Actually I have this rule implemented and can share its code.

Coordinator
Apr 4, 2011 at 10:23 PM
Great. Send it through to me and I'll take a look



On 4 Apr 2011, at 23:05, "abibok" <notifications@codeplex.com> wrote:

From: abibok

Not sure if this was discussed before.

I would like to have a new rule which verifies that source code doesn't have C++ style assignment proof comparisons.

Examples:

false == b

5 == a

10 < i

Actually I have this rule implemented and can share its code.

Apr 5, 2011 at 6:33 AM

See email.

Apr 5, 2011 at 10:55 PM

Why is this bad?

Apr 6, 2011 at 12:43 AM

Because it's like saying "if blue is the sky" or "if tall is the man". Quite a few people call this style "Yoda conditions". Such reverse order has no benefits in C# and affects code readability. 

http://www.removingalldoubt.com/commentview.aspx/1d2c1827-9b4d-493e-88ba-10ab27c0dae6

Apr 6, 2011 at 12:56 AM

Ok! Thnaks for the clarification.

You can still fall in the C++ assignment mistake if you're working with booleans:

if (Enable = flag)
{
    // ...
}
Developer
Apr 6, 2011 at 5:58 AM

Ah, yes, such "if (Enable = flag)" problems could be detected with another rule.  :)  I'll start another thread to propose.

Anyway, I agree with the proposed NoAssignmentProofComparison rule, but I wonder if we can use a more direct name like NoValueFirstComparisons?

Developer
Apr 6, 2011 at 9:14 PM

Abibok,

As I didn't see your code, just wanted to give some examples for you to check whether you handle it correctly.

5 != a
(5 + 6 / 7) == a
(5 + 6 / x) == a
5 + 6 == 6 + 5
Foo(5) == a
Foo(x) == a
5 == Foo(5)
5 == Foo(a) 

It would be nice if you told the result for each of them.

Best regards,
Oleg Shuruev

Apr 6, 2011 at 11:08 PM

Results:

5 != a Violation
(5 + 6 / 7) == a Ok
(5 + 6 / x) == a Ok
5 + 6 == 6 + 5 Ok
Foo(5) == a Ok
Foo(x) == a Ok
5 == Foo(5) Violation
5 == Foo(a)  Violation

Apr 6, 2011 at 11:14 PM

Here are some more test cases with violations:

while (true == b) { }
do{}while (1 < a);
if (true == b || a == 0) { }
if (a == 0 && (true == b) && null == x) { }
if (b || true || null != x) { }
if (null == x && 1 == a) { }
if (a == 1) { } else if (false == b) {

Jan 14 at 4:41 PM
Andy, I sent a pull request "SA1216 - NoValueFirstComparison". Could you review it?
Jan 26 at 2:53 PM
Definitely looking forward to this rule--I was going to write a custom one at work to do the same thing.