From 96703ca57a6e5dbd4a8a9b61dd0c72bb3e7171ba Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Fri, 29 Nov 2019 13:01:04 +0100 Subject: [PATCH 1/2] fix race in test case --- test/Compiler-caching.test.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/Compiler-caching.test.js b/test/Compiler-caching.test.js index 9c3df9488..e9b448f02 100644 --- a/test/Compiler-caching.test.js +++ b/test/Compiler-caching.test.js @@ -111,12 +111,8 @@ describe("Compiler (caching)", () => { // Copy over file since we"ll be modifying some of them fs.mkdirSync(fixturePath); - fs.createReadStream(path.join(__dirname, "fixtures", "a.js")).pipe( - fs.createWriteStream(aFilepath) - ); - fs.createReadStream(path.join(__dirname, "fixtures", "c.js")).pipe( - fs.createWriteStream(cFilepath) - ); + fs.copyFileSync(path.join(__dirname, "fixtures", "a.js"), aFilepath); + fs.copyFileSync(path.join(__dirname, "fixtures", "c.js"), cFilepath); fixtureCount++; return { From 2f3da77d386c95c9da3b335bf74c195d4499fed4 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Fri, 29 Nov 2019 20:24:13 +0100 Subject: [PATCH 2/2] Introduce a Parser base class to improve Parser types Parser.parse must be always sync make JSON and WASM modules strict fix inherit-types tooling to omit static methods --- lib/NormalModule.js | 45 ++++--------- lib/Parser.js | 34 ++++++++++ lib/asset/AssetParser.js | 17 ++++- lib/index.js | 1 + lib/javascript/JavascriptParser.js | 67 +++++++++++++------ lib/json/JsonParser.js | 17 +++-- lib/wasm-async/AsyncWebAssemblyParser.js | 22 ++++-- lib/wasm/WebAssemblyParser.js | 21 ++++-- test/JavascriptParser.unittest.js | 14 ++-- .../__snapshots__/StatsTestCases.test.js.snap | 8 +-- .../localization/webpack.config.js | 3 +- tooling/inherit-types.js | 5 +- 12 files changed, 175 insertions(+), 79 deletions(-) create mode 100644 lib/Parser.js diff --git a/lib/NormalModule.js b/lib/NormalModule.js index be9e97309..0136034fb 100644 --- a/lib/NormalModule.js +++ b/lib/NormalModule.js @@ -42,20 +42,13 @@ const makeSerializable = require("./util/makeSerializable"); /** @typedef {import("./Module").CodeGenerationResult} CodeGenerationResult */ /** @typedef {import("./Module").LibIdentOptions} LibIdentOptions */ /** @typedef {import("./Module").NeedBuildContext} NeedBuildContext */ +/** @typedef {import("./Parser")} Parser */ /** @typedef {import("./RequestShortener")} RequestShortener */ /** @typedef {import("./ResolverFactory").ResolverWithOptions} ResolverWithOptions */ /** @typedef {import("./RuntimeTemplate")} RuntimeTemplate */ /** @typedef {import("./util/Hash")} Hash */ /** @typedef {import("./util/fs").InputFileSystem} InputFileSystem */ -/** - * @typedef {Object} ParserState - * @property {NormalModule} current - * @property {NormalModule} module - * @property {Compilation} compilation - * @property {TODO} options - */ - /** * @param {string} context absolute context path * @param {string} source a source path @@ -160,7 +153,7 @@ class NormalModule extends Module { * @param {TODO[]} options.loaders list of loaders * @param {string} options.resource path + query of the real resource * @param {string | undefined} options.matchResource path + query of the matched resource (virtuel - * @param {TODO} options.parser the parser used + * @param {Parser} options.parser the parser used * @param {Generator} options.generator the generator used * @param {Object} options.resolveOptions options used for resolving requests from this module */ @@ -187,6 +180,7 @@ class NormalModule extends Module { this.rawRequest = rawRequest; /** @type {boolean} */ this.binary = /^(asset|webassembly)\b/.test(type); + /** @type {Parser} */ this.parser = parser; /** @type {Generator} */ this.generator = generator; @@ -730,34 +724,19 @@ class NormalModule extends Module { return callback(); }; - let inTry = true; + let result; try { - const result = this.parser.parse( - this._ast || this._source.source(), - { - current: this, - module: this, - compilation: compilation, - options: options - }, - (err, result) => { - inTry = false; - if (err) { - handleParseError(err); - } else { - handleParseResult(result); - } - } - ); - inTry = false; - if (result !== undefined) { - // parse is sync - handleParseResult(result); - } + result = this.parser.parse(this._ast || this._source.source(), { + current: this, + module: this, + compilation: compilation, + options: options + }); } catch (e) { - if (!inTry) throw e; handleParseError(e); + return; } + handleParseResult(result); }); } diff --git a/lib/Parser.js b/lib/Parser.js new file mode 100644 index 000000000..fe59f70e2 --- /dev/null +++ b/lib/Parser.js @@ -0,0 +1,34 @@ +/* + MIT License http://www.opensource.org/licenses/mit-license.php + Author Tobias Koppers @sokra +*/ + +"use strict"; + +const AbstractMethodError = require("./AbstractMethodError"); + +/** @typedef {import("./Compilation")} Compilation */ +/** @typedef {import("./NormalModule")} NormalModule */ + +/** @typedef {Record} PreparsedAst */ + +/** + * @typedef {Object} ParserState + * @property {NormalModule} current + * @property {NormalModule} module + * @property {Compilation} compilation + * @property {TODO} options + */ + +class Parser { + /** + * @param {string | Buffer | PreparsedAst} source the source to parse + * @param {ParserState} state the parser state + * @returns {ParserState} the parser state + */ + parse(source, state) { + throw new AbstractMethodError(); + } +} + +module.exports = Parser; diff --git a/lib/asset/AssetParser.js b/lib/asset/AssetParser.js index e8c9bd04f..e316efd6e 100644 --- a/lib/asset/AssetParser.js +++ b/lib/asset/AssetParser.js @@ -5,17 +5,30 @@ "use strict"; -/** @typedef {import("../../declarations/plugins/AssetModulesPluginParser").AssetModulesPluginParserOptions} AssetModulesPluginParserOptions */ +const Parser = require("../Parser"); -class AssetParser { +/** @typedef {import("../../declarations/plugins/AssetModulesPluginParser").AssetModulesPluginParserOptions} AssetModulesPluginParserOptions */ +/** @typedef {import("../Parser").ParserState} ParserState */ +/** @typedef {import("../Parser").PreparsedAst} PreparsedAst */ + +class AssetParser extends Parser { /** * @param {AssetModulesPluginParserOptions["dataUrlCondition"] | boolean} dataUrlCondition condition for inlining as DataUrl */ constructor(dataUrlCondition) { + super(); this.dataUrlCondition = dataUrlCondition; } + /** + * @param {string | Buffer | PreparsedAst} source the source to parse + * @param {ParserState} state the parser state + * @returns {ParserState} the parser state + */ parse(source, state) { + if (typeof source === "object" && !Buffer.isBuffer(source)) { + throw new Error("AssetParser doesn't accept preparsed AST"); + } state.module.buildInfo.strict = true; state.module.buildMeta.exportsType = "default"; diff --git a/lib/index.js b/lib/index.js index b9fe069ad..9ca0a666d 100644 --- a/lib/index.js +++ b/lib/index.js @@ -71,6 +71,7 @@ exportPlugins(module.exports, { NormalModule: () => require("./NormalModule"), NormalModuleReplacementPlugin: () => require("./NormalModuleReplacementPlugin"), + Parser: () => require("./Parser"), PrefetchPlugin: () => require("./PrefetchPlugin"), ProgressPlugin: () => require("./ProgressPlugin"), ProvidePlugin: () => require("./ProvidePlugin"), diff --git a/lib/javascript/JavascriptParser.js b/lib/javascript/JavascriptParser.js index 9210903af..307e00d17 100644 --- a/lib/javascript/JavascriptParser.js +++ b/lib/javascript/JavascriptParser.js @@ -5,12 +5,14 @@ "use strict"; -const { Parser } = require("acorn"); +const { Parser: AcornParser } = require("acorn"); const { SyncBailHook, HookMap } = require("tapable"); const vm = require("vm"); +const Parser = require("../Parser"); const StackedMap = require("../util/StackedMap"); const BasicEvaluatedExpression = require("./BasicEvaluatedExpression"); +/** @typedef {import("acorn").Options} AcornOptions */ /** @typedef {import("estree").ArrayExpression} ArrayExpressionNode */ /** @typedef {import("estree").BinaryExpression} BinaryExpressionNode */ /** @typedef {import("estree").BlockStatement} BlockStatementNode */ @@ -26,6 +28,7 @@ const BasicEvaluatedExpression = require("./BasicEvaluatedExpression"); /** @typedef {import("estree").LogicalExpression} LogicalExpressionNode */ /** @typedef {import("estree").MemberExpression} MemberExpressionNode */ /** @typedef {import("estree").ModuleDeclaration} ModuleDeclarationNode */ +/** @typedef {import("estree").Node} AnyNode */ /** @typedef {import("estree").Program} ProgramNode */ /** @typedef {import("estree").Statement} StatementNode */ /** @typedef {import("estree").Super} SuperNode */ @@ -35,10 +38,12 @@ const BasicEvaluatedExpression = require("./BasicEvaluatedExpression"); /** @typedef {import("estree").UnaryExpression} UnaryExpressionNode */ /** @typedef {import("estree").VariableDeclarator} VariableDeclaratorNode */ /** @template T @typedef {import("tapable").AsArray} AsArray */ +/** @typedef {import("../Parser").ParserState} ParserState */ +/** @typedef {import("../Parser").PreparsedAst} PreparsedAst */ // Syntax: https://developer.mozilla.org/en/SpiderMonkey/Parser_API -const parser = Parser.extend(require("../parsing/importAwaitAcornPlugin")); +const parser = AcornParser.extend(require("../parsing/importAwaitAcornPlugin")); class VariableInfo { /** @@ -55,6 +60,7 @@ class VariableInfo { /** @typedef {string | ScopeInfo | VariableInfo} ExportedVariableInfo */ /** @typedef {LiteralNode | string | null | undefined} ImportSource */ +/** @typedef {Omit & { sourceType: "module" | "script" | "auto" }} ParseOptions */ /** * @typedef {Object} TagInfo @@ -78,6 +84,7 @@ const joinRanges = (startRange, endRange) => { return [startRange[0], endRange[1]]; }; +/** @type {AcornOptions} */ const defaultParserOptions = { ranges: true, locations: true, @@ -95,8 +102,13 @@ const EMPTY_COMMENT_OPTIONS = { errors: null }; -class JavascriptParser { +class JavascriptParser extends Parser { + /** + * @param {TODO} options options + * @param {"module" | "script" | "auto"} sourceType default source type + */ constructor(options, sourceType = "auto") { + super(); this.hooks = Object.freeze({ /** @type {HookMap>} */ evaluateTypeof: new HookMap(() => new SyncBailHook(["expression"])), @@ -2624,12 +2636,23 @@ class JavascriptParser { }; } - parse(source, initialState) { + /** + * @param {string | Buffer | PreparsedAst} source the source to parse + * @param {ParserState} state the parser state + * @returns {ParserState} the parser state + */ + parse(source, state) { let ast; let comments; const semicolons = new Set(); - if (typeof source === "object" && source !== null) { - ast = source; + if (source === null) { + throw new Error("source must not be null"); + } + if (Buffer.isBuffer(source)) { + source = source.toString("utf-8"); + } + if (typeof source === "object") { + ast = /** @type {ProgramNode} */ (source); comments = source.comments; } else { comments = []; @@ -2654,7 +2677,7 @@ class JavascriptParser { isStrict: false, definitions: new StackedMap() }; - const state = (this.state = initialState || {}); + this.state = state; this.comments = comments; this.semicolons = semicolons; this.statementEndPos = NaN; @@ -2874,24 +2897,27 @@ class JavascriptParser { }; } + /** + * @param {string} code source code + * @param {ParseOptions} options parsing options + * @returns {ProgramNode} parsed ast + */ static parse(code, options) { const type = options ? options.sourceType : "module"; + /** @type {AcornOptions} */ const parserOptions = { ...defaultParserOptions, - ...options + allowReturnOutsideFunction: type === "script", + ...options, + sourceType: type === "auto" ? "module" : type }; - if (type === "auto") { - parserOptions.sourceType = "module"; - } else if (parserOptions.sourceType === "script") { - parserOptions.allowReturnOutsideFunction = true; - } - + /** @type {AnyNode} */ let ast; let error; let threw = false; try { - ast = parser.parse(code, parserOptions); + ast = /** @type {AnyNode} */ (parser.parse(code, parserOptions)); } catch (e) { error = e; threw = true; @@ -2899,15 +2925,18 @@ class JavascriptParser { if (threw && type === "auto") { parserOptions.sourceType = "script"; - parserOptions.allowReturnOutsideFunction = true; + if (!("allowReturnOutsideFunction" in options)) { + parserOptions.allowReturnOutsideFunction = true; + } if (Array.isArray(parserOptions.onComment)) { parserOptions.onComment.length = 0; } try { - ast = parser.parse(code, parserOptions); + ast = /** @type {AnyNode} */ (parser.parse(code, parserOptions)); threw = false; } catch (e) { - threw = true; + // we use the error from first parse try + // so nothing to do here } } @@ -2915,7 +2944,7 @@ class JavascriptParser { throw error; } - return ast; + return /** @type {ProgramNode} */ (ast); } } diff --git a/lib/json/JsonParser.js b/lib/json/JsonParser.js index 74682ac95..6451bac2f 100644 --- a/lib/json/JsonParser.js +++ b/lib/json/JsonParser.js @@ -6,19 +6,28 @@ "use strict"; const parseJson = require("json-parse-better-errors"); +const Parser = require("../Parser"); const JsonExportsDependency = require("../dependencies/JsonExportsDependency"); -/** @typedef {import("../NormalModule").ParserState} ParserState */ +/** @typedef {import("../Parser").ParserState} ParserState */ +/** @typedef {import("../Parser").PreparsedAst} PreparsedAst */ -class JsonParser { +class JsonParser extends Parser { /** - * @param {string} source the source to parse + * @param {string | Buffer | PreparsedAst} source the source to parse * @param {ParserState} state the parser state * @returns {ParserState} the parser state */ parse(source, state) { - const data = parseJson(source[0] === "\ufeff" ? source.slice(1) : source); + if (Buffer.isBuffer(source)) { + source = source.toString("utf-8"); + } + const data = + typeof source === "object" + ? source + : parseJson(source[0] === "\ufeff" ? source.slice(1) : source); state.module.buildInfo.jsonData = data; + state.module.buildInfo.strict = true; state.module.buildMeta.exportsType = "default"; state.module.addDependency( new JsonExportsDependency(JsonExportsDependency.getExportsFromData(data)) diff --git a/lib/wasm-async/AsyncWebAssemblyParser.js b/lib/wasm-async/AsyncWebAssemblyParser.js index ae93c4004..e784ba718 100644 --- a/lib/wasm-async/AsyncWebAssemblyParser.js +++ b/lib/wasm-async/AsyncWebAssemblyParser.js @@ -7,10 +7,13 @@ const t = require("@webassemblyjs/ast"); const { decode } = require("@webassemblyjs/wasm-parser"); - +const Parser = require("../Parser"); const StaticExportsDependency = require("../dependencies/StaticExportsDependency"); const WebAssemblyImportDependency = require("../dependencies/WebAssemblyImportDependency"); +/** @typedef {import("../Parser").ParserState} ParserState */ +/** @typedef {import("../Parser").PreparsedAst} PreparsedAst */ + const decoderOpts = { ignoreCodeSection: true, ignoreDataSection: true, @@ -19,19 +22,30 @@ const decoderOpts = { ignoreCustomNameSection: true }; -class WebAssemblyParser { +class WebAssemblyParser extends Parser { constructor(options) { + super(); this.hooks = Object.freeze({}); this.options = options; } - parse(binary, state) { + /** + * @param {string | Buffer | PreparsedAst} source the source to parse + * @param {ParserState} state the parser state + * @returns {ParserState} the parser state + */ + parse(source, state) { + if (!Buffer.isBuffer(source)) { + throw new Error("WebAssemblyParser input must be a Buffer"); + } + // flag it as async module + state.module.buildInfo.strict = true; state.module.buildMeta.exportsType = "namespace"; state.module.buildMeta.async = true; // parse it - const program = decode(binary, decoderOpts); + const program = decode(source, decoderOpts); const module = program.body[0]; const exports = []; diff --git a/lib/wasm/WebAssemblyParser.js b/lib/wasm/WebAssemblyParser.js index 6a0618e81..de4a59eb7 100644 --- a/lib/wasm/WebAssemblyParser.js +++ b/lib/wasm/WebAssemblyParser.js @@ -10,12 +10,14 @@ const { moduleContextFromModuleAST } = require("@webassemblyjs/helper-module-context"); const { decode } = require("@webassemblyjs/wasm-parser"); - +const Parser = require("../Parser"); const StaticExportsDependency = require("../dependencies/StaticExportsDependency"); const WebAssemblyExportImportedDependency = require("../dependencies/WebAssemblyExportImportedDependency"); const WebAssemblyImportDependency = require("../dependencies/WebAssemblyImportDependency"); /** @typedef {import("../Module")} Module */ +/** @typedef {import("../Parser").ParserState} ParserState */ +/** @typedef {import("../Parser").PreparsedAst} PreparsedAst */ const JS_COMPAT_TYPES = new Set(["i32", "f32", "f64"]); @@ -60,18 +62,29 @@ const decoderOpts = { ignoreCustomNameSection: true }; -class WebAssemblyParser { +class WebAssemblyParser extends Parser { constructor(options) { + super(); this.hooks = Object.freeze({}); this.options = options; } - parse(binary, state) { + /** + * @param {string | Buffer | PreparsedAst} source the source to parse + * @param {ParserState} state the parser state + * @returns {ParserState} the parser state + */ + parse(source, state) { + if (!Buffer.isBuffer(source)) { + throw new Error("WebAssemblyParser input must be a Buffer"); + } + // flag it as ESM + state.module.buildInfo.strict = true; state.module.buildMeta.exportsType = "namespace"; // parse it - const program = decode(binary, decoderOpts); + const program = decode(source, decoderOpts); const module = program.body[0]; const moduleContext = moduleContextFromModuleAST(module); diff --git a/test/JavascriptParser.unittest.js b/test/JavascriptParser.unittest.js index f608f6165..74456632f 100644 --- a/test/JavascriptParser.unittest.js +++ b/test/JavascriptParser.unittest.js @@ -321,7 +321,7 @@ describe("JavascriptParser", () => { testParser.state.xyz.push(testParser.parseString(expr.arguments[0])); return true; }); - const actual = testParser.parse(source); + const actual = testParser.parse(source, {}); expect(typeof actual).toBe("object"); expect(actual).toEqual(state); }); @@ -347,7 +347,7 @@ describe("JavascriptParser", () => { return true; }); - const actual = testParser.parse(source); + const actual = testParser.parse(source, {}); expect(typeof actual).toBe("object"); expect(typeof actual.comments).toBe("object"); actual.comments.forEach((element, index) => { @@ -376,7 +376,7 @@ describe("JavascriptParser", () => { .tap("JavascriptParserTest", expr => new BasicEvaluatedExpression().setNumber(123).setRange(expr.range) ); - return parser.parse("test(" + source + ");").result; + return parser.parse("test(" + source + ");", {}).result; } const testCases = { @@ -585,7 +585,7 @@ describe("JavascriptParser", () => { Object.keys(cases).forEach(name => { const expr = cases[name]; it(name, () => { - const actual = parser.parse(expr); + const actual = parser.parse(expr, {}); expect(typeof actual).toBe("object"); }); }); @@ -618,7 +618,7 @@ describe("JavascriptParser", () => { Object.keys(cases).forEach(name => { it(name, () => { - const actual = parser.parse(cases[name][0]); + const actual = parser.parse(cases[name][0], {}); expect(actual).toEqual(cases[name][1]); }); }); @@ -634,7 +634,7 @@ describe("JavascriptParser", () => { Object.keys(cases).forEach(name => { const expr = cases[name]; it(name, () => { - const actual = JavascriptParser.parse(expr); + const actual = JavascriptParser.parse(expr, {}); expect(typeof actual).toBe("object"); }); }); @@ -650,7 +650,7 @@ describe("JavascriptParser", () => { return true; }); - parser.parse("const { a, ...rest } = { a: 1, b: 2 };"); + parser.parse("const { a, ...rest } = { a: 1, b: 2 };", {}); expect(definitions.has("a")).toBe(true); expect(definitions.has("rest")).toBe(true); diff --git a/test/__snapshots__/StatsTestCases.test.js.snap b/test/__snapshots__/StatsTestCases.test.js.snap index 83a4e40fe..43081d4e4 100644 --- a/test/__snapshots__/StatsTestCases.test.js.snap +++ b/test/__snapshots__/StatsTestCases.test.js.snap @@ -3765,12 +3765,12 @@ Built at: 1970-04-20 12:42:42 Asset Size 1d55f77c08cd19684f13.module.wasm 154 bytes [emitted] [immutable] 200c03abdc3f4ae1e15c.module.wasm 290 bytes [emitted] [immutable] - 230.bundle.js 207 bytes [emitted] + 230.bundle.js 221 bytes [emitted] 256e72dd8b9a83a6e45b.module.wasm 120 bytes [emitted] [immutable] - 325.bundle.js 3.71 KiB [emitted] + 325.bundle.js 3.74 KiB [emitted] 526.bundle.js 368 bytes [emitted] [id hint: vendors] - 780.bundle.js 496 bytes [emitted] - 99.bundle.js 205 bytes [emitted] + 780.bundle.js 524 bytes [emitted] + 99.bundle.js 219 bytes [emitted] a0e9dd97d7ced35a5b2c.module.wasm 154 bytes [emitted] [immutable] bundle.js 11 KiB [emitted] [name: main-1df31ce3] d37b3336426771c2a6e2.module.wasm 531 bytes [emitted] [immutable] diff --git a/test/configCases/custom-source-type/localization/webpack.config.js b/test/configCases/custom-source-type/localization/webpack.config.js index 6f32a1f5f..b853c09f7 100644 --- a/test/configCases/custom-source-type/localization/webpack.config.js +++ b/test/configCases/custom-source-type/localization/webpack.config.js @@ -2,11 +2,12 @@ const { RawSource } = require("webpack-sources"); const Generator = require("../../../../").Generator; const RuntimeModule = require("../../../../").RuntimeModule; const RuntimeGlobals = require("../../../../").RuntimeGlobals; +const Parser = require("../../../../").Parser; const webpack = require("../../../../"); /** @typedef {import("../../../../lib/Compiler")} Compiler */ -class LocalizationParser { +class LocalizationParser extends Parser { parse(source, { module }) { module.buildInfo.content = JSON.parse(source); return true; diff --git a/tooling/inherit-types.js b/tooling/inherit-types.js index 869386e23..bf3c7d95f 100644 --- a/tooling/inherit-types.js +++ b/tooling/inherit-types.js @@ -76,7 +76,10 @@ for (const sourceFile of program.getSourceFiles()) { const nodeHandler = node => { if (ts.isClassDeclaration(node) || ts.isClassExpression(node)) { for (const member of node.members) { - if (ts.isMethodDeclaration(member)) { + if ( + ts.isMethodDeclaration(member) && + !(ts.getCombinedModifierFlags(member) & ts.ModifierFlags.Static) + ) { const baseDecl = findDeclarationInBaseClass( node, member.name.getText()