Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not include the whole namespace when illegally mutating a namespace #3637

Merged
merged 2 commits into from
Jun 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 12 additions & 10 deletions src/ast/nodes/MemberExpression.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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