diff --git a/packages/plugins/live-debugger/CONTRIBUTING.md b/packages/plugins/live-debugger/CONTRIBUTING.md index 001135be1..6e00d6625 100644 --- a/packages/plugins/live-debugger/CONTRIBUTING.md +++ b/packages/plugins/live-debugger/CONTRIBUTING.md @@ -6,6 +6,7 @@ Developer notes for the Live Debugger plugin. - [Development workflow](#development-workflow) +- [Function IDs](#function-ids) - [Symbol table](#symbol-table) - [Consumer build canary](#consumer-build-canary) - [Fresh checkouts and CI](#fresh-checkouts-and-ci) @@ -45,6 +46,18 @@ Generated code should keep the dormant runtime path small: call `$dd_probes(func When adding or changing `liveDebugger` configuration, update [`src/types.ts`](./src/types.ts), [`src/validate.ts`](./src/validate.ts), [`src/validate.test.ts`](./src/validate.test.ts), and the consumer-facing [`README.md`](./README.md). +## Function IDs + +Every instrumented function passes its ID to `$dd_probes`. Outside the plugin, IDs are opaque: Datadog stores them with the symbols and the Browser Debugger SDK matches probes on them, so the format can change here alone. [`src/transform/functionId.ts`](./src/transform/functionId.ts) generates them during the transform's traversal, and [`EXAMPLES.md`](./EXAMPLES.md#function-ids) describes the format. + +- The path part is computed once per module by [`getFunctionIdPath`](./src/paths.ts): the query string is removed, Yarn PnP virtual folders are resolved, and the path is made relative to the git repository root, or to the build root outside a repository. Symlinks are only resolved for files outside the root, so most modules don't touch the file system. +- The name part has no line numbers, so a function keeps its ID when the code around it changes. Duplicate qualified names get a `#` suffix in traversal order. The suffix is assigned before any skip check, so `namedOnly`, `functionTypes`, and skip comments don't change the IDs of other functions. +- A function's suffix changes when a function with the same qualified name is added before it. That's accepted, like a move. +- Names come from the code the plugin transforms, which earlier loaders may have compiled, not from the original source. Original names would need the input source map, which only webpack and Rspack provide, and decoding it for every module would cost build time. +- IDs are only unique within a module. Modules of the same file, such as a file imported with different queries, can share IDs. + +Keep the generation O(functions) per module, with no extra parsing and no per-function file system calls. + ## Symbol table The plugin can list the instrumented functions that ship in a build, for upload to Datadog's Symbol Database. Each entry has the function ID, a display name, the path relative to the git repository root, and the original line range. The table lives in [`src/symbol-table`](./src/symbol-table). @@ -62,7 +75,7 @@ How it works: - After a one-off build, outside watch mode and dev servers, the plugin scans the text of every emitted JavaScript chunk, without parsing it. It doesn't use the transform hook, which doesn't run for modules restored from a persistent cache and doesn't know which functions tree-shaking removes. - Every instrumented function has exactly one `$dd_probes('')` call at the start of its body and one `$dd_throw(` call at its end. The scan pairs them like brackets, which also gives each function's parent. - The transform anchors the `$dd_probes` call to the original body's opening brace, which gives the start line through the chunk's final source map. The end line is the last mapped position up to the `$dd_throw` call: for block bodies that's the closing brace. The code appended after an arrow function's expression body has no mapping, and minifiers leave some tokens unmapped, so the closest mapped token before it is used instead. -- The path and display name come from the function ID. Query strings are removed, Yarn PnP virtual paths and symlinks are resolved, and the path is made relative to the git root. Without a git repository, the table is skipped. +- The path and display name come from the function ID, whose path the transform already made relative to the git root. Without a git repository, the table is skipped. - Each chunk's debug ID is read from the runtime bootstrap, when debug IDs are enabled, to record which chunks contain each file. When changing instrumentation output, keep exactly one `$dd_probes` call at the start and one `$dd_throw` call at the end of every instrumented function, anchored to the body's braces. [`src/symbolTable.integration.test.ts`](./src/symbolTable.integration.test.ts) checks the table on every bundler. diff --git a/packages/plugins/live-debugger/EXAMPLES.md b/packages/plugins/live-debugger/EXAMPLES.md index badf90b77..e5c1ae9d9 100644 --- a/packages/plugins/live-debugger/EXAMPLES.md +++ b/packages/plugins/live-debugger/EXAMPLES.md @@ -28,7 +28,7 @@ A few conventions used throughout: - [Arrow inside a super call argument](#arrow-inside-a-super-call-argument) - [Arrow whose body is a super call](#arrow-whose-body-is-a-super-call) - [Function expressions keep their own this](#function-expressions-keep-their-own-this) -- [Anonymous function IDs](#anonymous-function-ids) +- [Function IDs](#function-ids) - [What is not instrumented](#what-is-not-instrumented) @@ -323,7 +323,7 @@ class Button extends Component { constructor(props) { let $dd_t; const handleClick = () => { - let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/Button.js;handleClick'); + let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/Button.js;Button.constructor/handleClick'); try { if ($dd_p0) $dd_p0 = $dd_entry($dd_p0, $dd_t); const $dd_rv0 = this.props.onClick(); @@ -341,8 +341,7 @@ class Button extends Component { An arrow nested in the `super(...)` argument is instrumented with the `$dd_t` receiver, and the surrounding `super(...)` call is wrapped to capture the receiver. This arrow is anonymous, so its -ID uses the `@::` form (see [Anonymous function IDs](#anonymous-function-ids)); -the exact numbers depend on the original source position. +ID names it `` (see [Function IDs](#function-ids)). ```js // Before @@ -357,7 +356,7 @@ class Widget extends Base { constructor(items) { let $dd_t; ($dd_t = super(items.map((x) => { - let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/widget.js;@4:16:0'); + let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/widget.js;Widget.constructor/'); try { if ($dd_p0) $dd_p0 = $dd_entry($dd_p0, $dd_t, {x}); const $dd_rv0 = x * 2; @@ -388,7 +387,7 @@ class Widget extends Base { constructor(args) { let $dd_t; const init = () => { - let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/widget.js;init'); + let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/widget.js;Widget.constructor/init'); try { if ($dd_p0) $dd_p0 = $dd_entry($dd_p0, $dd_t); const $dd_rv0 = ($dd_t = super(args)); @@ -420,7 +419,7 @@ class Widget extends Base { class Widget extends Base { constructor(items) { const double = function(x) { - let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/widget.js;double'); + let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/widget.js;Widget.constructor/double'); try { let $dd_rv0; if ($dd_p0) $dd_p0 = $dd_entry($dd_p0, this, {x}); @@ -432,25 +431,42 @@ class Widget extends Base { } ``` -## Anonymous function IDs +## Function IDs -Named functions get the ID `;`. Anonymous functions — including -callbacks that are not assigned to a variable, property, or method — instead use: +A function's ID is the path of its file, relative to the git repository root, followed by `;` +and the function's qualified name. Without a git repository, the path is relative to the build +root instead. -``` -;@:: -``` +The qualified name is the function's own name, prefixed by the qualified name of the closest +named function or class around it: `.` for methods and class properties, `/` +otherwise. Getters and setters end with `:get` and `:set`, and functions without a name are +``. Anonymous functions and classes don't prefix the names of the functions inside +them, and neither do object literals. For example, in `src/cart.js`: -where `` and `` are the function's position in the original source and -`` disambiguates multiple anonymous functions that share the same parent node. -For example, the callback in `[1, 2].map((x) => x * 2)` might become: +| Function | ID | +| --- | --- | +| `function add() {}` | `src/cart.js;add` | +| `total` in `class Cart { total() {} }` | `src/cart.js;Cart.total` | +| The `size` getter in `class Cart { get size() {} }` | `src/cart.js;Cart.size:get` | +| The arrow in `class Cart { handler = () => {} }` | `src/cart.js;Cart.handler` | +| The callback in `function doubleAll(xs) { return xs.map((x) => x * 2); }` | `src/cart.js;doubleAll/` | +| `open` in `function make() { return class Box { open() {} }; }` | `src/cart.js;make/Box.open` | + +When several functions of a file have the same qualified name, such as two callbacks in the same +function, the second one gets `#1`, the third one `#2`, and so on, in source order: ```js -let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/utils.js;@1:11:0'); +function load() { + // src/cart.js;load/ + fetchItems().then((items) => render(items)); + // src/cart.js;load/#1 + fetchPrices().then((prices) => render(prices)); +} ``` -> Setting [`liveDebugger.namedOnly`](./README.md#livedebuggernamedonly) to `true` skips -> anonymous callbacks entirely, so only the `;` form is produced. +IDs don't depend on line numbers, so a function keeps its ID when the code around it changes. It +gets a new ID when it's renamed, when it moves to another file, function, or class, or when a +function with the same qualified name is added before it. ## What is not instrumented diff --git a/packages/plugins/live-debugger/README.md b/packages/plugins/live-debugger/README.md index 6ae3575bf..1b4685b9d 100644 --- a/packages/plugins/live-debugger/README.md +++ b/packages/plugins/live-debugger/README.md @@ -95,7 +95,7 @@ Live Debugger also reads the build version from the top-level [`metadata.version The Live Debugger plugin automatically instruments all JavaScript functions in your application at build time. It adds lightweight checks that can be activated at runtime without rebuilding your code. Each instrumented function gets: -- A unique, stable function ID (format: `;`) +- A function ID that identifies the function and stays the same across builds until the function is renamed or moved - A `$dd_probes()` call that returns active probes for that function (or `undefined` if none) - Entry point tracking with parameter capture via `$dd_entry()`, which returns a handle identifying the current invocation - Return value tracking with local variable capture via `$dd_return()` diff --git a/packages/plugins/live-debugger/src/constants.ts b/packages/plugins/live-debugger/src/constants.ts index 2544e2adc..18656c365 100644 --- a/packages/plugins/live-debugger/src/constants.ts +++ b/packages/plugins/live-debugger/src/constants.ts @@ -14,9 +14,15 @@ export const SKIP_INSTRUMENTATION_COMMENT = '@dd-no-instrumentation'; // The symbol table relies on it marking the end of every instrumented function's body. export const LIVE_DEBUGGER_THROW_FUNCTION = '$dd_throw'; -// Function IDs are `;`. +// Function IDs are `;`. export const FUNCTION_ID_SEPARATOR = ';'; -// Function name used in the IDs of anonymous functions, followed by their location. +// Separates a function's name from the name of the function or class that contains it. +export const SCOPE_SEPARATOR = '/'; +// Separates the name of a method or class property from the name of its class. +export const CLASS_MEMBER_SEPARATOR = '.'; +// Starts the suffix that tells apart functions with the same qualified name in a module. +export const DUPLICATE_SUFFIX_SEPARATOR = '#'; +// Function name used in the IDs of anonymous functions. export const ANONYMOUS_FUNCTION_NAME = ''; // Path where an opt-in debug dump of the symbol table is written. diff --git a/packages/plugins/live-debugger/src/index.ts b/packages/plugins/live-debugger/src/index.ts index dd07454db..958169d25 100644 --- a/packages/plugins/live-debugger/src/index.ts +++ b/packages/plugins/live-debugger/src/index.ts @@ -11,6 +11,7 @@ import type { SourceMapCompact, UnpluginBuildContext } from 'unplugin'; import { CONFIG_KEY, PLUGIN_NAME } from './constants'; import { createFileExtensionPattern, createFileFilter } from './filter'; +import { getRepositoryRoot } from './paths'; import { getRuntimeBootstrap } from './runtime-bootstrap'; import { getSymbolTableHooks } from './symbol-table'; import { transformCode } from './transform'; @@ -43,6 +44,15 @@ export const getLiveDebuggerPlugin = ( let transformedFileCount = 0; let totalFunctions = 0; let totalFilesWithFunctions = 0; + let repositoryRoot: string | undefined; + + // Looked up on the first transform, once the bundler has set the build root. + const getFunctionIdRoot = () => { + if (repositoryRoot === undefined) { + repositoryRoot = getRepositoryRoot(context.buildRoot) ?? context.buildRoot; + } + return repositoryRoot; + }; return { name: PLUGIN_NAME, @@ -74,10 +84,11 @@ export const getLiveDebuggerPlugin = ( } try { + const functionIdRoot = getFunctionIdRoot(); const result = transformCode({ code, filePath: id, - buildRoot: context.buildRoot, + repositoryRoot: functionIdRoot, honorSkipComments: pluginOptions.honorSkipComments, functionTypes: pluginOptions.functionTypes, namedOnly: pluginOptions.namedOnly, diff --git a/packages/plugins/live-debugger/src/symbol-table/paths.test.ts b/packages/plugins/live-debugger/src/paths.test.ts similarity index 51% rename from packages/plugins/live-debugger/src/symbol-table/paths.test.ts rename to packages/plugins/live-debugger/src/paths.test.ts index 3bfdc0e23..233eb496c 100644 --- a/packages/plugins/live-debugger/src/symbol-table/paths.test.ts +++ b/packages/plugins/live-debugger/src/paths.test.ts @@ -2,16 +2,21 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +import { mkdir, outputFile, rm } from '@dd/core/helpers/fs'; +import { getTempWorkingDir } from '@dd/tests/_jest/helpers/env'; +import fsp from 'fs/promises'; +import path from 'path'; + import { - createSourceFileResolver, + getFunctionIdPath, isInsideDirectory, + isOutsideRoot, resolveYarnVirtualPath, stripQuery, } from './paths'; -// Paths that don't exist, so resolving symlinks leaves them unchanged. +// A path that doesn't exist, so resolving symlinks leaves paths in it unchanged. const REPOSITORY_ROOT = '/does-not-exist/repo'; -const BUILD_ROOT = `${REPOSITORY_ROOT}/app`; describe('stripQuery', () => { const cases = [ @@ -72,38 +77,93 @@ describe('resolveYarnVirtualPath', () => { }); }); -describe('createSourceFileResolver', () => { +describe('getFunctionIdPath', () => { const cases = [ { description: 'make a path relative to the repository root', - input: 'src/a.js', + input: `${REPOSITORY_ROOT}/app/src/a.js`, expected: 'app/src/a.js', }, { description: 'strip the query string', - input: 'src/Cart.vue?vue&type=script', + input: `${REPOSITORY_ROOT}/app/src/Cart.vue?vue&type=script`, expected: 'app/src/Cart.vue', }, - { - description: 'resolve a workspace package outside the build root', - input: '../packages/lib/index.js', - expected: 'packages/lib/index.js', - }, { description: 'resolve a Yarn virtual path', - input: '../.yarn/__virtual__/lib-virtual-abc/1/packages/lib/index.js', + input: `${REPOSITORY_ROOT}/.yarn/__virtual__/lib-virtual-abc/1/packages/lib/index.js`, expected: 'packages/lib/index.js', }, { - description: 'leave out a file outside the repository', - input: '../../shared/index.js', - expected: undefined, + description: 'keep a file outside the repository outside', + input: '/does-not-exist/shared/index.js', + expected: '../shared/index.js', }, ]; test.each(cases)('Should $description.', ({ input, expected }) => { - const resolveSourceFile = createSourceFileResolver(BUILD_ROOT, REPOSITORY_ROOT); - expect(resolveSourceFile(input)).toBe(expected); + expect(getFunctionIdPath(input, REPOSITORY_ROOT)).toBe(expected); + }); + + describe('with symlinks', () => { + let workingDir: string; + let repositoryRoot: string; + + beforeAll(async () => { + const seed = `${Math.abs(jest.getSeed())}.function-id-path.${Date.now()}`; + workingDir = getTempWorkingDir(seed); + repositoryRoot = path.resolve(workingDir, 'repo'); + const packageDir = path.resolve(repositoryRoot, 'packages/lib'); + const packageFile = path.resolve(packageDir, 'index.js'); + await outputFile(packageFile, 'export const lib = 1;'); + + // A link inside the repository, like a workspace package in node_modules. + const insideLink = path.resolve(repositoryRoot, 'app/node_modules/lib'); + const insideLinkDir = path.dirname(insideLink); + await mkdir(insideLinkDir); + await fsp.symlink(packageDir, insideLink, 'dir'); + + // A link outside the repository. + const outsideLink = path.resolve(workingDir, 'linked-lib'); + await fsp.symlink(packageDir, outsideLink, 'dir'); + }); + + afterAll(async () => { + await rm(workingDir); + }); + + const symlinkCases = [ + { + description: 'resolve a link outside the repository to its target', + input: 'linked-lib/index.js', + expected: 'packages/lib/index.js', + }, + { + description: 'keep the path of a link inside the repository', + input: 'repo/app/node_modules/lib/index.js', + expected: 'app/node_modules/lib/index.js', + }, + ]; + + test.each(symlinkCases)('Should $description.', ({ input, expected }) => { + const moduleId = path.resolve(workingDir, input); + expect(getFunctionIdPath(moduleId, repositoryRoot)).toBe(expected); + }); + }); +}); + +describe('isOutsideRoot', () => { + const cases = [ + { description: 'accept a nested path', input: 'src/a.js', expected: false }, + { description: 'accept a name starting with dots', input: '..a.js', expected: false }, + { description: 'reject a parent path', input: '../a.js', expected: true }, + { description: 'reject a Windows parent path', input: '..\\a.js', expected: true }, + { description: 'reject the parent directory', input: '..', expected: true }, + { description: 'reject an absolute path', input: '/repo/a.js', expected: true }, + ]; + + test.each(cases)('Should $description.', ({ input, expected }) => { + expect(isOutsideRoot(input)).toBe(expected); }); }); diff --git a/packages/plugins/live-debugger/src/symbol-table/paths.ts b/packages/plugins/live-debugger/src/paths.ts similarity index 64% rename from packages/plugins/live-debugger/src/symbol-table/paths.ts rename to packages/plugins/live-debugger/src/paths.ts index 6dfd7ef6b..05ef08fe8 100644 --- a/packages/plugins/live-debugger/src/symbol-table/paths.ts +++ b/packages/plugins/live-debugger/src/paths.ts @@ -49,37 +49,31 @@ export const getRepositoryRoot = (buildRoot: string): string | undefined => { return realpathSync(repositoryRoot); }; -const isOutside = (relativePath: string) => - relativePath === '' || - relativePath === '..' || - relativePath.startsWith(`..${path.sep}`) || - path.isAbsolute(relativePath); +// Whether a relative path, with either separator, points outside the directory it's relative to. +export const isOutsideRoot = (relativePath: string): boolean => { + const posixPath = toPosixPath(relativePath); + return posixPath === '..' || posixPath.startsWith('../') || path.isAbsolute(relativePath); +}; export const isInsideDirectory = (filepath: string, directory: string): boolean => { const relativePath = path.relative(directory, filepath); - return relativePath === '' || !isOutside(relativePath); + return !isOutsideRoot(relativePath); }; /** - * Return a function that converts the module path of a function ID to a path relative to the - * repository root, or to undefined when the module is outside the repository. + * Return the path part of the function IDs of a module: its file, without query string or Yarn PnP + * virtual folders, relative to `root`. Symlinks are only resolved for files outside `root`, which + * keeps the file system out of the common case. */ -export const createSourceFileResolver = (buildRoot: string, repositoryRoot: string) => { - const cache = new Map(); - - return (modulePath: string): string | undefined => { - if (cache.has(modulePath)) { - return cache.get(modulePath); - } - - const moduleFile = stripQuery(modulePath); - const absolutePath = path.resolve(buildRoot, moduleFile); - const unvirtualizedPath = resolveYarnVirtualPath(absolutePath); - const realPath = realpathSync(unvirtualizedPath); - const relativePath = path.relative(repositoryRoot, realPath); - const sourceFile = isOutside(relativePath) ? undefined : toPosixPath(relativePath); +export const getFunctionIdPath = (moduleId: string, root: string): string => { + const moduleFile = stripQuery(moduleId); + const unvirtualizedPath = resolveYarnVirtualPath(moduleFile); + const relativePath = path.relative(root, unvirtualizedPath); + if (!isOutsideRoot(relativePath)) { + return toPosixPath(relativePath); + } - cache.set(modulePath, sourceFile); - return sourceFile; - }; + const realPath = realpathSync(unvirtualizedPath); + const realRelativePath = path.relative(root, realPath); + return toPosixPath(realRelativePath); }; diff --git a/packages/plugins/live-debugger/src/symbol-table/index.ts b/packages/plugins/live-debugger/src/symbol-table/index.ts index 38dfff69d..d0af3b6da 100644 --- a/packages/plugins/live-debugger/src/symbol-table/index.ts +++ b/packages/plugins/live-debugger/src/symbol-table/index.ts @@ -7,11 +7,11 @@ import type { BuildReport, GlobalContext, Logger, Output, PluginOptions } from ' import path from 'path'; import { SYMBOL_TABLE_PATH_ENV } from '../constants'; +import { getRepositoryRoot, isInsideDirectory } from '../paths'; import type { LiveDebuggerOptionsWithDefaults } from '../types'; import { createBuildModeTracker } from './build-mode'; import { loadSourceMap, locateFunctions } from './locate'; -import { createSourceFileResolver, getRepositoryRoot, isInsideDirectory } from './paths'; import { readChunkDebugId, scanChunk } from './scan'; import type { SymbolTablePayload } from './serialize'; import { serializeSymbolTable } from './serialize'; @@ -45,6 +45,7 @@ export const generateSymbolTable = async ({ buildRoot, log, }: GenerateSymbolTableArgs): Promise => { + // Outside a git repository, function ID paths are relative to the build root instead. const repositoryRoot = getRepositoryRoot(buildRoot); if (!repositoryRoot) { log.warn(`Skipping ${LOG_PREFIX}: the build root isn't in a git repository.`); @@ -79,11 +80,7 @@ export const generateSymbolTable = async ({ chunks.push({ filepath: output.filepath, debugId, functions }); } - const resolveSourceFile = createSourceFileResolver(buildRoot, repositoryRoot); - const { table, outsideModulePaths, invalidFunctionIds } = buildSymbolTable( - chunks, - resolveSourceFile, - ); + const { table, outsideModulePaths, invalidFunctionIds } = buildSymbolTable(chunks); if (chunksWithoutSourceMap > 0) { log.warn( diff --git a/packages/plugins/live-debugger/src/symbol-table/table.test.ts b/packages/plugins/live-debugger/src/symbol-table/table.test.ts index 4c0bda07c..7b4818b28 100644 --- a/packages/plugins/live-debugger/src/symbol-table/table.test.ts +++ b/packages/plugins/live-debugger/src/symbol-table/table.test.ts @@ -9,10 +9,6 @@ import type { ChunkFunction, ChunkFunctions } from './types'; const DEBUG_ID_A = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa'; const DEBUG_ID_B = 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb'; -// Module paths are relative to the build root, `app` in the repository. -const resolveSourceFile = (modulePath: string) => - modulePath.startsWith('../../') ? undefined : `app/${modulePath.replace(/\?.*$/, '')}`; - const getFunction = ( functionId: string, lines: [number, number] | undefined, @@ -35,40 +31,42 @@ describe('buildSymbolTable', () => { { description: 'list the same function once when several chunks contain it', chunks: [ - getChunk(DEBUG_ID_A, [getFunction('src/a.js;add', [1, 3])]), - getChunk(DEBUG_ID_B, [getFunction('src/a.js;add', [1, 3])]), + getChunk(DEBUG_ID_A, [getFunction('app/src/a.js;add', [1, 3])]), + getChunk(DEBUG_ID_B, [getFunction('app/src/a.js;add', [1, 3])]), ], - expected: { functionIds: ['src/a.js;add'] }, + expected: { functionIds: ['app/src/a.js;add'] }, }, { + // Modules of the same file with different queries can share IDs. description: 'keep functions that share an ID at different places', chunks: [ getChunk(DEBUG_ID_A, [ - getFunction('src/a.js;render', [1, 3]), - getFunction('src/a.js;render', [5, 7]), + getFunction('app/src/a.js;render', [1, 3]), + getFunction('app/src/a.js;render', [5, 7]), ]), ], - expected: { functionIds: ['src/a.js;render', 'src/a.js;render'] }, + expected: { functionIds: ['app/src/a.js;render', 'app/src/a.js;render'] }, }, { description: 'leave out functions outside the repository and invalid IDs', chunks: [ getChunk(DEBUG_ID_A, [ - getFunction('../../shared/a.js;outside', [1, 3]), + getFunction('../shared/a.js;outside', [1, 3]), + getFunction('/elsewhere/a.js;absolute', [1, 3]), getFunction('no-separator', [1, 3]), - getFunction('src/a.js;inside', [1, 3]), + getFunction('app/src/a.js;inside', [1, 3]), ]), ], expected: { - functionIds: ['src/a.js;inside'], - outsideModulePaths: ['../../shared/a.js'], + functionIds: ['app/src/a.js;inside'], + outsideModulePaths: ['../shared/a.js', '/elsewhere/a.js'], invalidFunctionIds: ['no-separator'], }, }, ]; test.each(cases)('Should $description.', ({ chunks, expected }) => { - const result = buildSymbolTable(chunks, resolveSourceFile); + const result = buildSymbolTable(chunks); const functionIds = result.table.entries.map((entry) => entry.functionId); const outsideModulePaths = [...result.outsideModulePaths]; const invalidFunctionIds = [...result.invalidFunctionIds]; @@ -81,15 +79,15 @@ describe('buildSymbolTable', () => { test('Should drop copies without a line range when another copy has one.', () => { const chunks = [ getChunk(DEBUG_ID_A, [ - getFunction('src/a.js;outer', undefined), - getFunction('src/a.js;inner', [2, 4], 0), + getFunction('app/src/a.js;outer', undefined), + getFunction('app/src/a.js;inner', [2, 4], 0), ]), getChunk(DEBUG_ID_B, [ - getFunction('src/a.js;outer', [1, 9]), - getFunction('src/a.js;unmapped', undefined), + getFunction('app/src/a.js;outer', [1, 9]), + getFunction('app/src/a.js;unmapped', undefined), ]), ]; - const { table } = buildSymbolTable(chunks, resolveSourceFile); + const { table } = buildSymbolTable(chunks); const entries = table.entries.map(({ functionId, start, parentKey }) => ({ functionId, located: Boolean(start), @@ -97,22 +95,22 @@ describe('buildSymbolTable', () => { })); expect(entries).toEqual([ - { functionId: 'src/a.js;inner', located: true, hasParent: false }, - { functionId: 'src/a.js;outer', located: true, hasParent: false }, - { functionId: 'src/a.js;unmapped', located: false, hasParent: false }, + { functionId: 'app/src/a.js;inner', located: true, hasParent: false }, + { functionId: 'app/src/a.js;outer', located: true, hasParent: false }, + { functionId: 'app/src/a.js;unmapped', located: false, hasParent: false }, ]); }); test('Should record which chunks contain each file.', () => { const chunks = [ - getChunk(DEBUG_ID_B, [getFunction('src/a.js;add', [1, 3])]), + getChunk(DEBUG_ID_B, [getFunction('app/src/a.js;add', [1, 3])]), getChunk(DEBUG_ID_A, [ - getFunction('src/a.js;add', [1, 3]), - getFunction('src/b.js;sub', [1, 3]), + getFunction('app/src/a.js;add', [1, 3]), + getFunction('app/src/b.js;sub', [1, 3]), ]), getChunk(DEBUG_ID_A, []), ]; - const { table } = buildSymbolTable(chunks, resolveSourceFile); + const { table } = buildSymbolTable(chunks); const fileChunkDebugIds = Object.fromEntries(table.fileChunkDebugIds ?? []); expect(table.chunkDebugIds).toEqual([DEBUG_ID_A, DEBUG_ID_B]); @@ -124,10 +122,10 @@ describe('buildSymbolTable', () => { test('Should leave out chunk debug IDs when a chunk with functions has none.', () => { const chunks = [ - getChunk(DEBUG_ID_A, [getFunction('src/a.js;add', [1, 3])]), - getChunk(undefined, [getFunction('src/b.js;sub', [1, 3])]), + getChunk(DEBUG_ID_A, [getFunction('app/src/a.js;add', [1, 3])]), + getChunk(undefined, [getFunction('app/src/b.js;sub', [1, 3])]), ]; - const { table } = buildSymbolTable(chunks, resolveSourceFile); + const { table } = buildSymbolTable(chunks); expect(table.chunkDebugIds).toBeUndefined(); expect(table.fileChunkDebugIds).toBeUndefined(); @@ -136,13 +134,13 @@ describe('buildSymbolTable', () => { test('Should only nest functions under enclosing functions of the same file.', () => { const chunks = [ getChunk(DEBUG_ID_A, [ - getFunction('src/a.js;outer', [1, 9]), + getFunction('app/src/a.js;outer', [1, 9]), // Inlined into `outer` by a minifier. - getFunction('src/b.js;inlined', [1, 3], 0), - getFunction('src/a.js;inner', [2, 4], 1), + getFunction('app/src/b.js;inlined', [1, 3], 0), + getFunction('app/src/a.js;inner', [2, 4], 1), ]), ]; - const { table } = buildSymbolTable(chunks, resolveSourceFile); + const { table } = buildSymbolTable(chunks); const [outer, inlined, inner] = table.entries; expect(outer.parentKey).toBeUndefined(); @@ -155,13 +153,13 @@ describe('serializeSymbolTable', () => { test('Should convert the table to SymDB scopes in source order.', () => { const chunks = [ getChunk(DEBUG_ID_A, [ - getFunction('src/b.js;later', [10, 12]), - getFunction('src/b.js;outer', [1, 8]), - getFunction('src/b.js;@3:4:0', [3, 5], 1), - getFunction('src/a.js;unmapped', undefined), + getFunction('app/src/b.js;Cart.size:get', [10, 12]), + getFunction('app/src/b.js;outer', [1, 8]), + getFunction('app/src/b.js;outer/#1', [3, 5], 1), + getFunction('app/src/a.js;unmapped', undefined), ]), ]; - const { table } = buildSymbolTable(chunks, resolveSourceFile); + const { table } = buildSymbolTable(chunks); expect(serializeSymbolTable(table, '1.0.0')).toEqual({ language: 'browser', @@ -178,7 +176,7 @@ describe('serializeSymbolTable', () => { scope_type: 'FUNCTION', name: 'unmapped', source_file: 'app/src/a.js', - language_specifics: { function_id: 'src/a.js;unmapped' }, + language_specifics: { function_id: 'app/src/a.js;unmapped' }, }, ], }, @@ -194,7 +192,7 @@ describe('serializeSymbolTable', () => { source_file: 'app/src/b.js', start_line: 1, end_line: 8, - language_specifics: { function_id: 'src/b.js;outer' }, + language_specifics: { function_id: 'app/src/b.js;outer' }, scopes: [ { scope_type: 'FUNCTION', @@ -203,18 +201,18 @@ describe('serializeSymbolTable', () => { start_line: 3, end_line: 5, language_specifics: { - function_id: 'src/b.js;@3:4:0', + function_id: 'app/src/b.js;outer/#1', }, }, ], }, { scope_type: 'FUNCTION', - name: 'later', + name: 'Cart.size:get', source_file: 'app/src/b.js', start_line: 10, end_line: 12, - language_specifics: { function_id: 'src/b.js;later' }, + language_specifics: { function_id: 'app/src/b.js;Cart.size:get' }, }, ], }, @@ -223,8 +221,8 @@ describe('serializeSymbolTable', () => { }); test('Should leave out the version and chunk debug IDs when unknown.', () => { - const chunks = [getChunk(undefined, [getFunction('src/a.js;add', [1, 3])])]; - const { table } = buildSymbolTable(chunks, resolveSourceFile); + const chunks = [getChunk(undefined, [getFunction('app/src/a.js;add', [1, 3])])]; + const { table } = buildSymbolTable(chunks); const payload = serializeSymbolTable(table, undefined); expect(payload).not.toHaveProperty('version'); diff --git a/packages/plugins/live-debugger/src/symbol-table/table.ts b/packages/plugins/live-debugger/src/symbol-table/table.ts index 9a48e8eda..9010e9fbc 100644 --- a/packages/plugins/live-debugger/src/symbol-table/table.ts +++ b/packages/plugins/live-debugger/src/symbol-table/table.ts @@ -2,6 +2,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +import { isOutsideRoot } from '../paths'; import { parseFunctionId } from '../transform/functionId'; import type { @@ -24,8 +25,9 @@ const formatPosition = (position: SourcePosition | undefined) => position ? `${position.line}:${position.column}` : ''; // The same function can be in several chunks, or in several output formats of the same build. -// Functions that share an ID but are at different places are different functions (v1 IDs aren't -// unique), so the key includes the location. +// IDs are only unique within a module, and modules of the same file, such as a file imported with +// different queries, can share IDs. Functions that share an ID but are at different places are +// different functions, so the key includes the location. const getEntryKey = (chunkFunction: ChunkFunction, sourceFile: string) => { const start = formatPosition(chunkFunction.start); const end = formatPosition(chunkFunction.end); @@ -64,10 +66,7 @@ const removeUnlocatedCopies = (entries: SymbolTableEntry[]): SymbolTableEntry[] }; // Map the output's chunks to one entry per function, deduplicated across chunks. -export const buildSymbolTable = ( - chunks: ChunkFunctions[], - resolveSourceFile: (modulePath: string) => string | undefined, -): SymbolTableResult => { +export const buildSymbolTable = (chunks: ChunkFunctions[]): SymbolTableResult => { const entries = new Map(); const fileChunkDebugIds = new Map>(); const chunkDebugIds = new Set(); @@ -107,9 +106,10 @@ export const buildSymbolTable = ( continue; } - const sourceFile = resolveSourceFile(parsedId.modulePath); - if (!sourceFile) { - outsideModulePaths.add(parsedId.modulePath); + // The transform makes the path relative to the repository root. + const sourceFile = parsedId.modulePath; + if (isOutsideRoot(sourceFile)) { + outsideModulePaths.add(sourceFile); keys.push(undefined); sourceFiles.push(undefined); continue; diff --git a/packages/plugins/live-debugger/src/symbolTable.integration.test.ts b/packages/plugins/live-debugger/src/symbolTable.integration.test.ts index d1eb48702..cc2fb0729 100644 --- a/packages/plugins/live-debugger/src/symbolTable.integration.test.ts +++ b/packages/plugins/live-debugger/src/symbolTable.integration.test.ts @@ -231,6 +231,43 @@ describe('Live Debugger symbol table', () => { expect(uniqueFunctionIds).toEqual(outputFunctionIds[bundlerName]); }); + test('Should give function IDs a path relative to the repository root.', () => { + const functionIds = outputFunctionIds[bundlerName] ?? []; + const unexpectedFunctionIds = functionIds.filter( + (functionId) => + functionId.includes('__virtual__') || + functionId.includes('?') || + functionId.startsWith('../') || + path.isAbsolute(functionId), + ); + + expect(functionIds).toEqual( + expect.arrayContaining([ + 'app/src/math.js;add', + 'app/src/math.js;add/double', + 'app/src/main.js;', + 'packages/lib/index.js;libGreet', + 'packages/peer/index.js;peerHello', + ]), + ); + expect(unexpectedFunctionIds).toEqual([]); + + // The symbol table takes the path of each function from its ID. + const functionsInOtherFiles = getFunctions().filter( + (tableFunction) => + !tableFunction.functionId.startsWith(`${tableFunction.sourceFile};`), + ); + expect(functionsInOtherFiles).toEqual([]); + }); + + test('Should give each function ID a single location.', () => { + const functionIds = getFunctions().map((tableFunction) => tableFunction.functionId); + const uniqueFunctionIds = new Set(functionIds); + + expect(functionIds.length).toBeGreaterThan(0); + expect(uniqueFunctionIds.size).toBe(functionIds.length); + }); + test('Should describe the build.', () => { const table = getTable(); expect(table?.language).toBe('browser'); diff --git a/packages/plugins/live-debugger/src/transform/babel-traverse.d.ts b/packages/plugins/live-debugger/src/transform/babel-traverse.d.ts index 2d4b26907..e078c9abe 100644 --- a/packages/plugins/live-debugger/src/transform/babel-traverse.d.ts +++ b/packages/plugins/live-debugger/src/transform/babel-traverse.d.ts @@ -16,8 +16,15 @@ declare module '@babel/traverse' { type VisitorHandler = (path: BabelPath) => void; + // Handlers called when the traversal enters a node and when it leaves it. + interface VisitorHandlers { + enter?: VisitorHandler; + exit?: VisitorHandler; + } + interface Visitor { - Function?: VisitorHandler; + Class?: VisitorHandler | VisitorHandlers; + Function?: VisitorHandler | VisitorHandlers; } function traverse(ast: object, visitor: Visitor): void; diff --git a/packages/plugins/live-debugger/src/transform/corpus.test.ts b/packages/plugins/live-debugger/src/transform/corpus.test.ts index 25caa6de0..bc21c7005 100644 --- a/packages/plugins/live-debugger/src/transform/corpus.test.ts +++ b/packages/plugins/live-debugger/src/transform/corpus.test.ts @@ -24,7 +24,7 @@ const CORPUS_PACKAGES = [ 'picomatch', ]; -const BASE_OPTIONS: Omit = { +const BASE_OPTIONS: Omit = { honorSkipComments: true, functionTypes: undefined, namedOnly: false, @@ -65,7 +65,7 @@ describe('transformCode on real-world code', () => { ...BASE_OPTIONS, code, filePath, - buildRoot: packageRoot, + repositoryRoot: packageRoot, }); expect(result.failedCount).toBe(0); diff --git a/packages/plugins/live-debugger/src/transform/functionId.test.ts b/packages/plugins/live-debugger/src/transform/functionId.test.ts index 84ddce0d9..8ef691db7 100644 --- a/packages/plugins/live-debugger/src/transform/functionId.test.ts +++ b/packages/plugins/live-debugger/src/transform/functionId.test.ts @@ -3,35 +3,36 @@ // Copyright 2019-Present Datadog, Inc. import { parse } from '@babel/parser'; +import type _traverse from '@babel/traverse'; import type * as t from '@babel/types'; import type { BabelPath } from './babel-path.types'; import { resolveCjsDefaultExport } from './cjs-interop'; -import { generateFunctionId, getFunctionName, parseFunctionId } from './functionId'; +import { createFunctionIdGenerator, getFunctionName, parseFunctionId } from './functionId'; // eslint-disable-next-line @typescript-eslint/no-var-requires, global-require const babelTypes = require('@babel/types') as typeof import('@babel/types'); // eslint-disable-next-line @typescript-eslint/no-var-requires, global-require -const traverse = resolveCjsDefaultExport(require('@babel/traverse')) as ( - ast: t.Node, - visitors: Record) => void>, -) => void; +const traverse = resolveCjsDefaultExport(require('@babel/traverse')); -function parseFunctionPaths(code: string): BabelPath[] { - // The cast works around the @babel/parser bundling its own copy of - // @babel/types which is structurally identical but nominally distinct. - const ast = parse(code, { +const MODULE_PATH = 'src/cart.js'; + +function parseModule(code: string) { + return parse(code, { sourceType: 'module', plugins: ['typescript'], - }) as unknown as t.Node; + }); +} + +function parseFunctionPaths(code: string): BabelPath[] { + const ast = parseModule(code); const paths: BabelPath[] = []; traverse(ast, { - 'FunctionDeclaration|FunctionExpression|ArrowFunctionExpression|ObjectMethod|ClassMethod|ClassPrivateMethod': - (path: BabelPath) => { - paths.push(path); - }, + Function(path) { + paths.push(path); + }, }); return paths; @@ -45,6 +46,42 @@ function parseSingleFunction(code: string): BabelPath { return paths[0]; } +// The IDs of all the functions of a module, in source order, generated the way the transform does. +function getFunctionIds(code: string): string[] { + const ast = parseModule(code); + const generator = createFunctionIdGenerator(MODULE_PATH, babelTypes); + const functionIds: string[] = []; + + traverse(ast, { + Class: { + enter(path) { + generator.enterClass(path); + }, + exit() { + generator.exit(); + }, + }, + Function: { + enter(path) { + const functionId = generator.enterFunction(path); + functionIds.push(functionId); + }, + exit() { + generator.exit(); + }, + }, + }); + + return functionIds; +} + +// The qualified names of all the functions of a module, without the module path. +function getQualifiedNames(code: string): string[] { + const functionIds = getFunctionIds(code); + const prefix = `${MODULE_PATH};`; + return functionIds.map((functionId) => functionId.slice(prefix.length)); +} + describe('getFunctionName', () => { describe('named function declarations', () => { const cases = [ @@ -243,106 +280,320 @@ describe('getFunctionName', () => { }); }); -describe('generateFunctionId', () => { - describe('named functions', () => { +describe('createFunctionIdGenerator', () => { + test('should prefix the qualified name with the module path', () => { + expect(getFunctionIds('function add() {}')).toEqual(['src/cart.js;add']); + }); + + describe('qualified names', () => { const cases = [ { - description: 'produce relative-path;name for a named function', - code: 'function hello() {}', - filePath: '/project/src/utils.ts', - buildRoot: '/project', - expected: 'src/utils.ts;hello', + description: 'qualify methods by their class', + code: 'class Cart { total() {} static create() {} #secret() {} }', + expected: ['Cart.total', 'Cart.create', 'Cart.#secret'], + }, + { + description: 'mark class getters and setters', + code: 'class Cart { get size() {} set size(value) {} static get empty() {} }', + expected: ['Cart.size:get', 'Cart.size:set', 'Cart.empty:get'], + }, + { + description: 'mark private getters and setters', + code: 'class Cart { get #count() {} set #count(value) {} }', + expected: ['Cart.#count:get', 'Cart.#count:set'], + }, + { + description: 'mark object getters and setters', + code: 'const cart = { get size() {}, set size(value) {} };', + expected: ['size:get', 'size:set'], + }, + { + description: 'qualify class property values by their class', + code: 'class Cart { handler = () => {}; #onClick = function () {}; static from = () => {}; }', + expected: ['Cart.handler', 'Cart.#onClick', 'Cart.from'], + }, + { + description: 'name a class expression after its variable', + code: 'const Cart = class { total() {} };', + expected: ['Cart.total'], + }, + { + description: 'name a class expression after its assignment target', + code: 'exports.Cart = class { total() {} };', + expected: ['exports.Cart.total'], + }, + { + description: 'name a class expression after its object property', + code: 'const shop = { Cart: class { total() {} } };', + expected: ['Cart.total'], + }, + { + description: 'qualify a class assigned to a static property by its class', + code: 'class Cart { static Item = class { price() {} }; }', + expected: ['Cart.Item.price'], + }, + { + description: 'qualify functions by the function around them', + code: 'function doubleAll(items) { return items.map((item) => item * 2); }', + expected: ['doubleAll', 'doubleAll/'], + }, + { + description: 'qualify functions by the method around them', + code: 'class Cart { total() { return this.items.map(() => 1); } }', + expected: ['Cart.total', 'Cart.total/'], + }, + { + description: 'qualify a class by the function around it', + code: 'function make() { class Box { open() {} } return Box; }', + expected: ['make', 'make/Box.open'], + }, + { + description: + 'qualify functions that are part of a class property value by the class', + code: 'class Cart { handler = debounce(() => {}); static { init(() => {}); } }', + expected: ['Cart/', 'Cart/#1'], + }, + { + description: 'skip anonymous functions around a function', + code: '[1].forEach(function () { [2].forEach(() => { function helper() {} }); });', + expected: ['', '#1', 'helper'], + }, + { + description: 'skip an immediately invoked function around a function', + code: '(function () { const init = () => {}; })();', + expected: ['', 'init'], + }, + { + description: 'skip an anonymous class around a method', + code: 'function make() { return class { open() {} }; }', + expected: ['make', 'make/open'], + }, + { + description: 'skip an anonymous default export class around a method', + code: 'export default class { total() {} }', + expected: ['total'], + }, + { + description: 'not qualify functions by the object literal around them', + code: 'function setup() { return { load() {}, save: () => {} }; }', + expected: ['setup', 'setup/load', 'setup/save'], }, { - description: 'handle a deeply nested file path', - code: 'function hello() {}', - filePath: '/project/src/features/auth/utils.ts', - buildRoot: '/project', - expected: 'src/features/auth/utils.ts;hello', + description: 'keep the name of assignment targets', + code: 'function Cart() { this.total = function () {}; } Cart.prototype.size = function () {};', + expected: ['Cart', 'Cart/this.total', 'Cart.prototype.size'], }, { - description: 'handle a file at the build root', - code: 'function main() {}', - filePath: '/project/index.ts', - buildRoot: '/project', - expected: 'index.ts;main', + description: 'name a function without a name anonymous', + code: 'class Cart { [Symbol.iterator]() {} } export default function () {}', + expected: ['Cart.', ''], }, ]; - test.each(cases)('should $description', ({ code, filePath, buildRoot, expected }) => { - const fnPath = parseSingleFunction(code); - expect(generateFunctionId(filePath, buildRoot, fnPath, 0, babelTypes)).toBe(expected); + test.each(cases)('should $description', ({ code, expected }) => { + expect(getQualifiedNames(code)).toEqual(expected); }); }); - describe('anonymous functions', () => { - it('should produce @line:col:index for an anonymous function', () => { - const fnPath = parseSingleFunction('[() => {}]'); - const line = fnPath.node.loc?.start.line ?? 0; - const col = fnPath.node.loc?.start.column ?? 0; - - expect(generateFunctionId('/p/src/a.ts', '/p', fnPath, 3, babelTypes)).toBe( - `src/a.ts;@${line}:${col}:3`, - ); + describe('duplicate names', () => { + const cases = [ + { + description: 'number anonymous siblings in source order', + code: 'foo(() => {}); foo(() => {}); foo(() => {});', + expected: ['', '#1', '#2'], + }, + { + description: 'number functions with the same name in different blocks', + code: 'if (a) { function f() {} } else { function f() {} }', + expected: ['f', 'f#1'], + }, + { + description: 'number a method and a static method with the same name', + code: 'class Cart { load() {} static load() {} }', + expected: ['Cart.load', 'Cart.load#1'], + }, + { + description: 'only number functions with the same qualified name', + code: 'function a() { foo(() => {}); } function b() { foo(() => {}); }', + expected: ['a', 'a/', 'b', 'b/'], + }, + { + description: 'qualify functions by the numbered name of the function around them', + code: '{ const render = () => () => {}; } { const render = () => () => {}; }', + expected: ['render', 'render/', 'render#1', 'render#1/'], + }, + { + description: 'skip suffixes that another function has as its name', + code: "const a = { 'f#1'() {} }; const b = { f() {} }; const c = { f() {} };", + expected: ['f#1', 'f', 'f#2'], + }, + { + description: 'number a name that looks like a suffix when it is taken', + code: "const a = { f() {} }; const b = { f() {} }; const c = { 'f#1'() {} };", + expected: ['f', 'f#1', 'f#1#1'], + }, + ]; + + test.each(cases)('should $description', ({ code, expected }) => { + expect(getQualifiedNames(code)).toEqual(expected); }); + }); - it('should differentiate anonymous siblings by index', () => { - const fnPath = parseSingleFunction('foo(() => {})'); - const line = fnPath.node.loc?.start.line ?? 0; - const col = fnPath.node.loc?.start.column ?? 0; + test('should give every function of a module a different ID', () => { + const code = ` + export default function () { return () => {}; } + export function f() {} + { function f() {} } + const o = { f() {}, 'f#1'() {}, get f() {}, set f(v) {}, [key]() {}, ['f']() {}, 42: () => {} }; + class A { + constructor() { this.f = () => {}; } + f() {} + static f() {} + #f() {} + get f() {} + set f(v) {} + static { (() => {})(); } + f = () => {}; + [Symbol.iterator]() {} + [Symbol.asyncIterator]() {} + 'quoted name'() {} + } + const B = class { f() {} }; + class C extends class { f() {} } { f() { return [1].map(() => [2].map(() => {})); } } + (function () { function f() {} })(); + (() => { const f = () => {}; })(); + a.b.c = function () {}; + a.b.c = function () {}; + obj[key] = () => {}; + obj[key] = () => {}; + `; + const functionIds = getFunctionIds(code); + const uniqueFunctionIds = new Set(functionIds); + const functionCount = parseFunctionPaths(code).length; + + expect(functionIds).toHaveLength(functionCount); + expect(uniqueFunctionIds.size).toBe(functionCount); + }); - const id0 = generateFunctionId('/p/f.ts', '/p', fnPath, 0, babelTypes); - const id1 = generateFunctionId('/p/f.ts', '/p', fnPath, 1, babelTypes); + describe('edits that keep IDs', () => { + const cases = [ + { + description: 'keep IDs when a function body changes', + before: 'function a() { return 1; }\nfoo(() => 2);', + after: 'function a() {\n const one = 1;\n return one;\n}\nfoo(() => {\n return 2;\n});', + expected: { + before: ['a', ''], + after: ['a', ''], + }, + }, + { + description: 'keep IDs when lines are added above', + before: 'class Cart { total() {} }\nfoo(() => 1);', + after: "import { x } from 'x';\n\nconsole.log(x);\n\nclass Cart { total() {} }\nfoo(() => 1);", + expected: { + before: ['Cart.total', ''], + after: ['Cart.total', ''], + }, + }, + { + description: 'keep IDs when functions with other names are added before them', + before: 'class Cart { get size() {} total() {} }', + after: 'function helper() {}\nclass Cart { count() {} get size() {} total() {} }', + expected: { + before: ['Cart.size:get', 'Cart.total'], + after: ['helper', 'Cart.count', 'Cart.size:get', 'Cart.total'], + }, + }, + { + description: 'keep IDs when an anonymous function is added to another function', + before: 'function a() {}\nfunction b() { foo(() => 2); }', + after: 'function a() { foo(() => 1); }\nfunction b() { foo(() => 2); }', + expected: { + before: ['a', 'b', 'b/'], + after: ['a', 'a/', 'b', 'b/'], + }, + }, + { + description: 'keep IDs when an anonymous sibling is added after them', + before: 'function load() { a.then(() => 1); }', + after: 'function load() { a.then(() => 1); b.then(() => 2); }', + expected: { + before: ['load', 'load/'], + after: ['load', 'load/', 'load/#1'], + }, + }, + { + description: 'keep IDs when another function is renamed', + before: 'function a() {}\nfunction b() { foo(() => {}); }', + after: 'function renamed() {}\nfunction b() { foo(() => {}); }', + expected: { + before: ['a', 'b', 'b/'], + after: ['renamed', 'b', 'b/'], + }, + }, + ]; - expect(id0).toBe(`f.ts;@${line}:${col}:0`); - expect(id1).toBe(`f.ts;@${line}:${col}:1`); - expect(id0).not.toBe(id1); + test.each(cases)('should $description', ({ before, after, expected }) => { + expect(getQualifiedNames(before)).toEqual(expected.before); + expect(getQualifiedNames(after)).toEqual(expected.after); }); + }); - it('should use 0:0 when location info is missing', () => { - const fnPath = parseSingleFunction('foo(() => {})'); - const original = fnPath.node.loc; - fnPath.node.loc = undefined as unknown as typeof original; - - expect(generateFunctionId('/p/f.ts', '/p', fnPath, 0, babelTypes)).toBe( - 'f.ts;@0:0:0', - ); + test('should shift the suffix of an anonymous function when a sibling is added before it', () => { + // Like a move: the callback keeps its name, but not its place among its siblings. + const before = getQualifiedNames('function load() { a.then(() => 1); }'); + const after = getQualifiedNames('function load() { b.then(() => 0); a.then(() => 1); }'); - fnPath.node.loc = original; - }); + expect(before).toEqual(['load', 'load/']); + expect(after).toEqual(['load', 'load/', 'load/#1']); }); }); describe('parseFunctionId', () => { const cases = [ { - description: 'split a named function ID', + description: 'split a top-level function ID', input: 'src/utils.js;add', expected: { modulePath: 'src/utils.js', name: 'add' }, }, { - description: 'keep separators that are part of the function name', - input: 'src/utils.js;get;user', - expected: { modulePath: 'src/utils.js', name: 'get;user' }, + description: 'keep the class of a class member', + input: 'src/cart.js;Cart.size:get', + expected: { modulePath: 'src/cart.js', name: 'Cart.size:get' }, }, { - description: 'drop the location of anonymous functions', - input: 'src/widget.js;@4:16:0', - expected: { modulePath: 'src/widget.js', name: '' }, + description: 'keep the class of a private method', + input: 'src/cart.js;Cart.#secret', + expected: { modulePath: 'src/cart.js', name: 'Cart.#secret' }, }, { - description: 'keep the query string of the module path', - input: 'src/Cart.vue?vue&type=script;total', - expected: { modulePath: 'src/Cart.vue?vue&type=script', name: 'total' }, + description: 'drop the names of the functions around the function', + input: 'src/cart.js;Cart.total/doubleAll/', + expected: { modulePath: 'src/cart.js', name: '' }, + }, + { + description: 'drop the duplicate suffix', + input: 'src/cart.js;render#1/double#2', + expected: { modulePath: 'src/cart.js', name: 'double' }, + }, + { + description: 'keep separators that are part of the function name', + input: 'src/utils.js;get;user', + expected: { modulePath: 'src/utils.js', name: 'get;user' }, }, { description: 'reject an ID without a module path', input: ';add', expected: undefined, }, + { + description: 'reject an ID without a separator', + input: 'add', + expected: undefined, + }, ]; - test.each(cases)('Should $description.', ({ input, expected }) => { + test.each(cases)('should $description', ({ input, expected }) => { expect(parseFunctionId(input)).toEqual(expected); }); }); diff --git a/packages/plugins/live-debugger/src/transform/functionId.ts b/packages/plugins/live-debugger/src/transform/functionId.ts index f5aaa7a9f..4401ef78a 100644 --- a/packages/plugins/live-debugger/src/transform/functionId.ts +++ b/packages/plugins/live-debugger/src/transform/functionId.ts @@ -3,22 +3,30 @@ // Copyright 2019-Present Datadog, Inc. import type * as t from '@babel/types'; -import path from 'path'; -import { ANONYMOUS_FUNCTION_NAME, FUNCTION_ID_SEPARATOR } from '../constants'; +import { + ANONYMOUS_FUNCTION_NAME, + CLASS_MEMBER_SEPARATOR, + DUPLICATE_SUFFIX_SEPARATOR, + FUNCTION_ID_SEPARATOR, + SCOPE_SEPARATOR, +} from '../constants'; import type { BabelPath, BabelTypesModule } from './babel-path.types'; export type ParsedFunctionId = { - // The module ID relative to the build root, including any query string. + // Path of the function's file, relative to the repository root. modulePath: string; - // The function name, without the location suffix of anonymous functions. + // The function's own name, without the names of its containers and its duplicate suffix. name: string; }; +const DUPLICATE_SUFFIX_RX = new RegExp(`${DUPLICATE_SUFFIX_SEPARATOR}\\d+$`); + /** - * Split a function ID generated by `generateFunctionId` into its module path and function name. - * Names can contain the separator (e.g. string keys), so the ID is split at its first occurrence. + * Split a function ID generated by `createFunctionIdGenerator` into its module path and the + * function's display name. Names can contain the separator (e.g. string keys), so the ID is split + * at its first occurrence. */ export function parseFunctionId(functionId: string): ParsedFunctionId | undefined { const separatorIndex = functionId.indexOf(FUNCTION_ID_SEPARATOR); @@ -27,37 +35,103 @@ export function parseFunctionId(functionId: string): ParsedFunctionId | undefine } const modulePath = functionId.slice(0, separatorIndex); - const functionName = functionId.slice(separatorIndex + FUNCTION_ID_SEPARATOR.length); - const isAnonymous = functionName.startsWith(`${ANONYMOUS_FUNCTION_NAME}@`); + const qualifiedName = functionId.slice(separatorIndex + FUNCTION_ID_SEPARATOR.length); + const ownNameStart = qualifiedName.lastIndexOf(SCOPE_SEPARATOR) + SCOPE_SEPARATOR.length; + const ownName = qualifiedName.slice(ownNameStart); - return { modulePath, name: isAnonymous ? ANONYMOUS_FUNCTION_NAME : functionName }; + return { modulePath, name: ownName.replace(DUPLICATE_SUFFIX_RX, '') }; } +// A function or class that the traversal is in. +type Container = { + // Qualified name of the nearest named function or class, which prefixes the names inside it. + name: string | undefined; + // Whether `name` is the container's own name and the container is a class. + isNamedClass: boolean; +}; + +export type FunctionIdGenerator = { + // Return the ID of a function, and make it the container of the functions inside it. + enterFunction: (functionPath: BabelPath) => string; + // Make a class the container of the functions inside it. + enterClass: (classPath: BabelPath) => void; + // Leave the function or class entered last. + exit: () => void; +}; + /** - * Generate a stable, unique function ID - * Format (POC): ; - * Example: src/utils.js;add + * Generate the IDs of a module's functions, as `;`, during a traversal + * that enters and exits every function and class of the module. * - * NOTE: This POC format only supports uniquely named functions. - * Anonymous functions will use the format ;: + * The qualified name is the function's own name (`` without one), prefixed by the + * qualified name of the nearest named function or class around it: `Cart.total` for a direct + * class member, `doubleAll/` otherwise. Anonymous functions and classes don't prefix + * the names inside them. Functions whose qualified name is already used in the module get the + * next free `#` suffix, in traversal order. Line numbers aren't part of the ID, so editing a + * function leaves the IDs of other functions unchanged. */ -export function generateFunctionId( - filePath: string, - buildRoot: string, - functionPath: BabelPath, - anonymousSiblingIndex: number, +export function createFunctionIdGenerator( + modulePath: string, typesModule: BabelTypesModule, -): string { - const relativePath = path.relative(buildRoot, filePath).replace(/\\/g, '/'); - const functionName = getFunctionName(functionPath, typesModule); - - if (functionName) { - return `${relativePath}${FUNCTION_ID_SEPARATOR}${functionName}`; - } +): FunctionIdGenerator { + const containers: Container[] = []; + const usedNames = new Set(); + // Per qualified name, the first suffix that may be free. + const nextSuffixes = new Map(); + + const getContainerName = () => containers[containers.length - 1]?.name; + + const qualify = (ownName: string, isClassMember: boolean) => { + const container = containers[containers.length - 1]; + if (!container?.name) { + return ownName; + } - const locationSuffix = getFunctionLocation(functionPath); + const separator = + container.isNamedClass && isClassMember ? CLASS_MEMBER_SEPARATOR : SCOPE_SEPARATOR; + return `${container.name}${separator}${ownName}`; + }; + + const getUniqueName = (name: string) => { + let uniqueName = name; + let suffix = nextSuffixes.get(name) ?? 1; + while (usedNames.has(uniqueName)) { + uniqueName = `${name}${DUPLICATE_SUFFIX_SEPARATOR}${suffix}`; + suffix++; + } - return `${relativePath}${FUNCTION_ID_SEPARATOR}${ANONYMOUS_FUNCTION_NAME}@${locationSuffix}:${anonymousSiblingIndex}`; + nextSuffixes.set(name, suffix); + usedNames.add(uniqueName); + return uniqueName; + }; + + return { + enterFunction(functionPath) { + const ownName = getOwnFunctionName(functionPath, typesModule); + const isClassMember = isDirectClassMember(functionPath, typesModule); + const qualifiedName = qualify(ownName ?? ANONYMOUS_FUNCTION_NAME, isClassMember); + const uniqueName = getUniqueName(qualifiedName); + const containerName = ownName ? uniqueName : getContainerName(); + + containers.push({ name: containerName, isNamedClass: false }); + return `${modulePath}${FUNCTION_ID_SEPARATOR}${uniqueName}`; + }, + enterClass(classPath) { + const className = getClassName(classPath, typesModule); + if (!className) { + const containerName = getContainerName(); + containers.push({ name: containerName, isNamedClass: false }); + return; + } + + const isClassMember = isDirectClassMember(classPath, typesModule); + const qualifiedName = qualify(className, isClassMember); + containers.push({ name: qualifiedName, isNamedClass: true }); + }, + exit() { + containers.pop(); + }, + }; } /** @@ -68,7 +142,6 @@ export function getFunctionName( typesModule: BabelTypesModule, ): string | null { const node = functionPath.node; - const parent = functionPath.parent; // Named function declaration: function foo() {} if ('id' in node && typesModule.isIdentifier(node.id)) { @@ -84,6 +157,40 @@ export function getFunctionName( return getPropertyLikeName(node.key, typesModule); } + return getAssignedName(functionPath.parent, typesModule); +} + +// The function's name, with a marker for getters and setters, which share the name of their property. +function getOwnFunctionName( + functionPath: BabelPath, + typesModule: BabelTypesModule, +): string | null { + const name = getFunctionName(functionPath, typesModule); + const node = functionPath.node; + if ( + name && + (typesModule.isObjectMethod(node) || + typesModule.isClassMethod(node) || + typesModule.isClassPrivateMethod(node)) && + (node.kind === 'get' || node.kind === 'set') + ) { + return `${name}:${node.kind}`; + } + + return name; +} + +function getClassName(classPath: BabelPath, typesModule: BabelTypesModule): string | null { + const id = classPath.node.id; + if (id) { + return id.name; + } + + return getAssignedName(classPath.parent, typesModule); +} + +// Name of a function or class from what it's assigned to. +function getAssignedName(parent: t.Node, typesModule: BabelTypesModule): string | null { // Variable declaration: const foo = () => {} if (typesModule.isVariableDeclarator(parent) && typesModule.isIdentifier(parent.id)) { return parent.id.name; @@ -106,6 +213,19 @@ export function getFunctionName( return null; } +// Whether a function or class is a method of a class or the value of one of its properties. +function isDirectClassMember(memberPath: BabelPath, typesModule: BabelTypesModule): boolean { + const { node, parent } = memberPath; + if (typesModule.isClassMethod(node) || typesModule.isClassPrivateMethod(node)) { + return true; + } + + return ( + (typesModule.isClassProperty(parent) || typesModule.isClassPrivateProperty(parent)) && + parent.value === node + ); +} + function getAssignmentTargetName( target: t.LVal | t.OptionalMemberExpression, typesModule: BabelTypesModule, @@ -192,10 +312,3 @@ function getPropertyLikeName( return null; } - -function getFunctionLocation(functionPath: BabelPath): string { - const line = functionPath.node.loc?.start.line ?? 0; - const column = functionPath.node.loc?.start.column ?? 0; - - return `${line}:${column}`; -} diff --git a/packages/plugins/live-debugger/src/transform/index.test.ts b/packages/plugins/live-debugger/src/transform/index.test.ts index 8555e3d23..39d235b51 100644 --- a/packages/plugins/live-debugger/src/transform/index.test.ts +++ b/packages/plugins/live-debugger/src/transform/index.test.ts @@ -13,7 +13,7 @@ import type { TransformOptions } from './index'; const BASE_OPTIONS: Omit = { filePath: '/src/utils.ts', - buildRoot: '/', + repositoryRoot: '/', honorSkipComments: false, functionTypes: undefined, namedOnly: false, @@ -1132,7 +1132,7 @@ describe('transformCode', () => { }); expect(result.instrumentedCount).toBe(1); - expect(result.code).toContain("$dd_probes('src/utils.ts;classMethod')"); + expect(result.code).toContain("$dd_probes('src/utils.ts;Foo.classMethod')"); }); it('should instrument only class private methods', () => { @@ -1143,7 +1143,7 @@ describe('transformCode', () => { }); expect(result.instrumentedCount).toBe(1); - expect(result.code).toContain("$dd_probes('src/utils.ts;#secret')"); + expect(result.code).toContain("$dd_probes('src/utils.ts;Foo.#secret')"); }); it('should instrument multiple selected types', () => { @@ -1156,7 +1156,7 @@ describe('transformCode', () => { expect(result.instrumentedCount).toBe(3); expect(result.code).toContain("$dd_probes('src/utils.ts;decl')"); expect(result.code).toContain("$dd_probes('src/utils.ts;arrow')"); - expect(result.code).toContain("$dd_probes('src/utils.ts;classMethod')"); + expect(result.code).toContain("$dd_probes('src/utils.ts;Foo.classMethod')"); }); it('should return original code when no functions match the filter', () => { @@ -1288,7 +1288,7 @@ describe('transformCode', () => { // outer is skipped, but inner should still be instrumented expect(result.skippedByCommentCount).toBe(1); expect(result.instrumentedCount).toBe(1); - expect(result.code).toContain("$dd_probes('src/utils.ts;inner')"); + expect(result.code).toContain("$dd_probes('src/utils.ts;outer/inner')"); }); it('should skip all declarators in a multi-declarator const with a comment above', () => { @@ -1361,6 +1361,81 @@ describe('transformCode', () => { }); }); + describe('function IDs', () => { + const PROBE_ID_RX = /\$dd_probes\('([^']*)'\)/g; + const getProbeIds = (code: string) => { + const matches = code.matchAll(PROBE_ID_RX); + return [...matches].map(([, functionId]) => functionId); + }; + + // The generator isn't instrumented, but its name is taken all the same. + const code = [ + '[1].map(() => 1);', + '// @dd-no-instrumentation', + '[2].map(() => 2);', + '[3].map(function () { return 3; });', + '{ function* f() {} }', + '{ function f() { return 4; } }', + ].join('\n'); + + const cases: { + description: string; + options: Partial; + expected: string[]; + }[] = [ + { + description: 'number functions with the same name in source order', + options: {}, + expected: [ + 'src/utils.ts;', + 'src/utils.ts;#1', + 'src/utils.ts;#2', + 'src/utils.ts;f#1', + ], + }, + { + description: 'keep the IDs of other functions when a skip comment skips one', + options: { honorSkipComments: true }, + expected: [ + 'src/utils.ts;', + 'src/utils.ts;#2', + 'src/utils.ts;f#1', + ], + }, + { + description: 'keep the IDs of other functions when functionTypes skips one', + options: { functionTypes: ['arrowFunction', 'functionDeclaration'] }, + expected: [ + 'src/utils.ts;', + 'src/utils.ts;#1', + 'src/utils.ts;f#1', + ], + }, + { + description: 'keep the IDs of named functions when namedOnly skips anonymous ones', + options: { namedOnly: true }, + expected: ['src/utils.ts;f#1'], + }, + ]; + + test.each(cases)('should $description', ({ options, expected }) => { + const result = transformCode({ ...BASE_OPTIONS, ...options, code }); + + expect(getProbeIds(result.code)).toEqual(expected); + }); + + it('should give the path relative to the repository root, without the query string', () => { + const result = transformCode({ + ...BASE_OPTIONS, + code: 'function total() { return 1; }', + filePath: '/repo/app/src/Cart.vue?vue&type=script&lang.ts', + repositoryRoot: '/repo', + }); + + expect(getProbeIds(result.code)).toEqual(['app/src/Cart.vue;total']); + }); + }); + describe('deterministic output', () => { it('should produce identical output across repeated transforms in the same process', () => { const options = { @@ -1558,7 +1633,7 @@ describe('transformCode', () => { 'class B extends A {', ' constructor(items) {let $dd_t;', ' ($dd_t = super(items.map((x) => {', - " let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/utils.ts;@4:16:0');", + " let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/utils.ts;B.constructor/');", ' try {', ' if ($dd_p0) $dd_p0 = $dd_entry($dd_p0, $dd_t, {x});', ' const $dd_rv0 = x * 2;', @@ -1592,7 +1667,7 @@ describe('transformCode', () => { 'class B extends A {', ' constructor(args) {let $dd_t;', ' const init = () => {', - " let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/utils.ts;init');", + " let $dd_p0 = globalThis.$dd_probes !== void 0 && $dd_probes('src/utils.ts;B.constructor/init');", ' try {', ' if ($dd_p0) $dd_p0 = $dd_entry($dd_p0, $dd_t);', ' const $dd_rv0 = ($dd_t = super(args));', diff --git a/packages/plugins/live-debugger/src/transform/index.ts b/packages/plugins/live-debugger/src/transform/index.ts index 139a14072..db286e9dd 100644 --- a/packages/plugins/live-debugger/src/transform/index.ts +++ b/packages/plugins/live-debugger/src/transform/index.ts @@ -10,11 +10,12 @@ import type MagicStringType from 'magic-string'; import type { SourceMap } from 'magic-string'; import { LIVE_DEBUGGER_THROW_FUNCTION, SKIP_INSTRUMENTATION_COMMENT } from '../constants'; +import { getFunctionIdPath } from '../paths'; import type { DecoratorSyntax, FunctionKind } from '../types'; import type { BabelPath, BabelTypesModule } from './babel-path.types'; import { resolveCjsDefaultExport } from './cjs-interop'; -import { generateFunctionId, getFunctionName } from './functionId'; +import { createFunctionIdGenerator, getFunctionName } from './functionId'; import { canInstrumentFunction, shouldSkipFunction } from './instrumentation'; import { getLocalVariableDeclarations, getParameterNames } from './scopeTracker'; import type { LocalVariableDeclaration } from './scopeTracker'; @@ -206,7 +207,8 @@ interface ConstructorThisAliasTarget { export interface TransformOptions { code: string; filePath: string; - buildRoot: string; + // Function ID paths are relative to it: the git repository root, or the build root outside one. + repositoryRoot: string; honorSkipComments: boolean; functionTypes: FunctionKind[] | undefined; namedOnly: boolean; @@ -243,8 +245,15 @@ export interface TransformResult { * Uses Babel to parse and read the AST, then MagicString for injection. */ export function transformCode(options: TransformOptions): TransformResult { - const { code, filePath, buildRoot, honorSkipComments, functionTypes, namedOnly, decorators } = - options; + const { + code, + filePath, + repositoryRoot, + honorSkipComments, + functionTypes, + namedOnly, + decorators, + } = options; let failedCount = 0; let instrumentedCount = 0; @@ -288,137 +297,141 @@ export function transformCode(options: TransformOptions): TransformResult { const constructorBodyStartsByNode = new Map(); const constructorsUsingThisAlias = new Set(); const superCallsByConstructorNode = new Map(); - const anonymousCountByParent = new Map(); + const functionIdPath = getFunctionIdPath(filePath, repositoryRoot); + const functionIds = createFunctionIdGenerator(functionIdPath, babelTypes); let probeVarCounter = 0; // @babel/parser and @babel/traverse bundle separate copies of @babel/types, // causing a structural type mismatch. The cast is safe: parse() returns a // valid AST that traverse() accepts at runtime. traverse(ast as unknown as BabelTypes.Node, { - Function(path: BabelPath) { - totalFunctions++; - - const derivedConstructor = getDerivedConstructor(path, babelTypes); - if (derivedConstructor) { - constructorBodyStartsByNode.set(derivedConstructor, derivedConstructor.body.start!); - superCallsByConstructorNode.set( - derivedConstructor, - getSuperCallTargets(derivedConstructor.body.body, babelTypes), - ); - } - - if (!canInstrumentFunction(path, babelTypes)) { - skippedUnsupportedCount++; - return; - } - - if ( - honorSkipComments && - shouldSkipFunction(path, SKIP_INSTRUMENTATION_COMMENT, babelTypes) - ) { - skippedByCommentCount++; - return; - } - - if (functionTypes && !functionTypes.includes(getFunctionKind(path.node, babelTypes))) { - skippedUnsupportedCount++; - return; - } - - if (namedOnly && !getFunctionName(path, babelTypes)) { - skippedUnsupportedCount++; - return; - } - - // O(1) anonymous sibling index via pre-indexed Map - let anonymousSiblingIndex = 0; - if (!getFunctionName(path, babelTypes)) { - const parentNode = path.parentPath?.node; - if (parentNode) { - anonymousSiblingIndex = anonymousCountByParent.get(parentNode) || 0; - anonymousCountByParent.set(parentNode, anonymousSiblingIndex + 1); - } - } - - const functionId = generateFunctionId( - filePath, - buildRoot, - path, - anonymousSiblingIndex, - babelTypes, - ); - - try { - const node = path.node; - const idx = probeVarCounter++; - const probeVarName = `$dd_p${idx}`; - const entryVars = getParameterNames(node, babelTypes); - const localVars = getLocalVariableDeclarations(node, babelTypes); - - const isExpressionBody = - babelTypes.isArrowFunctionExpression(node) && - !babelTypes.isBlockStatement(node.body); - const hasSequenceExpressionBody = - isExpressionBody && babelTypes.isSequenceExpression(node.body); - const constructorPath = getDerivedConstructorForLexicalThis(path, babelTypes); - const useThisAlias = constructorPath != null; - if (constructorPath) { - constructorsUsingThisAlias.add(constructorPath.node); + Class: { + enter(path: BabelPath) { + functionIds.enterClass(path); + }, + exit() { + functionIds.exit(); + }, + }, + Function: { + enter(path: BabelPath) { + totalFunctions++; + // Before the skip checks, so skipped functions don't change the IDs of other functions. + const functionId = functionIds.enterFunction(path); + + const derivedConstructor = getDerivedConstructor(path, babelTypes); + if (derivedConstructor) { constructorBodyStartsByNode.set( - constructorPath.node, - constructorPath.node.body.start!, + derivedConstructor, + derivedConstructor.body.start!, + ); + superCallsByConstructorNode.set( + derivedConstructor, + getSuperCallTargets(derivedConstructor.body.body, babelTypes), ); } - const returns: ReturnInfo[] = []; - const needsTrailingReturn = - isExpressionBody || - !babelTypes.isBlockStatement(node.body) || - !alwaysReturns(node.body.body, babelTypes); - if (!isExpressionBody && babelTypes.isBlockStatement(node.body)) { - collectReturnStatements(code, node.body.body, returns, babelTypes, false); + if (!canInstrumentFunction(path, babelTypes)) { + skippedUnsupportedCount++; + return; } - let directivesEnd: number | undefined; if ( - !isExpressionBody && - babelTypes.isBlockStatement(node.body) && - node.body.directives.length > 0 + honorSkipComments && + shouldSkipFunction(path, SKIP_INSTRUMENTATION_COMMENT, babelTypes) ) { - const lastDirective = node.body.directives[node.body.directives.length - 1]; - directivesEnd = lastDirective.end!; + skippedByCommentCount++; + return; } - targets.push({ - bodyStart: node.body.start!, - bodyPrefixStart: getExpressionBodyPrefixStart(code, node.body), - bodyEnd: node.body.end!, - functionEnd: node.end!, - isExpressionBody, - hasSequenceExpressionBody, - aliasesExpressionBodySuperCall: - useThisAlias && - isExpressionBody && - isSuperCallExpression(node.body, babelTypes), - needsTrailingReturn, - useThisAlias, - bodyParenStart: - isExpressionBody && typeof node.body.extra?.parenStart === 'number' - ? node.body.extra.parenStart - : undefined, - directivesEnd, - functionId, - probeVarName, - probeIdx: String(idx), - entryVars, - localVars, - returns, - }); - - instrumentedCount++; - } catch (error) { - failedCount++; - } + if ( + functionTypes && + !functionTypes.includes(getFunctionKind(path.node, babelTypes)) + ) { + skippedUnsupportedCount++; + return; + } + + if (namedOnly && !getFunctionName(path, babelTypes)) { + skippedUnsupportedCount++; + return; + } + + try { + const node = path.node; + const idx = probeVarCounter++; + const probeVarName = `$dd_p${idx}`; + const entryVars = getParameterNames(node, babelTypes); + const localVars = getLocalVariableDeclarations(node, babelTypes); + + const isExpressionBody = + babelTypes.isArrowFunctionExpression(node) && + !babelTypes.isBlockStatement(node.body); + const hasSequenceExpressionBody = + isExpressionBody && babelTypes.isSequenceExpression(node.body); + const constructorPath = getDerivedConstructorForLexicalThis(path, babelTypes); + const useThisAlias = constructorPath != null; + if (constructorPath) { + constructorsUsingThisAlias.add(constructorPath.node); + constructorBodyStartsByNode.set( + constructorPath.node, + constructorPath.node.body.start!, + ); + } + + const returns: ReturnInfo[] = []; + const needsTrailingReturn = + isExpressionBody || + !babelTypes.isBlockStatement(node.body) || + !alwaysReturns(node.body.body, babelTypes); + if (!isExpressionBody && babelTypes.isBlockStatement(node.body)) { + collectReturnStatements(code, node.body.body, returns, babelTypes, false); + } + + let directivesEnd: number | undefined; + if ( + !isExpressionBody && + babelTypes.isBlockStatement(node.body) && + node.body.directives.length > 0 + ) { + const lastDirective = node.body.directives[node.body.directives.length - 1]; + directivesEnd = lastDirective.end!; + } + + targets.push({ + bodyStart: node.body.start!, + bodyPrefixStart: getExpressionBodyPrefixStart(code, node.body), + bodyEnd: node.body.end!, + functionEnd: node.end!, + isExpressionBody, + hasSequenceExpressionBody, + aliasesExpressionBodySuperCall: + useThisAlias && + isExpressionBody && + isSuperCallExpression(node.body, babelTypes), + needsTrailingReturn, + useThisAlias, + bodyParenStart: + isExpressionBody && typeof node.body.extra?.parenStart === 'number' + ? node.body.extra.parenStart + : undefined, + directivesEnd, + functionId, + probeVarName, + probeIdx: String(idx), + entryVars, + localVars, + returns, + }); + + instrumentedCount++; + } catch (error) { + failedCount++; + } + }, + exit() { + functionIds.exit(); + }, }, }); diff --git a/packages/plugins/live-debugger/src/transform/invocationHandle.test.ts b/packages/plugins/live-debugger/src/transform/invocationHandle.test.ts index 87caff494..1e1b8ae9d 100644 --- a/packages/plugins/live-debugger/src/transform/invocationHandle.test.ts +++ b/packages/plugins/live-debugger/src/transform/invocationHandle.test.ts @@ -9,7 +9,7 @@ import type { TransformOptions } from './index'; const BASE_OPTIONS: Omit = { filePath: '/src/utils.ts', - buildRoot: '/', + repositoryRoot: '/', honorSkipComments: false, functionTypes: undefined, namedOnly: false, diff --git a/packages/plugins/live-debugger/src/transform/lazy-deps.test.ts b/packages/plugins/live-debugger/src/transform/lazy-deps.test.ts index 3c269d993..0e56fa34e 100644 --- a/packages/plugins/live-debugger/src/transform/lazy-deps.test.ts +++ b/packages/plugins/live-debugger/src/transform/lazy-deps.test.ts @@ -6,7 +6,7 @@ import type { TransformOptions } from './index'; const BASE_OPTIONS: Omit = { filePath: '/src/utils.ts', - buildRoot: '/', + repositoryRoot: '/', honorSkipComments: false, functionTypes: undefined, namedOnly: false,