fix: Adjust autocomplete results ranking for function arguments (#28632)

## Description
> Leverage function arguments type generated in #28214 to better rank
autocomplete results for function arguments. Current entity and field
information is passed to the `CodemirrorTernService` at the time of hint
generation. We check to see if the cursor position is next to a function
argument and sort the results based on its type.
>
>
#### PR fixes following issue(s)
Fixes #28107
>
#### Media
> A video or a GIF is preferred. when using Loom, don’t embed because it
looks like it’s a GIF. instead, just link to the video
>
#### Type of change
- Bug fix (non-breaking change which fixes an issue)
>
>
## Testing
>
#### How Has This Been Tested?
- [x] Manual
- [x] Jest
>
>
#### Test Plan
> Add Testsmith test cases links that relate to this PR
>
>
#### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)
>
>
>
## Checklist:
#### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [ ] 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
- [ ] 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

---------

Co-authored-by: Aishwarya UR <aishwarya@appsmith.com>
This commit is contained in:
arunvjn 2023-11-10 14:18:42 +05:30 committed by GitHub
parent 863dbddb7f
commit d6e572f2b5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 96 additions and 38 deletions

View File

@ -127,7 +127,7 @@ export const entityDefinitions = {
...responseMetaDef,
},
run: {
"!type": "fn(params: ?) -> +Promise",
"!type": "fn(params?: {}) -> +Promise",
"!url":
"https://docs.appsmith.com/reference/appsmith-framework/query-object#queryrun",
"!doc": "Executes the query with the given parameters.",

View File

@ -25,6 +25,24 @@ function generateRandomTable() {
return table;
}
jest.mock("utils/getCodeMirrorNamespace", () => {
const actual = jest.requireActual("utils/getCodeMirrorNamespace");
return {
...actual,
getCodeMirrorNamespaceFromDoc: jest.fn((doc) => ({
...actual.getCodeMirrorNamespaceFromDoc(doc),
innerMode: jest.fn(() => ({
mode: {
name: "",
},
state: {
lexical: {},
},
})),
})),
};
});
describe("hint helpers", () => {
describe("binding hint helper", () => {
it("is initialized correctly", () => {
@ -80,16 +98,22 @@ describe("hint helpers", () => {
cases.forEach((testCase) => {
MockCodemirrorEditor.getValue.mockReturnValueOnce(testCase.value);
MockCodemirrorEditor.getCursor.mockReturnValueOnce(testCase.cursor);
MockCodemirrorEditor.getCursor.mockReturnValue(testCase.cursor);
if (testCase.getLine) {
testCase.getLine.forEach((line) => {
MockCodemirrorEditor.getLine.mockReturnValueOnce(line);
});
}
});
// Test
cases.forEach(() => {
MockCodemirrorEditor.getTokenAt.mockReturnValueOnce({
type: "string",
string: "",
});
MockCodemirrorEditor.getDoc.mockReturnValueOnce({
getCursor: () => testCase.cursor,
somethingSelected: () => false,
getValue: () => testCase.value,
getEditor: () => MockCodemirrorEditor,
} as unknown as CodeMirror.Doc);
// @ts-expect-error: Types are not available
const helper = bindingHintHelper(MockCodemirrorEditor, {});
// @ts-expect-error: Types are not available

View File

@ -37,12 +37,12 @@ export const bindingHintHelper: HintHelper = (editor: CodeMirror.Editor) => {
additionalData,
): boolean => {
if (additionalData && additionalData.blockCompletions) {
CodemirrorTernService.setEntityInformation({
CodemirrorTernService.setEntityInformation(editor, {
...entityInformation,
blockCompletions: additionalData.blockCompletions,
});
} else {
CodemirrorTernService.setEntityInformation(entityInformation);
CodemirrorTernService.setEntityInformation(editor, entityInformation);
}
const entityType = entityInformation?.entityType;

View File

@ -306,7 +306,7 @@ class ScopeMatchRule implements AutocompleteRule {
}
}
class BlockAsyncFnsInDataFieldRule implements AutocompleteRule {
class BlockAsyncFnsRule implements AutocompleteRule {
static threshold = -Infinity;
static blackList = [
"setTimeout",
@ -322,13 +322,21 @@ class BlockAsyncFnsInDataFieldRule implements AutocompleteRule {
entityInfo?: FieldEntityInformation | undefined,
): number {
const score = 0;
if (entityInfo?.isTriggerPath) return score;
if (completion.type !== "FUNCTION") return score;
if (completion.type !== AutocompleteDataType.FUNCTION) return score;
if (!completion.displayText) return score;
const isAsyncFunction = completion.data?.type?.endsWith("Promise");
if (isAsyncFunction) return BlockAsyncFnsInDataFieldRule.threshold;
if (BlockAsyncFnsInDataFieldRule.blackList.includes(completion.displayText))
return BlockAsyncFnsInDataFieldRule.threshold;
if (entityInfo?.isTriggerPath) {
// triggerPath = true and expectedType = undefined for JSObjects
if (!entityInfo.expectedType) return score;
// triggerPath = true and expectedType = FUNCTION or UNKNOWN for trigger fields.
if (entityInfo.expectedType === AutocompleteDataType.FUNCTION)
return score;
if (entityInfo.expectedType === AutocompleteDataType.UNKNOWN)
return score;
}
const isAsyncFunction =
completion.data?.type?.endsWith("Promise") ||
BlockAsyncFnsRule.blackList.includes(completion.displayText);
if (isAsyncFunction) return BlockAsyncFnsRule.threshold;
return score;
}
}
@ -382,7 +390,7 @@ export class AutocompleteSorter {
export class ScoredCompletion {
score = 0;
static rules = [
new BlockAsyncFnsInDataFieldRule(),
new BlockAsyncFnsRule(),
new NoDeepNestedSuggestionsRule(),
new NoSelfReferenceRule(),
new ScopeMatchRule(),

View File

@ -178,7 +178,6 @@ class CodeMirrorTernService {
DataTreeDefEntityInformation
>();
options: { async: boolean };
activeArgType: string | null = null;
recentEntities: string[] = [];
constructor(options: { async: boolean }) {
@ -216,7 +215,6 @@ class CodeMirrorTernService {
// Forked from app/client/node_modules/codemirror/addon/tern/tern.js
updateArgHints(cm: CodeMirror.Editor) {
this.closeArgHints();
this.activeArgType = null;
cm.state.ternTooltip = null;
if (cm.somethingSelected()) return false;
if (cm.state.completionActive) return false;
@ -272,7 +270,7 @@ class CodeMirrorTernService {
guess: data.guess,
doc: cm.getDoc(),
};
this.showArgHints(cm, argPos);
if (!cm.state.completionActive) this.showArgHints(cm, argPos);
},
);
return true;
@ -321,7 +319,6 @@ class CodeMirrorTernService {
if (this.activeArgHints.clear) this.activeArgHints.clear();
this.remove(this.activeArgHints);
this.activeArgHints = null;
this.activeArgType = null;
}
return true;
}
@ -354,9 +351,6 @@ class CodeMirrorTernService {
this.elt("span", cls + "type", shortTernType(arg.type)),
);
}
if (i == pos) {
this.activeArgType = arg.type;
}
}
tip.appendChild(document.createTextNode(")"));
if (tp.rettype)
@ -518,12 +512,6 @@ class CodeMirrorTernService {
const shouldComputeBestMatch =
this.fieldEntityInformation.entityType !== ENTITY_TYPE_VALUE.JSACTION;
if (this.activeArgType) {
this.fieldEntityInformation.expectedType = getDataType(
this.activeArgType,
);
}
completions = AutocompleteSorter.sort(
completions,
{ ...this.fieldEntityInformation, token },
@ -552,7 +540,7 @@ class CodeMirrorTernService {
(completion) => !completion.isHeader,
).length,
});
this.closeArgHints();
this.activeArgHints && this.remove(this.activeArgHints);
});
CodeMirror.on(obj, "close", () => this.remove(tooltip));
CodeMirror.on(obj, "update", () => this.remove(tooltip));
@ -612,8 +600,8 @@ class CodeMirrorTernService {
);
});
// When a function is picked, move the cursor between the parenthesis
const CodeMirror = getCodeMirrorNamespaceFromEditor(cm);
// When a function is picked, move the cursor between the parenthesis.
const CodeMirror = getCodeMirrorNamespaceFromDoc(cm.getDoc());
CodeMirror.on(hints, "pick", (selected: Completion) => {
const hintsWithoutHeaders = hints.list.filter(
(h: Record<string, unknown>) => h.isHeader !== true,
@ -1161,7 +1149,22 @@ class CodeMirrorTernService {
this.remove(tooltip);
}
setEntityInformation(entityInformation: FieldEntityInformation) {
setEntityInformation(
cm: CodeMirror.Editor,
entityInformation: FieldEntityInformation,
) {
const state = cm.getTokenAt(cm.getCursor()).state;
const CodeMirror = getCodeMirrorNamespaceFromDoc(cm.getDoc());
const inner = CodeMirror.innerMode(cm.getMode(), state);
if (inner.mode.name != "javascript") return false;
const lex = inner.state.lexical;
if (lex.info === "call") {
const argPos = lex.pos || 0;
const args = this.cachedArgHints?.type?.args || [];
const arg = args[argPos];
const argType = arg?.type;
entityInformation.expectedType = getDataType(argType);
}
this.fieldEntityInformation = entityInformation;
}

View File

@ -10,6 +10,25 @@ import { MockCodemirrorEditor } from "../../../../test/__mocks__/CodeMirrorEdito
import { ENTITY_TYPE_VALUE } from "entities/DataTree/dataTreeFactory";
import _ from "lodash";
import { AutocompleteSorter, ScoredCompletion } from "../AutocompleteSortRules";
import type CodeMirror from "codemirror";
jest.mock("utils/getCodeMirrorNamespace", () => {
const actual = jest.requireActual("utils/getCodeMirrorNamespace");
return {
...actual,
getCodeMirrorNamespaceFromDoc: jest.fn((doc) => ({
...actual.getCodeMirrorNamespaceFromDoc(doc),
innerMode: jest.fn(() => ({
mode: {
name: "",
},
state: {
lexical: {},
},
})),
})),
};
});
describe("Tern server", () => {
it("Check whether the correct value is being sent to tern", () => {
@ -340,11 +359,14 @@ describe("Tern server sorting", () => {
];
it("shows best match results", () => {
CodemirrorTernService.setEntityInformation({
entityName: "sameEntity",
entityType: ENTITY_TYPE_VALUE.WIDGET,
expectedType: AutocompleteDataType.OBJECT,
});
CodemirrorTernService.setEntityInformation(
MockCodemirrorEditor as unknown as CodeMirror.Editor,
{
entityName: "sameEntity",
entityType: ENTITY_TYPE_VALUE.WIDGET,
expectedType: AutocompleteDataType.OBJECT,
},
);
CodemirrorTernService.defEntityInformation = defEntityInformation;
const sortedCompletions = AutocompleteSorter.sort(
_.shuffle(completions),

View File

@ -18,6 +18,7 @@ export const MockCodemirrorEditor = {
getRange: jest.fn(),
getDoc: jest.fn(),
getTokenAt: jest.fn(),
getMode: jest.fn(),
constructor: MockCodemirrorNamespace,
};