From 4b3ef8ebd69634e0b90ff1e7460bfc80352dd732 Mon Sep 17 00:00:00 2001 From: Favour Ohanekwu Date: Wed, 25 Oct 2023 18:18:45 +0100 Subject: [PATCH] fix: improve autocompletion hints discovery (#28222) ## Description This PR improves autocompletion hints discovery by - Taking entities' recency of usage into consideration when sorting hints - Showing entity names at the top, before supported functions and properties - Deprioritizing the Function constructor and the MainContainer entity #### PR fixes following issue(s) Fixes #27870 Fixes #17684 Fixes https://github.com/appsmithorg/appsmith/issues/24975 #### Type of change - New feature (non-breaking change which adds functionality) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [ ] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan - [x] check for autocomplete results when multiple widgets of the same type are used - [x] check for entity renaming behavior - [x] verify function and mainContainer have been removed - [x] verify that recent entity changes affects autocomplete ranking > > #### Issues raised during DP testing https://github.com/appsmithorg/appsmith/pull/28222#issuecomment-1777487126 > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [x] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [x] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [x] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../PropertyPaneSuggestion_spec.ts | 2 +- .../Widgets/Others/Autocomplete_spec.js | 21 +---- app/client/packages/ast/src/index.ts | 6 +- app/client/packages/ast/src/utils.ts | 11 +++ app/client/src/ce/sagas/index.tsx | 2 + app/client/src/sagas/TernSaga.ts | 76 +++++++++++++++++++ .../autocomplete/AutocompleteSortRules.ts | 61 +++++++++++++-- .../autocomplete/CodemirrorTernService.ts | 37 ++++++++- .../autocomplete/__tests__/TernServer.test.ts | 4 +- .../utils/autocomplete/dataTypeSortRules.ts | 2 + 10 files changed, 190 insertions(+), 32 deletions(-) create mode 100644 app/client/src/sagas/TernSaga.ts diff --git a/app/client/cypress/e2e/Regression/ClientSide/Autocomplete/PropertyPaneSuggestion_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/Autocomplete/PropertyPaneSuggestion_spec.ts index 0664635888..ff037f1084 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Autocomplete/PropertyPaneSuggestion_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/Autocomplete/PropertyPaneSuggestion_spec.ts @@ -46,6 +46,6 @@ describe("Property Pane Suggestions", () => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore cy.get("body").tab(); - propPane.ValidatePropertyFieldValue("Label", "{{appsmith}}"); + propPane.ValidatePropertyFieldValue("Label", "{{JSObject1}}"); }); }); diff --git a/app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/Autocomplete_spec.js b/app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/Autocomplete_spec.js index 8ab713e68d..9cca4d369a 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/Autocomplete_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/Autocomplete_spec.js @@ -62,16 +62,8 @@ describe("Autocomplete using slash command and mustache tests", function () { .type("{shift}{{}{shift}{{}") .then(() => { cy.get(dynamicInputLocators.hints).should("exist"); - // validates all autocomplete functions on entering {{}} in onClick field - cy.get(`${dynamicInputLocators.hints} li`) - .eq(7) - .should("have.text", "storeValue"); - cy.get(`${dynamicInputLocators.hints} li`) - .eq(8) - .should("have.text", "showAlert"); - cy.get(`${dynamicInputLocators.hints} li`) - .eq(9) - .should("have.text", "navigateTo"); + _.agHelper.AssertContains("storeValue"); + _.agHelper.AssertContains("showAlert"); }); }); @@ -101,13 +93,8 @@ describe("Autocomplete using slash command and mustache tests", function () { cy.get(dynamicInputLocators.input) .first() .type("{shift}{{}{shift}{{}"); - // validates autocomplete binding on entering {{}} in text field - cy.get(`${dynamicInputLocators.hints} li`) - .eq(1) - .should("have.text", "Button1.text"); - cy.get(`${dynamicInputLocators.hints} li`) - .eq(2) - .should("have.text", "Button1.recaptchaToken"); + _.agHelper.AssertContains("Button1.text"); + _.agHelper.AssertContains("Button1.recaptchaToken"); }); }, ); diff --git a/app/client/packages/ast/src/index.ts b/app/client/packages/ast/src/index.ts index 39ceb5e437..9b88786a2e 100644 --- a/app/client/packages/ast/src/index.ts +++ b/app/client/packages/ast/src/index.ts @@ -2,8 +2,8 @@ import type { Node, SourceLocation, Options, Comment } from "acorn"; import { parse } from "acorn"; import { ancestor, simple } from "acorn-walk"; import { ECMA_VERSION, NodeTypes } from "./constants"; -import { has, isFinite, isString, toPath } from "lodash"; -import { isTrueObject, sanitizeScript } from "./utils"; +import { has, isFinite, isNil, isString, toPath } from "lodash"; +import { getStringValue, isTrueObject, sanitizeScript } from "./utils"; import { jsObjectDeclaration } from "./jsObject"; import { attachComments } from "astravel"; import { generate } from "astring"; @@ -907,7 +907,7 @@ export function getMemberExpressionObjectFromProperty( const propName = isLiteralNode(property) ? property.value : property.name; - if (propName && propName.toString() === propertyName) { + if (!isNil(propName) && getStringValue(propName) === propertyName) { const memberExpressionObjectString = generate(object); memberExpressionObjects.add(memberExpressionObjectString); } diff --git a/app/client/packages/ast/src/utils.ts b/app/client/packages/ast/src/utils.ts index b82005cb8b..6e595ebe8d 100644 --- a/app/client/packages/ast/src/utils.ts +++ b/app/client/packages/ast/src/utils.ts @@ -58,3 +58,14 @@ export const extractContentByPosition = ( } return returnedString; }; + +export const getStringValue = ( + inputValue: string | number | boolean | RegExp, +) => { + if (typeof inputValue === "object" || typeof inputValue === "boolean") { + inputValue = JSON.stringify(inputValue); + } else if (typeof inputValue === "number" || typeof inputValue === "string") { + inputValue += ""; + } + return inputValue; +}; diff --git a/app/client/src/ce/sagas/index.tsx b/app/client/src/ce/sagas/index.tsx index 5f9a0d1094..b74948bfb9 100644 --- a/app/client/src/ce/sagas/index.tsx +++ b/app/client/src/ce/sagas/index.tsx @@ -55,6 +55,7 @@ import communityTemplateSagas from "sagas/CommunityTemplatesSagas"; /* Sagas that are registered by a module that is designed to be independent of the core platform */ import LayoutElementPositionsSaga from "layoutSystems/anvil/integrations/sagas/LayoutElementPositionsSaga"; import anvilDraggingSagas from "layoutSystems/anvil/integrations/sagas/draggingSagas"; +import ternSagas from "sagas/TernSaga"; export const sagas = [ initSagas, @@ -112,4 +113,5 @@ export const sagas = [ LayoutElementPositionsSaga, communityTemplateSagas, anvilDraggingSagas, + ternSagas, ]; diff --git a/app/client/src/sagas/TernSaga.ts b/app/client/src/sagas/TernSaga.ts new file mode 100644 index 0000000000..78de77fae4 --- /dev/null +++ b/app/client/src/sagas/TernSaga.ts @@ -0,0 +1,76 @@ +import type { ReduxAction } from "@appsmith/constants/ReduxActionConstants"; +import { ReduxActionTypes } from "@appsmith/constants/ReduxActionConstants"; +import { + getActions, + getJSCollections, +} from "@appsmith/selectors/entitiesSelector"; +import type { AppState } from "@appsmith/reducers"; +import type { RecentEntity } from "components/editorComponents/GlobalSearch/utils"; +import type { Datasource } from "entities/Datasource"; +import { get } from "lodash"; +import { FocusEntity } from "navigation/FocusEntity"; +import { select, takeLatest } from "redux-saga/effects"; +import { getWidgets } from "./selectors"; +import CodemirrorTernService from "utils/autocomplete/CodemirrorTernService"; + +function* handleSetTernRecentEntities(action: ReduxAction) { + const recentEntities = action.payload || []; + + const actions: ReturnType = yield select(getActions); + const jsActions: ReturnType = + yield select(getJSCollections); + const reducerDatasources: Datasource[] = yield select((state: AppState) => { + return state.entities.datasources.list; + }); + const widgetsMap: ReturnType = yield select(getWidgets); + + const recentEntityNames = new Set(); + + for (const recentEntity of recentEntities) { + const { id, type } = recentEntity; + + switch (type) { + case FocusEntity.DATASOURCE: { + const datasource = reducerDatasources.find( + (reducerDatasource) => reducerDatasource.id === id, + ); + if (!datasource) break; + recentEntityNames.add(datasource.name); + break; + } + case FocusEntity.API: + case FocusEntity.QUERY: { + const action = actions.find((action) => action?.config?.id === id); + if (!action) break; + recentEntityNames.add(action.config.name); + break; + } + case FocusEntity.JS_OBJECT: { + const action = jsActions.find((action) => action?.config?.id === id); + if (!action) break; + recentEntityNames.add(action.config.name); + break; + } + case FocusEntity.PROPERTY_PANE: { + const widget = get(widgetsMap, id, null); + if (!widget) break; + recentEntityNames.add(widget.widgetName); + } + } + } + + CodemirrorTernService.updateRecentEntities(Array.from(recentEntityNames)); +} +function* handleResetTernRecentEntities() { + CodemirrorTernService.updateRecentEntities([]); +} +export default function* ternSagas() { + yield takeLatest( + ReduxActionTypes.SET_RECENT_ENTITIES, + handleSetTernRecentEntities, + ); + yield takeLatest( + ReduxActionTypes.RESET_RECENT_ENTITIES, + handleResetTernRecentEntities, + ); +} diff --git a/app/client/src/utils/autocomplete/AutocompleteSortRules.ts b/app/client/src/utils/autocomplete/AutocompleteSortRules.ts index 74641ee4c2..a098a61264 100644 --- a/app/client/src/utils/autocomplete/AutocompleteSortRules.ts +++ b/app/client/src/utils/autocomplete/AutocompleteSortRules.ts @@ -1,5 +1,9 @@ import type { FieldEntityInformation } from "components/editorComponents/CodeEditor/EditorConfig"; -import { DataTreeFunctionSortOrder, PriorityOrder } from "./dataTypeSortRules"; +import { + DataTreeFunctionSortOrder, + PriorityOrder, + blockedCompletions, +} from "./dataTypeSortRules"; import type { Completion, DataTreeDefEntityInformation, @@ -23,7 +27,9 @@ enum RuleWeight { JSLibrary, DataTreeFunction, DataTreeMatch, + RecentEntityMatch, TypeMatch, + DataTreeEntityNameMatch, PriorityMatch, ScopeMatch, } @@ -95,6 +101,11 @@ class RemoveBlackListedCompletionRule implements AutocompleteRule { const { currentFieldInfo } = AutocompleteSorter; const { blockCompletions } = currentFieldInfo; + if (blockedCompletions.includes(completion.text)) { + score = RemoveBlackListedCompletionRule.threshold; + return score; + } + if (blockCompletions) { for (let index = 0; index < blockCompletions.length; index++) { const { subPath } = blockCompletions[index]; @@ -192,11 +203,26 @@ class DataTreeFunctionRule implements AutocompleteRule { return score; } } +/** + * Sets threshold value for completions that are recent entities + * Max score - 10000 + number + * Min score - 0 + */ +class RecentEntityRule implements AutocompleteRule { + static threshold = 1 << RuleWeight.RecentEntityMatch; + computeScore(completion: Completion): number { + let score = 0; + if (completion.recencyWeight) { + score += RecentEntityRule.threshold + completion.recencyWeight; + } + return score; + } +} /** * Set's threshold value for completions that belong to the dataTree and sets higher score for * completions that are not functions - * Max score - 11000 - binary + * Max score - 110000 - binary * Min score - 0 */ class DataTreeRule implements AutocompleteRule { @@ -212,7 +238,7 @@ class DataTreeRule implements AutocompleteRule { /** * Set's threshold value for completions that match the expectedValue of the current field. - * Max score - 100000 - binary + * Max score - 1000000 - binary * Min score - 0 */ class TypeMatchRule implements AutocompleteRule { @@ -226,9 +252,23 @@ class TypeMatchRule implements AutocompleteRule { } } +/** + * Set's threshold value for completions that belong to the dataTree and are entity names + * Max score - 10000000 - binary + * Min score - 0 + */ +class DataTreeEntityNameRule implements AutocompleteRule { + static threshold = 1 << RuleWeight.DataTreeEntityNameMatch; + computeScore(completion: Completion): number { + let score = 0; + if (completion.isEntityName) score += DataTreeEntityNameRule.threshold; + return score; + } +} + /** * Set's threshold value for completions that resides in PriorityOrder, eg. selectedRow for Table1. - * Max score - 1000000 - binary + * Max score - 100000000 - binary * Min score - 0 */ class PriorityMatchRule implements AutocompleteRule { @@ -249,16 +289,19 @@ class PriorityMatchRule implements AutocompleteRule { } /** - * Sets threshold value.to completions from the same scop. - * Max score - 10000000 - binary + * Sets threshold value.to completions from the same scope. + * Max score - 1000000000 - binary * Min score - 0 */ class ScopeMatchRule implements AutocompleteRule { static threshold = 1 << RuleWeight.ScopeMatch; computeScore(completion: Completion): number { let score = 0; - if (completion.origin === "[doc]" || completion.origin === "customDataTree") - score += PriorityMatchRule.threshold; + if ( + completion.origin?.startsWith("[doc") || + completion.origin === "customDataTree" + ) + score += ScopeMatchRule.threshold; return score; } } @@ -341,8 +384,10 @@ export class ScoredCompletion { new NoSelfReferenceRule(), new ScopeMatchRule(), new PriorityMatchRule(), + new DataTreeEntityNameRule(), new TypeMatchRule(), new DataTreeRule(), + new RecentEntityRule(), new DataTreeFunctionRule(), new JSLibraryRule(), new GlobalJSRule(), diff --git a/app/client/src/utils/autocomplete/CodemirrorTernService.ts b/app/client/src/utils/autocomplete/CodemirrorTernService.ts index bef4722f50..bebdb22f77 100644 --- a/app/client/src/utils/autocomplete/CodemirrorTernService.ts +++ b/app/client/src/utils/autocomplete/CodemirrorTernService.ts @@ -19,7 +19,7 @@ import { getCodeMirrorNamespaceFromEditor, } from "../getCodeMirrorNamespace"; import AnalyticsUtil from "utils/AnalyticsUtil"; -import { findIndex } from "lodash"; +import { findIndex, isString } from "lodash"; const bigDoc = 250; const cls = "CodeMirror-Tern-"; @@ -35,6 +35,8 @@ export interface Completion< data: T; render?: any; isHeader?: boolean; + recencyWeight?: number; + isEntityName?: boolean; } export interface CommandsCompletion @@ -129,6 +131,28 @@ export function typeToIcon(type: string, isKeyword: boolean) { return cls + "completion " + cls + "completion-" + suffix; } +function getRecencyWeight( + completion: + | string + | { + name: string; + origin?: string | undefined; + }, + recentEntities: string[], +) { + const completionEntityName = isString(completion) + ? completion.split(".")[0] + : completion.name.split(".")[0]; + const completionOrigin = isString(completion) ? "" : completion.origin; + if (completionOrigin !== "DATA_TREE") return 0; + const recencyIndex = recentEntities.findIndex( + (entityName) => entityName === completionEntityName, + ); + if (recencyIndex === -1) return 0; + const recencyWeight = recentEntities.length - recencyIndex; + return recencyWeight; +} + class CodeMirrorTernService { server: Server; docs: TernDocs = Object.create(null); @@ -140,6 +164,7 @@ class CodeMirrorTernService { DataTreeDefEntityInformation >(); options: { async: boolean }; + recentEntities: string[] = []; constructor(options: { async: boolean }) { this.options = options; @@ -257,12 +282,17 @@ class CodeMirrorTernService { const isCustomKeyword = isCustomKeywordType(completion.name); const className = typeToIcon(completion.type as string, isCustomKeyword); const dataType = getDataType(completion.type as string); + const recencyWeight = getRecencyWeight(completion, this.recentEntities); + const isCompletionADataTreeEntityName = + completion.origin === "DATA_TREE" && + this.defEntityInformation.has(completion.name); let completionText = completion.name + after; if (dataType === "FUNCTION" && !completion.origin?.startsWith("LIB/")) { if (token.type !== "string" && token.string !== "[") { completionText = completionText + "()"; } } + const codeMirrorCompletion: Completion = { text: completionText, displayText: completion.name, @@ -271,6 +301,8 @@ class CodeMirrorTernService { origin: completion.origin as string, type: dataType, isHeader: false, + recencyWeight, + isEntityName: isCompletionADataTreeEntityName, }; if (isCustomKeyword) { @@ -932,6 +964,9 @@ class CodeMirrorTernService { return query; } + updateRecentEntities(recentEntities: string[]) { + this.recentEntities = recentEntities; + } } export const createCompletionHeader = (name: string): Completion => ({ diff --git a/app/client/src/utils/autocomplete/__tests__/TernServer.test.ts b/app/client/src/utils/autocomplete/__tests__/TernServer.test.ts index 3fd61bfbd0..c687ee7b4c 100644 --- a/app/client/src/utils/autocomplete/__tests__/TernServer.test.ts +++ b/app/client/src/utils/autocomplete/__tests__/TernServer.test.ts @@ -384,7 +384,7 @@ describe("Tern server sorting", () => { dataTreeCompletion, AutocompleteSorter.currentFieldInfo, ); - expect(scoredCompletion1.score).toEqual(2 ** 5 + 2 ** 4 + 2 ** 3); + expect(scoredCompletion1.score).toEqual(2 ** 6 + 2 ** 4 + 2 ** 3); //completion that belongs to the same entity. const scoredCompletion2 = new ScoredCompletion( sameEntityCompletion, @@ -396,6 +396,6 @@ describe("Tern server sorting", () => { priorityCompletion, AutocompleteSorter.currentFieldInfo, ); - expect(scoredCompletion3.score).toBe(2 ** 6 + 2 ** 4 + 2 ** 3); + expect(scoredCompletion3.score).toBe(2 ** 8 + 2 ** 4 + 2 ** 3); }); }); diff --git a/app/client/src/utils/autocomplete/dataTypeSortRules.ts b/app/client/src/utils/autocomplete/dataTypeSortRules.ts index 8a93cbee9f..896986ec0e 100644 --- a/app/client/src/utils/autocomplete/dataTypeSortRules.ts +++ b/app/client/src/utils/autocomplete/dataTypeSortRules.ts @@ -17,3 +17,5 @@ export const DataTreeFunctionSortOrder = [ "showModal()", "setInterval()", ]; + +export const blockedCompletions = ["Function()", "MainContainer"];