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

Throw errors when imports are used in unsupported formats #492

Merged
merged 3 commits into from
Feb 25, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Released: TBD
### New features

### Bug fixes
- [#490](https://github.com/peggyjs/peggy/issues/490) Throw error when imports are used in unsupported formats. Supported formats are now only "es" and "commonjs".

4.0.1
-----
Expand Down
12 changes: 10 additions & 2 deletions docs/documentation.html
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,16 @@ <h3 id="importing-external-rules">Importing External Rules</h3>
grammar, NOT the source. Grammars MUST be compiled by a version that
supports imports in order to be imported. Only rules that are allowed
start rules are valid. It can be useful to specify
<code>--allowed-start-rules \*</code> in library grammars. All of the
following are valid:</p>
<code>--allowed-start-rules \*</code> in library grammars. Imports are
only valid in output formats "es" and "commonjs". If you
use imports, you should use <code>{ output: "source" }</code>; the default
output of "parser" will call `eval` on the source which fails immediately
for some formats (e.g. "es") and will not find modules in the expected
places for others (e.g. "commonjs"). The
<a href="https://github.com/peggyjs/from-mem/">from-mem</a> project is
used by the Peggy CLI to resolve these issues, but note well its
relatively severe limitations.</p>
<p>All of the following are valid:</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something is missing here or am i wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed the format issue for the link. What else do you see?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing otherwise seems okay

<ul>
<li><code>import * as num from "number.js" // Call with num.number</code></li>
<li><code>import num from "number.js" // Calls the default rule</code></li>
Expand Down
2 changes: 1 addition & 1 deletion docs/js/benchmark-bundle.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/js/test-bundle.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/vendor/peggy/peggy.min.js

Large diffs are not rendered by default.

30 changes: 21 additions & 9 deletions lib/compiler/passes/generate-js.js
Original file line number Diff line number Diff line change
Expand Up @@ -1451,6 +1451,10 @@ function generateJS(ast, options) {

const generators = {
bare() {
if ((Object.keys(dependencies).length > 0)
|| (ast.imports.length > 0)) {
throw new Error("Dependencies not supported in format 'bare'.");
}
return [
...generateGeneratedByComment(),
"(function() {",
Expand Down Expand Up @@ -1564,12 +1568,12 @@ function generateJS(ast, options) {
},

amd() {
if (ast.imports.length > 0) {
throw new Error("Imports are not supported in format 'amd'.");
}

const dependencyVars = Object.keys(dependencies);
const dependencyIds = dependencyVars.map(v => dependencies[v]);
for (let i = 0; i < ast.imports.length; i++) {
dependencyVars.push(gi(i));
dependencyIds.push(ast.imports[i].from.module);
}
const deps = "["
+ dependencyIds.map(
id => "\"" + stringEscape(id) + "\""
Expand All @@ -1590,6 +1594,14 @@ function generateJS(ast, options) {
},

globals() {
if ((Object.keys(dependencies).length > 0)
|| (ast.imports.length > 0)) {
throw new Error("Dependencies not supported in format 'globals'.");
}
if (!options.exportVar) {
throw new Error("No export variable defined");
}

return [
...generateGeneratedByComment(),
"(function(root) {",
Expand All @@ -1603,12 +1615,12 @@ function generateJS(ast, options) {
},

umd() {
if (ast.imports.length > 0) {
throw new Error("Imports are not supported in format 'umd'.");
}

const dependencyVars = Object.keys(dependencies);
const dependencyIds = dependencyVars.map(v => dependencies[v]);
for (let i = 0; i < ast.imports.length; i++) {
dependencyVars.push(gi(i));
dependencyIds.push(ast.imports[i].from.module);
}
const deps = "["
+ dependencyIds.map(
id => "\"" + stringEscape(id) + "\""
Expand All @@ -1628,7 +1640,7 @@ function generateJS(ast, options) {
" module.exports = factory(" + requires + ");"
);

if (options.exportVar !== null) {
if (options.exportVar) {
parts.push(
" } else {",
" root." + options.exportVar + " = factory();"
Expand Down
47 changes: 45 additions & 2 deletions test/cli/run.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,18 @@ Options:
expected: /c = require\("commander"\)/,
});

await exec({
args: ["-D", '{"c": "commander", "jest": "jest"}', "--format", "amd"],
stdin: "foo = '1' { return new c.Command(); }",
expected: /define\(\["commander", "jest"\]/,
});

await exec({
args: ["-D", '{"c": "commander", "jest": "jest"}', "--format", "umd", "-e", "foo"],
stdin: "foo = '1' { return new c.Command(); }",
expected: /define\(\["commander", "jest"\]/,
});

await exec({
args: ["-D", '{"c": "commander"}', "-d", "c:jest"],
stdin: "foo = '1' { return c.run(); }",
Expand Down Expand Up @@ -566,6 +578,13 @@ Options:
expected: /^\s*root\.football = /m,
});

await exec({
args: ["--format", "globals"],
stdin: "foo = '1'",
exitCode: 1,
expected: "Error parsing grammar\nNo export variable defined\n",
});

await exec({
args: ["--export-var"],
stdin: "foo = '1'",
Expand All @@ -585,7 +604,7 @@ Options:

it("handles extra options", async() => {
await exec({
args: ["--extra-options", '{"format": "amd"}'],
args: ["-d", "fs", "--extra-options", '{"format": "amd"}'],
stdin: 'foo = "1"',
expected: /^define\(/m,
});
Expand Down Expand Up @@ -1221,7 +1240,7 @@ Error: Expected "1" but end of input found.
error: /Unsupported output format/,
});
await exec({
args: ["--format", "globals", "-t", "1", grammar],
args: ["--format", "globals", "-e", "foo", "-t", "1", grammar],
error: /Unsupported output format/,
});
await exec({
Expand Down Expand Up @@ -1263,6 +1282,30 @@ error: Rule "unknownRule" is not defined

const { parse } = await import(impjs);
expect(parse("baz")).toBe("baz");

await exec({
args: [imps, "--format", "globals"],
exitCode: 1,
expected: "Error parsing grammar\nDependencies not supported in format 'globals'.\n",
});

await exec({
args: [imps, "--format", "bare"],
exitCode: 1,
expected: "Error parsing grammar\nDependencies not supported in format 'bare'.\n",
});

await exec({
args: [imps, "--format", "amd"],
exitCode: 1,
expected: "Error parsing grammar\nImports are not supported in format 'amd'.\n",
});

await exec({
args: [imps, "--format", "umd"],
exitCode: 1,
expected: "Error parsing grammar\nImports are not supported in format 'umd'.\n",
});
});

it("produces library-style output", async() => {
Expand Down