From 1c38ad9220064013a335aca38750511dc409d3b8 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 21 Aug 2019 06:58:16 +0200 Subject: [PATCH 1/3] Prevent infinite loop when scanning for line-breaks and there are comment-like strings --- src/utils/renderHelpers.ts | 17 ++++++++++------- .../_config.js | 4 ++++ .../main.js | 5 +++++ 3 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 test/function/samples/asi-prevention-with-comment-like-strings/_config.js create mode 100644 test/function/samples/asi-prevention-with-comment-like-strings/main.js diff --git a/src/utils/renderHelpers.ts b/src/utils/renderHelpers.ts index 5f86c7e3c26..cd41986b1f2 100644 --- a/src/utils/renderHelpers.ts +++ b/src/utils/renderHelpers.ts @@ -47,8 +47,11 @@ export function findFirstOccurrenceOutsideComment(code: string, searchString: st } } -export function findFirstLineBreakOutsideComment(code: string, start = 0) { - let lineBreakPos, charCodeAfterSlash; +// This assumes there are no non-comment strings containing '/*' or '//' +function findFirstLineBreakOutsideComment(code: string) { + let lineBreakPos, + charCodeAfterSlash, + start = 0; lineBreakPos = code.indexOf('\n', start); while (true) { start = code.indexOf('/', start); @@ -166,14 +169,14 @@ export function getCommaSeparatedNodesWithBoundaries( return splitUpNodes; } +// This assumes there are no non-comment strings containing '/*' or '//' between start and end export function removeLineBreaks(code: MagicString, start: number, end: number) { - let lineBreakPos = start; while (true) { - lineBreakPos = findFirstLineBreakOutsideComment(code.original, lineBreakPos); - if (lineBreakPos === -1 || lineBreakPos >= end) { + const lineBreakPos = findFirstLineBreakOutsideComment(code.original.slice(start, end)); + if (lineBreakPos === -1) { break; } - code.remove(lineBreakPos, lineBreakPos + 1); - lineBreakPos++; + start = start + lineBreakPos + 1; + code.remove(start - 1, start); } } diff --git a/test/function/samples/asi-prevention-with-comment-like-strings/_config.js b/test/function/samples/asi-prevention-with-comment-like-strings/_config.js new file mode 100644 index 00000000000..97a6b347859 --- /dev/null +++ b/test/function/samples/asi-prevention-with-comment-like-strings/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: + 'do not hang when scanning for line-breaks in ASI prevention and there are comment-like strings' +}; diff --git a/test/function/samples/asi-prevention-with-comment-like-strings/main.js b/test/function/samples/asi-prevention-with-comment-like-strings/main.js new file mode 100644 index 00000000000..4f20d9bef3e --- /dev/null +++ b/test/function/samples/asi-prevention-with-comment-like-strings/main.js @@ -0,0 +1,5 @@ +function test() { + return true ? '/*' : '//' +} + +assert.strictEqual(test(), '/*'); From 6fca7d4b290c1bc7c781a5776de8f4ea23674559 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 21 Aug 2019 07:23:17 +0200 Subject: [PATCH 2/3] Improve coverage by removing dead code --- src/utils/renderHelpers.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/utils/renderHelpers.ts b/src/utils/renderHelpers.ts index cd41986b1f2..6b3a7781c9f 100644 --- a/src/utils/renderHelpers.ts +++ b/src/utils/renderHelpers.ts @@ -47,7 +47,7 @@ export function findFirstOccurrenceOutsideComment(code: string, searchString: st } } -// This assumes there are no non-comment strings containing '/*' or '//' +// This assumes "code" only contains white-space and comments function findFirstLineBreakOutsideComment(code: string) { let lineBreakPos, charCodeAfterSlash, @@ -56,14 +56,13 @@ function findFirstLineBreakOutsideComment(code: string) { while (true) { start = code.indexOf('/', start); if (start === -1 || start > lineBreakPos) return lineBreakPos; + + // With our assumption, '/' always starts a comment. Determine comment type: charCodeAfterSlash = code.charCodeAt(++start); if (charCodeAfterSlash === 47 /*"/"*/) return lineBreakPos; - ++start; - if (charCodeAfterSlash === 42 /*"*"*/) { - start = code.indexOf('*/', start) + 2; - if (start > lineBreakPos) { - lineBreakPos = code.indexOf('\n', start); - } + start = code.indexOf('*/', start + 2) + 2; + if (start > lineBreakPos) { + lineBreakPos = code.indexOf('\n', start); } } } @@ -75,7 +74,6 @@ export function renderStatementList( end: number, options: RenderOptions ) { - if (statements.length === 0) return; let currentNode, currentNodeStart, currentNodeNeedsBoundaries, nextNodeStart; let nextNode = statements[0]; let nextNodeNeedsBoundaries = !nextNode.included || nextNode.needsBoundaries; @@ -169,7 +167,7 @@ export function getCommaSeparatedNodesWithBoundaries( return splitUpNodes; } -// This assumes there are no non-comment strings containing '/*' or '//' between start and end +// This assumes there are only white-space and comments between start and end export function removeLineBreaks(code: MagicString, start: number, end: number) { while (true) { const lineBreakPos = findFirstLineBreakOutsideComment(code.original.slice(start, end)); From b6964db7e0957ebcf55c7e0f058458299de9e437 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 21 Aug 2019 07:53:28 +0200 Subject: [PATCH 3/3] Improve coverage by removing dead code --- src/utils/renderHelpers.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/utils/renderHelpers.ts b/src/utils/renderHelpers.ts index 6b3a7781c9f..92b9fdfc82f 100644 --- a/src/utils/renderHelpers.ts +++ b/src/utils/renderHelpers.ts @@ -24,6 +24,7 @@ export interface NodeRenderOptions { export const NO_SEMICOLON: NodeRenderOptions = { isNoStatement: true }; +// This assumes there are only white-space and comments between start and the string we are looking for export function findFirstOccurrenceOutsideComment(code: string, searchString: string, start = 0) { let searchPos, charCodeAfterSlash; searchPos = code.indexOf(searchString, start); @@ -32,17 +33,14 @@ export function findFirstOccurrenceOutsideComment(code: string, searchString: st if (start === -1 || start > searchPos) return searchPos; charCodeAfterSlash = code.charCodeAt(++start); ++start; - if (charCodeAfterSlash === 47 /*"/"*/) { - start = code.indexOf('\n', start) + 1; - if (start === 0) return -1; - if (start > searchPos) { - searchPos = code.indexOf(searchString, start); - } - } else if (charCodeAfterSlash === 42 /*"*"*/) { - start = code.indexOf('*/', start) + 2; - if (start > searchPos) { - searchPos = code.indexOf(searchString, start); - } + + // With our assumption, '/' always starts a comment. Determine comment type: + start = + charCodeAfterSlash === 47 /*"/"*/ + ? code.indexOf('\n', start) + 1 + : code.indexOf('*/', start) + 2; + if (start > searchPos) { + searchPos = code.indexOf(searchString, start); } } }