WIP: autocomplete fixes for associative expressions, select the right…

… type for functions
finos · texodus · Jun 24, 2020 · Jun 9, 2020 · Jun 9, 2020 · Jun 10, 2020
commit 0371d4bb1df0662e9ad2121b3250d3d20e6bdbf0
diff --git a/packages/perspective-viewer/src/js/computed_expressions/computed_expression_parser.js b/packages/perspective-viewer/src/js/computed_expressions/computed_expression_parser.js
@@ -130,9 +130,11 @@ class PerspectiveComputedExpressionParser {
      *
      * @param {ILexingResult} lexer_result
      * @param {String} expression
+     * @param {Array[String]} input_types an array of data types by which to
+     * filter down the suggestions.
      * @returns {Array}
      */
-    get_autocomplete_suggestions(expression, lexer_result) {
+    get_autocomplete_suggestions(expression, lexer_result, input_types) {
         this._check_initialized();
         let initial_suggestions = this._parser.computeContentAssist("SuperExpression", []);
 
@@ -141,13 +143,20 @@ class PerspectiveComputedExpressionParser {
         }
 
         if (lexer_result.errors.length > 0) {
-            // Check if the last token is partial AND not a column name (not in
-            // quotes). If true, the suggest function names that match.
+            // Check if the last string fragment is partial AND not a
+            // column name (not in quotes). If true, the suggest function
+            // names that match.
             const partial_function = this.extract_partial_function(expression);
 
-            if (partial_function && partial_function.search(/["']$/) === -1) {
+            // If the last fragment is partial, check if the last parsed
+            // token is a column name, as we should not be sending suggestions
+            // for a partial function that immediately follows a column name,
+            // i.e. `"Sales" a`.
+            const last_column_token = this.get_last_token_with_types([ColumnName], lexer_result, 1);
+
+            if (partial_function && partial_function.search(/["']$/) === -1 && !last_column_token) {
                 // Remove open parenthesis and column name rule
-                const suggestions = this._apply_suggestion_metadata(initial_suggestions.slice(2));
+                const suggestions = this._apply_suggestion_metadata(initial_suggestions.slice(1), input_types);
                 const exact_matches = [];
                 const fuzzy_matches = [];
 
@@ -170,8 +179,9 @@ class PerspectiveComputedExpressionParser {
 
         // Remove whitespace tokens
         lexer_result.tokens = clean_tokens(lexer_result.tokens);
-        const suggestions = this._parser.computeContentAssist("SuperExpression", lexer_result.tokens);
-        return this._apply_suggestion_metadata(suggestions);
+        let suggestions = this._apply_suggestion_metadata(this._parser.computeContentAssist("SuperExpression", lexer_result.tokens), input_types);
+
+        return suggestions;
     }
 
     /**
@@ -649,41 +659,57 @@ class PerspectiveComputedExpressionParser {
      * Given a list of suggestions, transform each suggestion into an object
      * with `label` and `value`.
      *
-     * @param {*} suggestions
+     * @param {Array} suggestions
+     * @param {Array[String]} input_types an array of input types as strings.
      */
-    _apply_suggestion_metadata(suggestions) {
+    _apply_suggestion_metadata(suggestions, input_types) {
         this._check_initialized();
         const suggestions_with_metadata = [];
 
-        for (const suggestion of suggestions) {
-            const token = suggestion.nextTokenType;
+        for (const s of suggestions) {
+            const token = s.nextTokenType;
 
             if (!token || !token.PATTERN.source) {
                 continue;
             }
 
-            const label = token.LABEL;
+            let label = token.LABEL;
             let pattern = token.PATTERN.source.replace(/\\/g, "");
             let value = pattern;
 
             if (tokenMatcher(token, FunctionTokenType)) {
                 value = `${value}(`;
             } else if (tokenMatcher(token, OperatorTokenType)) {
                 value = `${value} `;
+            } else if (tokenMatcher(token, As)) {
+                value = "AS ";
+                label = "AS";
+                token.signature = "x + y AS new column";
+                token.help = "Creates a custom name for the computed column.";
             }
 
-            suggestions_with_metadata.push(
-                new ComputedExpressionAutocompleteSuggestion({
-                    label,
-                    value,
-                    pattern,
-                    signature: token.signature,
-                    help: token.help,
-                    input_types: token.input_types,
-                    return_type: token.return_type,
-                    num_params: token.num_params
-                })
-            );
+            const suggestion = new ComputedExpressionAutocompleteSuggestion({
+                label,
+                value,
+                pattern,
+                signature: token.signature,
+                help: token.help,
+                input_types: token.input_types,
+                return_type: token.return_type,
+                num_params: token.num_params
+            });
+
+            if (input_types && suggestion.input_types) {
+                // Filter on input type if present
+                for (const type of input_types) {
+                    if (suggestion.input_types.includes(type)) {
+                        suggestions_with_metadata.push(suggestion);
+                        break;
+                    }
+                }
+            } else {
+                suggestions_with_metadata.push(suggestion);
+            }
         }
 
         return suggestions_with_metadata;

diff --git a/packages/perspective-viewer/src/js/computed_expressions/widget.js b/packages/perspective-viewer/src/js/computed_expressions/widget.js
@@ -194,9 +194,6 @@ class ComputedExpressionWidget extends HTMLElement {
             // share an instance of the parser throughout the viewer.
             this._parsed_expression = this._computed_expression_parser.parse(expression);
         } catch (e) {
-            // TODO: make sure autocomplete works for each step of the new
-            // expressions with associativity - it should actually decrease
-            // the amount of custom logic we need here.
             // Show autocomplete OR error, but not both
             this._clear_error();
             this._disable_save_button();
@@ -228,23 +225,26 @@ class ComputedExpressionWidget extends HTMLElement {
             // autocomplete.
             const show_column_names = (has_name_fragments && !is_alias && !last_column_name && !last_paren) || last_operator;
 
+            // Get autocomplete suggestions from Chevrotain
+            let suggestions = [];
+
             if (show_column_names) {
                 let column_names;
-                let suggestions;
 
                 // check previous token to see if it is a function or operator
                 const last_function_or_operator = this._computed_expression_parser.get_last_token_with_types([FunctionTokenType, OperatorTokenType], lex_result);
 
                 if (last_function_or_operator) {
                     const input_types = last_function_or_operator.tokenType.input_types;
                     column_names = this._get_view_column_names_by_types(input_types);
+                    suggestions = this._computed_expression_parser.get_autocomplete_suggestions(expression, lex_result, input_types);
                 } else {
                     // Show all column names
                     column_names = this._get_view_all_column_names();
                 }
 
                 // Convert list of names into objects with `label` and `value`
-                suggestions = this._make_column_name_suggestions(column_names);
+                let column_name_suggestions = this._make_column_name_suggestions(column_names);
 
                 // Filter down by `startsWith` and `contains`, putting the
                 // more exact matches first.
@@ -253,7 +253,7 @@ class ComputedExpressionWidget extends HTMLElement {
                     const exact_matches = [];
                     const fuzzy_matches = [];
 
-                    for (const suggestion of suggestions) {
+                    for (const suggestion of column_name_suggestions) {
                         const column_name = suggestion.label.toLowerCase();
                         const partial = fragment.toLowerCase();
 
@@ -264,26 +264,21 @@ class ComputedExpressionWidget extends HTMLElement {
                         }
                     }
 
-                    suggestions = exact_matches.concat(fuzzy_matches);
+                    column_name_suggestions = exact_matches.concat(fuzzy_matches);
                 }
 
-                if (last_operator) {
-                    // Make sure we have opening parenthesis if the last token
-                    // is an operator
-                    suggestions = [
-                        new ComputedExpressionAutocompleteSuggestion({
-                            label: "(",
-                            value: "("
-                        })
-                    ].concat(suggestions);
+                if (last_function_or_operator) {
+                    suggestions = suggestions.concat(column_name_suggestions);
+                } else {
+                    suggestions = column_name_suggestions;
                 }
 
                 // Render column names inside autocomplete
                 const markup = this.make_autocomplete_markup(suggestions);
                 this._autocomplete.render(markup);
                 return;
             } else {
-                const suggestions = this._computed_expression_parser.get_autocomplete_suggestions(expression, lex_result);
+                suggestions = this._computed_expression_parser.get_autocomplete_suggestions(expression, lex_result);
                 if (suggestions.length > 0) {
                     // Show autocomplete and not error box
                     const markup = this.make_autocomplete_markup(suggestions);

diff --git a/packages/perspective-viewer/test/js/computed_expressions.spec.js b/packages/perspective-viewer/test/js/computed_expressions.spec.js
@@ -181,6 +181,13 @@ utils.with_server({}, () => {
                 await page.shadow_type("day", "perspective-viewer", "perspective-computed-expression-widget", "perspective-expression-editor", ".perspective-expression-editor__edit_area");
             });
 
+            test.capture("Typing a column name followed by a partial function should not show autocomplete", async page => {
+                await page.shadow_click("perspective-viewer", "#config_button");
+                await page.$("perspective-viewer");
+                await page.shadow_click("perspective-viewer", "#add-computed-expression");
+                await page.shadow_type("'Sales' a", "perspective-viewer", "perspective-computed-expression-widget", "perspective-expression-editor", ".perspective-expression-editor__edit_area");
+            });
+
             test.capture("Pressing arrow down should select the next autocomplete item", async page => {
                 await page.shadow_click("perspective-viewer", "#config_button");
                 await page.$("perspective-viewer");