Skip to content

Commit 3f44bac

Browse files
ibbemibbem
committed
Remove NodeType.ENDIF
`NodeType.ENDIF` was only needed during parsing. Now, there are no more temporary nodes representing `endif` macros during parsing, instead endif macros are noticed earlier so no further parsing is performed. All serialisations using the ordinal of `NodeType.ENDIF` are changed in a backwards incompatible way. Most notably, the line graph format uses `DiffNode.getID` which encodes the `NodeType`. (This is the reason why many test cases changed).
1 parent b590f33 commit 3f44bac

12 files changed

Lines changed: 3705 additions & 3702 deletions

File tree

‎src/main/java/org/variantsync/diffdetective/diff/difftree/DiffNode.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -950,15 +950,6 @@ public boolean isArtifact() {
950950
return this.nodeType.equals(NodeType.ARTIFACT);
951951
}
952952

953-
/**
954-
* Returns true if this node represents the end of an annotation block.
955-
* Such a node should not be part of any {@link DiffTree}.
956-
* @see NodeType#ENDIF
957-
*/
958-
public boolean isEndif() {
959-
return this.nodeType.equals(NodeType.ENDIF);
960-
}
961-
962953
/**
963954
* Returns true if this node represents an ELSE annotation.
964955
* @see NodeType#ELSE
@@ -1117,7 +1108,7 @@ public String toTextDiff() {
11171108
// Add endif after macro
11181109
if (isAnnotation() && !isRoot()) {
11191110
diff
1120-
.append(toTextDiffLine(this.diffType, List.of(NodeType.ENDIF.asMacroText())))
1111+
.append(toTextDiffLine(this.diffType, List.of("#endif")))
11211112
.append(StringUtils.LINEBREAK);
11221113
}
11231114

‎src/main/java/org/variantsync/diffdetective/diff/difftree/NodeType.java‎

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

3-
import java.util.regex.Matcher;
4-
import java.util.regex.Pattern;
3+
import org.variantsync.diffdetective.diff.difftree.parse.MultiLineMacroParser;
54

65
/**
76
* The type of nodes in a {@link DiffTree}.
@@ -10,7 +9,6 @@
109
public enum NodeType {
1110
// Mapping types
1211
IF("if"),
13-
ENDIF("endif"),
1412
ELSE("else"),
1513
ELIF("elif"),
1614

@@ -36,24 +34,19 @@ public boolean isAnnotation() {
3634
return this != ARTIFACT;
3735
}
3836

39-
final static Pattern annotationRegex = Pattern.compile("^[+-]?\\s*#\\s*(if|endif|else|elif)");
40-
4137
/**
4238
* Parses the node type from a line taken from a text-based diff.
4339
* @param line A line in a patch.
4440
* @return The type of edit of <code>line</code>.
4541
*/
4642
public static NodeType ofDiffLine(String line) {
47-
Matcher matcher = annotationRegex.matcher(line);
48-
if (matcher.find()) {
49-
String id = matcher.group(1);
50-
if (id.equals(IF.name)) {
43+
String macro = MultiLineMacroParser.conditionalMacroName(line);
44+
if (macro != null) {
45+
if (macro.equals(IF.name)) {
5146
return IF;
52-
} else if (id.equals(ENDIF.name)) {
53-
return ENDIF;
54-
} else if (id.equals(ELSE.name)) {
47+
} else if (macro.equals(ELSE.name)) {
5548
return ELSE;
56-
} else if (id.equals(ELIF.name)) {
49+
} else if (macro.equals(ELIF.name)) {
5750
return ELIF;
5851
}
5952
}
@@ -76,11 +69,4 @@ public static NodeType fromName(final String name) {
7669

7770
throw new IllegalArgumentException("Given string \"" + name + "\" is not the name of a NodeType.");
7871
}
79-
80-
/**
81-
* Prints this value as a macro annotation (i.e., starting with #).
82-
*/
83-
public String asMacroText() {
84-
return "#" + this.name;
85-
}
8672
}

‎src/main/java/org/variantsync/diffdetective/diff/difftree/parse/DiffNodeParser.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public DiffNode fromDiffLine(String diffLine) throws IllFormedAnnotationExceptio
3131
String label = diffLine.isEmpty() ? diffLine : diffLine.substring(1);
3232
Node featureMapping;
3333

34-
if (nodeType == NodeType.ARTIFACT || nodeType == NodeType.ENDIF || nodeType == NodeType.ELSE) {
34+
if (nodeType == NodeType.ARTIFACT || nodeType == NodeType.ELSE) {
3535
featureMapping = null;
3636
} else {
3737
featureMapping = annotationParser.parseDiffLine(diffLine);

‎src/main/java/org/variantsync/diffdetective/diff/difftree/parse/DiffTreeParser.java‎

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -129,34 +129,11 @@ public static DiffResult<DiffTree> createDiffTree(
129129
case NotMyDuty: break;
130130
}
131131

132-
// This gets the node type and diff type of the current line and creates a node
133-
// Note that the node is not yet added to the diff tree.
134-
final DiffNode newNode;
135-
try {
136-
newNode = nodeParser.fromDiffLine(currentLine);
137-
} catch (IllFormedAnnotationException e) {
138-
return DiffResult.Failure(e);
139-
}
140-
141-
// collapse multiple code lines
142-
if (lastArtifact != null) {
143-
if (collapseMultipleCodeLines && newNode.isArtifact() && lastArtifact.diffType.equals(newNode.diffType)) {
144-
lastArtifact.addLines(newNode.getLines());
145-
continue;
146-
} else {
132+
if ("endif".equals(MultiLineMacroParser.conditionalMacroName(currentLine))) {
133+
if (lastArtifact != null) {
147134
lastArtifact = endCodeBlock(lastArtifact, lastLineNo);
148135
}
149-
}
150-
151-
newNode.getFromLine().set(lineNo);
152-
if (!newNode.isEndif()) {
153-
newNode.addBelow(beforeStack.peek(), afterStack.peek());
154-
nodes.add(newNode);
155-
}
156136

157-
if (newNode.isArtifact()) {
158-
lastArtifact = newNode;
159-
} else if (newNode.isEndif()) {
160137
final String currentLineFinal = currentLine;
161138
diffType.matchBeforeAfter(beforeStack, afterStack,
162139
stack -> {
@@ -174,12 +151,39 @@ public static DiffResult<DiffTree> createDiffTree(
174151
});
175152
if (error.get() != null) { return error.get(); }
176153
} else {
177-
// newNode is if, elif or else
178-
// push the node to the relevant stacks
179-
diffType.matchBeforeAfter(beforeStack, afterStack, stack ->
180-
pushNodeToStack(newNode, stack, lastLineNo).onError(errorPropagation)
181-
);
182-
if (error.get() != null) { return error.get(); }
154+
// This gets the node type and diff type of the current line and creates a node
155+
// Note that the node is not yet added to the diff tree.
156+
final DiffNode newNode;
157+
try {
158+
newNode = nodeParser.fromDiffLine(currentLine);
159+
} catch (IllFormedAnnotationException e) {
160+
return DiffResult.Failure(e);
161+
}
162+
163+
// collapse multiple code lines
164+
if (lastArtifact != null) {
165+
if (collapseMultipleCodeLines && newNode.isArtifact() && lastArtifact.diffType.equals(newNode.diffType)) {
166+
lastArtifact.addLines(newNode.getLines());
167+
continue;
168+
} else {
169+
lastArtifact = endCodeBlock(lastArtifact, lastLineNo);
170+
}
171+
}
172+
173+
newNode.getFromLine().set(lineNo);
174+
newNode.addBelow(beforeStack.peek(), afterStack.peek());
175+
nodes.add(newNode);
176+
177+
if (newNode.isArtifact()) {
178+
lastArtifact = newNode;
179+
} else {
180+
// newNode is if, elif or else
181+
// push the node to the relevant stacks
182+
diffType.matchBeforeAfter(beforeStack, afterStack, stack ->
183+
pushNodeToStack(newNode, stack, lastLineNo).onError(errorPropagation)
184+
);
185+
if (error.get() != null) { return error.get(); }
186+
}
183187
}
184188
}
185189

‎src/main/java/org/variantsync/diffdetective/diff/difftree/parse/MultiLineMacroParser.java‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package org.variantsync.diffdetective.diff.difftree.parse;
22

33
import org.variantsync.diffdetective.diff.DiffLineNumber;
4-
import org.variantsync.diffdetective.diff.difftree.NodeType;
54
import org.variantsync.diffdetective.diff.difftree.DiffNode;
65
import org.variantsync.diffdetective.diff.difftree.DiffType;
76

87
import java.io.BufferedReader;
98
import java.util.List;
109
import java.util.Stack;
10+
import java.util.regex.Pattern;
1111

1212
import static org.variantsync.diffdetective.diff.result.DiffError.MLMACRO_WITHIN_MLMACRO;
1313

@@ -16,6 +16,11 @@
1616
* @author Paul Bittner
1717
*/
1818
public class MultiLineMacroParser {
19+
/** Matches conditional macros. Note that it doesn't match the whole line or even the whole
20+
* macro name. For example {@code #ifdef} is also matched, but only {@code "if"} is captured.
21+
*/
22+
private final static Pattern macroPattern = Pattern.compile("^[+-]?\\s*#\\s*(if|elif|else|endif)");
23+
1924
private final DiffNodeParser nodeParser;
2025

2126
private MultilineMacro beforeMLMacro = null;
@@ -81,8 +86,7 @@ ParseResult consume(
8186

8287
if (continuesMultilineDefinition(line)) {
8388
// If this multiline macro line is a header...
84-
final NodeType nodeType = NodeType.ofDiffLine(line);
85-
if (nodeType.isConditionalAnnotation()) {
89+
if (conditionalMacroName(line) != null) {
8690
// ... create a new multi line macro to complete.
8791
if (!isAdd) {
8892
if (beforeMLMacro != null) {
@@ -200,4 +204,23 @@ ParseResult consume(
200204
public static boolean continuesMultilineDefinition(String line) {
201205
return line.trim().endsWith("\\");
202206
}
207+
208+
/**
209+
* Returns the shortened name of a conditional macro.
210+
*
211+
* Shortened means it's one of {@code if}, {@code elif}, {@code else} or {@code endif},
212+
* although the actual macro name may be longer (for example {@code ifdef}).
213+
*
214+
* @param line the first line of the macro
215+
* @return the shortened name of a conditional macro in {@code line} or {@code null} if there
216+
* is no conditional macro on {@code line}
217+
*/
218+
public static String conditionalMacroName(String line) {
219+
var matcher = macroPattern.matcher(line);
220+
if (matcher.find()) {
221+
return matcher.group(1);
222+
} else {
223+
return null;
224+
}
225+
}
203226
}

‎src/main/java/org/variantsync/diffdetective/diff/difftree/transform/CollapseNestedNonEditedAnnotations.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ private static void collapseChain(Stack<DiffNode> chain) {
115115
}
116116
case ARTIFACT ->
117117
throw new RuntimeException("Unexpected node type " + lastPopped.nodeType + " within annotation chain!");
118-
case ENDIF -> {}
119118
}
120119
}
121120

‎src/test/java/TestLineNumbers.java‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,37 +23,37 @@ public void initTestCases() {
2323

2424
final var elifchain_map = new HashMap<Integer, Pair<DiffLineNumber, DiffLineNumber>>();
2525
elifchain_map.put(144, new Pair<>(new DiffLineNumber(1, 1, 1), new DiffLineNumber(11, 9, 10)));
26-
elifchain_map.put(148, new Pair<>(new DiffLineNumber(1, 1, 1), new DiffLineNumber(2, 2, 2)));
26+
elifchain_map.put(147, new Pair<>(new DiffLineNumber(1, 1, 1), new DiffLineNumber(2, 2, 2)));
2727
elifchain_map.put(208, new Pair<>(new DiffLineNumber(2, 2, 2), new DiffLineNumber(4, 4, 4)));
28-
elifchain_map.put(276, new Pair<>(new DiffLineNumber(3, 3, 3), new DiffLineNumber(4, 4, 4)));
29-
elifchain_map.put(339, new Pair<>(new DiffLineNumber(4, 4, 4), new DiffLineNumber(8, 6, 6)));
30-
elifchain_map.put(404, new Pair<>(new DiffLineNumber(5, 5, 5), new DiffLineNumber(6, 6, 6)));
31-
elifchain_map.put(451, new Pair<>(new DiffLineNumber(6, -1, 6), new DiffLineNumber(10, -1, 9)));
32-
elifchain_map.put(516, new Pair<>(new DiffLineNumber(7, -1, 7), new DiffLineNumber(8, -1, 8)));
33-
elifchain_map.put(660, new Pair<>(new DiffLineNumber(9, 7, 8), new DiffLineNumber(10, 8, 9)));
34-
elifchain_map.put(587, new Pair<>(new DiffLineNumber(8, 6, -1), new DiffLineNumber(10, 8, -1)));
28+
elifchain_map.put(275, new Pair<>(new DiffLineNumber(3, 3, 3), new DiffLineNumber(4, 4, 4)));
29+
elifchain_map.put(338, new Pair<>(new DiffLineNumber(4, 4, 4), new DiffLineNumber(8, 6, 6)));
30+
elifchain_map.put(403, new Pair<>(new DiffLineNumber(5, 5, 5), new DiffLineNumber(6, 6, 6)));
31+
elifchain_map.put(450, new Pair<>(new DiffLineNumber(6, -1, 6), new DiffLineNumber(10, -1, 9)));
32+
elifchain_map.put(515, new Pair<>(new DiffLineNumber(7, -1, 7), new DiffLineNumber(8, -1, 8)));
33+
elifchain_map.put(659, new Pair<>(new DiffLineNumber(9, 7, 8), new DiffLineNumber(10, 8, 9)));
34+
elifchain_map.put(586, new Pair<>(new DiffLineNumber(8, 6, -1), new DiffLineNumber(10, 8, -1)));
3535
TestCase elifchain = new TestCase("elifchain.txt", elifchain_map);
3636

3737
final var lineno1_map = new HashMap<Integer, Pair<DiffLineNumber, DiffLineNumber>>();
3838
lineno1_map.put(144, new Pair<>(new DiffLineNumber(1, 1, 1), new DiffLineNumber(17, 14, 12)));
39-
lineno1_map.put(148, new Pair<>(new DiffLineNumber(1, 1, 1), new DiffLineNumber(2, 2, 2)));
39+
lineno1_map.put(147, new Pair<>(new DiffLineNumber(1, 1, 1), new DiffLineNumber(2, 2, 2)));
4040
lineno1_map.put(208, new Pair<>(new DiffLineNumber(2, 2, 2), new DiffLineNumber(4, 4, 4)));
41-
lineno1_map.put(276, new Pair<>(new DiffLineNumber(3, 3, 3), new DiffLineNumber(4, 4, 4)));
42-
lineno1_map.put(404, new Pair<>(new DiffLineNumber(5, 5, 5), new DiffLineNumber(6, 6, 6)));
43-
lineno1_map.put(452, new Pair<>(new DiffLineNumber(6, -1, 6), new DiffLineNumber(7, -1, 7)));
44-
lineno1_map.put(524, new Pair<>(new DiffLineNumber(7, 6, -1), new DiffLineNumber(8, 7, -1)));
45-
lineno1_map.put(596, new Pair<>(new DiffLineNumber(8, 7, 7), new DiffLineNumber(9, 8, 8)));
41+
lineno1_map.put(275, new Pair<>(new DiffLineNumber(3, 3, 3), new DiffLineNumber(4, 4, 4)));
42+
lineno1_map.put(403, new Pair<>(new DiffLineNumber(5, 5, 5), new DiffLineNumber(6, 6, 6)));
43+
lineno1_map.put(451, new Pair<>(new DiffLineNumber(6, -1, 6), new DiffLineNumber(7, -1, 7)));
44+
lineno1_map.put(523, new Pair<>(new DiffLineNumber(7, 6, -1), new DiffLineNumber(8, 7, -1)));
45+
lineno1_map.put(595, new Pair<>(new DiffLineNumber(8, 7, 7), new DiffLineNumber(9, 8, 8)));
4646
lineno1_map.put(640, new Pair<>(new DiffLineNumber(9, -1, 8), new DiffLineNumber(11, -1, 10)));
47-
lineno1_map.put(724, new Pair<>(new DiffLineNumber(10, 8, 9), new DiffLineNumber(11, 9, 10)));
48-
lineno1_map.put(852, new Pair<>(new DiffLineNumber(12, 9, 11), new DiffLineNumber(13, 10, 12)));
47+
lineno1_map.put(723, new Pair<>(new DiffLineNumber(10, 8, 9), new DiffLineNumber(11, 9, 10)));
48+
lineno1_map.put(851, new Pair<>(new DiffLineNumber(12, 9, 11), new DiffLineNumber(13, 10, 12)));
4949
lineno1_map.put(904, new Pair<>(new DiffLineNumber(13, 10, -1), new DiffLineNumber(16, 13, -1)));
50-
lineno1_map.put(1036, new Pair<>(new DiffLineNumber(15, 12, -1), new DiffLineNumber(16, 13, -1)));
50+
lineno1_map.put(1035, new Pair<>(new DiffLineNumber(15, 12, -1), new DiffLineNumber(16, 13, -1)));
5151
TestCase lineno1 = new TestCase("lineno1.txt", lineno1_map);
5252

5353
final var deleteMLM_map = new HashMap<Integer, Pair<DiffLineNumber, DiffLineNumber>>();
5454
deleteMLM_map.put(144, new Pair<>(new DiffLineNumber(1, 1, 1), new DiffLineNumber(5, 5, 1)));
5555
deleteMLM_map.put(136, new Pair<>(new DiffLineNumber(1, 1, -1), new DiffLineNumber(4, 4, -1)));
56-
deleteMLM_map.put(268, new Pair<>(new DiffLineNumber(3, 3, -1), new DiffLineNumber(4, 4, -1)));
56+
deleteMLM_map.put(267, new Pair<>(new DiffLineNumber(3, 3, -1), new DiffLineNumber(4, 4, -1)));
5757
TestCase deleteMLM = new TestCase("deleteMLM.txt", deleteMLM_map);
5858

5959
testCases = List.of(elifchain, lineno1, deleteMLM);

0 commit comments

Comments
 (0)