From 3de37654256220da361f84af9df46105f519dafd Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Tue, 14 Sep 2021 10:00:04 -0700 Subject: [PATCH] Add build-time code exclusion using code fencing (#12060) This PR adds build-time code exclusion by means of code fencing. For details, please see the README in `./development/build/transforms`. Note that linting of transformed files as a form of validation is added in a follow-up, #12075. Hopefully exhaustive tests are added to ensure that the transform works according to its specification. Since these tests are Node-only, they required their own Jest config. The recommended way to work with multiple Jest configs is using the `projects` field in the Jest config, however [that feature breaks coverage collection](https://github.com/facebook/jest/issues/9628). That being the case, I had to set up two separate Jest configs. In order to get both test suites to run in parallel, Jest is now invoked via a script, `./test/run-jest.sh`. By way of example, this build system feature allows us to add fences like this: ```javascript this.store.updateStructure({ ..., GasFeeController: this.gasFeeController, TokenListController: this.tokenListController, ///: BEGIN:ONLY_INCLUDE_IN(beta) PluginController: this.pluginController, ///: END:ONLY_INCLUDE_IN }); ``` Which at build time are transformed to the following if the build type is not `beta`: ```javascript this.store.updateStructure({ ..., GasFeeController: this.gasFeeController, TokenListController: this.tokenListController, }); ``` Co-authored-by: Mark Stacey --- .depcheckrc.yml | 13 +- .eslintrc.js | 28 +- development/build/index.js | 9 +- development/build/scripts.js | 46 +- development/build/transforms/README.md | 123 ++++ .../build/transforms/remove-fenced-code.js | 434 +++++++++++++ .../transforms/remove-fenced-code.test.js | 591 ++++++++++++++++++ development/build/utils.js | 7 + development/jest.config.js | 19 + jest.config.js | 18 +- package.json | 9 +- test/run-jest.sh | 10 + 12 files changed, 1262 insertions(+), 45 deletions(-) create mode 100644 development/build/transforms/README.md create mode 100644 development/build/transforms/remove-fenced-code.js create mode 100644 development/build/transforms/remove-fenced-code.test.js create mode 100644 development/jest.config.js create mode 100755 test/run-jest.sh diff --git a/.depcheckrc.yml b/.depcheckrc.yml index b1892acb9..70214b911 100644 --- a/.depcheckrc.yml +++ b/.depcheckrc.yml @@ -11,21 +11,22 @@ ignores: # # dev deps # - + # safety fallback for npm lifecycle scripts, not used normally - "@lavamoat/preinstall-always-fail" # used in testing + ci - "@metamask/auto-changelog" # invoked as `auto-changelog` - "@metamask/forwarder" - "@metamask/test-dapp" - - "chromedriver" - - "geckodriver" - - "ganache-cli" - - "lavamoat-viz" - "@sentry/cli" # invoked as `sentry-cli` + - "chromedriver" + - "depcheck" # ooo meta + - "ganache-cli" + - "geckodriver" + - "jest" + - "lavamoat-viz" - "prettier-plugin-sort-json" # automatically imported by prettier - "source-map-explorer" - - "depcheck" # ooo meta # development tool - "yarn-deduplicate" # storybook diff --git a/.eslintrc.js b/.eslintrc.js index b6094c794..14d5482de 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -53,6 +53,26 @@ module.exports = { 'prefer-object-spread': 'error', 'require-atomic-updates': 'off', + // This is the same as our default config, but for the noted exceptions + 'spaced-comment': [ + 'error', + 'always', + { + markers: [ + 'global', + 'globals', + 'eslint', + 'eslint-disable', + '*package', + '!', + ',', + // Local additions + '/:', // This is for our code fences + ], + exceptions: ['=', '-'], + }, + ], + 'import/no-unassigned-import': 'off', 'no-invalid-this': 'off', @@ -112,6 +132,7 @@ module.exports = { 'ui/**/*.test.js', 'ui/__mocks__/*.js', 'shared/**/*.test.js', + 'development/**/*.test.js', ], extends: ['@metamask/eslint-config-mocha'], rules: { @@ -129,7 +150,12 @@ module.exports = { }, }, { - files: ['ui/**/*.test.js', 'ui/__mocks__/*.js', 'shared/**/*.test.js'], + files: [ + 'ui/**/*.test.js', + 'ui/__mocks__/*.js', + 'shared/**/*.test.js', + 'development/**/*.test.js', + ], extends: ['@metamask/eslint-config-jest'], rules: { 'jest/no-restricted-matchers': 'off', diff --git a/development/build/index.js b/development/build/index.js index f3be5b184..1dbb95752 100755 --- a/development/build/index.js +++ b/development/build/index.js @@ -17,7 +17,7 @@ const createScriptTasks = require('./scripts'); const createStyleTasks = require('./styles'); const createStaticAssetTasks = require('./static'); const createEtcTasks = require('./etc'); -const { getNextBetaVersionMap } = require('./utils'); +const { BuildTypes, getNextBetaVersionMap } = require('./utils'); // packages required dynamically via browserify configuration in dependencies require('loose-envify'); @@ -150,11 +150,6 @@ function parseArgv() { SkipStats: 'skip-stats', }; - const BuildTypes = { - beta: 'beta', - main: 'main', - }; - const argv = minimist(process.argv.slice(2), { boolean: [NamedArgs.OmitLockdown, NamedArgs.SkipStats], string: [NamedArgs.BuildType], @@ -191,7 +186,7 @@ function parseArgv() { betaVersion: String(betaVersion), buildType, entryTask, - isBeta: argv[NamedArgs.BuildType] === 'beta', + isBeta: argv[NamedArgs.BuildType] === BuildTypes.beta, isLavaMoat: process.argv[0].includes('lavamoat'), shouldIncludeLockdown: argv[NamedArgs.OmitLockdown], skipStats: argv[NamedArgs.SkipStats], diff --git a/development/build/scripts.js b/development/build/scripts.js index fbcbbf50a..baef60c13 100644 --- a/development/build/scripts.js +++ b/development/build/scripts.js @@ -44,6 +44,9 @@ const { composeSeries, runInChildProcess, } = require('./task'); +const { + createRemoveFencedCodeTransform, +} = require('./transforms/remove-fenced-code'); module.exports = createScriptTasks; @@ -144,7 +147,12 @@ function createScriptTasks({ disableConsoleSubtask, installSentrySubtask, phishingDetectSubtask, - ].map((subtask) => runInChildProcess(subtask, { buildType, isLavaMoat })); + ].map((subtask) => + runInChildProcess(subtask, { + buildType, + isLavaMoat, + }), + ); // make a parent task that runs each task in a child thread return composeParallel(initiateLiveReload, ...allSubtasks); } @@ -231,10 +239,11 @@ function createFactoredBuild({ const envVars = getEnvironmentVariables({ buildType, devMode, testing }); setupBundlerDefaults(buildConfiguration, { + buildType, devMode, envVars, - reloadOnChange, minify, + reloadOnChange, }); // set bundle entries @@ -349,10 +358,11 @@ function createNormalBundle({ const envVars = getEnvironmentVariables({ buildType, devMode, testing }); setupBundlerDefaults(buildConfiguration, { + buildType, devMode, envVars, - reloadOnChange, minify, + reloadOnChange, }); // set bundle entries @@ -399,35 +409,37 @@ function createBuildConfiguration() { function setupBundlerDefaults( buildConfiguration, - { devMode, envVars, reloadOnChange, minify }, + { buildType, devMode, envVars, minify, reloadOnChange }, ) { const { bundlerOpts } = buildConfiguration; Object.assign(bundlerOpts, { - // source transforms + // Source transforms transform: [ - // transpile top-level code + // Remove code that should be excluded from builds of the current type + createRemoveFencedCodeTransform(buildType), + // Transpile top-level code babelify, - // inline `fs.readFileSync` files + // Inline `fs.readFileSync` files brfs, ], - // use entryFilepath for moduleIds, easier to determine origin file + // Use entryFilepath for moduleIds, easier to determine origin file fullPaths: devMode, - // for sourcemaps + // For sourcemaps debug: true, }); - // ensure react-devtools are not included in non-dev builds + // Ensure react-devtools are not included in non-dev builds if (!devMode) { bundlerOpts.manualIgnore.push('react-devtools'); } - // inject environment variables via node-style `process.env` + // Inject environment variables via node-style `process.env` if (envVars) { bundlerOpts.transform.push([envify(envVars), { global: true }]); } - // setup reload on change + // Setup reload on change if (reloadOnChange) { setupReloadOnChange(buildConfiguration); } @@ -436,21 +448,21 @@ function setupBundlerDefaults( setupMinification(buildConfiguration); } - // setup source maps + // Setup source maps setupSourcemaps(buildConfiguration, { devMode }); } function setupReloadOnChange({ bundlerOpts, events }) { - // add plugin to options + // Add plugin to options Object.assign(bundlerOpts, { plugin: [...bundlerOpts.plugin, watchify], - // required by watchify + // Required by watchify cache: {}, packageCache: {}, }); - // instrument pipeline + // Instrument pipeline events.on('configurePipeline', ({ bundleStream }) => { - // handle build error to avoid breaking build process + // Handle build error to avoid breaking build process // (eg on syntax error) bundleStream.on('error', (err) => { gracefulError(err); diff --git a/development/build/transforms/README.md b/development/build/transforms/README.md new file mode 100644 index 000000000..b2c40abc1 --- /dev/null +++ b/development/build/transforms/README.md @@ -0,0 +1,123 @@ +# Local Browserify Transforms + +This directory contains home-grown Browserify transforms. +Each file listed here exports a transform function factory. + +## Removing Fenced Code + +> `./remove-fenced-code.js` + +When creating builds that support different features, it is desirable to exclude +unsupported features, files, and dependencies at build time. Undesired files and +dependencies can be excluded wholesale, but the _use_ of undesired modules in +files that should otherwise be included – i.e. import statements and references +to those imports – cannot. + +To support the exclusion of the use of undesired modules at build time, we +introduce the concept of code fencing to our build system. Our code fencing +syntax amounts to a tiny DSL, which is specified below. + +The transform concatenates each file into a single string, and a string parser +identifies any fences in the file. If any fences that should not be included in +the current build are found, the fences and the lines that they wrap are +deleted. The transform errors if a malformed fence line is identified. + +For example, the following fenced code: + +```javascript +this.store.updateStructure({ + ..., + GasFeeController: this.gasFeeController, + TokenListController: this.tokenListController, + ///: BEGIN:ONLY_INCLUDE_IN(beta) + PluginController: this.pluginController, + ///: END:ONLY_INCLUDE_IN +}); +``` + +Is transformed to the following if the build type is not `beta`: + +```javascript +this.store.updateStructure({ + ..., + GasFeeController: this.gasFeeController, + TokenListController: this.tokenListController, +}); +``` + +Note that multiple build types can be specified by separating them with +commands inside the parameter parentheses: + +```javascript +///: BEGIN:ONLY_INCLUDE_IN(beta,flask) +``` + +### Code Fencing Syntax + +> In the specification, angle brackets, `< >`, indicate required tokens, while +> straight brackets, `[ ]`, indicate optional tokens. +> +> Alphabetical characters identify the name and purpose of a token. All other +> characters, including parentheses, `( )`, are literals. + +A fence line is a single-line JavaScript comment, optionally surrounded by +whitespace, in the following format: + +```text +///: :[(parameters)] + +|__| |________________________________| + | | + | | +sentinel directive +``` + +The first part of a fence line is the `sentinel`, which is always the string +"`///:`". If the first four non-whitespace characters of a line are not the +`sentinel`, the line will be ignored by the parser. The `sentinel` must be +succeeded by a single space character, or parsing will fail. + +The remainder of the fence line is called the `directive`. +The directive consists of a `terminus`, `command`, and (optionally) `parameters`. + +- The `terminus` is one of the strings `BEGIN` and `END`. It must be followed by + a single colon, `:`. +- The `command` is a string of uppercase alphabetical characters, optionally + including underscores, `_`. The possible commands are listed later in this + specification. +- The `parameters` are a comma-separated list of RegEx `\w` strings. They must + be parenthesized, only specified for `BEGIN` directives, and valid for the + command of the directive. + +A valid code fence consists of two fence lines surrounding one or more lines of +non-fence lines. The first fence line must consist of a `BEGIN` directive, and +the second an `END` directive. The command of both directives must be the same, +and the parameters (if any) must be valid for the command. + +If an invalid fence is detected, parsing will fail, and the transform stream +will end with an error. + +### Commands + +#### `ONLY_INCLUDE_IN` + +This, the only command defined so far, is used to exclude lines of code +depending on the type of the current build. If a particular set of lines should +only be included in a particular build type, say `beta`, they should be wrapped +as follows: + +```javascript +///: BEGIN:ONLY_INCLUDE_IN(beta) +console.log('I am only included in beta builds.'); +///: END:ONLY_INCLUDE_IN +``` + +At build time, the fences and the fenced lines will be removed if the build is +not `beta`. + +Parameters are required for this command, and they must be provided as a +comma-separated list of one or more of: + +- `main` (the build system default build type) +- `beta` +- `flask` diff --git a/development/build/transforms/remove-fenced-code.js b/development/build/transforms/remove-fenced-code.js new file mode 100644 index 000000000..fa23ada82 --- /dev/null +++ b/development/build/transforms/remove-fenced-code.js @@ -0,0 +1,434 @@ +const path = require('path'); +const { PassThrough, Transform } = require('stream'); +const { BuildTypes } = require('../utils'); + +const hasOwnProperty = (obj, key) => Reflect.hasOwnProperty.call(obj, key); + +module.exports = { + createRemoveFencedCodeTransform, + removeFencedCode, +}; + +class RemoveFencedCodeTransform extends Transform { + /** + * A transform stream that calls {@link removeFencedCode} on the complete + * string contents of the file read by Browserify. + * + * @param {string} filePath - The path to the file being transformed. + * @param {string} buildType - The type of the current build process.env. + */ + constructor(filePath, buildType) { + super(); + this.filePath = filePath; + this.buildType = buildType; + this._fileBuffers = []; + } + + // This function is called whenever data is written to the stream. + // It concatenates all buffers for the current file into a single buffer. + _transform(buffer, _encoding, next) { + this._fileBuffers.push(buffer); + next(); + } + + // "flush" is called when all data has been written to the + // stream, immediately before the "end" event is emitted. + // It applies the transform to the concatenated file contents. + _flush(end) { + const [fileContent] = removeFencedCode( + this.filePath, + this.buildType, + Buffer.concat(this._fileBuffers).toString('utf8'), + ); + + this.push(fileContent); + end(); + } +} + +/** + * A factory for a Browserify transform that removes fenced code from all + * JavaScript source files. The transform is applied to files with the following + * extensions: + * - `.js` + * - `.cjs` + * - `.mjs` + * + * For details on how the transform mutates source files, see + * {@link removeFencedCode} and the documentation. + * + * @param {string} buildType - The type of the current build. + * @returns {(filePath: string) => Transform} The transform function. + */ +function createRemoveFencedCodeTransform(buildType) { + if (!hasOwnProperty(BuildTypes, buildType)) { + throw new Error( + `Code fencing transform received unrecognized build type "${buildType}".`, + ); + } + + // Browserify transforms are functions that receive a file name and return a + // duplex stream. The stream receives the file contents piecemeal in the form + // of Buffers. + // To apply our code fencing transform, we concatenate all buffers and convert + // them to a single string, then apply the actual transform function on that + // string. + /** + * @returns {Transform} + */ + return function removeFencedCodeTransform(filePath) { + if (!['.js', '.cjs', '.mjs'].includes(path.extname(filePath))) { + return new PassThrough(); + } + + return new RemoveFencedCodeTransform(filePath, buildType); + }; +} + +const DirectiveTerminuses = { + BEGIN: 'BEGIN', + END: 'END', +}; + +const DirectiveCommands = { + ONLY_INCLUDE_IN: 'ONLY_INCLUDE_IN', +}; + +const CommandValidators = { + [DirectiveCommands.ONLY_INCLUDE_IN]: (params, filePath) => { + if (!params || params.length === 0) { + throw new Error( + getInvalidParamsMessage( + filePath, + DirectiveCommands.ONLY_INCLUDE_IN, + `No params specified.`, + ), + ); + } + + params.forEach((param) => { + if (!hasOwnProperty(BuildTypes, param)) { + throw new Error( + getInvalidParamsMessage( + filePath, + DirectiveCommands.ONLY_INCLUDE_IN, + `"${param}" is not a valid build type.`, + ), + ); + } + }); + }, +}; + +// Matches lines starting with "///:", and any preceding whitespace, except +// newlines. We except newlines to avoid eating blank lines preceding a fenced +// line. +// Double-negative RegEx credit: https://stackoverflow.com/a/3469155 +const linesWithFenceRegex = /^[^\S\r\n]*\/\/\/:.*$/gmu; + +// Matches the first "///:" in a string, and any preceding whitespace +const fenceSentinelRegex = /^\s*\/\/\/:/u; + +// Breaks a fence directive into its constituent components +// At this stage of parsing, we are looking for one of: +// - TERMINUS:COMMAND(PARAMS) +// - TERMINUS:COMMAND +const directiveParsingRegex = /^([A-Z]+):([A-Z_]+)(?:\(((?:\w+,)*\w+)\))?$/u; + +/** + * Removes fenced code from the given JavaScript source string. "Fenced code" + * includes the entire fence lines, including their trailing newlines, and the + * lines that they surround. + * + * A valid fence consists of two well-formed fence lines, separated by one or + * more lines that should be excluded. The first line must contain a `BEGIN` + * directive, and the second most contain an `END` directive. Both directives + * must specify the same command. + * + * Here's an example of a valid fence: + * + * ```javascript + * ///: BEGIN:ONLY_INCLUDE_IN(flask) + * console.log('I am Flask.'); + * ///: END:ONLY_INCLUDE_IN + * ``` + * + * For details, please see the documentation. + * + * @param {string} filePath - The path to the file being transformed. + * @param {string} typeOfCurrentBuild - The type of the current build process. + * @param {string} fileContent - The contents of the file being transformed. + * @returns {[string, modified]} A tuple of the post-transform file contents and + * a boolean indicating whether they were modified. + */ +function removeFencedCode(filePath, typeOfCurrentBuild, fileContent) { + const matchedLines = [...fileContent.matchAll(linesWithFenceRegex)]; + + // If we didn't match any lines, return the unmodified file contents. + if (matchedLines.length === 0) { + return [fileContent, false]; + } + + // Parse fence lines + const parsedDirectives = matchedLines.map((matchArray) => { + const line = matchArray[0]; + + /* istanbul ignore next: should be impossible */ + if (!fenceSentinelRegex.test(line)) { + throw new Error( + getInvalidFenceLineMessage( + filePath, + line, + `Fence sentinel may only appear at the start of a line, optionally preceded by whitespace.`, + ), + ); + } + + // Store the start and end indices of each line + // Increment the end index by 1 to including the trailing newline when + // performing string operations. + const indices = [matchArray.index, matchArray.index + line.length + 1]; + + const lineWithoutSentinel = line.replace(fenceSentinelRegex, ''); + if (!/^ \w\w+/u.test(lineWithoutSentinel)) { + throw new Error( + getInvalidFenceLineMessage( + filePath, + line, + `Fence sentinel must be followed by a single space and an alphabetical string of two or more characters.`, + ), + ); + } + + const directiveMatches = lineWithoutSentinel + .trim() + .match(directiveParsingRegex); + + if (!directiveMatches) { + throw new Error( + getInvalidFenceLineMessage( + filePath, + line, + `Failed to parse fence directive.`, + ), + ); + } + + // The first element of a RegEx match array is the input + const [, terminus, command, parameters] = directiveMatches; + + if (!hasOwnProperty(DirectiveTerminuses, terminus)) { + throw new Error( + getInvalidFenceLineMessage( + filePath, + line, + `Line contains invalid directive terminus "${terminus}".`, + ), + ); + } + if (!hasOwnProperty(DirectiveCommands, command)) { + throw new Error( + getInvalidFenceLineMessage( + filePath, + line, + `Line contains invalid directive command "${command}".`, + ), + ); + } + + const parsed = { + line, + indices, + terminus, + command, + }; + + if (parameters !== undefined) { + parsed.parameters = parameters.split(','); + } + return parsed; + }); + + if (parsedDirectives.length % 2 !== 0) { + throw new Error( + getInvalidFenceStructureMessage( + filePath, + `A valid fence consists of two fence lines, but the file contains an uneven number, "${parsedDirectives.length}", of fence lines.`, + ), + ); + } + + // The below for-loop iterates over the parsed fence directives and performs + // the following work: + // - Ensures that the array of parsed directives consists of valid directive + // pairs, as specified in the documentation. + // - For each directive pair, determines whether their fenced lines should be + // removed for the current build, and if so, stores the indices we will use + // to splice the file content string. + + const splicingIndices = []; + let shouldSplice = false; + let currentCommand; + + for (let i = 0; i < parsedDirectives.length; i++) { + const { line, indices, terminus, command, parameters } = parsedDirectives[ + i + ]; + if (i % 2 === 0) { + if (terminus !== DirectiveTerminuses.BEGIN) { + throw new Error( + getInvalidFencePairMessage( + filePath, + line, + `The first directive of a pair must be a "BEGIN" directive.`, + ), + ); + } + + currentCommand = command; + // Throws an error if the command parameters are invalid + CommandValidators[command](parameters, filePath); + + if (parameters.includes(typeOfCurrentBuild)) { + shouldSplice = false; + } else { + shouldSplice = true; + // Add start index of BEGIN directive line to splicing indices + splicingIndices.push(indices[0]); + } + } else { + if (terminus !== DirectiveTerminuses.END) { + throw new Error( + getInvalidFencePairMessage( + filePath, + line, + `The second directive of a pair must be an "END" directive.`, + ), + ); + } + + /* istanbul ignore next: impossible until there's more than one command */ + if (command !== currentCommand) { + throw new Error( + getInvalidFencePairMessage( + filePath, + line, + `Expected "END" directive to have command "${currentCommand}" but found "${command}".`, + ), + ); + } + + // Forbid empty fences + const { line: previousLine, indices: previousIndices } = parsedDirectives[ + i - 1 + ]; + if (fileContent.substring(previousIndices[1], indices[0]).trim() === '') { + throw new Error( + `Empty fence found in file "${filePath}":\n${previousLine}\n${line}\n`, + ); + } + + if (shouldSplice) { + // Add end index of END directive line to splicing indices + splicingIndices.push(indices[1]); + } + } + } + + // This indicates that the present build type should include all fenced code, + // and so we just returned the unmodified file contents. + if (splicingIndices.length === 0) { + return [fileContent, false]; + } + + /* istanbul ignore next: should be impossible */ + if (splicingIndices.length % 2 !== 0) { + throw new Error( + `Internal error while transforming file "${filePath}":\nCollected an uneven number of splicing indices: "${splicingIndices.length}"`, + ); + } + + return [multiSplice(fileContent, splicingIndices), true]; +} + +/** + * Returns a copy of the given string, without the character ranges specified + * by the splicing indices array. + * + * The splicing indices must be a non-empty, even-length array of non-negative + * integers, specifying the character ranges to remove from the given string, as + * follows: + * + * `[ start, end, start, end, start, end, ... ]` + * + * @param {string} toSplice - The string to splice. + * @param {number[]} splicingIndices - Indices to splice at. + * @returns {string} The spliced string. + */ +function multiSplice(toSplice, splicingIndices) { + const retainedSubstrings = []; + + // Get the first part to be included + // The substring() call returns an empty string if splicingIndices[0] is 0, + // which is exactly what we want in that case. + retainedSubstrings.push(toSplice.substring(0, splicingIndices[0])); + + // This loop gets us all parts of the string that should be retained, except + // the first and the last. + // It iterates over all "end" indices of the array except the last one, and + // pushes the substring between each "end" index and the next "begin" index + // to the array of retained substrings. + if (splicingIndices.length > 2) { + for (let i = 1; i < splicingIndices.length; i += 2) { + retainedSubstrings.push( + toSplice.substring(splicingIndices[i], splicingIndices[i + 1]), + ); + } + } + + // Get the last part to be included + retainedSubstrings.push( + toSplice.substring(splicingIndices[splicingIndices.length - 1]), + ); + return retainedSubstrings.join(''); +} + +/** + * @param {string} filePath - The path to the file that caused the error. + * @param {string} line - The contents of the line with the error. + * @param {string} details - An explanation of the error. + * @returns The error message. + */ +function getInvalidFenceLineMessage(filePath, line, details) { + return `Invalid fence line in file "${filePath}": "${line}":\n${details}`; +} + +/** + * @param {string} filePath - The path to the file that caused the error. + * @param {string} details - An explanation of the error. + * @returns The error message. + */ +function getInvalidFenceStructureMessage(filePath, details) { + return `Invalid fence structure in file "${filePath}":\n${details}`; +} + +/** + * @param {string} filePath - The path to the file that caused the error. + * @param {string} line - The contents of the line with the error. + * @param {string} details - An explanation of the error. + * @returns The error message. + */ +function getInvalidFencePairMessage(filePath, line, details) { + return `Invalid fence pair in file "${filePath}" due to line "${line}":\n${details}`; +} + +/** + * @param {string} filePath - The path to the file that caused the error. + * @param {string} command - The command of the directive with the invalid + * parameters. + * @param {string} details - An explanation of the error. + * @returns The error message. + */ +function getInvalidParamsMessage(filePath, command, details) { + return `Invalid code fence parameters in file "${filePath}" for command "${command}":\n${details}`; +} diff --git a/development/build/transforms/remove-fenced-code.test.js b/development/build/transforms/remove-fenced-code.test.js new file mode 100644 index 000000000..15ce94811 --- /dev/null +++ b/development/build/transforms/remove-fenced-code.test.js @@ -0,0 +1,591 @@ +const deepFreeze = require('deep-freeze-strict'); +const { BuildTypes } = require('../utils'); +const { + createRemoveFencedCodeTransform, + removeFencedCode, +} = require('./remove-fenced-code'); + +// The test data is just strings. We get it from a function at the end of this +// file because it takes up a lot of lines and is very distracting. +const testData = getTestData(); + +const getMinimalFencedCode = (params = 'flask') => + `///: BEGIN:ONLY_INCLUDE_IN(${params}) +Conditionally_Included +///: END:ONLY_INCLUDE_IN +`; + +describe('build/transforms/remove-fenced-code', () => { + describe('createRemoveFencedCodeTransform', () => { + const mockJsFileName = 'file.js'; + + it('rejects invalid build types', () => { + expect(() => createRemoveFencedCodeTransform('foobar')).toThrow( + /received unrecognized build type "foobar".$/u, + ); + }); + + it('returns a PassThrough stream for files with ignored extensions', async () => { + const fileContent = '"Valid JSON content"\n'; + const stream = createRemoveFencedCodeTransform('main')('file.json'); + let streamOutput = ''; + + await new Promise((resolve) => { + stream.on('data', (data) => { + streamOutput = streamOutput.concat(data.toString('utf8')); + }); + + stream.on('end', () => { + expect(streamOutput).toStrictEqual(fileContent); + resolve(); + }); + + stream.write(Buffer.from(fileContent)); + setTimeout(() => stream.end()); + }); + }); + + it('transforms a file read as a single chunk', async () => { + const filePrefix = '// A comment\n'; + const fileContent = filePrefix.concat(getMinimalFencedCode()); + + const stream = createRemoveFencedCodeTransform('main')(mockJsFileName); + let streamOutput = ''; + + await new Promise((resolve) => { + stream.on('data', (data) => { + streamOutput = streamOutput.concat(data.toString('utf8')); + }); + + stream.on('end', () => { + expect(streamOutput).toStrictEqual(filePrefix); + resolve(); + }); + + stream.end(fileContent); + }); + }); + + it('transforms a file read as multiple chunks', async () => { + const filePrefix = '// A comment\n'; + const chunks = filePrefix + .concat(getMinimalFencedCode()) + .split('\n') + // The final element in the split array is the empty string, which is + // useful for calling .join, but undesirable here. + .filter((line) => line !== '') + .map((line) => `${line}\n`); + + const stream = createRemoveFencedCodeTransform('main')(mockJsFileName); + let streamOutput = ''; + + await new Promise((resolve) => { + stream.on('data', (data) => { + streamOutput = streamOutput.concat(data.toString('utf8')); + }); + + stream.on('end', () => { + expect(streamOutput).toStrictEqual(filePrefix); + resolve(); + }); + + chunks.forEach((chunk) => stream.write(chunk)); + setTimeout(() => stream.end()); + }); + }); + + it('handles file with fences that is unmodified by the transform', async () => { + const fileContent = getMinimalFencedCode('main'); + + const stream = createRemoveFencedCodeTransform('main')(mockJsFileName); + let streamOutput = ''; + + await new Promise((resolve) => { + stream.on('data', (data) => { + streamOutput = streamOutput.concat(data.toString('utf8')); + }); + + stream.on('end', () => { + expect(streamOutput).toStrictEqual(fileContent); + resolve(); + }); + + stream.end(fileContent); + }); + }); + }); + + describe('removeFencedCode', () => { + const mockFileName = 'file.js'; + + // Valid inputs + Object.keys(BuildTypes).forEach((buildType) => { + it(`transforms file with fences for build type "${buildType}"`, () => { + expect( + removeFencedCode( + mockFileName, + buildType, + testData.validInputs.withFences, + ), + ).toStrictEqual(testData.validOutputs[buildType]); + + // Ensure that the minimal input template is in fact valid + const minimalInput = getMinimalFencedCode(buildType); + expect( + removeFencedCode(mockFileName, buildType, minimalInput), + ).toStrictEqual([minimalInput, false]); + }); + + it(`does not modify file without fences for build type "${buildType}"`, () => { + expect( + removeFencedCode( + mockFileName, + buildType, + testData.validInputs.withoutFences, + ), + ).toStrictEqual([testData.validInputs.withoutFences, false]); + }); + }); + + // This is an edge case for the splicing function + it('transforms file with two fence lines', () => { + expect( + removeFencedCode( + mockFileName, + BuildTypes.flask, + getMinimalFencedCode('main'), + ), + ).toStrictEqual(['', true]); + }); + + it('ignores sentinels preceded by non-whitespace', () => { + const validBeginDirective = '///: BEGIN:ONLY_INCLUDE_IN(flask)\n'; + const ignoredLines = [ + `a ${validBeginDirective}`, + `2 ${validBeginDirective}`, + `@ ${validBeginDirective}`, + ]; + + ignoredLines.forEach((ignoredLine) => { + // These inputs will be transformed + expect( + removeFencedCode( + mockFileName, + BuildTypes.flask, + getMinimalFencedCode('main').concat(ignoredLine), + ), + ).toStrictEqual([ignoredLine, true]); + + const modifiedInputWithoutFences = testData.validInputs.withoutFences.concat( + ignoredLine, + ); + + // These inputs will not be transformed + expect( + removeFencedCode( + mockFileName, + BuildTypes.flask, + modifiedInputWithoutFences, + ), + ).toStrictEqual([modifiedInputWithoutFences, false]); + }); + }); + + // Invalid inputs + it('rejects empty fences', () => { + const jsComment = '// A comment\n'; + + const emptyFence = getMinimalFencedCode() + .split('\n') + .filter((line) => line.startsWith('///:')) + .map((line) => `${line}\n`) + .join(''); + + const emptyFenceWithPrefix = jsComment.concat(emptyFence); + const emptyFenceWithSuffix = emptyFence.concat(jsComment); + const emptyFenceSurrounded = emptyFenceWithPrefix.concat(jsComment); + + const inputs = [ + emptyFence, + emptyFenceWithPrefix, + emptyFenceWithSuffix, + emptyFenceSurrounded, + ]; + + inputs.forEach((input) => { + expect(() => + removeFencedCode(mockFileName, BuildTypes.flask, input), + ).toThrow( + `Empty fence found in file "${mockFileName}":\n${emptyFence}`, + ); + }); + }); + + it('rejects sentinels not followed by a single space and a multi-character alphabetical string', () => { + // Matches the sentinel and terminus component of the first line + // beginning with "///: TERMINUS" + const fenceSentinelAndTerminusRegex = /^\/\/\/: \w+/mu; + + const replacements = [ + '///:BEGIN', + '///:XBEGIN', + '///:_BEGIN', + '///:B', + '///:_', + '///: ', + '///: B', + '///:', + ]; + + replacements.forEach((replacement) => { + expect(() => + removeFencedCode( + mockFileName, + BuildTypes.flask, + getMinimalFencedCode().replace( + fenceSentinelAndTerminusRegex, + replacement, + ), + ), + ).toThrow( + /Fence sentinel must be followed by a single space and an alphabetical string of two or more characters.$/u, + ); + }); + }); + + it('rejects malformed BEGIN directives', () => { + // This is the first line of the minimal input template + const directiveString = '///: BEGIN:ONLY_INCLUDE_IN(flask)'; + + const replacements = [ + // Invalid terminus + '///: BE_GIN:ONLY_INCLUDE_IN(flask)', + '///: BE6IN:ONLY_INCLUDE_IN(flask)', + '///: BEGIN7:ONLY_INCLUDE_IN(flask)', + '///: BeGIN:ONLY_INCLUDE_IN(flask)', + '///: BE3:ONLY_INCLUDE_IN(flask)', + '///: BEG-IN:ONLY_INCLUDE_IN(flask)', + '///: BEG N:ONLY_INCLUDE_IN(flask)', + + // Invalid commands + '///: BEGIN:ONLY-INCLUDE_IN(flask)', + '///: BEGIN:ONLY_INCLUDE:IN(flask)', + '///: BEGIN:ONL6_INCLUDE_IN(flask)', + '///: BEGIN:ONLY_IN@LUDE_IN(flask)', + '///: BEGIN:ONLy_INCLUDE_IN(flask)', + '///: BEGIN:ONLY INCLUDE_IN(flask)', + + // Invalid parameters + '///: BEGIN:ONLY_INCLUDE_IN(,flask)', + '///: BEGIN:ONLY_INCLUDE_IN(flask,)', + '///: BEGIN:ONLY_INCLUDE_IN(flask,,main)', + '///: BEGIN:ONLY_INCLUDE_IN(,)', + '///: BEGIN:ONLY_INCLUDE_IN()', + '///: BEGIN:ONLY_INCLUDE_IN( )', + '///: BEGIN:ONLY_INCLUDE_IN(flask]', + '///: BEGIN:ONLY_INCLUDE_IN[flask)', + '///: BEGIN:ONLY_INCLUDE_IN(flask.main)', + '///: BEGIN:ONLY_INCLUDE_IN(flask,@)', + '///: BEGIN:ONLY_INCLUDE_IN(fla k)', + + // Stuff after the directive + '///: BEGIN:ONLY_INCLUDE_IN(flask) A', + '///: BEGIN:ONLY_INCLUDE_IN(flask) 9', + '///: BEGIN:ONLY_INCLUDE_IN(flask)A', + '///: BEGIN:ONLY_INCLUDE_IN(flask)9', + '///: BEGIN:ONLY_INCLUDE_IN(flask)_', + '///: BEGIN:ONLY_INCLUDE_IN(flask))', + ]; + + replacements.forEach((replacement) => { + expect(() => + removeFencedCode( + mockFileName, + BuildTypes.flask, + getMinimalFencedCode().replace(directiveString, replacement), + ), + ).toThrow( + new RegExp( + `${replacement.replace( + /([()[\]])/gu, + '\\$1', + )}":\nFailed to parse fence directive.$`, + 'u', + ), + ); + }); + }); + + it('rejects malformed END directives', () => { + // This is the last line of the minimal input template + const directiveString = '///: END:ONLY_INCLUDE_IN'; + + const replacements = [ + // Invalid terminus + '///: ENx:ONLY_INCLUDE_IN', + '///: EN3:ONLY_INCLUDE_IN', + '///: EN_:ONLY_INCLUDE_IN', + '///: EN :ONLY_INCLUDE_IN', + '///: EN::ONLY_INCLUDE_IN', + + // Invalid commands + '///: END:ONLY-INCLUDE_IN', + '///: END::ONLY_INCLUDE_IN', + '///: END:ONLY_INCLUDE:IN', + '///: END:ONL6_INCLUDE_IN', + '///: END:ONLY_IN@LUDE_IN', + '///: END:ONLy_INCLUDE_IN', + '///: END:ONLY INCLUDE_IN', + + // Stuff after the directive + '///: END:ONLY_INCLUDE_IN A', + '///: END:ONLY_INCLUDE_IN 9', + '///: END:ONLY_INCLUDE_IN _', + ]; + + replacements.forEach((replacement) => { + expect(() => + removeFencedCode( + mockFileName, + BuildTypes.flask, + getMinimalFencedCode().replace(directiveString, replacement), + ), + ).toThrow( + new RegExp( + `${replacement}":\nFailed to parse fence directive.$`, + 'u', + ), + ); + }); + }); + + it('rejects files with uneven number of fence lines', () => { + const additions = [ + '///: BEGIN:ONLY_INCLUDE_IN(flask)', + '///: END:ONLY_INCLUDE_IN', + ]; + additions.forEach((addition) => { + expect(() => + removeFencedCode( + mockFileName, + BuildTypes.flask, + getMinimalFencedCode().concat(addition), + ), + ).toThrow( + /A valid fence consists of two fence lines, but the file contains an uneven number, "3", of fence lines.$/u, + ); + }); + }); + + it('rejects invalid terminuses', () => { + const testCases = [ + ['BEGIN', ['KAPLAR', 'FLASK', 'FOO']], + ['END', ['KAPLAR', 'FOO', 'BAR']], + ]; + + testCases.forEach(([validTerminus, replacements]) => { + replacements.forEach((replacement) => { + expect(() => + removeFencedCode( + mockFileName, + BuildTypes.flask, + getMinimalFencedCode().replace(validTerminus, replacement), + ), + ).toThrow( + new RegExp( + `Line contains invalid directive terminus "${replacement}".$`, + 'u', + ), + ); + }); + }); + }); + + it('rejects invalid commands', () => { + const testCases = [ + [/ONLY_INCLUDE_IN\(/mu, ['ONLY_KEEP_IN(', 'FLASK(', 'FOO(']], + [/ONLY_INCLUDE_IN$/mu, ['ONLY_KEEP_IN', 'FLASK', 'FOO']], + ]; + + testCases.forEach(([validCommand, replacements]) => { + replacements.forEach((replacement) => { + expect(() => + removeFencedCode( + mockFileName, + BuildTypes.flask, + getMinimalFencedCode().replace(validCommand, replacement), + ), + ).toThrow( + new RegExp( + `Line contains invalid directive command "${replacement.replace( + '(', + '', + )}".$`, + 'u', + ), + ); + }); + }); + }); + + it('rejects invalid command parameters', () => { + const testCases = [ + ['bar', ['bar', 'flask,bar', 'flask,beta,main,bar']], + ['Foo', ['Foo', 'flask,Foo', 'flask,beta,main,Foo']], + ['b3ta', ['b3ta', 'flask,b3ta', 'flask,beta,main,b3ta']], + ['bEta', ['bEta', 'flask,bEta', 'flask,beta,main,bEta']], + ]; + + testCases.forEach(([invalidParam, replacements]) => { + replacements.forEach((replacement) => { + expect(() => + removeFencedCode( + mockFileName, + BuildTypes.flask, + getMinimalFencedCode(replacement), + ), + ).toThrow( + new RegExp(`"${invalidParam}" is not a valid build type.$`, 'u'), + ); + }); + }); + + // Should fail for empty params + expect(() => + removeFencedCode( + mockFileName, + BuildTypes.flask, + getMinimalFencedCode('').replace('()', ''), + ), + ).toThrow(/No params specified.$/u); + }); + + it('rejects directive pairs with wrong terminus order', () => { + // We need more than one directive pair for this test + const input = getMinimalFencedCode().concat(getMinimalFencedCode('beta')); + + const expectedBeginError = + 'The first directive of a pair must be a "BEGIN" directive.'; + const expectedEndError = + 'The second directive of a pair must be an "END" directive.'; + const testCases = [ + [ + 'BEGIN:ONLY_INCLUDE_IN(flask)', + 'END:ONLY_INCLUDE_IN', + expectedBeginError, + ], + [ + /END:ONLY_INCLUDE_IN/mu, + 'BEGIN:ONLY_INCLUDE_IN(main)', + expectedEndError, + ], + [ + 'BEGIN:ONLY_INCLUDE_IN(beta)', + 'END:ONLY_INCLUDE_IN', + expectedBeginError, + ], + ]; + + testCases.forEach(([target, replacement, expectedError]) => { + expect(() => + removeFencedCode( + mockFileName, + BuildTypes.flask, + input.replace(target, replacement), + ), + ).toThrow(expectedError); + }); + }); + + // We can't do this until there's more than one command + it.todo('rejects directive pairs with mismatched commands'); + }); +}); + +function getTestData() { + const data = { + validInputs: { + withFences: ` +///: BEGIN:ONLY_INCLUDE_IN(flask,beta) +Conditionally_Included +///: END:ONLY_INCLUDE_IN + Always_Included +Always_Included + Always_Included +Always_Included + ///: BEGIN:ONLY_INCLUDE_IN(flask,beta) + Conditionally_Included + + Conditionally_Included + Conditionally_Included + ///: END:ONLY_INCLUDE_IN +Always_Included + +Always_Included + Always_Included + ///: BEGIN:ONLY_INCLUDE_IN(flask) + + Conditionally_Included + Conditionally_Included + ///: END:ONLY_INCLUDE_IN +Always_Included + Always_Included +Always_Included + +///: BEGIN:ONLY_INCLUDE_IN(flask) + Conditionally_Included +Conditionally_Included + + ///: END:ONLY_INCLUDE_IN +`, + + withoutFences: ` + Always_Included +Always_Included + Always_Included +Always_Included +Always_Included + +Always_Included + Always_Included +Always_Included + Always_Included +Always_Included + +`, + }, + + validOutputs: { + beta: [ + ` +///: BEGIN:ONLY_INCLUDE_IN(flask,beta) +Conditionally_Included +///: END:ONLY_INCLUDE_IN + Always_Included +Always_Included + Always_Included +Always_Included + ///: BEGIN:ONLY_INCLUDE_IN(flask,beta) + Conditionally_Included + + Conditionally_Included + Conditionally_Included + ///: END:ONLY_INCLUDE_IN +Always_Included + +Always_Included + Always_Included +Always_Included + Always_Included +Always_Included + +`, + true, + ], + }, + }; + + data.validOutputs.flask = [data.validInputs.withFences, false]; + data.validOutputs.main = [data.validInputs.withoutFences, true]; + return deepFreeze(data); +} diff --git a/development/build/utils.js b/development/build/utils.js index 2c38097ae..0b8503d19 100644 --- a/development/build/utils.js +++ b/development/build/utils.js @@ -20,6 +20,13 @@ function getNextBetaVersionMap(currentVersion, betaVersion, platforms) { }, {}); } +const BuildTypes = { + beta: 'beta', + flask: 'flask', + main: 'main', +}; + module.exports = { + BuildTypes, getNextBetaVersionMap, }; diff --git a/development/jest.config.js b/development/jest.config.js new file mode 100644 index 000000000..4eeaa048c --- /dev/null +++ b/development/jest.config.js @@ -0,0 +1,19 @@ +module.exports = { + displayName: '/development', + collectCoverageFrom: ['/**/*.js'], + coverageDirectory: '../jest-coverage/development/', + coverageReporters: ['json', 'lcov', 'text', 'clover'], + coverageThreshold: { + './development/build/transforms/**/*.js': { + branches: 100, + functions: 100, + lines: 100, + statements: 100, + }, + }, + resetMocks: true, + restoreMocks: true, + testEnvironment: 'node', + testMatch: ['/build/**/*.test.js'], + testTimeout: 2500, +}; diff --git a/jest.config.js b/jest.config.js index 5a6a326d7..1c0d0692a 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,8 +1,9 @@ module.exports = { - restoreMocks: true, - coverageDirectory: 'jest-coverage/', + displayName: '/ui, /shared', collectCoverageFrom: ['/ui/**/*.js', '/shared/**/*.js'], + coverageDirectory: './jest-coverage/main', coveragePathIgnorePatterns: ['.stories.js', '.snap'], + coverageReporters: ['json', 'lcov', 'text', 'clover'], coverageThreshold: { global: { branches: 35, @@ -11,10 +12,11 @@ module.exports = { statements: 43, }, }, - setupFiles: ['./test/setup.js', './test/env.js'], - setupFilesAfterEnv: ['./test/jest/setup.js'], - testMatch: [ - '/ui/**/?(*.)+(test).js', - '/shared/**/?(*.)+(test).js', - ], + // TODO: enable resetMocks + // resetMocks: true, + restoreMocks: true, + setupFiles: ['/test/setup.js', '/test/env.js'], + setupFilesAfterEnv: ['/test/jest/setup.js'], + testMatch: ['/ui/**/*.test.js', '/shared/**/*.test.js'], + testTimeout: 2500, }; diff --git a/package.json b/package.json index 0f26e77fd..01e0c9d63 100644 --- a/package.json +++ b/package.json @@ -26,10 +26,7 @@ "dapp-forwarder": "concurrently -k -n forwarder,dapp -p '[{time}][{name}]' 'yarn forwarder' 'yarn dapp'", "test:unit": "mocha --exit --require test/env.js --require test/setup.js --recursive './app/**/*.test.js'", "test:unit:global": "mocha --exit --require test/env.js --require test/setup.js --recursive test/unit-global/*.test.js", - "test:unit:jest": "jest", - "test:unit:jest:watch": "jest --watch", - "test:unit:jest:watch:silent": "jest --watch --silent", - "test:unit:jest:ci": "jest --maxWorkers=2", + "test:unit:jest": "./test/run-jest.sh", "test:unit:lax": "mocha --exit --require test/env.js --require test/setup.js --ignore './app/scripts/controllers/permissions/*.test.js' --recursive './app/**/*.test.js'", "test:unit:strict": "mocha --exit --require test/env.js --require test/setup.js --recursive './app/scripts/controllers/permissions/*.test.js'", "test:unit:path": "mocha --exit --require test/env.js --require test/setup.js --recursive", @@ -37,9 +34,9 @@ "test:e2e:chrome:metrics": "SELENIUM_BROWSER=chrome node test/e2e/run-e2e-test.js test/e2e/metrics.spec.js", "test:e2e:firefox": "SELENIUM_BROWSER=firefox node test/e2e/run-all.js", "test:e2e:firefox:metrics": "SELENIUM_BROWSER=firefox node test/e2e/run-e2e-test.js test/e2e/metrics.spec.js", - "test:coverage": "nyc --silent --check-coverage yarn test:unit:strict && nyc --silent --no-clean yarn test:unit:lax && nyc report --reporter=text --reporter=html", "test:e2e:single": "node test/e2e/run-e2e-test.js", - "test:coverage:jest": "jest --coverage --maxWorkers=2", + "test:coverage": "nyc --silent --check-coverage yarn test:unit:strict && nyc --silent --no-clean yarn test:unit:lax && nyc report --reporter=text --reporter=html", + "test:coverage:jest": "yarn test:unit:jest --coverage --maxWorkers=2", "test:coverage:strict": "nyc --check-coverage yarn test:unit:strict", "test:coverage:path": "nyc --check-coverage yarn test:unit:path", "ganache:start": "./development/run-ganache.sh", diff --git a/test/run-jest.sh b/test/run-jest.sh new file mode 100755 index 000000000..40b208732 --- /dev/null +++ b/test/run-jest.sh @@ -0,0 +1,10 @@ +#!/usr/bin/env bash + +set -x +set -e +set -u +set -o pipefail + +concurrently \ + "jest --config=./jest.config.js $*" \ + "jest --config=./development/jest.config.js $*"