Skip to content

Commit 48bef46

Browse files
CyrusNajmabadiCyrusNajmabadi
committed
Provide better error recovery when we encounter merge markers in the source.
Previously we would just treat each merge marker as trivia and then continue scanning and parsing like normal. This worked well in some scenarios, but fell down in others like: ``` class C { public foo() { <<<<<<< HEAD this.bar(); } ======= this.baz(); } >>>>>>> Branch public bar() { } } ``` The problem stems from the previous approach trying to incorporate both branches of the merge into the final tree. In a case like this, that approach breaks down entirely. The the parser ends up seeing the close curly in both included sections, and it considers the class finished. Then, it starts erroring when it encounters "public bar()". The fix is to only incorporate one of these sections into the tree. Specifically, we only include the first section. The second sectoin is treated like trivia and does not affect the parse at all. To make the experience more pleasant we do *lexically* classify the second section. That way it does not appear as just plain black text in the editor. Instead, it will have appropriate lexicla classifications for keywords, literals, comments, operators, punctuation, etc. However, any syntactic or semantic feature will not work in the second block due to this being trivia as far as any feature is concerned. This experience is still much better than what we had originally (where merge markers would absolutely) destroy the parse tree. And it is better than what we checked in last week, which could easily create a borked tree for many types of merges. Now, almost all merges should still leave the tree in good shape. All LS features will work in the first section, and lexical classification will work in the second.
1 parent 828b33a commit 48bef46

7 files changed

Lines changed: 270 additions & 163 deletions

File tree

‎src/compiler/scanner.ts‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,6 @@ module ts {
77
(message: DiagnosticMessage, length: number): void;
88
}
99

10-
export interface CommentCallback {
11-
(pos: number, end: number): void;
12-
}
13-
1410
export interface Scanner {
1511
getStartPos(): number;
1612
getToken(): SyntaxKind;
@@ -396,8 +392,10 @@ module ts {
396392
var mergeConflictMarkerLength = "<<<<<<<".length;
397393

398394
function isConflictMarkerTrivia(text: string, pos: number) {
395+
Debug.assert(pos >= 0);
396+
399397
// Conflict markers must be at the start of a line.
400-
if (pos > 0 && isLineBreak(text.charCodeAt(pos - 1))) {
398+
if (pos === 0 || isLineBreak(text.charCodeAt(pos - 1))) {
401399
var ch = text.charCodeAt(pos);
402400

403401
if ((pos + mergeConflictMarkerLength) < text.length) {
@@ -415,10 +413,31 @@ module ts {
415413
return false;
416414
}
417415

418-
function scanConflictMarkerTrivia(text: string, pos: number) {
419-
var len = text.length;
420-
while (pos < len && !isLineBreak(text.charCodeAt(pos))) {
421-
pos++;
416+
function scanConflictMarkerTrivia(text: string, pos: number, error?: ErrorCallback) {
417+
if (error) {
418+
error(Diagnostics.Merge_conflict_marker_encountered, mergeConflictMarkerLength);
419+
}
420+
421+
var ch = text.charCodeAt(pos);
422+
if (ch === CharacterCodes.lessThan || ch === CharacterCodes.greaterThan) {
423+
var len = text.length;
424+
while (pos < len && !isLineBreak(text.charCodeAt(pos))) {
425+
pos++;
426+
}
427+
}
428+
else {
429+
Debug.assert(ch === CharacterCodes.equals);
430+
// Consume everything from the start of the mid-conlict marker to the start of the next
431+
// end-conflict marker.
432+
var len = text.length;
433+
while (pos < len) {
434+
var ch = text.charCodeAt(pos);
435+
if (ch === CharacterCodes.greaterThan && isConflictMarkerTrivia(text, pos)) {
436+
break;
437+
}
438+
439+
pos++;
440+
}
422441
}
423442

424443
return pos;
@@ -1057,8 +1076,7 @@ module ts {
10571076
return pos++, token = SyntaxKind.SemicolonToken;
10581077
case CharacterCodes.lessThan:
10591078
if (isConflictMarkerTrivia(text, pos)) {
1060-
mergeConflictError();
1061-
pos = scanConflictMarkerTrivia(text, pos);
1079+
pos = scanConflictMarkerTrivia(text, pos, error);
10621080
if (skipTrivia) {
10631081
continue;
10641082
}
@@ -1079,8 +1097,7 @@ module ts {
10791097
return pos++, token = SyntaxKind.LessThanToken;
10801098
case CharacterCodes.equals:
10811099
if (isConflictMarkerTrivia(text, pos)) {
1082-
mergeConflictError();
1083-
pos = scanConflictMarkerTrivia(text, pos);
1100+
pos = scanConflictMarkerTrivia(text, pos, error);
10841101
if (skipTrivia) {
10851102
continue;
10861103
}
@@ -1101,8 +1118,7 @@ module ts {
11011118
return pos++, token = SyntaxKind.EqualsToken;
11021119
case CharacterCodes.greaterThan:
11031120
if (isConflictMarkerTrivia(text, pos)) {
1104-
mergeConflictError();
1105-
pos = scanConflictMarkerTrivia(text, pos);
1121+
pos = scanConflictMarkerTrivia(text, pos, error);
11061122
if (skipTrivia) {
11071123
continue;
11081124
}
@@ -1171,10 +1187,6 @@ module ts {
11711187
}
11721188
}
11731189

1174-
function mergeConflictError() {
1175-
error(Diagnostics.Merge_conflict_marker_encountered, mergeConflictMarkerLength);
1176-
}
1177-
11781190
function reScanGreaterToken(): SyntaxKind {
11791191
if (token === SyntaxKind.GreaterThanToken) {
11801192
if (text.charCodeAt(pos) === CharacterCodes.greaterThan) {

0 commit comments

Comments
 (0)