From 207aa53f9fcebc61b4889ce0d67657b5445cebc2 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 13 Oct 2025 17:27:20 +0200 Subject: [PATCH 01/29] delete deprecated ViewTest These tests were more a playground back then when writing the SPLC paper and were never supposed to be actual tests. --- src/test/java/ViewTest.java | 245 ------------------------------------ 1 file changed, 245 deletions(-) delete mode 100644 src/test/java/ViewTest.java diff --git a/src/test/java/ViewTest.java b/src/test/java/ViewTest.java deleted file mode 100644 index 595ce6dd1..000000000 --- a/src/test/java/ViewTest.java +++ /dev/null @@ -1,245 +0,0 @@ -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; -import org.prop4j.*; -import org.tinylog.Logger; -import org.variantsync.diffdetective.analysis.logic.UniqueViewsAlgorithm; -import org.variantsync.diffdetective.diff.result.DiffParseException; -import org.variantsync.diffdetective.show.Show; -import org.variantsync.diffdetective.show.engine.GameEngine; -import org.variantsync.diffdetective.util.StringUtils; -import org.variantsync.diffdetective.variation.DiffLinesLabel; -import org.variantsync.diffdetective.variation.diff.Time; -import org.variantsync.diffdetective.variation.diff.VariationDiff; -import org.variantsync.diffdetective.variation.diff.bad.BadVDiff; -import org.variantsync.diffdetective.variation.diff.parse.VariationDiffParseOptions; -import org.variantsync.diffdetective.variation.diff.transform.CutNonEditedSubtrees; -import org.variantsync.diffdetective.variation.diff.view.DiffView; -import org.variantsync.diffdetective.variation.diff.view.ViewSource; -import org.variantsync.diffdetective.variation.tree.VariationTree; -import org.variantsync.diffdetective.variation.tree.view.TreeView; -import org.variantsync.diffdetective.variation.tree.view.relevance.*; - -import java.io.IOException; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Collectors; - -import static org.variantsync.diffdetective.util.fide.FormulaUtils.*; - -@Disabled -public class ViewTest { - private static final Path resDir = Constants.RESOURCE_DIR.resolve("badvdiff"); - - private static void showViews( - VariationDiff initialVDiff, - Relevance query - ) { - // treeify - final BadVDiff badDiff = BadVDiff.fromGood(initialVDiff); - - // create view - final BadVDiff view = badDiff.deepCopy(); - TreeView.treeInline(view.diff(), query); - - // unify - final VariationDiff goodDiff = view.toGood(); - goodDiff.assertConsistency(); - - GameEngine.showAndAwaitAll( - Show.diff(initialVDiff, "initial edit e"), - Show.baddiff(badDiff, "tree(e)"), - Show.baddiff(view, "view(tree(e), " + query + ")"), - Show.diff(goodDiff, "unify(view(tree(e), " + query + "))") - ); - } - - @ParameterizedTest - @ValueSource(strings = { - "emacsbug1" - }) - void debugTest(String filename) throws IOException, DiffParseException { - final String filenameWithEnding = filename + ".diff"; - final Path testfile = resDir.resolve(filenameWithEnding); - Logger.debug("Testing " + testfile); -// is( /* Check both of the above conditions, for symbols. */) - final VariationDiff D = VariationDiff.fromFile(testfile, VariationDiffParseOptions.Default); - D.assertConsistency(); - - final Relevance debugQuery = new Search(" /* Check both of the above conditions, for symbols. */"); - final var imp = DiffView.computeWhenNodesAreRelevant(D, debugQuery); - Show.diff(DiffView.optimized(D, debugQuery, imp)).showAndAwait(); - Show.diff(DiffView.naive(D, debugQuery, imp)).showAndAwait(); - } - - - @ParameterizedTest - @ValueSource(strings = { - "1", - "diamond", - "deep_insertion" - }) - void test(String filename) throws IOException, DiffParseException { - final String filenameWithEnding = filename + ".diff"; - final Path testfile = resDir.resolve(filenameWithEnding); - Logger.debug("Testing " + testfile); - - // Load diff - final VariationDiff initialVDiff = VariationDiff.fromFile(testfile, VariationDiffParseOptions.Default); - initialVDiff.assertConsistency(); - - List queries = List.of( - new Trace("B"), - new Configure(negate(var("B"))), - new Search("foo") - ); - - for (Relevance q : queries) { - final var viewNodes = DiffView.computeWhenNodesAreRelevant(initialVDiff, q); - - GameEngine.showAndAwaitAll( - Show.diff(initialVDiff, "D = " + filenameWithEnding), - Show.diff(DiffView.naive(initialVDiff, q, viewNodes), "diff_naive(D, " + q + ")"), - Show.diff(DiffView.optimized(initialVDiff, q, viewNodes), "diff_smart(D, " + q + ")") - ); - } - -// showViews(initialVDiff, new VariantQuery(and(var("X")))); -// showViews(initialVDiff, new VariantQuery(and(var("Y")))); -// showViews(initialVDiff, new VariantQuery(and(negate(var("X"))))); -// showViews(initialVDiff, new VariantQuery(and(negate(var("Y"))))); -// showViews(initialVDiff, new FeatureQuery("X")); -// showViews(initialVDiff, new FeatureQuery("Y")); -// showViews(initialVDiff, new ArtifactQuery("y")); - } - - @ParameterizedTest - @ValueSource(strings = { - "runningexampleInDomain", - }) - void inspectRunningExample(String filename) throws IOException, DiffParseException { - final Path testfile = resDir.resolve(filename + ".diff"); - -// final Literal X = var("Mutable"); - final Literal featureRing = var("Ring"); - final Literal featureDoubleLink = var("DoubleLink"); - - final VariationDiff d = VariationDiff.fromFile(testfile, VariationDiffParseOptions.Default); - final VariationTree b = d.project(Time.BEFORE); - final VariationTree a = d.project(Time.AFTER); - - // Queries of Listing 3 and 4 - final Relevance bobsQuery1 = new Configure(and(negate(featureDoubleLink))); - final Relevance charlottesQuery = new Configure(negate(featureRing)); - - // Figure 1 - GameEngine.showAndAwaitAll( - Show.tree(b, "Figure 1: project(D, b)") - ); - - // Figure 2 - GameEngine.showAndAwaitAll( - Show.diff(d, "Figure 2: D") - ); - - // Figure 3 - final Configure configureExample1 = new Configure( - and(featureRing, /* FM = */ negate(and(featureDoubleLink, featureRing))) - ); - GameEngine.showAndAwaitAll( - Show.tree(TreeView.tree(b, configureExample1), "Figure 3: view_{tree}(Figure 1, " + configureExample1 + ")") - ); - - // Figure 4 - final Trace traceYesExample1 = new Trace( - featureDoubleLink.toString() - ); - GameEngine.showAndAwaitAll( - Show.tree(TreeView.tree(b, traceYesExample1), "Figure 4: view_{tree}(Figure 1, " + traceYesExample1 + ")") - ); - - // Figure 5 - GameEngine.showAndAwaitAll( - Show.diff(DiffView.optimized(d, charlottesQuery), "Figure 5: view_{naive}(Figure 2, " + charlottesQuery + ")") - ); - } - - @ParameterizedTest - @ValueSource(strings = { - "1", - "2" -// "runningexample", -//// "const", -//// "diamond", -//// "deep_insertion" - }) - void testAllConfigs(String filename) throws IOException, DiffParseException { - final Path testfile = resDir.resolve(filename + ".diff"); - final VariationDiff d = VariationDiff.fromFile(testfile, VariationDiffParseOptions.Default); - - final List configs = UniqueViewsAlgorithm.getUniquePartialConfigs(d, false); - final List> views = new ArrayList<>(); - - final StringBuilder str = new StringBuilder(); - for (int i = 0; i < configs.size(); ++i) { - final Node config = configs.get(i); - - final Relevance q = new Configure(config); - final VariationDiff view = DiffView.optimized(d, q); - views.add(view); - - str - .append(" ") - .append(org.apache.commons.lang3.StringUtils.leftPad( - Integer.toString(i), - 4) - ) - .append(".) ") - .append(org.apache.commons.lang3.StringUtils.rightPad( - config.toString(NodeWriter.logicalSymbols), - 40) - ) - .append(" --- ") - .append( - view.computeArtifactNodes().stream() - .map(a -> a.getLabel().getLines().stream() - .map(String::trim) - .collect(Collectors.joining(StringUtils.LINEBREAK)) - ) - .collect(Collectors.toList())) - .append(StringUtils.LINEBREAK); - } - - Logger.info("All unique partial configs:" + StringUtils.LINEBREAK + str); - Show.diff(d, "D").show(); -// { -// final Query q = new FeatureQuery("C"); -// Show.diff(DiffView.optimized(d, q), "view(D, " + q.getName() + ")").showAndAwait(); -// } - - for (int i = 0; i < configs.size(); ++i) { - final VariationDiff view = views.get(i); - final Relevance q = ((ViewSource) view.getSource()).relevance(); - Show.diff(view, i + ".) view(D, " + q + ")").showAndAwait(); - } - } - - @ParameterizedTest - @ValueSource(strings = { - "1", - "2", - "runningexample", - "const", - "diamond", - "deep_insertion" - }) - void cutTest(String filename) throws IOException, DiffParseException { - final Path testfile = resDir.resolve(filename + ".diff"); - final VariationDiff d = VariationDiff.fromFile(testfile, VariationDiffParseOptions.Default); - - Show.diff(d, "original").showAndAwait(); - CutNonEditedSubtrees.genericTransform(true, d); - Show.diff(d, "cut").showAndAwait(); - } -} From ed7ff6248ae2e957a504eafbce57016065b13eff Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 13 Oct 2025 17:27:58 +0200 Subject: [PATCH 02/29] feat: Label::observablyEqual --- .../variantsync/diffdetective/variation/Label.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/main/java/org/variantsync/diffdetective/variation/Label.java b/src/main/java/org/variantsync/diffdetective/variation/Label.java index 1250497e6..d15d83006 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/Label.java +++ b/src/main/java/org/variantsync/diffdetective/variation/Label.java @@ -48,4 +48,18 @@ default Label withTimeDependentStateFrom(Label other, Time time) { * Creates a deep copy of this label. */ Label clone(); + + /** + * Tests whether two labels are observably equal. + * Observably equal means that the two labels + * cannot be distinguished by using methods from this interface. + * The given labels might indeed be of different classes or might + * be considered unequal regarding {@link Object#equals(Object)}. + */ + static boolean observablyEqual(Label a, Label b) { + if (a == null) return false; + if (a == b) return true; + return a.getLines().equals(b.getLines()) + && a.getTrailingLines().equals(b.getTrailingLines()); + } } From dedd98b278556c4bef1010bdc5407656ae9a800d Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 13 Oct 2025 17:29:35 +0200 Subject: [PATCH 03/29] feat: DiffNode::isSameAsIgnoringLineNumbers --- .../variation/diff/DiffNode.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java b/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java index f0f05f13c..dbe10d5cb 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java @@ -924,6 +924,42 @@ private static boolean isSameAs(DiffNode a, DiffNode b, return aIt.hasNext() == bIt.hasNext(); } + /** + * Returns true if this subtree is exactly equal to {@code other} except for line numbers and other metadata in labels. + * This equality is a weaker equality than {@link DiffNode#isSameAs(DiffNode)} (i.e., whenever isSameAs returns true, so does + * isSameAsIgnoringLineNumbers). + * Labels of DiffNodes are compared via {@link Label#observablyEqual(Label, Label)}. + * This check uses equality checks instead of identity. + */ + public boolean isSameAsIgnoringLineNumbers(DiffNode other) { + return isSameAsIgnoringLineNumbers(this, other, new HashSet<>()); + } + + private static boolean isSameAsIgnoringLineNumbers(DiffNode a, DiffNode b, Set> visited) { + if (!visited.add(a)) { + return true; + } + + if (!( + a.getDiffType().equals(b.getDiffType()) && + a.getNodeType().equals(b.getNodeType()) && + Objects.equals(a.getFormula(), b.getFormula()) && + Label.observablyEqual(a.getLabel(), b.getLabel()) + )) { + return false; + } + + Iterator> aIt = a.getAllChildren().iterator(); + Iterator> bIt = b.getAllChildren().iterator(); + while (aIt.hasNext() && bIt.hasNext()) { + if (!isSameAsIgnoringLineNumbers(aIt.next(), bIt.next(), visited)) { + return false; + } + } + + return aIt.hasNext() == bIt.hasNext(); + } + @Override public String toString() { String s; From 312ff2e7c8c591a4421832b68d6829f5c3c94449 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 13 Oct 2025 17:29:51 +0200 Subject: [PATCH 04/29] feat: VariationDiff:isSameAsIgnoringLineNumbers --- .../diffdetective/variation/diff/VariationDiff.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/VariationDiff.java b/src/main/java/org/variantsync/diffdetective/variation/diff/VariationDiff.java index 0d6c365bb..c50396503 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/VariationDiff.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/VariationDiff.java @@ -535,10 +535,20 @@ public ConsistencyResult isConsistent() { return ConsistencyResult.Success(); } + /** + * @see DiffNode#isSameAs + */ public boolean isSameAs(VariationDiff b) { return getRoot().isSameAs(b.getRoot()); } + /** + * @see DiffNode#isSameAsIgnoringLineNumbers + */ + public boolean isSameAsIgnoringLineNumbers(VariationDiff b) { + return getRoot().isSameAsIgnoringLineNumbers(b.getRoot()); + } + @Override public String toString() { return "VariationDiff of " + source; From 66e09707655ddeb88668d3fd389f2a35543b5fd5 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 13 Oct 2025 17:30:13 +0200 Subject: [PATCH 05/29] fix: some typos in VariationTreeNode documentation --- .../diffdetective/variation/tree/VariationTreeNode.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTreeNode.java b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTreeNode.java index 0609a8fd1..35bcdbb5b 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTreeNode.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTreeNode.java @@ -33,8 +33,8 @@ * *

This class contains references to all of its children and its parent so all connected nodes of * a variation tree can be reached through each node of the variation tree. Nevertheless, most of - * the time only node itself or its subtree (the reflexive hull of {@link getChildren}) is meant - * when when referencing a {@code VariationTreeNode}. Use {@link VariationTree} to unambiguously + * the time only the node itself or its subtree (the reflexive hull of {@link getChildren}) is referred to + * when referencing a {@code VariationTreeNode}. Use {@link VariationTree} to unambiguously * refer to a whole variation tree. * *

If possible, algorithms should be using {@link VariationNode} instead of this concrete From 99f9df52f8cf359b25ae9d36a9b613ead88589b3 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 13 Oct 2025 17:32:18 +0200 Subject: [PATCH 06/29] feat: specification for Configure --- .../tree/view/relevance/Configure.java | 10 +--- .../view/relevance/spec/ConfigureSpec.java | 57 +++++++++++++++++++ 2 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/spec/ConfigureSpec.java diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java index 2b571b3f7..2b6ce11b3 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java @@ -2,7 +2,6 @@ import org.prop4j.Node; import org.prop4j.NodeWriter; -import org.variantsync.diffdetective.analysis.logic.SAT; import org.variantsync.diffdetective.util.fide.FixTrueFalse; import org.variantsync.diffdetective.util.fide.FixTrueFalse.Formula; import org.variantsync.diffdetective.variation.tree.VariationNode; @@ -10,6 +9,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.function.Consumer; +import org.variantsync.diffdetective.variation.tree.view.relevance.spec.ConfigureSpec; /** * Relevance predicate that generates (partial) variants from variation trees. @@ -73,12 +73,8 @@ public Configure(final Map assignment) { @Override public boolean test(VariationNode v) { - return SAT.isSatisfiable( - FixTrueFalse.Formula.and( - configuration, - FixTrueFalse.EliminateTrueAndFalse(v.getPresenceCondition()) - ) - ); + return ConfigureSpec.test(configuration, v); + } } @Override diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/spec/ConfigureSpec.java b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/spec/ConfigureSpec.java new file mode 100644 index 000000000..a1d1f0fe9 --- /dev/null +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/spec/ConfigureSpec.java @@ -0,0 +1,57 @@ +package org.variantsync.diffdetective.variation.tree.view.relevance.spec; + +import org.prop4j.Node; +import org.prop4j.NodeWriter; +import org.variantsync.diffdetective.analysis.logic.SAT; +import org.variantsync.diffdetective.util.fide.FixTrueFalse; +import org.variantsync.diffdetective.util.fide.FixTrueFalse.Formula; +import org.variantsync.diffdetective.variation.tree.VariationNode; +import org.variantsync.diffdetective.variation.tree.view.relevance.Relevance; + +/** + * This class serves as a specification for {@link org.variantsync.diffdetective.variation.tree.view.relevance.Configure}. + * Both classes must act semantically equivalent as a {@link Relevance} predicate. + * Whereas Configure is an implementation optimized to avoid SAT calls where necessary, the implementation in ConfigureSpec + * is kept simple by design to remain verifiable. + * ConfigureSpec tests a partial configuration against a variation tree or diff by checking each node individually. + * To this end ConfigureSpec uses the default, naive implementations of {@link Relevance} instead of providing + * optimized implementations for tree traversal as Configure does. + * + * This class is not intended for production use and rather is supposed to be used in tests. + * + * @author Paul Bittner + */ +public record ConfigureSpec(Formula config) implements Relevance { + public static boolean test(Formula formula, VariationNode v) { + return SAT.isSatisfiable( + Formula.and( + formula, + FixTrueFalse.EliminateTrueAndFalse(v.getPresenceCondition()) + ) + ); + } + + public ConfigureSpec(final Node configuration) { + this(FixTrueFalse.EliminateTrueAndFalse(configuration)); + } + + @Override + public boolean test(VariationNode t) { + return test(config, t); + } + + @Override + public String getFunctionName() { + return "configure_spec"; + } + + @Override + public String parametersToString() { + return config.get().toString(NodeWriter.logicalSymbols); + } + + @Override + public String toString() { + return Relevance.toString(this); + } +} From a51ad7590d2ea96bea1448fb0987cd7b6e34e0bc Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 13 Oct 2025 17:32:36 +0200 Subject: [PATCH 07/29] docs: refine documentation on Relevance::computeViewNodesCheckAll --- .../diffdetective/variation/tree/view/relevance/Relevance.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Relevance.java b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Relevance.java index 8bc06b5e6..9017a8513 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Relevance.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Relevance.java @@ -37,6 +37,7 @@ public interface Relevance extends Predicate> { * In particular, this function checks each node in the given tree v on relevance. * For each node that is deemed relevant by the given relevance predicate rho, that node and all its ancestors are * marked as relevant by invoking the given callback markRelevant. + * The callback may be invoked multiple times on the same node. * This function tests the relevance predicate on all nodes separately and performs no optimizations. * @param rho The relevance predicate to test on all nodes. * @param v The root node the tree to test for relevance. From 9f2c50dbf50be112a18861fa4d306a532b4aa379 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 13 Oct 2025 17:34:59 +0200 Subject: [PATCH 08/29] fix: Configure falsely removed ELSE and ELIF nodes This bug was reported by @piameier. --- .../tree/view/relevance/Configure.java | 69 ++++++++++++++++--- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java index 2b6ce11b3..9295b35c3 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java @@ -1,14 +1,17 @@ package org.variantsync.diffdetective.variation.tree.view.relevance; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.function.Consumer; + import org.prop4j.Node; import org.prop4j.NodeWriter; +import org.variantsync.diffdetective.util.Assert; import org.variantsync.diffdetective.util.fide.FixTrueFalse; import org.variantsync.diffdetective.util.fide.FixTrueFalse.Formula; +import org.variantsync.diffdetective.variation.NodeType; import org.variantsync.diffdetective.variation.tree.VariationNode; - -import java.util.Map; -import java.util.Map.Entry; -import java.util.function.Consumer; import org.variantsync.diffdetective.variation.tree.view.relevance.spec.ConfigureSpec; /** @@ -75,18 +78,66 @@ public Configure(final Map assignment) { public boolean test(VariationNode v) { return ConfigureSpec.test(configuration, v); } + + private > void computeViewNodes(List vs, Consumer markRelevant) { + for (final TreeNode c : vs) { + computeViewNodes(c, markRelevant); + } } @Override public > void computeViewNodes(TreeNode v, Consumer markRelevant) { - markRelevant.accept(v); + // The implementation of this method must be semantically equivalent to the default implementation Relevance.super.computeViewNodes(v, markRelevant); + // The implementation of this method is supposed to be optimized to avoid redundant SAT calls when possible. + // This requirement is modelled explicitly in terms of the ConfigureSpec class. - for (final TreeNode c : v.getChildren()) { - // If the child is an artifact it has the same presence condition as we do, so it is also included in the view. - if (c.isArtifact() || test(c)) { - computeViewNodes(c, markRelevant); + // If the child is an artifact, it has the same presence condition as v does, so it is also included in the view. + // The root must be in any view to ensure consistency. + if (v.isArtifact() || v.isRoot()) { + markRelevant.accept(v); + computeViewNodes(v.getChildren(), markRelevant); + } else { + computeViewNodesOfElifChain(v, markRelevant); + } + } + + /** + * @return true if the given node was marked relevant + */ + private > boolean computeViewNodesOfElifChain(TreeNode v, Consumer markRelevant) { + final NodeType vt = v.getNodeType(); + Assert.assertTrue(vt == NodeType.IF || vt == NodeType.ELSE || vt == NodeType.ELIF); + + if (test(v)) { + markRelevant.accept(v); + computeViewNodes(v.getChildren(), markRelevant); + return true; + } else { + // If a partial configuration excludes the node v (i.e, test(v) == false), + // then any ELIF or ELSE branches might still be included. + for (final TreeNode c : v.getChildren()) { + NodeType ct = c.getNodeType(); + + // We can skip all children that are not ELSE or ELIF nodes because these are excluded because v is exluded. + // If our node v was deemed irrelevant and it has an ELSE node c, that ELSE node c must be relevant. + if (ct == NodeType.ELSE) { + markRelevant.accept(v); + markRelevant.accept(c); + computeViewNodes(c.getChildren(), markRelevant); + return true; + } + + // An ELIF might hold any formula which we have to test. + // We handle ELIF nodes recursively because ELIFs might be nested. + // If at least one branch of the ELIF chain is included, we must include also v in the view to retain consistency of the tree. + if (ct == NodeType.ELIF && computeViewNodesOfElifChain(c, markRelevant)) { + markRelevant.accept(v); + return true; + } } } + + return false; } @Override From 48f93b4017c22e4aa15fbdaf6fa0c61c660e96fa Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 13 Oct 2025 17:40:49 +0200 Subject: [PATCH 09/29] test: new tests for Views --- src/test/java/ViewsTest.java | 145 ++++++++++++++++++ .../resources/diffs/views/tree/elif-chain.c | 15 ++ src/test/resources/diffs/views/tree/elif.c | 5 + src/test/resources/diffs/views/tree/else.c | 5 + 4 files changed, 170 insertions(+) create mode 100644 src/test/java/ViewsTest.java create mode 100644 src/test/resources/diffs/views/tree/elif-chain.c create mode 100644 src/test/resources/diffs/views/tree/elif.c create mode 100644 src/test/resources/diffs/views/tree/else.c diff --git a/src/test/java/ViewsTest.java b/src/test/java/ViewsTest.java new file mode 100644 index 000000000..ea618f3d1 --- /dev/null +++ b/src/test/java/ViewsTest.java @@ -0,0 +1,145 @@ +import static org.variantsync.diffdetective.util.fide.FormulaUtils.and; +import static org.variantsync.diffdetective.util.fide.FormulaUtils.negate; +import static org.variantsync.diffdetective.util.fide.FormulaUtils.var; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.prop4j.Literal; +import org.prop4j.Node; +import org.variantsync.diffdetective.diff.result.DiffParseException; +import org.variantsync.diffdetective.util.Assert; +import org.variantsync.diffdetective.variation.DiffLinesLabel; +import org.variantsync.diffdetective.variation.diff.DiffNode; +import org.variantsync.diffdetective.variation.diff.VariationDiff; +import org.variantsync.diffdetective.variation.diff.parse.VariationDiffParseOptions; +import org.variantsync.diffdetective.variation.diff.view.DiffView; +import org.variantsync.diffdetective.variation.tree.VariationTree; +import org.variantsync.diffdetective.variation.tree.view.TreeView; +import org.variantsync.diffdetective.variation.tree.view.relevance.Configure; +import org.variantsync.diffdetective.variation.tree.view.relevance.Relevance; +import org.variantsync.diffdetective.variation.tree.view.relevance.spec.ConfigureSpec; + +/** + * Tests for views on variation trees and diffs. + */ +public class ViewsTest { + private static final Path + resDir = Constants.RESOURCE_DIR.resolve("diffs").resolve("views"), + treeDir = resDir.resolve("tree"), // directory of test cases for variation trees + diffDir = resDir.resolve("diff") // directory of test cases for variation diffs + ; + + /** + * This record holds the implementation of a relevance predicate and its specification. + * Both predicates will be tested for semantic equivalence. + * The idea is that the first relevance is an optimized implementation, whereas the second + * relevance is an easy-to-understand and easy-to-verify but suboptimal implementation. + **/ + private static record RelevanceSpec(Relevance impl, Relevance spec) {} + + // These classes are test cases for testing consistency between a relevance predicate's implementation and specification + // on trees and diffs respectively. + private static record TreeConsistencyTestCase(VariationTree tree, List relevances) {} + private static record DiffConsistencyTestCase(VariationDiff diff, List relevances) {} + + /** + * Creates a TreeConsistencyTestCase for the Configure relevance. + * @param fileName The name of the file to parse as variation tree. + * @param fs any number of formulas to test view generation with + */ + private static TreeConsistencyTestCase treeConf(String fileName, Node... fs) throws IOException, DiffParseException { + return new TreeConsistencyTestCase( + VariationTree.fromFile(treeDir.resolve(fileName), VariationDiffParseOptions.Default), + Arrays.stream(fs).map(f -> new RelevanceSpec(new Configure(f), new ConfigureSpec(f))).toList() + ); + } + + /** + * Creates a DiffConsistencyTestCase for the Configure relevance. + * @param fileName The name of the file to parse as variation diff. + * @param fs any number of formulas to test view generation with + */ + private static DiffConsistencyTestCase diffConf(String fileName, Node... fs) throws IOException, DiffParseException { + return new DiffConsistencyTestCase( + VariationDiff.fromFile(diffDir.resolve(fileName), VariationDiffParseOptions.Default), + Arrays.stream(fs).map(f -> new RelevanceSpec(new Configure(f), new ConfigureSpec(f))).toList() + ); + } + + private static List treeConsistencyTestCases() throws IOException, DiffParseException { + final Literal A = var("A"), B = var("B"), C = var("C"), D = var("D"), E = var("E"); + return List.of( + treeConf("else.c", A, negate(A), C), + treeConf("elif.c", A, negate(A), B, negate(B)), + treeConf("elif-chain.c", negate(E), and(negate(A), negate(B), negate(C), negate(D))) + ); + } + + public static List diffConsistencyTestCases() throws IOException, DiffParseException { + List testCases = new ArrayList<>(); + + // We reuse all test cases for variation trees for variation diffs. + testCases.addAll(treeConsistencyTestCases().stream().map( + treeTestCase -> new DiffConsistencyTestCase(treeTestCase.tree.toCompletelyUnchangedVariationDiff(), treeTestCase.relevances)).toList() + ); + + // TODO: Create some additional test cases targeted specifically at variation diffs. + // In these test cases, the projections should be different. + + return testCases; + } + + /** + * Tests that a {@link Relevance} predicate is consistent with its specification on all + * supplied test cases. + */ + @ParameterizedTest + @MethodSource("treeConsistencyTestCases") + public void consistencyOnTrees(TreeConsistencyTestCase t) { + for (RelevanceSpec relevance : t.relevances) { + final VariationTree impl = TreeView.tree(t.tree, relevance.impl); + final VariationTree spec = TreeView.tree(t.tree, relevance.spec); + + // To compare the trees, we create a diff. + // If there are no deleted or removed nodes, both trees are equal. + final VariationDiff d = VariationDiff.fromTrees(impl, spec); + Assert.assertTrue(d.allMatch(DiffNode::isNon)); + } + } + + /** + * Tests that + * (1) a {@link Relevance} predicate is consistent with its specification, and + * (2) the optimized generation of views on variation diffs + * ({@link DiffView#optimized(VariationDiff, Relevance)}) is consistent + * with the naive implementation ({@link DiffView#naive(VariationDiff, Relevance)}) + * which rather serves as a specification. + */ + @ParameterizedTest + @MethodSource("diffConsistencyTestCases") + public void consistencyOnDiffs(DiffConsistencyTestCase t) throws IOException, DiffParseException { + for (RelevanceSpec relevance : t.relevances) { + // All of these must be equal. + final VariationDiff naive_impl = DiffView.naive(t.diff, relevance.impl); + final VariationDiff naive_spec = DiffView.naive(t.diff, relevance.spec); + final VariationDiff opt_impl = DiffView.optimized(t.diff, relevance.impl); + final VariationDiff opt_spec = DiffView.optimized(t.diff, relevance.spec); + + // First, we test consistency among relevance implementation and relevance specification. + Assert.assertTrue(naive_impl.isSameAsIgnoringLineNumbers(naive_spec)); + Assert.assertTrue(opt_impl.isSameAsIgnoringLineNumbers(opt_spec)); + + // Second, assuming relevance consistency, we test consistency between optimized and naive view generation. + // We get the last comparison Assert.assertTrue(opt_impl.isSameAsIgnoringLineNumbers(naive_impl)) by transitivity. + Assert.assertTrue(opt_spec.isSameAsIgnoringLineNumbers(naive_spec)); + } + } + + // TODO: Test cases where we check the result of computing a view against a predefined ground truth. +} diff --git a/src/test/resources/diffs/views/tree/elif-chain.c b/src/test/resources/diffs/views/tree/elif-chain.c new file mode 100644 index 000000000..d9dd33cec --- /dev/null +++ b/src/test/resources/diffs/views/tree/elif-chain.c @@ -0,0 +1,15 @@ +int x = +#ifdef A + 0 +#elif B + 1 +#elif C + 2 +#elif D + 3 +#elif E + 4 +#else + 5 +#endif + ; diff --git a/src/test/resources/diffs/views/tree/elif.c b/src/test/resources/diffs/views/tree/elif.c new file mode 100644 index 000000000..ced5c84a2 --- /dev/null +++ b/src/test/resources/diffs/views/tree/elif.c @@ -0,0 +1,5 @@ +#ifdef A + foo +#elif B + bar +#endif diff --git a/src/test/resources/diffs/views/tree/else.c b/src/test/resources/diffs/views/tree/else.c new file mode 100644 index 000000000..c6c093c56 --- /dev/null +++ b/src/test/resources/diffs/views/tree/else.c @@ -0,0 +1,5 @@ +#ifdef A + foo +#else + bar +#endif From 8c63c205f35b5577f204a16ee18cb4416d6f53fe Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 20 Oct 2025 16:37:36 +0200 Subject: [PATCH 10/29] refactor: extract Transformer super interface ... ... from VariationDiffTransformer --- .../analysis/PreprocessingAnalysis.java | 3 +- .../internal/SimpleRenderer.java | 3 +- .../mining/postprocessing/Postprocessor.java | 3 +- .../CollapseNestedNonEditedAnnotations.java | 2 +- .../diff/transform/LabelWithEditClass.java | 2 +- .../variation/diff/transform/Transformer.java | 70 +++++++++++++++++++ .../transform/VariationDiffTransformer.java | 62 +--------------- 7 files changed, 79 insertions(+), 66 deletions(-) create mode 100644 src/main/java/org/variantsync/diffdetective/variation/diff/transform/Transformer.java diff --git a/src/main/java/org/variantsync/diffdetective/analysis/PreprocessingAnalysis.java b/src/main/java/org/variantsync/diffdetective/analysis/PreprocessingAnalysis.java index 1ea16ece0..f89e7c689 100644 --- a/src/main/java/org/variantsync/diffdetective/analysis/PreprocessingAnalysis.java +++ b/src/main/java/org/variantsync/diffdetective/analysis/PreprocessingAnalysis.java @@ -3,6 +3,7 @@ import java.util.Arrays; import java.util.List; +import org.variantsync.diffdetective.variation.diff.transform.Transformer; import org.variantsync.diffdetective.variation.diff.transform.VariationDiffTransformer; import org.variantsync.diffdetective.variation.DiffLinesLabel; @@ -20,7 +21,7 @@ public PreprocessingAnalysis(VariationDiffTransformer... preproc @Override public boolean analyzeVariationDiff(Analysis analysis) { - VariationDiffTransformer.apply(preprocessors, analysis.getCurrentVariationDiff()); + Transformer.apply(preprocessors, analysis.getCurrentVariationDiff()); analysis.getCurrentVariationDiff().assertConsistency(); return true; } diff --git a/src/main/java/org/variantsync/diffdetective/internal/SimpleRenderer.java b/src/main/java/org/variantsync/diffdetective/internal/SimpleRenderer.java index 7737afedb..21e1296cb 100644 --- a/src/main/java/org/variantsync/diffdetective/internal/SimpleRenderer.java +++ b/src/main/java/org/variantsync/diffdetective/internal/SimpleRenderer.java @@ -18,6 +18,7 @@ import org.variantsync.diffdetective.variation.diff.render.RenderOptions; import org.variantsync.diffdetective.variation.diff.render.VariationDiffRenderer; import org.variantsync.diffdetective.variation.diff.serialize.nodeformat.MappingsDiffNodeFormat; +import org.variantsync.diffdetective.variation.diff.transform.Transformer; import org.variantsync.diffdetective.variation.diff.transform.VariationDiffTransformer; import java.io.IOException; @@ -165,7 +166,7 @@ public static void main(String[] args) throws IOException { final List> transform = VariationDiffMiner.Postprocessing(repository); final PatchDiff patch = VariationDiffParser.parsePatch(repository, file, commit); Assert.assertNotNull(patch != null); - VariationDiffTransformer.apply(transform, patch.getVariationDiff()); + Transformer.apply(transform, patch.getVariationDiff()); renderer.render(patch, Path.of("render", repoName), RENDER_OPTIONS_TO_USE); } diff --git a/src/main/java/org/variantsync/diffdetective/mining/postprocessing/Postprocessor.java b/src/main/java/org/variantsync/diffdetective/mining/postprocessing/Postprocessor.java index 8b11d209b..19984ed7c 100644 --- a/src/main/java/org/variantsync/diffdetective/mining/postprocessing/Postprocessor.java +++ b/src/main/java/org/variantsync/diffdetective/mining/postprocessing/Postprocessor.java @@ -7,6 +7,7 @@ import org.variantsync.diffdetective.variation.diff.filter.ExplainedFilter; import org.variantsync.diffdetective.variation.diff.filter.TaggedPredicate; import org.variantsync.diffdetective.variation.diff.transform.CutNonEditedSubtrees; +import org.variantsync.diffdetective.variation.diff.transform.Transformer; import org.variantsync.diffdetective.variation.diff.transform.VariationDiffTransformer; import java.util.List; @@ -66,7 +67,7 @@ public static Postprocessor Default() { public Result postprocess(final List> frequentSubgraphs) { final List> processedTrees = frequentSubgraphs.stream() .filter(filters) - .peek(tree -> VariationDiffTransformer.apply(transformers, tree)) + .peek(tree -> Transformer.apply(transformers, tree)) .toList(); final Map filterCounts = new ExplainedFilterSummary(filters).snapshot(); diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CollapseNestedNonEditedAnnotations.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CollapseNestedNonEditedAnnotations.java index 30da444b1..985cf4258 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CollapseNestedNonEditedAnnotations.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CollapseNestedNonEditedAnnotations.java @@ -36,7 +36,7 @@ public class CollapseNestedNonEditedAnnotations implements VariationDiffTransfor private final List>> chains = new ArrayList<>(); @Override - public List>> getDependencies() { + public List>>> getDependencies() { return List.of(Cast.unchecked(CutNonEditedSubtrees.class)); } diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/LabelWithEditClass.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/LabelWithEditClass.java index 8d3fe1446..61c14f7b0 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/LabelWithEditClass.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/LabelWithEditClass.java @@ -39,7 +39,7 @@ public void transform(VariationDiff variationDiff) { } @Override - public List>> getDependencies() { + public List>>> getDependencies() { return relabelNodes.getDependencies(); } } diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/Transformer.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/Transformer.java new file mode 100644 index 000000000..ce9115638 --- /dev/null +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/Transformer.java @@ -0,0 +1,70 @@ +package org.variantsync.diffdetective.variation.diff.transform; + +import java.util.ArrayList; +import java.util.List; + +/** + * Models an operation that changes an object of type T inplace (i.e., the object is altered). + * To model further assumptions on the given elements T (i.e., assumptions that cannot easily be expressed as a type), + * Transformers may have dependencies to other transformers, which should be applied first. + */ +public interface Transformer { + /** + * Apply a transformation to the given Object inplace. + * The object will be changed. + * @param element The T object to transform. + */ + void transform(final T element); + + /** + * Returns a list of dependencies to other transformers. + * A transformer should only be run, if another transformation with the respective type was run for each type on the dependencies. + * @return List of types of which instances should be run before applying this transformation. + */ + default List>> getDependencies() { + return new ArrayList<>(0); + } + + /** + * Checks that the dependencies of all given transformers are satisfied when + * applying the transformers sequentially. + * @param transformers The transformers whose dependencies to check. + * @throws RuntimeException when a dependency is not met. + */ + static void checkDependencies(final List> transformers) { + for (int i = transformers.size() - 1; i >= 0; --i) { + final Transformer currentTransformer = transformers.get(i); + final List>> currentDependencies = currentTransformer.getDependencies(); + for (final Class> dependency : currentDependencies) { + boolean dependencyMet = false; + for (int j = i - 1; j >= 0; --j) { + if (dependency.isInstance(transformers.get(j))) { + dependencyMet = true; + break; + } + } + if (!dependencyMet) { + throw new RuntimeException("Dependency not met! Transformer " + + currentTransformer + + " requires a transformer of type " + + dependency + + " applied before!"); + } + } + } + } + + /** + * Applies all given transformers to the given element sequentially. + * First checks that all dependencies between transformers are met via {@link #checkDependencies(List)}. + * @param transformers Transformers to apply sequentially. + * @param tree Tree to transform inplace. + */ + static void apply(final List> transformers, final T element) { + checkDependencies(transformers); + for (final Transformer t : transformers) { + t.transform(element); + } + } + +} diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationDiffTransformer.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationDiffTransformer.java index e300a3917..56a1b88c5 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationDiffTransformer.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationDiffTransformer.java @@ -3,70 +3,10 @@ import org.variantsync.diffdetective.variation.Label; import org.variantsync.diffdetective.variation.diff.VariationDiff; -import java.util.ArrayList; -import java.util.List; - /** * Interface that represents inplace transformations of VariationDiffs. * A VariationDiffTransformer is intended to alter a given VariationDiff. * @author Paul Bittner */ -public interface VariationDiffTransformer { - /** - * Apply a transformation to the given VariationDiff inplace. - * The given tree will be changed. - * @param variationDiff The VariationDiff to transform. - */ - void transform(final VariationDiff variationDiff); - - /** - * Returns a list of dependencies to other transformers. - * A transformer should only be run, if another transformation with the respective type was run for each type on the dependencies. - * @return List of types of which instances should be run before applying this transformation. - */ - default List>> getDependencies() { - return new ArrayList<>(0); - } - - /** - * Checks that the dependencies of all given VariationDiffTransformers are satisfied when - * applying the transformers sequentially. - * @param transformers The transformers whose dependencies to check. - * @throws RuntimeException when a dependency is not met. - */ - static void checkDependencies(final List> transformers) { - for (int i = transformers.size() - 1; i >= 0; --i) { - final VariationDiffTransformer currentTransformer = transformers.get(i); - final List>> currentDependencies = currentTransformer.getDependencies(); - for (final Class> dependency : currentDependencies) { - boolean dependencyMet = false; - for (int j = i - 1; j >= 0; --j) { - if (dependency.isInstance(transformers.get(j))) { - dependencyMet = true; - break; - } - } - if (!dependencyMet) { - throw new RuntimeException("Dependency not met! VariationDiffTransformer " - + currentTransformer - + " requires a transformer of type " - + dependency - + " applied before!"); - } - } - } - } - - /** - * Applies all given transformers to the given VariationDiff sequentially. - * First checks that all dependencies between transformers are met via {@link #checkDependencies(List)}. - * @param transformers Transformers to apply sequentially. - * @param tree Tree to transform inplace. - */ - static void apply(final List> transformers, final VariationDiff tree) { - checkDependencies(transformers); - for (final VariationDiffTransformer t : transformers) { - t.transform(tree); - } - } +public interface VariationDiffTransformer extends Transformer> { } From 9dfbfb2d0532906c8feda085c7c83accfd5cdc40 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 20 Oct 2025 16:38:06 +0200 Subject: [PATCH 11/29] VariationTreeTransformer interface --- .../diff/transform/VariationTreeTransformer.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationTreeTransformer.java diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationTreeTransformer.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationTreeTransformer.java new file mode 100644 index 000000000..b36a5688d --- /dev/null +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationTreeTransformer.java @@ -0,0 +1,12 @@ +package org.variantsync.diffdetective.variation.diff.transform; + +import org.variantsync.diffdetective.variation.Label; +import org.variantsync.diffdetective.variation.tree.VariationTree; + +/** + * Interface that represents inplace transformations of VariationTrees. + * A VariationTreeTransformer is intended to alter a given VariationTree. + * @author Paul Bittner + */ +public interface VariationTreeTransformer extends Transformer> { +} From fa62e30bfe3740263bd30ece7ad3fd65cb1a8db4 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 20 Oct 2025 16:40:55 +0200 Subject: [PATCH 12/29] VariationTreeNode::setFormula --- .../variation/tree/VariationTreeNode.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTreeNode.java b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTreeNode.java index 35bcdbb5b..50ac1bae0 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTreeNode.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTreeNode.java @@ -226,6 +226,22 @@ public void removeAllChildren() { childOrder.clear(); } + /** + * Sets the formula that is stored in this node. + * The formula should not be not {@code null} for + * {@link NodeType#isConditionalAnnotation mapping nodes with annotations} and should be {@code null} + * otherwise ({@link NodeType#ARTIFACT}, {@link NodeType#ELSE}). + */ + public void setFormula(Node formula) { + if (isConditionalAnnotation()) { + Assert.assertNotNull(formula); + } else { + Assert.assertNull(formula); + } + + this.featureMapping = formula; + } + @Override public Node getFormula() { return featureMapping; From 6b2bcb08a7090983a257a338d9312eb3deab08e2 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 20 Oct 2025 16:41:29 +0200 Subject: [PATCH 13/29] feat: traversing variation trees in post-order --- .../diffdetective/variation/tree/VariationNode.java | 11 +++++++++++ .../diffdetective/variation/tree/VariationTree.java | 12 +++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java index c3e8d1f39..207f71e38 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java @@ -419,6 +419,17 @@ public void forAllPreorder(Consumer action) { } } + /** + * Traverses all nodes in this subtree in postorder. + */ + public void forAllPostorder(Consumer action) { + for (var child : getChildren()) { + child.forAllPostorder(action); + } + + action.accept(this.upCast()); + } + public void forMeAndMyAncestors(final Consumer action) { action.accept(this.upCast()); final T p = getParent(); diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTree.java b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTree.java index 722744ed7..bff147612 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTree.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationTree.java @@ -151,7 +151,7 @@ public VariationDiff toCompletelyUnchangedVariationDiff() { } /** - * Invokes the given callback for each node in this Variation Tree in depth-first order. + * Invokes the given callback for each node in this Variation Tree in pre-order. * @param action callback * @return this */ @@ -160,6 +160,16 @@ public VariationTree forAllPreorder(final Consumer> acti return this; } + /** + * Invokes the given callback for each node in this Variation Tree in post-order. + * @param action callback + * @return this + */ + public VariationTree forAllPostorder(final Consumer> action) { + root.forAllPostorder(action); + return this; + } + /** * Checks whether any node in this tree satisfies the given condition. * The condition might not be invoked on every node in case a node is found. From a48b4fd5a785fcef51662f102eab9064645314c0 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 20 Oct 2025 16:41:52 +0200 Subject: [PATCH 14/29] feat: DiffNode::stealChildrenOf at time --- .../diffdetective/variation/diff/DiffNode.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java b/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java index dbe10d5cb..18b64b269 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java @@ -367,12 +367,21 @@ public List> removeChildren(Time time) { } /** - * Removes all children from the given node and adds them as children to this node at the respective times. + * Removes all children from the given node and adds them as children to this node at the given time. + * The given node will have no children afterwards at the given time. + * @param other The node whose children should be stolen for the given time. + */ + public void stealChildrenOf(Time time, final DiffNode other) { + addChildren(other.removeChildren(time), time); + } + + /** + * Removes all children from the given node and adds them as children to this node (at all times). * The given node will have no children afterwards. * @param other The node whose children should be stolen. */ public void stealChildrenOf(final DiffNode other) { - Time.forAll(time -> addChildren(other.removeChildren(time), time)); + Time.forAll(time -> stealChildrenOf(time, other)); } /** From fd0d96ec1021c8f142188df42280ee65ade46f9d Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 20 Oct 2025 16:42:11 +0200 Subject: [PATCH 15/29] fix: VariationNode::stealChildrenOf Previously, this method always crashed because addChildren assumes the given children to have no parent. Yet, the children always had 'other' as parent because we only removed them afterwards. --- .../diffdetective/variation/tree/VariationNode.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java index 207f71e38..f36f13a9a 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java @@ -282,8 +282,9 @@ public void drop() { * @see removeAllChildren */ public void stealChildrenOf(final T other) { - addChildren(other.getChildren()); + List children = other.getChildren(); other.removeAllChildren(); + addChildren(children); } /** From 1939f504bf73c74a6f8081242cfb23690098bba8 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 20 Oct 2025 16:43:11 +0200 Subject: [PATCH 16/29] feat: EliminateEmptyAlternatives transformer --- .../transform/EliminateEmptyAlternatives.java | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java new file mode 100644 index 000000000..7a0065279 --- /dev/null +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java @@ -0,0 +1,72 @@ + +package org.variantsync.diffdetective.variation.diff.transform; + +import org.prop4j.Node; +import org.variantsync.diffdetective.variation.Label; +import org.variantsync.diffdetective.variation.tree.VariationTree; +import org.variantsync.diffdetective.variation.tree.VariationTreeNode; + +import java.util.List; + +import static org.variantsync.diffdetective.util.fide.FormulaUtils.*; + +/** + * This transformer simplifies annotations such that empty alternatives do not appear in choices. + * This means, that nestings without any siblings such as + * + * 

+ * #if A
+ * #elif B
+ * #elif C
+ * #else
+ *   foo
+ * #endif
+ *
+ * + * will be simplified to + * + * 
+ * #if !A && !B && !C
+ *   foo
+ * #endif
+ *
+ * + * Annotations without any children also get eliminated. + * + * @author Paul Bittner + */ +public class EliminateEmptyAlternatives implements VariationTreeTransformer { + private void elim(VariationTreeNode subtree) { + // We simplify only annotations. + if (!subtree.isAnnotation()) return; + + final List> children = subtree.getChildren(); + + // When there are no children, 'subtree' is an empty annotation that can be eliminated. + if (children.isEmpty()) { + subtree.drop(); + } + // When there is exactly one child and that child is an 'else' or 'elif' we can simplify that nesting. + else if (children.size() == 1) { + final VariationTreeNode child = children.getFirst(); + + if ((subtree.isIf() || subtree.isElif()) && (child.isElif() || child.isElse())) { + // determine new feaure mapping + Node newFormula = negate(subtree.getFormula()); + if (child.isElif()) { + newFormula = and(newFormula, child.getFormula()); + } + subtree.setFormula(newFormula); + + // simplify tree + child.drop(); + subtree.stealChildrenOf(child); + } + } + } + + @Override + public void transform(VariationTree tree) { + tree.forAllPostorder(this::elim); + } +} From 7f70843198b2f6ad53dc9d9a55cdfd72fae6fd6b Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 20 Oct 2025 16:52:23 +0200 Subject: [PATCH 17/29] fix: javadoc error in Transformer.java --- .../diffdetective/variation/diff/transform/Transformer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/Transformer.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/Transformer.java index ce9115638..ba10a2a12 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/Transformer.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/Transformer.java @@ -58,7 +58,7 @@ static void checkDependencies(final List> transform * Applies all given transformers to the given element sequentially. * First checks that all dependencies between transformers are met via {@link #checkDependencies(List)}. * @param transformers Transformers to apply sequentially. - * @param tree Tree to transform inplace. + * @param element Tree to transform inplace. */ static void apply(final List> transformers, final T element) { checkDependencies(transformers); From db2fccfc7995ce2550153d6852ff4aaa56a726b7 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 20 Oct 2025 17:02:21 +0200 Subject: [PATCH 18/29] fix: javadoc error in EliminateEmptyAlternatives --- .../diff/transform/EliminateEmptyAlternatives.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java index 7a0065279..83e8a7473 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java @@ -14,22 +14,22 @@ * This transformer simplifies annotations such that empty alternatives do not appear in choices. * This means, that nestings without any siblings such as * - * 
+ * 
{@code
  * #if A
  * #elif B
  * #elif C
  * #else
  *   foo
  * #endif
- * 

+ * }
* * will be simplified to * - * 
+ * 
{@code
  * #if !A && !B && !C
  *   foo
  * #endif
- * 

+ * }
* * Annotations without any children also get eliminated. * From 7daeff09f3154a4dde311a71c5413b10fd435551 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Fri, 24 Oct 2025 09:39:58 +0200 Subject: [PATCH 19/29] fix: VariationNode::stealChildrenOf once again We have to create a copy of the children list because it is cleared by some implementations of removeAllChildren(). So effectively, no children were added via addChildren. --- .../variantsync/diffdetective/variation/tree/VariationNode.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java index f36f13a9a..5e8eb571b 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java @@ -282,7 +282,7 @@ public void drop() { * @see removeAllChildren */ public void stealChildrenOf(final T other) { - List children = other.getChildren(); + List children = new ArrayList<>(other.getChildren()); other.removeAllChildren(); addChildren(children); } From 2c6d6511a8ef1a8caae151c8b21d0c0d8610e042 Mon Sep 17 00:00:00 2001 From: Paul Bittner Date: Sun, 26 Oct 2025 09:59:19 +0100 Subject: [PATCH 20/29] Simplify Assertion in Configure Co-authored-by: ibbem --- .../diffdetective/variation/tree/view/relevance/Configure.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java index 9295b35c3..0f613a417 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/Configure.java @@ -105,8 +105,7 @@ public boolean test(VariationNode v) { * @return true if the given node was marked relevant */ private > boolean computeViewNodesOfElifChain(TreeNode v, Consumer markRelevant) { - final NodeType vt = v.getNodeType(); - Assert.assertTrue(vt == NodeType.IF || vt == NodeType.ELSE || vt == NodeType.ELIF); + Assert.assertTrue(v.isAnnotation()); if (test(v)) { markRelevant.accept(v); From 81978806cf4d5ca0bd3b6831827ca896860c9b0b Mon Sep 17 00:00:00 2001 From: pmbittner Date: Sun, 26 Oct 2025 10:09:52 +0100 Subject: [PATCH 21/29] remove boilerplate Variation(Tree|Diff)Transformer These where interfaces without a real purpose. --- .../analysis/PreprocessingAnalysis.java | 8 ++++---- .../diffdetective/internal/SimpleRenderer.java | 3 +-- .../diffdetective/mining/VariationDiffMiner.java | 7 ++++--- .../mining/postprocessing/Postprocessor.java | 5 ++--- .../CollapseNestedNonEditedAnnotations.java | 2 +- .../diff/transform/CutNonEditedSubtrees.java | 2 +- .../diff/transform/EliminateEmptyAlternatives.java | 2 +- .../diff/transform/FeatureExpressionFilter.java | 2 +- .../variation/diff/transform/LabelWithEditClass.java | 4 ++-- .../diff/transform/NaiveMovedArtifactDetection.java | 2 +- .../variation/diff/transform/RelabelNodes.java | 2 +- .../variation/diff/transform/RelabelRoot.java | 2 +- .../diff/transform/VariationDiffTransformer.java | 12 ------------ .../diff/transform/VariationTreeTransformer.java | 12 ------------ src/test/java/TreeTransformersTest.java | 8 ++++---- 15 files changed, 24 insertions(+), 49 deletions(-) delete mode 100644 src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationDiffTransformer.java delete mode 100644 src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationTreeTransformer.java diff --git a/src/main/java/org/variantsync/diffdetective/analysis/PreprocessingAnalysis.java b/src/main/java/org/variantsync/diffdetective/analysis/PreprocessingAnalysis.java index f89e7c689..75c967adb 100644 --- a/src/main/java/org/variantsync/diffdetective/analysis/PreprocessingAnalysis.java +++ b/src/main/java/org/variantsync/diffdetective/analysis/PreprocessingAnalysis.java @@ -3,19 +3,19 @@ import java.util.Arrays; import java.util.List; +import org.variantsync.diffdetective.variation.diff.VariationDiff; import org.variantsync.diffdetective.variation.diff.transform.Transformer; -import org.variantsync.diffdetective.variation.diff.transform.VariationDiffTransformer; import org.variantsync.diffdetective.variation.DiffLinesLabel; public class PreprocessingAnalysis implements Analysis.Hooks { - private final List> preprocessors; + private final List>> preprocessors; - public PreprocessingAnalysis(List> preprocessors) { + public PreprocessingAnalysis(List>> preprocessors) { this.preprocessors = preprocessors; } @SafeVarargs - public PreprocessingAnalysis(VariationDiffTransformer... preprocessors) { + public PreprocessingAnalysis(Transformer>... preprocessors) { this.preprocessors = Arrays.asList(preprocessors); } diff --git a/src/main/java/org/variantsync/diffdetective/internal/SimpleRenderer.java b/src/main/java/org/variantsync/diffdetective/internal/SimpleRenderer.java index 21e1296cb..2ef7fce47 100644 --- a/src/main/java/org/variantsync/diffdetective/internal/SimpleRenderer.java +++ b/src/main/java/org/variantsync/diffdetective/internal/SimpleRenderer.java @@ -19,7 +19,6 @@ import org.variantsync.diffdetective.variation.diff.render.VariationDiffRenderer; import org.variantsync.diffdetective.variation.diff.serialize.nodeformat.MappingsDiffNodeFormat; import org.variantsync.diffdetective.variation.diff.transform.Transformer; -import org.variantsync.diffdetective.variation.diff.transform.VariationDiffTransformer; import java.io.IOException; import java.nio.file.Files; @@ -163,7 +162,7 @@ public static void main(String[] args) throws IOException { final Repository repository = Repository.fromDirectory(repoPath, repoName); repository.setParseOptions(repository.getParseOptions().withDiffStoragePolicy(PatchDiffParseOptions.DiffStoragePolicy.REMEMBER_STRIPPED_DIFF)); - final List> transform = VariationDiffMiner.Postprocessing(repository); + final List>> transform = VariationDiffMiner.Postprocessing(repository); final PatchDiff patch = VariationDiffParser.parsePatch(repository, file, commit); Assert.assertNotNull(patch != null); Transformer.apply(transform, patch.getVariationDiff()); diff --git a/src/main/java/org/variantsync/diffdetective/mining/VariationDiffMiner.java b/src/main/java/org/variantsync/diffdetective/mining/VariationDiffMiner.java index 48401bea3..e2ec661be 100644 --- a/src/main/java/org/variantsync/diffdetective/mining/VariationDiffMiner.java +++ b/src/main/java/org/variantsync/diffdetective/mining/VariationDiffMiner.java @@ -13,6 +13,7 @@ import org.variantsync.diffdetective.mining.formats.MiningNodeFormat; import org.variantsync.diffdetective.mining.formats.ReleaseMiningDiffNodeFormat; import org.variantsync.diffdetective.variation.DiffLinesLabel; +import org.variantsync.diffdetective.variation.diff.VariationDiff; import org.variantsync.diffdetective.variation.diff.filter.VariationDiffFilter; import org.variantsync.diffdetective.variation.diff.serialize.GraphFormat; import org.variantsync.diffdetective.variation.diff.serialize.LineGraphExportOptions; @@ -20,7 +21,7 @@ import org.variantsync.diffdetective.variation.diff.serialize.treeformat.CommitDiffVariationDiffLabelFormat; import org.variantsync.diffdetective.variation.diff.transform.CollapseNestedNonEditedAnnotations; import org.variantsync.diffdetective.variation.diff.transform.CutNonEditedSubtrees; -import org.variantsync.diffdetective.variation.diff.transform.VariationDiffTransformer; +import org.variantsync.diffdetective.variation.diff.transform.Transformer; import java.io.IOException; import java.nio.file.Path; @@ -38,8 +39,8 @@ public class VariationDiffMiner { // public static final int PRINT_LARGEST_SUBJECTS = 3; public static final boolean DEBUG_TEST = false; - public static List> Postprocessing(final Repository repository) { - final List> processing = new ArrayList<>(); + public static List>> Postprocessing(final Repository repository) { + final List>> processing = new ArrayList<>(); processing.add(new CutNonEditedSubtrees<>()); processing.add(new CollapseNestedNonEditedAnnotations()); return processing; diff --git a/src/main/java/org/variantsync/diffdetective/mining/postprocessing/Postprocessor.java b/src/main/java/org/variantsync/diffdetective/mining/postprocessing/Postprocessor.java index 19984ed7c..e03ba9a5b 100644 --- a/src/main/java/org/variantsync/diffdetective/mining/postprocessing/Postprocessor.java +++ b/src/main/java/org/variantsync/diffdetective/mining/postprocessing/Postprocessor.java @@ -8,7 +8,6 @@ import org.variantsync.diffdetective.variation.diff.filter.TaggedPredicate; import org.variantsync.diffdetective.variation.diff.transform.CutNonEditedSubtrees; import org.variantsync.diffdetective.variation.diff.transform.Transformer; -import org.variantsync.diffdetective.variation.diff.transform.VariationDiffTransformer; import java.util.List; import java.util.Map; @@ -18,7 +17,7 @@ * Patterns are represented as VariationDiffs and might be filtered or transformed. */ public class Postprocessor { - private final List> transformers; + private final List>> transformers; private final ExplainedFilter> filters; /** @@ -31,7 +30,7 @@ public class Postprocessor { public record Result(List> processedTrees, Map filterCounts) {} private Postprocessor( - final List> transformers, + final List>> transformers, final List>> namedFilters) { this.transformers = transformers; this.filters = new ExplainedFilter>(namedFilters.stream()); diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CollapseNestedNonEditedAnnotations.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CollapseNestedNonEditedAnnotations.java index 985cf4258..d2c19d2fa 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CollapseNestedNonEditedAnnotations.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CollapseNestedNonEditedAnnotations.java @@ -31,7 +31,7 @@ * * @author Paul Bittner */ -public class CollapseNestedNonEditedAnnotations implements VariationDiffTransformer { +public class CollapseNestedNonEditedAnnotations implements Transformer> { private final List>> chainCandidates = new ArrayList<>(); private final List>> chains = new ArrayList<>(); diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CutNonEditedSubtrees.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CutNonEditedSubtrees.java index 5e4bf16a2..cdd7afae7 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CutNonEditedSubtrees.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/CutNonEditedSubtrees.java @@ -19,7 +19,7 @@ * of our edit classes in our ESEC/FSE'22 paper. * @author Paul Bittner */ -public class CutNonEditedSubtrees implements VariationDiffTransformer, VariationDiffVisitor { +public class CutNonEditedSubtrees implements Transformer>, VariationDiffVisitor { private final boolean keepDummy; /** diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java index 83e8a7473..009e67b71 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java @@ -35,7 +35,7 @@ * * @author Paul Bittner */ -public class EliminateEmptyAlternatives implements VariationTreeTransformer { +public class EliminateEmptyAlternatives implements Transformer> { private void elim(VariationTreeNode subtree) { // We simplify only annotations. if (!subtree.isAnnotation()) return; diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/FeatureExpressionFilter.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/FeatureExpressionFilter.java index ad84421c2..34729fbe5 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/FeatureExpressionFilter.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/FeatureExpressionFilter.java @@ -13,7 +13,7 @@ * For example, it might remove an IF but keep its ELSE branches which is illegal. */ @Deprecated -public record FeatureExpressionFilter(Predicate> isFeatureAnnotation) implements VariationDiffTransformer { +public record FeatureExpressionFilter(Predicate> isFeatureAnnotation) implements Transformer> { @Override public void transform(VariationDiff variationDiff) { final List> illegalNodes = new ArrayList<>(); diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/LabelWithEditClass.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/LabelWithEditClass.java index 61c14f7b0..c0292345b 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/LabelWithEditClass.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/LabelWithEditClass.java @@ -15,8 +15,8 @@ * All other nodes will be labeled by the {@link NodeType#name name of their node type}. * @author Paul Bittner */ -public class LabelWithEditClass implements VariationDiffTransformer { - private final VariationDiffTransformer relabelNodes; +public class LabelWithEditClass implements Transformer> { + private final Transformer> relabelNodes; /** * Creates a new transformation that will use the given catalog of edit classes diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/NaiveMovedArtifactDetection.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/NaiveMovedArtifactDetection.java index fd95d0046..128ccb7db 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/NaiveMovedArtifactDetection.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/NaiveMovedArtifactDetection.java @@ -20,7 +20,7 @@ * but for each line in the nodes individually. * @author Paul Bittner */ -public class NaiveMovedArtifactDetection implements VariationDiffTransformer { +public class NaiveMovedArtifactDetection implements Transformer> { @Override public void transform(final VariationDiff variationDiff) { final List, DiffNode>> twins = findArtifactTwins(variationDiff); diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RelabelNodes.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RelabelNodes.java index 2716e32b8..9fe933efb 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RelabelNodes.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RelabelNodes.java @@ -10,7 +10,7 @@ * Transformer that changes the label of each node using a relable function. * @author Paul Bittner */ -public class RelabelNodes implements VariationDiffTransformer { +public class RelabelNodes implements Transformer> { private final Function, L> getLabel; /** diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RelabelRoot.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RelabelRoot.java index ebf161165..47b0a0417 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RelabelRoot.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RelabelRoot.java @@ -7,7 +7,7 @@ * Transformer that relabels the root of a VariationDiff. * @author Paul Bittner */ -public class RelabelRoot implements VariationDiffTransformer { +public class RelabelRoot implements Transformer> { private final L newLabel; /** diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationDiffTransformer.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationDiffTransformer.java deleted file mode 100644 index 56a1b88c5..000000000 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationDiffTransformer.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.variantsync.diffdetective.variation.diff.transform; - -import org.variantsync.diffdetective.variation.Label; -import org.variantsync.diffdetective.variation.diff.VariationDiff; - -/** - * Interface that represents inplace transformations of VariationDiffs. - * A VariationDiffTransformer is intended to alter a given VariationDiff. - * @author Paul Bittner - */ -public interface VariationDiffTransformer extends Transformer> { -} diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationTreeTransformer.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationTreeTransformer.java deleted file mode 100644 index b36a5688d..000000000 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/VariationTreeTransformer.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.variantsync.diffdetective.variation.diff.transform; - -import org.variantsync.diffdetective.variation.Label; -import org.variantsync.diffdetective.variation.tree.VariationTree; - -/** - * Interface that represents inplace transformations of VariationTrees. - * A VariationTreeTransformer is intended to alter a given VariationTree. - * @author Paul Bittner - */ -public interface VariationTreeTransformer extends Transformer> { -} diff --git a/src/test/java/TreeTransformersTest.java b/src/test/java/TreeTransformersTest.java index c9132c445..2854c009b 100644 --- a/src/test/java/TreeTransformersTest.java +++ b/src/test/java/TreeTransformersTest.java @@ -17,7 +17,7 @@ import org.variantsync.diffdetective.variation.diff.serialize.edgeformat.DefaultEdgeLabelFormat; import org.variantsync.diffdetective.variation.diff.serialize.nodeformat.TypeDiffNodeFormat; import org.variantsync.diffdetective.variation.diff.serialize.treeformat.CommitDiffVariationDiffLabelFormat; -import org.variantsync.diffdetective.variation.diff.transform.VariationDiffTransformer; +import org.variantsync.diffdetective.variation.diff.transform.Transformer; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -64,8 +64,8 @@ private void transformAndRender(VariationDiff variationDiff, Str int i = 1; int prevSize = variationDiff.computeSize(); - final List> transformers = VariationDiffMiner.Postprocessing(repository); - for (final VariationDiffTransformer f : transformers) { + final List>> transformers = VariationDiffMiner.Postprocessing(repository); + for (final Transformer> f : transformers) { INFO.accept("Applying transformation " + f + "."); f.transform(variationDiff); @@ -87,7 +87,7 @@ private void transformAndRender(VariationDiff variationDiff, Str @BeforeEach public void init() { // Main.setupLogger(Level.INFO); -// VariationDiffTransformer.checkDependencies(transformers); + // Transformer.checkDependencies(transformers); } @Test From 7e0365b4481cca6967829fc0b3a4962c5d644e02 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 27 Oct 2025 09:47:51 +0100 Subject: [PATCH 22/29] DiffNode::makeUnchanged() --- .../variation/diff/DiffNode.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java b/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java index 18b64b269..bcb3fe10e 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java @@ -887,6 +887,40 @@ public DiffNode shallowCopy() { ); } + /** + * Turns this node into a node with {@link DiffType} {@link DiffType#NON}. + * To retain consistency of the variation diff, this node will also ensure that this + * node will have a parent at all times. + * To this end, the parent of this node will also be made unchanged if necessary, potentially + * making some or all ancestors of this node unchanged recursively. + * This method has no effect when this node is already unchanged. + */ + public void makeUnchanged() { + if (isNon()) return; + + this.diffType = DiffType.NON; + + final DiffNode bp = at(Time.BEFORE).parent; + final DiffNode ap = at(Time.AFTER).parent; + + // If we have a parent before the change and after the change, making this node unchanged is fine. + // Otherwise, if at least one parent is null, we have to set the other parent and make our parent unchanged as well. + if (bp == null || ap == null) { + // There is only one parent, which we store in this field. + final DiffNode p = bp == null ? ap : bp; + Assert.assertTrue(p != null); + + // If the parent is not unchanged, we have to make it unchanged so that it can be our + // parent at all times. + if (!p.isNon()) { + p.makeUnchanged(); + } + + // Now make p our parent at all times, not just at a single time. + Time.forAll(t -> at(t).parent = p); + } + } + /** * Transforms a {@code VariationNode} into a {@code DiffNode} by diffing {@code variationNode} * to itself. Recursively translates all children. From ddd4289e9a0f70a6b0dfd864f55812b9ee5b6f5b Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 27 Oct 2025 09:48:01 +0100 Subject: [PATCH 23/29] feat: HideSomeChanges --- .../diff/transform/RevertSomeChanges.java | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 src/main/java/org/variantsync/diffdetective/variation/diff/transform/RevertSomeChanges.java diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RevertSomeChanges.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RevertSomeChanges.java new file mode 100644 index 000000000..f6ec51d9e --- /dev/null +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/RevertSomeChanges.java @@ -0,0 +1,52 @@ +package org.variantsync.diffdetective.variation.diff.transform; + +import java.util.ArrayList; +import java.util.List; +import java.util.function.Predicate; + +import org.variantsync.diffdetective.util.Assert; +import org.variantsync.diffdetective.variation.Label; +import org.variantsync.diffdetective.variation.diff.DiffNode; +import org.variantsync.diffdetective.variation.diff.VariationDiff; + +/** + * This transformer reverts some changes in a variation diff. + * The transformation requires a predicate on {@link DiffNode nodes} to identify the changes which should be undone. + * An inserted node will be removed, so that the insertion does not happen. + * A removed node will be made unchanged so that it will not be deleted. + * When a node is made unchanged, also its parent node will be made unchanged if necessary to retain diff consistency. + * @see DiffNode#makeUnchanged() + * @author Paul Bittner + */ +public class RevertSomeChanges implements Transformer> { + private Predicate> isInteresting; + + public RevertSomeChanges(final Predicate> isInteresting) { + Assert.assertNotNull(isInteresting); + this.isInteresting = isInteresting; + } + + @Override + public void transform(VariationDiff d) { + final List> nodesToEliminate = new ArrayList<>(); + final List> toUnchanged = new ArrayList<>(); + d.forAll((DiffNode node) -> { + if (isInteresting.test(node)) { + if (node.isAdd()) { + nodesToEliminate.add(node); + } else if (node.isRem()) { + toUnchanged.add(node); + } + } + }); + + for (DiffNode zombie : nodesToEliminate) { + // We also drop all children. + zombie.drop(); + } + + for (DiffNode n : toUnchanged) { + n.makeUnchanged(); + } + } +} From 551a1b6bef5119da157455f3db9d35d0787de28d Mon Sep 17 00:00:00 2001 From: pmbittner Date: Tue, 28 Oct 2025 19:20:58 +0100 Subject: [PATCH 24/29] fix: bug + alignment in DiffNode::makeUnchanged --- .../variation/diff/DiffNode.java | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java b/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java index bcb3fe10e..73e474df2 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java @@ -908,6 +908,9 @@ public void makeUnchanged() { if (bp == null || ap == null) { // There is only one parent, which we store in this field. final DiffNode p = bp == null ? ap : bp; + final Time timeOfExistingEdge = bp == null ? AFTER : BEFORE; + final Time timeOfNewEdge = timeOfExistingEdge.other(); + Assert.assertTrue(p != null); // If the parent is not unchanged, we have to make it unchanged so that it can be our @@ -917,7 +920,29 @@ public void makeUnchanged() { } // Now make p our parent at all times, not just at a single time. - Time.forAll(t -> at(t).parent = p); + // To this end, we essentially have to "patch" this node into our parent scope at timeOfNewEdge. + // Technically, this means that we have to add this node to the children list of p at a specific index. + // We run into the alignment problem here if there is an insertion (or multiple insertions) right next to a deleted node we make unchanged or vice versa. + // Hence, the index at which to patch our node is not unique. + // There are multiple heuristics or strategies we could use to determine the index: + // - constant index: always use index 0 for example + // - line numbers: use the index right before the node with a higher line number at timeOfNewEdge + // This solution requires knowledge on line numbers which are not always present (e.g., in diffs generated in code). + // - context-based patching: Try to locate the node where its neighbors at timeOfNewEdge are most similar to the neighbors at timeOfExistingEdge + // This requires some knowledge on the labels to match contexts. + // We lightweight context-based patching here by trying to insert the node directly right of its closest unchanged left neighbor. + int patchIndex = 0; // the index at which to insert this node at timeOfNewEdge + final List> siblingsAndMe = p.getChildOrder(timeOfExistingEdge); + // We start walking from our closest left neighbor towards the leftmost sibling (at index 0) and try to find the first unchanged sibling. + for (int i = p.indexOfChild(this, timeOfExistingEdge) - 1; i >= 0; i--) { + final DiffNode candidate = siblingsAndMe.get(i); + if (candidate.isNon()) { // i.e., exists at timeOfNewEdge as well + // Insert ourselves as the new right neighbor of the candidate node + patchIndex = p.indexOfChild(candidate, timeOfNewEdge) + 1; + break; + } + } + p.insertChild(this, patchIndex, timeOfNewEdge); } } From 02804e59ace73b9de6245196288f5cfd80760370 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Tue, 28 Oct 2025 19:21:33 +0100 Subject: [PATCH 25/29] feat: IndexFormat --- .../serialize/nodeformat/IndexFormat.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 src/main/java/org/variantsync/diffdetective/variation/diff/serialize/nodeformat/IndexFormat.java diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/serialize/nodeformat/IndexFormat.java b/src/main/java/org/variantsync/diffdetective/variation/diff/serialize/nodeformat/IndexFormat.java new file mode 100644 index 000000000..e10938a96 --- /dev/null +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/serialize/nodeformat/IndexFormat.java @@ -0,0 +1,27 @@ +package org.variantsync.diffdetective.variation.diff.serialize.nodeformat; + +import java.util.ArrayList; +import java.util.List; + +import org.variantsync.diffdetective.variation.Label; +import org.variantsync.diffdetective.variation.diff.DiffNode; +import org.variantsync.diffdetective.variation.diff.Time; +import org.variantsync.functjonal.Cast; + +/** + * Labels nodes using their index in their parent list at all times. + */ +public class IndexFormat implements DiffNodeLabelFormat { + @Override + public String toLabel(final DiffNode n) { + final DiffNode node = Cast.unchecked(n); + final Time[] allTimes = Time.values(); + final List values = new ArrayList<>(allTimes.length); + for (final Time t : allTimes) { + values.add(node.getDiffType().existsAtTime(t) && !node.isRoot() + ? Integer.toString(node.getParent(t).indexOfChild(node, t)) + : "_"); + } + return "(" + String.join(", ", values) + ")"; + } +} From 6a2c9928382da2c8c58a7af069c7deda43dd8d2b Mon Sep 17 00:00:00 2001 From: pmbittner Date: Tue, 28 Oct 2025 19:21:57 +0100 Subject: [PATCH 26/29] support IndexFormat in GUI --- .../diffdetective/show/variation/VariationDiffApp.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/variantsync/diffdetective/show/variation/VariationDiffApp.java b/src/main/java/org/variantsync/diffdetective/show/variation/VariationDiffApp.java index 219e6bda8..3e5e5eb4f 100644 --- a/src/main/java/org/variantsync/diffdetective/show/variation/VariationDiffApp.java +++ b/src/main/java/org/variantsync/diffdetective/show/variation/VariationDiffApp.java @@ -48,7 +48,8 @@ public final static List> DEFAULT_FORMA new LabelOnlyDiffNodeFormat<>(), new EditClassesDiffNodeFormat<>(), new LineNumberFormat<>(), - new FormulasAndLineNumbersNodeFormat<>() + new FormulasAndLineNumbersNodeFormat<>(), + new IndexFormat<>() ); } From 75be10f896254b35ec201a9992140b7996da49d9 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 3 Nov 2025 10:41:33 +0100 Subject: [PATCH 27/29] StringUtils::getLeadingWhitespace --- .../variantsync/diffdetective/util/StringUtils.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/main/java/org/variantsync/diffdetective/util/StringUtils.java b/src/main/java/org/variantsync/diffdetective/util/StringUtils.java index 868ab08f8..8f378dd21 100644 --- a/src/main/java/org/variantsync/diffdetective/util/StringUtils.java +++ b/src/main/java/org/variantsync/diffdetective/util/StringUtils.java @@ -48,4 +48,17 @@ public static String prettyPrintNestedCollections(final Collection collection public static String clamp(int maxlen, String s) { return s.substring(0, Math.min(s.length(), maxlen)); } + + /** + * @return the longest prefix of the given string that contains only of whitespace + */ + public static String getLeadingWhitespace(String s) { + if (s == null) return null; + if (s.isEmpty()) return ""; + int i = 0; + while (i < s.length() && Character.isWhitespace(s.charAt(i))) { + ++i; + } + return s.substring(0, i); + } } From 1d227f557ee0040d87fd1dc57a52646408aa0110 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Mon, 3 Nov 2025 10:42:12 +0100 Subject: [PATCH 28/29] fix: EliminateEmptyAlt. making inconsistent labels --- .../transform/EliminateEmptyAlternatives.java | 52 ++++++++++++++++--- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java index 009e67b71..f03201235 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java +++ b/src/main/java/org/variantsync/diffdetective/variation/diff/transform/EliminateEmptyAlternatives.java @@ -1,8 +1,11 @@ - package org.variantsync.diffdetective.variation.diff.transform; import org.prop4j.Node; -import org.variantsync.diffdetective.variation.Label; +import org.prop4j.NodeWriter; +import org.variantsync.diffdetective.util.Assert; +import org.variantsync.diffdetective.util.StringUtils; +import org.variantsync.diffdetective.variation.DiffLinesLabel; +import static org.variantsync.diffdetective.variation.DiffLinesLabel.Line; import org.variantsync.diffdetective.variation.tree.VariationTree; import org.variantsync.diffdetective.variation.tree.VariationTreeNode; @@ -35,12 +38,44 @@ * * @author Paul Bittner */ -public class EliminateEmptyAlternatives implements Transformer> { - private void elim(VariationTreeNode subtree) { +public class EliminateEmptyAlternatives implements Transformer> { + /** + * Creates a copy of the given label but where the formula is set to the given formula. + * This method also updates the text in the DiffLinesLabel accordingly so that the text is + * consistent with the formula. + * This method assumes that the label has at least one line of text, otherwise the given label + * could not have a formula. + */ + private static DiffLinesLabel updatedLabel(DiffLinesLabel l, Node formula) { + final List lines = l.getDiffLines(); + Assert.assertFalse(lines.isEmpty()); + + // Assumption: + // The only case in which there is more than one line of text is, when we parsed a multiline macro. + // + // We hence may safely ignore any subsequent lines from our existing label because these correspond + // only to lines of a multiline macro, which we ought to replace anyway. + final Line head = lines.get(0); + Assert.assertTrue(head.content().contains("if")); + final String indent = StringUtils.getLeadingWhitespace(head.content()); + + final String newText = indent + "#if " + formula.toString(NodeWriter.javaSymbols); + + // We might have replaced multiple lines by a single line here. + // In this case, some line numbers got lost and any variation tree using this updated label somewhere might not + // have consecutive line numbering anymore. We could consider inserting empty lines to retain + // consecutive line numbers but that might be a more artifical change than inconsecutive line numbers. + return new DiffLinesLabel( + List.of(new Line(newText, head.lineNumber())), + l.getDiffTrailingLines() + ); + } + + private static void elim(VariationTreeNode subtree) { // We simplify only annotations. if (!subtree.isAnnotation()) return; - final List> children = subtree.getChildren(); + final List> children = subtree.getChildren(); // When there are no children, 'subtree' is an empty annotation that can be eliminated. if (children.isEmpty()) { @@ -48,7 +83,7 @@ private void elim(VariationTreeNode subtree) { } // When there is exactly one child and that child is an 'else' or 'elif' we can simplify that nesting. else if (children.size() == 1) { - final VariationTreeNode child = children.getFirst(); + final VariationTreeNode child = children.getFirst(); if ((subtree.isIf() || subtree.isElif()) && (child.isElif() || child.isElse())) { // determine new feaure mapping @@ -57,6 +92,7 @@ else if (children.size() == 1) { newFormula = and(newFormula, child.getFormula()); } subtree.setFormula(newFormula); + subtree.setLabel(updatedLabel(subtree.getLabel(), newFormula)); // simplify tree child.drop(); @@ -66,7 +102,7 @@ else if (children.size() == 1) { } @Override - public void transform(VariationTree tree) { - tree.forAllPostorder(this::elim); + public void transform(VariationTree tree) { + tree.forAllPostorder(EliminateEmptyAlternatives::elim); } } From a991942f6bd88c72e34a1f5eb9ef0bd8e88cb367 Mon Sep 17 00:00:00 2001 From: pmbittner Date: Wed, 5 Nov 2025 16:41:03 +0100 Subject: [PATCH 29/29] fix: ConfigureSpec vs new Source interface --- .../tree/view/relevance/spec/ConfigureSpec.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/spec/ConfigureSpec.java b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/spec/ConfigureSpec.java index a1d1f0fe9..c05650686 100644 --- a/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/spec/ConfigureSpec.java +++ b/src/main/java/org/variantsync/diffdetective/variation/tree/view/relevance/spec/ConfigureSpec.java @@ -1,5 +1,7 @@ package org.variantsync.diffdetective.variation.tree.view.relevance.spec; +import java.util.List; + import org.prop4j.Node; import org.prop4j.NodeWriter; import org.variantsync.diffdetective.analysis.logic.SAT; @@ -41,13 +43,13 @@ public boolean test(VariationNode t) { } @Override - public String getFunctionName() { - return "configure_spec"; + public List getSourceArguments() { + return List.of(config.get().toString(NodeWriter.logicalSymbols)); } @Override - public String parametersToString() { - return config.get().toString(NodeWriter.logicalSymbols); + public String getSourceExplanation() { + return "configure_spec"; } @Override