Should IDEA warn on empty if and while?

I have a coworker who spent some time tracking down a problem to finally realize the code had an empty if. Here is what I mean:


Clearly the semicolon after the if was a mistake. At least, it's clear to you and me. Might it also be clear to IDEA?

I'm thinking it would be nice if IDEA would highlight if and while statments that have no body in yellow (a warning). The downside is that some code does this intentionally:


Our thoughts where that you could ignore the yellow marker if you desired this code. Or, IDEA could skip the warning if curly braces were used. So that

would not cause a warning message.

Does anyone else think this check would be useful, or have a better heuristic for finding just the problem cases?

0
15 comments
Avatar
Permanently deleted user


The functionality you describe exists as part of the InspectionGadgets plugin (along without about 200 other semantic checks), assuming you're running the latest Aurora EAP. Look in the PluginManager inspections.

0
Avatar
Permanently deleted user

Can the inspection run with my having to explicitly execute "Inspect Code"? Will it label things as warnings or errors?

BTW, great job with the Inspection Gadgets, that is very cool.

0
Avatar
Permanently deleted user

I meant WITHOUT my having to explicitly execute the inspection. Opps.

0
Avatar
Permanently deleted user

Never mind, I just read the instructions on the Twiki site. RTFM to myself!

:)

0
Avatar
Permanently deleted user

John Murph <no_mail@jetbrains.com> wrote:

I have a coworker who spent some time tracking down a problem to finally
realize the code had an empty if. Here is what I mean:

     if (condition == true);
>     {
>         do work;
>     }]]>


Clearly the semicolon after the if was a mistake. At least, it's clear to
you and me. Might it also be clear to IDEA?


How often does this happen? If have never seen such a mistake in one of
my projects.

Dirk Dittert

0
Avatar
Permanently deleted user

I agree it probably doesn't happen very often, but having made this goof
once a few years back I can tell you that it can be very hard (for
humans--at least this human) to spot! If it's not to hard to add an
inspection, I think it may be worthwhile.

Kendall

"Dirk Dittert" <dittert@despammed.com> wrote in message
news:1g3cuuj.1awvgg21ydanm3N%dittert@despammed.com...

John Murph <no_mail@jetbrains.com> wrote:

>

I have a coworker who spent some time tracking down a problem to finally
realize the code had an empty if. Here is what I mean:

 >     if (condition == true);
> >     {
> >         do work;
> >     }]]>

>

Clearly the semicolon after the if was a mistake. At least, it's clear

to

you and me. Might it also be clear to IDEA?

>

How often does this happen? If have never seen such a mistake in one of
my projects.



0
Avatar
Permanently deleted user

That would be unacceptable to me. Forcing people to get into the habit
of ignoring warnings is a bad idea. And forcing people to sort through
warnings to decide which are OK to ignore and which aren't degrades
productivity and invites mistakes. However, it would be acceptable if
this mark-up could be turned off, as most of the others (e.g., non-final
declarations that could be final) can be.

If, while, and especially for statements with empty bodies that are
perfectly legitimate do turn up from time to time. Having this as an
inspection is probably the best way to go. If, indeed, the frequency of
this kind of coding error justifies any remedy at all, which I doubt.

John Murph wrote:

Our thoughts where that you could ignore the yellow marker if you desired this code.


0
Avatar
Permanently deleted user

Kendall Collett <tm237hq8jk4302@sneakemail.com> wrote:

If it's not to hard to add an inspection, I think it may be worthwhile.


I'd prefer if these things are offline inspections that are run from
time to time. Checking for every possible bug just slows down error
checking in the editor.

Dirk Dittert

0
Avatar
Permanently deleted user

>However, it would be acceptable if
>this mark-up could be turned off, as most of the others
>(e.g., non-final declarations that could be final) can be.

All inspections and on-line error checks are independently toggleable, for ust that reason.

>If, while, and especially for statements with empty bodies that are
>perfectly legitimate do turn up from time to time.

I would disagree, as would many coding standards. That's why I wrote the inspection.

0
Avatar
Permanently deleted user

You are, of course, free to disagree. But a development tool should
support ANY reasonable coding standard, not just yours.

Dave Griffith wrote:
>>However, it would be acceptable if
>>this mark-up could be turned off, as most of the others
>>(e.g., non-final declarations that could be final) can be.


All inspections and on-line error checks are independently toggleable, for ust that reason.

>>If, while, and especially for statements with empty bodies that are
>>perfectly legitimate do turn up from time to time.


I would disagree, as would many coding standards. That's why I wrote the inspection.


0
Avatar
Permanently deleted user


Did you miss the part where I described these inspections and warnings as independently toggleable? If you don't like the "warn on empty statements" rule in InspectionGadgets, simply disable it. In any case, as I'm actually the one who put in hundreds of free hours coding these inspections, it actually really does come down to what standards do I wish to support. I've tried to be flexible, and do support more semantic checks than any other package on the market, but there are limits. If you don't like it, either write your own inspection package or try to get JetBrains to do so. Lately, they haven't really been all that interested.

--Dave

0
Avatar
Permanently deleted user

Have you considered starting something like a BileBlog? I think you'd do a great job bashing people like Hani does. (I'm serious, I hope you don't take this as an insult. I love IG, I swear.)

0
Avatar
Permanently deleted user


Naw, those two guys just caught me on a bad day, and after spending about four hours polishing up the next IG release (Many bugfixes! Better cosmetics! 11 new Inspections! Monday!). As an general practice, I prefer "mature but wry voice of common sense" to "bomb-throwing anarchist" as my day-to-day voice. Don't get me wrong, I like what Hani does, it's just not me.

--Dave

0
Avatar
Permanently deleted user

I just want to pipe up again, and say THANKS Dave for the IG plugin. It's exactly what I was looking for.

To everyone else:

1) No, this error does not often occur, but I like my editor/compiler to catch my errors rather than having to sift through code for hours trying to find an extra semicolon. If you disagree, SmallTalk is probably a better language for you to be using than Java. :)

2) With Dave's InspectionGadgets, you can enable just the checks you want, and can make them only happen on a code inspection or during a file parse (and can make them warnings or errors during the parse as you see fit). What a great, flexible solution! So, why are people complaining about it??

0
Avatar
Permanently deleted user

John Murph wrote:
> 2) With Dave's InspectionGadgets, you can enable just the checks you
want, and can make them only happen on a code inspection or during a
file parse (and can make them warnings or errors during the parse as you
see fit). What a great, flexible solution! So, why are people
complaining about it??

I'm complaining because I can't get it on 3.0.4. :(

Sumit.

0

Please sign in to leave a comment.