Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not show vSorterInfo as completion item and show correct diagnostic #741

Merged
merged 7 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/quick-bobcats-doubt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@ui5-language-assistant/vscode-ui5-language-assistant-bas-ext": patch
"vscode-ui5-language-assistant": patch
"@ui5-language-assistant/binding": patch
---

fix: do not show vSorterInfo as completion item and show correct diagnostic
97 changes: 66 additions & 31 deletions packages/binding/src/definition/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { getSorterPossibleElement } from "./sorter";
import { getFiltersPossibleElement } from "./filter";
import type {
UI5Aggregation,
UI5ConstructorParameters,
UI5TypedefProp,
} from "@ui5-language-assistant/semantic-model-types";

Expand Down Expand Up @@ -111,6 +112,55 @@ const getReference = (type: UI5Type) => {
return reference;
};

function getParameterProperties(param: {
context: BindContext;
parameterProperties: UI5ConstructorParameters[];
ui5Aggregation?: UI5Aggregation;
forHover?: boolean;
type: UI5Class;
}): BindingInfoElement[] {
const result: BindingInfoElement[] = [];
const {
ui5Aggregation,
forHover = false,
type,
context,
parameterProperties,
} = param;
for (const param of parameterProperties) {
if (!param.type) {
continue;
}
// add reference to type and avoid recursion
const paramType = param.type;
const reference = getReference(paramType);
const data: BindingInfoElement = {
name: param.name,
// eslint-disable-next-line @typescript-eslint/no-use-before-define
type: buildType({
context,
type: paramType,
name: param.name,
collection: false,
ui5Aggregation,
forHover,
reference,
}),
documentation: getDocumentation({
context,
prop: param,
FQN: ui5NodeToFQN(type),
titlePrefix: "(class)",
forHover,
}),
};
data.required = !param.optional;
result.push(data);
}

return result;
}

/**
* Currently [api.json](https://ui5.sap.com/1.118.1/test-resources/sap/ui/core/designtime/api.json) provides these constructor parameters as old school convention
* e.g `sPath` for `path` where `s` stands for string type. These params are [map in runtime](https://github.com/SAP/openui5/blob/master/src/sap.ui.core/src/sap/ui/model/Sorter.js#L54-L60).
Expand Down Expand Up @@ -144,8 +194,17 @@ const getPossibleElement = (param: {
if (!constParam.type) {
continue;
}
const reference = getReference(constParam.type);
const name = sorterMap.get(constParam.name) ?? constParam.name;
if (name === "vSorterInfo" && constParam.parameterProperties) {
result.push(
...getParameterProperties({
...param,
parameterProperties: constParam.parameterProperties,
})
);
continue;
}
const reference = getReference(constParam.type);
const data: BindingInfoElement = {
name,
// eslint-disable-next-line @typescript-eslint/no-use-before-define
Expand Down Expand Up @@ -179,36 +238,12 @@ const getPossibleElement = (param: {
// use fallback filter
return getFiltersPossibleElement();
}
for (const param of vFilter.parameterProperties) {
if (!param.type) {
continue;
}
// add reference to type and avoid recursion
const paramType = param.type;
const reference = getReference(paramType);
const data: BindingInfoElement = {
name: param.name,
// eslint-disable-next-line @typescript-eslint/no-use-before-define
type: buildType({
context,
type: paramType,
name: param.name,
collection: false,
ui5Aggregation,
forHover,
reference,
}),
documentation: getDocumentation({
context,
prop: param,
FQN: ui5NodeToFQN(type),
titlePrefix: "(class)",
forHover,
}),
};
data.required = !param.optional;
result.push(data);
}
result.push(
...getParameterProperties({
...param,
parameterProperties: vFilter.parameterProperties,
})
);
}
return result;
};
Expand Down
8 changes: 2 additions & 6 deletions packages/binding/src/services/completion/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,10 @@ export function getCompletionItems(opts: {
attributeValue: attributeValueProviders,
},
});
const uniqueSuggestion = [
...new Set(suggestions.map((i) => JSON.stringify(i))),
].map((i) => JSON.parse(i));

getLogger().trace("computed completion items", {
uniqueSuggestion,
suggestions,
});
return uniqueSuggestion;
return suggestions;
} catch (error) {
getLogger().debug("getCompletionItems failed:", error);
return [];
Expand Down
13 changes: 12 additions & 1 deletion packages/binding/src/services/completion/providers/binding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { BindContext } from "../../../types";
import { createInitialSnippet } from "./create-initial-snippet";
import {
getAltTypesPrime,
getCursorContext,
getLogger,
isMacrosMetaContextPath,
Expand Down Expand Up @@ -138,7 +139,17 @@ export function bindingSuggestions({
)
);
}
return completionItems;

const altTypes = getAltTypesPrime(ui5Aggregation);
if (altTypes) {
// for `altTypes`, `PROPERTY_BINDING_INFO` properties are added (duplicate allowed)
return completionItems;
}
// Remove duplicates
const uniqueCompletionItems = Array.from(
new Map(completionItems.map((item) => [item.label, item])).values()
);
return uniqueCompletionItems;
} catch (error) {
getLogger().debug("bindingSuggestions failed:", error);
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
ViewCompletionProviderType,
} from "../../helper";
import { initI18n } from "../../../../src/api";
import { readFile, writeFile } from "fs/promises";

describe("aggregation binding", () => {
let getCompletionResult: ViewCompletionProviderType;
Expand Down Expand Up @@ -105,6 +106,24 @@ describe("aggregation binding", () => {
const result = await getCompletionResult(snippet);
expect(result).toMatchSnapshot();
});

it("do not show `vSorterInfo`. Some UI5 version e.g. 1.131.0 has `vSorterInfo` as param of constructor ", async function () {
// adapt manifest.json file
const manifestPath = join(
root,
"app",
"manage_travels",
"webapp",
"manifest.json"
);
const manifest = JSON.parse(await readFile(manifestPath, "utf-8"));
manifest["sap.ui5"]["dependencies"]["minUI5Version"] = "1.131.0";
await writeFile(manifestPath, JSON.stringify(manifest, null, 2));
const snippet = `
<List items="{sorter: {${CURSOR_ANCHOR}} }"> </List>`;
const result = await getCompletionResult(snippet);
expect(result.find((i) => i.label === "vSorterInfo")).toBeUndefined();
});
});
describe("filters", function () {
it("initial", async function () {
Expand Down
Loading