From 48002d4d48dd5da11373144dcf0883fd7b606496 Mon Sep 17 00:00:00 2001 From: Yui T Date: Thu, 18 Sep 2014 12:41:41 -0700 Subject: [PATCH 1/4] Fix language service; doesn't capture declaration emit errors --- src/services/services.ts | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 6742e86dc38..cd9613b15e4 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1599,9 +1599,9 @@ module ts { } // Only perform incremental parsing on open files that are being edited. If a file was - // open, but is now closed, we want to reparse entirely so we don't have any tokens that + // open, but is now closed, we want to re-parse entirely so we don't have any tokens that // are holding onto expensive script snapshot instances on the host. Similarly, if a - // file was closed, then we always want to reparse. This is so our tree doesn't keep + // file was closed, then we always want to re-parse. This is so our tree doesn't keep // the old buffer alive that represented the file on disk (as the host has moved to a // new text buffer). var textChangeRange: TypeScript.TextChangeRange = null; @@ -1651,12 +1651,28 @@ module ts { return program.getDiagnostics(getSourceFile(filename).getSourceFile()); } + // In a case when '-d' is not enabled, only report semantic errors + // If '-d' enabled, report both semantic and emitter errors function getSemanticDiagnostics(filename: string) { synchronizeHostData(); filename = TypeScript.switchToForwardSlashes(filename) + var compilerOptions = program.getCompilerOptions(); + var checker = getFullTypeCheckChecker(); + var targetSourceFile = getSourceFile(filename); - return getFullTypeCheckChecker().getDiagnostics(getSourceFile(filename)); + // Only perform the action per file regardless off '-out' flag + // As an errors message in Visual Studio will maintain an error message life-time per file + + var allDiagnostics = checker.getDiagnostics(targetSourceFile); + if (compilerOptions.declaration) { + // If '-d' is enabled, check for emitter error which requires calling to TypeChecker.emitFiles + // Define CompilerHost.writer which does nothing as this is a side effect of emitFiles + writer = (filename: string, data: string, writeByteOrderMark: boolean) => { }; + allDiagnostics = allDiagnostics.concat(checker.emitFiles(targetSourceFile).errors); + writer = undefined; + } + return allDiagnostics } function getCompilerOptionsDiagnostics() { From b08cd8c9a93a224cc4e820f1999a7c4c84628772 Mon Sep 17 00:00:00 2001 From: Yui T Date: Thu, 18 Sep 2014 12:42:44 -0700 Subject: [PATCH 2/4] Add fourslash testcases --- .../fourslash/getSemanticDiagnosticForDeclaration.ts | 10 ++++++++++ .../fourslash/getSemanticDiagnosticForNoDeclaration.ts | 9 +++++++++ 2 files changed, 19 insertions(+) create mode 100644 tests/cases/fourslash/getSemanticDiagnosticForDeclaration.ts create mode 100644 tests/cases/fourslash/getSemanticDiagnosticForNoDeclaration.ts diff --git a/tests/cases/fourslash/getSemanticDiagnosticForDeclaration.ts b/tests/cases/fourslash/getSemanticDiagnosticForDeclaration.ts new file mode 100644 index 00000000000..fbd086d191d --- /dev/null +++ b/tests/cases/fourslash/getSemanticDiagnosticForDeclaration.ts @@ -0,0 +1,10 @@ +/// + +// @declaration: true +//// interface privateInterface {} +//// export class Bar implements /*1*/privateInterface/*2*/{ } + +verify.errorExistsBetweenMarkers("1", "2"); +verify.numberOfErrorsInCurrentFile(1); + + diff --git a/tests/cases/fourslash/getSemanticDiagnosticForNoDeclaration.ts b/tests/cases/fourslash/getSemanticDiagnosticForNoDeclaration.ts new file mode 100644 index 00000000000..a08afbe26a9 --- /dev/null +++ b/tests/cases/fourslash/getSemanticDiagnosticForNoDeclaration.ts @@ -0,0 +1,9 @@ +/// + +//// interface privateInterface {} +//// export class Bar implements /*1*/privateInterface/*2*/{ } + +debugger +verify.numberOfErrorsInCurrentFile(0); + + From 360d332bd495e23e9e70a2f3d3369ef02850f2ca Mon Sep 17 00:00:00 2001 From: Yui T Date: Thu, 18 Sep 2014 13:50:38 -0700 Subject: [PATCH 3/4] Address code-review comments --- src/services/services.ts | 13 +++++++------ .../getSemanticDiagnosticForNoDeclaration.ts | 1 - 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index cd9613b15e4..ad4b5682f54 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1651,7 +1651,7 @@ module ts { return program.getDiagnostics(getSourceFile(filename).getSourceFile()); } - // In a case when '-d' is not enabled, only report semantic errors + // getSemanticDiagnostiscs return array of Diagnostics. If '-d' is not enabled, only report semantic errors // If '-d' enabled, report both semantic and emitter errors function getSemanticDiagnostics(filename: string) { synchronizeHostData(); @@ -1661,16 +1661,17 @@ module ts { var checker = getFullTypeCheckChecker(); var targetSourceFile = getSourceFile(filename); - // Only perform the action per file regardless off '-out' flag - // As an errors message in Visual Studio will maintain an error message life-time per file + // Only perform the action per file regardless of '-out' flag as LanguageServiceHost is expected to call this function per file. + // Therefore only get diagnostics for given file. var allDiagnostics = checker.getDiagnostics(targetSourceFile); if (compilerOptions.declaration) { - // If '-d' is enabled, check for emitter error which requires calling to TypeChecker.emitFiles - // Define CompilerHost.writer which does nothing as this is a side effect of emitFiles + // If '-d' is enabled, check for emitter error. One example of emitter error is export class implements non-export interface + // Get emitter-diagnostics requires calling TypeChecker.emitFiles so define CompilerHost.writer which does nothing as this is a side effect of emitFiles + var savedWriter = writer; writer = (filename: string, data: string, writeByteOrderMark: boolean) => { }; allDiagnostics = allDiagnostics.concat(checker.emitFiles(targetSourceFile).errors); - writer = undefined; + writer = savedWriter; } return allDiagnostics } diff --git a/tests/cases/fourslash/getSemanticDiagnosticForNoDeclaration.ts b/tests/cases/fourslash/getSemanticDiagnosticForNoDeclaration.ts index a08afbe26a9..57a490fb31c 100644 --- a/tests/cases/fourslash/getSemanticDiagnosticForNoDeclaration.ts +++ b/tests/cases/fourslash/getSemanticDiagnosticForNoDeclaration.ts @@ -3,7 +3,6 @@ //// interface privateInterface {} //// export class Bar implements /*1*/privateInterface/*2*/{ } -debugger verify.numberOfErrorsInCurrentFile(0); From c91c52de39a9c5d060a0d45d69785f7beacafffd Mon Sep 17 00:00:00 2001 From: Yui T Date: Thu, 18 Sep 2014 14:07:48 -0700 Subject: [PATCH 4/4] Clarify comments --- src/services/services.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/services.ts b/src/services/services.ts index ad4b5682f54..4e1d9089855 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1667,7 +1667,7 @@ module ts { var allDiagnostics = checker.getDiagnostics(targetSourceFile); if (compilerOptions.declaration) { // If '-d' is enabled, check for emitter error. One example of emitter error is export class implements non-export interface - // Get emitter-diagnostics requires calling TypeChecker.emitFiles so define CompilerHost.writer which does nothing as this is a side effect of emitFiles + // Get emitter-diagnostics requires calling TypeChecker.emitFiles so we have to define CompilerHost.writer which does nothing because emitFiles function has side effects defined by CompilerHost.writer var savedWriter = writer; writer = (filename: string, data: string, writeByteOrderMark: boolean) => { }; allDiagnostics = allDiagnostics.concat(checker.emitFiles(targetSourceFile).errors);