Lint transformed files, init lint tests

development/build/transforms/remove-fenced-code.js

@@ -2,6 +2,7 @@ const path = require('path');
 const through = require('through2');
 const { PassThrough } = require('readable-stream');
 const { BuildTypes } = require('../utils');
+const { lintTransformedFile } = require('./utils');
 
 /**
  * @typedef {import('readable-stream').Duplex} Duplex
@@ -40,23 +41,35 @@ function createRemoveFencedCodeTransform(buildType) {
     const fileBuffers = [];
 
     return through(
-      // Concatenate all buffers for the current file into a single buffer.
+      // This function is called whenever data is written to the stream.
+      // It concatenates all buffers for the current file into a single buffer.
       function (fileBuffer, _encoding, next) {
         fileBuffers.push(fileBuffer);
         next();
       },
 
-      // Apply the transform
+      // This "flush" function 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.
       function (end) {
-        this.push(
-          removeFencedCode(
-            filePath,
-            buildType,
-            Buffer.concat(fileBuffers).toString('utf8'),
-          ),
+        const [fileContent, didModify] = removeFencedCode(
+          filePath,
+          buildType,
+          Buffer.concat(fileBuffers).toString('utf8'),
         );
 
-        end();
+        const _end = () => {
+          this.push(fileContent);
+          end();
+        };
+
+        if (didModify) {
+          lintTransformedFile(fileContent, filePath)
+            .then(_end)
+            .catch((error) => this.destroy(error));
+        } else {
+          _end();
+        }
       },
     );
   };
@@ -105,15 +118,16 @@ PluginController: this.pluginController,
 /**
  * @param {string} filePath - The path to the file being transformed.
  * @param {string} typeOfCurrentBuild - The type of the current build process.
- * @param {string} fileContents - The contents of the file being transformed.
- * @returns {string} The transformed file contents.
+ * @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, fileContents) {
-  const matchedLines = [...fileContents.matchAll(linesWithFenceRegex)];
+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 fileContents;
+    return [fileContent, false];
   }
 
   // Parse fence lines
@@ -271,7 +285,7 @@ function removeFencedCode(filePath, typeOfCurrentBuild, fileContents) {
   // 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 fileContents;
+    return [fileContent, false];
   }
 
   /* istanbul ignore next: should be impossible */
@@ -281,7 +295,7 @@ function removeFencedCode(filePath, typeOfCurrentBuild, fileContents) {
     );
   }
 
-  return multiSplice(fileContents, splicingIndices);
+  return [multiSplice(fileContent, splicingIndices), true];
 }
 
 /**

development/build/transforms/remove-fenced-code.test.js

@@ -4,6 +4,11 @@ const {
   createRemoveFencedCodeTransform,
   removeFencedCode,
 } = require('./remove-fenced-code');
+const transformUtils = require('./utils');
+
+jest.mock('./utils', () => ({
+  lintTransformedFile: jest.fn(),
+}));
 
 // 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.
@@ -17,13 +22,19 @@ Conditionally_Included
 
 describe('build/transforms/remove-fenced-code', () => {
   describe('createRemoveFencedCodeTransform', () => {
+    const { lintTransformedFile: lintTransformedFileMock } = transformUtils;
+
+    beforeEach(() => {
+      lintTransformedFileMock.mockImplementation(() => Promise.resolve());
+    });
+
     it('rejects invalid build types', () => {
       expect(() => createRemoveFencedCodeTransform('foobar')).toThrow(
         /received unrecognized build type "foobar".$/u,
       );
     });
 
-    it('does not modify files with ignored file extensions', async () => {
+    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 = '';
@@ -60,8 +71,7 @@ describe('build/transforms/remove-fenced-code', () => {
           resolve();
         });
 
-        stream.write(fileContent);
-        setTimeout(() => stream.end());
+        stream.end(fileContent);
       });
     });
 
@@ -92,6 +102,47 @@ describe('build/transforms/remove-fenced-code', () => {
         setTimeout(() => stream.end());
       });
     });
+
+    it('handles file with fences that is unmodified by the transform', async () => {
+      const fileContent = getMinimalFencedCode('main');
+
+      const stream = createRemoveFencedCodeTransform('main')('file.js');
+      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);
+      });
+    });
+
+    it('handles transformed file lint failure', async () => {
+      lintTransformedFileMock.mockImplementationOnce(() =>
+        Promise.reject(new Error('lint failure')),
+      );
+
+      const filePrefix = '// A comment\n';
+      const fileContent = filePrefix.concat(getMinimalFencedCode());
+
+      const stream = createRemoveFencedCodeTransform('main')('file.js');
+
+      await new Promise((resolve) => {
+        stream.on('error', (error) => {
+          expect(error).toStrictEqual(new Error('lint failure'));
+          expect(stream.destroyed).toStrictEqual(true);
+          resolve();
+        });
+
+        stream.end(fileContent);
+      });
+    });
   });
 
   describe('removeFencedCode', () => {
@@ -112,7 +163,7 @@ describe('build/transforms/remove-fenced-code', () => {
         const minimalInput = getMinimalFencedCode(buildType);
         expect(
           removeFencedCode(mockFileName, buildType, minimalInput),
-        ).toStrictEqual(minimalInput);
+        ).toStrictEqual([minimalInput, false]);
       });
 
       it(`does not modify file without fences for build type "${buildType}"`, () => {
@@ -122,7 +173,7 @@ describe('build/transforms/remove-fenced-code', () => {
             buildType,
             testData.validInputs.withoutFences,
           ),
-        ).toStrictEqual(testData.validInputs.withoutFences);
+        ).toStrictEqual([testData.validInputs.withoutFences, false]);
       });
     });
 
@@ -453,7 +504,8 @@ Always_Included
     },
 
     validOutputs: {
-      beta: `
+      beta: [
+        `
 ///: BEGIN:ONLY_INCLUDE_IN(flask,beta)
 Conditionally_Included
 ///: END:ONLY_INCLUDE_IN
@@ -476,10 +528,12 @@ Always_Included
 Always_Included
 
 `,
+        true,
+      ],
     },
   };
 
-  data.validOutputs.flask = data.validInputs.withFences;
-  data.validOutputs.main = data.validInputs.withoutFences;
+  data.validOutputs.flask = [data.validInputs.withFences, false];
+  data.validOutputs.main = [data.validInputs.withoutFences, true];
   return deepFreeze(data);
 }

development/build/transforms/utils.js

@@ -0,0 +1,65 @@
+const { ESLint } = require('eslint');
+const eslintrc = require('../../../.eslintrc.js');
+
+// This is a singleton
+let eslintInstance;
+
+// Only initialize the ESLint instance if necessary
+// This also makes it easier to test this module
+const initializeESLint = () => {
+  if (!eslintInstance) {
+    eslintInstance = new ESLint({ baseConfig: eslintrc, useEslintrc: false });
+  }
+};
+
+// Four spaces
+const TAB = '    ';
+
+module.exports = {
+  lintTransformedFile,
+};
+
+/**
+ * Lints a transformed file by invoking ESLint programmatically on the string
+ * file contents. The path to the file must be specified so that the repository
+ * ESLint config can be applied properly.
+ *
+ * An error is thrown if linting produced any errors, or if the file is ignored
+ * by ESLint. Files linted by this function should never be ignored.
+ *
+ * @param {string} content - The file content.
+ * @param {string} filePath - The path to the file.
+ * @returns {Promise<void>} Returns `undefined` or throws an error if linting produced
+ * any errors, or if the linted file is ignored.
+ */
+async function lintTransformedFile(content, filePath) {
+  initializeESLint();
+
+  const lintResult = (
+    await eslintInstance.lintText(content, { filePath, warnIgnored: false })
+  )[0];
+
+  // This indicates that the file is ignored, which should never be the case for
+  // a transformed file.
+  if (lintResult === undefined) {
+    throw new Error(
+      `MetaMask build: Transformed file "${filePath}" appears to be ignored by ESLint.`,
+    );
+  }
+
+  // This is the success case
+  if (lintResult.errorCount === 0) {
+    return;
+  }
+
+  // Errors are stored in the messages array, and their "severity" is 2
+  const errorsString = lintResult.messages
+    .filter(({ severity }) => severity === 2)
+    .reduce((allErrors, { message, ruleId }) => {
+      return allErrors.concat(`${TAB}${ruleId}\n${TAB}${message}\n\n`);
+    }, '');
+
+  throw new Error(
+    `MetaMask build: Lint errors found in transformed file "${filePath}":\n\n${errorsString}`,
+  );
+}

development/build/transforms/utils.test.js

@@ -0,0 +1,38 @@
+const { lintTransformedFile } = require('./utils');
+
+let mockESLint;
+
+jest.mock('eslint', () => ({
+  ESLint: class MockESLint {
+    constructor() {
+      if (mockESLint) {
+        throw new Error('Mock ESLint ref already assigned!');
+      }
+
+      // eslint-disable-next-line consistent-this
+      mockESLint = this;
+
+      // eslint-disable-next-line jest/prefer-spy-on
+      this.lintText = jest.fn();
+    }
+  },
+}));
+
+describe('transform utils', () => {
+  describe('lintTransformedFile', () => {
+    it('initializes the ESLint singleton', async () => {
+      expect(mockESLint).not.toBeDefined();
+
+      // This error is an artifact of how we're mocking the ESLint singleton,
+      // and won't actually occur in production.
+      await expect(() => lintTransformedFile()).rejects.toThrow(
+        `Cannot read property '0' of undefined`,
+      );
+      expect(mockESLint).toBeDefined();
+    });
+
+    it.todo('returns if linting passes with no errors');
+    it.todo('throws if the file is ignored by ESLint');
+    it.todo('throws if linting produced any errors');
+  });
+});