Show accurate location in resolution errors

Summary:
Changelog:
* **[Fix]**: Resolution errors now show accurate source locations.

D20107057 added code frames to resolution errors. That diff used a heuristic (simple string search for the import specifier) that has some unfortunate failure modes. For instance, if there is another import (say a Flow/TS `import type` declaration) with the same module specifier, the resulting error can falsely suggest that Metro is trying to follow the type import, which is confusing for users.

Here, we use the `dependency` object from the resolver context (newly available since D47453640) to instead construct a code frame based on the true location. We fall back to the old heuristic since the location can sometimes be missing.

This diff also adds integration tests to prevent regressions.

Reviewed By: GijsWeterings

Differential Revision: D47471112

fbshipit-source-id: 6f468059d91250a21f250e52aacc9dffcd36d07e

.flowconfig

@@ -19,6 +19,7 @@ suppress_type=$FlowFixMe
 suppress_type=$FlowFixMeProps
 suppress_type=$FlowFixMeState
 suppress_type=$FlowFixMeEmpty
+suppress_type=$FlowExpectedError
 
 [lints]
 sketchy-null-number=warn

packages/metro/src/integration_tests/__tests__/__snapshots__/build-errors-test.js.snap

@@ -0,0 +1,121 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`APIs/semantics reports resolution errors with CJS + non-inlined requires 1`] = `
+Unable to resolve module ./does-not-exist from <dir>/cannot-resolve-require.js: 
+
+None of these files exist:
+  * build-errors/does-not-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  * build-errors/does-not-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  13 |
+  14 | // $FlowExpectedError[cannot-resolve-module]
+> 15 | const DoesNotExist = require('./does-not-exist');
+     |                               ^
+  16 |
+  17 | global.x = (DoesNotExist: DoesNotExistT);
+  18 |
+`;
+
+exports[`APIs/semantics reports resolution errors with ESM + non-inlined requires 1`] = `
+Unable to resolve module ./does-not-exist from <dir>/cannot-resolve-import.js: 
+
+None of these files exist:
+  * build-errors/does-not-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  * build-errors/does-not-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  13 |
+  14 | // $FlowExpectedError[cannot-resolve-module]
+> 15 | import DoesNotExist from './does-not-exist';
+     |                           ^
+  16 |
+  17 | global.x = (DoesNotExist: DoesNotExistT);
+  18 |
+`;
+
+exports[`APIs/semantics reports resolution errors with inline requires + CJS 1`] = `
+Unable to resolve module ./does-not-exist from <dir>/inline-requires-cannot-resolve-require.js: 
+
+None of these files exist:
+  * build-errors/does-not-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  * build-errors/does-not-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  13 |
+  14 | // $FlowExpectedError[cannot-resolve-module]
+> 15 | const DoesNotExist = require('./does-not-exist');
+     |                               ^
+  16 |
+  17 | global.x = (DoesNotExist: DoesNotExistT);
+  18 |
+`;
+
+exports[`APIs/semantics reports resolution errors with inline requires + ESM 1`] = `
+Unable to resolve module ./does-not-exist from <dir>/inline-requires-cannot-resolve-import.js: 
+
+None of these files exist:
+  * build-errors/does-not-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  * build-errors/does-not-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  13 |
+  14 | // $FlowExpectedError[cannot-resolve-module]
+> 15 | import DoesNotExist from './does-not-exist';
+     |                           ^
+  16 |
+  17 | global.x = (DoesNotExist: DoesNotExistT);
+  18 |
+`;
+
+exports[`formatting edge cases reports resolution errors with a multi-line loc + specifier containing an escape sequence 1`] = `
+Unable to resolve module ./does-not'"-exist from <dir>/cannot-resolve-multi-line-import-with-escapes.js: 
+
+None of these files exist:
+  * build-errors/does-not'"-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  * build-errors/does-not'"-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  14 | import type DoesNotExistT from './does-not/'"-exist';
+  15 |
+> 16 | import {
+     | ^
+  17 |   aaaaaaaaaa,
+  18 |   bbbbbbbbbb,
+  19 |   cccccccccc,
+`;
+
+exports[`formatting edge cases reports resolution errors with a specifier containing an escape sequence 1`] = `
+Unable to resolve module ./does-not'"-exist from <dir>/cannot-resolve-specifier-with-escapes.js: 
+
+None of these files exist:
+  * build-errors/does-not'"-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  * build-errors/does-not'"-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  15 |
+  16 | // $FlowExpectedError[cannot-resolve-module]
+> 17 | import {DoesNotExist} from './does-not/'"-exist';
+     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  18 |
+  19 | global.x = (DoesNotExist: DoesNotExistT);
+  20 |
+`;
+
+exports[`formatting edge cases reports resolution errors with embedded comment after the specifier 1`] = `
+Unable to resolve module ./foo from <dir>/cannot-resolve-require-with-embedded-comment.js: 
+
+None of these files exist:
+  * build-errors/foo(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  * build-errors/foo/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  13 |
+  14 | // $FlowExpectedError[cannot-resolve-module]
+> 15 | const DoesNotExist = require('./foo' /* ./foo */);
+     |                               ^
+  16 |
+  17 | global.x = (DoesNotExist: DoesNotExistT);
+  18 |
+`;
+
+exports[`formatting edge cases reports resolution errors with multi-line locs 1`] = `
+Unable to resolve module ./does-not-exist from <dir>/cannot-resolve-multi-line-import.js: 
+
+None of these files exist:
+  * build-errors/does-not-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  * build-errors/does-not-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx)
+  25 |   iiiiiiiiii,
+  26 |   // $FlowExpectedError[cannot-resolve-module]
+> 27 | } from './does-not-exist';
+     |         ^
+  28 |
+  29 | global.x = (aaaaaaaaaa: DoesNotExistT);
+  30 |
+`;

packages/metro/src/integration_tests/__tests__/build-errors-test.js

@@ -0,0 +1,130 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ * @format
+ * @oncall react_native
+ */
+
+'use strict';
+
+const Metro = require('../../..');
+
+jest.unmock('cosmiconfig');
+
+jest.setTimeout(30 * 1000);
+
+const path = require('path');
+const BUILD_ERRORS_SRC_DIR =
+  path.resolve(__dirname, '..', 'basic_bundle', 'build-errors') + path.sep;
+
+expect.addSnapshotSerializer({
+  test: val => val instanceof Error,
+  print: (val: Error) =>
+    val.message
+      .replaceAll(BUILD_ERRORS_SRC_DIR, '<dir>/')
+      .replaceAll(path.win32.sep, path.posix.sep),
+});
+
+describe('APIs/semantics', () => {
+  test('reports resolution errors with inline requires + ESM', async () => {
+    const config = await Metro.loadConfig({
+      config: require.resolve('../metro.config.js'),
+    });
+
+    const buildPromise = Metro.runBuild(config, {
+      entry: 'build-errors/inline-requires-cannot-resolve-import.js',
+    });
+
+    await expect(buildPromise).rejects.toMatchSnapshot();
+  });
+
+  test('reports resolution errors with ESM + non-inlined requires', async () => {
+    const config = await Metro.loadConfig({
+      config: require.resolve('../metro.config.js'),
+    });
+
+    const buildPromise = Metro.runBuild(config, {
+      entry: 'build-errors/cannot-resolve-import.js',
+    });
+
+    await expect(buildPromise).rejects.toMatchSnapshot();
+  });
+
+  test('reports resolution errors with inline requires + CJS', async () => {
+    const config = await Metro.loadConfig({
+      config: require.resolve('../metro.config.js'),
+    });
+
+    const buildPromise = Metro.runBuild(config, {
+      entry: 'build-errors/inline-requires-cannot-resolve-require.js',
+    });
+
+    await expect(buildPromise).rejects.toMatchSnapshot();
+  });
+
+  test('reports resolution errors with CJS + non-inlined requires', async () => {
+    const config = await Metro.loadConfig({
+      config: require.resolve('../metro.config.js'),
+    });
+
+    const buildPromise = Metro.runBuild(config, {
+      entry: 'build-errors/cannot-resolve-require.js',
+    });
+
+    await expect(buildPromise).rejects.toMatchSnapshot();
+  });
+});
+
+describe('formatting edge cases', () => {
+  test('reports resolution errors with multi-line locs', async () => {
+    const config = await Metro.loadConfig({
+      config: require.resolve('../metro.config.js'),
+    });
+
+    const buildPromise = Metro.runBuild(config, {
+      entry: 'build-errors/cannot-resolve-multi-line-import.js',
+    });
+
+    await expect(buildPromise).rejects.toMatchSnapshot();
+  });
+
+  test('reports resolution errors with a specifier containing an escape sequence', async () => {
+    const config = await Metro.loadConfig({
+      config: require.resolve('../metro.config.js'),
+    });
+
+    const buildPromise = Metro.runBuild(config, {
+      entry: 'build-errors/cannot-resolve-specifier-with-escapes.js',
+    });
+
+    await expect(buildPromise).rejects.toMatchSnapshot();
+  });
+
+  test('reports resolution errors with a multi-line loc + specifier containing an escape sequence', async () => {
+    const config = await Metro.loadConfig({
+      config: require.resolve('../metro.config.js'),
+    });
+
+    const buildPromise = Metro.runBuild(config, {
+      entry: 'build-errors/cannot-resolve-multi-line-import-with-escapes.js',
+    });
+
+    await expect(buildPromise).rejects.toMatchSnapshot();
+  });
+
+  test('reports resolution errors with embedded comment after the specifier', async () => {
+    const config = await Metro.loadConfig({
+      config: require.resolve('../metro.config.js'),
+    });
+
+    const buildPromise = Metro.runBuild(config, {
+      entry: 'build-errors/cannot-resolve-require-with-embedded-comment.js',
+    });
+
+    await expect(buildPromise).rejects.toMatchSnapshot();
+  });
+});

packages/metro/src/integration_tests/basic_bundle/build-errors/cannot-resolve-import.js

@@ -0,0 +1,17 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ * @format
+ */
+
+// $FlowExpectedError[cannot-resolve-module]
+import type DoesNotExistT from './does-not-exist';
+
+// $FlowExpectedError[cannot-resolve-module]
+import DoesNotExist from './does-not-exist';
+
+global.x = (DoesNotExist: DoesNotExistT);

packages/metro/src/integration_tests/basic_bundle/build-errors/cannot-resolve-multi-line-import-with-escapes.js

@@ -0,0 +1,29 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ * @format
+ */
+
+/* eslint-disable no-unused-vars */
+
+// $FlowExpectedError[cannot-resolve-module]
+import type DoesNotExistT from './does-not\'"-exist';
+
+import {
+  aaaaaaaaaa,
+  bbbbbbbbbb,
+  cccccccccc,
+  dddddddddd,
+  eeeeeeeeee,
+  ffffffffff,
+  gggggggggg,
+  hhhhhhhhhh,
+  iiiiiiiiii,
+  // $FlowExpectedError[cannot-resolve-module]
+} from './does-not\'"-exist';
+
+global.x = (aaaaaaaaaa: DoesNotExistT);

packages/metro/src/integration_tests/basic_bundle/build-errors/cannot-resolve-multi-line-import.js

@@ -0,0 +1,29 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ * @format
+ */
+
+/* eslint-disable no-unused-vars */
+
+// $FlowExpectedError[cannot-resolve-module]
+import type DoesNotExistT from './does-not-exist';
+
+import {
+  aaaaaaaaaa,
+  bbbbbbbbbb,
+  cccccccccc,
+  dddddddddd,
+  eeeeeeeeee,
+  ffffffffff,
+  gggggggggg,
+  hhhhhhhhhh,
+  iiiiiiiiii,
+  // $FlowExpectedError[cannot-resolve-module]
+} from './does-not-exist';
+
+global.x = (aaaaaaaaaa: DoesNotExistT);

packages/metro/src/integration_tests/basic_bundle/build-errors/cannot-resolve-require-with-embedded-comment.js

@@ -0,0 +1,17 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ * @format
+ */
+
+// $FlowExpectedError[cannot-resolve-module]
+import type DoesNotExistT from './foo';
+
+// $FlowExpectedError[cannot-resolve-module]
+const DoesNotExist = require('./foo' /* ./foo */);
+
+global.x = (DoesNotExist: DoesNotExistT);

packages/metro/src/integration_tests/basic_bundle/build-errors/cannot-resolve-require.js

@@ -0,0 +1,17 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ * @format
+ */
+
+// $FlowExpectedError[cannot-resolve-module]
+import type DoesNotExistT from './does-not-exist';
+
+// $FlowExpectedError[cannot-resolve-module]
+const DoesNotExist = require('./does-not-exist');
+
+global.x = (DoesNotExist: DoesNotExistT);

packages/metro/src/integration_tests/basic_bundle/build-errors/cannot-resolve-specifier-with-escapes.js

@@ -0,0 +1,19 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ * @format
+ */
+
+/* eslint-disable no-unused-vars */
+
+// $FlowExpectedError[cannot-resolve-module]
+import type DoesNotExistT from './does-not\'"-exist';
+
+// $FlowExpectedError[cannot-resolve-module]
+import {DoesNotExist} from './does-not\'"-exist';
+
+global.x = (DoesNotExist: DoesNotExistT);