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..63de04a883 100644 --- a/packages/builders/src/node-module-esbuild-plugin.test.ts +++ b/packages/builders/src/node-module-esbuild-plugin.test.ts @@ -378,6 +378,62 @@ 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 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 268ed7716b..4d23c94069 100644 --- a/packages/builders/src/node-module-esbuild-plugin.ts +++ b/packages/builders/src/node-module-esbuild-plugin.ts @@ -159,28 +159,59 @@ 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]; - - // Skip comments (both // and /* */ style) - const withoutComments = line - .replace(/\/\*[\s\S]*?\*\//g, '') - .replace(/\/\/.*$/, ''); + // 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) { + continue; + } + line = ' '.repeat(endIdx + 2) + line.slice(endIdx + 2); + inBlockComment = false; + } - // 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 - .replace(/'(?:[^'\\]|\\.)*'/g, (segment) => ' '.repeat(segment.length)) - .replace(/"(?:[^"\\]|\\.)*"/g, (segment) => ' '.repeat(segment.length)) - .replace(/`(?:[^`\\]|\\.)*`/g, (segment) => ' '.repeat(segment.length)); + // Scan for comments, replacing them with spaces + let processed = ''; + let j = 0; + while (j < line.length) { + if (line[j] === '/' && line[j + 1] === '/') { + processed += ' '.repeat(line.length - j); + break; + } + if (line[j] === '/' && line[j + 1] === '*') { + const endIdx = line.indexOf('*/', j + 2); + if (endIdx !== -1) { + const len = endIdx + 2 - j; + processed += ' '.repeat(len); + j = endIdx + 2; + } else { + processed += ' '.repeat(line.length - j); + inBlockComment = true; + break; + } + } else { + processed += line[j]; + j += 1; + } + } - const match = withoutStrings.match(usageRegex); + const match = processed.match(usageRegex); if (match && match.index !== undefined) { return { line: i, column: match.index, - lineText: line, + lineText: lines[i], }; } }