fix: better handling of multiple/relative outFiles

.ci/pipeline.yml

@@ -14,15 +14,15 @@ jobs:
 
 - job: Linux
  pool:
- vmImage: 'ubuntu-18.04'
+ vmImage: 'ubuntu-latest'
  steps:
  - template: common-validation.yml
  variables:
  node_version: 16.14.2
 
 - job: LinuxMinspec
  pool:
- vmImage: 'ubuntu-18.04'
+ vmImage: 'ubuntu-latest'
  steps:
  - template: common-validation.yml
  variables:

CHANGELOG.md

@@ -5,6 +5,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
 ## Nightly (only)
 
 - fix: breakpoints not setting in paths with special glob characters ([vscode#166400](https:/microsoft/vscode/issues/166400))
+- fix: better handling of multiple glob patterns and negations ([#1479](https:/microsoft/vscode-js-debug/issues/1479))
 - fix: skipFiles making catastrophic regexes ([#1469](https:/microsoft/vscode-js-debug/issues/1469))
 - fix: perScriptSourcemaps not reliably breaking ([vscode#166369](https:/microsoft/vscode/issues/166369))
 - fix: custom object `toString()` previews being too short ([vscode#155142](https:/microsoft/vscode/issues/155142))

src/adapter/breakpointPredictor.ts

@@ -211,7 +211,7 @@ export class BreakpointsPredictor implements IBreakpointsPredictor {
  this.logger.warn(LogTag.RuntimeSourceMap, 'Long breakpoint predictor runtime', {
  type: this.repo.constructor.name,
  longPredictionWarning,
- patterns: this.outFiles.patterns,
+ patterns: [...this.outFiles.explode()].join(', '),
  });
  }, longPredictionWarning);
 

src/common/fileGlobList.ts

@@ -5,6 +5,13 @@
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { AnyLaunchConfiguration } from '../configuration';
+import { forceForwardSlashes } from './pathUtils';
+
+export interface IExplodedGlob {
+ cwd: string;
+ negations: string[];
+ pattern: string;
+}
 
 @injectable()
 export class FileGlobList {
@@ -16,7 +23,7 @@ export class FileGlobList {
  /**
  * Search patterns, relative to the rootPath.
  */
- public readonly patterns: ReadonlyArray<string>;
+ private readonly patterns: ReadonlyArray<{ pattern: string; negated: boolean }>;
 
  /**
  * Returns whether there are any outFiles defined.
@@ -31,7 +38,68 @@ export class FileGlobList {
  this.patterns = [];
  } else {
  this.rootPath = rootPath;
- this.patterns = patterns.map(p => (path.isAbsolute(p) ? path.relative(rootPath, p) : p));
+ this.patterns = patterns.map(p => {
+ const negated = p.startsWith('!');
+ return { negated, pattern: p.slice(negated ? 1 : 0) };
+ });
+ }
+ }
+
+ /**
+ * Given a set of globs like (ignore spaces):
+ * - /project/foo/** /*.js
+ * - /project/../bar/** /*.js
+ * - !** /node_modules/**
+ *
+ * It returns an array where each entry is a positive glob and the processed
+ * negations that apply to that glob. Star-prefixed negations are applied to
+ * every path:
+ *
+ * - [ /project/foo/** /*.js, [ !/project/foo/** /node_modules/** ] ]
+ * - [ /bar/** /*.js, [ !/bar/** /node_modules/** ] ]
+ */
+ public *explode(): IterableIterator<IExplodedGlob> {
+ for (let i = 0; i < this.patterns.length; i++) {
+ const { negated, pattern } = this.patterns[i];
+ if (negated) {
+ continue;
+ }
+
+ const parts = path.resolve(this.rootPath, pattern).split(/[\\/]/g);
+ const firstGlobSegment = parts.findIndex(p => p.includes('*'));
+ // if including a single file, just return a glob that yields only that.
+ // note, here and below we intentionally use / instead of path.sep for globs
+ if (firstGlobSegment === -1) {
+ yield {
+ cwd: parts.slice(0, -1).join('/'),
+ pattern: parts[parts.length - 1],
+ negations: [],
+ };
+ continue;
+ }
+
+ const cwd = parts.slice(0, firstGlobSegment).join('/');
+ const negations = [];
+
+ for (let k = i + 1; k < this.patterns.length; k++) {
+ const { negated, pattern } = this.patterns[k];
+ if (!negated) {
+ continue;
+ }
+
+ // Make **-prefixed negations apply to _all_ included folders
+ if (pattern.startsWith('**')) {
+ negations.push(forceForwardSlashes(pattern));
+ } else {
+ // otherwise just resolve relative to this cwd
+ const rel = path.relative(cwd, path.resolve(this.rootPath, pattern));
+ if (!rel.startsWith('..')) {
+ negations.push(forceForwardSlashes(rel));
+ }
+ }
+ }
+
+ yield { cwd, negations, pattern: parts.slice(firstGlobSegment).join('/') };
  }
  }
 }

src/common/sourceMaps/codeSearchStrategy.ts

@@ -4,9 +4,9 @@
 
 import { injectable } from 'inversify';
 import type * as vscodeType from 'vscode';
-import { FileGlobList } from '../fileGlobList';
+import { FileGlobList, IExplodedGlob } from '../fileGlobList';
 import { ILogger, LogTag } from '../logging';
-import { forceForwardSlashes } from '../pathUtils';
+import { truthy } from '../objUtils';
 import { NodeSearchStrategy } from './nodeSearchStrategy';
 import { ISourceMapMetadata } from './sourceMap';
 import { createMetadataForFile, ISearchStrategy } from './sourceMapRepository';
@@ -53,29 +53,37 @@ export class CodeSearchStrategy implements ISearchStrategy {
  outFiles: FileGlobList,
  onChild: (child: Required<ISourceMapMetadata>) => T | Promise<T>,
  ): Promise<T[]> {
- // Fallback for unhandled case: https:/microsoft/vscode/issues/104889#issuecomment-993722692
- if (outFiles.patterns.some(p => p.startsWith('..'))) {
- return this.nodeStrategy.streamChildrenWithSourcemaps(outFiles, onChild);
- }
+ const todo: Promise<T | undefined>[] = [];
+ await Promise.all(
+ [...outFiles.explode()].map(glob => this._streamChildrenWithSourcemaps(onChild, glob, todo)),
+ );
+ const done = await Promise.all(todo);
+ return done.filter(truthy);
+ }
 
- const todo: Promise<T | void>[] = [];
+ private async _streamChildrenWithSourcemaps<T>(
+ onChild: (child: Required<ISourceMapMetadata>) => T | Promise<T>,
+ glob: IExplodedGlob,
+ todo: Promise<T | undefined>[],
+ ) {
  await this.vscode.workspace.findTextInFiles(
  { pattern: 'sourceMappingURL', isCaseSensitive: true },
  {
- ...this.getTextSearchOptions(outFiles),
+ ...this.getTextSearchOptions(glob),
  previewOptions: { charsPerLine: Number.MAX_SAFE_INTEGER, matchLines: 1 },
  },
  result => {
  const text = 'text' in result ? result.text : result.preview.text;
  todo.push(
  createMetadataForFile(result.uri.fsPath, text)
  .then(parsed => parsed && onChild(parsed))
- .catch(error =>
+ .catch(error => {
  this.logger.warn(LogTag.SourceMapParsing, 'Error parsing source map', {
  error,
  file: result.uri.fsPath,
- }),
- ),
+ });
+ return undefined;
+ }),
  );
  },
  );
@@ -87,16 +95,10 @@ export class CodeSearchStrategy implements ISearchStrategy {
  return results.filter((t): t is T => t !== undefined);
  }
 
- private getTextSearchOptions(files: FileGlobList): vscodeType.FindTextInFilesOptions {
+ private getTextSearchOptions(glob: IExplodedGlob): vscodeType.FindTextInFilesOptions {
  return {
- include: new this.vscode.RelativePattern(
- files.rootPath,
- forceForwardSlashes(files.patterns.filter(p => !p.startsWith('!')).join(', ')),
- ),
- exclude: files.patterns
- .filter(p => p.startsWith('!'))
- .map(p => forceForwardSlashes(p.slice(1)))
- .join(','),
+ include: new this.vscode.RelativePattern(this.vscode.Uri.file(glob.cwd), glob.pattern),
+ exclude: glob.negations.join(','),
  useDefaultExcludes: false,
  useIgnoreFiles: false,
  useGlobalIgnoreFiles: false,

src/common/sourceMaps/nodeSearchStrategy.ts

@@ -6,7 +6,7 @@ import globStream from 'glob-stream';
 import { inject, injectable } from 'inversify';
 import { FileGlobList } from '../fileGlobList';
 import { ILogger, LogTag } from '../logging';
-import { fixDriveLetterAndSlashes, forceForwardSlashes } from '../pathUtils';
+import { fixDriveLetterAndSlashes } from '../pathUtils';
 import { ISourceMapMetadata } from './sourceMap';
 import { createMetadataForFile, ISearchStrategy } from './sourceMapRepository';
 
@@ -26,13 +26,8 @@ export class NodeSearchStrategy implements ISearchStrategy {
  ): Promise<T[]> {
  const todo: (T | Promise<T>)[] = [];
 
- await new Promise((resolve, reject) =>
- this.globForFiles(files)
- .on('data', (value: globStream.Entry) =>
- todo.push(onChild(fixDriveLetterAndSlashes(value.path))),
- )
- .on('finish', resolve)
- .on('error', reject),
+ await this.globForFiles(files, value =>
+ todo.push(onChild(fixDriveLetterAndSlashes(value.path))),
  );
 
  // Type annotation is necessary for https:/microsoft/TypeScript/issues/47144
@@ -49,42 +44,48 @@ export class NodeSearchStrategy implements ISearchStrategy {
  ): Promise<T[]> {
  const todo: (T | Promise<T | void>)[] = [];
 
- await new Promise((resolve, reject) =>
- this.globForFiles(files)
- .on('data', (value: globStream.Entry) =>
- todo.push(
- createMetadataForFile(fixDriveLetterAndSlashes(value.path))
- .then(parsed => parsed && onChild(parsed))
- .catch(error =>
- this.logger.warn(LogTag.SourceMapParsing, 'Error parsing source map', {
- error,
- file: value.path,
- }),
- ),
+ await this.globForFiles(files, value =>
+ todo.push(
+ createMetadataForFile(fixDriveLetterAndSlashes(value.path))
+ .then(parsed => parsed && onChild(parsed))
+ .catch(error =>
+ this.logger.warn(LogTag.SourceMapParsing, 'Error parsing source map', {
+ error,
+ file: value.path,
+ }),
  ),
- )
- .on('finish', resolve)
- .on('error', reject),
+ ),
  );
 
  // Type annotation is necessary for https:/microsoft/TypeScript/issues/47144
  const results: (T | void)[] = await Promise.all(todo);
  return results.filter((t): t is T => t !== undefined);
  }
 
- private globForFiles(files: FileGlobList) {
- return globStream(
- [
- ...files.patterns.map(forceForwardSlashes),
- // Avoid reading asar files: electron patches in support for them, but
- // if we see an invalid one then it throws a synchronous error that
- // breaks glob. We don't care about asar's here, so just skip that:
- '!**/*.asar/**',
- ],
- {
- matchBase: true,
- cwd: files.rootPath,
- },
+ private async globForFiles(files: FileGlobList, onFile: (file: globStream.Entry) => void) {
+ await Promise.all(
+ [...files.explode()].map(
+ glob =>
+ new Promise((resolve, reject) =>
+ globStream(
+ [
+ glob.pattern,
+ // Avoid reading asar files: electron patches in support for them, but
+ // if we see an invalid one then it throws a synchronous error that
+ // breaks glob. We don't care about asar's here, so just skip that:
+ '!**/*.asar/**',
+ ],
+ {
+ ignore: glob.negations,
+ matchBase: true,
+ cwd: glob.cwd,
+ },
+ )
+ .on('data', onFile)
+ .on('finish', resolve)
+ .on('error', reject),
+ ),
+ ),
  );
  }
 }