Merge pull request #174 from VariantSync/fix-configure

Fix a crucial bug in the Configure relevance predicate

Author: pmbittner

src/main/java/org/variantsync/diffdetective/analysis/PreprocessingAnalysis.java

@@ -3,24 +3,25 @@
 import java.util.Arrays;
 import java.util.List;
 
-import org.variantsync.diffdetective.variation.diff.transform.VariationDiffTransformer;
+import org.variantsync.diffdetective.variation.diff.VariationDiff;
+import org.variantsync.diffdetective.variation.diff.transform.Transformer;
 import org.variantsync.diffdetective.variation.DiffLinesLabel;
 
 public class PreprocessingAnalysis implements Analysis.Hooks {
-    private final List<VariationDiffTransformer<DiffLinesLabel>> preprocessors;
+    private final List<Transformer<VariationDiff<DiffLinesLabel>>> preprocessors;
 
-    public PreprocessingAnalysis(List<VariationDiffTransformer<DiffLinesLabel>> preprocessors) {
+    public PreprocessingAnalysis(List<Transformer<VariationDiff<DiffLinesLabel>>> preprocessors) {
         this.preprocessors = preprocessors;
     }
 
     @SafeVarargs
-    public PreprocessingAnalysis(VariationDiffTransformer<DiffLinesLabel>... preprocessors) {
+    public PreprocessingAnalysis(Transformer<VariationDiff<DiffLinesLabel>>... preprocessors) {
         this.preprocessors = Arrays.asList(preprocessors);
     }
 
     @Override
     public boolean analyzeVariationDiff(Analysis analysis) {
-        VariationDiffTransformer.apply(preprocessors, analysis.getCurrentVariationDiff());
+        Transformer.apply(preprocessors, analysis.getCurrentVariationDiff());
         analysis.getCurrentVariationDiff().assertConsistency();
         return true;
     }

src/main/java/org/variantsync/diffdetective/internal/SimpleRenderer.java

@@ -18,7 +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.VariationDiffTransformer;
+import org.variantsync.diffdetective.variation.diff.transform.Transformer;
 
 import java.io.IOException;
 import java.nio.file.Files;
@@ -162,10 +162,10 @@ 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<VariationDiffTransformer<DiffLinesLabel>> transform = VariationDiffMiner.Postprocessing(repository);
+            final List<Transformer<VariationDiff<DiffLinesLabel>>> 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);
         }
 

src/main/java/org/variantsync/diffdetective/mining/VariationDiffMiner.java

@@ -13,14 +13,15 @@
 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;
 import org.variantsync.diffdetective.variation.diff.serialize.edgeformat.EdgeLabelFormat;
 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<VariationDiffTransformer<DiffLinesLabel>> Postprocessing(final Repository repository) {
-        final List<VariationDiffTransformer<DiffLinesLabel>> processing = new ArrayList<>();
+    public static List<Transformer<VariationDiff<DiffLinesLabel>>> Postprocessing(final Repository repository) {
+        final List<Transformer<VariationDiff<DiffLinesLabel>>> processing = new ArrayList<>();
         processing.add(new CutNonEditedSubtrees<>());
         processing.add(new CollapseNestedNonEditedAnnotations());
         return processing;

src/main/java/org/variantsync/diffdetective/mining/postprocessing/Postprocessor.java

@@ -7,7 +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.VariationDiffTransformer;
+import org.variantsync.diffdetective.variation.diff.transform.Transformer;
 
 import java.util.List;
 import java.util.Map;
@@ -17,7 +17,7 @@
  * Patterns are represented as VariationDiffs and might be filtered or transformed.
  */
 public class Postprocessor<L extends Label> {
-    private final List<VariationDiffTransformer<L>> transformers;
+    private final List<Transformer<VariationDiff<L>>> transformers;
     private final ExplainedFilter<VariationDiff<L>> filters;
 
     /**
@@ -30,7 +30,7 @@ public class Postprocessor<L extends Label> {
     public record Result<L extends Label>(List<VariationDiff<L>> processedTrees, Map<String, Integer> filterCounts) {}
 
     private Postprocessor(
-            final List<VariationDiffTransformer<L>> transformers,
+            final List<Transformer<VariationDiff<L>>> transformers,
             final List<TaggedPredicate<String, ? super VariationDiff<L>>> namedFilters) {
         this.transformers = transformers;
         this.filters = new ExplainedFilter<VariationDiff<L>>(namedFilters.stream());
@@ -66,7 +66,7 @@ public static <L extends Label> Postprocessor<L> Default() {
     public Result<L> postprocess(final List<VariationDiff<L>> frequentSubgraphs) {
         final List<VariationDiff<L>> processedTrees = frequentSubgraphs.stream()
                 .filter(filters)
-                .peek(tree -> VariationDiffTransformer.apply(transformers, tree))
+                .peek(tree -> Transformer.apply(transformers, tree))
                 .toList();
 
         final Map<String, Integer> filterCounts = new ExplainedFilterSummary(filters).snapshot();

src/main/java/org/variantsync/diffdetective/show/variation/VariationDiffApp.java

@@ -48,7 +48,8 @@ public final static <L extends Label> List<DiffNodeLabelFormat<L>> DEFAULT_FORMA
             new LabelOnlyDiffNodeFormat<>(),
             new EditClassesDiffNodeFormat<>(),
             new LineNumberFormat<>(),
-            new FormulasAndLineNumbersNodeFormat<>()
+            new FormulasAndLineNumbersNodeFormat<>(),
+            new IndexFormat<>()
         );
     }
 

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);
+    }
 }

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());
+    }
 }

src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java

@@ -367,12 +367,21 @@ public List<DiffNode<L>> 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<L> 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<L> other) {
-        Time.forAll(time -> addChildren(other.removeChildren(time), time));
+        Time.forAll(time -> stealChildrenOf(time, other));
     }
 
     /**
@@ -870,6 +879,65 @@ public DiffNode<L> 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<L> bp = at(Time.BEFORE).parent;
+        final DiffNode<L> 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<L> 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
+            // parent at all times.
+            if (!p.isNon()) {
+                p.makeUnchanged();
+            }
+
+            // Now make p our parent at all times, not just at a single time.
+            // 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<DiffNode<L>> 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<L> 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);
+        }
+    }
+
     /**
      * Transforms a {@code VariationNode} into a {@code DiffNode} by diffing {@code variationNode}
      * to itself. Recursively translates all children.
@@ -916,6 +984,42 @@ private static <L extends Label> boolean isSameAs(DiffNode<L> a, DiffNode<L> 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<L> other) {
+        return isSameAsIgnoringLineNumbers(this, other, new HashSet<>());
+    }
+
+    private static <L extends Label> boolean isSameAsIgnoringLineNumbers(DiffNode<L> a, DiffNode<L> b, Set<DiffNode<L>> 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<DiffNode<L>> aIt = a.getAllChildren().iterator();
+        Iterator<DiffNode<L>> 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;

src/main/java/org/variantsync/diffdetective/variation/diff/VariationDiff.java

@@ -542,10 +542,20 @@ public ConsistencyResult isConsistent() {
         return ConsistencyResult.Success();
     }
 
+    /**
+     * @see DiffNode#isSameAs
+     */
     public boolean isSameAs(VariationDiff<L> b) {
         return getRoot().isSameAs(b.getRoot());
     }
 
+    /**
+     * @see DiffNode#isSameAsIgnoringLineNumbers
+     */
+    public boolean isSameAsIgnoringLineNumbers(VariationDiff<L> b) {
+        return getRoot().isSameAsIgnoringLineNumbers(b.getRoot());
+    }
+
     @Override
     public String toString() {
         return "VariationDiff of " + source;

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<L extends Label> implements DiffNodeLabelFormat<L> {
+    @Override
+    public String toLabel(final DiffNode<? extends L> n) {
+        final DiffNode<L> node = Cast.unchecked(n);
+        final Time[] allTimes = Time.values();
+        final List<String> 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) + ")";
+    }
+}