Skip to content

Commit 1a1115e

Browse files
marufrasullymarufrasully
authored
fix: enhance check for binding expression (#653)
* fix: enhance check for binding expression * fix: change set and fallback for hover * fix: add support for ui5object
1 parent fffeff2 commit 1a1115e

File tree

19 files changed

+312
-43
lines changed

19 files changed

+312
-43
lines changed

‎.changeset/quiet-stingrays-hug.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"vscode-ui5-language-assistant": patch
3+
"@ui5-language-assistant/binding-parser": patch
4+
"@ui5-language-assistant/binding": patch
5+
"@ui5-language-assistant/vscode-ui5-language-assistant-bas-ext": patch
6+
---
7+
8+
fix: check binding expression only if it has at least one known property

‎packages/binding-parser/src/utils/expression.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { isPrimitiveValue } from "../api";
12
import { ExtractBindingSyntax } from "../types";
23
import type {
34
ParseResultErrors,
@@ -97,7 +98,7 @@ export const isMetadataPath = (
9798
*
9899
* a. is empty curly bracket e.g `{}` or `{ }`
99100
*
100-
* b. has starting or closing curly bracket and key property with colon e.g `{anyKey: }` or `{"anyKey":}` or `{'anyKey':}`
101+
* b. has starting or closing curly bracket and known properties with colon e.g `{anyKey: }` or `{"anyKey":}` or `{'anyKey':}`
101102
*
102103
* c. empty string [for initial code completion snippet]
103104
*
@@ -107,8 +108,9 @@ export const isMetadataPath = (
107108
*/
108109
export const isBindingAllowed = (
109110
input: string,
110-
binding?: StructureValue,
111-
errors?: ParseResultErrors
111+
binding: StructureValue,
112+
errors: ParseResultErrors,
113+
properties: string[]
112114
): boolean => {
113115
// check empty string
114116
if (input.trim().length === 0) {
@@ -137,10 +139,22 @@ export const isBindingAllowed = (
137139
// check empty curly brackets
138140
return true;
139141
}
142+
// check if `ui5object` has a truthy value.
143+
const ui5Obj = binding.elements.find((i) => i.key?.text === "ui5object");
144+
if (ui5Obj && isPrimitiveValue(ui5Obj.value)) {
145+
// if truthy value [not false value], it is not a binding expression
146+
if (!["null", `''`, `""`, "0", "false"].includes(ui5Obj.value.text)) {
147+
return false;
148+
}
149+
}
150+
// check if only `ui5object` - show code completion
151+
if (ui5Obj && binding.elements.length === 1) {
152+
return true;
153+
}
140154
// check it has at least one key with colon
141155
const result = binding.elements.find(
142156
/* istanbul ignore next */
143-
(item) => item.key?.text && item.colon?.text
157+
(item) => properties.find((p) => p === item.key?.text) && item.colon?.text
144158
);
145159
if (result && binding.leftCurly && binding.leftCurly.text) {
146160
return true;

‎packages/binding-parser/test/unit/utils/expression.test.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -213,61 +213,64 @@ describe("expression", () => {
213213
it("empty string", () => {
214214
const input = " ";
215215
const { ast, errors } = parseBinding(input);
216-
const result = isBindingAllowed(input, ast.bindings[0], errors);
216+
const result = isBindingAllowed(input, ast.bindings[0], errors, []);
217217
expect(result).toBeTrue();
218218
});
219219
it("string value", () => {
220220
const input = "40";
221221
const { ast, errors } = parseBinding(input);
222-
const result = isBindingAllowed(input, ast.bindings[0], errors);
222+
const result = isBindingAllowed(input, ast.bindings[0], errors, []);
223223
expect(result).toBeFalse();
224224
});
225225
it("empty curly bracket without space", () => {
226226
const input = "{}";
227227
const { ast, errors } = parseBinding(input);
228-
const result = isBindingAllowed(input, ast.bindings[0], errors);
228+
const result = isBindingAllowed(input, ast.bindings[0], errors, []);
229229
expect(result).toBeTrue();
230230
});
231231
it("empty curly bracket with space", () => {
232232
const input = "{ }";
233233
const { ast, errors } = parseBinding(input);
234-
const result = isBindingAllowed(input, ast.bindings[0], errors);
234+
const result = isBindingAllowed(input, ast.bindings[0], errors, []);
235235
expect(result).toBeTrue();
236236
});
237-
it("key with colone [true]", () => {
237+
it("property binding info key with colon [true]", () => {
238238
const input = ' {path: "some/path"}';
239239
const { ast, errors } = parseBinding(input);
240-
const result = isBindingAllowed(input, ast.bindings[0], errors);
240+
const result = isBindingAllowed(input, ast.bindings[0], errors, ["path"]);
241241
expect(result).toBeTrue();
242242
});
243-
it("key with colone any where [true]", () => {
244-
const input = ' {path "some/path", thisKey: {}}';
243+
it("property binding info key with colon any where [true]", () => {
244+
const input = ' {path "some/path", event: {}}';
245245
const { ast, errors } = parseBinding(input);
246-
const result = isBindingAllowed(input, ast.bindings[0], errors);
246+
const result = isBindingAllowed(input, ast.bindings[0], errors, [
247+
"path",
248+
"event",
249+
]);
247250
expect(result).toBeTrue();
248251
});
249252
it("missing colon [false]", () => {
250253
const input = '{path "some/path"}';
251254
const { ast, errors } = parseBinding(input);
252-
const result = isBindingAllowed(input, ast.bindings[0], errors);
255+
const result = isBindingAllowed(input, ast.bindings[0], errors, ["path"]);
253256
expect(result).toBeFalse();
254257
});
255258
it("contains > after first key [false]", () => {
256259
const input = "{i18n>myTestModel}";
257260
const { ast, errors } = parseBinding(input);
258-
const result = isBindingAllowed(input, ast.bindings[0], errors);
261+
const result = isBindingAllowed(input, ast.bindings[0], errors, ["path"]);
259262
expect(result).toBeFalse();
260263
});
261264
it("contains / before first key [false]", () => {
262265
const input = "{/oData/path/to/some/dynamic/value}";
263266
const { ast, errors } = parseBinding(input);
264-
const result = isBindingAllowed(input, ast.bindings[0], errors);
267+
const result = isBindingAllowed(input, ast.bindings[0], errors, ["path"]);
265268
expect(result).toBeFalse();
266269
});
267270
it("contains / after first key [false]", () => {
268271
const input = "{/oData/path/to/some/dynamic/value}";
269272
const { ast, errors } = parseBinding(input);
270-
const result = isBindingAllowed(input, ast.bindings[0], errors);
273+
const result = isBindingAllowed(input, ast.bindings[0], errors, ["path"]);
271274
expect(result).toBeFalse();
272275
});
273276
});

‎packages/binding/src/api.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,5 @@ export function isBindingIssue<T extends { issueType: string }>(
1111
): issue is BindingIssue {
1212
return issue.issueType === BINDING_ISSUE_TYPE;
1313
}
14+
15+
export { AGGREGATION_BINDING_INFO, PROPERTY_BINDING_INFO } from "./constant";

‎packages/binding/src/constant.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
export const BINDING_ISSUE_TYPE = "binding-issue";
22
export const PROPERTY_BINDING_INFO =
33
"sap.ui.base.ManagedObject.PropertyBindingInfo";
4+
export const AGGREGATION_BINDING_INFO =
5+
"sap.ui.base.ManagedObject.AggregationBindingInfo";
46
export const Binding_Mode = "sap.ui.model.BindingMode";

‎packages/binding/src/definition/definition.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ const buildType = (
154154
};
155155

156156
export const getPropertyBindingInfoElements = (
157-
context: BindContext
157+
context: BindContext,
158+
forHover = false
158159
): PropertyBindingInfoElement[] => {
159160
const elements: PropertyBindingInfoElement[] = [];
160161
const propBinding = context.ui5Model.typedefs[PROPERTY_BINDING_INFO];
@@ -192,7 +193,7 @@ export const getPropertyBindingInfoElements = (
192193
elements.push({
193194
name: name,
194195
type: builtType,
195-
documentation: getDocumentation(context, property),
196+
documentation: getDocumentation(context, property, forHover),
196197
});
197198
}
198199
return elements;

‎packages/binding/src/services/completion/providers/property-binding-info.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { createAllSupportedElements } from "./create-all-supported-elements";
1818
import { createKeyProperties } from "./create-key-properties";
1919
import { createValue } from "./create-value";
2020
import { createKeyValue } from "./create-key-value";
21+
import { PROPERTY_BINDING_INFO } from "./../../../constant";
2122

2223
export const getCompletionItems = (
2324
context: BindContext,
@@ -62,6 +63,9 @@ export function propertyBindingInfoSuggestions({
6263
if (!ui5Property) {
6364
return completionItems;
6465
}
66+
const propBinding = context.ui5Model.typedefs[PROPERTY_BINDING_INFO];
67+
/* istanbul ignore next */
68+
const properties = propBinding?.properties?.map((i) => i.name) ?? [];
6569
const value = attribute.syntax.value;
6670
const startChar = value && value.image.charAt(0);
6771
context.doubleQuotes = startChar === '"';
@@ -94,7 +98,7 @@ export function propertyBindingInfoSuggestions({
9498
if (!binding) {
9599
continue;
96100
}
97-
if (!isBindingAllowed(text, binding, errors)) {
101+
if (!isBindingAllowed(text, binding, errors, properties)) {
98102
continue;
99103
}
100104
completionItems.push(...getCompletionItems(context, binding, ast.spaces));

‎packages/binding/src/services/diagnostics/validators/check-key.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ export const checkKey = (
2424
const bindingElement = getPropertyBindingInfoElements(context).find(
2525
(el) => el.name === text
2626
);
27+
if (text === "ui5object") {
28+
return issues;
29+
}
2730
if (!bindingElement) {
2831
issues.push({
2932
issueType: BINDING_ISSUE_TYPE,

‎packages/binding/src/services/diagnostics/validators/property-binding-info-validator.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { XMLAttribute } from "@xml-tools/ast";
22
import { Context } from "@ui5-language-assistant/context";
33
import { getUI5PropertyByXMLAttributeKey } from "@ui5-language-assistant/logic-utils";
44
import { BindingIssue } from "../../../types";
5-
import { BINDING_ISSUE_TYPE } from "../../../constant";
5+
import { BINDING_ISSUE_TYPE, PROPERTY_BINDING_INFO } from "../../../constant";
66
import { getLogger } from "../../../utils";
77
import { Position } from "vscode-languageserver-types";
88
import {
@@ -27,7 +27,8 @@ export function validatePropertyBindingInfo(
2727
if (!ui5Property) {
2828
return [];
2929
}
30-
30+
const propBinding = context.ui5Model.typedefs[PROPERTY_BINDING_INFO];
31+
const properties = propBinding.properties.map((i) => i.name);
3132
const value = attribute.syntax.value;
3233
const text = attribute.value ?? "";
3334
const startChar = value?.image.charAt(0);
@@ -49,7 +50,7 @@ export function validatePropertyBindingInfo(
4950
};
5051
const { ast, errors } = parseBinding(expression, position);
5152
for (const binding of ast.bindings) {
52-
if (!isBindingAllowed(expression, binding, errors)) {
53+
if (!isBindingAllowed(expression, binding, errors, properties)) {
5354
continue;
5455
}
5556

‎packages/binding/src/services/hover/index.ts

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,12 @@ import {
1111
BindingParserTypes,
1212
} from "@ui5-language-assistant/binding-parser";
1313
import { Position } from "vscode-languageserver-types";
14-
import { BindContext } from "../../types";
15-
import { PROPERTY_BINDING_INFO } from "../../constant";
16-
import { getDocumentation } from "../../utils";
17-
import { UI5Typedef } from "@ui5-language-assistant/semantic-model-types";
14+
import { BindContext, PropertyBindingInfoElement } from "../../types";
15+
import { getPropertyBindingInfoElements } from "../../definition/definition";
1816

1917
const getHoverFromBinding = (
2018
context: BindContext,
21-
propertyBinding: UI5Typedef,
19+
propertyBinding: PropertyBindingInfoElement[],
2220
binding: BindingParserTypes.StructureValue
2321
): Hover | undefined => {
2422
let hover: Hover | undefined;
@@ -43,11 +41,11 @@ const getHoverFromBinding = (
4341
})
4442
) {
4543
// check valid key
46-
const property = propertyBinding.properties.find(
44+
const property = propertyBinding.find(
4745
(prop) => prop.name === element.key?.originalText
4846
);
4947
if (property) {
50-
return { contents: getDocumentation(context, property, true) };
48+
return { contents: property.documentation };
5149
}
5250
}
5351

@@ -73,10 +71,6 @@ export const getHover = (
7371
attribute: XMLAttribute
7472
): Hover | undefined => {
7573
try {
76-
const propBinding = context.ui5Model.typedefs[PROPERTY_BINDING_INFO];
77-
if (!propBinding) {
78-
return;
79-
}
8074
const ui5Property = getUI5PropertyByXMLAttributeKey(
8175
attribute,
8276
context.ui5Model
@@ -90,7 +84,8 @@ export const getHover = (
9084
if (text.trim().length === 0) {
9185
return;
9286
}
93-
87+
const propBinding = getPropertyBindingInfoElements(context, true);
88+
const properties = propBinding.map((i) => i.name);
9489
const extractedText = extractBindingSyntax(text);
9590
for (const bindingSyntax of extractedText) {
9691
const { expression, startIndex } = bindingSyntax;
@@ -114,7 +109,7 @@ export const getHover = (
114109
if (!binding) {
115110
continue;
116111
}
117-
if (!isBindingAllowed(text, binding, errors)) {
112+
if (!isBindingAllowed(text, binding, errors, properties)) {
118113
continue;
119114
}
120115
return getHoverFromBinding(context, propBinding, binding);

‎packages/binding/test/unit/services/completion/__snapshots__/index.test.ts.snap

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,29 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`index getCompletionItems multiple binding check if ui5object has false value: '': '' 1`] = `
4+
Array [
5+
"label: ''; text: '$0'; kind:5; commit:undefined; sort:",
6+
]
7+
`;
8+
9+
exports[`index getCompletionItems multiple binding check if ui5object has false value: 0: 0 1`] = `
10+
Array [
11+
"label: ''; text: '$0'; kind:5; commit:undefined; sort:",
12+
]
13+
`;
14+
15+
exports[`index getCompletionItems multiple binding check if ui5object has false value: false: false 1`] = `
16+
Array [
17+
"label: ''; text: '$0'; kind:5; commit:undefined; sort:",
18+
]
19+
`;
20+
21+
exports[`index getCompletionItems multiple binding check if ui5object has false value: null: null 1`] = `
22+
Array [
23+
"label: ''; text: '$0'; kind:5; commit:undefined; sort:",
24+
]
25+
`;
26+
327
exports[`index getCompletionItems provides CC for empty [with documentation] 1`] = `
428
Array [
529
"label: path; text: path: '$0'; kind:15; commit:undefined; sort:; documentation: kind:markdown,value:\`(typedef) sap.ui.base.ManagedObject.PropertyBindingInfo\`

‎packages/binding/test/unit/services/completion/index.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,35 @@ describe("index", () => {
708708
"label: {}; text: {$0}; kind:5; commit:undefined; sort:",
709709
]);
710710
});
711+
it("no CC for ui5 property which does not contain any property binding info key", async () => {
712+
const snippet = `
713+
<core:ComponentContainer id="test-id" name="a.b.name" url="/a/b"
714+
settings='{${CURSOR_ANCHOR} AppMode: false, WidgetStyleClass: "ab"}' componentCreated=".extension.customer.a.b">
715+
</core:ComponentContainer>`;
716+
const result = await getCompletionResult(snippet);
717+
expect(
718+
result.map((item) => completionItemToSnapshot(item))
719+
).toStrictEqual([]);
720+
});
721+
it("no CC if ui5object has truthy value", async () => {
722+
const snippet = `
723+
<Text text="{ui5object: true, path:${CURSOR_ANCHOR}}" id="test-id"></Text>`;
724+
const result = await getCompletionResult(snippet);
725+
expect(
726+
result.map((item) => completionItemToSnapshot(item))
727+
).toStrictEqual([]);
728+
});
729+
730+
["null", `''`, "0", "false"].forEach((value) => {
731+
it(`check if ui5object has false value: ${value}`, async () => {
732+
const snippet = `
733+
<Text text="{ui5object: ${value}, path:${CURSOR_ANCHOR} }" id="test-id"></Text>`;
734+
const result = await getCompletionResult(snippet);
735+
expect(
736+
result.map((item) => completionItemToSnapshot(item))
737+
).toMatchSnapshot(value);
738+
});
739+
});
711740
});
712741
});
713742
});

‎packages/binding/test/unit/services/diagnostics/validators/__snapshots__/property-binding-info-validator.test.ts.snap

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`property-binding-info-validator check if ui5object has false value: '': '' 1`] = `
4+
Array [
5+
"kind: MissingValue; text: Expect '' as a value; severity:error; range:9:38-9:43",
6+
]
7+
`;
8+
9+
exports[`property-binding-info-validator check if ui5object has false value: 0: 0 1`] = `
10+
Array [
11+
"kind: MissingValue; text: Expect '' as a value; severity:error; range:9:37-9:42",
12+
]
13+
`;
14+
15+
exports[`property-binding-info-validator check if ui5object has false value: false: false 1`] = `
16+
Array [
17+
"kind: MissingValue; text: Expect '' as a value; severity:error; range:9:41-9:46",
18+
]
19+
`;
20+
21+
exports[`property-binding-info-validator check if ui5object has false value: null: null 1`] = `
22+
Array [
23+
"kind: MissingValue; text: Expect '' as a value; severity:error; range:9:40-9:45",
24+
]
25+
`;

0 commit comments

Comments
 (0)