Skip to content

Commit

Permalink
Do not include the whole namespace when illegally mutating a namespace (
Browse files Browse the repository at this point in the history
  • Loading branch information
lukastaegert committed Jun 17, 2020
1 parent 6d977a3 commit cc2182d
Show file tree
Hide file tree
Showing 34 changed files with 148 additions and 90 deletions.
22 changes: 12 additions & 10 deletions src/ast/nodes/MemberExpression.ts
Expand Up @@ -93,16 +93,16 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
const baseVariable = path && this.scope.findVariable(path[0].key);
if (baseVariable && baseVariable.isNamespace) {
const resolvedVariable = this.resolveNamespaceVariables(baseVariable, path!.slice(1));
if (!resolvedVariable) {
super.bind();
} else if (typeof resolvedVariable === 'string') {
this.replacement = resolvedVariable;
} else {
if (resolvedVariable instanceof ExternalVariable && resolvedVariable.module) {
resolvedVariable.module.suggestName(path![0].key);
if (resolvedVariable) {
if (typeof resolvedVariable === 'string') {
this.replacement = resolvedVariable;
} else {
if (resolvedVariable instanceof ExternalVariable && resolvedVariable.module) {
resolvedVariable.module.suggestName(path![0].key);
}
this.variable = resolvedVariable;
this.scope.addNamespaceMemberAccess(getStringFromPath(path!), resolvedVariable);
}
this.variable = resolvedVariable;
this.scope.addNamespaceMemberAccess(getStringFromPath(path!), resolvedVariable);
}
} else {
super.bind();
Expand Down Expand Up @@ -261,7 +261,9 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
if (this.object instanceof Identifier) {
const variable = this.scope.findVariable(this.object.name);
if (variable.isNamespace) {
variable.include();
if (this.variable) {
this.context.includeVariable(this.variable);
}
this.context.warn(
{
code: 'ILLEGAL_NAMESPACE_REASSIGNMENT',
Expand Down
8 changes: 4 additions & 4 deletions test/cli/samples/config-no-module/_config.js
@@ -1,17 +1,17 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'provides a helpful error message if a transpiled config is interpreted as "module"',
minNodeVersion: 13,
command: 'cd sub && rollup -c',
error: () => true,
stderr: (stderr) =>
assertStderrIncludes(
stderr: stderr =>
assertIncludes(
stderr,
'[!] Error: While loading the Rollup configuration from "rollup.config.js", Node tried to require an ES module from a CommonJS ' +
'file, which is not supported. A common cause is if there is a package.json file with "type": "module" in the same folder. You can ' +
'try to fix this by changing the extension of your configuration file to ".cjs" or ".mjs" depending on the content, which will ' +
'prevent Rollup from trying to preprocess the file but rather hand it to Node directly.\n' +
'https://rollupjs.org/guide/en/#using-untranspiled-config-files'
),
)
};
4 changes: 2 additions & 2 deletions test/cli/samples/config-warnings/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'displays warnings when a config is loaded',
command: 'rollup -c',
stderr: stderr =>
assertStderrIncludes(
assertIncludes(
stderr,
'loaded rollup.config.js with warnings\n(!) Use of eval is strongly discouraged'
)
Expand Down
4 changes: 2 additions & 2 deletions test/cli/samples/custom-frame-with-pos/_config.js
@@ -1,11 +1,11 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'custom (plugin generated) code frame taking priority over pos generated one',
command: 'rollup -c',
error: () => true,
stderr: stderr =>
assertStderrIncludes(
assertIncludes(
stderr,
'[!] (plugin at position 1) Error: My error.\n' +
'main.js (1:5)\n' +
Expand Down
6 changes: 3 additions & 3 deletions test/cli/samples/custom-frame/_config.js
@@ -1,16 +1,16 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'errors with plugin generated code frames also contain stack',
command: 'rollup -c',
error: () => true,
stderr: stderr => {
assertStderrIncludes(
assertIncludes(
stderr,
'[!] (plugin at position 1) Error: My error.\n' +
'main.js\ncustom code frame\nError: My error.\n' +
' at Object.transform'
);
assertStderrIncludes(stderr, 'rollup.config.js:11:17');
assertIncludes(stderr, 'rollup.config.js:11:17');
}
};
4 changes: 2 additions & 2 deletions test/cli/samples/duplicate-import-options/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'throws if different types of entries are combined',
command: 'rollup main.js --format es --input main.js',
error: () => true,
stderr(stderr) {
assertStderrIncludes(stderr, '[!] Either use --input, or pass input path as argument');
assertIncludes(stderr, '[!] Either use --input, or pass input path as argument');
}
};
4 changes: 2 additions & 2 deletions test/cli/samples/empty-chunk-multiple/_config.js
@@ -1,8 +1,8 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'shows warning when multiple chunks empty',
command: 'rollup -c',
error: () => true,
stderr: stderr => assertStderrIncludes(stderr, '(!) Generated empty chunks\na, b')
stderr: stderr => assertIncludes(stderr, '(!) Generated empty chunks\na, b')
};
4 changes: 2 additions & 2 deletions test/cli/samples/empty-chunk/_config.js
@@ -1,8 +1,8 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'shows warning when chunk empty',
command: 'rollup -c',
error: () => true,
stderr: stderr => assertStderrIncludes(stderr, '(!) Generated an empty chunk\nmain')
stderr: stderr => assertIncludes(stderr, '(!) Generated an empty chunk\nmain')
};
4 changes: 2 additions & 2 deletions test/cli/samples/node-config-not-found/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'throws if a config in node_modules cannot be found',
command: 'rollup --config node:baz',
error: () => true,
stderr(stderr) {
assertStderrIncludes(stderr, '[!] Could not resolve config file "node:baz"');
assertIncludes(stderr, '[!] Could not resolve config file "node:baz"');
}
};
4 changes: 2 additions & 2 deletions test/cli/samples/plugin/cannot-load/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../../utils.js');
const { assertIncludes } = require('../../../../utils.js');

module.exports = {
description: 'unknown CLI --plugin results in an error',
skipIfWindows: true,
command: `echo "console.log(123);" | rollup --plugin foobar`,
error(err) {
assertStderrIncludes(err.message, '[!] Error: Cannot load plugin "foobar"');
assertIncludes(err.message, '[!] Error: Cannot load plugin "foobar"');
}
};
4 changes: 2 additions & 2 deletions test/cli/samples/plugin/invalid-argument/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../../utils.js');
const { assertIncludes } = require('../../../../utils.js');

module.exports = {
description: 'invalid CLI --plugin argument format',
skipIfWindows: true,
command: `echo "console.log(123);" | rollup --plugin 'foo bar'`,
error(err) {
assertStderrIncludes(err.message, '[!] Error: Invalid --plugin argument format: "foo bar"');
assertIncludes(err.message, '[!] Error: Invalid --plugin argument format: "foo bar"');
}
};
4 changes: 2 additions & 2 deletions test/cli/samples/stdin/no-stdin-tty/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../../utils.js');
const { assertIncludes } = require('../../../../utils.js');

module.exports = {
description: 'does not use input as stdin on TTY interfaces',
skipIfWindows: true,
command: `echo "console.log('PASS');" | ./wrapper.js -f es`,
error(err) {
assertStderrIncludes(err.message, 'You must supply options.input to rollup');
assertIncludes(err.message, 'You must supply options.input to rollup');
}
};
4 changes: 2 additions & 2 deletions test/cli/samples/stdin/stdin-error/_config.js
@@ -1,9 +1,9 @@
const { assertStderrIncludes } = require('../../../../utils.js');
const { assertIncludes } = require('../../../../utils.js');

module.exports = {
description: 'handles stdin errors',
command: `node wrapper.js`,
error(err) {
assertStderrIncludes(err.message, 'Could not load -: Stream is broken.');
assertIncludes(err.message, 'Could not load -: Stream is broken.');
}
};
11 changes: 4 additions & 7 deletions test/cli/samples/stdout-only-inline-sourcemaps/_config.js
@@ -1,13 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'fails when using non-inline sourcemaps when bundling to stdout',
command: 'rollup -i main.js -f es -m',
error: () => true,
stderr: (stderr) => {
assertStderrIncludes(
stderr,
'[!] Only inline sourcemaps are supported when bundling to stdout.\n'
);
},
stderr: stderr => {
assertIncludes(stderr, '[!] Only inline sourcemaps are supported when bundling to stdout.\n');
}
};
4 changes: 2 additions & 2 deletions test/cli/samples/warn-broken-sourcemap/_config.js
@@ -1,14 +1,14 @@
const fs = require('fs');
const path = require('path');
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'displays warnings for broken sourcemaps',
command: 'rollup -c',
stderr: stderr => {
fs.unlinkSync(path.resolve(__dirname, 'bundle'));
fs.unlinkSync(path.resolve(__dirname, 'bundle.map'));
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Broken sourcemap\n' +
'https://rollupjs.org/guide/en/#warning-sourcemap-is-likely-to-be-incorrect\n' +
Expand Down
4 changes: 2 additions & 2 deletions test/cli/samples/warn-broken-sourcemaps/_config.js
@@ -1,14 +1,14 @@
const fs = require('fs');
const path = require('path');
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'displays warnings for broken sourcemaps',
command: 'rollup -c',
stderr: stderr => {
fs.unlinkSync(path.resolve(__dirname, 'bundle'));
fs.unlinkSync(path.resolve(__dirname, 'bundle.map'));
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Broken sourcemap\n' +
'https://rollupjs.org/guide/en/#warning-sourcemap-is-likely-to-be-incorrect\n' +
Expand Down
4 changes: 2 additions & 2 deletions test/cli/samples/warn-circular-multiple/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'warns for multiple circular dependencies',
command: 'rollup -c',
stderr: stderr =>
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Circular dependencies\n' +
'main.js -> dep1.js -> main.js\n' +
Expand Down
4 changes: 2 additions & 2 deletions test/cli/samples/warn-circular/_config.js
@@ -1,9 +1,9 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'warns for circular dependencies',
command: 'rollup -c',
stderr(stderr) {
assertStderrIncludes(stderr, '(!) Circular dependency\nmain.js -> dep.js -> main.js\n');
assertIncludes(stderr, '(!) Circular dependency\nmain.js -> dep.js -> main.js\n');
}
};
4 changes: 2 additions & 2 deletions test/cli/samples/warn-eval-multiple/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'warns when eval is used multiple times',
command: 'rollup -c',
stderr: stderr =>
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Use of eval is strongly discouraged\n' +
'https://rollupjs.org/guide/en/#avoiding-eval\n' +
Expand Down
4 changes: 2 additions & 2 deletions test/cli/samples/warn-eval/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'warns when eval is used',
command: 'rollup -c',
stderr: stderr =>
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Use of eval is strongly discouraged\n' +
'https://rollupjs.org/guide/en/#avoiding-eval\n' +
Expand Down
14 changes: 7 additions & 7 deletions test/cli/samples/warn-import-export/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'warns about import and export related issues',
command: 'rollup -c',
stderr: stderr => {
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Mixing named and default exports\n' +
'https://rollupjs.org/guide/en/#output-exports\n' +
Expand All @@ -13,12 +13,12 @@ module.exports = {
'\n' +
"Consumers of your bundle will have to use chunk['default'] to access their default export, which may not be what you want. Use `output.exports: 'named'` to disable this warning\n"
);
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Unused external imports\n' +
"default imported from external module 'external' but never used\n"
);
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Import of non-existent export\n' +
'main.js\n' +
Expand All @@ -28,7 +28,7 @@ module.exports = {
' ^\n' +
"4: import 'unresolvedExternal';\n"
);
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Missing exports\n' +
'https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module\n' +
Expand All @@ -40,13 +40,13 @@ module.exports = {
' ^\n' +
'7: export default 42;\n'
);
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Conflicting re-exports\n' +
"main.js re-exports 'foo' from both dep.js and dep2.js (will be ignored)\n" +
"main.js re-exports 'bar' from both dep.js and dep2.js (will be ignored)\n"
);
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Unresolved dependencies\n' +
'https://rollupjs.org/guide/en/#warning-treating-module-as-external-dependency\n' +
Expand Down
4 changes: 2 additions & 2 deletions test/cli/samples/warn-missing-global-multiple/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'warns when there are multiple missing globals',
command: 'rollup -c',
stderr: stderr =>
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Missing global variable names\n' +
'Use output.globals to specify browser global variable names corresponding to external modules\n' +
Expand Down
4 changes: 2 additions & 2 deletions test/cli/samples/warn-missing-global/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'warns when there is a missing global variable name',
command: 'rollup -c',
stderr: stderr =>
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Missing global variable name\n' +
'Use output.globals to specify browser global variable names corresponding to external modules\n' +
Expand Down
4 changes: 2 additions & 2 deletions test/cli/samples/warn-mixed-exports-multiple/_config.js
@@ -1,10 +1,10 @@
const { assertStderrIncludes } = require('../../../utils.js');
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'warns when mixed exports are used',
command: 'rollup -c',
stderr: stderr => {
assertStderrIncludes(
assertIncludes(
stderr,
'(!) Mixing named and default exports\n' +
'https://rollupjs.org/guide/en/#output-exports\n' +
Expand Down

0 comments on commit cc2182d

Please sign in to comment.