diff --git a/src/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts b/src/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts index 143a4022e..551cecbcf 100644 --- a/src/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts +++ b/src/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts @@ -4,14 +4,14 @@ export class ControllerByIdDtsGenerator { // Maps module names to local names private imports = new Map(); - 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(); + 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); @@ -19,7 +19,7 @@ export class ControllerByIdDtsGenerator { return this.generateCollectedImports() + out; } - generateCollectedImports() { + private generateCollectedImports() { let out = ""; this.imports.forEach((localName, moduleName) => { out += `import ${localName} from "${moduleName}";\n`; @@ -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(sId: T): ByIdMapping[T];\n`; - out += `\t\tbyId(sId: string): UI5Element;\n`; + out += `\ttype ByIdFunction = {\n`; + out += `\t\t(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, "_"); } } diff --git a/src/linter/xmlTemplate/linter.ts b/src/linter/xmlTemplate/linter.ts index 350b2254e..251bb6395 100644 --- a/src/linter/xmlTemplate/linter.ts +++ b/src/linter/xmlTemplate/linter.ts @@ -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", diff --git a/test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/controller/Main.controller.js b/test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/controller/Main.controller.js index 5dbb0849e..846c542be 100644 --- a/test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/controller/Main.controller.js +++ b/test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/controller/Main.controller.js @@ -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"); } }); }); diff --git a/test/lib/linter/snapshots/linter.ts.md b/test/lib/linter/snapshots/linter.ts.md index a8bed9ec5..0c04ca774 100644 --- a/test/lib/linter/snapshots/linter.ts.md +++ b/test/lib/linter/snapshots/linter.ts.md @@ -414,23 +414,22 @@ 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', @@ -438,12 +437,36 @@ Generated by [AVA](https://avajs.dev). }, { 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, }, @@ -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, }, @@ -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 @@ -2548,23 +2622,22 @@ 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', @@ -2572,12 +2645,36 @@ Generated by [AVA](https://avajs.dev). }, { 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, }, diff --git a/test/lib/linter/snapshots/linter.ts.snap b/test/lib/linter/snapshots/linter.ts.snap index ffffec994..17bb86a56 100644 Binary files a/test/lib/linter/snapshots/linter.ts.snap and b/test/lib/linter/snapshots/linter.ts.snap differ diff --git a/test/lib/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts b/test/lib/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts index 38a299e43..667cc704c 100644 --- a/test/lib/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts +++ b/test/lib/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts @@ -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); }); diff --git a/test/lib/linter/xmlTemplate/generator/snapshots/ControllerByIdDtsGenerator.ts.md b/test/lib/linter/xmlTemplate/generator/snapshots/ControllerByIdDtsGenerator.ts.md index 18461b321..490bc9d61 100644 --- a/test/lib/linter/xmlTemplate/generator/snapshots/ControllerByIdDtsGenerator.ts.md +++ b/test/lib/linter/xmlTemplate/generator/snapshots/ControllerByIdDtsGenerator.ts.md @@ -8,7 +8,9 @@ Generated by [AVA](https://avajs.dev). > Snapshot 1 - `import sap_m_Button from "sap/m/Button";␊ + `import sap_ui_core_mvc_View from "sap/ui/core/mvc/View";␊ + import sap_ui_core_Element from "sap/ui/core/Element";␊ + import sap_m_Button from "sap/m/Button";␊ import sap_ui_commons_Button from "sap/ui/commons/Button";␊ import sap_m_Input from "sap/m/Input";␊ ␊ @@ -17,9 +19,16 @@ Generated by [AVA](https://avajs.dev). "button": sap_m_Button | sap_ui_commons_Button;␊ "input": sap_m_Input;␊ }␊ - export default interface App {␊ - byId(sId: T): ByIdMapping[T];␊ - byId(sId: string): UI5Element;␊ + type ByIdFunction = {␊ + (sId: T): ByIdMapping[T];␊ + (sId: string): sap_ui_core_Element;␊ + };␊ + interface ControllerView extends sap_ui_core_mvc_View {␊ + byId: ByIdFunction;␊ + };␊ + export default interface Controller {␊ + byId: ByIdFunction;␊ + getView(): ControllerView;␊ };␊ };␊ ␊ @@ -28,9 +37,16 @@ Generated by [AVA](https://avajs.dev). "button": sap_m_Button;␊ "input": sap_m_Input;␊ }␊ - export default interface Main {␊ - byId(sId: T): ByIdMapping[T];␊ - byId(sId: string): UI5Element;␊ + type ByIdFunction = {␊ + (sId: T): ByIdMapping[T];␊ + (sId: string): sap_ui_core_Element;␊ + };␊ + interface ControllerView extends sap_ui_core_mvc_View {␊ + byId: ByIdFunction;␊ + };␊ + export default interface Controller {␊ + byId: ByIdFunction;␊ + getView(): ControllerView;␊ };␊ };␊ ␊ diff --git a/test/lib/linter/xmlTemplate/generator/snapshots/ControllerByIdDtsGenerator.ts.snap b/test/lib/linter/xmlTemplate/generator/snapshots/ControllerByIdDtsGenerator.ts.snap index 354cf96ad..a55c8748f 100644 Binary files a/test/lib/linter/xmlTemplate/generator/snapshots/ControllerByIdDtsGenerator.ts.snap and b/test/lib/linter/xmlTemplate/generator/snapshots/ControllerByIdDtsGenerator.ts.snap differ diff --git a/test/lib/snapshots/index.ts.md b/test/lib/snapshots/index.ts.md index 2f18ac899..3812cd7af 100644 --- a/test/lib/snapshots/index.ts.md +++ b/test/lib/snapshots/index.ts.md @@ -57,24 +57,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, }, @@ -426,24 +454,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, }, @@ -1399,24 +1455,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, }, @@ -1768,24 +1852,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, }, @@ -2137,24 +2249,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, }, diff --git a/test/lib/snapshots/index.ts.snap b/test/lib/snapshots/index.ts.snap index 8fa330b48..69dbdff36 100644 Binary files a/test/lib/snapshots/index.ts.snap and b/test/lib/snapshots/index.ts.snap differ