Address code review comments, other updates

* Get rid of simple source map mode. For now, the detailed source maps will be
  the only mode. Also inline `computeDetailedSourceMap` now that it's the only
  case.
* Remove `WRITE_SOURCE_MAPS` in test and related details. In alangpierce#793 I added source
  map info to the demo website, which is a nicer system for manually verifying
  correctness.
* Update test code to assert the full source map and a JSON representation of
  the mappings.
* Add integration test for Jest inline snapshots.
* Change lint config to ignore unused rest siblings.
* Add bounds checks and change empty `for` loops to `while` (just a style
  preference).
* Add comments in a few more places.
* Remove unnecessary special case when handling hashbang lines.

.eslintignore

@@ -4,4 +4,3 @@ example-runner/example-repos
 integration-test/test-cases
 spec-compliance-tests/babel-tests/babel-tests-checkout
 spec-compliance-tests/test262/test262-checkout
-test/output/*

.eslintrc.js

@@ -90,7 +90,7 @@ module.exports = {
     "@typescript-eslint/no-unsafe-call": "off",
     "@typescript-eslint/no-unsafe-member-access": "off",
     "@typescript-eslint/no-unsafe-return": "off",
-    "@typescript-eslint/no-unused-vars": ["error", {args: "none"}],
+    "@typescript-eslint/no-unused-vars": ["error", {args: "none", ignoreRestSiblings: true}],
     "@typescript-eslint/restrict-template-expressions": [
       "error",
       {

.gitignore

@@ -11,4 +11,3 @@ integrations/gulp-plugin/dist
 coverage.lcov
 benchmark/sample/jest
 .perf-comparison
-test/output

integration-test/integration-tests.ts

@@ -1,10 +1,11 @@
 import assert from "assert";
 import {exec} from "child_process";
 import {readdirSync, statSync} from "fs";
+import {writeFile} from "fs/promises";
 import {join, dirname} from "path";
 import {promisify} from "util";
 
-import {readJSONFileContentsIfExists} from "../script/util/readFileContents";
+import {readFileContents, readJSONFileContentsIfExists} from "../script/util/readFileContents";
 
 const execPromise = promisify(exec);
 
@@ -68,6 +69,20 @@ describe("integration tests", () => {
     });
   }
 
+  it("allows Jest inline snapshots", async () => {
+    process.chdir("./test-cases/other-cases/allows-inline-snapshots");
+    const originalContents = await readFileContents("./main.test.ts");
+    assert(originalContents.includes("toMatchInlineSnapshot()"));
+    try {
+      await execPromise(`npx jest --no-cache`);
+      // Running the test should have worked and updated the inline snapshot.
+      const newContents = await readFileContents("./main.test.ts");
+      assert(newContents.includes("toMatchInlineSnapshot(`3`)"));
+    } finally {
+      await writeFile("./main.test.ts", originalContents);
+    }
+  });
+
   /**
    * Find ts-node integration tests.
    *

integration-test/test-cases/other-cases/README.md

@@ -0,0 +1,5 @@
+# Other integration test cases
+
+Tests in this folder help power test cases that don't fit nicely into the usual
+framework of discovering and running each project as a test case. These projects
+are referenced directly by tests.

integration-test/test-cases/other-cases/allows-inline-snapshots/jest.config.js

@@ -0,0 +1,3 @@
+module.exports = {
+  transform: {"\\.(js|jsx|ts|tsx)$": "@sucrase/jest-plugin"},
+};

integration-test/test-cases/other-cases/allows-inline-snapshots/main.test.ts

@@ -0,0 +1,3 @@
+test("fills inline snapshot", () => {
+  expect(3 as number).toMatchInlineSnapshot();
+});

package.json

@@ -48,6 +48,7 @@
   },
   "devDependencies": {
     "@babel/core": "^7.18.6",
+    "@jridgewell/trace-mapping": "^0.3.18",
     "@types/glob": "^7",
     "@types/mocha": "^9.1.1",
     "@types/mz": "^2.7.4",

src/Options-gen-types.ts

@@ -15,7 +15,6 @@ export const Transform = t.union(
 
 export const SourceMapOptions = t.iface([], {
   compiledFilename: "string",
-  simple: t.opt("boolean"),
 });
 
 export const Options = t.iface([], {

src/Options.ts

@@ -12,11 +12,6 @@ export interface SourceMapOptions {
    * file.
    */
   compiledFilename: string;
-  /**
-   * Generate source maps which simply map each line to the original line
-   * without any mappings within lines
-   */
-  simple?: boolean;
 }
 
 export interface Options {

src/computeSourceMap.ts

@@ -16,95 +16,74 @@ export interface RawSourceMap {
 }
 
 /**
- * Generate a source map indicating that each line maps directly to the original line, with the tokens in their new positions.
+ * Generate a source map indicating that each line maps directly to the original line,
+ * with the tokens in their new positions.
  */
 export default function computeSourceMap(
-  {code, mappings: rawMappings}: RootTransformerResult,
+  {code: generatedCode, mappings: rawMappings}: RootTransformerResult,
   filePath: string,
   options: SourceMapOptions,
   source: string,
   tokens: Array<Token>,
 ): RawSourceMap {
-  if (!source || options.simple) {
-    return computeSimpleSourceMap(code, filePath, options);
-  }
   const sourceColumns = computeSourceColumns(source, tokens);
-  return computeDetailedSourceMap(code, filePath, options, rawMappings, sourceColumns);
-}
-
-function computeSimpleSourceMap(
-  code: string,
-  filePath: string,
-  {compiledFilename}: SourceMapOptions,
-): RawSourceMap {
-  let mappings = "AAAA";
-  for (let i = 0; i < code.length; i++) {
-    if (code.charCodeAt(i) === charCodes.lineFeed) {
-      mappings += ";AACA";
-    }
-  }
-  return {
-    version: 3,
-    file: compiledFilename || "",
-    sources: [filePath],
-    mappings,
-    names: [],
-  };
-}
-
-function computeSourceColumns(code: string, tokens: Array<Token>): Array<number> {
-  const sourceColumns: Array<number> = new Array(tokens.length);
-  let j = 0;
-  let currentMapping = tokens[j].start;
-  let lineStart = 0;
-  for (let i = 0; i < code.length; i++) {
-    if (i === currentMapping) {
-      sourceColumns[j] = currentMapping - lineStart;
-      currentMapping = tokens[++j].start;
-    }
-    if (code.charCodeAt(i) === charCodes.lineFeed) {
-      lineStart = i + 1;
-    }
+  const map = new GenMapping({file: options.compiledFilename});
+  let tokenIndex = 0;
+  // currentMapping is the output source index for the current input token being
+  // considered.
+  let currentMapping = rawMappings[0];
+  while (currentMapping === undefined && tokenIndex < rawMappings.length - 1) {
+    tokenIndex++;
+    currentMapping = rawMappings[tokenIndex];
   }
-  return sourceColumns;
-}
-
-function computeDetailedSourceMap(
-  code: string,
-  filePath: string,
-  {compiledFilename}: SourceMapOptions,
-  rawMappings: Array<number | undefined>,
-  sourceColumns: Array<number>,
-): RawSourceMap {
-  const map = new GenMapping({file: compiledFilename});
-  let j = 0;
-  let currentMapping = rawMappings[j];
-  for (; currentMapping === undefined; currentMapping = rawMappings[++j]);
   let line = 0;
   let lineStart = 0;
   if (currentMapping !== lineStart) {
     maybeAddSegment(map, line, 0, filePath, line, 0);
   }
-  for (let i = 0; i < code.length; i++) {
+  for (let i = 0; i < generatedCode.length; i++) {
     if (i === currentMapping) {
       const genColumn = currentMapping - lineStart;
-      const sourceColumn = sourceColumns[j];
+      const sourceColumn = sourceColumns[tokenIndex];
       maybeAddSegment(map, line, genColumn, filePath, line, sourceColumn);
-      for (
-        currentMapping = rawMappings[++j];
-        currentMapping === i || currentMapping === undefined;
-        currentMapping = rawMappings[++j]
-      );
+      while (
+        (currentMapping === i || currentMapping === undefined) &&
+        tokenIndex < rawMappings.length - 1
+      ) {
+        tokenIndex++;
+        currentMapping = rawMappings[tokenIndex];
+      }
     }
-    if (code.charCodeAt(i) === charCodes.lineFeed) {
+    if (generatedCode.charCodeAt(i) === charCodes.lineFeed) {
       line++;
       lineStart = i + 1;
       if (currentMapping !== lineStart) {
         maybeAddSegment(map, line, 0, filePath, line, 0);
       }
     }
   }
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
   const {sourceRoot, sourcesContent, ...sourceMap} = toEncodedMap(map);
   return sourceMap as RawSourceMap;
 }
+
+/**
+ * Create an array mapping each token index to the 0-based column of the start
+ * position of the token.
+ */
+function computeSourceColumns(code: string, tokens: Array<Token>): Array<number> {
+  const sourceColumns: Array<number> = new Array(tokens.length);
+  let tokenIndex = 0;
+  let currentMapping = tokens[tokenIndex].start;
+  let lineStart = 0;
+  for (let i = 0; i < code.length; i++) {
+    if (i === currentMapping) {
+      sourceColumns[tokenIndex] = currentMapping - lineStart;
+      tokenIndex++;
+      currentMapping = tokens[tokenIndex].start;
+    }
+    if (code.charCodeAt(i) === charCodes.lineFeed) {
+      lineStart = i + 1;
+    }
+  }
+  return sourceColumns;
+}

src/transformers/RootTransformer.ts

@@ -20,6 +20,8 @@ import TypeScriptTransformer from "./TypeScriptTransformer";
 
 export interface RootTransformerResult {
   code: string;
+  // Array mapping input token index to optional string index position in the
+  // output code.
   mappings: Array<number | undefined>;
 }
 
@@ -154,7 +156,9 @@ export default class RootTransformer {
       }
       return {
         code: code.slice(0, newlineIndex + 1) + prefix + code.slice(newlineIndex + 1) + suffix,
-        mappings: this.shiftMappings(result.mappings, prefix.length, newlineIndex),
+        // The hashbang line has no tokens, so shifting the tokens to account
+        // for prefix can happen normally.
+        mappings: this.shiftMappings(result.mappings, prefix.length),
       };
     } else {
       return {
@@ -442,18 +446,8 @@ export default class RootTransformer {
   shiftMappings(
     mappings: Array<number | undefined>,
     prefixLength: number,
-    newlineIndex = -1,
   ): Array<number | undefined> {
-    let i = 0;
-    if (newlineIndex >= 0) {
-      for (; i < mappings.length; i++) {
-        const mapping = mappings[i];
-        if (mapping !== undefined && mapping > newlineIndex) {
-          break;
-        }
-      }
-    }
-    for (; i < mappings.length; i++) {
+    for (let i = 0; i < mappings.length; i++) {
       const mapping = mappings[i];
       if (mapping !== undefined) {
         mappings[i] = mapping + prefixLength;

test/source-maps-test.ts

@@ -1,48 +1,70 @@
+import {eachMapping, TraceMap, SourceMapInput, EachMapping} from "@jridgewell/trace-mapping";
 import * as assert from "assert";
-import * as fs from "fs";
-import * as path from "path";
 
 import {transform} from "../src";
 
 describe("source maps", () => {
-  const testCase = (simple: boolean): void => {
+  it("generates a detailed line-based source map", () => {
     const source = `\
       import a from "./a";
-      const x: number = 1;
+      const x: number = a;
       console.log(x + 1);
     `;
     const result = transform(source, {
       transforms: ["imports", "typescript"],
-      sourceMapOptions: {compiledFilename: "test.js", simple},
+      sourceMapOptions: {compiledFilename: "test.js"},
       filePath: "test.ts",
     });
-    const simpleMappings = "AAAA;AACA;AACA;AACA";
+    assert.equal(
+      result.code,
+      `"use strict"; function _interopRequireDefault(obj) { \
+return obj && obj.__esModule ? obj : { default: obj }; }      \
+var _a = require('./a'); var _a2 = _interopRequireDefault(_a);
+      const x = _a2.default;
+      console.log(x + 1);
+    `,
+    );
     assert.deepEqual(result.sourceMap, {
       version: 3,
       sources: ["test.ts"],
       names: [],
-      mappings: simple ? simpleMappings : result.sourceMap!.mappings,
+      mappings: `AAAA,mHAAM,8DAAmB;AACzB,MAAM,MAAM,CAAC,CAAS,EAAE,WAAC;AACzB,\
+MAAM,OAAO,CAAC,GAAG,CAAC,EAAE,EAAE,CAAC,CAAC;AACxB`,
       file: "test.js",
     });
-    if (!simple) {
-      if (process.env.WRITE_SOURCE_MAPS) {
-        const outDir = path.join(__dirname, "output");
-        fs.mkdirSync(outDir, {recursive: true});
-        result.sourceMap!.sourcesContent = [source];
-        let suffix = "//# sourceMapping";
-        suffix += `URL=test.js.map`;
-        fs.writeFileSync(path.join(outDir, "test.js"), `${result.code}\n${suffix}`);
-        fs.writeFileSync(path.join(outDir, "test.js.map"), JSON.stringify(result.sourceMap));
-      }
-      const {mappings} = result.sourceMap!;
-      assert.match(mappings, /^[^;]+(;[^;]+){3}$/); // 4 lines
-      assert.notEqual(mappings, simpleMappings);
-    }
-  };
-  it("generates a simple line-based source map", () => {
-    testCase(true);
-  });
-  it("generates a detailed line-based source map", () => {
-    testCase(false);
+    const traceMap = new TraceMap(result.sourceMap as SourceMapInput);
+    const mappings: Array<
+      Pick<EachMapping, "generatedLine" | "generatedColumn" | "originalLine" | "originalColumn">
+    > = [];
+    eachMapping(traceMap, ({generatedLine, generatedColumn, originalLine, originalColumn}) => {
+      mappings.push({generatedLine, generatedColumn, originalLine, originalColumn});
+    });
+    assert.deepEqual(
+      mappings,
+      [
+        {generatedLine: 1, generatedColumn: 0, originalLine: 1, originalColumn: 0},
+        {generatedLine: 1, generatedColumn: 115, originalLine: 1, originalColumn: 6},
+        {generatedLine: 1, generatedColumn: 177, originalLine: 1, originalColumn: 25},
+        {generatedLine: 2, generatedColumn: 0, originalLine: 2, originalColumn: 0},
+        {generatedLine: 2, generatedColumn: 6, originalLine: 2, originalColumn: 6},
+        {generatedLine: 2, generatedColumn: 12, originalLine: 2, originalColumn: 12},
+        {generatedLine: 2, generatedColumn: 13, originalLine: 2, originalColumn: 13},
+        {generatedLine: 2, generatedColumn: 14, originalLine: 2, originalColumn: 22},
+        {generatedLine: 2, generatedColumn: 16, originalLine: 2, originalColumn: 24},
+        {generatedLine: 2, generatedColumn: 27, originalLine: 2, originalColumn: 25},
+        {generatedLine: 3, generatedColumn: 0, originalLine: 3, originalColumn: 0},
+        {generatedLine: 3, generatedColumn: 6, originalLine: 3, originalColumn: 6},
+        {generatedLine: 3, generatedColumn: 13, originalLine: 3, originalColumn: 13},
+        {generatedLine: 3, generatedColumn: 14, originalLine: 3, originalColumn: 14},
+        {generatedLine: 3, generatedColumn: 17, originalLine: 3, originalColumn: 17},
+        {generatedLine: 3, generatedColumn: 18, originalLine: 3, originalColumn: 18},
+        {generatedLine: 3, generatedColumn: 20, originalLine: 3, originalColumn: 20},
+        {generatedLine: 3, generatedColumn: 22, originalLine: 3, originalColumn: 22},
+        {generatedLine: 3, generatedColumn: 23, originalLine: 3, originalColumn: 23},
+        {generatedLine: 3, generatedColumn: 24, originalLine: 3, originalColumn: 24},
+        {generatedLine: 4, generatedColumn: 0, originalLine: 4, originalColumn: 0},
+      ],
+      `Expected:\n${mappings.map((m) => `${JSON.stringify(m)},`).join("\n")}`,
+    );
   });
 });

yarn.lock

@@ -298,6 +298,14 @@
     "@jridgewell/resolve-uri" "3.1.0"
     "@jridgewell/sourcemap-codec" "1.4.14"
 
+"@jridgewell/trace-mapping@^0.3.18":
+  version "0.3.18"
+  resolved "https://registry.yarnpkg.com/@jridgewell/trace-mapping/-/trace-mapping-0.3.18.tgz#25783b2086daf6ff1dcb53c9249ae480e4dd4cd6"
+  integrity sha512-w+niJYzMHdd7USdiH2U6869nqhD2nbfZXND5Yp93qIbEmnDNk7PD48o+YchRVpzMU7M6jVCbenTR7PA1FLQ9pA==
+  dependencies:
+    "@jridgewell/resolve-uri" "3.1.0"
+    "@jridgewell/sourcemap-codec" "1.4.14"
+
 "@nodelib/fs.scandir@2.1.5":
   version "2.1.5"
   resolved "https://registry.yarnpkg.com/@nodelib/fs.scandir/-/fs.scandir-2.1.5.tgz#7619c2eb21b25483f6d167548b4cfd5a7488c3d5"