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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from Jun 17, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44479fe
Do not include the whole namespace when illegally mutating a namespace
lukastaegert Jun 14, 2020
fbe4c7a
Merge branch 'master' into refine-illegal-namespace-access-handling
lukastaegert Jun 17, 2020
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
View file
Open in desktop
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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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
View file
Open in desktop
@@ -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