From a7cd69ec1dc43908c3b2aae4f92a830d96a318e3 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 30 Mar 2020 19:47:14 -0600 Subject: [PATCH 1/3] feat: call namespace as a warning --- src/ast/nodes/CallExpression.ts | 12 ++++----- src/ast/nodes/TaggedTemplateExpression.ts | 8 +++--- .../cannot-call-external-namespace/_config.js | 26 ++++++------------- .../cannot-call-external-namespace/main.js | 8 ++++-- .../cannot-call-internal-namespace/_config.js | 21 +++------------ .../cannot-call-internal-namespace/main.js | 5 +++- 6 files changed, 32 insertions(+), 48 deletions(-) diff --git a/src/ast/nodes/CallExpression.ts b/src/ast/nodes/CallExpression.ts index 90ee05daa53..bb336ce4339 100644 --- a/src/ast/nodes/CallExpression.ts +++ b/src/ast/nodes/CallExpression.ts @@ -3,7 +3,7 @@ import { BLANK } from '../../utils/blank'; import { findFirstOccurrenceOutsideComment, NodeRenderOptions, - RenderOptions + RenderOptions, } from '../../utils/renderHelpers'; import { CallOptions } from '../CallOptions'; import { DeoptimizableEntity } from '../DeoptimizableEntity'; @@ -13,7 +13,7 @@ import { ObjectPath, PathTracker, SHARED_RECURSION_TRACKER, - UNKNOWN_PATH + UNKNOWN_PATH, } from '../utils/PathTracker'; import { LiteralValueOrUnknown, UnknownValue, UNKNOWN_EXPRESSION } from '../values'; import Identifier from './Identifier'; @@ -40,10 +40,10 @@ export default class CallExpression extends NodeBase implements DeoptimizableEnt const variable = this.scope.findVariable(this.callee.name); if (variable.isNamespace) { - return this.context.error( + this.context.warn( { code: 'CANNOT_CALL_NAMESPACE', - message: `Cannot call a namespace ('${this.callee.name}')` + message: `Cannot call a namespace ('${this.callee.name}')`, }, this.start ); @@ -54,7 +54,7 @@ export default class CallExpression extends NodeBase implements DeoptimizableEnt { code: 'EVAL', message: `Use of eval is strongly discouraged, as it poses security risks and may cause issues with minification`, - url: 'https://rollupjs.org/guide/en/#avoiding-eval' + url: 'https://rollupjs.org/guide/en/#avoiding-eval', }, this.start ); @@ -212,7 +212,7 @@ export default class CallExpression extends NodeBase implements DeoptimizableEnt initialise() { this.callOptions = { args: this.arguments, - withNew: false + withNew: false, }; } diff --git a/src/ast/nodes/TaggedTemplateExpression.ts b/src/ast/nodes/TaggedTemplateExpression.ts index daadda5e85e..84e9dffc3fe 100644 --- a/src/ast/nodes/TaggedTemplateExpression.ts +++ b/src/ast/nodes/TaggedTemplateExpression.ts @@ -20,10 +20,10 @@ export default class TaggedTemplateExpression extends NodeBase { const variable = this.scope.findVariable(name); if (variable.isNamespace) { - return this.context.error( + this.context.warn( { code: 'CANNOT_CALL_NAMESPACE', - message: `Cannot call a namespace ('${name}')` + message: `Cannot call a namespace ('${name}')`, }, this.start ); @@ -34,7 +34,7 @@ export default class TaggedTemplateExpression extends NodeBase { { code: 'EVAL', message: `Use of eval is strongly discouraged, as it poses security risks and may cause issues with minification`, - url: 'https://rollupjs.org/guide/en/#avoiding-eval' + url: 'https://rollupjs.org/guide/en/#avoiding-eval', }, this.start ); @@ -52,7 +52,7 @@ export default class TaggedTemplateExpression extends NodeBase { initialise() { this.callOptions = { args: NO_ARGS, - withNew: false + withNew: false, }; } } diff --git a/test/function/samples/cannot-call-external-namespace/_config.js b/test/function/samples/cannot-call-external-namespace/_config.js index 1dd8124c147..4774f7de44b 100644 --- a/test/function/samples/cannot-call-external-namespace/_config.js +++ b/test/function/samples/cannot-call-external-namespace/_config.js @@ -1,21 +1,11 @@ -const path = require('path'); +const assert = require('assert'); module.exports = { - description: 'errors if code calls an external namespace', - error: { - code: 'CANNOT_CALL_NAMESPACE', - message: `Cannot call a namespace ('foo')`, - pos: 28, - watchFiles: [path.resolve(__dirname, 'main.js')], - loc: { - file: path.resolve(__dirname, 'main.js'), - line: 2, - column: 0 - }, - frame: ` - 1: import * as foo from 'foo'; - 2: foo(); - ^ - ` - } + description: 'warns if code calls an external namespace', + options: { + external: ['fs'], + }, + warnings(warnings) { + assert.deepStrictEqual(warnings.map(String), ["main.js (3:2) Cannot call a namespace ('foo')"]); + }, }; diff --git a/test/function/samples/cannot-call-external-namespace/main.js b/test/function/samples/cannot-call-external-namespace/main.js index c1ad7c6f778..7f58e3c0da6 100644 --- a/test/function/samples/cannot-call-external-namespace/main.js +++ b/test/function/samples/cannot-call-external-namespace/main.js @@ -1,2 +1,6 @@ -import * as foo from 'foo'; -foo(); +import * as foo from 'fs'; +try { + foo(); +} +catch (e) { +} diff --git a/test/function/samples/cannot-call-internal-namespace/_config.js b/test/function/samples/cannot-call-internal-namespace/_config.js index a75a74e7f5d..c96cd50f57d 100644 --- a/test/function/samples/cannot-call-internal-namespace/_config.js +++ b/test/function/samples/cannot-call-internal-namespace/_config.js @@ -1,21 +1,8 @@ -const path = require('path'); +const assert = require('assert'); module.exports = { description: 'errors if code calls an internal namespace', - error: { - code: 'CANNOT_CALL_NAMESPACE', - message: `Cannot call a namespace ('foo')`, - pos: 33, - watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')], - loc: { - file: path.resolve(__dirname, 'main.js'), - line: 2, - column: 0 - }, - frame: ` - 1: import * as foo from './foo.js'; - 2: foo(); - ^ - ` - } + warnings(warnings) { + assert.deepStrictEqual(warnings.map(String), ["main.js (3:2) Cannot call a namespace ('foo')"]); + }, }; diff --git a/test/function/samples/cannot-call-internal-namespace/main.js b/test/function/samples/cannot-call-internal-namespace/main.js index 74f0acb2482..745e43f541c 100644 --- a/test/function/samples/cannot-call-internal-namespace/main.js +++ b/test/function/samples/cannot-call-internal-namespace/main.js @@ -1,2 +1,5 @@ import * as foo from './foo.js'; -foo(); +try { + foo(); +} +catch {} From ee5a4feb20b068c2b47fc6fca7431b2b2b709a9a Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 31 Mar 2020 21:23:21 +0200 Subject: [PATCH 2/3] Fix prettier config --- .prettierrc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.prettierrc b/.prettierrc index 2a1740fd7ae..ea138e8454c 100644 --- a/.prettierrc +++ b/.prettierrc @@ -1,5 +1,7 @@ { + "arrowParens": "avoid", + "printWidth": 100, "singleQuote": true, - "useTabs": true, - "printWidth": 100 + "trailingComma": "none", + "useTabs": true } From acf0777483cc4a5af19fbd822df28be274e3087d Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 31 Mar 2020 21:27:12 +0200 Subject: [PATCH 3/3] Add tagged template literals to test to improve coverage --- .../samples/cannot-call-external-namespace/_config.js | 9 ++++++--- .../samples/cannot-call-external-namespace/main.js | 11 +++++++---- .../samples/cannot-call-internal-namespace/_config.js | 9 ++++++--- .../samples/cannot-call-internal-namespace/main.js | 10 +++++++--- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/test/function/samples/cannot-call-external-namespace/_config.js b/test/function/samples/cannot-call-external-namespace/_config.js index 4774f7de44b..20b6a35ada5 100644 --- a/test/function/samples/cannot-call-external-namespace/_config.js +++ b/test/function/samples/cannot-call-external-namespace/_config.js @@ -3,9 +3,12 @@ const assert = require('assert'); module.exports = { description: 'warns if code calls an external namespace', options: { - external: ['fs'], + external: ['fs'] }, warnings(warnings) { - assert.deepStrictEqual(warnings.map(String), ["main.js (3:2) Cannot call a namespace ('foo')"]); - }, + assert.deepStrictEqual(warnings.map(String), [ + "main.js (4:1) Cannot call a namespace ('foo')", + "main.js (8:1) Cannot call a namespace ('foo')" + ]); + } }; diff --git a/test/function/samples/cannot-call-external-namespace/main.js b/test/function/samples/cannot-call-external-namespace/main.js index 7f58e3c0da6..3c5998d3518 100644 --- a/test/function/samples/cannot-call-external-namespace/main.js +++ b/test/function/samples/cannot-call-external-namespace/main.js @@ -1,6 +1,9 @@ import * as foo from 'fs'; + try { - foo(); -} -catch (e) { -} + foo(); +} catch (e) {} + +try { + foo``; +} catch (e) {} diff --git a/test/function/samples/cannot-call-internal-namespace/_config.js b/test/function/samples/cannot-call-internal-namespace/_config.js index c96cd50f57d..b61157e6158 100644 --- a/test/function/samples/cannot-call-internal-namespace/_config.js +++ b/test/function/samples/cannot-call-internal-namespace/_config.js @@ -1,8 +1,11 @@ const assert = require('assert'); module.exports = { - description: 'errors if code calls an internal namespace', + description: 'warns if code calls an internal namespace', warnings(warnings) { - assert.deepStrictEqual(warnings.map(String), ["main.js (3:2) Cannot call a namespace ('foo')"]); - }, + assert.deepStrictEqual(warnings.map(String), [ + "main.js (4:1) Cannot call a namespace ('foo')", + "main.js (8:1) Cannot call a namespace ('foo')" + ]); + } }; diff --git a/test/function/samples/cannot-call-internal-namespace/main.js b/test/function/samples/cannot-call-internal-namespace/main.js index 745e43f541c..dcb5f372c44 100644 --- a/test/function/samples/cannot-call-internal-namespace/main.js +++ b/test/function/samples/cannot-call-internal-namespace/main.js @@ -1,5 +1,9 @@ import * as foo from './foo.js'; + try { - foo(); -} -catch {} + foo(); +} catch {} + +try { + foo``; +} catch {}