From 020029a6c95b2f887f621728e5714d9c011762e8 Mon Sep 17 00:00:00 2001 From: Corbin Uselton Date: Sat, 14 Jan 2017 15:49:41 -0800 Subject: [PATCH 1/7] Add check for node.body in referencer --- src/referencer.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/referencer.js b/src/referencer.js index bd81080..f577266 100644 --- a/src/referencer.js +++ b/src/referencer.js @@ -222,11 +222,13 @@ export default class Referencer extends esrecurse.Visitor { }); } - // Skip BlockStatement to prevent creating BlockStatement scope. - if (node.body.type === Syntax.BlockStatement) { - this.visitChildren(node.body); - } else { - this.visit(node.body); + if (node.body) { + // Skip BlockStatement to prevent creating BlockStatement scope. + if (node.body.type === Syntax.BlockStatement) { + this.visitChildren(node.body); + } else { + this.visit(node.body); + } } this.close(node); From e0e65a0cd6fa0fe85bfc343648ffdb4a0e1ed758 Mon Sep 17 00:00:00 2001 From: Corbin Uselton Date: Sun, 15 Jan 2017 09:10:44 -0800 Subject: [PATCH 2/7] Add comment explaining the node.body check --- src/referencer.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/referencer.js b/src/referencer.js index f577266..bb99130 100644 --- a/src/referencer.js +++ b/src/referencer.js @@ -222,6 +222,8 @@ export default class Referencer extends esrecurse.Visitor { }); } + // In TypeScript there are a number of function-like constructs which have no body, + // so check it exists before traversing if (node.body) { // Skip BlockStatement to prevent creating BlockStatement scope. if (node.body.type === Syntax.BlockStatement) { From 6779e1c2c7e552c06dd05cc3db132c62921816b5 Mon Sep 17 00:00:00 2001 From: Reyad Attiyat Date: Tue, 7 Feb 2017 20:42:38 -0600 Subject: [PATCH 3/7] Allow tests to run with babel We will remove this soon but need to add tests for new features in the mean time. --- gulpfile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gulpfile.js b/gulpfile.js index 64cc31d..c19373b 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -40,7 +40,7 @@ var gulp = require('gulp'), fs = require('fs'); require('babel-register')({ - only: /escope\/(src|test)\// + only: /eslint-scope\/(src|test)\// }); var TEST = [ 'test/*.js' ]; From daba8a73db6f506c9a08afab0757f0c113b88bc8 Mon Sep 17 00:00:00 2001 From: Reyad Attiyat Date: Tue, 7 Feb 2017 21:05:04 -0600 Subject: [PATCH 4/7] Add typescript-eslint-parser as dev dependency This used in tests for typescript features. --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 018dc7b..6f0cca3 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,8 @@ "gulp-tag-version": "^1.3.0", "jsdoc": "^3.4.0", "lazypipe": "^1.0.1", - "vinyl-source-stream": "^1.1.0" + "vinyl-source-stream": "^1.1.0", + "typescript-eslint-parser": "^1.0.0" }, "license": "BSD-2-Clause", "scripts": { From c35e6f321a0a972b18c386cb2584f42fe7126cac Mon Sep 17 00:00:00 2001 From: Reyad Attiyat Date: Tue, 7 Feb 2017 21:32:48 -0600 Subject: [PATCH 5/7] Strict scope should be false when there is no body This fixes an issue with typescript having an empty body for some functions --- src/scope.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/scope.js b/src/scope.js index 0e4d8c2..38ec0f0 100644 --- a/src/scope.js +++ b/src/scope.js @@ -61,6 +61,10 @@ function isStrictScope(scope, block, isMethodDefinition, useDirective) { } else { body = block.body; } + + if (!body) { + return false; + } } else if (scope.type === 'global') { body = block; } else { From ced5abec9940bf5ef357da1cea6899931e63a610 Mon Sep 17 00:00:00 2001 From: Reyad Attiyat Date: Tue, 7 Feb 2017 21:22:37 -0600 Subject: [PATCH 6/7] Add tests for typescript scope analysis This includes a test for multiple call signatures. --- test/typescript.js | 67 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 test/typescript.js diff --git a/test/typescript.js b/test/typescript.js new file mode 100644 index 0000000..866a010 --- /dev/null +++ b/test/typescript.js @@ -0,0 +1,67 @@ +/** + * @fileoverview Typescript scope tests + * @author Reyad Attiyat + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +require("babel-register"); + +const expect = require('chai').expect, + parse = require('typescript-eslint-parser').parse, + analyze = require('../src').analyze; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +describe('typescript', () => { + describe('multiple call signatures', () => { + it('should create a function scope', () => { + const ast = parse(` + function foo(bar: number): number; + function foo(bar: string): string; + function foo(bar: string | number): string | number { + return bar; + } + `); + + const scopeManager = analyze(ast); + + expect(scopeManager.scopes).to.have.length(4); + + const globalScope = scopeManager.scopes[0]; + expect(globalScope.type).to.be.equal('global'); + expect(globalScope.variables).to.have.length(1); + expect(globalScope.references).to.have.length(0); + expect(globalScope.isArgumentsMaterialized()).to.be.true; + + // Function scopes + let scope = scopeManager.scopes[1]; + expect(scope.type).to.be.equal('function'); + expect(scope.variables).to.have.length(2); + expect(scope.variables[0].name).to.be.equal('arguments'); + expect(scope.isArgumentsMaterialized()).to.be.false; + expect(scope.references).to.have.length(0); + + scope = scopeManager.scopes[2]; + expect(scope.type).to.be.equal('function'); + expect(scope.variables).to.have.length(2); + expect(scope.variables[0].name).to.be.equal('arguments'); + expect(scope.isArgumentsMaterialized()).to.be.false; + expect(scope.references).to.have.length(0); + + scope = scopeManager.scopes[3]; + expect(scope.type).to.be.equal('function'); + expect(scope.variables).to.have.length(2); + expect(scope.variables[0].name).to.be.equal('arguments'); + expect(scope.isArgumentsMaterialized()).to.be.false; + expect(scope.references).to.have.length(1); + + + }); + }); +}); From 9655efd676b9da656f61bb00fb36278c75a201fd Mon Sep 17 00:00:00 2001 From: Corbin Uselton Date: Sat, 14 Jan 2017 15:49:41 -0800 Subject: [PATCH 7/7] Add check for node.body in referencer Add comment explaining the node.body check Allow tests to run with babel We will remove this soon but need to add tests for new features in the mean time. Add typescript-eslint-parser as dev dependency This used in tests for typescript features. Strict scope should be false when there is no body This fixes an issue with typescript having an empty body for some functions Add tests for typescript scope analysis This includes a test for multiple call signatures. Add comment explaining the node.body check Allow tests to run with babel We will remove this soon but need to add tests for new features in the mean time. Strict scope should be false when there is no body This fixes an issue with typescript having an empty body for some functions Add tests for typescript scope analysis This includes a test for multiple call signatures. --- package.json | 3 ++- src/referencer.js | 14 ++++++---- src/scope.js | 4 +++ test/typescript.js | 67 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 test/typescript.js diff --git a/package.json b/package.json index fe019d1..6220196 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,7 @@ "gulp-sourcemaps": "^1.6.0", "gulp-tag-version": "^1.3.0", "lazypipe": "^1.0.1", - "vinyl-source-stream": "^1.1.0" + "vinyl-source-stream": "^1.1.0", + "typescript-eslint-parser": "^1.0.0" } } diff --git a/src/referencer.js b/src/referencer.js index bd81080..bb99130 100644 --- a/src/referencer.js +++ b/src/referencer.js @@ -222,11 +222,15 @@ export default class Referencer extends esrecurse.Visitor { }); } - // Skip BlockStatement to prevent creating BlockStatement scope. - if (node.body.type === Syntax.BlockStatement) { - this.visitChildren(node.body); - } else { - this.visit(node.body); + // In TypeScript there are a number of function-like constructs which have no body, + // so check it exists before traversing + if (node.body) { + // Skip BlockStatement to prevent creating BlockStatement scope. + if (node.body.type === Syntax.BlockStatement) { + this.visitChildren(node.body); + } else { + this.visit(node.body); + } } this.close(node); diff --git a/src/scope.js b/src/scope.js index 0e4d8c2..38ec0f0 100644 --- a/src/scope.js +++ b/src/scope.js @@ -61,6 +61,10 @@ function isStrictScope(scope, block, isMethodDefinition, useDirective) { } else { body = block.body; } + + if (!body) { + return false; + } } else if (scope.type === 'global') { body = block; } else { diff --git a/test/typescript.js b/test/typescript.js new file mode 100644 index 0000000..866a010 --- /dev/null +++ b/test/typescript.js @@ -0,0 +1,67 @@ +/** + * @fileoverview Typescript scope tests + * @author Reyad Attiyat + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +require("babel-register"); + +const expect = require('chai').expect, + parse = require('typescript-eslint-parser').parse, + analyze = require('../src').analyze; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +describe('typescript', () => { + describe('multiple call signatures', () => { + it('should create a function scope', () => { + const ast = parse(` + function foo(bar: number): number; + function foo(bar: string): string; + function foo(bar: string | number): string | number { + return bar; + } + `); + + const scopeManager = analyze(ast); + + expect(scopeManager.scopes).to.have.length(4); + + const globalScope = scopeManager.scopes[0]; + expect(globalScope.type).to.be.equal('global'); + expect(globalScope.variables).to.have.length(1); + expect(globalScope.references).to.have.length(0); + expect(globalScope.isArgumentsMaterialized()).to.be.true; + + // Function scopes + let scope = scopeManager.scopes[1]; + expect(scope.type).to.be.equal('function'); + expect(scope.variables).to.have.length(2); + expect(scope.variables[0].name).to.be.equal('arguments'); + expect(scope.isArgumentsMaterialized()).to.be.false; + expect(scope.references).to.have.length(0); + + scope = scopeManager.scopes[2]; + expect(scope.type).to.be.equal('function'); + expect(scope.variables).to.have.length(2); + expect(scope.variables[0].name).to.be.equal('arguments'); + expect(scope.isArgumentsMaterialized()).to.be.false; + expect(scope.references).to.have.length(0); + + scope = scopeManager.scopes[3]; + expect(scope.type).to.be.equal('function'); + expect(scope.variables).to.have.length(2); + expect(scope.variables[0].name).to.be.equal('arguments'); + expect(scope.isArgumentsMaterialized()).to.be.false; + expect(scope.references).to.have.length(1); + + + }); + }); +});