From 2dc8b06e0f6299965d2cadca77b7abb9d57a7ad9 Mon Sep 17 00:00:00 2001 From: "Georg.Dotzler" Date: Wed, 9 Aug 2017 15:37:29 +0200 Subject: [PATCH 1/5] Fix bug in leafIterator. The old version always returns the last element instead of null at the end. --- .../com/github/gumtreediff/tree/TreeUtils.java | 3 +++ .../com/github/gumtreediff/test/TestTreeUtils.java | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/core/src/main/java/com/github/gumtreediff/tree/TreeUtils.java b/core/src/main/java/com/github/gumtreediff/tree/TreeUtils.java index e92bb1bbf..82cfadaa8 100644 --- a/core/src/main/java/com/github/gumtreediff/tree/TreeUtils.java +++ b/core/src/main/java/com/github/gumtreediff/tree/TreeUtils.java @@ -306,6 +306,9 @@ public ITree next() { if (current.isLeaf()) break; } + if (!it.hasNext()) { + current = null; + } return val; } diff --git a/core/src/test/java/com/github/gumtreediff/test/TestTreeUtils.java b/core/src/test/java/com/github/gumtreediff/test/TestTreeUtils.java index 30739dcd7..72615fe7b 100644 --- a/core/src/test/java/com/github/gumtreediff/test/TestTreeUtils.java +++ b/core/src/test/java/com/github/gumtreediff/test/TestTreeUtils.java @@ -170,4 +170,18 @@ public void testBfs3() { Iterator it = TreeUtils.breadthFirstIterator(big); compareListIterator(lst, it); } + + @Test + public void testLeafIterator() { + ITree src = TreeLoader.getDummySrc(); + Iterator srcLeaves = TreeUtils.leafIterator(TreeUtils.postOrderIterator(src)); + ITree leaf = null; + leaf = srcLeaves.next(); + leaf = srcLeaves.next(); + leaf = srcLeaves.next(); + leaf = srcLeaves.next(); + leaf = srcLeaves.next(); + assertNull(leaf); + } + } From c53bd9f260d015c0b98a68076cad20d8321f6e6b Mon Sep 17 00:00:00 2001 From: "Georg.Dotzler" Date: Wed, 9 Aug 2017 16:08:05 +0200 Subject: [PATCH 2/5] Fix bug in the ChangeDistiller leaves matcher. The old version did not compare all possible leaf pairs. --- .../cd/ChangeDistillerLeavesMatcher.java | 2 +- .../gumtreediff/test/TestCdMatcher.java | 48 +++++++++++++++++++ .../github/gumtreediff/test/TreeLoader.java | 4 ++ core/src/test/resources/cd_v0.xml | 23 +++++++++ core/src/test/resources/cd_v1.xml | 23 +++++++++ 5 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/com/github/gumtreediff/test/TestCdMatcher.java create mode 100644 core/src/test/resources/cd_v0.xml create mode 100644 core/src/test/resources/cd_v1.xml diff --git a/core/src/main/java/com/github/gumtreediff/matchers/heuristic/cd/ChangeDistillerLeavesMatcher.java b/core/src/main/java/com/github/gumtreediff/matchers/heuristic/cd/ChangeDistillerLeavesMatcher.java index c1ac64bce..e4f37b3ac 100644 --- a/core/src/main/java/com/github/gumtreediff/matchers/heuristic/cd/ChangeDistillerLeavesMatcher.java +++ b/core/src/main/java/com/github/gumtreediff/matchers/heuristic/cd/ChangeDistillerLeavesMatcher.java @@ -45,8 +45,8 @@ public void match() { for (Iterator srcLeaves = TreeUtils.leafIterator( TreeUtils.postOrderIterator(src)); srcLeaves.hasNext();) { + ITree srcLeaf = srcLeaves.next(); for (ITree dstLeaf: dstLeaves) { - ITree srcLeaf = srcLeaves.next(); if (isMappingAllowed(srcLeaf, dstLeaf)) { double sim = StringMetrics.qGramsDistance().compare(srcLeaf.getLabel(), dstLeaf.getLabel()); if (sim > LABEL_SIM_THRESHOLD) leafMappings.add(new Mapping(srcLeaf, dstLeaf)); diff --git a/core/src/test/java/com/github/gumtreediff/test/TestCdMatcher.java b/core/src/test/java/com/github/gumtreediff/test/TestCdMatcher.java new file mode 100644 index 000000000..26612a854 --- /dev/null +++ b/core/src/test/java/com/github/gumtreediff/test/TestCdMatcher.java @@ -0,0 +1,48 @@ +/* + * This file is part of GumTree. + * + * GumTree is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * GumTree is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with GumTree. If not, see . + * + * Copyright 2011-2015 Jean-Rémy Falleri + * Copyright 2011-2015 Floréal Morandat + */ + +package com.github.gumtreediff.test; + +import com.github.gumtreediff.matchers.MappingStore; +import com.github.gumtreediff.matchers.Matcher; +import com.github.gumtreediff.matchers.heuristic.cd.ChangeDistillerLeavesMatcher; +import com.github.gumtreediff.tree.ITree; +import com.github.gumtreediff.utils.Pair; +import com.github.gumtreediff.tree.TreeContext; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class TestCdMatcher { + + @Test + public void testLeafMatcher() { + Pair trees = TreeLoader.getCdCustomPair(); + ITree src = trees.getFirst().getRoot(); + ITree dst = trees.getSecond().getRoot(); + Matcher matcher = new ChangeDistillerLeavesMatcher(src, dst, new MappingStore()); + matcher.match(); + assertEquals(2, matcher.getMappingSet().size()); + assertTrue(matcher.getMappings().has(src.getChild(0), dst.getChild(1))); + assertTrue(matcher.getMappings().has(src.getChild(1), dst.getChild(0))); + } + +} diff --git a/core/src/test/java/com/github/gumtreediff/test/TreeLoader.java b/core/src/test/java/com/github/gumtreediff/test/TreeLoader.java index 6d7382f50..0ec66b2dd 100644 --- a/core/src/test/java/com/github/gumtreediff/test/TreeLoader.java +++ b/core/src/test/java/com/github/gumtreediff/test/TreeLoader.java @@ -51,6 +51,10 @@ public static Pair getDummyPair() { return new Pair<>(load("/Dummy_v0.xml"), load("/Dummy_v1.xml")); } + public static Pair getCdCustomPair() { + return new Pair<>(load("/cd_v0.xml"), load("/cd_v1.xml")); + } + public static ITree getDummySrc() { return load("/Dummy_v0.xml").getRoot(); } diff --git a/core/src/test/resources/cd_v0.xml b/core/src/test/resources/cd_v0.xml new file mode 100644 index 000000000..2c974085c --- /dev/null +++ b/core/src/test/resources/cd_v0.xml @@ -0,0 +1,23 @@ + + + + + diff --git a/core/src/test/resources/cd_v1.xml b/core/src/test/resources/cd_v1.xml new file mode 100644 index 000000000..94b12f065 --- /dev/null +++ b/core/src/test/resources/cd_v1.xml @@ -0,0 +1,23 @@ + + + + + From 5b566423abca96325d4b19e3aa8cd1c66b5075be Mon Sep 17 00:00:00 2001 From: "Georg.Dotzler" Date: Wed, 9 Aug 2017 16:21:12 +0200 Subject: [PATCH 3/5] Fix bug in gumtree that causes non-deterministic results. As the similarity between two entries is not unique it is possible that across different runs of gumtree on the same changed file the order in ambiguousList changes. This can cause a change in the mapping. The added ID criteria enforces an absolute order. Of course other absolute orders would also be valid. --- .../matchers/heuristic/gt/AbstractMappingComparator.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/github/gumtreediff/matchers/heuristic/gt/AbstractMappingComparator.java b/core/src/main/java/com/github/gumtreediff/matchers/heuristic/gt/AbstractMappingComparator.java index 97d169f01..ebb4b5162 100644 --- a/core/src/main/java/com/github/gumtreediff/matchers/heuristic/gt/AbstractMappingComparator.java +++ b/core/src/main/java/com/github/gumtreediff/matchers/heuristic/gt/AbstractMappingComparator.java @@ -45,7 +45,13 @@ public AbstractMappingComparator(List ambiguousMappings, MappingStore m } public int compare(Mapping m1, Mapping m2) { - return Double.compare(similarities.get(m2), similarities.get(m1)); + if (similarities.get(m2).compareTo(similarities.get(m1)) != 0) { + return Double.compare(similarities.get(m2), similarities.get(m1)); + } + if (m1.first.getId() != m2.first.getId()) { + return Integer.compare(m1.first.getId(), m2.first.getId()); + } + return Integer.compare(m1.second.getId(), m2.second.getId()); } protected abstract double similarity(ITree src, ITree dst); From 0cbd854f701cf0d8ba1d0c6fc2661d5ce83f3730 Mon Sep 17 00:00:00 2001 From: "Georg.Dotzler" Date: Wed, 9 Aug 2017 19:26:30 +0200 Subject: [PATCH 4/5] Change default MIN_HEIGHT value. In the publication, leaves have height 1, in the source code they have height 0. MIN_HEIGHT=1 also leads to better results than MIN_HEIGHT=2. --- .../matchers/heuristic/gt/AbstractSubtreeMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/github/gumtreediff/matchers/heuristic/gt/AbstractSubtreeMatcher.java b/core/src/main/java/com/github/gumtreediff/matchers/heuristic/gt/AbstractSubtreeMatcher.java index f005fb5d3..7301dc507 100644 --- a/core/src/main/java/com/github/gumtreediff/matchers/heuristic/gt/AbstractSubtreeMatcher.java +++ b/core/src/main/java/com/github/gumtreediff/matchers/heuristic/gt/AbstractSubtreeMatcher.java @@ -31,7 +31,7 @@ public abstract class AbstractSubtreeMatcher extends Matcher { - public static int MIN_HEIGHT = Integer.parseInt(System.getProperty("gt.stm.mh", "2")); + public static int MIN_HEIGHT = Integer.parseInt(System.getProperty("gt.stm.mh", "1")); public AbstractSubtreeMatcher(ITree src, ITree dst, MappingStore store) { super(src, dst, store); From ef4df732fafb98f33383ddf9f84ccabb1bf96488 Mon Sep 17 00:00:00 2001 From: "Georg.Dotzler" Date: Wed, 9 Aug 2017 19:34:49 +0200 Subject: [PATCH 5/5] Fix bug in cache clause handling. --- .../com/github/gumtreediff/gen/jdt/cd/CdJdtVisitor.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gen.jdt/src/main/java/com/github/gumtreediff/gen/jdt/cd/CdJdtVisitor.java b/gen.jdt/src/main/java/com/github/gumtreediff/gen/jdt/cd/CdJdtVisitor.java index 0488ef82b..8563b9d80 100644 --- a/gen.jdt/src/main/java/com/github/gumtreediff/gen/jdt/cd/CdJdtVisitor.java +++ b/gen.jdt/src/main/java/com/github/gumtreediff/gen/jdt/cd/CdJdtVisitor.java @@ -397,7 +397,11 @@ public void endVisit(BreakStatement node) { @Override public boolean visit(CatchClause node) { - pushNode(node, ((SimpleType) node.getException().getType()).getName().getFullyQualifiedName()); + if (node.getException().getType() instanceof SimpleType) { + pushNode(node, ((SimpleType) node.getException().getType()).getName().getFullyQualifiedName()); + } else { + pushNode(node, ((UnionType) node.getException().getType()).toString()); + } // since exception type is used as value, visit children by hand node.getBody().accept(this); return false;