From 44e43d618639065a7a71337b6205c1a6db723e86 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 11 Apr 2020 20:02:48 +0200 Subject: [PATCH 1/4] [core] Fix HTMLRenderer not using linkPrefix/linePrefix * add new option "htmlExtension", so that the mechanism can be used with github as well * render links for suppressed violations and errors Fixes #2412 --- docs/pages/release_notes.md | 1 + .../pmd/renderers/HTMLRenderer.java | 58 +++++++++++++++---- .../pmd/renderers/SummaryHTMLRenderer.java | 2 + .../pmd/renderers/AbstractRendererTest.java | 2 +- .../pmd/renderers/HTMLRendererTest.java | 33 ++++++++++- .../renderers/SummaryHTMLRendererTest.java | 5 +- 6 files changed, 85 insertions(+), 16 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index a470494bab..b20e4ec7ff 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -48,6 +48,7 @@ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD * core * [#2355](https://github.com/pmd/pmd/issues/2355): \[doc] Improve documentation about incremental analysis * [#2356](https://github.com/pmd/pmd/issues/2356): \[doc] Add missing doc about pmd.github.io + * [#2412](https://github.com/pmd/pmd/issues/2412): \[core] HTMLRenderer doesn't render links to source files * java * [#2378](https://github.com/pmd/pmd/issues/2378): \[java] AbstractJUnitRule has bad performance on large code bases * java-codestyle diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/HTMLRenderer.java b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/HTMLRenderer.java index f9ef8b739a..5be922ea56 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/HTMLRenderer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/HTMLRenderer.java @@ -15,7 +15,10 @@ import org.apache.commons.lang3.StringUtils; import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Report.ConfigurationError; +import net.sourceforge.pmd.Rule; import net.sourceforge.pmd.RuleViolation; +import net.sourceforge.pmd.properties.PropertyDescriptor; +import net.sourceforge.pmd.properties.PropertyFactory; import net.sourceforge.pmd.properties.StringProperty; /** @@ -32,9 +35,15 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { public static final StringProperty LINE_PREFIX = new StringProperty("linePrefix", "Prefix for line number anchor in the source file.", null, 1); public static final StringProperty LINK_PREFIX = new StringProperty("linkPrefix", "Path to HTML source.", null, 0); + public static final PropertyDescriptor HTML_EXTENSION = PropertyFactory.booleanProperty("htmlExtension") + .desc("Replace file extension with .html for the links (default: false)") + // default value is false - to have the old (pre 6.23.0) behavior, this needs to be set to true. + .defaultValue(false) + .build(); private String linkPrefix; private String linePrefix; + private boolean replaceHtmlExtension; private int violationCount = 1; boolean colorize = true; @@ -44,6 +53,7 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { definePropertyDescriptor(LINK_PREFIX); definePropertyDescriptor(LINE_PREFIX); + definePropertyDescriptor(HTML_EXTENSION); } @Override @@ -61,6 +71,7 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { public void renderBody(Writer writer, Report report) throws IOException { linkPrefix = getProperty(LINK_PREFIX); linePrefix = getProperty(LINE_PREFIX); + replaceHtmlExtension = getProperty(HTML_EXTENSION); writer.write("

PMD report

"); writer.write("

Problems found

"); @@ -78,6 +89,10 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { @Override public void start() throws IOException { + linkPrefix = getProperty(LINK_PREFIX); + linePrefix = getProperty(LINE_PREFIX); + replaceHtmlExtension = getProperty(HTML_EXTENSION); + writer.write("PMD" + PMD.EOL); writer.write("

PMD report

"); writer.write("

Problems found

"); @@ -116,8 +131,7 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { buf.append("> " + PMD.EOL); buf.append("" + violationCount + "" + PMD.EOL); buf.append("" - + maybeWrap(StringEscapeUtils.escapeHtml4(determineFileName(rv.getFilename())), - linePrefix == null ? "" : linePrefix + Integer.toString(rv.getBeginLine())) + + renderFileName(rv.getFilename(), rv.getBeginLine()) + "" + PMD.EOL); buf.append("" + Integer.toString(rv.getBeginLine()) + "" + PMD.EOL); @@ -134,6 +148,20 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { } } + private String renderFileName(String filename, int beginLine) { + return maybeWrap(StringEscapeUtils.escapeHtml4(determineFileName(filename)), + linePrefix == null || beginLine < 0 ? "" : linePrefix + beginLine); + } + + private String renderRuleName(Rule rule) { + String name = rule.getName(); + String infoUrl = rule.getExternalInfoUrl(); + if (StringUtils.isNotBlank(infoUrl)) { + return "" + name + ""; + } + return name; + } + private void glomProcessingErrors(Writer writer, List errors) throws IOException { if (errors.isEmpty()) { @@ -155,7 +183,7 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { } colorize = !colorize; buf.append("> " + PMD.EOL); - buf.append("" + determineFileName(pe.getFile()) + "" + PMD.EOL); + buf.append("" + renderFileName(pe.getFile(), -1) + "" + PMD.EOL); buf.append("
" + pe.getDetail() + "
" + PMD.EOL); buf.append("" + PMD.EOL); writer.write(buf.toString()); @@ -183,9 +211,10 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { } colorize = !colorize; buf.append("> " + PMD.EOL); - buf.append("" + determineFileName(sv.getRuleViolation().getFilename()) + "" + PMD.EOL); - buf.append("" + sv.getRuleViolation().getBeginLine() + "" + PMD.EOL); - buf.append("" + sv.getRuleViolation().getRule().getName() + "" + PMD.EOL); + RuleViolation rv = sv.getRuleViolation(); + buf.append("" + renderFileName(rv.getFilename(), rv.getBeginLine()) + "" + PMD.EOL); + buf.append("" + rv.getBeginLine() + "" + PMD.EOL); + buf.append("" + renderRuleName(rv.getRule()) + "" + PMD.EOL); buf.append("" + (sv.suppressedByNOPMD() ? "NOPMD" : "Annotation") + "" + PMD.EOL); buf.append("" + (sv.getUserMessage() == null ? "" : sv.getUserMessage()) + "" + PMD.EOL); @@ -215,7 +244,7 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { } colorize = !colorize; buf.append("> " + PMD.EOL); - buf.append("" + ce.rule().getName() + "" + PMD.EOL); + buf.append("" + renderRuleName(ce.rule()) + "" + PMD.EOL); buf.append("" + ce.issue() + "" + PMD.EOL); buf.append("" + PMD.EOL); writer.write(buf.toString()); @@ -227,11 +256,16 @@ public class HTMLRenderer extends AbstractIncrementingRenderer { if (StringUtils.isBlank(linkPrefix)) { return filename; } - String newFileName = filename; - int index = filename.lastIndexOf('.'); - if (index >= 0) { - newFileName = filename.substring(0, index).replace('\\', '/'); + String newFileName = filename.replace('\\', '/'); + + if (replaceHtmlExtension) { + int index = filename.lastIndexOf('.'); + if (index >= 0) { + newFileName = filename.substring(0, index); + } } - return "" + newFileName + ""; + + return "" + + newFileName + ""; } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/SummaryHTMLRenderer.java b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/SummaryHTMLRenderer.java index cc5f56dbb3..fb6d8cf272 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/SummaryHTMLRenderer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/SummaryHTMLRenderer.java @@ -24,6 +24,7 @@ public class SummaryHTMLRenderer extends AbstractAccumulatingRenderer { // Renderer definePropertyDescriptor(HTMLRenderer.LINK_PREFIX); definePropertyDescriptor(HTMLRenderer.LINE_PREFIX); + definePropertyDescriptor(HTMLRenderer.HTML_EXTENSION); } @Override @@ -41,6 +42,7 @@ public class SummaryHTMLRenderer extends AbstractAccumulatingRenderer { HTMLRenderer htmlRenderer = new HTMLRenderer(); htmlRenderer.setProperty(HTMLRenderer.LINK_PREFIX, getProperty(HTMLRenderer.LINK_PREFIX)); htmlRenderer.setProperty(HTMLRenderer.LINE_PREFIX, getProperty(HTMLRenderer.LINE_PREFIX)); + htmlRenderer.setProperty(HTMLRenderer.HTML_EXTENSION, getProperty(HTMLRenderer.HTML_EXTENSION)); htmlRenderer.setShowSuppressedViolations(showSuppressedViolations); htmlRenderer.setUseShortNames(inputPathPrefixes); htmlRenderer.renderBody(writer, report); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java index bf3892f032..97e0e6664d 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java @@ -61,7 +61,7 @@ public abstract class AbstractRendererTest { getRenderer().renderFileReport(null); } - private Report reportOneViolation() { + protected Report reportOneViolation() { Report report = new Report(); report.addRuleViolation(newRuleViolation(1)); return report; diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java index 02651b0b3b..0644230a44 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java @@ -4,9 +4,17 @@ package net.sourceforge.pmd.renderers; +import static org.junit.Assert.assertEquals; + +import java.io.IOException; + +import org.junit.Test; + import net.sourceforge.pmd.PMD; +import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Report.ConfigurationError; import net.sourceforge.pmd.Report.ProcessingError; +import net.sourceforge.pmd.ReportTest; public class HTMLRendererTest extends AbstractRendererTest { @@ -26,9 +34,18 @@ public class HTMLRendererTest extends AbstractRendererTest { @Override public String getExpected() { + return getExpected(null, null); + } + + private String getExpected(String linkPrefix, String linePrefix) { + String filename = getEscapedFilename(); + if (linkPrefix != null) { + filename = "" + + filename + ""; + } return getHeader() + " " + PMD.EOL + "1" + PMD.EOL - + "" + getEscapedFilename() + "" + PMD.EOL + "1" + PMD.EOL + + "" + filename + "" + PMD.EOL + "1" + PMD.EOL + "blah" + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL; } @@ -72,4 +89,18 @@ public class HTMLRendererTest extends AbstractRendererTest { + "

PMD report

Problems found

" + PMD.EOL + "" + PMD.EOL; } + + @Test + public void testLinkPrefix() throws IOException { + final HTMLRenderer renderer = new HTMLRenderer(); + final String linkPrefix = "https://github.com/pmd/pmd/blob/master/"; + final String linePrefix = "L"; + renderer.setProperty(HTMLRenderer.LINK_PREFIX, linkPrefix); + renderer.setProperty(HTMLRenderer.LINE_PREFIX, linePrefix); + renderer.setProperty(HTMLRenderer.HTML_EXTENSION, false); + + Report rep = reportOneViolation(); + String actual = ReportTest.render(renderer, rep); + assertEquals(filter(getExpected(linkPrefix, linePrefix)), filter(actual)); + } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java index e4421d2347..bcbb301431 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java @@ -28,6 +28,7 @@ public class SummaryHTMLRendererTest extends AbstractRendererTest { Renderer result = new SummaryHTMLRenderer(); result.setProperty(HTMLRenderer.LINK_PREFIX, "link_prefix"); result.setProperty(HTMLRenderer.LINE_PREFIX, "line_prefix"); + result.setProperty(HTMLRenderer.HTML_EXTENSION, true); return result; } @@ -95,7 +96,7 @@ public class SummaryHTMLRendererTest extends AbstractRendererTest { + PMD.EOL + "" + PMD.EOL + "
#FileLineProblem
#FileLineProblem

Processing errors

" + PMD.EOL + "" + PMD.EOL + " " + PMD.EOL - + "" + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL + + "" + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL + "
FileProblem
file
" + error.getDetail() + "
file
" + error.getDetail() + "
" + PMD.EOL; } @@ -129,7 +130,7 @@ public class SummaryHTMLRendererTest extends AbstractRendererTest { + PMD.EOL + "#FileLineProblem" + PMD.EOL + "

Suppressed warnings

" + PMD.EOL + "" - + PMD.EOL + " " + PMD.EOL + "" + PMD.EOL + + PMD.EOL + " " + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL + "
FileLineRuleNOPMD or AnnotationReason
1FooNOPMDtest
" + PMD.EOL, actual); From 1572602de9b690da60fd2848ca194d98d0595b86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 20 Apr 2020 10:25:14 +0200 Subject: [PATCH 2/4] Clarify doc --- docs/pages/pmd/userdocs/pmd_report_formats.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/pages/pmd/userdocs/pmd_report_formats.md b/docs/pages/pmd/userdocs/pmd_report_formats.md index ff7fd639e7..60c5c8c13d 100644 --- a/docs/pages/pmd/userdocs/pmd_report_formats.md +++ b/docs/pages/pmd/userdocs/pmd_report_formats.md @@ -82,8 +82,11 @@ Example: HTML format. -This renderer provides two properties. If these are provided, then a link to the source where the violations -have been found is rendered. The following example has been created with `-property linkPrefix=https://github.com/pmd/pmd/blob/master/ -property linePrefix=L -shortnames -d pmd`. +This renderer provides two properties to render a link to the source where the violations +have been found. The following example has been created with `-property linkPrefix=https://github.com/pmd/pmd/blob/master/ -property linePrefix=L -shortnames -d pmd`. +If "linkPrefix" is not set, then "linePrefix" has no effect anyway: just the filename will +be rendered, with no html link. Otherwise if "linePrefix" is not set, then the link will +not contain a line number. When using [Maven JXR Plugin](https://maven.apache.org/jxr/maven-jxr-plugin/index.html) to generate a html view of the project's sources, then the property "htmlExtension" needs to be set to "true". This will then replace the From c7fce361a5182f94cdf2586a992e9a4f384e00ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 20 Apr 2020 10:30:45 +0200 Subject: [PATCH 3/4] Test when linePrefix is not set --- .../pmd/renderers/HTMLRendererTest.java | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java index 0644230a44..6c18ab9440 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java @@ -37,10 +37,10 @@ public class HTMLRendererTest extends AbstractRendererTest { return getExpected(null, null); } - private String getExpected(String linkPrefix, String linePrefix) { + private String getExpected(String linkPrefix, String lineAnchor) { String filename = getEscapedFilename(); if (linkPrefix != null) { - filename = "" + filename = "" + filename + ""; } return getHeader() @@ -101,6 +101,19 @@ public class HTMLRendererTest extends AbstractRendererTest { Report rep = reportOneViolation(); String actual = ReportTest.render(renderer, rep); - assertEquals(filter(getExpected(linkPrefix, linePrefix)), filter(actual)); + assertEquals(filter(getExpected(linkPrefix, "L1")), filter(actual)); + } + + @Test + public void testLinePrefixNotSet() throws IOException { + final HTMLRenderer renderer = new HTMLRenderer(); + final String linkPrefix = "https://github.com/pmd/pmd/blob/master/"; + renderer.setProperty(HTMLRenderer.LINK_PREFIX, linkPrefix); + // dont set line prefix renderer.setProperty(HTMLRenderer.LINE_PREFIX, linePrefix); + renderer.setProperty(HTMLRenderer.HTML_EXTENSION, false); + + Report rep = reportOneViolation(); + String actual = ReportTest.render(renderer, rep); + assertEquals(filter(getExpected(linkPrefix, "")), filter(actual)); } } From ca75d9492da3b9ea1916c13dd48e0528cb374f8a Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 23 Apr 2020 11:13:18 +0200 Subject: [PATCH 4/4] [core] Add test case with empty line prefix for HtmlRenderer --- .../sourceforge/pmd/renderers/HTMLRendererTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java index 6c18ab9440..9daba62e06 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/HTMLRendererTest.java @@ -116,4 +116,17 @@ public class HTMLRendererTest extends AbstractRendererTest { String actual = ReportTest.render(renderer, rep); assertEquals(filter(getExpected(linkPrefix, "")), filter(actual)); } + + @Test + public void testEmptyLinePrefix() throws IOException { + final HTMLRenderer renderer = new HTMLRenderer(); + final String linkPrefix = "https://github.com/pmd/pmd/blob/master/"; + renderer.setProperty(HTMLRenderer.LINK_PREFIX, linkPrefix); + renderer.setProperty(HTMLRenderer.LINE_PREFIX, ""); + renderer.setProperty(HTMLRenderer.HTML_EXTENSION, false); + + Report rep = reportOneViolation(); + String actual = ReportTest.render(renderer, rep); + assertEquals(filter(getExpected(linkPrefix, "1")), filter(actual)); + } }