On 16/06/14 16:10, Glenn Adams wrote:
Given that, with the exception of yourself, the findbugs exclude file has
been continually updated to produce no warnings, I conclude that the FOP
dev ha implicitly or explicitly accepted it for quite some time.

And yet, consensus has not been reached on this, since at least one
developer is not keen on using it.

If there were a real benefit in using FindBugs, then I would be the
first one to promote its use. But that’s not the case, at least not in
the present state.

Quite on the contrary, I’ve seen bad things done to the code for the
sake of keeping FindBugs quiet. Like surrounding whole chunks of code
with checks for null when the variables are actually not meant to be
null. Or instanceof checks when the variable can have only one type.

Such checks seriously impede the readability of the code. Whoever comes
after that will wonder why there is a check for null, and why nothing is
done when the variable is null.

And if that were only that. They also make debugging much harder by
silently ignoring errors instead of making the code fail-fast.

I have yet to see cases where I think: “Oh yes that’s right, there was
a serious bug here, thanks FindBugs for letting me know.” In most cases,
it’s rather: “Naah, that’s not an error, I’m going to have to update
this exclude file again, which pattern should I put again, where’s the
page that lists them, ah here it is, oh dear, which one is it among
those hundreds?” And that’s to remain polite :-)

We don't need to do anything to the excludes file to make it acceptable. It
is acceptable now. That doesn't mean it couldn't be improved by additional
review and revision, but that is true of the entire FOP code base.

I hope the above shows why it’s not acceptable as it is. At the moment
and in my opinion, running FindBugs is a waste of time and energy rather
than a help.


On Mon, Jun 16, 2014 at 3:32 AM, Vincent Hennebert

Rather than repeating myself, I’ll just point to the past conversations
on this topic:

This one is particularly interesting:
It more or less matches my own experience with FindBugs.

The only way to ensure zero FindBugs warnings is to enforce it in
continuous integration, just like we did about Checkstyle. But before
doing that, you’ll have to come up with a satisfying exclude file that
filters out uninteresting warnings, and get the community to agree on


On 15/06/14 17:03, Glenn Adams wrote:

I'm getting tired of fixing findbugs warnings that people are leaving in
code base. I'm going to start inserting comments that will break
compiles/builds if people don't check and fix these. Today I see four new

*Dodgy Warnings*




Unchecked/unconfirmed cast from org.apache.fop.fo.FONode to
org.apache.fop.fo.flow.MultiCase in


Unchecked/unconfirmed cast from org.apache.fop.fo.FONode to
org.apache.fop.fo.flow.MultiSwitch in


Unchecked/unconfirmed cast from org.apache.fop.fo.FObj to
org.apache.fop.fo.flow.MultiSwitch in new


Redundant nullcheck of footnoteList, which is known to be non-null in
List, LayoutContext)

Reply via email to