Skip to content

Commit

Permalink
fix: Support getView().byId / fix fallback signature
Browse files Browse the repository at this point in the history
  • Loading branch information
@matz3
matz3 committed Nov 25, 2024
1 parent dfba99a commit 2542d52
Show file tree
Hide file tree
Showing 10 changed files with 353 additions and 84 deletions.
52 changes: 31 additions & 21 deletions src/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,22 @@ export class ControllerByIdDtsGenerator {
// Maps module names to local names
private imports = new Map<string, string>();

constructor(private controllerByIdInfo: ControllerByIdInfo) {
}

generate() {
const mappings = this.controllerByIdInfo.getMappings();
generate(controllerByIdInfo: ControllerByIdInfo) {
const mappings = controllerByIdInfo.getMappings();
if (mappings.size === 0) {
return null;
}
this.imports = new Map<string, string>();
this.addImport("sap/ui/core/mvc/View"); // View is needed for interface ControllerView
this.addImport("sap/ui/core/Element"); // Element is needed for byId fallback signature
let out = "";
mappings.forEach((idToModules, controllerName) => {
out += this.generateModuleDeclaration(controllerName, idToModules);
});
return this.generateCollectedImports() + out;
}

generateCollectedImports() {
private generateCollectedImports() {
let out = "";
this.imports.forEach((localName, moduleName) => {
out += `import ${localName} from "${moduleName}";\n`;
Expand All @@ -28,38 +28,48 @@ export class ControllerByIdDtsGenerator {
return out;
}

generateByIdMapping(idToModules: IdModulesMap) {
private generateByIdMapping(idToModules: IdModulesMap) {
let out = "\tinterface ByIdMapping {\n";
idToModules.forEach((modules, id) => {
const localNames: string[] = [];
modules.forEach((moduleName) => {
const localName = this.getLocalModuleName(moduleName);
localNames.push(localName);
if (!this.imports.has(moduleName)) {
this.imports.set(moduleName, localName);
}
});
modules.forEach((moduleName) => localNames.push(this.addImport(moduleName)));
out += `\t\t"${id}": ${localNames.join(" | ")};\n`;
});
out += "\t}\n";
return out;
}

generateModuleDeclaration(controllerName: string, idToModules: IdModulesMap) {
private generateModuleDeclaration(controllerName: string, idToModules: IdModulesMap) {
const moduleName = controllerName.replace(/\./g, "/") + ".controller";
// The interface name actually does not really matter as the declaration refers to the default export
const controllerClassName = controllerName.split(".").pop();

let out = `declare module "${moduleName}" {\n`;
out += this.generateByIdMapping(idToModules);
out += `\texport default interface ${controllerClassName} {\n`;
out += `\t\tbyId<T extends keyof ByIdMapping>(sId: T): ByIdMapping[T];\n`;
out += `\t\tbyId(sId: string): UI5Element;\n`;
out += `\ttype ByIdFunction = {\n`;
out += `\t\t<T extends keyof ByIdMapping>(sId: T): ByIdMapping[T];\n`;
out += `\t\t(sId: string): sap_ui_core_Element;\n`; // Fallback signature for unknown IDs
out += `\t};\n`;
out += `\tinterface ControllerView extends sap_ui_core_mvc_View {\n`;
out += `\t\tbyId: ByIdFunction;\n`;
out += `\t};\n`;
// The interface name does not matter as the declaration refers to the default export.
// To avoid name clashes we are just using "Controller" here.
out += `\texport default interface Controller {\n`;
out += `\t\tbyId: ByIdFunction;\n`;
out += `\t\tgetView(): ControllerView;\n`;
out += `\t};\n`;
out += `};\n\n`;
return out;
}

getLocalModuleName(moduleName: string) {
private addImport(moduleName: string) {
const localName = this.getLocalModuleName(moduleName);
if (!this.imports.has(moduleName)) {
this.imports.set(moduleName, localName);
}
return localName;
}

private getLocalModuleName(moduleName: string) {
return moduleName.replace(/[/.]/g, "_");
}
}
4 changes: 2 additions & 2 deletions src/linter/xmlTemplate/linter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export default async function lintXml({filePathsWorkspace, workspace, context}:
}));

// Generate dts file with specific byId signatures for controllers based on view IDs
const controllerByIdDtsGenerator = new ControllerByIdDtsGenerator(controllerByIdInfo);
const controllerByIdDts = controllerByIdDtsGenerator.generate();
const controllerByIdDtsGenerator = new ControllerByIdDtsGenerator();
const controllerByIdDts = controllerByIdDtsGenerator.generate(controllerByIdInfo);
if (controllerByIdDts) {
const dtsResource = createResource({
path: "/types/@ui5/linter/virtual/ControllerById.d.ts",
Expand Down
18 changes: 12 additions & 6 deletions test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/controller/Main.controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,29 @@ sap.ui.define(["./BaseController", "sap/m/MessageBox"], function (BaseController
},

registerButtonEventHandlers() {
// this.byId and this.getView().byId should report the same issues
this.byId("helloButton").attachTap(function() {
console.log("Tapped");
});
this.getView().byId("helloButton").attachTap(function() {
console.log("Tapped");
});

// testButton exists in two views and could be a sap.m.Button or a sap.ui.commons.Button.
// For this reason, the detection of deprecated button API does not work
// but for base control API it should still work.
// The detection of deprecated button API depends requires TypeScript compliant probing (e.g. using "attachTap" in testButton).
// In any case, the detection of UI5 Base Control API should still work as both inherit from it.
const testButton = this.byId("testButton");

testButton.getBlocked(); // This should be reported

// This is currently not reported as TypeScript does not know the exact type of testButton
if (testButton.attachTap) {
if ("attachTap" in testButton) {
// When probing for the existence of the method, the type can be determined and the issue should be reported
testButton.attachTap(function() {
console.log("Tapped");
});
}

// UI5 Element API should still be checked for unknown IDs
this.byId("unknown").prop("foo", "bar");
this.getView().byId("unknown").prop("foo", "bar");
}
});
});
155 changes: 126 additions & 29 deletions test/lib/linter/snapshots/linter.ts.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,36 +414,59 @@ Generated by [AVA](https://avajs.dev).
warningCount: 0,
},
{
coverageInfo: [
{
category: 1,
column: 5,
line: 23,
message: `Unable to analyze this method call because the type of identifier "attachTap" in "testButton.attachTap(function () {␊
console.log("Tapped");␊
})"" could not be determined`,
},
],
errorCount: 2,
coverageInfo: [],
errorCount: 6,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
{
column: 29,
line: 10,
line: 11,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 39,
line: 14,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 15,
line: 19,
line: 22,
message: 'Call to deprecated function \'getBlocked\' (testButton.getBlocked)',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 16,
line: 25,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 25,
line: 31,
message: 'Call to deprecated function \'prop\' of class \'UI5Element\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 35,
line: 32,
message: 'Call to deprecated function \'prop\' of class \'UI5Element\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down Expand Up @@ -1124,24 +1147,52 @@ Generated by [AVA](https://avajs.dev).
},
{
coverageInfo: [],
errorCount: 2,
errorCount: 6,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
{
column: 29,
line: 10,
line: 11,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 39,
line: 14,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 15,
line: 19,
line: 22,
message: 'Call to deprecated function \'getBlocked\' (testButton.getBlocked)',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 16,
line: 25,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 25,
line: 31,
message: 'Call to deprecated function \'prop\' of class \'UI5Element\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 35,
line: 32,
message: 'Call to deprecated function \'prop\' of class \'UI5Element\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down Expand Up @@ -1537,6 +1588,29 @@ Generated by [AVA](https://avajs.dev).
],
warningCount: 0,
},
{
coverageInfo: [],
errorCount: 2,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
{
column: 25,
line: 31,
message: 'Call to deprecated function \'prop\' of class \'UI5Element\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 35,
line: 32,
message: 'Call to deprecated function \'prop\' of class \'UI5Element\'',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
]

## lint: All files of library.with.custom.paths
Expand Down Expand Up @@ -2548,36 +2622,59 @@ Generated by [AVA](https://avajs.dev).
warningCount: 0,
},
{
coverageInfo: [
{
category: 1,
column: 5,
line: 23,
message: `Unable to analyze this method call because the type of identifier "attachTap" in "testButton.attachTap(function () {␊
console.log("Tapped");␊
})"" could not be determined`,
},
],
errorCount: 2,
coverageInfo: [],
errorCount: 6,
fatalErrorCount: 0,
filePath: 'webapp/controller/Main.controller.js',
messages: [
{
column: 29,
line: 10,
line: 11,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 39,
line: 14,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 15,
line: 19,
line: 22,
message: 'Call to deprecated function \'getBlocked\' (testButton.getBlocked)',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 16,
line: 25,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 25,
line: 31,
message: 'Call to deprecated function \'prop\' of class \'UI5Element\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 35,
line: 32,
message: 'Call to deprecated function \'prop\' of class \'UI5Element\'',
messageDetails: 'Deprecated test message',
ruleId: 'no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
Expand Down
Binary file modified test/lib/linter/snapshots/linter.ts.snap
View file
Binary file not shown.
8 changes: 4 additions & 4 deletions test/lib/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ test("ControllerByIdDtsGenerator: generate", (t) => {
["button", "sap/ui/commons/Button"],
]));

const generator = new ControllerByIdDtsGenerator(controllerByIdInfo);
const result = generator.generate();
const generator = new ControllerByIdDtsGenerator();
const result = generator.generate(controllerByIdInfo);
t.snapshot(result);
});

test("ControllerByIdDtsGenerator: generate with empty info should return null", (t) => {
const controllerByIdInfo = new ControllerByIdInfo();
const generator = new ControllerByIdDtsGenerator(controllerByIdInfo);
const result = generator.generate();
const generator = new ControllerByIdDtsGenerator();
const result = generator.generate(controllerByIdInfo);
t.is(result, null);
});
Loading

0 comments on commit 2542d52

Please sign in to comment.