diff --git a/src/linter/ui5Types/TypeLinter.ts b/src/linter/ui5Types/TypeLinter.ts index bc1405c5a..5250e493c 100644 --- a/src/linter/ui5Types/TypeLinter.ts +++ b/src/linter/ui5Types/TypeLinter.ts @@ -8,6 +8,7 @@ import path from "node:path/posix"; import {AbstractAdapter} from "@ui5/fs"; import {createAdapter, createResource} from "@ui5/fs/resourceFactory"; import {loadApiExtract} from "../../utils/ApiExtract.js"; +import {CONTROLLER_BY_ID_DTS_PATH} from "../xmlTemplate/linter.js"; const log = getLogger("linter:ui5Types:TypeLinter"); @@ -151,13 +152,22 @@ export default class TypeChecker { resourcePath, fileContent, sourceMap); } } + // Although not being a typical transformed source, write out the byId dts file for debugging purposes + let byIdDts = files.get(CONTROLLER_BY_ID_DTS_PATH); + if (byIdDts) { + if (typeof byIdDts === "function") { + byIdDts = byIdDts(); + } + await writeTransformedSources(process.env.UI5LINT_WRITE_TRANSFORMED_SOURCES, + CONTROLLER_BY_ID_DTS_PATH, byIdDts); + } } } } async function writeTransformedSources(fsBasePath: string, originalResourcePath: string, - source: string, map: string | undefined) { + source: string, map?: string) { const transformedWriter = createAdapter({ fsBasePath, virBasePath: "/", @@ -165,7 +175,7 @@ async function writeTransformedSources(fsBasePath: string, await transformedWriter.write( createResource({ - path: originalResourcePath + ".ui5lint.transformed.js", + path: originalResourcePath, string: source, }) ); @@ -173,7 +183,7 @@ async function writeTransformedSources(fsBasePath: string, if (map) { await transformedWriter.write( createResource({ - path: originalResourcePath + ".ui5lint.transformed.js.map", + path: originalResourcePath + ".map", string: JSON.stringify(JSON.parse(map), null, "\t"), }) ); diff --git a/src/linter/xmlTemplate/ControllerByIdInfo.ts b/src/linter/xmlTemplate/ControllerByIdInfo.ts new file mode 100644 index 000000000..8ea7f2a20 --- /dev/null +++ b/src/linter/xmlTemplate/ControllerByIdInfo.ts @@ -0,0 +1,38 @@ +// Multiple views/fragments can use the same ID and link to the same controller, +// so we need to store a set of module names for each ID. +export type IdModulesMap = Map>; + +type ControllerElementsMap = Map; + +export default class ControllerByIdInfo { + private map: ControllerElementsMap = new Map(); + + private getControllerMapping(controllerName: string) { + let controllerMap = this.map.get(controllerName); + if (!controllerMap) { + controllerMap = new Map(); + this.map.set(controllerName, controllerMap); + } + return controllerMap; + } + + public addMappings(controllerName: string, idModuleMap: Map) { + // Do not add empty mappings + if (!idModuleMap.size) { + return; + } + const controllerMapping = this.getControllerMapping(controllerName); + for (const [id, module] of idModuleMap) { + let existingModules = controllerMapping.get(id); + if (!existingModules) { + existingModules = new Set(); + controllerMapping.set(id, existingModules); + } + existingModules.add(module); + } + } + + public getMappings() { + return this.map; + } +} diff --git a/src/linter/xmlTemplate/Parser.ts b/src/linter/xmlTemplate/Parser.ts index 0722571e8..feb857d74 100644 --- a/src/linter/xmlTemplate/Parser.ts +++ b/src/linter/xmlTemplate/Parser.ts @@ -9,6 +9,7 @@ import AbstractGenerator from "./generator/AbstractGenerator.js"; import {getLogger} from "@ui5/logger"; import {MESSAGE} from "../messages.js"; import {ApiExtract} from "../../utils/ApiExtract.js"; +import ControllerByIdInfo from "./ControllerByIdInfo.js"; const log = getLogger("linter:xmlTemplate:Parser"); export type Namespace = string; @@ -133,7 +134,9 @@ export default class Parser { #generator: AbstractGenerator; #apiExtract: ApiExtract; - constructor(resourceName: string, apiExtract: ApiExtract, context: LinterContext) { + constructor( + resourceName: string, apiExtract: ApiExtract, context: LinterContext, controllerByIdInfo: ControllerByIdInfo + ) { const xmlDocumentKind = determineDocumentKind(resourceName); if (xmlDocumentKind === null) { throw new Error(`Unknown document type for resource ${resourceName}`); @@ -141,8 +144,8 @@ export default class Parser { this.#resourceName = resourceName; this.#xmlDocumentKind = xmlDocumentKind; this.#generator = xmlDocumentKind === DocumentKind.View ? - new ViewGenerator(resourceName) : - new FragmentGenerator(resourceName); + new ViewGenerator(resourceName, controllerByIdInfo) : + new FragmentGenerator(resourceName, controllerByIdInfo); this.#apiExtract = apiExtract; this.#context = context; diff --git a/src/linter/xmlTemplate/generator/AbstractGenerator.ts b/src/linter/xmlTemplate/generator/AbstractGenerator.ts index fd48025f8..3c429a5d5 100644 --- a/src/linter/xmlTemplate/generator/AbstractGenerator.ts +++ b/src/linter/xmlTemplate/generator/AbstractGenerator.ts @@ -1,3 +1,4 @@ +import ControllerByIdInfo from "../ControllerByIdInfo.js"; import { ControlDeclaration, RequireExpression, Position, } from "../Parser.js"; @@ -17,10 +18,13 @@ export default abstract class AbstractGenerator { _imports = new Set(); _variableNames = new Set(); _body: Writer; + _idToModule = new Map(); + _controllerByIdInfo: ControllerByIdInfo; - constructor(filePath: string) { + constructor(filePath: string, controllerByIdInfo: ControllerByIdInfo) { const fileName = path.basename(filePath, ".xml"); this._body = new Writer(fileName + ".js", fileName + ".xml"); + this._controllerByIdInfo = controllerByIdInfo; } // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -31,9 +35,11 @@ export default abstract class AbstractGenerator { writeControl(controlDeclaration: ControlDeclaration) { const importVariableName = this._getUniqueVariableName(controlDeclaration.name); + const moduleName = controlDeclaration.namespace.replaceAll(".", "/") + `/${controlDeclaration.name}`; + // Add import this._imports.add({ - moduleName: controlDeclaration.namespace.replaceAll(".", "/") + `/${controlDeclaration.name}`, + moduleName, variableName: importVariableName, start: controlDeclaration.start, end: controlDeclaration.end, @@ -46,6 +52,13 @@ export default abstract class AbstractGenerator { this._body.writeln(`new ${importVariableName}({`, controlDeclaration.start, controlDeclaration.end); // Write properties controlDeclaration.properties.forEach((attribute) => { + // Add mapping of id to module name so that specific byId lookup for controllers can be generated. + // This information can only be added to the ControllerByIdInfo once the controllerName is known, + // which happens last as outer nodes are visited last. + if (attribute.name === "id") { + this._idToModule.set(attribute.value, moduleName); + } + // TODO: Determine attribute type based on metadata and parse/write value accordingly this._body.write(` `); this._body.writeln( diff --git a/src/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts b/src/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts new file mode 100644 index 000000000..5a9feb50b --- /dev/null +++ b/src/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts @@ -0,0 +1,70 @@ +import ControllerByIdInfo, {IdModulesMap} from "../ControllerByIdInfo.js"; + +export class ControllerByIdDtsGenerator { + // Maps module names to local names + private imports = new Map(); + + generate(controllerByIdInfo: ControllerByIdInfo) { + const mappings = controllerByIdInfo.getMappings(); + if (!mappings.size) { + 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); + }); + return this.generateCollectedImports() + out; + } + + private generateCollectedImports() { + return Array.from(this.imports.entries()) + .map(([moduleName, localName]) => `import ${localName} from "${moduleName}";`) + .join("\n") + "\n"; + } + + private generateByIdMapping(idToModules: IdModulesMap) { + let out = "interface ByIdMapping {\n"; + idToModules.forEach((modules, id) => { + const localNames = Array.from(modules).map((moduleName: string) => this.addImport(moduleName)); + out += `\t\t"${id}": ${localNames.join(" | ")};\n`; + }); + out += "\t}"; + return out; + } + + private generateModuleDeclaration(controllerName: string, idToModules: IdModulesMap) { + const moduleName = controllerName.replace(/\./g, "/") + ".controller"; + + return ` +declare module "${moduleName}" { + ${this.generateByIdMapping(idToModules)} + type ByIdFunction = { + (sId: T): ByIdMapping[T]; + (sId: string): ${this.imports.get("sap/ui/core/Element")}; + } + interface ControllerView extends ${this.imports.get("sap/ui/core/mvc/View")} { + byId: ByIdFunction; + } + export default interface Controller { + byId: ByIdFunction; + getView(): ControllerView; + } +} +`; + } + + 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/generator/ViewGenerator.ts b/src/linter/xmlTemplate/generator/ViewGenerator.ts index cf45f4de4..a6dd4674b 100644 --- a/src/linter/xmlTemplate/generator/ViewGenerator.ts +++ b/src/linter/xmlTemplate/generator/ViewGenerator.ts @@ -3,6 +3,13 @@ import {ControlDeclaration} from "../Parser.js"; export default class ViewGenerator extends AbstractGenerator { writeRootControl(controlInfo: ControlDeclaration) { + controlInfo.properties.forEach((attribute) => { + if (attribute.name === "controllerName") { + // Outer nodes are visited last, so the controllerName is only known after + // all controls have been visited. Only then the collected mappings can be added. + this._controllerByIdInfo.addMappings(attribute.value, this._idToModule); + } + }); this._body.write(`export default `); this.writeControl(controlInfo); } diff --git a/src/linter/xmlTemplate/linter.ts b/src/linter/xmlTemplate/linter.ts index 8f2d651f7..b34186190 100644 --- a/src/linter/xmlTemplate/linter.ts +++ b/src/linter/xmlTemplate/linter.ts @@ -2,12 +2,19 @@ import {Resource} from "@ui5/fs"; import {createResource} from "@ui5/fs/resourceFactory"; import transpileXml from "./transpiler.js"; import {LinterParameters} from "../LinterContext.js"; +import ControllerByIdInfo from "./ControllerByIdInfo.js"; +import {ControllerByIdDtsGenerator} from "./generator/ControllerByIdDtsGenerator.js"; + +// For usage in TypeLinter to write the file as part of the internal WRITE_TRANSFORMED_SOURCES debug mode +export const CONTROLLER_BY_ID_DTS_PATH = "/types/@ui5/linter/virtual/ControllerById.d.ts"; export default async function lintXml({filePathsWorkspace, workspace, context}: LinterParameters) { const xmlResources = await filePathsWorkspace.byGlob("**/{*.view.xml,*.fragment.xml}"); + const controllerByIdInfo = new ControllerByIdInfo(); + await Promise.all(xmlResources.map(async (resource: Resource) => { - const res = await transpileXml(resource.getPath(), resource.getStream(), context); + const res = await transpileXml(resource.getPath(), resource.getStream(), context, controllerByIdInfo); if (!res) { return; } @@ -31,4 +38,15 @@ export default async function lintXml({filePathsWorkspace, workspace, context}: await filePathsWorkspace.write(transpiledResourceSourceMap); await workspace.write(transpiledResourceSourceMap); })); + + // Generate dts file with specific byId signatures for controllers based on view IDs + const controllerByIdDtsGenerator = new ControllerByIdDtsGenerator(); + const controllerByIdDts = controllerByIdDtsGenerator.generate(controllerByIdInfo); + if (controllerByIdDts) { + const dtsResource = createResource({ + path: CONTROLLER_BY_ID_DTS_PATH, + string: controllerByIdDts, + }); + await workspace.write(dtsResource); + } } diff --git a/src/linter/xmlTemplate/transpiler.ts b/src/linter/xmlTemplate/transpiler.ts index def84184f..9d36a52db 100644 --- a/src/linter/xmlTemplate/transpiler.ts +++ b/src/linter/xmlTemplate/transpiler.ts @@ -9,6 +9,7 @@ import {getLogger} from "@ui5/logger"; import {createRequire} from "node:module"; import {MESSAGE} from "../messages.js"; import {loadApiExtract, ApiExtract} from "../../utils/ApiExtract.js"; +import ControllerByIdInfo from "./ControllerByIdInfo.js"; const require = createRequire(import.meta.url); const log = getLogger("linter:xmlTemplate:transpiler"); @@ -17,12 +18,12 @@ let saxWasmBuffer: Buffer; let apiExtract: ApiExtract; export default async function transpileXml( - resourcePath: string, contentStream: ReadStream, context: LinterContext + resourcePath: string, contentStream: ReadStream, context: LinterContext, controllerByIdInfo: ControllerByIdInfo ): Promise { await init(); try { const taskEnd = taskStart("Transpile XML", resourcePath, true); - const res = await transpileXmlToJs(resourcePath, contentStream, context); + const res = await transpileXmlToJs(resourcePath, contentStream, context, controllerByIdInfo); taskEnd(); if (!res.source) { log.verbose(`XML transpiler returned no result for ${resourcePath}`); @@ -56,9 +57,9 @@ async function init() { } async function transpileXmlToJs( - resourcePath: string, contentStream: ReadStream, context: LinterContext + resourcePath: string, contentStream: ReadStream, context: LinterContext, controllerByIdInfo: ControllerByIdInfo ): Promise { - const parser = new Parser(resourcePath, apiExtract, context); + const parser = new Parser(resourcePath, apiExtract, context, controllerByIdInfo); // Initialize parser const options = {highWaterMark: 32 * 1024}; // 32k chunks 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 c112e1155..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 @@ -4,6 +4,32 @@ sap.ui.define(["./BaseController", "sap/m/MessageBox"], function (BaseController return BaseController.extend("com.ui5.troublesome.app.controller.Main", { sayHello: function () { MessageBox.show("Hello World!"); + }, + + 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. + // 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 + 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/fixtures/linter/projects/com.ui5.troublesome.app/webapp/view/DesktopMain.view.xml b/test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/view/DesktopMain.view.xml new file mode 100644 index 000000000..9f0714756 --- /dev/null +++ b/test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/view/DesktopMain.view.xml @@ -0,0 +1,17 @@ + + + +