hiifong/pmd
1
0 Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Applied patch from Juan Jesus Garcia de Soria on CPD

Browse Source 
git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@7107 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Romain Pelisse committed 2010-07-24 14:34:44 +00:00
1 parent 6dd5cba823
commit da79fd8333
7 files changed
+75 -165
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

pmd/etc/changelog.txt
+1
View File
@@ -431,6 +431,7 @@ The RuleSet XML Schema namespace is now: http://pmd.sourceforge.net/ruleset/2.0.
The RuleSet XML Schema is located in the source at: etc/ruleset_2_0_0.xsd
The RuleSet DTD is located in the source at: etc/ruleset_2_0_0.dtd
Improved include/exclude pattern matching performance for ends-with type patterns.
Modify (and hopefully fixed) CPD algorithm thanks to a patch from Juan Jesús García de Soria.
New Java rules:
pmd/regress/test/net/sourceforge/pmd/cpd/XMLRendererTest.java
+10 -7
View File
@@ -27,13 +27,16 @@ import javax.xml.parsers.DocumentBuilderFactory;
*/
public class XMLRendererTest {
private final static String ENCODING = "utf-8"; // TODO: Switch this to System.getProperties().get("file.encoding"); ?
@Test
public void test_no_dupes() {
Renderer renderer = new XMLRenderer();
Renderer renderer = new XMLRenderer(ENCODING);
List<Match> list = new ArrayList<Match>();
String report = renderer.render(list.iterator());
try {
Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new ByteArrayInputStream(report.getBytes()));
Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new ByteArrayInputStream(report.getBytes(ENCODING)));
NodeList nodes = doc.getChildNodes();
Node n = nodes.item(0);
assertEquals("pmd-cpd", n.getNodeName());
@@ -46,7 +49,7 @@ public class XMLRendererTest {
@Test
public void test_one_dupe() {
Renderer renderer = new XMLRenderer();
Renderer renderer = new XMLRenderer("utf-8");
List<Match> list = new ArrayList<Match>();
Match match = new Match(75, new TokenEntry("public", "/var/Foo.java", 48), new TokenEntry("stuff", "/var/Foo.java", 73));
match.setLineCount(6);
@@ -54,7 +57,7 @@ public class XMLRendererTest {
list.add(match);
String report = renderer.render(list.iterator());
try {
Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new ByteArrayInputStream(report.getBytes()));
Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new ByteArrayInputStream(report.getBytes(ENCODING)));
NodeList dupes = doc.getElementsByTagName("duplication");
assertEquals(1, dupes.getLength());
Node file = dupes.item(0).getFirstChild();
@@ -81,7 +84,7 @@ public class XMLRendererTest {
@Test
public void testRender_MultipleMatch() {
Renderer renderer = new XMLRenderer();
Renderer renderer = new XMLRenderer(ENCODING);
List<Match> list = new ArrayList<Match>();
Match match1 = new Match(75, new TokenEntry("public", "/var/Foo.java", 48), new TokenEntry("void", "/var/Foo.java", 73));
match1.setLineCount(6);
@@ -93,7 +96,7 @@ public class XMLRendererTest {
list.add(match2);
String report = renderer.render(list.iterator());
try {
Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new ByteArrayInputStream(report.getBytes()));
Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new ByteArrayInputStream(report.getBytes(ENCODING)));
assertEquals(2, doc.getElementsByTagName("duplication").getLength());
assertEquals(4, doc.getElementsByTagName("file").getLength());
} catch (Exception e) {
@@ -104,7 +107,7 @@ public class XMLRendererTest {
@Test
public void testRendererEncodedPath() {
Renderer renderer = new XMLRenderer();
Renderer renderer = new XMLRenderer("utf-8");
List<Match> list = new ArrayList<Match>();
Match match1 = new Match(75, new TokenEntry("public", "/var/F" + XMLRenderer.BASIC_ESCAPE[2][0] + "oo.java", 48), new TokenEntry("void", "/var/F<oo.java", 73));
match1.setLineCount(6);
pmd/src/net/sourceforge/pmd/cpd/CPD.java
+2 -2
View File
@@ -6,18 +6,18 @@ package net.sourceforge.pmd.cpd;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import net.sourceforge.pmd.util.FileFinder;
public class CPD {
private Map<String, SourceCode> source = new HashMap<String, SourceCode>();
private Map<String, SourceCode> source = new TreeMap<String, SourceCode>();
private CPDListener listener = new CPDNullListener();
private Tokens tokens = new Tokens();
private int minimumTileSize;
pmd/src/net/sourceforge/pmd/cpd/Match.java
+19 -49
View File
@@ -14,10 +14,8 @@ public class Match implements Comparable<Match> {
private int tokenCount;
private int lineCount;
private Set<TokenEntry> markSet = new TreeSet<TokenEntry>();
private TokenEntry[] marks = new TokenEntry[2];
private String code;
private MatchCode mc;
private Set<TokenEntry> markSet = new TreeSet<TokenEntry>();
private String code;
private String label;
public static final Comparator<Match> MATCHES_COMPARATOR = new Comparator<Match>() {
@@ -49,44 +47,10 @@ public class Match implements Comparable<Match> {
return mb.getLineCount() - ma.getLineCount();
}
};
public static class MatchCode {
private int first;
private int second;
public MatchCode() {
}
public MatchCode(TokenEntry m1, TokenEntry m2) {
first = m1.getIndex();
second = m2.getIndex();
}
public int hashCode() {
return first + 37 * second;
}
public boolean equals(Object other) {
MatchCode mc = (MatchCode) other;
return mc.first == first && mc.second == second;
}
public void setFirst(int first) {
this.first = first;
}
public void setSecond(int second) {
this.second = second;
}
}
public Match(int tokenCount, TokenEntry first, TokenEntry second) {
markSet.add(first);
markSet.add(second);
marks[0] = first;
marks[1] = second;
this.tokenCount = tokenCount;
}
@@ -123,15 +87,15 @@ public class Match implements Comparable<Match> {
if (diff != 0) {
return diff;
}
return other.getFirstMark().getIndex() - getFirstMark().getIndex();
return getFirstMark().getIndex() - other.getFirstMark().getIndex();
}
public TokenEntry getFirstMark() {
return marks[0];
return getMark(0);
}
public TokenEntry getSecondMark() {
return marks[1];
return getMark(1);
}
public String toString() {
@@ -142,15 +106,8 @@ public class Match implements Comparable<Match> {
return markSet;
}
public MatchCode getMatchCode() {
if (mc == null) {
mc = new MatchCode(marks[0], marks[1]);
}
return mc;
}
public int getEndIndex() {
return marks[1].getIndex() + getTokenCount() - 1;
return getMark(0).getIndex() + getTokenCount() - 1;
}
public void setMarkSet(Set<TokenEntry> markSet) {
@@ -164,4 +121,17 @@ public class Match implements Comparable<Match> {
public String getLabel() {
return label;
}
public void addTokenEntry(TokenEntry entry){
markSet.add(entry);
}
private TokenEntry getMark(int index) {
TokenEntry result = null;
int i = 0;
for (Iterator<TokenEntry> it = markSet.iterator(); it.hasNext() && i < index + 1; ){
result = it.next();
}
return result;
}
}
pmd/src/net/sourceforge/pmd/cpd/MatchAlgorithm.java
+7 -8
View File
@@ -73,16 +73,15 @@ public class MatchAlgorithm {
cpdListener.phaseUpdate(CPDListener.GROUPING);
matches = matchCollector.getMatches();
matchCollector = null;
for (Match match: matches) {
for (Iterator<TokenEntry> occurrences = match.iterator(); occurrences.hasNext();) {
for (Match match : matches) {
Iterator<TokenEntry> occurrences = match.iterator();
if (occurrences.hasNext()) {
TokenEntry mark = occurrences.next();
match.setLineCount(tokens.getLineCount(mark, match));
if (!occurrences.hasNext()) {
int start = mark.getBeginLine();
int end = start + match.getLineCount() - 1;
SourceCode sourceCode = source.get(mark.getTokenSrcID());
match.setSourceCodeSlice(sourceCode.getSlice(start, end));
}
int start = mark.getBeginLine();
int end = start + match.getLineCount() - 1;
SourceCode sourceCode = source.get(mark.getTokenSrcID());
match.setSourceCodeSlice(sourceCode.getSlice(start, end));
}
}
cpdListener.phaseUpdate(CPDListener.DONE);
pmd/src/net/sourceforge/pmd/cpd/MatchCollector.java
+34 -98
View File
@@ -5,18 +5,14 @@ package net.sourceforge.pmd.cpd;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
public class MatchCollector {
private List<Match> matchList = new ArrayList<Match>();
private Map<Integer, Map<Integer, Match>> matchTree = new TreeMap<Integer, Map<Integer, Match>>();
private MatchAlgorithm ma;
private Map<Match.MatchCode, Match> startMap = new HashMap<Match.MatchCode, Match>();
private Map<String, List<Match>> fileMap = new HashMap<String, List<Match>>();
public MatchCollector(MatchAlgorithm ma) {
this.ma = ma;
@@ -45,105 +41,45 @@ public class MatchCollector {
if (diff + dupes >= 1) {
continue;
}
determineMatch(mark1, mark2, dupes);
reportMatch(mark1, mark2, dupes);
}
}
}
private void reportMatch(TokenEntry mark1, TokenEntry mark2, int dupes) {
Map<Integer, Match> matches = matchTree.get(dupes);
if (matches == null) {
matches = new TreeMap<Integer, Match>();
matchTree.put(dupes, matches);
addNewMatch(mark1, mark2, dupes, matches);
} else {
Match matchA = matchTree.get(dupes).get(mark1.getIndex());
Match matchB = matchTree.get(dupes).get(mark2.getIndex());
if (matchA == null && matchB == null) {
addNewMatch(mark1, mark2, dupes, matches);
} else if(matchA == null) {
matchB.addTokenEntry(mark1);
matches.put(mark1.getIndex(), matchB);
} else if(matchB == null) {
matchA.addTokenEntry(mark2);
matches.put(mark2.getIndex(), matchA);
}
}
}
private void addNewMatch(TokenEntry mark1, TokenEntry mark2, int dupes, Map<Integer, Match> matches){
Match match = new Match(dupes, mark1, mark2);
matches.put(mark1.getIndex(), match);
matches.put(mark2.getIndex(), match);
matchList.add(match);
}
@SuppressWarnings("PMD.CompareObjectsWithEquals")
public List<Match> getMatches() {
List<Match> matchList = new ArrayList<Match>(startMap.values());
Collections.sort(matchList);
Set<Match.MatchCode> matchSet = new HashSet<Match.MatchCode>();
Match.MatchCode matchCode = new Match.MatchCode();
for (int i = matchList.size(); i > 1; i--) {
Match match1 = matchList.get(i - 1);
TokenEntry mark1 = match1.getMarkSet().iterator().next();
matchSet.clear();
matchSet.add(match1.getMatchCode());
for (int j = i - 1; j > 0; j--) {
Match match2 = matchList.get(j - 1);
if (match1.getTokenCount() != match2.getTokenCount()) {
break;
}
TokenEntry mark2 = null;
for (Iterator<TokenEntry> iter = match2.getMarkSet().iterator(); iter.hasNext();) {
mark2 = iter.next();
if (mark2 != mark1) {
break;
}
}
int dupes = countDuplicateTokens(mark1, mark2);
if (dupes < match1.getTokenCount()) {
break;
}
matchSet.add(match2.getMatchCode());
match1.getMarkSet().addAll(match2.getMarkSet());
matchList.remove(i - 2);
i--;
}
if (matchSet.size() == 1) {
continue;
}
//prune the mark set
Set<TokenEntry> pruned = match1.getMarkSet();
boolean done = false;
ArrayList<TokenEntry> a1 = new ArrayList<TokenEntry>(match1.getMarkSet());
Collections.sort(a1);
for (int outer = 0; outer < a1.size() - 1 && !done; outer++) {
TokenEntry cmark1 = a1.get(outer);
for (int inner = outer + 1; inner < a1.size() && !done; inner++) {
TokenEntry cmark2 = a1.get(inner);
matchCode.setFirst(cmark1.getIndex());
matchCode.setSecond(cmark2.getIndex());
if (!matchSet.contains(matchCode)) {
if (pruned.size() > 2) {
pruned.remove(cmark2);
}
if (pruned.size() == 2) {
done = true;
}
}
}
}
}
return matchList;
}
/**
* A greedy algorithm for determining non-overlapping matches
*/
private void determineMatch(TokenEntry mark1, TokenEntry mark2, int dupes) {
Match match = new Match(dupes, mark1, mark2);
String fileKey = mark1.getTokenSrcID() + mark2.getTokenSrcID();
List<Match> pairMatches = fileMap.get(fileKey);
if (pairMatches == null) {
pairMatches = new ArrayList<Match>();
fileMap.put(fileKey, pairMatches);
}
boolean add = true;
for (int i = 0; i < pairMatches.size(); i++) {
Match other = pairMatches.get(i);
if (other.getFirstMark().getIndex() + other.getTokenCount() - mark1.getIndex()
> 0) {
boolean ordered = other.getSecondMark().getIndex() - mark2.getIndex() < 0;
if ((ordered && (other.getEndIndex() - mark2.getIndex() > 0))
|| (!ordered && (match.getEndIndex() - other.getSecondMark().getIndex()) > 0)) {
if (other.getTokenCount() >= match.getTokenCount()) {
add = false;
break;
} else {
pairMatches.remove(i);
startMap.remove(other.getMatchCode());
}
}
}
}
if (add) {
pairMatches.add(match);
startMap.put(match.getMatchCode(), match);
}
}
}
private boolean hasPreviousDupe(TokenEntry mark1, TokenEntry mark2) {
if (mark1.getIndex() == 0) {
pmd/xdocs/credits.xml
+2 -1
View File
@@ -58,7 +58,8 @@
<subsection name="Contributors">
<ul>
<li>Nicolas Dordet - Fixed an issue on CloseResource</li>
<li>Sergey Pariev - Fixed an ugly ArrayIndexOutOfBoundsException in CPD for Ruby</li>
<li>Juan Jesús García de Soria - rework CPD algorithm</li>
<li>Sergey Pariev - Fixed an ugly ArrayIndexOutOfBoundsException in CPD for Ruby</li>
<li>Chris Heister - Reported and noted proper fix for bug in IDEAJ renderer operations</li>
<li>Ralf Wagner - Reported bug in UselessOperationOnImmutable, reported and noted proper fix for broken XSLT</li>
<li>Caroline Rioux - Reported bug in ImmutableField</li>