fix(core): endless recursion in getTypeName()

close #66
mockdeep · Apr 17, 2018 · e949457 · e949457
1 parent 2c4e224
commit e949457
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 59 deletions.
diff --git a/packages/typewiz-core/src/integration.spec.ts b/packages/typewiz-core/src/integration.spec.ts
@@ -46,7 +46,7 @@ function typeWiz(input: string, typeCheck = false, options?: IApplyTypesOptions)
     applyTypes(collectedTypes, options);
 
     if (options && options.tsConfig) {
-        expect(options.tsConfigHost.readFile).toHaveBeenCalledWith(options.tsConfig);
+        expect(options.tsConfigHost!.readFile).toHaveBeenCalledWith(options.tsConfig);
     }
 
     if (mockFs.writeFileSync.mock.calls.length) {
@@ -447,6 +447,42 @@ describe('object types', () => {
     });
 });
 
+describe('regression tests', () => {
+    it('issue #66: endless recursion in getTypeName()', () => {
+        const input = `
+            function log(o) {
+                console.log(o);
+            }
+            function f(o) {
+                return o.someVal;
+            }
+            const obj = {
+                get someVal() {
+                    log(this);
+                    return 5;
+                }
+            };
+            f(obj);
+        `;
+
+        expect(typeWiz(input)).toBe(`
+            function log(o: { someVal: number }) {
+                console.log(o);
+            }
+            function f(o: { someVal: number }) {
+                return o.someVal;
+            }
+            const obj = {
+                get someVal() {
+                    log(this);
+                    return 5;
+                }
+            };
+            f(obj);
+        `);
+    });
+});
+
 describe('apply-types options', () => {
     describe('prefix', () => {
         it('should add the given prefix in front of the detected types', () => {

diff --git a/packages/typewiz-core/src/type-collector-snippet.spec.ts b/packages/typewiz-core/src/type-collector-snippet.spec.ts
@@ -49,6 +49,15 @@ describe('type-collector', () => {
             expect(() => $_$twiz.typeName(a)).toThrowError('NestError');
         });
 
+        it('should throw a NestError if invoked while already running (e.g. from an object getter)', () => {
+            const obj = {
+                get recurse() {
+                    return $_$twiz.typeName(obj);
+                },
+            };
+            expect(() => $_$twiz.typeName(obj)).toThrow('Called getTypeName() while it was already running');
+        });
+
         describe('functions', () => {
             /* tslint:disable:only-arrow-functions*/
             it('should return "() => any" for functions without arguments', () => {

diff --git a/packages/typewiz-core/src/type-collector-snippet.ts b/packages/typewiz-core/src/type-collector-snippet.ts
@@ -19,74 +19,86 @@ interface IKey {
     opts: ICollectedTypeInfo;
 }
 
+let typeNameRunning = false;
 export function getTypeName(value: any, nest = 0): string | null {
-    if (nest === 5) {
-        throw new NestError('NestError');
+    if (nest === 0 && typeNameRunning) {
+        throw new NestError('Called getTypeName() while it was already running');
     }
-    if (value === null) {
-        return 'null';
-    }
-    if (['undefined', 'number', 'string', 'boolean'].indexOf(typeof value) >= 0) {
-        return typeof value;
-    }
-    if (value instanceof Array) {
-        const itemTypes = Array.from(new Set(value.map((v) => getTypeName(v, nest + 1)))).filter((t) => t !== null);
-        if (itemTypes.length === 0) {
-            return null;
+    typeNameRunning = true;
+
+    try {
+        if (nest === 5) {
+            throw new NestError('NestError');
         }
-        if (itemTypes.length === 1) {
-            return itemTypes[0] + '[]';
+        if (value === null) {
+            return 'null';
         }
-        return `Array<${itemTypes.sort().join('|')}>`;
-    }
-    if (value instanceof Function) {
-        let argsStr: string = value.toString().split('=>')[0];
-
-        // make sure argsStr is in a form of (arg1,arg2) for the following cases
-        // fn = a => 3
-        // fn = (a) => 3
-        // function fn(a) { return 3 }
-
-        argsStr = argsStr.includes('(') ? (argsStr.match(/\(.*?\)/gi) || '()')[0] : `(${argsStr})`;
-        const args: string[] = argsStr
-            .replace(/[()]/g, '')
-            .split(',')
-            .filter((e: string) => e !== '');
-
-        const typedArgs = args.map((arg) => {
-            let [name] = arg.split('=');
-            name = name.trim();
-
-            if (name.includes('[')) {
-                const nakedName = name.replace(/\[|\]/gi, '').trim();
-                name = `${nakedName}Array`;
-                return `${name}: any`;
-            }
-            if (name.includes('{')) {
-                const nakedName = name.replace(/\{|\}/gi, '').trim();
-                name = `${nakedName}Object: {${nakedName}: any}`;
-                return `${name}`;
+        if (['undefined', 'number', 'string', 'boolean'].indexOf(typeof value) >= 0) {
+            return typeof value;
+        }
+        if (value instanceof Array) {
+            const itemTypes = Array.from(new Set(value.map((v) => getTypeName(v, nest + 1)))).filter((t) => t !== null);
+            if (itemTypes.length === 0) {
+                return null;
             }
-            if (name.includes('...')) {
-                name = `${name}Array: any[]`;
-                return `${name}`;
+            if (itemTypes.length === 1) {
+                return itemTypes[0] + '[]';
             }
+            return `Array<${itemTypes.sort().join('|')}>`;
+        }
+        if (value instanceof Function) {
+            let argsStr: string = value.toString().split('=>')[0];
+
+            // make sure argsStr is in a form of (arg1,arg2) for the following cases
+            // fn = a => 3
+            // fn = (a) => 3
+            // function fn(a) { return 3 }
+
+            argsStr = argsStr.includes('(') ? (argsStr.match(/\(.*?\)/gi) || '()')[0] : `(${argsStr})`;
+            const args: string[] = argsStr
+                .replace(/[()]/g, '')
+                .split(',')
+                .filter((e: string) => e !== '');
+
+            const typedArgs = args.map((arg) => {
+                let [name] = arg.split('=');
+                name = name.trim();
+
+                if (name.includes('[')) {
+                    const nakedName = name.replace(/\[|\]/gi, '').trim();
+                    name = `${nakedName}Array`;
+                    return `${name}: any`;
+                }
+                if (name.includes('{')) {
+                    const nakedName = name.replace(/\{|\}/gi, '').trim();
+                    name = `${nakedName}Object: {${nakedName}: any}`;
+                    return `${name}`;
+                }
+                if (name.includes('...')) {
+                    name = `${name}Array: any[]`;
+                    return `${name}`;
+                }
 
-            return `${name}: any`;
-        });
+                return `${name}: any`;
+            });
 
-        return `(${typedArgs}) => any`;
-    }
-    if (value.constructor && value.constructor.name) {
-        const { name } = value.constructor;
-        if (name === 'Object') {
-            return getObjectTypes(value, nest);
-        } else {
-            return name;
+            return `(${typedArgs}) => any`;
+        }
+        if (value.constructor && value.constructor.name) {
+            const { name } = value.constructor;
+            if (name === 'Object') {
+                return getObjectTypes(value, nest);
+            } else {
+                return name;
+            }
         }
-    }
 
-    return typeof value;
+        return typeof value;
+    } finally {
+        if (nest === 0) {
+            typeNameRunning = false;
+        }
+    }
 }
 
 function getObjectTypes(obj: any, nest: number): string {