microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-05-17 10:40:34 -05:00
Code Issues Packages Projects Releases 212 Wiki Activity

Handle comment directives in incremental parsing (#37632)

Browse Source 
* Add test case that shows failure to handle commentDirectives in incremental parsing
Testcase for #37536

* Handle comment directives in incremental parsing
Fixes #37536
This commit is contained in:
Sheetal Nandi
2020-03-27 12:00:34 -07:00
committed by GitHub
parent 0f3a9d4d4b
commit 7f5994958b
9 changed files with 307 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
src/compiler/parser.ts
View File

@@ -7752,7 +7752,6 @@ namespace ts {
const incrementalSourceFile = <IncrementalNode><Node>sourceFile;
Debug.assert(!incrementalSourceFile.hasBeenIncrementallyParsed);
incrementalSourceFile.hasBeenIncrementallyParsed = true;
const oldText = sourceFile.text;
const syntaxCursor = createSyntaxCursor(sourceFile);
@@ -7805,10 +7804,68 @@ namespace ts {
// will immediately bail out of walking any subtrees when we can see that their parents
// are already correct.
const result = Parser.parseSourceFile(sourceFile.fileName, newText, sourceFile.languageVersion, syntaxCursor, /*setParentNodes*/ true, sourceFile.scriptKind);
result.commentDirectives = getNewCommentDirectives(
sourceFile.commentDirectives,
result.commentDirectives,
changeRange.span.start,
textSpanEnd(changeRange.span),
delta,
oldText,
newText,
aggressiveChecks
);
return result;
}
function getNewCommentDirectives(
oldDirectives: CommentDirective[] | undefined,
newDirectives: CommentDirective[] | undefined,
changeStart: number,
changeRangeOldEnd: number,
delta: number,
oldText: string,
newText: string,
aggressiveChecks: boolean
): CommentDirective[] | undefined {
if (!oldDirectives) return newDirectives;
let commentDirectives: CommentDirective[] | undefined;
let addedNewlyScannedDirectives = false;
for (const directive of oldDirectives) {
const { range, type } = directive;
// Range before the change
if (range.end < changeStart) {
commentDirectives = append(commentDirectives, directive);
}
else if (range.pos > changeRangeOldEnd) {
addNewlyScannedDirectives();
// Node is entirely past the change range. We need to move both its pos and
// end, forward or backward appropriately.
const updatedDirective: CommentDirective = {
range: { pos: range.pos + delta, end: range.end + delta },
type
};
commentDirectives = append(commentDirectives, updatedDirective);
if (aggressiveChecks) {
Debug.assert(oldText.substring(range.pos, range.end) === newText.substring(updatedDirective.range.pos, updatedDirective.range.end));
}
}
// Ignore ranges that fall in change range
}
addNewlyScannedDirectives();
return commentDirectives;
function addNewlyScannedDirectives() {
if (addedNewlyScannedDirectives) return;
addedNewlyScannedDirectives = true;
if (!commentDirectives) {
commentDirectives = newDirectives;
}
else if (newDirectives) {
commentDirectives.push(...newDirectives);
}
}
}
function moveElementEntirelyPastChangeRange(element: IncrementalElement, isArray: boolean, delta: number, oldText: string, newText: string, aggressiveChecks: boolean) {
if (isArray) {
visitArray(<IncrementalNodeArray>element);

153
src/testRunner/unittests/incrementalParser.ts
View File

@@ -831,5 +831,158 @@ module m3 { }\
const index = source.indexOf("enum ") + "enum ".length;
insertCode(source, index, "Fo");
});
describe("comment directives", () => {
const tsIgnoreComment = "// @ts-ignore";
const textWithIgnoreComment = `const x = 10;
function foo() {
// @ts-ignore
let y: string = x;
return y;
}
function bar() {
// @ts-ignore
let z : string = x;
return z;
}
function bar3() {
// @ts-ignore
let z : string = x;
return z;
}
foo();
bar();
bar3();`;
verifyScenario("when deleting ts-ignore comment", verifyDelete);
verifyScenario("when inserting ts-ignore comment", verifyInsert);
verifyScenario("when changing ts-ignore comment to blah", verifyChangeToBlah);
verifyScenario("when changing blah comment to ts-ignore", verifyChangeBackToDirective);
verifyScenario("when deleting blah comment", verifyDeletingBlah);
verifyScenario("when changing text that adds another comment", verifyChangeDirectiveType);
verifyScenario("when changing text that keeps the comment but adds more nodes", verifyReuseChange);
function verifyCommentDirectives(oldText: IScriptSnapshot, newTextAndChange: { text: IScriptSnapshot; textChangeRange: TextChangeRange; }) {
const { incrementalNewTree, newTree } = compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, -1);
assert.deepEqual(incrementalNewTree.commentDirectives, newTree.commentDirectives);
}
function verifyScenario(scenario: string, verifyChange: (atIndex: number, singleIgnore?: true) => void) {
it(`${scenario} - 0`, () => {
verifyChange(0);
});
it(`${scenario} - 1`, () => {
verifyChange(1);
});
it(`${scenario} - 2`, () => {
verifyChange(2);
});
it(`${scenario} - with single ts-ignore`, () => {
verifyChange(0, /*singleIgnore*/ true);
});
}
function getIndexOfTsIgnoreComment(atIndex: number) {
let index: number;
for (let i = 0; i <= atIndex; i++) {
index = textWithIgnoreComment.indexOf(tsIgnoreComment);
}
return index!;
}
function textWithIgnoreCommentFrom(text: string, singleIgnore: true | undefined) {
if (!singleIgnore) return text;
const splits = text.split(tsIgnoreComment);
if (splits.length > 2) {
const tail = splits[splits.length - 2] + splits[splits.length - 1];
splits.length = splits.length - 2;
return splits.join(tsIgnoreComment) + tail;
}
else {
return splits.join(tsIgnoreComment);
}
}
function verifyDelete(atIndex: number, singleIgnore?: true) {
const index = getIndexOfTsIgnoreComment(atIndex);
const oldText = ScriptSnapshot.fromString(textWithIgnoreCommentFrom(textWithIgnoreComment, singleIgnore));
const newTextAndChange = withDelete(oldText, index, tsIgnoreComment.length);
verifyCommentDirectives(oldText, newTextAndChange);
}
function verifyInsert(atIndex: number, singleIgnore?: true) {
const index = getIndexOfTsIgnoreComment(atIndex);
const source = textWithIgnoreCommentFrom(textWithIgnoreComment.slice(0, index) + textWithIgnoreComment.slice(index + tsIgnoreComment.length), singleIgnore);
const oldText = ScriptSnapshot.fromString(source);
const newTextAndChange = withInsert(oldText, index, tsIgnoreComment);
verifyCommentDirectives(oldText, newTextAndChange);
}
function verifyChangeToBlah(atIndex: number, singleIgnore?: true) {
const index = getIndexOfTsIgnoreComment(atIndex) + "// ".length;
const oldText = ScriptSnapshot.fromString(textWithIgnoreCommentFrom(textWithIgnoreComment, singleIgnore));
const newTextAndChange = withChange(oldText, index, 1, "blah ");
verifyCommentDirectives(oldText, newTextAndChange);
}
function verifyChangeBackToDirective(atIndex: number, singleIgnore?: true) {
const index = getIndexOfTsIgnoreComment(atIndex) + "// ".length;
const source = textWithIgnoreCommentFrom(textWithIgnoreComment.slice(0, index) + "blah " + textWithIgnoreComment.slice(index + 1), singleIgnore);
const oldText = ScriptSnapshot.fromString(source);
const newTextAndChange = withChange(oldText, index, "blah ".length, "@");
verifyCommentDirectives(oldText, newTextAndChange);
}
function verifyDeletingBlah(atIndex: number, singleIgnore?: true) {
const tsIgnoreIndex = getIndexOfTsIgnoreComment(atIndex);
const index = tsIgnoreIndex + "// ".length;
const source = textWithIgnoreCommentFrom(textWithIgnoreComment.slice(0, index) + "blah " + textWithIgnoreComment.slice(index + 1), singleIgnore);
const oldText = ScriptSnapshot.fromString(source);
const newTextAndChange = withDelete(oldText, tsIgnoreIndex, tsIgnoreComment.length + "blah".length);
verifyCommentDirectives(oldText, newTextAndChange);
}
function verifyChangeDirectiveType(atIndex: number, singleIgnore?: true) {
const index = getIndexOfTsIgnoreComment(atIndex) + "// @ts-".length;
const oldText = ScriptSnapshot.fromString(textWithIgnoreCommentFrom(textWithIgnoreComment, singleIgnore));
const newTextAndChange = withChange(oldText, index, "ignore".length, "expect-error");
verifyCommentDirectives(oldText, newTextAndChange);
}
function verifyReuseChange(atIndex: number, singleIgnore?: true) {
const source = `const x = 10;
function foo1() {
const x1 = 10;
// @ts-ignore
let y0: string = x;
let y1: string = x;
return y1;
}
function foo2() {
const x2 = 10;
// @ts-ignore
let y0: string = x;
let y2: string = x;
return y2;
}
function foo3() {
const x3 = 10;
// @ts-ignore
let y0: string = x;
let y3: string = x;
return y3;
}
foo1();
foo2();
foo3();`;
const oldText = ScriptSnapshot.fromString(textWithIgnoreCommentFrom(source, singleIgnore));
const start = source.indexOf(`const x${atIndex + 1}`);
const letStr = `let y${atIndex + 1}: string = x;`;
const end = source.indexOf(letStr) + letStr.length;
const oldSubStr = source.slice(start, end);
const newText = oldSubStr.replace(letStr, `let yn : string = x;`);
const newTextAndChange = withChange(oldText, start, end - start, newText);
verifyCommentDirectives(oldText, newTextAndChange);
}
});
});
}

23
src/testRunner/unittests/tsserver/configuredProjects.ts
View File

@@ -896,14 +896,7 @@ declare var console: {
const host = createServerHost([barConfig, barIndex, fooConfig, fooIndex, barSymLink, lib2017, libDom]);
const session = createSession(host, { canUseEvents: true, });
openFilesForSession([fooIndex, barIndex], session);
verifyGetErrRequest({
session,
host,
expected: [
{ file: barIndex, syntax: [], semantic: [], suggestion: [] },
{ file: fooIndex, syntax: [], semantic: [], suggestion: [] },
]
});
verifyGetErrRequestNoErrors({ session, host, files: [barIndex, fooIndex] });
});
it("when file name starts with ^", () => {
@@ -983,18 +976,12 @@ declare var console: {
}
const service = session.getProjectService();
checkProjectBeforeError(service);
verifyGetErrRequest({
verifyGetErrRequestNoErrors({
session,
host,
expected: errorOnNewFileBeforeOldFile ?
[
{ file: fooBar, syntax: [], semantic: [], suggestion: [] },
{ file: foo, syntax: [], semantic: [], suggestion: [] },
] :
[
{ file: foo, syntax: [], semantic: [], suggestion: [] },
{ file: fooBar, syntax: [], semantic: [], suggestion: [] },
],
files: errorOnNewFileBeforeOldFile ?
[fooBar, foo] :
[foo, fooBar],
existingTimeouts: 2
});
checkProjectAfterError(service);

25
src/testRunner/unittests/tsserver/forceConsistentCasingInFileNames.ts
View File

@@ -65,13 +65,7 @@ namespace ts.projectSystem {
checkNumberOfProjects(service, { configuredProjects: 1 });
const project = service.configuredProjects.get(tsconfig.path)!;
checkProjectActualFiles(project, [loggerFile.path, anotherFile.path, libFile.path, tsconfig.path]);
verifyGetErrRequest({
host,
session,
expected: [
{ file: loggerFile.path, syntax: [], semantic: [], suggestion: [] }
]
});
verifyGetErrRequestNoErrors({ session, host, files: [loggerFile] });
const newLoggerPath = loggerFile.path.toLowerCase();
host.renameFile(loggerFile.path, newLoggerPath);
@@ -97,14 +91,7 @@ namespace ts.projectSystem {
});
// Check errors in both files
verifyGetErrRequest({
host,
session,
expected: [
{ file: newLoggerPath, syntax: [], semantic: [], suggestion: [] },
{ file: anotherFile.path, syntax: [], semantic: [], suggestion: [] }
]
});
verifyGetErrRequestNoErrors({ session, host, files: [newLoggerPath, anotherFile] });
});
it("when changing module name with different casing", () => {
@@ -130,13 +117,7 @@ namespace ts.projectSystem {
checkNumberOfProjects(service, { configuredProjects: 1 });
const project = service.configuredProjects.get(tsconfig.path)!;
checkProjectActualFiles(project, [loggerFile.path, anotherFile.path, libFile.path, tsconfig.path]);
verifyGetErrRequest({
host,
session,
expected: [
{ file: anotherFile.path, syntax: [], semantic: [], suggestion: [] }
]
});
verifyGetErrRequestNoErrors({ session, host, files: [anotherFile] });
session.executeCommandSeq<protocol.UpdateOpenRequest>({
command: protocol.CommandTypes.UpdateOpen,

10
src/testRunner/unittests/tsserver/helpers.ts
View File

@@ -837,6 +837,16 @@ namespace ts.projectSystem {
checkAllErrors({ ...request, expectedSequenceId });
}
export interface VerifyGetErrRequestNoErrors extends VerifyGetErrRequestBase {
files: readonly (string | File)[];
}
export function verifyGetErrRequestNoErrors(request: VerifyGetErrRequestNoErrors) {
verifyGetErrRequest({
...request,
expected: request.files.map(file => ({ file, syntax: [], semantic: [], suggestion: [] }))
});
}
export interface CheckAllErrors extends VerifyGetErrRequest {
expectedSequenceId: number;
}

69
src/testRunner/unittests/tsserver/openFile.ts
View File

@@ -128,5 +128,74 @@ namespace ts.projectSystem {
assert.equal(project.getCurrentProgram()?.getSourceFile(aFile.path)!.text, aFileContent);
}
});
it("when file makes edits to add/remove comment directives, they are handled correcrly", () => {
const file: File = {
path: `${tscWatch.projectRoot}/file.ts`,
content: `const x = 10;
function foo() {
// @ts-ignore
let y: string = x;
return y;
}
function bar() {
// @ts-ignore
let z : string = x;
return z;
}
foo();
bar();`
};
const host = createServerHost([file, libFile]);
const session = createSession(host, { canUseEvents: true, });
openFilesForSession([file], session);
verifyGetErrRequestNoErrors({ session, host, files: [file] });
// Remove first ts-ignore and check only first error is reported
const tsIgnoreComment = `// @ts-ignore`;
const locationOfTsIgnore = protocolTextSpanFromSubstring(file.content, tsIgnoreComment);
session.executeCommandSeq<protocol.UpdateOpenRequest>({
command: protocol.CommandTypes.UpdateOpen,
arguments: {
changedFiles: [{
fileName: file.path,
textChanges: [{
newText: " ",
...locationOfTsIgnore
}]
}]
}
});
const locationOfY = protocolTextSpanFromSubstring(file.content, "y");
verifyGetErrRequest({
session,
host,
expected: [
{
file,
syntax: [],
semantic: [
createDiagnostic(locationOfY.start, locationOfY.end, Diagnostics.Type_0_is_not_assignable_to_type_1, ["10", "string"]),
],
suggestion: []
},
]
});
// Revert the change and no errors should be reported
session.executeCommandSeq<protocol.UpdateOpenRequest>({
command: protocol.CommandTypes.UpdateOpen,
arguments: {
changedFiles: [{
fileName: file.path,
textChanges: [{
newText: tsIgnoreComment,
...locationOfTsIgnore
}]
}]
}
});
verifyGetErrRequestNoErrors({ session, host, files: [file] });
});
});
}

31
src/testRunner/unittests/tsserver/projectErrors.ts
View File

@@ -350,16 +350,7 @@ namespace ts.projectSystem {
verifyErrorsInApp();
function verifyErrorsInApp() {
verifyGetErrRequest({
session,
host,
expected: [{
file: app,
syntax: [],
semantic: [],
suggestion: []
}],
});
verifyGetErrRequestNoErrors({ session, host, files: [app] });
}
});
@@ -418,12 +409,7 @@ namespace ts.projectSystem {
checkErrors([serverUtilities.path, app.path]);
function checkErrors(openFiles: [string, string]) {
verifyGetErrRequest({
session,
host,
expected: openFiles.map(file => ({ file, syntax: [], semantic: [], suggestion: [] })),
existingTimeouts: 2
});
verifyGetErrRequestNoErrors({ session, host, files: openFiles, existingTimeouts: 2 });
}
});
@@ -961,18 +947,7 @@ console.log(blabla);`
const { host, session, test } = createSessionForTest({
include: ["./src/*.ts", "./src/*.json"]
});
verifyGetErrRequest({
session,
host,
expected: [
{
file: test,
syntax: [],
semantic: [],
suggestion: []
}
]
});
verifyGetErrRequestNoErrors({ session, host, files: [test] });
});
it("should report error when json is not root file found by tsconfig", () => {

24
src/testRunner/unittests/tsserver/projectReferences.ts
View File

@@ -1487,13 +1487,7 @@ function foo() {
project,
[aConfig.path, aTest.path, bFoo.path, bBar.path, libFile.path]
);
verifyGetErrRequest({
host,
session,
expected: [
{ file: aTest, syntax: [], semantic: [], suggestion: [] }
]
});
verifyGetErrRequestNoErrors({ session, host, files: [aTest] });
session.executeCommandSeq<protocol.UpdateOpenRequest>({
command: protocol.CommandTypes.UpdateOpen,
arguments: {
@@ -1507,13 +1501,7 @@ function foo() {
}]
}
});
verifyGetErrRequest({
host,
session,
expected: [
{ file: aTest, syntax: [], semantic: [], suggestion: [] }
]
});
verifyGetErrRequestNoErrors({ session, host, files: [aTest] });
}
function config(packageName: string, extraOptions: CompilerOptions, references?: string[]): File {
@@ -1937,13 +1925,7 @@ foo;`
assert.equal(service.findDefaultConfiguredProject(info), project);
// Verify errors
verifyGetErrRequest({
session,
host,
expected: [
{ file: main, syntax: [], semantic: [], suggestion: [] },
]
});
verifyGetErrRequestNoErrors({ session, host, files: [main] });
// Verify collection of script infos
service.openClientFile(dummyFile.path);

7
src/testRunner/unittests/tsserver/projects.ts
View File

@@ -1558,13 +1558,10 @@ namespace ts.projectSystem {
host.reloadFS(files);
host.checkTimeoutQueueLength(2);
verifyGetErrRequest({
verifyGetErrRequestNoErrors({
session,
host,
expected: [
{ file: fileB, syntax: [], semantic: [], suggestion: [] },
{ file: fileSubA, syntax: [], semantic: [], suggestion: [] },
],
files: [fileB, fileSubA],
existingTimeouts: 2,
onErrEvent: () => assert.isFalse(hasErrorMsg())
});