fix!: isExported should use the type checker

This partially adds support for export declarations under the same name. BREAKING CHANGE: Global files previously had all declarations marked as not-exported. This was inconsistent with TypeScript's concept of visibility. Now non-modules will have all members exported.
TypeStrong · Dec 28, 2019 · 9c3114d · 9c3114d
1 parent f388b42
commit 9c3114d
Show file tree

Hide file tree

Showing 34 changed files with 236 additions and 209 deletions.
diff --git a/src/lib/converter/context.ts b/src/lib/converter/context.ts
@@ -151,6 +151,14 @@ export class Context {
         return nodeType;
     }
 
+    getSymbolAtLocation(node: ts.Node): ts.Symbol | undefined {
+        let symbol = this.checker.getSymbolAtLocation(node);
+        if (!symbol && isNamedNode(node)) {
+            symbol = this.checker.getSymbolAtLocation(node.name);
+        }
+        return symbol;
+    }
+
     /**
      * Return the current logger instance.
      *
@@ -390,3 +398,10 @@ export class Context {
         return typeParameters;
     }
 }
+
+function isNamedNode(node: ts.Node): node is ts.Node & { name: ts.Identifier | ts.ComputedPropertyName } {
+    return node.hasOwnProperty('name') && (
+        ts.isIdentifier(node['name']) ||
+        ts.isComputedPropertyName(node['name'])
+    );
+}
diff --git a/src/lib/converter/convert-expression.ts b/src/lib/converter/convert-expression.ts
@@ -26,8 +26,9 @@ export function convertExpression(expression: ts.Expression): string {
             return 'true';
         case ts.SyntaxKind.FalseKeyword:
             return 'false';
+        case ts.SyntaxKind.NullKeyword:
+            return 'null';
         default:
-            const source = expression.getSourceFile();
-            return source.text.substring(expression.pos, expression.end);
+            return expression.getText(expression.getSourceFile());
     }
 }
diff --git a/src/lib/converter/factories/declaration.ts b/src/lib/converter/factories/declaration.ts
@@ -71,21 +71,15 @@ export function createDeclaration(context: Context, node: ts.Declaration, kind:
 
     // Test whether the node is exported
     let isExported: boolean;
-    if (container.kindOf([ReflectionKind.Module, ReflectionKind.ExternalModule])) {
-        isExported = false; // Don't inherit exported state in modules and namespaces
+    if (kind === ReflectionKind.ExternalModule || kind === ReflectionKind.Global) {
+        isExported = true;
+    } else if (container.kindOf([ReflectionKind.Module, ReflectionKind.ExternalModule])) {
+        const symbol = context.getSymbolAtLocation(node);
+        isExported = !!symbol?.parent?.exports?.get(symbol.escapedName);
     } else {
         isExported = container.flags.isExported;
     }
 
-    if (kind === ReflectionKind.ExternalModule) {
-        isExported = true; // Always mark external modules as exported
-    } else if (node.parent && node.parent.kind === ts.SyntaxKind.VariableDeclarationList) {
-        const parentModifiers = ts.getCombinedModifierFlags(node.parent.parent as ts.Declaration);
-        isExported = isExported || !!(parentModifiers & ts.ModifierFlags.Export);
-    } else {
-        isExported = isExported || !!(modifiers & ts.ModifierFlags.Export);
-    }
-
     if (!isExported && context.converter.excludeNotExported) {
         return;
     }

diff --git a/src/lib/models/reflections/abstract.ts b/src/lib/models/reflections/abstract.ts
@@ -139,7 +139,18 @@ export class ReflectionFlags extends Array<string> {
     }
 
     /**
-     * Is this member exported?
+     * True if the reflection is exported from its containing declaration. Note that if a file
+     * has no imports or exports, then TS assumes that the file is in a global scope and *all*
+     * declarations are exported.
+     * ```ts
+     * // a.ts
+     * namespace A { // isExported = false
+     *   export const b = 1 // isExported = true, even though the container is false.
+     * }
+     * export const b = 2 // isExported = true
+     * // b.ts
+     * const c = 1 // isExported = true, no imports/exports
+     * ```
      */
     get isExported(): boolean {
         return this.hasFlag(ReflectionFlag.Exported);

diff --git a/src/test/converter.test.ts b/src/test/converter.test.ts
@@ -69,7 +69,8 @@ describe('Converter', function() {
             module: 'CommonJS',
             experimentalDecorators: true,
             jsx: 'react',
-            name: 'typedoc'
+            name: 'typedoc',
+            ignoreCompilerErrors: true
         });
     });
 
@@ -114,7 +115,8 @@ describe('Converter with categorizeByGroup=false', function() {
             experimentalDecorators: true,
             categorizeByGroup: false,
             jsx: 'react',
-            name: 'typedoc'
+            name: 'typedoc',
+            ignoreCompilerErrors: true
         });
     });
 
@@ -169,7 +171,8 @@ describe('Converter with excludeNotExported=true', function() {
             experimentalDecorators: true,
             excludeNotExported: true,
             jsx: 'react',
-            name: 'typedoc'
+            name: 'typedoc',
+            ignoreCompilerErrors: true
         });
     });
 

diff --git a/src/test/converter/array/specs.json b/src/test/converter/array/specs.json
@@ -4057,7 +4057,7 @@
               }
             }
           },
-          "defaultValue": " []"
+          "defaultValue": "[]"
         },
         {
           "id": 233,

diff --git a/src/test/converter/constructor-properties/constructor-properties.ts b/src/test/converter/constructor-properties/constructor-properties.ts
@@ -1,32 +1,31 @@
 /**
  * A class with constructor properties.
  */
-class Vector2
-{
+class Vector2 {
     /**
      * @param x  X component of the Vector
      * @param y  Y component of the Vector
      * @param name Vector name
      */
-    constructor(public x:number, public y:number,
+    constructor(public x: number, public y: number,
                 readonly name: string) {
     }
 }
 
-
 /**
  * A class with inherited and overwritten constructor properties.
  */
-class Vector3 extends Vector2
-{
+class Vector3 extends Vector2 {
     /**
      * @param x  X component of the Vector
      * @param y  Y component of the Vector
      * @param z  Z component of the Vector
      * @param name Vector name
      */
-    constructor(x:number, public y:number, public z:number,
+    constructor(x: number, public y: number, public z: number,
                 readonly name: string) {
         super(x, y, name);
     }
 }
+
+export {};
diff --git a/src/test/converter/constructor-properties/specs.json b/src/test/converter/constructor-properties/specs.json
@@ -93,8 +93,8 @@
               "sources": [
                 {
                   "fileName": "constructor-properties.ts",
-                  "line": 5,
-                  "character": 1
+                  "line": 4,
+                  "character": 15
                 }
               ]
             },
@@ -112,7 +112,7 @@
               "sources": [
                 {
                   "fileName": "constructor-properties.ts",
-                  "line": 12,
+                  "line": 11,
                   "character": 29
                 }
               ],
@@ -136,7 +136,7 @@
               "sources": [
                 {
                   "fileName": "constructor-properties.ts",
-                  "line": 11,
+                  "line": 10,
                   "character": 24
                 }
               ],
@@ -160,8 +160,8 @@
               "sources": [
                 {
                   "fileName": "constructor-properties.ts",
-                  "line": 11,
-                  "character": 41
+                  "line": 10,
+                  "character": 42
                 }
               ],
               "type": {
@@ -301,8 +301,8 @@
               "sources": [
                 {
                   "fileName": "constructor-properties.ts",
-                  "line": 21,
-                  "character": 1
+                  "line": 18,
+                  "character": 31
                 }
               ],
               "overwrites": {
@@ -325,7 +325,7 @@
               "sources": [
                 {
                   "fileName": "constructor-properties.ts",
-                  "line": 29,
+                  "line": 26,
                   "character": 29
                 }
               ],
@@ -354,7 +354,7 @@
               "sources": [
                 {
                   "fileName": "constructor-properties.ts",
-                  "line": 11,
+                  "line": 10,
                   "character": 24
                 }
               ],
@@ -383,8 +383,8 @@
               "sources": [
                 {
                   "fileName": "constructor-properties.ts",
-                  "line": 28,
-                  "character": 34
+                  "line": 25,
+                  "character": 35
                 }
               ],
               "type": {
@@ -412,8 +412,8 @@
               "sources": [
                 {
                   "fileName": "constructor-properties.ts",
-                  "line": 28,
-                  "character": 51
+                  "line": 25,
+                  "character": 53
                 }
               ],
               "type": {
@@ -444,7 +444,7 @@
           "sources": [
             {
               "fileName": "constructor-properties.ts",
-              "line": 20,
+              "line": 18,
               "character": 13
             }
           ],

diff --git a/src/test/converter/decorators/decorators.ts b/src/test/converter/decorators/decorators.ts
@@ -4,8 +4,7 @@
 @decoratorWithOptions({
     name: 'Name of class'
 })
-class DecoratedClass
-{
+class DecoratedClass {
     /**
      * A decorated method.
      */
@@ -14,35 +13,33 @@ class DecoratedClass
     decoratedMethod() { }
 }
 
-
 /**
  * A decorator with no options.
  */
-function decoratorAtom(target:Object, propertyKey:string|symbol, descriptor:TypedPropertyDescriptor<any>) {
+function decoratorAtom(target: Object, propertyKey: string|symbol, descriptor: TypedPropertyDescriptor<any>) {
     target[propertyKey].writable = true;
 }
 
-
 /**
  * A decorator with a parameter.
  *
  * @param value  The parameter of this decorator.
  */
-function decoratorWithParam(value:boolean):MethodDecorator {
-    return function (target:Object, propertyKey:string|symbol, descriptor:TypedPropertyDescriptor<any>) {
+function decoratorWithParam(value: boolean): MethodDecorator {
+    return function (target: Object, propertyKey: string|symbol, descriptor: TypedPropertyDescriptor<any>) {
         target[propertyKey].enumerable = value;
-    }
+    };
 }
 
-
 /**
  * A decorator consuming an options object.
  *
  * @param options  The options object of this decorator.
  * @param options.name  A property on the options object of this decorator.
  */
-function decoratorWithOptions(options:{name:string}): ClassDecorator {
+function decoratorWithOptions(options: {name: string}): ClassDecorator {
     return function (target) {
         (target as any).options = options;
     };
 }
+export {}
diff --git a/src/test/converter/decorators/specs.json b/src/test/converter/decorators/specs.json
@@ -83,7 +83,7 @@
               "sources": [
                 {
                   "fileName": "decorators.ts",
-                  "line": 14,
+                  "line": 13,
                   "character": 19
                 }
               ]
@@ -188,7 +188,7 @@
           "sources": [
             {
               "fileName": "decorators.ts",
-              "line": 21,
+              "line": 19,
               "character": 22
             }
           ]
@@ -247,8 +247,8 @@
                           "sources": [
                             {
                               "fileName": "decorators.ts",
-                              "line": 44,
-                              "character": 43
+                              "line": 40,
+                              "character": 44
                             }
                           ],
                           "type": {
@@ -269,7 +269,7 @@
                       "sources": [
                         {
                           "fileName": "decorators.ts",
-                          "line": 44,
+                          "line": 40,
                           "character": 38
                         }
                       ]
@@ -286,7 +286,7 @@
           "sources": [
             {
               "fileName": "decorators.ts",
-              "line": 44,
+              "line": 40,
               "character": 29
             }
           ]
@@ -339,7 +339,7 @@
           "sources": [
             {
               "fileName": "decorators.ts",
-              "line": 31,
+              "line": 28,
               "character": 27
             }
           ]

diff --git a/src/test/converter/destructuring/destructuring.ts b/src/test/converter/destructuring/destructuring.ts
@@ -1,8 +1,7 @@
 /**
  * Destructuring objects.
  */
-const {destructObjectA, destructObjectB, destructObjectC} = {destructObjectA:0, destructObjectB:'string', destructObjectC:0};
-
+const {destructObjectA, destructObjectB, destructObjectC} = {destructObjectA: 0, destructObjectB: 'string', destructObjectC: 0};
 
 /**
  * Destructuring arrays.
@@ -22,4 +21,6 @@ const [destructArrayWithIgnoresA, , ...destructArrayWithIgnoresRest] = [1, 2, 3,
 /**
  * Destructuring function parameters.
  */
-function drawText({text = "", location:[x, y] = [0, 0], bold = false}) { }
+function drawText({text = '', location: [x, y] = [0, 0], bold = false}) { }
+
+export {};