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.
This commit is contained in:
Cyrus Najmabadi
2014-12-18 19:18:13 -08:00
parent 828b33aae7
commit 48bef4698b
7 changed files with 270 additions and 163 deletions

View File

@@ -1,24 +1,18 @@
tests/cases/compiler/conflictMarkerTrivia1.ts(2,1): error TS1185: Merge conflict marker encountered.
tests/cases/compiler/conflictMarkerTrivia1.ts(3,5): error TS2300: Duplicate identifier 'v'.
tests/cases/compiler/conflictMarkerTrivia1.ts(4,1): error TS1185: Merge conflict marker encountered.
tests/cases/compiler/conflictMarkerTrivia1.ts(5,5): error TS2300: Duplicate identifier 'v'.
tests/cases/compiler/conflictMarkerTrivia1.ts(6,1): error TS1185: Merge conflict marker encountered.
==== tests/cases/compiler/conflictMarkerTrivia1.ts (5 errors) ====
==== tests/cases/compiler/conflictMarkerTrivia1.ts (3 errors) ====
class C {
<<<<<<< HEAD
~~~~~~~
!!! error TS1185: Merge conflict marker encountered.
v = 1;
~
!!! error TS2300: Duplicate identifier 'v'.
=======
~~~~~~~
!!! error TS1185: Merge conflict marker encountered.
v = 2;
~
!!! error TS2300: Duplicate identifier 'v'.
>>>>>>> Branch-a
~~~~~~~
!!! error TS1185: Merge conflict marker encountered.

View File

@@ -1,15 +1,10 @@
tests/cases/compiler/conflictMarkerTrivia2.ts(3,1): error TS1185: Merge conflict marker encountered.
tests/cases/compiler/conflictMarkerTrivia2.ts(4,6): error TS2304: Cannot find name 'a'.
tests/cases/compiler/conflictMarkerTrivia2.ts(6,1): error TS1185: Merge conflict marker encountered.
tests/cases/compiler/conflictMarkerTrivia2.ts(7,6): error TS2391: Function implementation is missing or not immediately following the declaration.
tests/cases/compiler/conflictMarkerTrivia2.ts(9,1): error TS1185: Merge conflict marker encountered.
tests/cases/compiler/conflictMarkerTrivia2.ts(11,3): error TS1128: Declaration or statement expected.
tests/cases/compiler/conflictMarkerTrivia2.ts(11,10): error TS2304: Cannot find name 'bar'.
tests/cases/compiler/conflictMarkerTrivia2.ts(11,16): error TS1005: ';' expected.
tests/cases/compiler/conflictMarkerTrivia2.ts(12,1): error TS1128: Declaration or statement expected.
==== tests/cases/compiler/conflictMarkerTrivia2.ts (9 errors) ====
==== tests/cases/compiler/conflictMarkerTrivia2.ts (4 errors) ====
class C {
foo() {
<<<<<<< B
@@ -23,21 +18,11 @@ tests/cases/compiler/conflictMarkerTrivia2.ts(12,1): error TS1128: Declaration o
~~~~~~~
!!! error TS1185: Merge conflict marker encountered.
b();
~
!!! error TS2391: Function implementation is missing or not immediately following the declaration.
}
>>>>>>> A
~~~~~~~
!!! error TS1185: Merge conflict marker encountered.
public bar() { }
~~~~~~
!!! error TS1128: Declaration or statement expected.
~~~
!!! error TS2304: Cannot find name 'bar'.
~
!!! error TS1005: ';' expected.
}
~
!!! error TS1128: Declaration or statement expected.

View File

@@ -0,0 +1,19 @@
/// <reference path="fourslash.ts"/>
////class C {
////<<<<<<< HEAD
//// v = 1;
////=======
//// v = 2;
////>>>>>>> Branch - a
////}
debugger;
var c = classification;
verify.syntacticClassificationsAre(
c.keyword("class"), c.className("C"), c.punctuation("{"),
c.comment("<<<<<<< HEAD"),
c.text("v"), c.punctuation("="), c.numericLiteral("1"), c.punctuation(";"),
c.comment("======="),
c.text("v"), c.punctuation("="), c.numericLiteral("2"), c.punctuation(";"),
c.comment(">>>>>>> Branch - a"),
c.punctuation("}"));

View File

@@ -0,0 +1,15 @@
/// <reference path="fourslash.ts"/>
////<<<<<<< HEAD
////class C { }
////=======
////class D { }
////>>>>>>> Branch - a
var c = classification;
verify.syntacticClassificationsAre(
c.comment("<<<<<<< HEAD"),
c.keyword("class"), c.className("C"), c.punctuation("{"), c.punctuation("}"),
c.comment("======="),
c.keyword("class"), c.text("D"), c.punctuation("{"), c.punctuation("}"),
c.comment(">>>>>>> Branch - a"));

View File

@@ -21,7 +21,7 @@ describe('Colorization', function () {
var mytypescriptLS = new Harness.LanguageService.TypeScriptLS();
var myclassifier = mytypescriptLS.getClassifier();
function getClassifications(code: string, initialEndOfLineState: ts.EndOfLineState = ts.EndOfLineState.Start): ClassiferResult {
function getLexicalClassifications(code: string, initialEndOfLineState: ts.EndOfLineState = ts.EndOfLineState.Start): ClassiferResult {
var classResult = myclassifier.getClassificationsForLine(code, initialEndOfLineState).split('\n');
var tuples: Classification[] = [];
var i = 0;
@@ -71,8 +71,8 @@ describe('Colorization', function () {
function regExpLiteral(text: string) { return { value: text, class: ts.TokenClass.RegExpLiteral }; }
function finalEndOfLineState(value: number) { return { value: value, class: <ts.TokenClass>undefined }; }
function test(text: string, initialEndOfLineState: ts.EndOfLineState, ...expectedEntries: ClassificationEntry[]): void {
var result = getClassifications(text, initialEndOfLineState);
function testLexicalClassification(text: string, initialEndOfLineState: ts.EndOfLineState, ...expectedEntries: ClassificationEntry[]): void {
var result = getLexicalClassifications(text, initialEndOfLineState);
for (var i = 0, n = expectedEntries.length; i < n; i++) {
var expectedEntry = expectedEntries[i];
@@ -95,7 +95,7 @@ describe('Colorization', function () {
describe("test getClassifications", function () {
it("Returns correct token classes", function () {
test("var x: string = \"foo\"; //Hello",
testLexicalClassification("var x: string = \"foo\"; //Hello",
ts.EndOfLineState.Start,
keyword("var"),
whitespace(" "),
@@ -109,7 +109,7 @@ describe('Colorization', function () {
});
it("correctly classifies a comment after a divide operator", function () {
test("1 / 2 // comment",
testLexicalClassification("1 / 2 // comment",
ts.EndOfLineState.Start,
numberLiteral("1"),
whitespace(" "),
@@ -119,7 +119,7 @@ describe('Colorization', function () {
});
it("correctly classifies a literal after a divide operator", function () {
test("1 / 2, 3 / 4",
testLexicalClassification("1 / 2, 3 / 4",
ts.EndOfLineState.Start,
numberLiteral("1"),
whitespace(" "),
@@ -131,131 +131,131 @@ describe('Colorization', function () {
});
it("correctly classifies a multi-line string with one backslash", function () {
test("'line1\\",
testLexicalClassification("'line1\\",
ts.EndOfLineState.Start,
stringLiteral("'line1\\"),
finalEndOfLineState(ts.EndOfLineState.InSingleQuoteStringLiteral));
});
it("correctly classifies a multi-line string with three backslashes", function () {
test("'line1\\\\\\",
testLexicalClassification("'line1\\\\\\",
ts.EndOfLineState.Start,
stringLiteral("'line1\\\\\\"),
finalEndOfLineState(ts.EndOfLineState.InSingleQuoteStringLiteral));
});
it("correctly classifies an unterminated single-line string with no backslashes", function () {
test("'line1",
testLexicalClassification("'line1",
ts.EndOfLineState.Start,
stringLiteral("'line1"),
finalEndOfLineState(ts.EndOfLineState.Start));
});
it("correctly classifies an unterminated single-line string with two backslashes", function () {
test("'line1\\\\",
testLexicalClassification("'line1\\\\",
ts.EndOfLineState.Start,
stringLiteral("'line1\\\\"),
finalEndOfLineState(ts.EndOfLineState.Start));
});
it("correctly classifies an unterminated single-line string with four backslashes", function () {
test("'line1\\\\\\\\",
testLexicalClassification("'line1\\\\\\\\",
ts.EndOfLineState.Start,
stringLiteral("'line1\\\\\\\\"),
finalEndOfLineState(ts.EndOfLineState.Start));
});
it("correctly classifies the continuing line of a multi-line string ending in one backslash", function () {
test("\\",
testLexicalClassification("\\",
ts.EndOfLineState.InDoubleQuoteStringLiteral,
stringLiteral("\\"),
finalEndOfLineState(ts.EndOfLineState.InDoubleQuoteStringLiteral));
});
it("correctly classifies the continuing line of a multi-line string ending in three backslashes", function () {
test("\\",
testLexicalClassification("\\",
ts.EndOfLineState.InDoubleQuoteStringLiteral,
stringLiteral("\\"),
finalEndOfLineState(ts.EndOfLineState.InDoubleQuoteStringLiteral));
});
it("correctly classifies the last line of an unterminated multi-line string ending in no backslashes", function () {
test(" ",
testLexicalClassification(" ",
ts.EndOfLineState.InDoubleQuoteStringLiteral,
stringLiteral(" "),
finalEndOfLineState(ts.EndOfLineState.Start));
});
it("correctly classifies the last line of an unterminated multi-line string ending in two backslashes", function () {
test("\\\\",
testLexicalClassification("\\\\",
ts.EndOfLineState.InDoubleQuoteStringLiteral,
stringLiteral("\\\\"),
finalEndOfLineState(ts.EndOfLineState.Start));
});
it("correctly classifies the last line of an unterminated multi-line string ending in four backslashes", function () {
test("\\\\\\\\",
testLexicalClassification("\\\\\\\\",
ts.EndOfLineState.InDoubleQuoteStringLiteral,
stringLiteral("\\\\\\\\"),
finalEndOfLineState(ts.EndOfLineState.Start));
});
it("correctly classifies the last line of a multi-line string", function () {
test("'",
testLexicalClassification("'",
ts.EndOfLineState.InSingleQuoteStringLiteral,
stringLiteral("'"),
finalEndOfLineState(ts.EndOfLineState.Start));
});
it("correctly classifies an unterminated multiline comment", function () {
test("/*",
testLexicalClassification("/*",
ts.EndOfLineState.Start,
comment("/*"),
finalEndOfLineState(ts.EndOfLineState.InMultiLineCommentTrivia));
});
it("correctly classifies the termination of a multiline comment", function () {
test(" */ ",
testLexicalClassification(" */ ",
ts.EndOfLineState.InMultiLineCommentTrivia,
comment(" */"),
finalEndOfLineState(ts.EndOfLineState.Start));
});
it("correctly classifies the continuation of a multiline comment", function () {
test("LOREM IPSUM DOLOR ",
testLexicalClassification("LOREM IPSUM DOLOR ",
ts.EndOfLineState.InMultiLineCommentTrivia,
comment("LOREM IPSUM DOLOR "),
finalEndOfLineState(ts.EndOfLineState.InMultiLineCommentTrivia));
});
it("correctly classifies an unterminated multiline comment on a line ending in '/*/'", function () {
test(" /*/",
testLexicalClassification(" /*/",
ts.EndOfLineState.Start,
comment("/*/"),
finalEndOfLineState(ts.EndOfLineState.InMultiLineCommentTrivia));
});
it("correctly classifies an unterminated multiline comment with trailing space", function () {
test("/* ",
testLexicalClassification("/* ",
ts.EndOfLineState.Start,
comment("/* "),
finalEndOfLineState(ts.EndOfLineState.InMultiLineCommentTrivia));
});
it("correctly classifies a keyword after a dot", function () {
test("a.var",
testLexicalClassification("a.var",
ts.EndOfLineState.Start,
identifier("var"));
});
it("correctly classifies a string literal after a dot", function () {
test("a.\"var\"",
testLexicalClassification("a.\"var\"",
ts.EndOfLineState.Start,
stringLiteral("\"var\""));
});
it("correctly classifies a keyword after a dot separated by comment trivia", function () {
test("a./*hello world*/ var",
testLexicalClassification("a./*hello world*/ var",
ts.EndOfLineState.Start,
identifier("a"),
punctuation("."),
@@ -264,27 +264,27 @@ describe('Colorization', function () {
});
it("classifies a property access with whitespace around the dot", function () {
test(" x .\tfoo ()",
testLexicalClassification(" x .\tfoo ()",
ts.EndOfLineState.Start,
identifier("x"),
identifier("foo"));
});
it("classifies a keyword after a dot on previous line", function () {
test("var",
testLexicalClassification("var",
ts.EndOfLineState.Start,
keyword("var"),
finalEndOfLineState(ts.EndOfLineState.Start));
});
it("classifies multiple keywords properly", function () {
test("public static",
testLexicalClassification("public static",
ts.EndOfLineState.Start,
keyword("public"),
keyword("static"),
finalEndOfLineState(ts.EndOfLineState.Start));
test("public var",
testLexicalClassification("public var",
ts.EndOfLineState.Start,
keyword("public"),
identifier("var"),
@@ -292,7 +292,7 @@ describe('Colorization', function () {
});
it("classifies partially written generics correctly.", function () {
test("Foo<number",
testLexicalClassification("Foo<number",
ts.EndOfLineState.Start,
identifier("Foo"),
operator("<"),
@@ -300,14 +300,14 @@ describe('Colorization', function () {
finalEndOfLineState(ts.EndOfLineState.Start));
// Looks like a cast, should get classified as a keyword.
test("<number",
testLexicalClassification("<number",
ts.EndOfLineState.Start,
operator("<"),
keyword("number"),
finalEndOfLineState(ts.EndOfLineState.Start));
// handle nesting properly.
test("Foo<Foo,Foo<number",
testLexicalClassification("Foo<Foo,Foo<number",
ts.EndOfLineState.Start,
identifier("Foo"),
operator("<"),
@@ -319,19 +319,11 @@ describe('Colorization', function () {
finalEndOfLineState(ts.EndOfLineState.Start));
});
it("ClassifiesConflictTokens", () => {
// no longer in something that looks generic.
test("Foo<Foo> number",
ts.EndOfLineState.Start,
identifier("Foo"),
operator("<"),
identifier("Foo"),
operator(">"),
keyword("number"),
finalEndOfLineState(ts.EndOfLineState.Start));
it("LexicallyClassifiesConflictTokens", () => {
debugger;
// Test conflict markers.
test(
testLexicalClassification(
"class C {\r\n\
<<<<<<< HEAD\r\n\
v = 1;\r\n\
@@ -348,14 +340,26 @@ describe('Colorization', function () {
operator("="),
numberLiteral("1"),
punctuation(";"),
comment("======="),
identifier("v"),
operator("="),
numberLiteral("2"),
punctuation(";"),
comment("=======\r\n v = 2;\r\n"),
comment(">>>>>>> Branch - a"),
punctuation("}"),
finalEndOfLineState(ts.EndOfLineState.Start));
testLexicalClassification(
"<<<<<<< HEAD\r\n\
class C { }\r\n\
=======\r\n\
class D { }\r\n\
>>>>>>> Branch - a\r\n",
ts.EndOfLineState.Start,
comment("<<<<<<< HEAD"),
keyword("class"),
identifier("C"),
punctuation("{"),
punctuation("}"),
comment("=======\r\nclass D { }\r\n"),
comment(">>>>>>> Branch - a"),
finalEndOfLineState(ts.EndOfLineState.Start));
});
});
});