Re: new findbug errors
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
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
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
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
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
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