diff --git a/devtools/client/debugger/dist/pretty-print-worker.js b/devtools/client/debugger/dist/pretty-print-worker.js index 2802b9c848f0..6fdbb82e5495 100644 --- a/devtools/client/debugger/dist/pretty-print-worker.js +++ b/devtools/client/debugger/dist/pretty-print-worker.js @@ -9427,6 +9427,10 @@ #currentCode = ""; #currentLine = 1; #currentColumn = 0; + // The tokens parsed by acorn. + #tokenQueue; + // The index of the current token in this.#tokenQueue. + #currentTokenIndex; // The last token we added to the pretty printed code. #lastToken; // Stack of token types/keywords that can affect whether we want to add a @@ -9440,6 +9444,7 @@ // - "{" // - "{\n" // - "(" + // - "(\n" // - "[" // - "[\n" // - "do" @@ -9448,11 +9453,12 @@ // - "case" // - "default" // - // The difference between "[" and "[\n" (as well as "{" and "{\n") is that "\n" is used - // when we are treating (curly) brackets as line delimiters and should increment and - // decrement the indent level when we find them. + // The difference between "[" and "[\n" (as well as "{" and "{\n", and "(" and "(\n") + // is that "\n" is used when we are treating (curly) brackets/parens as line delimiters + // and should increment and decrement the indent level when we find them. // "[" can represent either a property access (e.g. `x["hi"]`), or an empty array literal // "{" only represents an empty object literals + // "(" can represent lots of different things (wrapping expression, if/loop condition, function call, …) #stack = []; /** @@ -9479,11 +9485,12 @@ // After this process, tokenQueue has the following token stream: // // [ foo, '// a', '// b', bar] - const tokenQueue = this.#getTokens(input); + this.#tokenQueue = this.#getTokens(input); - for (let i = 0, len = tokenQueue.length; i < len; i++) { - const token = tokenQueue[i]; - const nextToken = tokenQueue[i + 1]; + for (let i = 0, len = this.#tokenQueue.length; i < len; i++) { + this.#currentTokenIndex = i; + const token = this.#tokenQueue[i]; + const nextToken = this.#tokenQueue[i + 1]; this.#handleToken(token, nextToken); // Acorn's tokenizer re-uses tokens, so we have to copy the last token on @@ -9691,6 +9698,14 @@ } else if (isObjectLiteral(token, this.#lastToken)) { // Don't add new lines for empty object literals stackEntry = nextToken?.type?.label === "}" ? "{" : "{\n"; + } else if ( + isRoundBracketStartingLongParenthesis( + token, + this.#tokenQueue, + this.#currentTokenIndex + ) + ) { + stackEntry = "(\n"; } else if (ttl == "{") { // We need to add a line break for "{" which are not empty object literals stackEntry = "{\n"; @@ -9748,7 +9763,8 @@ (token.type.label == "{" && this.#stack.at(-1) === "{\n") || // Don't increment indent for empty array literals (token.type.label == "[" && this.#stack.at(-1) === "[\n") || - token.type.keyword == "switch" + token.type.keyword == "switch" || + (token.type.label == "(" && this.#stack.at(-1) === "(\n") ) { this.#indentLevel++; } @@ -9757,7 +9773,11 @@ #shouldDecrementIndent(token) { const top = this.#stack.at(-1); const ttl = token.type.label; - return (ttl == "}" && top == "{\n") || (ttl == "]" && top == "[\n"); + return ( + (ttl == "}" && top == "{\n") || + (ttl == "]" && top == "[\n") || + (ttl == ")" && top == "(\n") + ); } #maybeDecrementIndent(token) { @@ -9952,6 +9972,75 @@ ); } + /** + * Determines if we think that the given token starts a long parenthesis + * + * @param {Object} token + * The token we want to determine if it is the beginning of a long paren. + * @param {Array} tokenQueue + * The whole list of tokens parsed by acorn + * @param {Integer} currentTokenIndex + * The index of `token` in `tokenQueue` + * @returns + */ + function isRoundBracketStartingLongParenthesis( + token, + tokenQueue, + currentTokenIndex + ) { + if (token.type.label !== "(") { + return false; + } + + // If we're just wrapping an object, we'll have a new line right after + if (tokenQueue[currentTokenIndex + 1].type.label == "{") { + return false; + } + + // We're going to iterate through the following tokens until : + // - we find the closing parent + // - or we reached the maximum character we think should be in parenthesis + const longParentContentLength = 60; + + // Keep track of other parens so we know when we get the closing one for `token` + let parenCount = 0; + let parenContentLength = 0; + for (let i = currentTokenIndex + 1, len = tokenQueue.length; i < len; i++) { + const currToken = tokenQueue[i]; + const ttl = currToken.type.label; + + if (ttl == "(") { + parenCount++; + } else if (ttl == ")") { + if (parenCount == 0) { + // Matching closing paren, if we got here, we didn't reach the length limit, + // as we return when parenContentLength is greater than the limit. + return false; + } + parenCount--; + } + + // Aside block comments, all tokens start and end location are on the same line, so + // we can use `start` and `end` to deduce the token length. + const tokenLength = currToken.comment + ? currToken.text.length + : currToken.end - currToken.start; + parenContentLength += tokenLength; + + // If we didn't find the matching closing paren yet and the characters from the + // tokens we evaluated so far are longer than the limit, so consider the token + // a long paren. + if (parenContentLength > longParentContentLength) { + return true; + } + } + + // if we get to here, we didn't found a closing paren, which shouldn't happen + // (scripts with syntax error are not displayed in the debugger), but just to + // be safe, return false. + return false; + } + // If any of these tokens are followed by a token on a new line, we know that // ASI cannot happen. const PREVENT_ASI_AFTER_TOKENS = new Set([ @@ -10122,8 +10211,9 @@ (ttl == "{" && top == "{\n") || // Don't add a new line for empty array literals (ttl == "[" && top == "[\n") || - (ttl == "," && top != "(") || - (ttl == ":" && (top == "case" || top == "default")) + ((ttl == "," || ttl == "||" || ttl == "&&") && top != "(") || + (ttl == ":" && (top == "case" || top == "default")) || + (ttl == "(" && top == "(\n") ); } diff --git a/devtools/client/debugger/src/workers/pretty-print/pretty-fast.js b/devtools/client/debugger/src/workers/pretty-print/pretty-fast.js index ab4cbb34d961..a161af03a710 100644 --- a/devtools/client/debugger/src/workers/pretty-print/pretty-fast.js +++ b/devtools/client/debugger/src/workers/pretty-print/pretty-fast.js @@ -165,6 +165,10 @@ class PrettyFast { #currentCode = ""; #currentLine = 1; #currentColumn = 0; + // The tokens parsed by acorn. + #tokenQueue; + // The index of the current token in this.#tokenQueue. + #currentTokenIndex; // The last token we added to the pretty printed code. #lastToken; // Stack of token types/keywords that can affect whether we want to add a @@ -178,6 +182,7 @@ class PrettyFast { // - "{" // - "{\n" // - "(" + // - "(\n" // - "[" // - "[\n" // - "do" @@ -186,11 +191,12 @@ class PrettyFast { // - "case" // - "default" // - // The difference between "[" and "[\n" (as well as "{" and "{\n") is that "\n" is used - // when we are treating (curly) brackets as line delimiters and should increment and - // decrement the indent level when we find them. + // The difference between "[" and "[\n" (as well as "{" and "{\n", and "(" and "(\n") + // is that "\n" is used when we are treating (curly) brackets/parens as line delimiters + // and should increment and decrement the indent level when we find them. // "[" can represent either a property access (e.g. `x["hi"]`), or an empty array literal // "{" only represents an empty object literals + // "(" can represent lots of different things (wrapping expression, if/loop condition, function call, …) #stack = []; /** @@ -217,11 +223,12 @@ class PrettyFast { // After this process, tokenQueue has the following token stream: // // [ foo, '// a', '// b', bar] - const tokenQueue = this.#getTokens(input); + this.#tokenQueue = this.#getTokens(input); - for (let i = 0, len = tokenQueue.length; i < len; i++) { - const token = tokenQueue[i]; - const nextToken = tokenQueue[i + 1]; + for (let i = 0, len = this.#tokenQueue.length; i < len; i++) { + this.#currentTokenIndex = i; + const token = this.#tokenQueue[i]; + const nextToken = this.#tokenQueue[i + 1]; this.#handleToken(token, nextToken); // Acorn's tokenizer re-uses tokens, so we have to copy the last token on @@ -429,6 +436,14 @@ class PrettyFast { } else if (isObjectLiteral(token, this.#lastToken)) { // Don't add new lines for empty object literals stackEntry = nextToken?.type?.label === "}" ? "{" : "{\n"; + } else if ( + isRoundBracketStartingLongParenthesis( + token, + this.#tokenQueue, + this.#currentTokenIndex + ) + ) { + stackEntry = "(\n"; } else if (ttl == "{") { // We need to add a line break for "{" which are not empty object literals stackEntry = "{\n"; @@ -486,7 +501,8 @@ class PrettyFast { (token.type.label == "{" && this.#stack.at(-1) === "{\n") || // Don't increment indent for empty array literals (token.type.label == "[" && this.#stack.at(-1) === "[\n") || - token.type.keyword == "switch" + token.type.keyword == "switch" || + (token.type.label == "(" && this.#stack.at(-1) === "(\n") ) { this.#indentLevel++; } @@ -495,7 +511,11 @@ class PrettyFast { #shouldDecrementIndent(token) { const top = this.#stack.at(-1); const ttl = token.type.label; - return (ttl == "}" && top == "{\n") || (ttl == "]" && top == "[\n"); + return ( + (ttl == "}" && top == "{\n") || + (ttl == "]" && top == "[\n") || + (ttl == ")" && top == "(\n") + ); } #maybeDecrementIndent(token) { @@ -690,6 +710,75 @@ function isObjectLiteral(token, lastToken) { ); } +/** + * Determines if we think that the given token starts a long parenthesis + * + * @param {Object} token + * The token we want to determine if it is the beginning of a long paren. + * @param {Array} tokenQueue + * The whole list of tokens parsed by acorn + * @param {Integer} currentTokenIndex + * The index of `token` in `tokenQueue` + * @returns + */ +function isRoundBracketStartingLongParenthesis( + token, + tokenQueue, + currentTokenIndex +) { + if (token.type.label !== "(") { + return false; + } + + // If we're just wrapping an object, we'll have a new line right after + if (tokenQueue[currentTokenIndex + 1].type.label == "{") { + return false; + } + + // We're going to iterate through the following tokens until : + // - we find the closing parent + // - or we reached the maximum character we think should be in parenthesis + const longParentContentLength = 60; + + // Keep track of other parens so we know when we get the closing one for `token` + let parenCount = 0; + let parenContentLength = 0; + for (let i = currentTokenIndex + 1, len = tokenQueue.length; i < len; i++) { + const currToken = tokenQueue[i]; + const ttl = currToken.type.label; + + if (ttl == "(") { + parenCount++; + } else if (ttl == ")") { + if (parenCount == 0) { + // Matching closing paren, if we got here, we didn't reach the length limit, + // as we return when parenContentLength is greater than the limit. + return false; + } + parenCount--; + } + + // Aside block comments, all tokens start and end location are on the same line, so + // we can use `start` and `end` to deduce the token length. + const tokenLength = currToken.comment + ? currToken.text.length + : currToken.end - currToken.start; + parenContentLength += tokenLength; + + // If we didn't find the matching closing paren yet and the characters from the + // tokens we evaluated so far are longer than the limit, so consider the token + // a long paren. + if (parenContentLength > longParentContentLength) { + return true; + } + } + + // if we get to here, we didn't found a closing paren, which shouldn't happen + // (scripts with syntax error are not displayed in the debugger), but just to + // be safe, return false. + return false; +} + // If any of these tokens are followed by a token on a new line, we know that // ASI cannot happen. const PREVENT_ASI_AFTER_TOKENS = new Set([ @@ -860,8 +949,9 @@ function isLineDelimiter(token, stack) { (ttl == "{" && top == "{\n") || // Don't add a new line for empty array literals (ttl == "[" && top == "[\n") || - (ttl == "," && top != "(") || - (ttl == ":" && (top == "case" || top == "default")) + ((ttl == "," || ttl == "||" || ttl == "&&") && top != "(") || + (ttl == ":" && (top == "case" || top == "default")) || + (ttl == "(" && top == "(\n") ); } diff --git a/devtools/client/debugger/src/workers/pretty-print/tests/__snapshots__/prettyFast.spec.js.snap b/devtools/client/debugger/src/workers/pretty-print/tests/__snapshots__/prettyFast.spec.js.snap index 2bf8fbf44bfd..2d8cf7dffd2c 100644 --- a/devtools/client/debugger/src/workers/pretty-print/tests/__snapshots__/prettyFast.spec.js.snap +++ b/devtools/client/debugger/src/workers/pretty-print/tests/__snapshots__/prettyFast.spec.js.snap @@ -1063,6 +1063,106 @@ Array [ ] `; +exports[`Long parenthesis 1`] = ` +"if ( + thisIsAVeryLongVariable && + thisIsAnotherOne || + yetAnotherVeryLongVariable +) { + ( + thisIsAnotherOne = thisMayReturnNull() || + 'hi', + thisIsAVeryLongVariable = 42, + yetAnotherVeryLongVariable && + doSomething( + true /* do it well */ , + thisIsAVeryLongVariable, + thisIsAnotherOne, + yetAnotherVeryLongVariable + ) + ) +} +for ( + let thisIsAnotherVeryLongVariable = 0; + i < thisIsAnotherVeryLongVariable.length; + thisIsAnotherVeryLongVariable++ +) { +} +const x = ({ + thisIsAnotherVeryLongPropertyName: 'but should not cause the paren to be a line delimiter' +}) +" +`; + +exports[`Long parenthesis 2`] = ` +Array [ + "(1, 0) -> (2, 6)", + "(1, 3) -> (2, 9)", + "(2, 2) -> (2, 10)", + "(2, 26) -> (2, 34)", + "(3, 2) -> (2, 37)", + "(3, 19) -> (2, 54)", + "(4, 2) -> (2, 57)", + "(5, 0) -> (2, 83)", + "(5, 2) -> (2, 85)", + "(6, 2) -> (3, 8)", + "(7, 4) -> (3, 9)", + "(7, 21) -> (3, 26)", + "(7, 23) -> (3, 28)", + "(7, 40) -> (3, 45)", + "(7, 41) -> (3, 46)", + "(7, 43) -> (3, 48)", + "(8, 4) -> (3, 51)", + "(8, 8) -> (3, 55)", + "(9, 4) -> (3, 57)", + "(9, 28) -> (3, 81)", + "(9, 30) -> (3, 83)", + "(9, 32) -> (3, 85)", + "(10, 4) -> (3, 87)", + "(10, 31) -> (3, 114)", + "(11, 4) -> (3, 117)", + "(11, 15) -> (3, 128)", + "(12, 6) -> (3, 129)", + "(12, 28) -> (3, 150)", + "(13, 6) -> (3, 151)", + "(13, 29) -> (3, 174)", + "(14, 6) -> (3, 176)", + "(14, 22) -> (3, 192)", + "(15, 6) -> (3, 194)", + "(16, 4) -> (3, 220)", + "(17, 2) -> (3, 221)", + "(18, 0) -> (4, 6)", + "(19, 0) -> (5, 6)", + "(19, 4) -> (5, 10)", + "(20, 2) -> (5, 11)", + "(20, 6) -> (5, 15)", + "(20, 36) -> (5, 45)", + "(20, 38) -> (5, 47)", + "(20, 39) -> (5, 48)", + "(21, 2) -> (5, 50)", + "(21, 4) -> (5, 52)", + "(21, 6) -> (5, 54)", + "(21, 35) -> (5, 83)", + "(21, 36) -> (5, 84)", + "(21, 42) -> (5, 90)", + "(22, 2) -> (5, 92)", + "(22, 31) -> (5, 121)", + "(23, 0) -> (5, 123)", + "(23, 2) -> (5, 125)", + "(24, 0) -> (5, 126)", + "(25, 0) -> (6, 6)", + "(25, 6) -> (6, 12)", + "(25, 8) -> (6, 14)", + "(25, 10) -> (6, 16)", + "(25, 11) -> (6, 17)", + "(26, 2) -> (6, 18)", + "(26, 35) -> (6, 51)", + "(26, 37) -> (6, 53)", + "(27, 0) -> (6, 108)", + "(27, 1) -> (6, 109)", +] +`; + exports[`Multi line comment 1`] = ` "/* Comment more comment */ diff --git a/devtools/client/debugger/src/workers/pretty-print/tests/prettyFast.spec.js b/devtools/client/debugger/src/workers/pretty-print/tests/prettyFast.spec.js index 202f75ae2676..ba33e8a5f5c7 100644 --- a/devtools/client/debugger/src/workers/pretty-print/tests/prettyFast.spec.js +++ b/devtools/client/debugger/src/workers/pretty-print/tests/prettyFast.spec.js @@ -390,6 +390,16 @@ const cases = [ } `, }, + { + name: "Long parenthesis", + input: ` + if (thisIsAVeryLongVariable && thisIsAnotherOne || yetAnotherVeryLongVariable) { + (thisIsAnotherOne = thisMayReturnNull() || "hi", thisIsAVeryLongVariable = 42, yetAnotherVeryLongVariable && doSomething(true /* do it well */,thisIsAVeryLongVariable, thisIsAnotherOne, yetAnotherVeryLongVariable)) + } + for (let thisIsAnotherVeryLongVariable = 0; i < thisIsAnotherVeryLongVariable.length; thisIsAnotherVeryLongVariable++) {} + const x = ({thisIsAnotherVeryLongPropertyName: "but should not cause the paren to be a line delimiter"}) + `, + }, ]; const includesOnly = cases.find(({ only }) => only); diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-features-breakable-positions.js b/devtools/client/debugger/test/mochitest/browser_dbg-features-breakable-positions.js index 10a187abb65a..a60c7c26ba89 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-features-breakable-positions.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-features-breakable-positions.js @@ -42,12 +42,12 @@ add_task(async function testBreakableLinesOverReloads() { info("Pretty print first html page load and assert breakable lines"); await prettyPrint(dbg); - await assertBreakablePositions(dbg, "index.html:formatted", 80, [ + await assertBreakablePositions(dbg, "index.html:formatted", 84, [ { line: 16, columns: [0, 8] }, { line: 22, columns: [0, 8, 35] }, { line: 27, columns: [0, 8] }, { line: 28, columns: [] }, - { line: 34, columns: [] }, + { line: 36, columns: [] }, ]); await closeTab(dbg, "index.html:formatted"); diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-console.js b/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-console.js index dcf3f0f322b2..a7f1d9b22305 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-console.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-console.js @@ -34,7 +34,7 @@ add_task(async function() { const formattedLink = await waitForConsoleLink( dbg, "arithmetic", - "math.min.js:formatted:22:18" + "math.min.js:formatted:31:24" ); ok(true, "Message location was updated as expected"); @@ -43,7 +43,7 @@ add_task(async function() { ); formattedLink.click(); await selectSource(dbg, "math.min.js:formatted"); - await waitForSelectedLocation(dbg, 22); + await waitForSelectedLocation(dbg, 31); }); async function waitForConsoleLink(dbg, messageText, linkText) { diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-inline-scripts.js b/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-inline-scripts.js index 21555cdf21cb..bb45afeb4dbc 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-inline-scripts.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-inline-scripts.js @@ -92,7 +92,7 @@ add_task(async function() { const secondMessageLink = secondMessage.querySelector(".frame-link-source"); is( secondMessageLink.innerText, - `${PRETTY_PRINTED_FILENAME}:38:12`, + `${PRETTY_PRINTED_FILENAME}:41:12`, "second console message has expected location" ); info( @@ -101,7 +101,7 @@ add_task(async function() { secondMessageLink.click(); await waitForSelectedSource(dbg, PRETTY_PRINTED_FILENAME); ok(true, "pretty printed file was selected in debugger…"); - await waitForSelectedLocation(dbg, 38); + await waitForSelectedLocation(dbg, 41); ok(true, "…at the expected location"); info("Check that event listener popup is pointing to pretty-printed file"); @@ -132,7 +132,7 @@ add_task(async function() { const headerFilename = header.querySelector(".event-tooltip-filename") .innerText; ok( - headerFilename.endsWith(`${PRETTY_PRINTED_FILENAME}:48`), + headerFilename.endsWith(`${PRETTY_PRINTED_FILENAME}:51`), `Location in event tooltip is the pretty printed one (${headerFilename})` ); info( @@ -141,7 +141,7 @@ add_task(async function() { header.querySelector(".event-tooltip-debugger-icon").click(); await waitForSelectedSource(dbg, PRETTY_PRINTED_FILENAME); ok(true, "pretty printed file was selected in debugger…"); - await waitForSelectedLocation(dbg, 48); + await waitForSelectedLocation(dbg, 51); ok(true, "…at the expected location"); }); @@ -199,12 +199,15 @@ function getExpectedPrettyPrintedHtml() { diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-paused-anonymous.js b/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-paused-anonymous.js index 13f282c7d20d..7363bdd19651 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-paused-anonymous.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-paused-anonymous.js @@ -78,15 +78,18 @@ document.addEventListener('click', e=>{ is( prettyEvalSourceValue.trim(), - `setTimeout(() =>{ - debugger; - document.addEventListener('click', e=>{ + ` +setTimeout( + () =>{ debugger; - }, { - once: true - }) -}, 100) -`.trim(), + document.addEventListener('click', e=>{ + debugger; + }, { + once: true + }) + }, + 100 +)`.trim(), "script was pretty printed as expected" ); await resume(dbg); @@ -96,7 +99,7 @@ document.addEventListener('click', e=>{ content.document.body.click(); }); await waitForPaused(dbg); - await assertPausedAtSourceAndLine(dbg, prettyEvalSource.id, 4); + await assertPausedAtSourceAndLine(dbg, prettyEvalSource.id, 5); await resume(dbg); info("Check that pretty printing works in `new Function` source"); diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print.js b/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print.js index 3385328c8178..1ae5f1384f84 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print.js @@ -31,7 +31,7 @@ add_task(async function() { await stepOver(dbg); - assertPausedAtSourceAndLine(dbg, ppSrc.id, 27); + assertPausedAtSourceAndLine(dbg, ppSrc.id, 39); await resume(dbg); diff --git a/devtools/client/inspector/markup/test/browser_markup_shadowdom_open_debugger_pretty_printed.js b/devtools/client/inspector/markup/test/browser_markup_shadowdom_open_debugger_pretty_printed.js index aef429c16af2..9ae1a8165cf3 100644 --- a/devtools/client/inspector/markup/test/browser_markup_shadowdom_open_debugger_pretty_printed.js +++ b/devtools/client/inspector/markup/test/browser_markup_shadowdom_open_debugger_pretty_printed.js @@ -49,5 +49,5 @@ add_task(async function() { customBadge.click(); await waitForSelectedSource(dbg, "shadowdom_open_debugger.min.js:formatted"); - await waitForSelectedLocation(dbg, 3); + await waitForSelectedLocation(dbg, 5); });