fix: skipFiles ignoring negations of absolute paths and node internals

src/adapter/scriptSkipper/implementation.ts

@@ -33,17 +33,27 @@ interface ISharedSkipToggleEvent {
 }
 
 function preprocessNodeInternals(userSkipPatterns: ReadonlyArray<string>): string[] | undefined {
-  const nodeInternalRegex = /^<node_internals>[\/\\](.*)$/;
+  const nodeInternalRegex = /^(!?)<node_internals>[\/\\](.*)$/;
 
+  let hasPositive = false;
   const nodeInternalPatterns = userSkipPatterns
     .map(userPattern => {
       userPattern = userPattern.trim();
       const nodeInternalPattern = nodeInternalRegex.exec(userPattern);
-      return nodeInternalPattern ? nodeInternalPattern[1] : null;
+      if (!nodeInternalPattern?.[2]) {
+        return null;
+      }
+
+      // keep a leading `!`, micromatch applies it as a negation
+      const [, negation, glob] = nodeInternalPattern;
+      hasPositive ||= !negation;
+      return negation + glob;
     })
     .filter(truthy);
 
-  return nodeInternalPatterns.length > 0 ? nodeInternalPatterns : undefined;
+  // A negation only excludes internals that a positive entry skips. On its
+  // own, micromatch would match every module the negation does not name.
+  return hasPositive ? nodeInternalPatterns : undefined;
 }
 
 function preprocessAuthoredGlobs(
@@ -52,11 +62,16 @@ function preprocessAuthoredGlobs(
 ): string[] {
   const authoredGlobs = userSkipPatterns
     .filter(pattern => !pattern.includes(nodeInternalsToken))
-    .map(pattern =>
-      urlUtils.isAbsolute(pattern)
-        ? urlUtils.absolutePathToFileUrlWithDetection(spr.rebaseLocalToRemote(pattern))
-        : pathUtils.forceForwardSlashes(pattern)
-    )
+    .map(pattern => {
+      // Convert the glob without its negation, so negated absolute paths
+      // (`!${workspaceFolder}/foo/**`) also become file URLs.
+      const negated = pattern.startsWith('!');
+      const glob = negated ? pattern.slice(1) : pattern;
+      const converted = urlUtils.isAbsolute(glob)
+        ? urlUtils.absolutePathToFileUrlWithDetection(spr.rebaseLocalToRemote(glob))
+        : pathUtils.forceForwardSlashes(glob);
+      return negated ? `!${converted}` : converted;
+    })
     .map(urlUtils.lowerCaseInsensitivePath);
 
   return authoredGlobs;

src/adapter/scriptSkipper/scriptSkipper.test.ts

@@ -0,0 +1,109 @@
+/*---------------------------------------------------------
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ *--------------------------------------------------------*/
+
+import { expect } from 'chai';
+import Cdp from '../../cdp/api';
+import { Logger } from '../../common/logging/logger';
+import { upcastPartial } from '../../common/objUtils';
+import { ISourcePathResolver } from '../../common/sourcePathResolver';
+import { AnyLaunchConfiguration } from '../../configuration';
+import { ITarget } from '../../targets/targets';
+import { ScriptSkipper } from './implementation';
+
+describe('ScriptSkipper', () => {
+  let skipper: ScriptSkipper | undefined;
+  let blackboxPatterns: string[] = [];
+
+  const createSkipper = (skipFiles: string[]) => {
+    const cdp = upcastPartial<Cdp.Api>({
+      Debugger: upcastPartial<Cdp.DebuggerApi>({
+        setBlackboxPatterns: async ({ patterns }) => {
+          blackboxPatterns = patterns;
+          return {};
+        },
+      }),
+      Runtime: upcastPartial<Cdp.RuntimeApi>({
+        evaluate: async () => ({ result: { type: 'object', value: ['events', 'fs'] } }),
+      }),
+    });
+
+    skipper = new ScriptSkipper(
+      upcastPartial<AnyLaunchConfiguration>({ skipFiles }),
+      upcastPartial<ISourcePathResolver>({ rebaseLocalToRemote: p => p }),
+      Logger.null,
+      cdp,
+      upcastPartial<ITarget>({ type: () => 'node', id: () => 'a', parent: () => undefined }),
+    );
+
+    return skipper;
+  };
+
+  const isBlackboxed = (url: string) => blackboxPatterns.some(p => new RegExp(p, 'i').test(url));
+
+  afterEach(() => {
+    skipper?.dispose();
+    skipper = undefined;
+    blackboxPatterns = [];
+  });
+
+  const lodash = 'file:///proj/node_modules/lodash/index.js';
+  const math = 'file:///proj/node_modules/math/index.js';
+
+  it('applies relative negations', () => {
+    const s = createSkipper(['**/node_modules/**', '!**/node_modules/math/**']);
+    expect(s.isScriptSkipped(lodash)).to.be.true;
+    expect(s.isScriptSkipped(math)).to.be.false;
+    expect(isBlackboxed(lodash)).to.be.true;
+    expect(isBlackboxed(math)).to.be.false;
+  });
+
+  it('applies absolute negations', () => {
+    // e.g. "!${workspaceFolder}/node_modules/math/**" once variables are resolved
+    const s = createSkipper(['/proj/node_modules/**', '!/proj/node_modules/math/**']);
+    expect(s.isScriptSkipped(lodash)).to.be.true;
+    expect(s.isScriptSkipped(math)).to.be.false;
+    expect(isBlackboxed(lodash)).to.be.true;
+    expect(isBlackboxed(math)).to.be.false;
+  });
+
+  it('applies absolute negations to relative globs', () => {
+    const s = createSkipper(['**/node_modules/**', '!/proj/node_modules/math/**']);
+    expect(s.isScriptSkipped(lodash)).to.be.true;
+    expect(s.isScriptSkipped(math)).to.be.false;
+    expect(isBlackboxed(lodash)).to.be.true;
+    expect(isBlackboxed(math)).to.be.false;
+  });
+
+  it('applies node internals negations', async () => {
+    // Node 15+ names internals `node:events`, shown as `<node_internals>/events`
+    const s = createSkipper(['<node_internals>/**', '!<node_internals>/events']);
+    await new Promise(r => setImmediate(r)); // let the builtin module list load
+    expect(s.isScriptSkipped('node:fs')).to.be.true;
+    expect(s.isScriptSkipped('node:events')).to.be.false;
+  });
+
+  it('applies node internals negations to Node 14 module names', async () => {
+    // before Node 15, internals are named `events.js`
+    const s = createSkipper(['<node_internals>/**', '!<node_internals>/events.js']);
+    await new Promise(r => setImmediate(r));
+    expect(s.isScriptSkipped('fs.js')).to.be.true;
+    expect(s.isScriptSkipped('events.js')).to.be.false;
+    expect(isBlackboxed('fs.js')).to.be.true;
+    expect(isBlackboxed('events.js')).to.be.false;
+  });
+
+  it('ignores node internals negations without a positive glob', async () => {
+    const s = createSkipper(['!<node_internals>/events']);
+    await new Promise(r => setImmediate(r));
+    expect(s.isScriptSkipped('node:fs')).to.be.false;
+    expect(s.isScriptSkipped('node:events')).to.be.false;
+  });
+
+  it('keeps node internals globs that start with an extglob', async () => {
+    const s = createSkipper(['<node_internals>/!(events)']);
+    await new Promise(r => setImmediate(r));
+    expect(s.isScriptSkipped('node:fs')).to.be.true;
+    expect(s.isScriptSkipped('node:events')).to.be.false;
+  });
+});