#1206 SummaryHTMLRenderer always shows suppressed warnings/violations

SummaryHTMLRenderer creates an instance of HTMLRenderer to print the main part of the report after SummaryHTMLRenderer's summary at the top. HTMLRenderer inherits showSuppressedViolations from AbstractRenderer, which sets it to true by default. As SummaryHTMLRenderer does nothing to make HTMLRenderer's showSuppressedViolations correspond to the setting currently being used by SummaryHTMLRenderer, HTMLRenderer ends up printing the suppressions even if pmd is not passed the commandline option to show suppressions -- whereas under the same circumstances using HTMLRenderer directly will not print the suppressions.

For example, this java class...

@suppresswarnings("PMD")
public class EmptyTryCatch {
    public static void Main(String args[]) {
        try {
        } catch (Exception e) {
        }
    }
}

...yields these results when analyzed:

REM no suppressions shown, correctly, in HTML report
pmd -d [path to file] -f html -report CorrectNoSuppressions.html -R rulesets/java/empty.xml

REM suppressions shown, incorrectly, in Summary HTML report
pmd -d [path to file] -f summaryhtml -report IncorrectSuppressionsSummary.html -R rulesets/java/empty.xml

REM suppressions shown, correctly, in HTML report
pmd -d [path to file] -f html -report CorrectSuppressions.html -R rulesets/java/empty.xml -showsuppressed

REM suppressions shown, correctly but only coincidentally, in Summary HTML report
pmd -d [path to file] -f summaryhtml -report CorrectSuppressionsSummary.html -R rulesets/java/empty.xml -showsuppressed

I can think of three or four possible fixes for this.

A partial fix would be to make showSuppressedViolations default to false in AbstractRenderer, since not showing suppressions is the default behavior. This would reduce the incorrect behavior since the default behavior would then be correct, but would then incorrectly not print the suppressions when the option is used. It may have similar effects on other renderers that are not correctly setting this value as well; in fact, while it would be in a sense "less buggy" because the default behavior is correct, it would in another sense be more pervasively bugged since the incorrect behavior, being non-default, is less obvious and therefore less likely to be noticed and fixed.

A second, and complete solution would be to have SummaryHTMLRenderer set up its HTMLRenderer instance "properly". That is, either set it up in the same manner and providing the same properties and settings as whatever part of the code does for the SummaryHTMLRenderer itself, or at least once it has instantiated the HTMLRenderer explicitly set any relevant settings such as showSuppressedViolations based on the relevant equivalent settings in SummaryHTMLRenderer. (Whether these are one or two solutions depends, I suppose, on whether simply instantiating it and then setting the relevant settings isn't also how the SummaryHTMLRenderer is being set up anyway. I haven't found that part of the code yet.)

A third hypothetical solution would be for SummaryHTMLRenderer to extend HTMLRenderer and call super methods where appropriate to pass on the settings and to use HTMLRenderer's printing as part of SummarHTMLRenderer's printing. In a sense this is more automatic and structured than making SummaryHTMLRenderer repeat the work of setting up a renderer after other code has done that work to set the SummaryHTMLRenderer up. However, I don't know if this is feasible -- it looks like the two already inherit different abstract bases, for whatever reason, and I'm not familiar enough with the system to know if the reason for that is substantial or not.

As an aside, it looks like there are also a couple little oddities/bugs in the HTML output of SummaryHTMLRenderer. One, it outputs "<h2><center>Detail</h2></center>" instead of "<h2><center>Detail</center></h2>" or "<center><h2>Detail</h2></center>". Two, it outputs an opening "<tr>" before the HTMLRenderer's output but afterward no closing "</tr>" before the closing "</table></body></html>". It might be a good opportunity to fix these if any changes have to be made to SummaryHTMLRenderer anyway.

[Anonymous]

(So, I only figured out after this ticket that I need to have a blank line before/after ~~~~~~ code blocks for the formatting to work; if someone with the authority to edit this can fix those, I'd appreciate it -- I don't know if sourceforge will have already discarded the tab indentation in the Java code, but at least it won't just look like text...)

[Anonymous]

So, on further examination of the code, I think I can conclude:
1) SummaryHTMLRenderer is an accumulating renderer because it needs the accumulated statistics for the summary at the top. It can't be otherwise. HTMLRenderer being an incremental renderer, however, saves memory overhead when not using the summary. It's best that way. So, making the one extend the other is undesirable.
2) SummaryHTMLRenderer is setting the other settings (the properties) of HTMLRenderer simply by calling the set functions after creating the object. So, it'd be simple and consistent to have it call setShowSuppressedViolations(showSuppressedViolations); a line after that. I can probably figure out how to submit a patch with this later.

[Anonymous]

Well, this might be overkill for changing one line, but I think this patch will fix it.

[Andreas Dangel]

Thank you for the patch!
The bug will be fixed with the next release.