From c1dd141140c5787ce7f6040bf4ae26e92acc6dbf Mon Sep 17 00:00:00 2001 From: Nicolas Chevobbe Date: Wed, 18 Dec 2019 11:29:06 +0000 Subject: [PATCH] Bug 1604411 - Add expression variables in autocomplete popup. r=Honza. Variables are retrieved from CodeMirror state and sent to the webconsole actor, where the filtering is done. Differential Revision: https://phabricator.services.mozilla.com/D57427 --- .../client/webconsole/actions/autocomplete.js | 9 +++- .../webconsole/components/Input/JSTerm.js | 46 ++++++++++++++-- .../webconsole/test/browser/browser.ini | 1 + ...sterm_autocomplete_expression_variables.js | 53 +++++++++++++++++++ ...owser_jsterm_autocomplete_race_on_enter.js | 2 +- devtools/server/actors/webconsole.js | 4 +- devtools/shared/specs/webconsole.js | 1 + .../shared/webconsole/js-property-provider.js | 15 +++++- .../test/unit/test_js_property_provider.js | 21 ++++++++ 9 files changed, 142 insertions(+), 10 deletions(-) create mode 100644 devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_expression_variables.js diff --git a/devtools/client/webconsole/actions/autocomplete.js b/devtools/client/webconsole/actions/autocomplete.js index d10e44b2ac4a2..62c11667d47cd 100644 --- a/devtools/client/webconsole/actions/autocomplete.js +++ b/devtools/client/webconsole/actions/autocomplete.js @@ -18,8 +18,9 @@ const { * from the cache). * @param {Array} getterPath: Array representing the getter access (i.e. * `a.b.c.d.` is described as ['a', 'b', 'c', 'd'] ). + * @param {Array} expressionVars: Array of the variables defined in the expression. */ -function autocompleteUpdate(force, getterPath) { +function autocompleteUpdate(force, getterPath, expressionVars) { return async ({ dispatch, getState, webConsoleUI, hud }) => { if (hud.inputHasSelection()) { return dispatch(autocompleteClear()); @@ -82,6 +83,7 @@ function autocompleteUpdate(force, getterPath) { webConsoleFront, authorizedEvaluations, force, + expressionVars, }) ); }; @@ -133,6 +135,7 @@ function autocompleteDataFetch({ force, webConsoleFront, authorizedEvaluations, + expressionVars, }) { return ({ dispatch, webConsoleUI }) => { const selectedNodeActor = webConsoleUI.getSelectedNodeActor(); @@ -144,7 +147,8 @@ function autocompleteDataFetch({ undefined, frameActorId, selectedNodeActor, - authorizedEvaluations + authorizedEvaluations, + expressionVars ) .then(data => { dispatch( @@ -155,6 +159,7 @@ function autocompleteDataFetch({ frameActorId, data, authorizedEvaluations, + expressionVars, }) ); }) diff --git a/devtools/client/webconsole/components/Input/JSTerm.js b/devtools/client/webconsole/components/Input/JSTerm.js index 63e0d5ea977cd..042cd703c21e0 100644 --- a/devtools/client/webconsole/components/Input/JSTerm.js +++ b/devtools/client/webconsole/components/Input/JSTerm.js @@ -241,7 +241,10 @@ class JSTerm extends Component { autoCloseBrackets: false, lineNumbers: this.props.editorMode, lineWrapping: true, - mode: Editor.modes.js, + mode: { + name: "javascript", + globalVars: true, + }, styleActiveLine: false, tabIndex: "0", viewportMargin: Infinity, @@ -464,7 +467,11 @@ class JSTerm extends Component { "Ctrl-Space": () => { if (!this.autocompletePopup.isOpen) { - this.props.autocompleteUpdate(true); + this.props.autocompleteUpdate( + true, + null, + this._getExpressionVariables() + ); return null; } @@ -770,6 +777,35 @@ class JSTerm extends Component { } } + /** + * Retrieve variable declared in the expression from the CodeMirror state, in order + * to display them in the autocomplete popup. + */ + _getExpressionVariables() { + const cm = this.editor.codeMirror; + const { state } = cm.getTokenAt(cm.getCursor()); + const variables = []; + + if (state.context) { + for (let c = state.context; c; c = c.prev) { + for (let v = c.vars; v; v = v.next) { + variables.push(v.name); + } + } + } + + const keys = ["localVars", "globalVars"]; + for (const key of keys) { + if (state[key]) { + for (let v = state[key]; v; v = v.next) { + variables.push(v.name); + } + } + } + + return variables; + } + /** * The editor "changes" event handler. */ @@ -787,7 +823,7 @@ class JSTerm extends Component { !isJsTermChangeOnly && (this.props.autocomplete || this.hasAutocompletionSuggestion()) ) { - this.autocompleteUpdate(); + this.autocompleteUpdate(false, null, this._getExpressionVariables()); } this.lastInputValue = value; this.terminalInputChanged(value); @@ -1281,8 +1317,8 @@ function mapDispatchToProps(dispatch) { return { updateHistoryPosition: (direction, expression) => dispatch(actions.updateHistoryPosition(direction, expression)), - autocompleteUpdate: (force, getterPath) => - dispatch(actions.autocompleteUpdate(force, getterPath)), + autocompleteUpdate: (force, getterPath, expressionVars) => + dispatch(actions.autocompleteUpdate(force, getterPath, expressionVars)), autocompleteClear: () => dispatch(actions.autocompleteClear()), evaluateExpression: expression => dispatch(actions.evaluateExpression(expression)), diff --git a/devtools/client/webconsole/test/browser/browser.ini b/devtools/client/webconsole/test/browser/browser.ini index 7d33adb09b024..4740716fbae61 100644 --- a/devtools/client/webconsole/test/browser/browser.ini +++ b/devtools/client/webconsole/test/browser/browser.ini @@ -211,6 +211,7 @@ skip-if = verify [browser_jsterm_autocomplete_crossdomain_iframe.js] [browser_jsterm_autocomplete_disabled.js] [browser_jsterm_autocomplete_escape_key.js] +[browser_jsterm_autocomplete_expression_variables.js] [browser_jsterm_autocomplete_extraneous_closing_brackets.js] [browser_jsterm_autocomplete_getters_cache.js] [browser_jsterm_autocomplete_getters_cancel.js] diff --git a/devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_expression_variables.js b/devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_expression_variables.js new file mode 100644 index 0000000000000..2c5bac03a504b --- /dev/null +++ b/devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_expression_variables.js @@ -0,0 +1,53 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Tests that variable created in the expression are displayed in the autocomplete. + +"use strict"; + +const TEST_URI = `data:text/html;charset=utf8,Test autocompletion for expression variables`; + +add_task(async function() { + const hud = await openNewTabAndConsole(TEST_URI); + const { jsterm } = hud; + const { autocompletePopup } = jsterm; + + await setInputValueForAutocompletion( + hud, + ` + var testVar; + let testLet; + const testConst; + class testClass {} + function testFunc(testParam1, testParam2, ...testParamRest) { + var [testParamRestFirst] = testParamRest; + let {testDeconstruct1,testDeconstruct2, ...testDeconstructRest} = testParam1; + test` + ); + + is( + getPopupLabels(autocompletePopup).join("\n"), + [ + "testClass", + "testConst", + "testDeconstruct1", + "testDeconstruct2", + "testDeconstructRest", + "testFunc", + "testGlobal", + "testLet", + "testParam1", + "testParam2", + "testParamRest", + "testParamRestFirst", + "testVar", + ].join("\n"), + "Autocomplete popup displays both global and local variables" + ); +}); + +function getPopupLabels(popup) { + return popup.getItems().map(item => item.label); +} diff --git a/devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_race_on_enter.js b/devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_race_on_enter.js index 5fb82bd17b13a..c26800307d930 100644 --- a/devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_race_on_enter.js +++ b/devtools/client/webconsole/test/browser/browser_jsterm_autocomplete_race_on_enter.js @@ -119,7 +119,7 @@ add_task(async function() { "Hitting Enter quickly after a letter that should close the popup evaluates the expression" ); onPopupOpened = autocompletePopup.once("popup-opened"); - await setInputValueForAutocompletion(hud, "var docx = 1; doc"); + await setInputValueForAutocompletion(hud, "doc"); await onPopupOpened; checkInputCompletionValue(hud, "ument", "completeNode has expected value"); diff --git a/devtools/server/actors/webconsole.js b/devtools/server/actors/webconsole.js index ba42e57e09bff..02d96debd1df0 100644 --- a/devtools/server/actors/webconsole.js +++ b/devtools/server/actors/webconsole.js @@ -1346,7 +1346,8 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, { cursor, frameActorId, selectedNodeActor, - authorizedEvaluations + authorizedEvaluations, + expressionVars = [] ) { let dbgObject = null; let environment = null; @@ -1393,6 +1394,7 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, { webconsoleActor: this, selectedNodeActor, authorizedEvaluations, + expressionVars, }); if (result === null) { diff --git a/devtools/shared/specs/webconsole.js b/devtools/shared/specs/webconsole.js index aead280d2dc74..b655acacc2dfb 100644 --- a/devtools/shared/specs/webconsole.js +++ b/devtools/shared/specs/webconsole.js @@ -179,6 +179,7 @@ const webconsoleSpecPrototype = { frameActor: Arg(2, "nullable:string"), selectedNodeActor: Arg(3, "nullable:string"), authorizedEvaluations: Arg(4, "nullable:json"), + expressionVars: Arg(5, "nullable:json"), }, response: RetVal("console.autocomplete"), }, diff --git a/devtools/shared/webconsole/js-property-provider.js b/devtools/shared/webconsole/js-property-provider.js index ec0451d144783..09d0604165db2 100644 --- a/devtools/shared/webconsole/js-property-provider.js +++ b/devtools/shared/webconsole/js-property-provider.js @@ -57,6 +57,9 @@ const MAX_AUTOCOMPLETIONS = (exports.MAX_AUTOCOMPLETIONS = 1500); * evaluation result or create a debuggee value. * - {String}: selectedNodeActor * The actor id of the selected node in the inspector. + * - {Array}: expressionVars + * Optional array containing variable defined in the expression. Those variables + * are extracted from CodeMirror state. * @returns null or object * If the inputValue is an unsafe getter and invokeUnsafeGetter is false, the * following form is returned: @@ -88,6 +91,7 @@ function JSPropertyProvider({ authorizedEvaluations = [], webconsoleActor, selectedNodeActor, + expressionVars = [], }) { if (cursor === undefined) { cursor = inputValue.length; @@ -242,10 +246,19 @@ function JSPropertyProvider({ let obj = dbgObject; if (properties.length === 0) { + const environmentProperties = getMatchedPropsInEnvironment(env, search); + const expressionVariables = new Set( + expressionVars.filter(variableName => variableName.startsWith(matchProp)) + ); + + for (const prop of environmentProperties) { + expressionVariables.add(prop); + } + return { isElementAccess, matchProp, - matches: getMatchedPropsInEnvironment(env, search), + matches: expressionVariables, }; } diff --git a/devtools/shared/webconsole/test/unit/test_js_property_provider.js b/devtools/shared/webconsole/test/unit/test_js_property_provider.js index 48d9badff52d4..988c94a848590 100644 --- a/devtools/shared/webconsole/test/unit/test_js_property_provider.js +++ b/devtools/shared/webconsole/test/unit/test_js_property_provider.js @@ -573,6 +573,27 @@ function runChecks(dbgObject, environment, sandbox) { results = propertyProvider(`/*I'm a comment\n \t */\n\nt`); test_has_result(results, "testObject"); + + info("Test local expression variables"); + results = propertyProvider("b", { expressionVars: ["a", "b", "c"] }); + test_has_result(results, "b"); + Assert.equal(results.matches.has("a"), false); + Assert.equal(results.matches.has("c"), false); + + info( + "Test that local expression variables are not included when accessing an object properties" + ); + results = propertyProvider("testObject.prop", { + expressionVars: ["propLocal"], + }); + Assert.equal(results.matches.has("propLocal"), false); + test_has_result(results, "propA"); + + results = propertyProvider("testObject['prop", { + expressionVars: ["propLocal"], + }); + test_has_result(results, "'propA'"); + Assert.equal(results.matches.has("propLocal"), false); } /**