Re: new findbug errors

2010-11-22 Thread Vincent Hennebert
Hi,

On 19/11/10 15:28, Glenn Adams wrote:
 Thanks! You say:
 
 I note that we have not accepted findbugs as a tool of the project. I
 think that not all developers are a fan of the tool.

Findbugs has not been accepted as a tool of the project simply because
nobody has promoted its use strong enough yet, I believe.


 Is there an alternative, and equally or more effective tool that is
 acceptable to more developers, e.g., PMD? If there is, then perhaps we
 should migrate to it? If not, then perhaps we should make findbugs a tool
 of the project. Since there has been a findbugs target in build.xml for a
 while (at least it was there when I started on the project), it seems like
 it is in some way a tool of the project.
 
 In any case, I believe we should make regular use of findbugs or a suitable
 alternative, and that no commit should occur without verifying that no new
 errors/warnings are being introduced.

It is my intention to have a closer look at findbugs or PMD, and propose
that we adopt them eventually. (Although I strongly believe that this
should be done at the XML Graphics level.) Meanwhile, I’d be very happy
if experienced users could share their thoughts.


 Regards,
 Glenn
 
 On Fri, Nov 19, 2010 at 8:05 AM, Simon Pepping spepp...@leverkruid.euwrote:
 
 I committed an exclusion filter that suppresses all existing findbug
 warnings (966). I generated it from the findbugs xml report. Then I
 moved all NM_CONFUSING out of the automatically generated block.

 When you get a new findbugs warning, you may
 1. resolve it;
 2. leave it unresolved, and add it to the exclusion filter, with
 comment;
 3. leave it unsolved, and add it to the automatic block of the
 exclusion filter.

 I note that we have not accepted findbugs as a tool of the project. I
 think that not all developers are a fan of the tool.

 The main template of the xslt script that generated the exclusion
 filter is as follows:

 xsl:template match='//BugInstance'
  xsl:variable name=type select=@type/
  xsl:for-each select=Class
Match
  Class name={...@classname}/
  xsl:choose
xsl:when test=$type='EQ_DOESNT_OVERRIDE_EQUALS'
  xsl:text#xA;/xsl:text
  xsl:comment Listing the method 'equals' does not work
 /xsl:comment
/xsl:when
xsl:when test=$type='MF_CLASS_MASKS_FIELD'
  xsl:text#xA;/xsl:text
  xsl:comment Listing the field does not work; this makes the
 filter apply to all masked fields /xsl:comment
/xsl:when
xsl:when test=following-sibling::Method
  Method name={following-sibling::Method[1]/@name}/
/xsl:when
xsl:when test=following-sibling::Field
  Field name={following-sibling::Field[1]/@name}/
/xsl:when
xsl:otherwise
  xsl:text#xA;/xsl:text
  xsl:commentNeither method nor field/xsl:comment
/xsl:otherwise
  /xsl:choose
  Bug pattern={$type}/
/Match
  /xsl:for-each
 /xsl:template

 Simon

 On Wed, Nov 17, 2010 at 07:24:43PM +0100, Simon Pepping wrote:
 I will take care of this. I am now squashing the easier
 findbugs-reported bugs/problems. Then I will add the remaining ones to
 the exclusion file, such that those that are really excluded can be
 distinguished from those which have not yet been examined.

Thanks,
Vincent


Re: new findbug errors

2010-11-19 Thread Simon Pepping
I committed an exclusion filter that suppresses all existing findbug
warnings (966). I generated it from the findbugs xml report. Then I
moved all NM_CONFUSING out of the automatically generated block.

When you get a new findbugs warning, you may
1. resolve it;
2. leave it unresolved, and add it to the exclusion filter, with
comment;
3. leave it unsolved, and add it to the automatic block of the
exclusion filter.

I note that we have not accepted findbugs as a tool of the project. I
think that not all developers are a fan of the tool.

The main template of the xslt script that generated the exclusion
filter is as follows:

xsl:template match='//BugInstance'
  xsl:variable name=type select=@type/
  xsl:for-each select=Class
Match
  Class name={...@classname}/
  xsl:choose
xsl:when test=$type='EQ_DOESNT_OVERRIDE_EQUALS'
  xsl:text#xA;/xsl:text
  xsl:comment Listing the method 'equals' does not work /xsl:comment
/xsl:when
xsl:when test=$type='MF_CLASS_MASKS_FIELD'
  xsl:text#xA;/xsl:text
  xsl:comment Listing the field does not work; this makes the filter 
apply to all masked fields /xsl:comment
/xsl:when
xsl:when test=following-sibling::Method
  Method name={following-sibling::Method[1]/@name}/
/xsl:when
xsl:when test=following-sibling::Field
  Field name={following-sibling::Field[1]/@name}/
/xsl:when
xsl:otherwise
  xsl:text#xA;/xsl:text
  xsl:commentNeither method nor field/xsl:comment
/xsl:otherwise
  /xsl:choose
  Bug pattern={$type}/
/Match
  /xsl:for-each
/xsl:template

Simon

On Wed, Nov 17, 2010 at 07:24:43PM +0100, Simon Pepping wrote:
 I will take care of this. I am now squashing the easier
 findbugs-reported bugs/problems. Then I will add the remaining ones to
 the exclusion file, such that those that are really excluded can be
 distinguished from those which have not yet been examined.


Re: new findbug errors

2010-11-19 Thread Glenn Adams
Thanks! You say:

I note that we have not accepted findbugs as a tool of the project. I
think that not all developers are a fan of the tool.

Is there an alternative, and equally or more effective tool that is
acceptable to more developers, e.g., PMD? If there is, then perhaps we
should migrate to it? If not, then perhaps we should make findbugs a tool
of the project. Since there has been a findbugs target in build.xml for a
while (at least it was there when I started on the project), it seems like
it is in some way a tool of the project.

In any case, I believe we should make regular use of findbugs or a suitable
alternative, and that no commit should occur without verifying that no new
errors/warnings are being introduced.

Regards,
Glenn

On Fri, Nov 19, 2010 at 8:05 AM, Simon Pepping spepp...@leverkruid.euwrote:

 I committed an exclusion filter that suppresses all existing findbug
 warnings (966). I generated it from the findbugs xml report. Then I
 moved all NM_CONFUSING out of the automatically generated block.

 When you get a new findbugs warning, you may
 1. resolve it;
 2. leave it unresolved, and add it to the exclusion filter, with
 comment;
 3. leave it unsolved, and add it to the automatic block of the
 exclusion filter.

 I note that we have not accepted findbugs as a tool of the project. I
 think that not all developers are a fan of the tool.

 The main template of the xslt script that generated the exclusion
 filter is as follows:

 xsl:template match='//BugInstance'
  xsl:variable name=type select=@type/
  xsl:for-each select=Class
Match
  Class name={...@classname}/
  xsl:choose
xsl:when test=$type='EQ_DOESNT_OVERRIDE_EQUALS'
  xsl:text#xA;/xsl:text
  xsl:comment Listing the method 'equals' does not work
 /xsl:comment
/xsl:when
xsl:when test=$type='MF_CLASS_MASKS_FIELD'
  xsl:text#xA;/xsl:text
  xsl:comment Listing the field does not work; this makes the
 filter apply to all masked fields /xsl:comment
/xsl:when
xsl:when test=following-sibling::Method
  Method name={following-sibling::Method[1]/@name}/
/xsl:when
xsl:when test=following-sibling::Field
  Field name={following-sibling::Field[1]/@name}/
/xsl:when
xsl:otherwise
  xsl:text#xA;/xsl:text
  xsl:commentNeither method nor field/xsl:comment
/xsl:otherwise
  /xsl:choose
  Bug pattern={$type}/
/Match
  /xsl:for-each
 /xsl:template

 Simon

 On Wed, Nov 17, 2010 at 07:24:43PM +0100, Simon Pepping wrote:
  I will take care of this. I am now squashing the easier
  findbugs-reported bugs/problems. Then I will add the remaining ones to
  the exclusion file, such that those that are really excluded can be
  distinguished from those which have not yet been examined.



Re: new findbug errors

2010-11-19 Thread Simon Pepping
Glenn,

One note: You introduced findbugs as a tool, and you created the
findbugs target in the build file.

Simon

On Fri, Nov 19, 2010 at 08:28:09AM -0700, Glenn Adams wrote:
 Thanks! You say:
 
 I note that we have not accepted findbugs as a tool of the project. I
 think that not all developers are a fan of the tool.
 
 Is there an alternative, and equally or more effective tool that is
 acceptable to more developers, e.g., PMD? If there is, then perhaps we
 should migrate to it? If not, then perhaps we should make findbugs a tool
 of the project. Since there has been a findbugs target in build.xml for a
 while (at least it was there when I started on the project), it seems like
 it is in some way a tool of the project.
 
 In any case, I believe we should make regular use of findbugs or a suitable
 alternative, and that no commit should occur without verifying that no new
 errors/warnings are being introduced.


Re: new findbug errors

2010-11-19 Thread Glenn Adams
Nope. It was introduced along with PMD target by Max Berger in June, 2008,
see

http://svn.apache.org/viewvc?view=revisionrevision=666967

I only made some enhancements to it, like adding the exclusion file.

G.

On Fri, Nov 19, 2010 at 11:55 AM, Simon Pepping spepp...@leverkruid.euwrote:

 Glenn,

 One note: You introduced findbugs as a tool, and you created the
 findbugs target in the build file.

 Simon

 On Fri, Nov 19, 2010 at 08:28:09AM -0700, Glenn Adams wrote:
  Thanks! You say:
 
  I note that we have not accepted findbugs as a tool of the project. I
  think that not all developers are a fan of the tool.
 
  Is there an alternative, and equally or more effective tool that is
  acceptable to more developers, e.g., PMD? If there is, then perhaps we
  should migrate to it? If not, then perhaps we should make findbugs a
 tool
  of the project. Since there has been a findbugs target in build.xml
 for a
  while (at least it was there when I started on the project), it seems
 like
  it is in some way a tool of the project.
 
  In any case, I believe we should make regular use of findbugs or a
 suitable
  alternative, and that no commit should occur without verifying that no
 new
  errors/warnings are being introduced.



Re: new findbug errors

2010-11-17 Thread Simon Pepping
I will take care of this. I am now squashing the easier
findbugs-reported bugs/problems. Then I will add the remaining ones to
the exclusion file, such that those that are really excluded can be
distinguished from those which have not yet been examined.

Simon

On Tue, Nov 16, 2010 at 09:41:00AM -0700, Glenn Adams wrote:
 I notice a few new findbugs errors creeping into trunk. Please try to run
 the findbugs target before commiting in order to fix any newly introduced
 bugs. Here are the new ones I see:
 
  a href=#MS_SHOULD_BE_FINALBug type MS_SHOULD_BE_FINAL (click for
 details)/a
  br/In class org.apache.fop.render.rtf.rtflib.rtfdoc.RtfTextrunbr/Field
 org.apache.fop.render.rtf.rtflib.rtfdoc.RtfTextrun.logbr/At
 RtfTextrun.java:[line 57]/p
 
  a href=#DM_NUMBER_CTORBug type DM_NUMBER_CTOR (click for details)/a
  br/In class org.apache.fop.render.rtf.rtflib.rtfdoc.RtfTextrunbr/In
 method
 org.apache.fop.render.rtf.rtflib.rtfdoc.RtfTextrun.addParagraphBreak()br/Called
 method new Integer(int)br/Should cal\
 l Integer.valueOf(int) insteadbr/At RtfTextrun.java:[line 304]/p
 
  a href=#DLS_DEAD_LOCAL_STOREBug type DLS_DEAD_LOCAL_STORE (click for
 details)/a
  br/In class org.apache.fop.render.rtf.rtflib.rtfdoc.RtfTextrunbr/In
 method
 org.apache.fop.render.rtf.rtflib.rtfdoc.RtfTextrun.addString(String)br/Local
 variable named rbr/At RtfTextrun.java:[\
 line 274]/p
 
 After fixing the above three, there should remain 1048 errors/warnings
 reported by findbugs in trunk. It would be nice if we could eliminate all of
 these (either by fixing or by adding exclusions to findbugs-exclude.xml), as
 it is much easier to check for new ones if there are none to start with.
 
 Regards,
 Glenn