Switch to iterative model and clean up error reporting

src/compiler/declarationEmitter.ts

@@ -7,6 +7,7 @@ namespace ts {
         visited?: boolean;
         collected?: boolean;
         hasExportDeclarations?: boolean;
+        errorReported?: boolean;
     }
 
     interface PreprocessResults {
@@ -1039,6 +1040,7 @@ namespace ts {
     function preprocessDeclarations(sourceFiles: SourceFile[], resolver: EmitResolver, diagnostics: Diagnostic[], compilerOptions: CompilerOptions): PreprocessResults {
         let currentSourceFile: SourceFile;
         let nodeLinks: NodeLinks[] = [];
+        let declarationsToProcess: { declaration: Node; errorNode?: Node }[] = [];
 
         for (let sourceFile of sourceFiles) {
             preprocessSourceFile(sourceFile);
@@ -1298,10 +1300,17 @@ namespace ts {
             }
 
             function collectDeclarations(symbol: Symbol, errorNode: Node): void {
-                forEach(symbol.declarations, d => collectDeclaration(d, errorNode));
+                forEach(symbol.declarations, declaration => {
+                    collectDeclatation(declaration, errorNode);
+                });
             }
         }
 
+        function collectDeclatation(declaration:Node, errorNode?: Node) {
+            declarationsToProcess.push({ declaration, errorNode });
+            //preprocessDeclaration(declaration, errorNode);
+        }
+
         function isDeclarationExported(node: Node): boolean {
             if (compilerOptions.stripInternal && isInternal(node)) {
                 // TODO: this is the correct place for this check, enable this 
@@ -1421,31 +1430,31 @@ namespace ts {
          *
          */
         function canWriteDeclaration(node: Node): boolean {
-            if (node.kind === SyntaxKind.VariableDeclaration || node.kind === SyntaxKind.BindingElement) {
-                node = node.parent.parent;
-            }
-
-            // If the declaration is exported from its parent, we can
-            // safelly write a declaration for it
-            if (isDeclarationExported(node)) {
+            if (node.kind === SyntaxKind.SourceFile ||
+                node.kind === SyntaxKind.ImportEqualsDeclaration || node.kind === SyntaxKind.ImportDeclaration) {
                 return true;
             }
 
-            if (node.kind === SyntaxKind.ImportEqualsDeclaration || node.kind === SyntaxKind.ImportDeclaration) {
-                return true;
+            if (node.kind === SyntaxKind.VariableDeclaration || node.kind === SyntaxKind.BindingElement) {
+                node = node.parent.parent;
             }
 
-            // If the parent module has an export declaration  or export
-            // assingment, then the scoping rules change, and only declarations
-            // with export modifier are visible, so we can safely write 
-            // a declaration of a non-exported entity without exposing it
             let parent = getEnclosingDeclaration(node);
-            if (parent) {
-                return (parent.kind === SyntaxKind.ModuleDeclaration || parent.kind === SyntaxKind.SourceFile) &&
-                    hasExportDeclatations(parent);
+
+            if (!isDeclarationExported(node)) {
+                // If the parent module has an export declaration  or export
+                // assingment, then the scoping rules change, and only declarations
+                // with export modifier are visible, so we can safely write 
+                // a declaration of a non-exported entity without exposing it
+                if (!((parent.kind === SyntaxKind.ModuleDeclaration || parent.kind === SyntaxKind.SourceFile) &&
+                    hasExportDeclatations(parent))) {
+                    return false;
+                }
             }
 
-            return false;
+            // The declaration is exported from its parent, or can be written without
+            // an export modifier. Now check if the parent is visible as well.
+            return canWriteDeclaration(parent);
         }
 
         function getNameText(node: ParameterDeclaration|PropertyDeclaration|VariableDeclaration|FunctionDeclaration| AccessorDeclaration| MethodDeclaration): string {
@@ -1597,44 +1606,64 @@ namespace ts {
             Debug.fail("Could not find container node");
         }
 
-        function collectDeclaration(node: Node, errorNode?: Node): void {
-            if (node.kind === SyntaxKind.TypeParameter || node.kind === SyntaxKind.Parameter) {
+        function preprocessDeclaration(declaration: Node, errorNode?: Node): void {
+            if (declaration.kind === SyntaxKind.TypeParameter || declaration.kind === SyntaxKind.Parameter) {
                 return;
             }
 
-            // Make sure this declaration can be emitted, that is it is accessible
-            // and is connected to its parent, or else report a accessibility error
-            ensureDeclarationVisible(node, errorNode);
+            // Always report the error, even if the node was visited before, so that 
+            // diffrent references to the same declaration are flagged
+            if (errorNode && reportDeclarationAccessiblityError(declaration, errorNode)) {
+                return;
+            }
 
             // If this declaration is from a diffrent file, we will get to it later. 
-            if (getSourceFileOfNode(node) !== currentSourceFile) {
+            // Skip it for now.
+            if (getSourceFileOfNode(declaration) !== currentSourceFile) {
                 return;
             }
 
-            let links = getNodeLinks(node);
+            let links = getNodeLinks(declaration);
             if (!links.collected) {
                 links.collected = true;
 
+                // Make sure this declaration can be emitted, that is it is accessible
+                // and is connected to its parent, or else report a accessibility error
+                ensureDeclarationVisible(declaration, errorNode);
+
                 // Collect any dependencies
-                collectReferencedDeclarations(node);
+                collectReferencedDeclarations(declaration);
 
                 // Collect children as well
-                switch (node.kind) {
+                switch (declaration.kind) {
                     case SyntaxKind.ModuleDeclaration:
                     case SyntaxKind.InterfaceDeclaration:
                     case SyntaxKind.ClassDeclaration:
                     case SyntaxKind.EnumDeclaration:
                     case SyntaxKind.VariableStatement:
-                        collectTopLevelChildDeclarations(node);
+                        collectTopLevelChildDeclarations(declaration);
                         break;
                 }
             }
         }
 
+        function reportDeclarationAccessiblityError(declaration: Node, errorNode: Node): boolean {
+            let node = errorNode;
+            while (node.parent.kind === SyntaxKind.QualifiedName || node.parent.kind === SyntaxKind.PropertyAccessExpression) {
+                node = node.parent;
+            }
+            let links = getNodeLinks(node);
+            if (!links.errorReported && !canWriteDeclaration(declaration)) {
+                links.errorReported = true;
+                reportDeclarationAccessiblityMessage(<Declaration>declaration, errorNode);
+            }
+            return links.errorReported;
+        }
+
         function collectTopLevelChildDeclarations(node: Node): void {
             forEachTopLevelDeclaration(node, child => {
                 if (isDeclarationExported(child)) {
-                    collectDeclaration(child);
+                    collectDeclatation(child);
                 }
             });
         }
@@ -1654,13 +1683,6 @@ namespace ts {
                     attachVisibleChild(parent, node);
                 }
             }
-
-            // Always report the error, even if the node was visited before
-            if (errorNode) {
-                if (!canWriteDeclaration(node)) {
-                    reportDeclarationAccessiblityMessage(<Declaration>node, errorNode);
-                }
-            }
         }
 
         function attachVisibleChild(node: Node, child: Node): void {
@@ -1676,6 +1698,11 @@ namespace ts {
 
             // Collect all visible declarations
             collectTopLevelChildDeclarations(sourceFile);
+
+            while (declarationsToProcess.length > 0) {
+                let {declaration, errorNode} = declarationsToProcess.shift();
+                preprocessDeclaration(declaration, errorNode);
+            }
         }
     }
 

tests/baselines/reference/declFileTypeAnnotationVisibilityErrorTypeLiteral.errors.txt

@@ -4,21 +4,17 @@ tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(13,13):
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(14,19): error TS4038: Return type of method 'method' references inaccessible name 'private1'.
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(15,22): error TS4049: Return type of index signature references inaccessible name 'private1'.
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(16,22): error TS4049: Return type of index signature references inaccessible name 'm2'.
-tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(18,16): error TS4025: Type of variable 'x2' references inaccessible name 'm2'.
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(18,16): error TS4025: Type of variable 'x2' references inaccessible name 'private1'.
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(25,16): error TS4025: Type of variable 'x3' references inaccessible name 'm2'.
-tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(25,16): error TS4025: Type of variable 'x3' references inaccessible name 'private1'.
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(28,23): error TS4030: Type of parameter 'a' references inaccessible name 'private1'.
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(28,36): error TS4025: Type of variable 'y' references inaccessible name 'm2'.
-tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(29,16): error TS4025: Type of variable 'y2' references inaccessible name 'm2'.
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(29,16): error TS4025: Type of variable 'y2' references inaccessible name 'private1'.
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(32,27): error TS4030: Type of parameter 'a' references inaccessible name 'private1'.
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(32,40): error TS4025: Type of variable 'z' references inaccessible name 'm2'.
-tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(33,16): error TS4025: Type of variable 'z2' references inaccessible name 'm2'.
 tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(33,16): error TS4025: Type of variable 'z2' references inaccessible name 'private1'.
 
 
-==== tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts (18 errors) ====
+==== tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts (14 errors) ====
 
     module m {
         class private1 {
@@ -50,8 +46,6 @@ tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(33,16):
         };
         export var x2 = {
                    ~~
-!!! error TS4025: Type of variable 'x2' references inaccessible name 'm2'.
-                   ~~
 !!! error TS4025: Type of variable 'x2' references inaccessible name 'private1'.
             x: new private1(),
             y: new m2.public1(),
@@ -62,8 +56,6 @@ tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(33,16):
         export var x3 = x;
                    ~~
 !!! error TS4025: Type of variable 'x3' references inaccessible name 'm2'.
-                   ~~
-!!! error TS4025: Type of variable 'x3' references inaccessible name 'private1'.
 
         // Function type
         export var y: (a: private1) => m2.public1;
@@ -73,8 +65,6 @@ tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(33,16):
 !!! error TS4025: Type of variable 'y' references inaccessible name 'm2'.
         export var y2 = y;
                    ~~
-!!! error TS4025: Type of variable 'y2' references inaccessible name 'm2'.
-                   ~~
 !!! error TS4025: Type of variable 'y2' references inaccessible name 'private1'.
 
         // constructor type
@@ -85,7 +75,5 @@ tests/cases/compiler/declFileTypeAnnotationVisibilityErrorTypeLiteral.ts(33,16):
 !!! error TS4025: Type of variable 'z' references inaccessible name 'm2'.
         export var z2 = z;
                    ~~
-!!! error TS4025: Type of variable 'z2' references inaccessible name 'm2'.
-                   ~~
 !!! error TS4025: Type of variable 'z2' references inaccessible name 'private1'.
     }

tests/baselines/reference/expressionInExtendsClauseWithVisibilityErrors.errors.txt

@@ -1,8 +1,7 @@
 tests/cases/compiler/expressionInExtendsClauseWithVisibilityErrors.ts(16,30): error TS4020: Extends clause references inaccessible name 'C'.
-tests/cases/compiler/expressionInExtendsClauseWithVisibilityErrors.ts(16,30): error TS4020: Extends clause references inaccessible name 'I'.
 
 
-==== tests/cases/compiler/expressionInExtendsClauseWithVisibilityErrors.ts (2 errors) ====
+==== tests/cases/compiler/expressionInExtendsClauseWithVisibilityErrors.ts (1 errors) ====
 
     interface I {
     }
@@ -21,8 +20,6 @@ tests/cases/compiler/expressionInExtendsClauseWithVisibilityErrors.ts(16,30): er
     export class MyClass extends getClass(2) <string, I> {
                                  ~~~~~~~~~~~~~~~~~~~~~~~
 !!! error TS4020: Extends clause references inaccessible name 'C'.
-                                 ~~~~~~~~~~~~~~~~~~~~~~~
-!!! error TS4020: Extends clause references inaccessible name 'I'.
     }