From 87fcc365f90a601fe919a85307fe3000a3349668 Mon Sep 17 00:00:00 2001 From: Pranay Prakash Date: Mon, 30 Mar 2026 00:52:47 -0700 Subject: [PATCH 1/2] Fix node-module-error plugin matching identifiers in multi-line comments The findIdentifierUsage function only stripped single-line comments and same-line block comments, but didn't track multi-line block comment state. Lines inside JSDoc/block comments (e.g. ` * ... Writable stream`) passed through unstripped, causing the plugin to point at comments instead of actual code usage. Co-Authored-By: Claude Opus 4.6 (1M context) --- .changeset/fix-multiline-comment-skip.md | 5 ++ .../src/node-module-esbuild-plugin.test.ts | 38 ++++++++++++++ .../src/node-module-esbuild-plugin.ts | 50 ++++++++++++++++--- 3 files changed, 86 insertions(+), 7 deletions(-) create mode 100644 .changeset/fix-multiline-comment-skip.md diff --git a/.changeset/fix-multiline-comment-skip.md b/.changeset/fix-multiline-comment-skip.md new file mode 100644 index 0000000000..8f0633b5c3 --- /dev/null +++ b/.changeset/fix-multiline-comment-skip.md @@ -0,0 +1,5 @@ +--- +'@workflow/builders': patch +--- + +Fix node-module-error plugin pointing at multi-line comments instead of code usage diff --git a/packages/builders/src/node-module-esbuild-plugin.test.ts b/packages/builders/src/node-module-esbuild-plugin.test.ts index cbd0cd0a6b..5072601929 100644 --- a/packages/builders/src/node-module-esbuild-plugin.test.ts +++ b/packages/builders/src/node-module-esbuild-plugin.test.ts @@ -378,6 +378,44 @@ describe('workflow-node-module-error plugin', () => { } }); + it('should not point to JSDoc comments when finding usage', async () => { + const testCode = ` + import { Writable } from "stream"; + /** + * Convert a Web WritableStream into a Node.js Writable stream + */ + export function workflow() { + return new Writable(); + } + `; + + const { failure } = await buildWorkflowWithViolation(testCode); + + expect(failure.errors).toHaveLength(1); + const violation = failure.errors[0]; + expect(violation.text).toContain('"stream" which is a Node.js module'); + // Should point to the actual code usage, not the JSDoc comment + expect(violation.location?.lineText).toContain('new Writable()'); + expect(violation.location?.lineText).not.toContain('*'); + }); + + it('should not point to single-line block comments when finding usage', async () => { + const testCode = ` + import { Writable } from "stream"; + /* Writable is used below */ + export function workflow() { + return new Writable(); + } + `; + + const { failure } = await buildWorkflowWithViolation(testCode); + + expect(failure.errors).toHaveLength(1); + const violation = failure.errors[0]; + // Should point to the actual code usage, not the comment + expect(violation.location?.lineText).toContain('new Writable()'); + }); + it('should error on Bun module imports', async () => { const testCode = ` import { serve } from "bun"; diff --git a/packages/builders/src/node-module-esbuild-plugin.ts b/packages/builders/src/node-module-esbuild-plugin.ts index 268ed7716b..356860c38d 100644 --- a/packages/builders/src/node-module-esbuild-plugin.ts +++ b/packages/builders/src/node-module-esbuild-plugin.ts @@ -159,18 +159,54 @@ function findIdentifierUsage( identifier: string ) { const usageRegex = new RegExp(`\\b${escapeRegExp(identifier)}\\b`); + let inBlockComment = false; for (let i = startIndex; i < lines.length; i += 1) { - const line = lines[i]; + let line = lines[i]; + + // Handle multi-line block comments (including JSDoc) + if (inBlockComment) { + const endIdx = line.indexOf('*/'); + if (endIdx === -1) { + // Entire line is inside a block comment + continue; + } + // Replace everything up to and including */ with spaces + line = ' '.repeat(endIdx + 2) + line.slice(endIdx + 2); + inBlockComment = false; + } - // Skip comments (both // and /* */ style) - const withoutComments = line - .replace(/\/\*[\s\S]*?\*\//g, '') - .replace(/\/\/.*$/, ''); + // Remove single-line block comments and detect block comment starts + let processed = ''; + let j = 0; + while (j < line.length) { + if (line[j] === '/' && line[j + 1] === '/') { + // Rest of line is a comment + processed += ' '.repeat(line.length - j); + break; + } + if (line[j] === '/' && line[j + 1] === '*') { + const endIdx = line.indexOf('*/', j + 2); + if (endIdx !== -1) { + // Inline block comment - replace with spaces + const len = endIdx + 2 - j; + processed += ' '.repeat(len); + j = endIdx + 2; + } else { + // Block comment starts here, continues on next lines + processed += ' '.repeat(line.length - j); + inBlockComment = true; + break; + } + } else { + processed += line[j]; + j += 1; + } + } // Remove (replace with spaces) string literals to avoid matching inside paths // Use escaped quote handling to properly match strings with escaped quotes - const withoutStrings = withoutComments + const withoutStrings = processed .replace(/'(?:[^'\\]|\\.)*'/g, (segment) => ' '.repeat(segment.length)) .replace(/"(?:[^"\\]|\\.)*"/g, (segment) => ' '.repeat(segment.length)) .replace(/`(?:[^`\\]|\\.)*`/g, (segment) => ' '.repeat(segment.length)); @@ -180,7 +216,7 @@ function findIdentifierUsage( return { line: i, column: match.index, - lineText: line, + lineText: lines[i], }; } } From aa319fe7217b5aaac5132ebdbfeef6b34978d99d Mon Sep 17 00:00:00 2001 From: Pranay Prakash Date: Mon, 30 Mar 2026 12:27:33 -0700 Subject: [PATCH 2/2] Strip string literals before scanning for comment delimiters Move string stripping before comment detection so that comment delimiters inside string literals (e.g. `const s = "/*"`) don't incorrectly trigger block comment mode. Adds regression test. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/node-module-esbuild-plugin.test.ts | 18 +++++++++++++ .../src/node-module-esbuild-plugin.ts | 25 ++++++++----------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/packages/builders/src/node-module-esbuild-plugin.test.ts b/packages/builders/src/node-module-esbuild-plugin.test.ts index 5072601929..63de04a883 100644 --- a/packages/builders/src/node-module-esbuild-plugin.test.ts +++ b/packages/builders/src/node-module-esbuild-plugin.test.ts @@ -416,6 +416,24 @@ describe('workflow-node-module-error plugin', () => { expect(violation.location?.lineText).toContain('new Writable()'); }); + it('should not treat comment delimiters inside strings as comments', async () => { + const testCode = ` + import { Writable } from "stream"; + const pattern = "/* Writable */"; + export function workflow() { + return new Writable(); + } + `; + + const { failure } = await buildWorkflowWithViolation(testCode); + + expect(failure.errors).toHaveLength(1); + const violation = failure.errors[0]; + expect(violation.text).toContain('"stream" which is a Node.js module'); + // Should point to actual code, not the string containing "Writable" + expect(violation.location?.lineText).toContain('new Writable()'); + }); + it('should error on Bun module imports', async () => { const testCode = ` import { serve } from "bun"; diff --git a/packages/builders/src/node-module-esbuild-plugin.ts b/packages/builders/src/node-module-esbuild-plugin.ts index 356860c38d..4d23c94069 100644 --- a/packages/builders/src/node-module-esbuild-plugin.ts +++ b/packages/builders/src/node-module-esbuild-plugin.ts @@ -162,38 +162,40 @@ function findIdentifierUsage( let inBlockComment = false; for (let i = startIndex; i < lines.length; i += 1) { - let line = lines[i]; + // Strip string literals first so that comment delimiters inside strings + // (e.g. `const s = "/*";`) don't confuse the comment scanner. + const stringsStripped = lines[i] + .replace(/'(?:[^'\\]|\\.)*'/g, (s) => ' '.repeat(s.length)) + .replace(/"(?:[^"\\]|\\.)*"/g, (s) => ' '.repeat(s.length)) + .replace(/`(?:[^`\\]|\\.)*`/g, (s) => ' '.repeat(s.length)); + + let line = stringsStripped; // Handle multi-line block comments (including JSDoc) if (inBlockComment) { const endIdx = line.indexOf('*/'); if (endIdx === -1) { - // Entire line is inside a block comment continue; } - // Replace everything up to and including */ with spaces line = ' '.repeat(endIdx + 2) + line.slice(endIdx + 2); inBlockComment = false; } - // Remove single-line block comments and detect block comment starts + // Scan for comments, replacing them with spaces let processed = ''; let j = 0; while (j < line.length) { if (line[j] === '/' && line[j + 1] === '/') { - // Rest of line is a comment processed += ' '.repeat(line.length - j); break; } if (line[j] === '/' && line[j + 1] === '*') { const endIdx = line.indexOf('*/', j + 2); if (endIdx !== -1) { - // Inline block comment - replace with spaces const len = endIdx + 2 - j; processed += ' '.repeat(len); j = endIdx + 2; } else { - // Block comment starts here, continues on next lines processed += ' '.repeat(line.length - j); inBlockComment = true; break; @@ -204,14 +206,7 @@ function findIdentifierUsage( } } - // Remove (replace with spaces) string literals to avoid matching inside paths - // Use escaped quote handling to properly match strings with escaped quotes - const withoutStrings = processed - .replace(/'(?:[^'\\]|\\.)*'/g, (segment) => ' '.repeat(segment.length)) - .replace(/"(?:[^"\\]|\\.)*"/g, (segment) => ' '.repeat(segment.length)) - .replace(/`(?:[^`\\]|\\.)*`/g, (segment) => ' '.repeat(segment.length)); - - const match = withoutStrings.match(usageRegex); + const match = processed.match(usageRegex); if (match && match.index !== undefined) { return { line: i,