feat: Add type support for Controller#byId

This feature adds type definitions for `byId` within controllers so that
the usage of deprecated functionality can be detected when controls or
elements from the view are accessed by their ID.
Previously, only the base element class was known, which is still the
case as a fallback.

JIRA: CPOUI5FOUNDATION-843

src/linter/xmlTemplate/ControllerByIdInfo.ts

@@ -0,0 +1,34 @@
+// 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<string, Set<string>>;
+
+type ControllerElementsMap = Map<string, IdModulesMap>;
+
+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<string, string>) {
+		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;
+	}
+}

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,16 +134,18 @@ 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}`);
 		}
 		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;

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<ImportStatement>();
 	_variableNames = new Set<string>();
 	_body: Writer;
+	_idToModule = new Map<string, string>();
+	_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(

src/linter/xmlTemplate/generator/ControllerByIdDtsGenerator.ts

@@ -0,0 +1,62 @@
+import ControllerByIdInfo, {IdModulesMap} from "../ControllerByIdInfo.js";
+
+interface Import {
+	localName: string;
+	moduleName: string;
+}
+export class ControllerByIdDtsGenerator {
+	private imports = new Set<Import>();
+
+	constructor(private controllerByIdInfo: ControllerByIdInfo) {
+	}
+
+	generate() {
+		let out = "";
+		this.controllerByIdInfo.getMappings().forEach((idToModules, controllerName) => {
+			out += this.generateModuleDeclaration(controllerName, idToModules);
+		});
+		return this.generateCollectedImports() + out;
+	}
+
+	generateCollectedImports() {
+		let out = "";
+		this.imports.forEach((moduleImport) => {
+			out += `import ${moduleImport.localName} from "${moduleImport.moduleName}";\n`;
+		});
+		out += "\n";
+		return out;
+	}
+
+	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);
+				this.imports.add({localName, moduleName});
+			});
+			out += `\t\t"${id}": ${localNames.join(" | ")};\n`;
+		});
+		out += "\t}\n";
+		return out;
+	}
+
+	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 += `\t};\n`;
+		out += `};\n\n`;
+		return out;
+	}
+
+	getLocalModuleName(moduleName: string) {
+		return moduleName.replace(/[/.]/g, "_");
+	}
+}

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);
 	}

src/linter/xmlTemplate/linter.ts

@@ -2,12 +2,16 @@ 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";
 
 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 +35,14 @@ 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(controllerByIdInfo);
+	const controllerByIdDts = controllerByIdDtsGenerator.generate();
+
+	const dtsResource = createResource({
+		path: "/types/@ui5/linter/virtual/ControllerById.d.ts",
+		string: controllerByIdDts,
+	});
+	await workspace.write(dtsResource);
 }

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<TranspileResult | undefined> {
 	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<TranspileResult> {
-	const parser = new Parser(resourcePath, apiExtract, context);
+	const parser = new Parser(resourcePath, apiExtract, context, controllerByIdInfo);
 
 	// Initialize parser
 	const options = {highWaterMark: 32 * 1024}; // 32k chunks

test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/controller/Main.controller.js

@@ -4,6 +4,26 @@ 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("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.
+			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) {
+				testButton.attachTap(function() {
+					console.log("Tapped");
+				});
+			}
 		}
 	});
 });

test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/view/DesktopMain.view.xml

@@ -0,0 +1,17 @@
+<!--
+	This file is a partial copy of Main.view.xml but uses other controls.
+	Its purpose is to cover the case where multiple views refer to
+	the same controller (controllerName) and contain controls with
+	the same ID but a different type. The resulting type for the
+	controller byId() call should be the union of the types.
+-->
+<mvc:View
+	controllerName="com.ui5.troublesome.app.controller.Main"
+	displayBlock="true"
+	xmlns="sap.ui.commons"
+	xmlns:mvc="sap.ui.core.mvc"
+	xmlns:core="sap.ui.core">
+
+	<Button id="testButton" />
+
+</mvc:View>

test/fixtures/linter/projects/com.ui5.troublesome.app/webapp/view/Main.view.xml

@@ -23,4 +23,6 @@
 		</buttons>
 	</MessagePage>
 
+	<Button id="testButton" />
+
 </mvc:View>

test/fixtures/transpiler/xml/XMLView.view.xml

@@ -2,20 +2,21 @@
 	xmlns="sap.m"
 	xmlns:table="sap.ui.table"
 	xmlns:tablePlugins="sap.ui.table.plugins"
+	controllerName="com.myapp.controller.Main"
 >
 
-	<DateTimeInput /> <!-- DateTimeInput is deprecated -->
+	<DateTimeInput id="date-time-input" /> <!-- DateTimeInput is deprecated -->
 
-	<Button blocked="true" /> <!-- Property "blocked" is deprecated -->
+	<Button id="button" blocked="true" /> <!-- Property "blocked" is deprecated -->
 
 	<table:Table groupBy="some-column"> <!-- Association "groupBy" is deprecated -->
 			<table:plugins> <!-- Aggregation "plugins" is deprecated -->
-					<tablePlugins:MultiSelectionPlugin />
+					<tablePlugins:MultiSelectionPlugin id="multi-selection-plugin" />
 			</table:plugins>
 	</table:Table>
 
-	<SegmentedButton> <!-- Default aggregation "buttons" is deprecated -->
-		<Button tap=".onButtonTap"/> <!-- Event "tap" is deprecated -->
+	<SegmentedButton id="segmented-button"> <!-- Default aggregation "buttons" is deprecated -->
+		<Button id="segmented-button-inner" tap=".onButtonTap"/> <!-- Event "tap" is deprecated -->
 	</SegmentedButton>
 
 </mvc:View>