Skip to content

Commit de10f75

Browse files
committed
refactor: store endifs in the label
This is necessary because when `endif`s are fully integrated (included in deep copies, assertConsistency and comparisons) some assumptions are broken (which fields are compare by `DiffNode`s) which is especially annoying in `BadVDiff`. By storing the `endif`s in the label instead of the nodes directly, no user needs to be updated except if the label is actually modified (there seems to be no instance of that in this code base). This also generalized the concept to trailing lines instead of limiting the concept to endifs. This is also kind of necessary because not all labels know the node type. Depending on the user of `VariationDiffParser`, this might be a breaking change because we now consider the `#endif` line as part of the label. Hence, if the `#endif` line changed (e.g., a comment is added) the node can no longer be `DiffType.NON` but we instead split the node into two `DiffType.REM` and `DiffType.ADD` nodes. Artifacts which are below such a split node are thus classified as refactored instead of unchanged.
1 parent c860a88 commit de10f75

10 files changed

Lines changed: 171 additions & 81 deletions

File tree

src/main/java/org/variantsync/diffdetective/variation/DiffLinesLabel.java

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import java.util.stream.Collectors;
77

88
import org.variantsync.diffdetective.diff.text.DiffLineNumber;
9+
import org.variantsync.diffdetective.util.Assert;
910
import org.variantsync.diffdetective.util.StringUtils;
1011
import org.variantsync.diffdetective.variation.diff.VariationDiff; // For Javadoc
1112

@@ -18,6 +19,7 @@
1819
*/
1920
public class DiffLinesLabel implements Label {
2021
private final List<Line> lines;
22+
private List<Line> trailingLines;
2123

2224
public record Line(String content, DiffLineNumber lineNumber) {
2325
public static Line withInvalidLineNumber(String content) {
@@ -30,7 +32,15 @@ public DiffLinesLabel() {
3032
}
3133

3234
public DiffLinesLabel(List<Line> lines) {
35+
this(lines, new ArrayList<>());
36+
}
37+
38+
public DiffLinesLabel(List<Line> lines, List<Line> trailingLines) {
39+
Assert.assertNotNull(lines);
40+
Assert.assertNotNull(trailingLines);
41+
3342
this.lines = lines;
43+
this.trailingLines = trailingLines;
3444
}
3545

3646
public static DiffLinesLabel withInvalidLineNumbers(List<String> lines) {
@@ -53,11 +63,25 @@ public List<Line> getDiffLines() {
5363
return lines;
5464
}
5565

66+
public void setDiffTrailingLines(List<Line> newLines) {
67+
Assert.assertNotNull(newLines);
68+
trailingLines = newLines;
69+
}
70+
71+
public List<Line> getDiffTrailingLines() {
72+
return trailingLines;
73+
}
74+
5675
@Override
5776
public List<String> getLines() {
5877
return getDiffLines().stream().map(Line::content).toList();
5978
}
6079

80+
@Override
81+
public List<String> getTrailingLines() {
82+
return getDiffTrailingLines().stream().map(Line::content).toList();
83+
}
84+
6185
@Override
6286
public String toString() {
6387
return lines
@@ -68,6 +92,15 @@ public String toString() {
6892

6993
@Override
7094
public DiffLinesLabel clone() {
71-
return new DiffLinesLabel(new ArrayList<>(lines));
95+
return new DiffLinesLabel(new ArrayList<>(lines), new ArrayList<>(trailingLines));
96+
}
97+
98+
@Override
99+
public boolean equals(Object o) {
100+
if (this == o) return true;
101+
if (o == null || getClass() != o.getClass()) return false;
102+
DiffLinesLabel that = (DiffLinesLabel) o;
103+
return lines.equals(that.lines) &&
104+
trailingLines.equals(that.trailingLines);
72105
}
73106
}

src/main/java/org/variantsync/diffdetective/variation/Label.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@
99
* Base interface for labels of {@link VariationTree}s and {@link VariationDiff}s.
1010
*/
1111
public interface Label {
12+
/**
13+
* Returns the lines which need to be printed before the children of a node.
14+
* Most lines associated with a node should be included in this list, for example, {@code #if}s are stored here.
15+
*/
1216
List<String> getLines();
17+
/**
18+
* Returns the lines which need to be printed after the children of a node.
19+
* For example, {@code #endif}s are stored as trailing lines.
20+
*/
21+
List<String> getTrailingLines();
22+
/**
23+
* Creates a deep copy of this label.
24+
*/
1325
Label clone();
1426
}

src/main/java/org/variantsync/diffdetective/variation/LinesLabel.java

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,30 +5,46 @@
55
import java.util.List;
66
import java.util.stream.Collectors;
77

8+
import org.variantsync.diffdetective.util.Assert;
89
import org.variantsync.diffdetective.util.StringUtils;
910

1011
/**
1112
* A label containing a list of lines represented as {@code String}s.
1213
*/
1314
public class LinesLabel implements Label {
1415
private final List<String> lines;
16+
private final List<String> trailingLines;
1517

1618
public LinesLabel() {
17-
this(new ArrayList<>());
19+
this(new ArrayList<>(), new ArrayList<>());
1820
}
1921

2022
public LinesLabel(List<String> lines) {
23+
this(lines, new ArrayList<>());
24+
}
25+
26+
public LinesLabel(List<String> lines, List<String> trailingLines) {
27+
Assert.assertNotNull(lines);
28+
Assert.assertNotNull(trailingLines);
29+
2130
this.lines = lines;
31+
this.trailingLines = trailingLines;
2232
}
2333

2434
public static LinesLabel ofCodeBlock(String codeBlock) {
2535
return new LinesLabel(Arrays.asList(StringUtils.LINEBREAK_REGEX.split(codeBlock, -1)));
2636
}
2737

38+
@Override
2839
public List<String> getLines() {
2940
return lines;
3041
}
3142

43+
@Override
44+
public List<String> getTrailingLines() {
45+
return trailingLines;
46+
}
47+
3248
@Override
3349
public String toString() {
3450
return lines
@@ -38,6 +54,15 @@ public String toString() {
3854

3955
@Override
4056
public LinesLabel clone() {
41-
return new LinesLabel(new ArrayList<>(lines));
57+
return new LinesLabel(new ArrayList<>(lines), new ArrayList<>(trailingLines));
58+
}
59+
60+
@Override
61+
public boolean equals(Object o) {
62+
if (this == o) return true;
63+
if (o == null || getClass() != o.getClass()) return false;
64+
LinesLabel that = (LinesLabel) o;
65+
return lines.equals(that.lines) &&
66+
trailingLines.equals(that.trailingLines);
4267
}
4368
}

src/main/java/org/variantsync/diffdetective/variation/VariationLabel.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.util.List;
44

5+
import org.variantsync.diffdetective.util.Assert;
56
import org.variantsync.diffdetective.variation.tree.HasNodeType;
67
import org.variantsync.diffdetective.variation.tree.VariationTree; // For Javadoc
78
import org.variantsync.functjonal.Cast;
@@ -23,6 +24,9 @@ public class VariationLabel<L extends Label> implements Label, HasNodeType {
2324
private L innerLabel;
2425

2526
public VariationLabel(NodeType type, L innerLabel) {
27+
Assert.assertNotNull(type);
28+
Assert.assertNotNull(innerLabel);
29+
2630
this.type = type;
2731
this.innerLabel = innerLabel;
2832
}
@@ -40,6 +44,11 @@ public List<String> getLines() {
4044
return innerLabel.getLines();
4145
}
4246

47+
@Override
48+
public List<String> getTrailingLines() {
49+
return innerLabel.getTrailingLines();
50+
}
51+
4352
@Override
4453
public NodeType getNodeType() {
4554
return type;
@@ -49,9 +58,18 @@ public NodeType getNodeType() {
4958
public VariationLabel<L> clone() {
5059
return new VariationLabel<L>(type, Cast.unchecked(innerLabel.clone()));
5160
}
52-
61+
5362
@Override
5463
public String toString() {
55-
return innerLabel.toString();
64+
return innerLabel.toString();
65+
}
66+
67+
@Override
68+
public boolean equals(Object o) {
69+
if (this == o) return true;
70+
if (o == null || getClass() != o.getClass()) return false;
71+
VariationLabel<?> that = (VariationLabel<?>) o;
72+
return type.equals(that.type) &&
73+
innerLabel.equals(that.innerLabel);
5674
}
5775
}

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

Lines changed: 60 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@ public class DiffNode<L extends Label> implements HasNodeType {
5151

5252
private Node featureMapping;
5353

54-
private List<String>[] endIf = new List[2];
55-
5654
/**
5755
* The parents {@link DiffNode} before and after the edit.
5856
* This array has to be indexed by {@code Time.ordinal()}
@@ -135,8 +133,6 @@ public static <L extends Label> DiffNode<L> createArtifact(DiffType diffType, Di
135133

136134
/**
137135
* Returns the lines in the diff that are represented by this DiffNode as a single text.
138-
*
139-
* @see getEndIf
140136
*/
141137
public L getLabel() {
142138
return label.getInnerLabel();
@@ -150,24 +146,6 @@ public void setLabel(L newLabel) {
150146
label.setInnerLabel(newLabel);
151147
}
152148

153-
/**
154-
* Returns the line with the endif of the corresponding if, if the node is an if node, otherwise null
155-
* @param time when the returned endif exists
156-
* @return String, the Line with endif
157-
*/
158-
public List<String> getEndIf(Time time) {
159-
return endIf[time.ordinal()];
160-
}
161-
162-
/**
163-
* Sets the line with the endif of the corresponding if, if the node is an if node
164-
* @param endIf String, the Line with endif
165-
* @param time when {@code endIf} exists
166-
*/
167-
public void setEndIf(List<String> endIf, Time time) {
168-
this.endIf[time.ordinal()] = endIf;
169-
}
170-
171149
/**
172150
* Gets the first {@code if} node in the path from the root to this node at the time
173151
* {@code time}.
@@ -367,6 +345,64 @@ public void stealChildrenOf(final DiffNode<L> other) {
367345
Time.forAll(time -> addChildren(other.removeChildren(time), time));
368346
}
369347

348+
/**
349+
* Splits an {@link isNon unmodified} node into a removed and an added node.
350+
* Only one new node is created. Depending on {@code time}, {@code this} is changed into
351+
* {@link DiffType#ADD} for {@link Time#BEFORE} or {@link DiffType#REM} for {@link Time#AFTER}.
352+
*
353+
* @param time decides which node is created
354+
* @return the new node that exists at time {@code time}.
355+
*/
356+
public DiffNode<L> split(Time time) {
357+
Assert.assertTrue(isNon());
358+
359+
DiffType otherDiffType = DiffType.thatExistsOnlyAt(time);
360+
var other = new DiffNode<L>(
361+
otherDiffType,
362+
getNodeType(),
363+
getFromLine().as(otherDiffType),
364+
getToLine().as(otherDiffType),
365+
getFormula(),
366+
Cast.unchecked(label.getInnerLabel().clone())
367+
);
368+
369+
this.diffType = otherDiffType.inverse();
370+
this.from = this.from.as(this.diffType);
371+
this.to = this.to.as(this.diffType);
372+
373+
other.addChildren(this.removeChildren(time), time);
374+
getParent(time).replaceChild(this, other, time);
375+
376+
// FIXME? If we allowed access to the backing node,
377+
// we could preserve the identity of this projection by moving it to `other`.
378+
this.projections[time.ordinal()] = null;
379+
380+
return other;
381+
}
382+
383+
/**
384+
* Replaces a child of this node with another node.
385+
* <p>
386+
* If {@code oldChild} is not a child of this node, nothing happens.
387+
*
388+
* @param oldChild the child that is removed
389+
* @param newChild the child that is inserted
390+
* @param time at which the child relation is changed
391+
*/
392+
public void replaceChild(DiffNode<L> oldChild, DiffNode<L> newChild, Time time) {
393+
Assert.assertNull(newChild.getParent(time));
394+
Assert.assertTrue(newChild.getDiffType().existsAtTime(time));
395+
396+
for (ListIterator<DiffNode<L>> it = children[time.ordinal()].listIterator(); it.hasNext(); ) {
397+
if (it.next() == oldChild) {
398+
it.set(newChild);
399+
newChild.parents[time.ordinal()] = oldChild.parents[time.ordinal()];
400+
oldChild.parents[time.ordinal()] = null;
401+
break;
402+
}
403+
}
404+
}
405+
370406
/**
371407
* Returns the parent of this node before or after the edit.
372408
*/
@@ -775,7 +811,7 @@ public DiffNode<L> shallowCopy() {
775811
getFromLine(),
776812
getToLine(),
777813
getFormula(),
778-
Cast.unchecked(label.clone())
814+
Cast.unchecked(label.getInnerLabel().clone())
779815
);
780816
}
781817

@@ -808,7 +844,7 @@ private static <L extends Label> boolean isSameAs(DiffNode<L> a, DiffNode<L> b,
808844
a.getNodeType().equals(b.getNodeType()) &&
809845
a.getFromLine().equals(b.getFromLine()) &&
810846
a.getToLine().equals(b.getToLine()) &&
811-
(a.getFormula() == null ? b.getFormula() == null : a.getFormula().equals(b.getFormula())) &&
847+
Objects.equals(a.getFormula(), b.getFormula()) &&
812848
a.getLabel().equals(b.getLabel())
813849
)) {
814850
return false;

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,4 @@ public Node getFormula() {
121121
public int getID() {
122122
return getBackingNode().getID();
123123
}
124-
125-
@Override
126-
public List<String> getEndIf() {
127-
return getBackingNode().getEndIf(getTime());
128-
}
129124
};

src/main/java/org/variantsync/diffdetective/variation/diff/parse/VariationDiffParser.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package org.variantsync.diffdetective.variation.diff.parse;
22

3-
import java.util.ArrayList;
4-
import java.util.List;
53
import org.apache.commons.lang3.function.FailableSupplier;
64
import org.eclipse.jgit.api.Git;
75
import org.eclipse.jgit.lib.ObjectId;
@@ -379,11 +377,15 @@ private void popIfChain(
379377

380378
// Save endif
381379
if (annotation.isIf()) {
382-
List<String> list = new ArrayList<>();
383-
for (int i = 0; i < line.getLines().size(); i++) {
384-
list.add(line.getLines().get(i).content());
380+
var endIf = line.getLines();
381+
var otherEndIf = annotation.getLabel().getDiffTrailingLines();
382+
383+
// Split the node if two different endif lines are associated to one if node.
384+
if (!otherEndIf.isEmpty() && !endIf.equals(otherEndIf)) {
385+
annotation = annotation.split(time);
385386
}
386-
annotation.setEndIf(list, time);
387+
388+
annotation.getLabel().setDiffTrailingLines(endIf);
387389
}
388390

389391
// Set the line number of now closed annotations to the beginning of the

0 commit comments

Comments
 (0)