Should empty "if" statement in C# lead to an error or warning?

Question

Let me start from a real life example:

Customer: Alex, just noticed something strange in the RemovalProcessor at line 138:

if (Session.Handler.ExecutePrefetchTasks()==null); 
  Session.ExecuteDelayedQueries(); 

Should the semicolumn behind the 'if' be there?

Me: Oops... I'll send this to our guys to check, but most likely, you're right.

Although the case is rare, I admit nearly any big project has similar issue.

I understand that semicolon (and statement block) usage rules in C# can't be changed (personally I'd prefer Python style). But I think it's a good idea to identify exactly this case with if statement, and classify it as an error or warning.

Few Q/A I have in mind:

• Why warning or error should be generated in this case?

  Because it's a developer's mistake with may be 99% probability.

• Why error is preferable in this case?

  In many cases warnings are ignored by developers.

  I understand this is their own problem, and there is /warnaserror (threat warnings as errors) switch, but since this is an error with very high probability, and, if it isn't an error (really? ;) ), it's quite easy to fix this, may be it's better to classify this case as an error.

  Finally, error in this case won't "restrict" the developer, since such code can (and likely, must) always be rewritten without if statement.

• Why warning is preferable in this case?

  This won't break the compatibility; I also suspect some code generators may generate code relying on the current behavior.

So I'd be glad to hear your opinions about this.

Answer 1

It already produces a warning:

Possible mistaken empty statement

I agree with you that an error would have been preferable (if you really want an empty statement, can always write it as { }, which is more explicit) — but they are not going to change the C# language in this way. It would be a breaking change, and I suspect their (read: Eric Lippert’s) justification will be “the benefit doesn’t outweigh the cost”.

Answer 2

This example, similar to yours, should not produce a compile error because it has the purpose to execute logic conditionally.

if ( DoSomething() || SolveEquation() ) ;  // Intentional - compiles and  runs as expected.

If the if keyword is removed you will get a compile error.

DoSomething() || SolveEquation();  // Compile ERROR.

Short circuit logic of C# determines: if DoSomething() returns true then SolveEquation() is not evaluated; otherwise it is evaluated.

We'll hypothesize that both functions change the state of the running program in some way, so it's important which one is executed - because that will determine a specific kind of program state. Of course the better way to write it would be...

if ( ! DoSomething() )
    SolveEquation();

or (Thanks Alex for this one from the comments)

bool result = DoSomething() || SolveEquation(); 

.. but why rob the user of the first way?

Because the first way can be argued to be bad form, but it proves you don't necessarily want the situation to produce a compile error.

This is my argument that it should not produce a compile error. The existing warning is good just in case the developer made a common mistake.