Merge branch 'master' into issue-2625
This commit is contained in:
10
Gemfile.lock
10
Gemfile.lock
@ -31,9 +31,9 @@ GEM
|
||||
multipart-post (>= 1.2, < 3)
|
||||
faraday-http-cache (1.3.1)
|
||||
faraday (~> 0.8)
|
||||
fugit (1.3.5)
|
||||
fugit (1.3.6)
|
||||
et-orbi (~> 1.1, >= 1.1.8)
|
||||
raabro (~> 1.1)
|
||||
raabro (~> 1.3)
|
||||
git (1.7.0)
|
||||
rchardet (~> 1.8)
|
||||
kramdown (1.17.0)
|
||||
@ -42,13 +42,13 @@ GEM
|
||||
multipart-post (2.1.1)
|
||||
nap (1.1.0)
|
||||
no_proxy_fix (0.1.2)
|
||||
nokogiri (1.10.9)
|
||||
nokogiri (1.10.10)
|
||||
mini_portile2 (~> 2.4.0)
|
||||
octokit (4.18.0)
|
||||
faraday (>= 0.9)
|
||||
sawyer (~> 0.8.0, >= 0.5.3)
|
||||
open4 (1.3.4)
|
||||
pmdtester (1.0.0)
|
||||
pmdtester (1.0.1)
|
||||
differ (~> 0.1)
|
||||
nokogiri (~> 1.8)
|
||||
rufus-scheduler (~> 3.5)
|
||||
@ -56,7 +56,7 @@ GEM
|
||||
public_suffix (4.0.5)
|
||||
raabro (1.3.1)
|
||||
rchardet (1.8.0)
|
||||
rouge (3.19.0)
|
||||
rouge (3.20.0)
|
||||
rufus-scheduler (3.6.0)
|
||||
fugit (~> 1.1, >= 1.1.6)
|
||||
safe_yaml (1.0.5)
|
||||
|
@ -180,6 +180,9 @@ class JavadocTag < Liquid::Tag
|
||||
if member_suffix && Regexp.new('(\w+|<init>)(' + ARGUMENTS_REGEX.source + ")?") =~ member_suffix
|
||||
|
||||
suffix = $1 # method or field name
|
||||
if suffix == '<init>'
|
||||
suffix = '<init>'
|
||||
end
|
||||
|
||||
if opts.show_args? && $2 && !$2.empty? # is method
|
||||
|
||||
|
@ -30,6 +30,10 @@ This is a {{ site.pmd.release_type }} release.
|
||||
* core
|
||||
* [#710](https://github.com/pmd/pmd/issues/710): \[core] Review used dependencies
|
||||
* [#2594](https://github.com/pmd/pmd/issues/2594): \[core] Update exec-maven-plugin and align it in all project
|
||||
* [#2615](https://github.com/pmd/pmd/issues/2615): \[core] PMD/CPD produces invalid XML (insufficient escaping/wrong encoding)
|
||||
* java-bestpractices
|
||||
* [#2543](https://github.com/pmd/pmd/issues/2543): \[java] UseCollectionIsEmpty can not detect the case this.foo.size()
|
||||
* [#2569](https://github.com/pmd/pmd/issues/2569): \[java] LiteralsFirstInComparisons: False negative for methods returning Strings
|
||||
* java-design
|
||||
* [#2174](https://github.com/pmd/pmd/issues/2174): \[java] LawOfDemeter: False positive with 'this' pointer
|
||||
* [#2189](https://github.com/pmd/pmd/issues/2189): \[java] LawOfDemeter: False positive when casting to derived class
|
||||
@ -50,8 +54,8 @@ This is a {{ site.pmd.release_type }} release.
|
||||
|
||||
* {% jdoc core::lang.rule.RuleChainVisitor %} and all implementations in language modules
|
||||
* {% jdoc core::lang.rule.AbstractRuleChainVisitor %}
|
||||
* {% jdoc core::lang.Language#getRuleChainVisitorClass() %}
|
||||
* {% jdoc core::lang.BaseLanguageModule#<init>(java.lang.String,java.lang.String,java.lang.String,java.lang.Class,java.lang.String...) %}
|
||||
* {% jdoc !!core::lang.Language#getRuleChainVisitorClass() %}
|
||||
* {% jdoc !!core::lang.BaseLanguageModule#<init>(java.lang.String,java.lang.String,java.lang.String,java.lang.Class,java.lang.String...) %}
|
||||
|
||||
|
||||
### External Contributions
|
||||
@ -63,6 +67,8 @@ This is a {{ site.pmd.release_type }} release.
|
||||
* [#2640](https://github.com/pmd/pmd/pull/2640): \[java] NullPointerException in rule ProperCloneImplementation - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2641](https://github.com/pmd/pmd/pull/2641): \[java] AvoidThrowingNullPointerException marks all NullPointerException… - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2643](https://github.com/pmd/pmd/pull/2643): \[java] AvoidCallingFinalize detects some false positives (2578) - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2651](https://github.com/pmd/pmd/pull/2651): \[java] False negative: LiteralsFirstInComparisons for methods... (2569) - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
* [#2652](https://github.com/pmd/pmd/pull/2652): \[java] UseCollectionIsEmpty can not detect the case this.foo.size() - [Mykhailo Palahuta](https://github.com/Drofff)
|
||||
|
||||
{% endtocmaker %}
|
||||
|
||||
|
@ -137,6 +137,10 @@
|
||||
<groupId>org.apache.commons</groupId>
|
||||
<artifactId>commons-lang3</artifactId>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.apache.commons</groupId>
|
||||
<artifactId>commons-text</artifactId>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.ow2.asm</groupId>
|
||||
<artifactId>asm</artifactId>
|
||||
|
@ -217,8 +217,7 @@ public class PMD {
|
||||
try (TimedOperation to = TimeTracker.startOperation(TimedOperationCategory.REPORTING)) {
|
||||
renderer = configuration.createRenderer();
|
||||
renderers = Collections.singletonList(renderer);
|
||||
|
||||
renderer.setWriter(IOUtil.createWriter(configuration.getReportFile()));
|
||||
renderer.setReportFile(configuration.getReportFile());
|
||||
renderer.start();
|
||||
}
|
||||
|
||||
|
@ -19,7 +19,6 @@ import net.sourceforge.pmd.lang.LanguageVersionDiscoverer;
|
||||
import net.sourceforge.pmd.renderers.Renderer;
|
||||
import net.sourceforge.pmd.renderers.RendererFactory;
|
||||
import net.sourceforge.pmd.util.ClasspathClassLoader;
|
||||
import net.sourceforge.pmd.util.IOUtil;
|
||||
|
||||
/**
|
||||
* This class contains the details for the runtime configuration of PMD. There
|
||||
@ -406,7 +405,7 @@ public class PMDConfiguration extends AbstractConfiguration {
|
||||
renderer.setUseShortNames(Arrays.asList(inputPaths.split(",")));
|
||||
}
|
||||
if (withReportWriter) {
|
||||
renderer.setWriter(IOUtil.createWriter(reportFile));
|
||||
renderer.setReportFile(reportFile);
|
||||
}
|
||||
return renderer;
|
||||
}
|
||||
|
@ -18,10 +18,12 @@ import javax.xml.transform.TransformerFactory;
|
||||
import javax.xml.transform.dom.DOMSource;
|
||||
import javax.xml.transform.stream.StreamResult;
|
||||
|
||||
import org.apache.commons.text.StringEscapeUtils;
|
||||
import org.w3c.dom.Document;
|
||||
import org.w3c.dom.Element;
|
||||
|
||||
import net.sourceforge.pmd.cpd.renderer.CPDRenderer;
|
||||
import net.sourceforge.pmd.util.StringUtil;
|
||||
|
||||
/**
|
||||
* @author Philippe T'Seyen - original implementation
|
||||
@ -76,6 +78,7 @@ public final class XMLRenderer implements Renderer, CPDRenderer {
|
||||
try {
|
||||
TransformerFactory tf = TransformerFactory.newInstance();
|
||||
Transformer transformer = tf.newTransformer();
|
||||
transformer.setOutputProperty(OutputKeys.VERSION, "1.0");
|
||||
transformer.setOutputProperty(OutputKeys.METHOD, "xml");
|
||||
transformer.setOutputProperty(OutputKeys.ENCODING, encoding);
|
||||
transformer.setOutputProperty(OutputKeys.INDENT, "yes");
|
||||
@ -119,7 +122,9 @@ public final class XMLRenderer implements Renderer, CPDRenderer {
|
||||
mark = iterator.next();
|
||||
final Element file = doc.createElement("file");
|
||||
file.setAttribute("line", String.valueOf(mark.getBeginLine()));
|
||||
file.setAttribute("path", mark.getFilename());
|
||||
// only remove invalid characters, escaping is done by the DOM impl.
|
||||
String filenameXml10 = StringUtil.removedInvalidXml10Characters(mark.getFilename());
|
||||
file.setAttribute("path", filenameXml10);
|
||||
file.setAttribute("endline", String.valueOf(mark.getEndLine()));
|
||||
final int beginCol = mark.getBeginColumn();
|
||||
final int endCol = mark.getEndColumn();
|
||||
@ -140,7 +145,7 @@ public final class XMLRenderer implements Renderer, CPDRenderer {
|
||||
// the code snippet has normalized line endings
|
||||
String platformSpecific = codeSnippet.replace("\n", System.lineSeparator());
|
||||
Element codefragment = doc.createElement("codefragment");
|
||||
codefragment.appendChild(doc.createCDATASection(platformSpecific));
|
||||
codefragment.appendChild(doc.createCDATASection(StringEscapeUtils.escapeXml10(platformSpecific)));
|
||||
duplication.appendChild(codefragment);
|
||||
}
|
||||
return duplication;
|
||||
|
@ -12,9 +12,11 @@ import java.util.List;
|
||||
import org.apache.commons.io.IOUtils;
|
||||
|
||||
import net.sourceforge.pmd.PMDConfiguration;
|
||||
import net.sourceforge.pmd.annotation.Experimental;
|
||||
import net.sourceforge.pmd.cli.PMDParameters;
|
||||
import net.sourceforge.pmd.internal.util.ShortFilenameUtil;
|
||||
import net.sourceforge.pmd.properties.AbstractPropertySource;
|
||||
import net.sourceforge.pmd.util.IOUtil;
|
||||
|
||||
/**
|
||||
* Abstract base class for {@link Renderer} implementations.
|
||||
@ -109,4 +111,16 @@ public abstract class AbstractRenderer extends AbstractPropertySource implements
|
||||
IOUtils.closeQuietly(writer);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritDoc}
|
||||
*
|
||||
* <p>This default implementation always uses the system default charset for the writer.
|
||||
* Overwrite in specific renderers to support other charsets.
|
||||
*/
|
||||
@Experimental
|
||||
@Override
|
||||
public void setReportFile(String reportFilename) {
|
||||
this.setWriter(IOUtil.createWriter(reportFilename));
|
||||
}
|
||||
}
|
||||
|
@ -9,6 +9,7 @@ import java.io.Writer;
|
||||
import java.util.List;
|
||||
|
||||
import net.sourceforge.pmd.Report;
|
||||
import net.sourceforge.pmd.annotation.Experimental;
|
||||
import net.sourceforge.pmd.properties.PropertyDescriptor;
|
||||
import net.sourceforge.pmd.properties.PropertySource;
|
||||
import net.sourceforge.pmd.util.datasource.DataSource;
|
||||
@ -163,4 +164,17 @@ public interface Renderer extends PropertySource {
|
||||
void end() throws IOException;
|
||||
|
||||
void flush() throws IOException;
|
||||
|
||||
/**
|
||||
* Sets the filename where the report should be written to. If no filename is provided,
|
||||
* the renderer should write to stdout.
|
||||
*
|
||||
* <p>Implementations must initialize the writer of the renderer.
|
||||
*
|
||||
* <p>See {@link AbstractRenderer#setReportFile(String)} for the default impl.
|
||||
*
|
||||
* @param reportFilename the filename (optional).
|
||||
*/
|
||||
@Experimental
|
||||
void setReportFile(String reportFilename);
|
||||
}
|
||||
|
File diff suppressed because it is too large
Load Diff
@ -45,6 +45,7 @@ public class XSLTRenderer extends XMLRenderer {
|
||||
private Transformer transformer;
|
||||
private String xsltFilename = "/pmd-nicerhtml.xsl";
|
||||
private Writer outputWriter;
|
||||
private StringWriter stringWriter;
|
||||
|
||||
public XSLTRenderer() {
|
||||
super();
|
||||
@ -71,8 +72,8 @@ public class XSLTRenderer extends XMLRenderer {
|
||||
// We keep the inital writer to put the final html output
|
||||
this.outputWriter = getWriter();
|
||||
// We use a new one to store the XML...
|
||||
Writer w = new StringWriter();
|
||||
setWriter(w);
|
||||
this.stringWriter = new StringWriter();
|
||||
setWriter(stringWriter);
|
||||
// If don't find the xsl no need to bother doing the all report,
|
||||
// so we check this here...
|
||||
InputStream xslt = null;
|
||||
@ -100,16 +101,14 @@ public class XSLTRenderer extends XMLRenderer {
|
||||
* The stylesheet provided as an InputStream
|
||||
*/
|
||||
private void prepareTransformer(InputStream xslt) {
|
||||
if (xslt != null) {
|
||||
try {
|
||||
// Get a TransformerFactory object
|
||||
TransformerFactory factory = TransformerFactory.newInstance();
|
||||
StreamSource src = new StreamSource(xslt);
|
||||
// Get an XSL Transformer object
|
||||
this.transformer = factory.newTransformer(src);
|
||||
} catch (TransformerConfigurationException e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
try {
|
||||
// Get a TransformerFactory object
|
||||
TransformerFactory factory = TransformerFactory.newInstance();
|
||||
StreamSource src = new StreamSource(xslt);
|
||||
// Get an XSL Transformer object
|
||||
this.transformer = factory.newTransformer(src);
|
||||
} catch (TransformerConfigurationException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
}
|
||||
|
||||
@ -118,25 +117,17 @@ public class XSLTRenderer extends XMLRenderer {
|
||||
// First we finish the XML report
|
||||
super.end();
|
||||
// Now we transform it using XSLT
|
||||
if (writer instanceof StringWriter) {
|
||||
StringWriter w = (StringWriter) writer;
|
||||
Document doc = this.getDocument(w.toString());
|
||||
this.transform(doc);
|
||||
} else {
|
||||
// Should not happen !
|
||||
throw new RuntimeException("Wrong writer");
|
||||
}
|
||||
|
||||
Document doc = this.getDocument(stringWriter.toString());
|
||||
this.transform(doc);
|
||||
}
|
||||
|
||||
private void transform(Document doc) {
|
||||
DOMSource source = new DOMSource(doc);
|
||||
this.setWriter(new StringWriter());
|
||||
StreamResult result = new StreamResult(this.outputWriter);
|
||||
try {
|
||||
transformer.transform(source, result);
|
||||
} catch (TransformerException e) {
|
||||
e.printStackTrace();
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
}
|
||||
|
||||
@ -145,8 +136,7 @@ public class XSLTRenderer extends XMLRenderer {
|
||||
DocumentBuilder parser = DocumentBuilderFactory.newInstance().newDocumentBuilder();
|
||||
return parser.parse(new InputSource(new StringReader(xml)));
|
||||
} catch (ParserConfigurationException | SAXException | IOException e) {
|
||||
e.printStackTrace();
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
@ -12,7 +12,11 @@ import java.io.OutputStreamWriter;
|
||||
import java.io.Reader;
|
||||
import java.io.Writer;
|
||||
import java.nio.charset.Charset;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.nio.charset.UnsupportedCharsetException;
|
||||
import java.nio.file.Files;
|
||||
import java.security.AccessController;
|
||||
import java.security.PrivilegedAction;
|
||||
|
||||
import org.apache.commons.io.IOUtils;
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
@ -35,10 +39,39 @@ public final class IOUtil {
|
||||
return new OutputStreamWriter(System.out);
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the current default charset.
|
||||
*
|
||||
* <p>In contrast to {@link Charset#defaultCharset()}, the result is not cached,
|
||||
* so that in unit tests, the charset can be changed.
|
||||
* @return
|
||||
*/
|
||||
private static Charset getDefaultCharset() {
|
||||
String csn = AccessController.doPrivileged(new PrivilegedAction<String>() {
|
||||
@Override
|
||||
public String run() {
|
||||
return System.getProperty("file.encoding");
|
||||
}
|
||||
});
|
||||
try {
|
||||
return Charset.forName(csn);
|
||||
} catch (UnsupportedCharsetException e) {
|
||||
return StandardCharsets.UTF_8;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a writer that writes to the given file or to stdout.
|
||||
*
|
||||
* <p>Warning: This writer always uses the system default charset.
|
||||
*
|
||||
* @param reportFile the file name (optional)
|
||||
* @return the writer, never <code>null</code>
|
||||
*/
|
||||
public static Writer createWriter(String reportFile) {
|
||||
try {
|
||||
return StringUtils.isBlank(reportFile) ? createWriter()
|
||||
: Files.newBufferedWriter(new File(reportFile).toPath(), Charset.defaultCharset());
|
||||
: Files.newBufferedWriter(new File(reportFile).toPath(), getDefaultCharset());
|
||||
} catch (IOException e) {
|
||||
throw new IllegalArgumentException(e);
|
||||
}
|
||||
|
@ -9,8 +9,11 @@ import java.util.Arrays;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
import org.apache.commons.text.StringEscapeUtils;
|
||||
|
||||
import net.sourceforge.pmd.annotation.InternalApi;
|
||||
|
||||
@ -27,6 +30,12 @@ public final class StringUtil {
|
||||
|
||||
private static final String[] EMPTY_STRINGS = new String[0];
|
||||
|
||||
private static final Pattern XML_10_INVALID_CHARS = Pattern.compile(
|
||||
"\\x00|\\x01|\\x02|\\x03|\\x04|\\x05|\\x06|\\x07|\\x08|"
|
||||
+ "\\x0b|\\x0c|\\x0e|\\x0f|"
|
||||
+ "\\x10|\\x11|\\x12|\\x13|\\x14|\\x15|\\x16|\\x17|\\x18|"
|
||||
+ "\\x19|\\x1a|\\x1b|\\x1c|\\x1d|\\x1e|\\x1f");
|
||||
|
||||
private StringUtil() {
|
||||
}
|
||||
|
||||
@ -211,8 +220,9 @@ public final class StringUtil {
|
||||
* @param src
|
||||
* @param supportUTF8 override the default setting, whether special characters should be replaced with entities (
|
||||
* <code>false</code>) or should be included as is ( <code>true</code>).
|
||||
*
|
||||
* @deprecated for removal. Use {@link StringEscapeUtils#escapeXml10(String)} instead.
|
||||
*/
|
||||
@Deprecated
|
||||
public static void appendXmlEscaped(StringBuilder buf, String src, boolean supportUTF8) {
|
||||
char c;
|
||||
int i = 0;
|
||||
@ -245,6 +255,20 @@ public final class StringUtil {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove characters, that are not allowed in XML 1.0 documents.
|
||||
*
|
||||
* <p>Allowed characters are:
|
||||
* <blockquote>
|
||||
* Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]
|
||||
* // any Unicode character, excluding the surrogate blocks, FFFE, and FFFF.
|
||||
* </blockquote>
|
||||
* (see <a href="https://www.w3.org/TR/xml/#charsets">Extensible Markup Language (XML) 1.0 (Fifth Edition)</a>).
|
||||
*/
|
||||
public static String removedInvalidXml10Characters(String text) {
|
||||
Matcher matcher = XML_10_INVALID_CHARS.matcher(text);
|
||||
return matcher.replaceAll("");
|
||||
}
|
||||
|
||||
/**
|
||||
* Replace some whitespace characters so they are visually apparent.
|
||||
|
@ -1,4 +1,4 @@
|
||||
/**
|
||||
/*
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
|
||||
@ -9,11 +9,16 @@ import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileInputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.StringWriter;
|
||||
import java.nio.charset.Charset;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.Iterator;
|
||||
import java.util.Map;
|
||||
|
||||
import org.apache.commons.io.IOUtils;
|
||||
import org.junit.Test;
|
||||
|
||||
import net.sourceforge.pmd.lang.ast.DummyNode;
|
||||
@ -196,4 +201,21 @@ public class ReportTest implements ThreadSafeReportListener {
|
||||
renderer.end();
|
||||
return writer.toString();
|
||||
}
|
||||
|
||||
public static String renderTempFile(Renderer renderer, Report report, Charset expectedCharset) throws IOException {
|
||||
Path tempFile = Files.createTempFile("pmd-report-test", null);
|
||||
String absolutePath = tempFile.toAbsolutePath().toString();
|
||||
|
||||
renderer.setReportFile(absolutePath);
|
||||
renderer.start();
|
||||
renderer.renderFileReport(report);
|
||||
renderer.end();
|
||||
renderer.flush();
|
||||
|
||||
try (FileInputStream input = new FileInputStream(absolutePath)) {
|
||||
return IOUtils.toString(input, expectedCharset);
|
||||
} finally {
|
||||
Files.delete(tempFile);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -5,6 +5,7 @@
|
||||
package net.sourceforge.pmd.cpd;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
@ -30,6 +31,9 @@ import net.sourceforge.pmd.cpd.renderer.CPDRenderer;
|
||||
public class XMLRendererTest {
|
||||
|
||||
private static final String ENCODING = (String) System.getProperties().get("file.encoding");
|
||||
private static final String FORM_FEED = "\u000C"; // this character is invalid in XML 1.0 documents
|
||||
private static final String FORM_FEED_ENTITY = ""; // this is also not allowed in XML 1.0 documents
|
||||
|
||||
|
||||
@Test
|
||||
public void testWithNoDuplication() throws IOException {
|
||||
@ -185,8 +189,8 @@ public class XMLRendererTest {
|
||||
CPDRenderer renderer = new XMLRenderer();
|
||||
List<Match> list = new ArrayList<>();
|
||||
final String espaceChar = "<";
|
||||
Mark mark1 = createMark("public", "/var/F" + '<' + "oo.java", 48, 6, "code fragment");
|
||||
Mark mark2 = createMark("void", "/var/F<oo.java", 73, 6, "code fragment");
|
||||
Mark mark1 = createMark("public", "/var/A<oo.java" + FORM_FEED, 48, 6, "code fragment");
|
||||
Mark mark2 = createMark("void", "/var/B<oo.java", 73, 6, "code fragment");
|
||||
Match match1 = new Match(75, mark1, mark2);
|
||||
list.add(match1);
|
||||
|
||||
@ -194,6 +198,25 @@ public class XMLRendererTest {
|
||||
renderer.render(list.iterator(), sw);
|
||||
String report = sw.toString();
|
||||
assertTrue(report.contains(espaceChar));
|
||||
assertFalse(report.contains(FORM_FEED));
|
||||
assertFalse(report.contains(FORM_FEED_ENTITY));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testRendererXMLEscaping() throws IOException {
|
||||
String codefragment = "code fragment" + FORM_FEED + "\nline2\nline3";
|
||||
CPDRenderer renderer = new XMLRenderer();
|
||||
List<Match> list = new ArrayList<>();
|
||||
Mark mark1 = createMark("public", "file1", 1, 2, codefragment);
|
||||
Mark mark2 = createMark("public", "file2", 5, 2, codefragment);
|
||||
Match match1 = new Match(75, mark1, mark2);
|
||||
list.add(match1);
|
||||
|
||||
StringWriter sw = new StringWriter();
|
||||
renderer.render(list.iterator(), sw);
|
||||
String report = sw.toString();
|
||||
assertFalse(report.contains(FORM_FEED));
|
||||
assertFalse(report.contains(FORM_FEED_ENTITY));
|
||||
}
|
||||
|
||||
private Mark createMark(String image, String tokenSrcID, int beginLine, int lineCount, String code) {
|
||||
@ -214,8 +237,4 @@ public class XMLRendererTest {
|
||||
result.setSourceCode(new SourceCode(new SourceCode.StringCodeLoader(code)));
|
||||
return result;
|
||||
}
|
||||
|
||||
public static junit.framework.Test suite() {
|
||||
return new junit.framework.JUnit4TestAdapter(XMLRendererTest.class);
|
||||
}
|
||||
}
|
||||
|
@ -6,10 +6,14 @@ package net.sourceforge.pmd.renderers;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.StringReader;
|
||||
import java.nio.charset.Charset;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import javax.xml.parsers.DocumentBuilderFactory;
|
||||
|
||||
import org.junit.Assert;
|
||||
import org.junit.Rule;
|
||||
import org.junit.Test;
|
||||
import org.junit.contrib.java.lang.system.RestoreSystemProperties;
|
||||
import org.w3c.dom.Document;
|
||||
import org.w3c.dom.NodeList;
|
||||
import org.xml.sax.InputSource;
|
||||
@ -28,6 +32,8 @@ import net.sourceforge.pmd.lang.ast.Node;
|
||||
import net.sourceforge.pmd.lang.rule.ParametricRuleViolation;
|
||||
|
||||
public class XMLRendererTest extends AbstractRendererTest {
|
||||
@Rule // Restores system properties after test
|
||||
public final RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
|
||||
|
||||
@Override
|
||||
public Renderer getRenderer() {
|
||||
@ -90,12 +96,13 @@ public class XMLRendererTest extends AbstractRendererTest {
|
||||
return new ParametricRuleViolation<Node>(new FooRule(), ctx, node, description);
|
||||
}
|
||||
|
||||
private void verifyXmlEscaping(Renderer renderer, String shouldContain) throws Exception {
|
||||
private void verifyXmlEscaping(Renderer renderer, String shouldContain, Charset charset) throws Exception {
|
||||
renderer.setProperty(XMLRenderer.ENCODING, charset.name());
|
||||
Report report = new Report();
|
||||
String surrogatePair = "\ud801\udc1c";
|
||||
String msg = "The String literal \"Tokenizer " + surrogatePair + "\" appears...";
|
||||
String msg = "The String 'literal' \"TokénizĀr " + surrogatePair + "\" appears...";
|
||||
report.addRuleViolation(createRuleViolation(msg));
|
||||
String actual = ReportTest.render(renderer, report);
|
||||
String actual = ReportTest.renderTempFile(renderer, report, charset);
|
||||
Assert.assertTrue(actual.contains(shouldContain));
|
||||
Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder()
|
||||
.parse(new InputSource(new StringReader(actual)));
|
||||
@ -107,22 +114,44 @@ public class XMLRendererTest extends AbstractRendererTest {
|
||||
@Test
|
||||
public void testXMLEscapingWithUTF8() throws Exception {
|
||||
Renderer renderer = getRenderer();
|
||||
renderer.setProperty(XMLRenderer.ENCODING, "UTF-8");
|
||||
verifyXmlEscaping(renderer, "\ud801\udc1c");
|
||||
verifyXmlEscaping(renderer, "\ud801\udc1c", StandardCharsets.UTF_8);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testXMLEscapingWithoutUTF8() throws Exception {
|
||||
Renderer renderer = getRenderer();
|
||||
renderer.setProperty(XMLRenderer.ENCODING, "ISO-8859-1");
|
||||
verifyXmlEscaping(renderer, "𐐜");
|
||||
verifyXmlEscaping(renderer, "𐐜", StandardCharsets.ISO_8859_1);
|
||||
}
|
||||
|
||||
public String getHeader() {
|
||||
return "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" + PMD.EOL
|
||||
+ "<pmd xmlns=\"http://pmd.sourceforge.net/report/2.0.0\"" + PMD.EOL
|
||||
+ " xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"" + PMD.EOL
|
||||
+ " xsi:schemaLocation=\"http://pmd.sourceforge.net/report/2.0.0 http://pmd.sourceforge.net/report_2_0_0.xsd\"" + PMD.EOL
|
||||
+ " version=\"" + PMDVersion.VERSION + "\" timestamp=\"2014-10-06T19:30:51.262\">" + PMD.EOL;
|
||||
+ "<pmd xmlns=\"http://pmd.sourceforge.net/report/2.0.0\""
|
||||
+ " xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\""
|
||||
+ " xsi:schemaLocation=\"http://pmd.sourceforge.net/report/2.0.0 http://pmd.sourceforge.net/report_2_0_0.xsd\""
|
||||
+ " version=\"" + PMDVersion.VERSION + "\" timestamp=\"2014-10-06T19:30:51.262\">" + PMD.EOL;
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCorrectCharset() throws Exception {
|
||||
System.setProperty("file.encoding", StandardCharsets.ISO_8859_1.name());
|
||||
|
||||
Renderer renderer = getRenderer();
|
||||
|
||||
Report report = new Report();
|
||||
String formFeed = "\u000C";
|
||||
// é = U+00E9 : can be represented in ISO-8859-1 as is
|
||||
// Ā = U+0100 : cannot be represented in ISO-8859-1 -> would be a unmappable character, needs to be escaped
|
||||
String specialChars = "éĀ";
|
||||
String originalChars = formFeed + specialChars; // u000C should be removed, é should be encoded correctly as UTF-8
|
||||
String msg = "The String literal \"" + originalChars + "\" appears...";
|
||||
report.addRuleViolation(createRuleViolation(msg));
|
||||
String actual = ReportTest.renderTempFile(renderer, report, StandardCharsets.UTF_8);
|
||||
Assert.assertTrue(actual.contains(specialChars));
|
||||
Assert.assertFalse(actual.contains(formFeed));
|
||||
Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder()
|
||||
.parse(new InputSource(new StringReader(actual)));
|
||||
NodeList violations = doc.getElementsByTagName("violation");
|
||||
Assert.assertEquals(1, violations.getLength());
|
||||
Assert.assertEquals(msg.replaceAll(formFeed, ""), violations.item(0).getTextContent().trim());
|
||||
}
|
||||
}
|
||||
|
@ -36,14 +36,6 @@ public class StringUtilTest {
|
||||
assertEquals("replaceString didn't work with a char", "f", StringUtil.replaceString("foo", 'o', null));
|
||||
}
|
||||
|
||||
/**
|
||||
* Usually you would set the system property
|
||||
* "net.sourceforge.pmd.supportUTF8" to either "no" or "yes", to switch UTF8
|
||||
* support.
|
||||
*
|
||||
* e.g.
|
||||
* <code>System.setProperty("net.sourceforge.pmd.supportUTF8","yes");</code>
|
||||
*/
|
||||
@Test
|
||||
public void testUTF8NotSupported() {
|
||||
StringBuilder sb = new StringBuilder();
|
||||
@ -68,8 +60,4 @@ public class StringUtilTest {
|
||||
StringUtil.appendXmlEscaped(sb, test, true);
|
||||
assertEquals("é", sb.toString());
|
||||
}
|
||||
|
||||
public static junit.framework.Test suite() {
|
||||
return new junit.framework.JUnit4TestAdapter(StringUtilTest.class);
|
||||
}
|
||||
}
|
||||
|
@ -94,7 +94,6 @@
|
||||
<dependency>
|
||||
<groupId>org.apache.commons</groupId>
|
||||
<artifactId>commons-text</artifactId>
|
||||
<version>1.6</version>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.yaml</groupId>
|
||||
|
@ -30,6 +30,11 @@ public final class GenerateRuleDocsCmd {
|
||||
}
|
||||
|
||||
public static void main(String[] args) throws RuleSetNotFoundException {
|
||||
if (args.length != 1) {
|
||||
System.err.println("One argument is required: The base directory of the module pmd-doc.");
|
||||
System.exit(1);
|
||||
}
|
||||
|
||||
long start = System.currentTimeMillis();
|
||||
Path output = FileSystems.getDefault().getPath(args[0]).resolve("..").toAbsolutePath().normalize();
|
||||
System.out.println("Generating docs into " + output);
|
||||
|
@ -75,6 +75,12 @@ public class RuleDocGenerator {
|
||||
private final Path root;
|
||||
private final FileWriter writer;
|
||||
|
||||
/** Caches rule class name to java source file mapping. */
|
||||
private final Map<String, String> allRules = new HashMap<>();
|
||||
/** Caches ruleset to ruleset xml file mapping. */
|
||||
private final Map<String, String> allRulesets = new HashMap<>();
|
||||
|
||||
|
||||
public RuleDocGenerator(FileWriter writer, Path root) {
|
||||
this.writer = Objects.requireNonNull(writer, "A file writer must be provided");
|
||||
this.root = Objects.requireNonNull(root, "Root directory must be provided");
|
||||
@ -91,6 +97,7 @@ public class RuleDocGenerator {
|
||||
try {
|
||||
sortedRulesets = sortRulesets(registeredRulesets);
|
||||
sortedAdditionalRulesets = sortRulesets(resolveAdditionalRulesets(additionalRulesets));
|
||||
determineRuleClassSourceFiles(sortedRulesets);
|
||||
generateLanguageIndex(sortedRulesets, sortedAdditionalRulesets);
|
||||
generateRuleSetIndex(sortedRulesets);
|
||||
|
||||
@ -345,7 +352,7 @@ public class RuleDocGenerator {
|
||||
String permalink = RULESET_INDEX_PERMALINK_PATTERN
|
||||
.replace("${language.tersename}", languageTersename)
|
||||
.replace("${ruleset.name}", rulesetFilename);
|
||||
String ruleSetSourceFilepath = "../" + getRuleSetSourceFilepath(ruleset);
|
||||
String ruleSetSourceFilepath = "../" + allRulesets.get(ruleset.getFileName());
|
||||
|
||||
List<String> lines = new LinkedList<>();
|
||||
lines.add("---");
|
||||
@ -415,7 +422,7 @@ public class RuleDocGenerator {
|
||||
} else {
|
||||
lines.add("**This rule is defined by the following Java class:** "
|
||||
+ "[" + rule.getRuleClass() + "]("
|
||||
+ GITHUB_SOURCE_LINK + getRuleClassSourceFilepath(rule.getRuleClass())
|
||||
+ GITHUB_SOURCE_LINK + allRules.get(rule.getRuleClass())
|
||||
+ ")");
|
||||
}
|
||||
lines.add("");
|
||||
@ -607,63 +614,74 @@ public class RuleDocGenerator {
|
||||
}
|
||||
|
||||
/**
|
||||
* Searches for the source file of the given ruleset. This provides the information
|
||||
* for the "editme" link.
|
||||
* Walks through the root directory once to get all rule source file path names and ruleset names.
|
||||
* This provides the information for the "editme" links.
|
||||
*
|
||||
* @param ruleset the ruleset to search for.
|
||||
* @return
|
||||
* @throws IOException
|
||||
* @param sortedRulesets all the rulesets and rules
|
||||
*/
|
||||
private String getRuleSetSourceFilepath(RuleSet ruleset) throws IOException {
|
||||
final String rulesetFilename = FilenameUtils.normalize(StringUtils.chomp(ruleset.getFileName()));
|
||||
final List<Path> foundPathResult = new LinkedList<>();
|
||||
|
||||
Files.walkFileTree(root, new SimpleFileVisitor<Path>() {
|
||||
@Override
|
||||
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
|
||||
String path = file.toString();
|
||||
if (path.contains("src") && path.endsWith(rulesetFilename)) {
|
||||
foundPathResult.add(file);
|
||||
return FileVisitResult.TERMINATE;
|
||||
private void determineRuleClassSourceFiles(Map<Language, List<RuleSet>> sortedRulesets) {
|
||||
// first collect all the classes, we need to resolve and the rulesets
|
||||
// this also provides a default fallback path, which is used in unit tests.
|
||||
// if the actual file is found during walkFileTree, then the default fallback path
|
||||
// is replaced by a correct path.
|
||||
for (List<RuleSet> rulesets : sortedRulesets.values()) {
|
||||
for (RuleSet ruleset : rulesets) {
|
||||
// Note: the path is normalized to unix path separators, so that the editme link
|
||||
// uses forward slashes
|
||||
String rulesetFilename = FilenameUtils.normalize(StringUtils.chomp(ruleset.getFileName()), true);
|
||||
allRulesets.put(ruleset.getFileName(), rulesetFilename);
|
||||
for (Rule rule : ruleset.getRules()) {
|
||||
String ruleClass = rule.getRuleClass();
|
||||
String relativeSourceFilename = ruleClass.replaceAll("\\.", Matcher.quoteReplacement(File.separator))
|
||||
+ ".java";
|
||||
// Note: the path is normalized to unix path separators, so that the editme link
|
||||
// uses forward slashes
|
||||
allRules.put(ruleClass, FilenameUtils.normalize(relativeSourceFilename, true));
|
||||
}
|
||||
return super.visitFile(file, attrs);
|
||||
}
|
||||
});
|
||||
|
||||
if (!foundPathResult.isEmpty()) {
|
||||
Path foundPath = foundPathResult.get(0);
|
||||
foundPath = root.relativize(foundPath);
|
||||
// Note: the path is normalized to unix path separators, so that the editme link
|
||||
// uses forward slashes
|
||||
return FilenameUtils.normalize(foundPath.toString(), true);
|
||||
}
|
||||
|
||||
return StringUtils.chomp(ruleset.getFileName());
|
||||
}
|
||||
// then go and search the actual files
|
||||
try {
|
||||
Files.walkFileTree(root, new SimpleFileVisitor<Path>() {
|
||||
@Override
|
||||
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
|
||||
String path = file.toString();
|
||||
|
||||
private String getRuleClassSourceFilepath(String ruleClass) throws IOException {
|
||||
final String relativeSourceFilename = ruleClass.replaceAll("\\.", Matcher.quoteReplacement(File.separator))
|
||||
+ ".java";
|
||||
final List<Path> foundPathResult = new LinkedList<>();
|
||||
if (path.contains("src")) {
|
||||
String foundRuleClass = null;
|
||||
for (Map.Entry<String, String> entry : allRules.entrySet()) {
|
||||
if (path.endsWith(entry.getValue())) {
|
||||
foundRuleClass = entry.getKey();
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (foundRuleClass != null) {
|
||||
Path foundPath = root.relativize(file);
|
||||
// Note: the path is normalized to unix path separators, so that the editme link
|
||||
// uses forward slashes
|
||||
allRules.put(foundRuleClass, FilenameUtils.normalize(foundPath.toString(), true));
|
||||
}
|
||||
|
||||
Files.walkFileTree(root, new SimpleFileVisitor<Path>() {
|
||||
@Override
|
||||
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
|
||||
String path = file.toString();
|
||||
if (path.contains("src") && path.endsWith(relativeSourceFilename)) {
|
||||
foundPathResult.add(file);
|
||||
return FileVisitResult.TERMINATE;
|
||||
String foundRuleset = null;
|
||||
for (Map.Entry<String, String> entry : allRulesets.entrySet()) {
|
||||
if (path.endsWith(entry.getValue())) {
|
||||
foundRuleset = entry.getKey();
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (foundRuleset != null) {
|
||||
Path foundPath = root.relativize(file);
|
||||
// Note: the path is normalized to unix path separators, so that the editme link
|
||||
// uses forward slashes
|
||||
allRulesets.put(foundRuleset, FilenameUtils.normalize(foundPath.toString(), true));
|
||||
}
|
||||
}
|
||||
return FileVisitResult.CONTINUE;
|
||||
}
|
||||
return super.visitFile(file, attrs);
|
||||
}
|
||||
});
|
||||
|
||||
if (!foundPathResult.isEmpty()) {
|
||||
Path foundPath = foundPathResult.get(0);
|
||||
foundPath = root.relativize(foundPath);
|
||||
return FilenameUtils.normalize(foundPath.toString(), true);
|
||||
});
|
||||
} catch (IOException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
|
||||
return FilenameUtils.normalize(relativeSourceFilename, true);
|
||||
}
|
||||
}
|
||||
|
@ -4,7 +4,8 @@
|
||||
|
||||
package net.sourceforge.pmd.lang.java.rule.bestpractices;
|
||||
|
||||
import net.sourceforge.pmd.lang.ast.Node;
|
||||
import java.util.List;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTArguments;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression;
|
||||
@ -29,63 +30,83 @@ public class LiteralsFirstInComparisonsRule extends AbstractJavaRule {
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTPrimaryExpression node, Object data) {
|
||||
ASTPrimaryPrefix primaryPrefix = node.getFirstChildOfType(ASTPrimaryPrefix.class);
|
||||
ASTPrimarySuffix primarySuffix = node.getFirstChildOfType(ASTPrimarySuffix.class);
|
||||
if (primaryPrefix != null && primarySuffix != null) {
|
||||
ASTName name = primaryPrefix.getFirstChildOfType(ASTName.class);
|
||||
if (name == null || isIrrelevantImage(name.getImage())) {
|
||||
return data;
|
||||
}
|
||||
if (!isSingleStringLiteralArgument(primarySuffix)) {
|
||||
return data;
|
||||
}
|
||||
if (isWithinNullComparison(node)) {
|
||||
return data;
|
||||
}
|
||||
addViolation(data, node);
|
||||
public Object visit(ASTPrimaryExpression expression, Object data) {
|
||||
if (violatesLiteralsFirstInComparisonsRule(expression)) {
|
||||
addViolation(data, expression);
|
||||
}
|
||||
return node;
|
||||
return data;
|
||||
}
|
||||
|
||||
private boolean isIrrelevantImage(String image) {
|
||||
private boolean violatesLiteralsFirstInComparisonsRule(ASTPrimaryExpression expression) {
|
||||
return !hasStringLiteralFirst(expression) && isNullableComparisonWithStringLiteral(expression);
|
||||
}
|
||||
|
||||
private boolean hasStringLiteralFirst(ASTPrimaryExpression expression) {
|
||||
ASTPrimaryPrefix primaryPrefix = expression.getFirstChildOfType(ASTPrimaryPrefix.class);
|
||||
ASTLiteral firstLiteral = primaryPrefix.getFirstDescendantOfType(ASTLiteral.class);
|
||||
return firstLiteral != null && firstLiteral.isStringLiteral();
|
||||
}
|
||||
|
||||
private boolean isNullableComparisonWithStringLiteral(ASTPrimaryExpression expression) {
|
||||
String opName = getOperationName(expression);
|
||||
ASTPrimarySuffix argsSuffix = getSuffixOfArguments(expression);
|
||||
return opName != null && argsSuffix != null && isStringLiteralComparison(opName, argsSuffix)
|
||||
&& isNotWithinNullComparison(expression);
|
||||
}
|
||||
|
||||
private String getOperationName(ASTPrimaryExpression primaryExpression) {
|
||||
return isMethodsChain(primaryExpression) ? getOperationNameBySuffix(primaryExpression)
|
||||
: getOperationNameByPrefix(primaryExpression);
|
||||
}
|
||||
|
||||
private boolean isMethodsChain(ASTPrimaryExpression primaryExpression) {
|
||||
return primaryExpression.getNumChildren() > 2;
|
||||
}
|
||||
|
||||
private String getOperationNameBySuffix(ASTPrimaryExpression primaryExpression) {
|
||||
ASTPrimarySuffix opAsSuffix = getPrimarySuffixAtIndexFromEnd(primaryExpression, 1);
|
||||
if (opAsSuffix != null) {
|
||||
String opName = opAsSuffix.getImage(); // name of pattern "operation"
|
||||
return "." + opName;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private String getOperationNameByPrefix(ASTPrimaryExpression primaryExpression) {
|
||||
ASTPrimaryPrefix opAsPrefix = primaryExpression.getFirstChildOfType(ASTPrimaryPrefix.class);
|
||||
if (opAsPrefix != null) {
|
||||
ASTName opName = opAsPrefix.getFirstChildOfType(ASTName.class); // name of pattern "*.operation"
|
||||
return opName != null ? opName.getImage() : null;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private ASTPrimarySuffix getSuffixOfArguments(ASTPrimaryExpression primaryExpression) {
|
||||
return getPrimarySuffixAtIndexFromEnd(primaryExpression, 0);
|
||||
}
|
||||
|
||||
private ASTPrimarySuffix getPrimarySuffixAtIndexFromEnd(ASTPrimaryExpression primaryExpression, int indexFromEnd) {
|
||||
List<ASTPrimarySuffix> primarySuffixes = primaryExpression.findChildrenOfType(ASTPrimarySuffix.class);
|
||||
if (!primarySuffixes.isEmpty()) {
|
||||
int suffixIndex = primarySuffixes.size() - 1 - indexFromEnd;
|
||||
return primarySuffixes.get(suffixIndex);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private boolean isStringLiteralComparison(String opName, ASTPrimarySuffix argsSuffix) {
|
||||
return isComparisonOperation(opName) && isSingleStringLiteralArgument(argsSuffix);
|
||||
}
|
||||
|
||||
private boolean isComparisonOperation(String op) {
|
||||
for (String comparisonOp : COMPARISON_OPS) {
|
||||
if (image.endsWith(comparisonOp)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
private boolean isWithinNullComparison(ASTPrimaryExpression node) {
|
||||
for (ASTExpression parentExpr : node.getParentsOfType(ASTExpression.class)) {
|
||||
if (isComparisonWithNull(parentExpr, "==", ASTConditionalOrExpression.class)
|
||||
|| isComparisonWithNull(parentExpr, "!=", ASTConditionalAndExpression.class)) {
|
||||
if (op.endsWith(comparisonOp)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/*
|
||||
* Expression/ConditionalAndExpression//EqualityExpression(@Image='!=']//NullLiteral
|
||||
* Expression/ConditionalOrExpression//EqualityExpression(@Image='==']//NullLiteral
|
||||
*/
|
||||
private boolean isComparisonWithNull(ASTExpression parentExpr, String equalOperator, Class<? extends JavaNode> condition) {
|
||||
Node condExpr = null;
|
||||
ASTEqualityExpression eqExpr = null;
|
||||
if (parentExpr != null) {
|
||||
condExpr = parentExpr.getFirstChildOfType(condition);
|
||||
}
|
||||
if (condExpr != null) {
|
||||
eqExpr = condExpr.getFirstDescendantOfType(ASTEqualityExpression.class);
|
||||
}
|
||||
if (eqExpr != null) {
|
||||
return eqExpr.hasImageEqualTo(equalOperator) && eqExpr.hasDescendantOfType(ASTNullLiteral.class);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/*
|
||||
* This corresponds to the following XPath expression:
|
||||
* (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal[@StringLiteral= true()])
|
||||
@ -93,35 +114,77 @@ public class LiteralsFirstInComparisonsRule extends AbstractJavaRule {
|
||||
* ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 )
|
||||
*/
|
||||
private boolean isSingleStringLiteralArgument(ASTPrimarySuffix primarySuffix) {
|
||||
if (!primarySuffix.isArguments() || primarySuffix.getArgumentCount() != 1) {
|
||||
return isSingleArgumentSuffix(primarySuffix) && isStringLiteralFirstArgumentOfSuffix(primarySuffix);
|
||||
}
|
||||
|
||||
private boolean isSingleArgumentSuffix(ASTPrimarySuffix primarySuffix) {
|
||||
return primarySuffix.getArgumentCount() == 1;
|
||||
}
|
||||
|
||||
private boolean isStringLiteralFirstArgumentOfSuffix(ASTPrimarySuffix primarySuffix) {
|
||||
try {
|
||||
JavaNode firstArg = getFirstArgument(primarySuffix);
|
||||
return isStringLiteral(firstArg);
|
||||
} catch (NullPointerException e) {
|
||||
return false;
|
||||
}
|
||||
Node node = primarySuffix;
|
||||
node = node.getFirstChildOfType(ASTArguments.class);
|
||||
if (node != null) {
|
||||
node = node.getFirstChildOfType(ASTArgumentList.class);
|
||||
if (node.getNumChildren() != 1) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
if (node != null) {
|
||||
node = node.getFirstChildOfType(ASTExpression.class);
|
||||
}
|
||||
if (node != null) {
|
||||
node = node.getFirstChildOfType(ASTPrimaryExpression.class);
|
||||
}
|
||||
if (node != null) {
|
||||
node = node.getFirstChildOfType(ASTPrimaryPrefix.class);
|
||||
}
|
||||
if (node != null) {
|
||||
node = node.getFirstChildOfType(ASTLiteral.class);
|
||||
}
|
||||
if (node != null) {
|
||||
}
|
||||
|
||||
private JavaNode getFirstArgument(ASTPrimarySuffix primarySuffix) {
|
||||
ASTArguments arguments = primarySuffix.getFirstChildOfType(ASTArguments.class);
|
||||
ASTArgumentList argumentList = arguments.getFirstChildOfType(ASTArgumentList.class);
|
||||
ASTExpression expression = argumentList.getFirstChildOfType(ASTExpression.class);
|
||||
ASTPrimaryExpression primaryExpression = expression.getFirstChildOfType(ASTPrimaryExpression.class);
|
||||
ASTPrimaryPrefix primaryPrefix = primaryExpression.getFirstChildOfType(ASTPrimaryPrefix.class);
|
||||
return primaryPrefix.getFirstChildOfType(ASTLiteral.class);
|
||||
}
|
||||
|
||||
private boolean isStringLiteral(JavaNode node) {
|
||||
if (node instanceof ASTLiteral) {
|
||||
ASTLiteral literal = (ASTLiteral) node;
|
||||
if (literal.isStringLiteral()) {
|
||||
return literal.isStringLiteral();
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private boolean isNotWithinNullComparison(ASTPrimaryExpression node) {
|
||||
return !isWithinNullComparison(node);
|
||||
}
|
||||
|
||||
/*
|
||||
* Expression/ConditionalAndExpression//EqualityExpression(@Image='!=']//NullLiteral
|
||||
* Expression/ConditionalOrExpression//EqualityExpression(@Image='==']//NullLiteral
|
||||
*/
|
||||
private boolean isWithinNullComparison(ASTPrimaryExpression node) {
|
||||
for (ASTExpression parentExpr : node.getParentsOfType(ASTExpression.class)) {
|
||||
if (isNullComparison(parentExpr)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private boolean isNullComparison(ASTExpression expression) {
|
||||
return isAndNotNullComparison(expression) || isOrNullComparison(expression);
|
||||
}
|
||||
|
||||
private boolean isAndNotNullComparison(ASTExpression expression) {
|
||||
ASTConditionalAndExpression andExpression = expression
|
||||
.getFirstChildOfType(ASTConditionalAndExpression.class);
|
||||
return andExpression != null && hasEqualityExpressionWithNullLiteral(andExpression, "!=");
|
||||
}
|
||||
|
||||
private boolean isOrNullComparison(ASTExpression expression) {
|
||||
ASTConditionalOrExpression orExpression = expression
|
||||
.getFirstChildOfType(ASTConditionalOrExpression.class);
|
||||
return orExpression != null && hasEqualityExpressionWithNullLiteral(orExpression, "==");
|
||||
}
|
||||
|
||||
private boolean hasEqualityExpressionWithNullLiteral(JavaNode node, String equalityOp) {
|
||||
ASTEqualityExpression equalityExpression = node.getFirstDescendantOfType(ASTEqualityExpression.class);
|
||||
if (equalityExpression != null && equalityExpression.hasImageEqualTo(equalityOp)) {
|
||||
return equalityExpression.hasDescendantOfType(ASTNullLiteral.class);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
@ -4,18 +4,23 @@
|
||||
|
||||
package net.sourceforge.pmd.lang.java.rule.bestpractices;
|
||||
|
||||
import java.util.Arrays;
|
||||
import static java.util.Arrays.asList;
|
||||
import static java.util.Collections.singletonList;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import net.sourceforge.pmd.lang.ast.Node;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTName;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTResultType;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
|
||||
import net.sourceforge.pmd.lang.java.rule.AbstractInefficientZeroCheck;
|
||||
import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
|
||||
import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
|
||||
@ -45,43 +50,78 @@ public class UseCollectionIsEmptyRule extends AbstractInefficientZeroCheck {
|
||||
*/
|
||||
@Override
|
||||
public boolean isTargetMethod(JavaNameOccurrence occ) {
|
||||
if (occ.getNameForWhichThisIsAQualifier() != null) {
|
||||
if (occ.getLocation().getImage().endsWith(".size")) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
return occ.getNameForWhichThisIsAQualifier() != null
|
||||
&& occ.getLocation().getImage().endsWith(".size");
|
||||
}
|
||||
|
||||
@Override
|
||||
public Map<String, List<String>> getComparisonTargets() {
|
||||
List<String> zeroAndOne = asList("0", "1");
|
||||
List<String> zero = singletonList("0");
|
||||
Map<String, List<String>> rules = new HashMap<>();
|
||||
rules.put("<", Arrays.asList("0", "1"));
|
||||
rules.put(">", Arrays.asList("0"));
|
||||
rules.put("==", Arrays.asList("0"));
|
||||
rules.put("!=", Arrays.asList("0"));
|
||||
rules.put(">=", Arrays.asList("0", "1"));
|
||||
rules.put("<=", Arrays.asList("0"));
|
||||
rules.put("<", zeroAndOne);
|
||||
rules.put(">", zero);
|
||||
rules.put("==", zero);
|
||||
rules.put("!=", zero);
|
||||
rules.put(">=", zeroAndOne);
|
||||
rules.put("<=", zero);
|
||||
return rules;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTPrimarySuffix node, Object data) {
|
||||
if (node.getImage() != null && node.getImage().endsWith("size")) {
|
||||
|
||||
ASTClassOrInterfaceType type = getTypeOfPrimaryPrefix(node);
|
||||
if (type == null) {
|
||||
type = getTypeOfMethodCall(node);
|
||||
}
|
||||
|
||||
if (type != null && CollectionUtil.isCollectionType(type.getType(), true)) {
|
||||
Node expr = node.getParent().getParent();
|
||||
checkNodeAndReport(data, node, expr);
|
||||
}
|
||||
if (isSizeMethodCall(node) && isCalledOnCollection(node)) {
|
||||
Node expr = node.getParent().getParent();
|
||||
checkNodeAndReport(data, node, expr);
|
||||
}
|
||||
return data;
|
||||
}
|
||||
|
||||
private boolean isSizeMethodCall(ASTPrimarySuffix primarySuffix) {
|
||||
String calledMethodName = primarySuffix.getImage();
|
||||
return calledMethodName != null && calledMethodName.endsWith("size");
|
||||
}
|
||||
|
||||
private boolean isCalledOnCollection(ASTPrimarySuffix primarySuffix) {
|
||||
ASTClassOrInterfaceType calledOnType = getTypeOfVariable(primarySuffix);
|
||||
if (calledOnType == null) {
|
||||
calledOnType = getTypeOfMethodCall(primarySuffix);
|
||||
}
|
||||
return calledOnType != null
|
||||
&& CollectionUtil.isCollectionType(calledOnType.getType(), true);
|
||||
}
|
||||
|
||||
private ASTClassOrInterfaceType getTypeOfVariable(ASTPrimarySuffix primarySuffix) {
|
||||
ASTPrimaryExpression primaryExpression = primarySuffix.getFirstParentOfType(ASTPrimaryExpression.class);
|
||||
ASTPrimaryPrefix varPrefix = primaryExpression.getFirstChildOfType(ASTPrimaryPrefix.class);
|
||||
if (prefixWithNoModifiers(varPrefix)) {
|
||||
return varPrefix.getFirstDescendantOfType(ASTClassOrInterfaceType.class);
|
||||
}
|
||||
String varName = getVariableNameBySuffix(primaryExpression);
|
||||
return varName != null ? getTypeOfVariableByName(varName, primaryExpression) : null;
|
||||
}
|
||||
|
||||
private boolean prefixWithNoModifiers(ASTPrimaryPrefix primaryPrefix) {
|
||||
return !primaryPrefix.usesSuperModifier() && !primaryPrefix.usesThisModifier();
|
||||
}
|
||||
|
||||
private String getVariableNameBySuffix(ASTPrimaryExpression primaryExpression) {
|
||||
ASTPrimarySuffix varSuffix = primaryExpression
|
||||
.getFirstChildOfType(ASTPrimarySuffix.class);
|
||||
return varSuffix.getImage();
|
||||
}
|
||||
|
||||
private ASTClassOrInterfaceType getTypeOfVariableByName(String varName, ASTPrimaryExpression expr) {
|
||||
ASTClassOrInterfaceBody classBody = expr.getFirstParentOfType(ASTClassOrInterfaceBody.class);
|
||||
List<ASTVariableDeclarator> varDeclarators = classBody.findDescendantsOfType(ASTVariableDeclarator.class);
|
||||
for (ASTVariableDeclarator varDeclarator : varDeclarators) {
|
||||
if (varDeclarator.getName().equals(varName)) {
|
||||
return varDeclarator.getFirstDescendantOfType(ASTClassOrInterfaceType.class);
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private ASTClassOrInterfaceType getTypeOfMethodCall(ASTPrimarySuffix node) {
|
||||
ASTClassOrInterfaceType type = null;
|
||||
ASTName methodName = node.getParent().getFirstChildOfType(ASTPrimaryPrefix.class)
|
||||
@ -100,9 +140,4 @@ public class UseCollectionIsEmptyRule extends AbstractInefficientZeroCheck {
|
||||
}
|
||||
return type;
|
||||
}
|
||||
|
||||
private ASTClassOrInterfaceType getTypeOfPrimaryPrefix(ASTPrimarySuffix node) {
|
||||
return node.getParent().getFirstChildOfType(ASTPrimaryPrefix.class)
|
||||
.getFirstDescendantOfType(ASTClassOrInterfaceType.class);
|
||||
}
|
||||
}
|
||||
|
@ -7,10 +7,9 @@ package net.sourceforge.pmd.ant;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import java.lang.reflect.Field;
|
||||
import java.io.UnsupportedEncodingException;
|
||||
import java.nio.charset.Charset;
|
||||
import java.util.Locale;
|
||||
import java.util.Objects;
|
||||
|
||||
import org.apache.commons.io.FileUtils;
|
||||
import org.junit.Rule;
|
||||
@ -111,17 +110,8 @@ public class PMDTaskTest extends AbstractAntTestHelper {
|
||||
}
|
||||
};
|
||||
|
||||
// See http://stackoverflow.com/questions/361975/setting-the-default-java-character-encoding and http://stackoverflow.com/a/14987992/1169968
|
||||
private static void setDefaultCharset(String charsetName) {
|
||||
try {
|
||||
System.setProperty("file.encoding", charsetName);
|
||||
Field charset = Charset.class.getDeclaredField("defaultCharset");
|
||||
charset.setAccessible(true);
|
||||
charset.set(null, null);
|
||||
Objects.requireNonNull(Charset.defaultCharset());
|
||||
} catch (Exception e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
System.setProperty("file.encoding", charsetName);
|
||||
}
|
||||
|
||||
@Rule
|
||||
@ -149,14 +139,28 @@ public class PMDTaskTest extends AbstractAntTestHelper {
|
||||
assertTrue(report.contains("unusedVariableWithÜmlaut"));
|
||||
}
|
||||
|
||||
private static String convert(String report) {
|
||||
// reinterpret output as cp1252 - ant BuildFileRule can only unicode
|
||||
StringBuilder sb = new StringBuilder(report.length());
|
||||
for (int i = 0; i < report.length(); i++) {
|
||||
char c = report.charAt(i);
|
||||
if (c > 0x7f) {
|
||||
sb.append((char) (c & 0xff));
|
||||
} else {
|
||||
sb.append(c);
|
||||
}
|
||||
}
|
||||
return sb.toString();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFormatterEncodingWithXMLConsole() {
|
||||
public void testFormatterEncodingWithXMLConsole() throws UnsupportedEncodingException {
|
||||
setDefaultCharset("cp1252");
|
||||
|
||||
executeTarget("testFormatterEncodingWithXMLConsole");
|
||||
String report = buildRule.getOutput();
|
||||
String report = convert(buildRule.getOutput());
|
||||
assertTrue(report.startsWith("<?xml version=\"1.0\" encoding=\"windows-1252\"?>"));
|
||||
assertTrue(report.contains("unusedVariableWithÜmlaut"));
|
||||
assertTrue(report.contains("unusedVariableWithÜmlaut"));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -77,7 +77,7 @@ public class CPDCommandLineInterfaceTest extends BaseCPDCLITest {
|
||||
|
||||
String out = getOutput();
|
||||
Assert.assertTrue(out.startsWith("<?xml version=\"1.0\" encoding=\"UTF-8\"?>"));
|
||||
Assert.assertTrue(Pattern.compile("System\\.out\\.println\\([ij] \\+ \"ä\"\\);").matcher(out).find());
|
||||
Assert.assertTrue(Pattern.compile("System\\.out\\.println\\([ij] \\+ "ä"\\);").matcher(out).find());
|
||||
Assert.assertEquals(4, Integer.parseInt(System.getProperty(CPDCommandLineInterface.STATUS_CODE_PROPERTY)));
|
||||
}
|
||||
|
||||
|
@ -272,6 +272,49 @@ public class Foo {
|
||||
boolean bar(String x) {
|
||||
return contentEquals("2");
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>bad, testing false negative at the end of a chain</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public boolean bar() {
|
||||
File f;
|
||||
return f.getFileType().equals("testStr");
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>ok, should be ignored in case both operands are string literals</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
boolean isFoo;
|
||||
public void bar() {
|
||||
this.isFoo = "Hello".equals("World");
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>Equals on method result with String argument</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-linenumbers>6</expected-linenumbers>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
private String getStr(String a) {
|
||||
return "a" + a;
|
||||
}
|
||||
public void bar() {
|
||||
if (getStr("b").equals("ab")) { } // nok
|
||||
if ("ab".equals(getStr("b"))) { } // ok
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
@ -7,6 +7,7 @@
|
||||
<test-code>
|
||||
<description>fail, == 0</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-linenumbers>3</expected-linenumbers>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public static boolean bar(List lst) {
|
||||
@ -37,6 +38,7 @@ public class Foo {
|
||||
<test-code>
|
||||
<description>fail, != 0</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-linenumbers>3</expected-linenumbers>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public static boolean bar(List lst) {
|
||||
@ -67,6 +69,7 @@ public class Foo {
|
||||
<test-code>
|
||||
<description>fail, != 0</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-linenumbers>3</expected-linenumbers>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public static boolean bar(List lst, boolean b) {
|
||||
@ -97,6 +100,7 @@ public class Foo {
|
||||
<test-code>
|
||||
<description>fail, 0 ==</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-linenumbers>3</expected-linenumbers>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public static boolean bar(List lst) {
|
||||
@ -112,6 +116,7 @@ public class Foo {
|
||||
<test-code>
|
||||
<description>fail, > 0</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-linenumbers>3</expected-linenumbers>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public static boolean bar(List lst) {
|
||||
@ -159,6 +164,7 @@ public class Foo {
|
||||
<test-code>
|
||||
<description>#1214 UseCollectionIsEmpty misses some usage</description>
|
||||
<expected-problems>5</expected-problems>
|
||||
<expected-linenumbers>25,28,31,34,37</expected-linenumbers>
|
||||
<code><![CDATA[
|
||||
import java.util.ArrayList;
|
||||
|
||||
@ -311,6 +317,34 @@ public class PmdBugBait {
|
||||
throw new IllegalArgumentException();
|
||||
}
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>#2543 false negative on this.collection.size</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-linenumbers>10</expected-linenumbers>
|
||||
<code><![CDATA[
|
||||
import java.util.List;
|
||||
import java.util.ArrayList;
|
||||
|
||||
public class Foo {
|
||||
|
||||
private List<String> list = new ArrayList<>();
|
||||
private Bar notACollection = new Bar();
|
||||
|
||||
public void bar() {
|
||||
if (this.list.size() == 0) {
|
||||
throw new RuntimeException("Empty list");
|
||||
}
|
||||
if (notACollection.size() == 0) { }
|
||||
if (this.notACollection.size() == 0) { }
|
||||
}
|
||||
|
||||
public int size() {
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
5
pom.xml
5
pom.xml
@ -696,6 +696,11 @@
|
||||
<artifactId>commons-lang3</artifactId>
|
||||
<version>3.8.1</version>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.apache.commons</groupId>
|
||||
<artifactId>commons-text</artifactId>
|
||||
<version>1.6</version>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.slf4j</groupId>
|
||||
<artifactId>slf4j-api</artifactId>
|
||||
|
Reference in New Issue
Block a user