From c6c345bff2cc9e241e0ae07e1a35929219f0daff Mon Sep 17 00:00:00 2001 From: Nicolas Chevobbe Date: Wed, 17 Oct 2018 15:02:41 +0000 Subject: [PATCH] Bug 1498598 - Make js-property-provider better; r=bgrins. This patches solves 2 issues: - it doesn't return any result when the user is trying to perform a variable, function or class declaration (e.g. var d). - js-property-provider used to compute the last statement by only looking for space or ; chars. But there are a lot of cases (basically each time using an operator), where we should return results and we weren't. Test cases are added to cover those fixes. Differential Revision: https://phabricator.services.mozilla.com/D8968 --HG-- extra : moz-landing-system : lando --- .../shared/webconsole/js-property-provider.js | 82 +++++++++++-------- .../test/test_jsterm_autocomplete.html | 72 ++++++++++++++++ 2 files changed, 120 insertions(+), 34 deletions(-) diff --git a/devtools/shared/webconsole/js-property-provider.js b/devtools/shared/webconsole/js-property-provider.js index ddcf971903d2..70c6d2809b7c 100644 --- a/devtools/shared/webconsole/js-property-provider.js +++ b/devtools/shared/webconsole/js-property-provider.js @@ -32,6 +32,8 @@ const OPEN_CLOSE_BODY = { "(": ")", }; +const NO_AUTOCOMPLETE_PREFIXES = ["var", "const", "let", "function", "class"]; + function hasArrayIndex(str) { return /\[\d+\]$/.test(str); } @@ -66,6 +68,24 @@ function analyzeInputString(str) { // Use an array in order to handle character with a length > 2 (e.g. 😎). const characters = Array.from(str); + + const buildReturnObject = () => { + let isElementAccess = false; + if (bodyStack.length === 1 && bodyStack[0].token === "[") { + start = bodyStack[0].start; + isElementAccess = true; + if ([STATE_DQUOTE, STATE_QUOTE, STATE_TEMPLATE_LITERAL].includes(state)) { + state = STATE_NORMAL; + } + } + + return { + state, + lastStatement: characters.slice(start).join(""), + isElementAccess, + }; + }; + for (let i = 0; i < characters.length; i++) { c = characters[i]; @@ -78,9 +98,11 @@ function analyzeInputString(str) { state = STATE_QUOTE; } else if (c == "`") { state = STATE_TEMPLATE_LITERAL; - } else if (c == ";") { + } else if (";,:=<>+-*/%|&^~?!".split("").includes(c)) { + // If the character is an operator, we need to update the start position. start = i + 1; } else if (c == " ") { + const currentLastStatement = characters.slice(start, i).join(""); const before = characters.slice(0, i); const after = characters.slice(i + 1); const trimmedBefore = Array.from(before.join("").trimRight()); @@ -90,24 +112,23 @@ function analyzeInputString(str) { const nextNonSpaceCharIndex = after.indexOf(nextNonSpaceChar); const previousNonSpaceChar = trimmedBefore[trimmedBefore.length - 1]; - // If the previous meaningful char was a dot and there is no meaningful char - // after, we can break out of the loop. - if (previousNonSpaceChar === "." && !nextNonSpaceChar) { - break; + // There's only spaces after that, so we can return. + if (!nextNonSpaceChar) { + return buildReturnObject(); } - if (nextNonSpaceChar) { - // If the previous char wasn't a dot, and the next one isn't a dot either, - // update the start pos. - if (previousNonSpaceChar !== "." && nextNonSpaceChar !== ".") { - start = i + nextNonSpaceCharIndex; - } - // Let's jump to handle the next non-space char. - i = i + nextNonSpaceCharIndex; - } else { - // There's only spaces after that, so we can break out of the loop. - break; + // If the previous char in't a dot, and the next one isn't a dot either, + // and the current computed statement is not a variable/function/class + // declaration, update the start position. + if ( + previousNonSpaceChar !== "." && nextNonSpaceChar !== "." + && !NO_AUTOCOMPLETE_PREFIXES.includes(currentLastStatement) + ) { + start = i + nextNonSpaceCharIndex; } + + // Let's jump to handle the next non-space char. + i = i + nextNonSpaceCharIndex; } else if (OPEN_BODY.includes(c)) { bodyStack.push({ token: c, @@ -166,20 +187,7 @@ function analyzeInputString(str) { } } - let isElementAccess = false; - if (bodyStack.length === 1 && bodyStack[0].token === "[") { - start = bodyStack[0].start; - isElementAccess = true; - if ([STATE_DQUOTE, STATE_QUOTE, STATE_TEMPLATE_LITERAL].includes(state)) { - state = STATE_NORMAL; - } - } - - return { - state, - lastStatement: characters.slice(start).join(""), - isElementAccess, - }; + return buildReturnObject(); } /** @@ -237,16 +245,22 @@ function JSPropertyProvider(dbgObject, anEnvironment, inputValue, cursor) { if (state != STATE_NORMAL) { return null; } + + // Don't complete on just an empty string. + if (lastStatement.trim() == "") { + return null; + } + + if (NO_AUTOCOMPLETE_PREFIXES.some(prefix => lastStatement.startsWith(prefix + " "))) { + return null; + } + const completionPart = lastStatement; const lastDotIndex = completionPart.lastIndexOf("."); const lastOpeningBracketIndex = isElementAccess ? completionPart.lastIndexOf("[") : -1; const lastCompletionCharIndex = Math.max(lastDotIndex, lastOpeningBracketIndex); const startQuoteRegex = /^('|"|`)/; - // Don't complete on just an empty string. - if (completionPart.trim() == "") { - return null; - } // Catch literals like [1,2,3] or "foo" and return the matches from // their prototypes. // Don't run this is a worker, migrating to acorn should allow this diff --git a/devtools/shared/webconsole/test/test_jsterm_autocomplete.html b/devtools/shared/webconsole/test/test_jsterm_autocomplete.html index 3aa410675694..d0872faac3fa 100644 --- a/devtools/shared/webconsole/test/test_jsterm_autocomplete.html +++ b/devtools/shared/webconsole/test/test_jsterm_autocomplete.html @@ -85,6 +85,8 @@ 'da\`ta\`test': "", "da'ta'test": "", })); + + window.varify = true; `; await state.client.evaluateJSAsync(script); @@ -101,6 +103,8 @@ doAutocompleteAfterOr, doInsensitiveAutocomplete, doElementAccessAutocomplete, + doAutocompleteAfterOperator, + dontAutocompleteAfterDeclaration, ]; if (!isWorker) { @@ -448,6 +452,74 @@ matches = (await client.autocomplete(`window[;c`)).matches; ok(!matches.includes("cd") && !matches.includes("clear"), "commands are not returned"); } + + async function doAutocompleteAfterOperator(client) { + const inputs = [ + "true;foob", + "true,foob", + "({key:foob", + "a=foob", + "if(afoob", + "1+foob", + "1-foob", + "++foob", + "--foob", + "1*foob", + "2**foob", + "1/foob", + "1%foob", + "1|foob", + "1&foob", + "1^foob", + "~foob", + "1<>foob", + "1>>>foob", + "false||foob", + "false&&foob", + "x=true?foob", + "x=false?1:foob", + "!foob", + ]; + + for (const input of inputs) { + info(`test autocomplete for "${input}"`); + let matches = (await client.autocomplete(input)).matches; + ok(matches.includes("foobarObject"), `Expected autocomplete result for ${input}"`); + } + } + + async function dontAutocompleteAfterDeclaration(client) { + info("test autocomplete for 'var win'"); + let matches = (await client.autocomplete("var win")).matches; + is(matches.length, 0, "no autocompletion on a var declaration"); + + info("test autocomplete for 'const win'"); + matches = (await client.autocomplete("const win")).matches; + is(matches.length, 0, "no autocompletion on a const declaration"); + + info("test autocomplete for 'let win'"); + matches = (await client.autocomplete("let win")).matches; + is(matches.length, 0, "no autocompletion on a let declaration"); + + info("test autocomplete for 'function win'"); + matches = (await client.autocomplete("function win")).matches; + is(matches.length, 0, "no autocompletion on a function declaration"); + + info("test autocomplete for 'class win'"); + matches = (await client.autocomplete("class win")).matches; + is(matches.length, 0, "no autocompletion on a class declaration"); + + info("test autocomplete for 'const win = win'"); + matches = (await client.autocomplete("const win = win")).matches; + ok(matches.includes("window"), "autocompletion still happens after the `=` sign"); + + info("test autocomplete for 'in var'"); + matches = (await client.autocomplete("in var")).matches; + ok(matches.includes("varify"), + "autocompletion still happens with a property name starting with 'var'"); + }