IG - does it find this?
Dave,
Is there an inspection that would catch this bug/unexecutable code?
if (expression) {
// some code
} else if (expression) { // same expression
// some other code
}
I just ran into it.
Thanks,
Jon
Please sign in to leave a comment.
Not IG, but IDEA's own inspection called Local Code Analysis | Constant conditions
& exceptions though for very limited cases of expression.
For example:
if (a instanceof JComponent) {
} else if (a instanceof JComponent) {
}
would be catched while
if ( a > 0) {
} else if ( a > 0) {}
won't be.
Anyway, the expression should not have any method calls inside since IDEA
takes a pessimistic assumption methods could return different values even
if called against same parameters.
What was your 'expression' by the way?
The expression was:
(outbound == Outbound.REQUESTS_OUTBOUND)
Jon
I suppose what I was looking for in a new Inspection is a check to see whether the else expression is identical regardless of whether the evaulation of it might change due to multi-threading.
Jon
This one should be catched. Enable that inspection. It mostly reports very
useful stuff indeed.
>> What was your 'expression' by the way?
>>
Hi,
You can find this patterns by structural search with:
if ($condition$) {
$statements$;
} else if ($condition$) {
$statements2$;
}
statements, statements2 occurences 0..MaxInt
Jon Steelman wrote:
--
Maxim Mossienko
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"
Hmm, doesn't seem to be working. I enabled it from Errors > Local Code Analysis > Constant conditions & exception. Tried it both severities just in case. I'm on 3177.
Thanks,
Jon
And just for a sanity check, I confirmed that your example is caught by the Inspection.
Is that Outbound.REQUESTS_OUTBOUND a final static field? If it isn't final
its value could be changed by another thread we're trying to be aware of.
The other probable reason could be the method is a way to long/complex to
be analyzed. We're breaking the analysis if it takes too much time.
Outbound is an enum and REQUESTS_OUTBOUND is one of its enums.
Jon
Thanks a lot! It seem we missed this particular role of constants when enums
appear in 1.5. To be fixed really soon.
Maxim Shafirov wrote:
> For example:
>
> if (a instanceof JComponent) {
> } else if (a instanceof JComponent) {
> }
>
> would be catched while
> if ( a > 0) {
> } else if ( a > 0) {} won't be.
Ok, here's my stupid question for today. Where's the difference between
(a > 0) and (a == 0) ?
Would it be possible to have an option that always flags identical conditions
in if/else if branches, regardless of threading and side effect issues? False
positives shouldn't be an issue with the new possibilities to suppress certain
inspections, but this would greatly help to identify bad code as well as code
smells.
Sascha
This particual inspection's algoritm is just much smarter than that. It doesn't
check conditions for equality but tries to evaluate as many as possible paths
of the control flow, which lead to the different states of the JVM memory.
For example:
int x;
if (someExpr) {
x = 2;
}
else {
x = 3;
}
if (x == 4) {
}
'x == 4' condition will be identified as always false cause there's no paths
of execution goes into the if ever.
>, <, >=, <= conditions are just a bit harder to implement in this respect
since they require interval logic memory states.
Max
Fixed.
Maxim Shafirov wrote:
I knew about that one,
but didn't know about the algorithm and the implications behind it. Thanks for
the explanation.
Anyway, don't you think that an additional inspection that just detects identical
if-conditions would make sense (even though it wouldn't be as smart as this one)?
Sascha
Makes sense. Really simple one.
You're welcome. I look forward to the fix. What release # has it?
Thanks,
Jon
P.S. I appreciate the high level of interaction that has occurred in this thread.
#3183
Sascha,
Are you going to create an SCR or JIRA entry for this so Maxim can track it conveniently?
Jon
Don't bother, I've already coded it up. Can't ship it, because the've moved my repository to subversion, and I can't get the subversion integration to work, but that's the sort of thing that makes EAP such an adventure.
--Dave Griffith