Addres code review

Author: Yui T

src/compiler/checker.ts

@@ -71,7 +71,7 @@ module ts {
         var checker: TypeChecker = {
             getProgram: () => program,
             getDiagnostics: getDiagnostics,
-            getDeclarationDiagnostics: getDeclarationDiagnosticsFromSourceFile,
+            getDeclarationDiagnostics: getDeclarationDiagnostics,
             getGlobalDiagnostics: getGlobalDiagnostics,
             getNodeCount: () => sum(program.getSourceFiles(), "nodeCount"),
             getIdentifierCount: () => sum(program.getSourceFiles(), "identifierCount"),
@@ -7039,10 +7039,10 @@ module ts {
             return getSortedDiagnostics();
         }
 
-        function getDeclarationDiagnosticsFromSourceFile(targetSourceFile: SourceFile): Diagnostic[] {
+        function getDeclarationDiagnostics(targetSourceFile: SourceFile): Diagnostic[] {
             var resolver = createResolver();
-            checkProgram();
-            return getDeclarationDiagnostics(program, resolver, targetSourceFile);
+            checkSourceFile(targetSourceFile);
+            return ts.getDeclarationDiagnostics(program, resolver, targetSourceFile);
         }
 
         function getGlobalDiagnostics(): Diagnostic[] {

src/compiler/emitter.ts

@@ -15,6 +15,13 @@ module ts {
         getIndent(): number;
     }
 
+    interface aliasDeclaration {
+        declaration: ImportDeclaration;
+        outputPos: number;
+        indent: number;
+        asynchronousOutput?: string; // If the output for alias was written asynchronously, the corresponding output
+    }
+
     var indentStrings: string[] = ["", "    "];
     export function getIndentString(level: number) {
         if (indentStrings[level] === undefined) {
@@ -326,12 +333,7 @@ module ts {
 
         var emitJsDocComments = compilerOptions.removeComments ? function (declaration: Declaration) { } : writeJsDocComments;
 
-        var aliasDeclarationEmitInfo: {
-            declaration: ImportDeclaration;
-            outputPos: number;
-            indent: number;
-            asynchronousOutput?: string; // If the output for alias was written asynchronously, the corresponding output
-        }[] = [];
+        var aliasDeclarationEmitInfo: aliasDeclaration[] = [];
 
         var getSymbolVisibilityDiagnosticMessage: (symbolAccesibilityResult: SymbolAccessiblityResult) => {
             errorNode: Node;
@@ -1213,26 +1215,29 @@ module ts {
                 }
             });
         }
-
-        // TODO(shkamat): Should we not write any declaration file if any of them can produce error, 
-        // or should we just not write this file like we are doing now
-        if (!reportedDeclarationError) {
-            var declarationOutput = referencePathsOutput;
-            var synchronousDeclarationOutput = writer.getText();
-            // apply additions
-            var appliedSyncOutputPos = 0;
-            forEach(aliasDeclarationEmitInfo, aliasEmitInfo => {
-                if (aliasEmitInfo.asynchronousOutput) {
-                    declarationOutput += synchronousDeclarationOutput.substring(appliedSyncOutputPos, aliasEmitInfo.outputPos);
-                    declarationOutput += aliasEmitInfo.asynchronousOutput;
-                    appliedSyncOutputPos = aliasEmitInfo.outputPos;
-                }
-            });
-            declarationOutput += synchronousDeclarationOutput.substring(appliedSyncOutputPos);
-            writeFile(compilerHost, diagnostics, removeFileExtension(jsFilePath) + ".d.ts", declarationOutput, compilerOptions.emitBOM);
+        return {
+            reportedDeclarationError: reportedDeclarationError,
+            aliasDeclarationEmitInfo: aliasDeclarationEmitInfo,
+            synchronousDeclarationOutput: writer.getText(),
+            declarationOutput: referencePathsOutput,
         }
     }
 
+    function writeDeclarationToFile(compilerHost: CompilerHost, compilerOptions: CompilerOptions, diagnostics: Diagnostic[], aliasDeclarationEmitInfo: aliasDeclaration[],
+        synchronousDeclarationOutput: string, jsFilePath: string, declarationOutput: string) {
+        // apply additions
+        var appliedSyncOutputPos = 0;
+        forEach(aliasDeclarationEmitInfo, aliasEmitInfo => {
+            if (aliasEmitInfo.asynchronousOutput) {
+                declarationOutput += synchronousDeclarationOutput.substring(appliedSyncOutputPos, aliasEmitInfo.outputPos);
+                declarationOutput += aliasEmitInfo.asynchronousOutput;
+                appliedSyncOutputPos = aliasEmitInfo.outputPos;
+            }
+        });
+        declarationOutput += synchronousDeclarationOutput.substring(appliedSyncOutputPos);
+        writeFile(compilerHost, diagnostics, removeFileExtension(jsFilePath) + ".d.ts", declarationOutput, compilerOptions.emitBOM); 
+    }
+
     export function getDeclarationDiagnostics(program: Program, resolver: EmitResolver, targetSourceFile: SourceFile): Diagnostic[] {
         var diagnostics: Diagnostic[] = [];
         var jsFilePath = getOwnEmitOutputFilePath(targetSourceFile, program, ".js");
@@ -3252,7 +3257,13 @@ module ts {
         function emitFile(jsFilePath: string, sourceFile?: SourceFile) {
             emitJavaScript(jsFilePath, sourceFile);
             if (!hasSemanticErrors && compilerOptions.declaration) {
-                emitDeclarations(program, resolver, diagnostics, jsFilePath, sourceFile);
+                var emitDeclarationResult = emitDeclarations(program, resolver, diagnostics, jsFilePath, sourceFile);
+                // TODO(shkamat): Should we not write any declaration file if any of them can produce error, 
+                // or should we just not write this file like we are doing now
+                if (!emitDeclarationResult.reportedDeclarationError) {
+                    writeDeclarationToFile(compilerHost, compilerOptions, diagnostics, emitDeclarationResult.aliasDeclarationEmitInfo, emitDeclarationResult.synchronousDeclarationOutput,
+                        jsFilePath, emitDeclarationResult.declarationOutput);
+                }
             }
         }
 

src/services/services.ts

@@ -2203,11 +2203,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 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.getDeclarationDiagnostics(targetSourceFile));
-                writer = savedWriter;
             }
             return allDiagnostics
         }