feat: Check control renderer declaration (#374)

Checks the renderer declaration of a UI5 Control and reports an error if
deprecated usage is detected.

JIRA: CPOUI5FOUNDATION-876

src/linter/messages.ts

@@ -5,6 +5,7 @@ const RULES = {
 	"csp-unsafe-inline-script": "csp-unsafe-inline-script",
 	"no-deprecated-api": "no-deprecated-api",
 	"no-deprecated-component": "no-deprecated-component",
+	"no-deprecated-control-renderer-declaration": "no-deprecated-control-renderer-declaration",
 	"no-deprecated-library": "no-deprecated-library",
 	"no-deprecated-theme": "no-deprecated-theme",
 	"no-globals": "no-globals",
@@ -60,6 +61,8 @@ export enum MESSAGE {
 	REDUNDANT_VIEW_CONFIG_PROPERTY,
 	SPELLING_BOOTSTRAP_PARAM,
 	SVG_IN_XML,
+	MISSING_CONTROL_RENDERER_DECLARATION,
+	CONTROL_RENDERER_DECLARATION_STRING,
 }
 export const MESSAGE_INFO = {
 
@@ -457,4 +460,30 @@ export const MESSAGE_INFO = {
 		details: () => `{@link topic:28fcd55b04654977b63dacbee0552712 See Best Practices for Developers}`,
 	},
 
+	[MESSAGE.MISSING_CONTROL_RENDERER_DECLARATION]: {
+		severity: LintMessageSeverity.Error,
+		ruleId: RULES["no-deprecated-control-renderer-declaration"],
+
+		message: ({className}: {className: string}) =>
+			`Control '${className}' is missing a renderer declaration`,
+		details: ({className}: {className: string}) => `Not defining a 'renderer' for control '${className}' ` +
+			`may lead to synchronous loading of the '${className}Renderer' module. ` +
+			`If no renderer exists, set 'renderer: null'. Otherwise, either import the renderer module ` +
+			`and assign it to the 'renderer' property or implement the renderer inline.`,
+	},
+
+	[MESSAGE.CONTROL_RENDERER_DECLARATION_STRING]: {
+		severity: LintMessageSeverity.Error,
+		ruleId: RULES["no-deprecated-control-renderer-declaration"],
+
+		message: ({className, rendererName}: {className: string; rendererName: string | undefined}) =>
+			`Deprecated declaration of renderer ${rendererName ? `'${rendererName}' ` : ""}for control '${className}'`,
+		details: ({className, rendererName}: {className: string; rendererName: string | undefined}) => {
+			const rendererModuleName = rendererName ? `'${rendererName.replace(/\./g, "/")}'` : "renderer";
+			return `Defining the 'renderer' for control '${className}' by its name may lead to synchronous ` +
+				`loading of the ${rendererModuleName} module. ` +
+				`Import the ${rendererModuleName} module and assign it to the 'renderer' property.`;
+		},
+	},
+
 } as const;

src/linter/ui5Types/SourceFileLinter.ts

@@ -124,81 +124,124 @@ export default class SourceFileLinter {
 				context: this.#context,
 				checker: this.#checker,
 			});
-		} else if (ts.isPropertyDeclaration(node) && node.name.getText() === "metadata") {
+		} else if (
+			ts.isPropertyDeclaration(node) && node.name.getText() === "metadata" &&
+			this.isUi5ClassDeclaration(node.parent, "sap/ui/base/ManagedObject")
+		) {
 			const visitMetadataNodes = (childNode: ts.Node) => {
 				if (ts.isPropertyAssignment(childNode)) { // Skip nodes out of interest
 					this.analyzeMetadataProperty(childNode.name.getText(), childNode);
 				}
 
 				ts.forEachChild(childNode, visitMetadataNodes);
 			};
-
-			if (this.isUi5controlMetadataNode(node)) {
-				ts.forEachChild(node, visitMetadataNodes);
-			}
+			ts.forEachChild(node, visitMetadataNodes);
+		} else if (this.isUi5ClassDeclaration(node, "sap/ui/core/Control")) {
+			this.analyzeControlRendererDeclaration(node);
 		}
 
 		// Traverse the whole AST from top to bottom
 		ts.forEachChild(node, this.#boundVisitNode);
 	}
 
-	isUi5controlMetadataNode(node: ts.PropertyAssignment | ts.PropertyDeclaration): boolean {
-		// Go up the hierarchy chain to find whether the class extends from "sap/ui/base/ManagedObject"
-		const isObjectMetadataAncestor = (node: ts.ClassDeclaration): boolean => {
+	isUi5ClassDeclaration(node: ts.Node, baseClassModule: string | string[]): node is ts.ClassDeclaration {
+		if (!ts.isClassDeclaration(node)) {
+			return false;
+		}
+		const baseClassModules = Array.isArray(baseClassModule) ? baseClassModule : [baseClassModule];
+		const baseClasses = baseClassModules.map((baseClassModule) => {
+			return {module: baseClassModule, name: baseClassModule.split("/").pop()};
+		});
+
+		// Go up the hierarchy chain to find whether the class extends from the provided base class
+		const isClassUi5Subclass = (node: ts.ClassDeclaration): boolean => {
 			return node?.heritageClauses?.flatMap((parentClasses: ts.HeritageClause) => {
 				return parentClasses.types.flatMap((parentClass) => {
 					const parentClassType = this.#checker.getTypeAtLocation(parentClass);
 
 					return parentClassType.symbol?.declarations?.flatMap((declaration) => {
-						if (ts.isClassDeclaration(declaration)) {
-							if (declaration.name?.getText() === "ManagedObject" &&
+						if (!ts.isClassDeclaration(declaration)) {
+							return false;
+						}
+						for (const baseClass of baseClasses) {
+							if (declaration.name?.getText() === baseClass.name &&
 								(
-									// Declaration via type definitions
+								// Declaration via type definitions
 									(
 										declaration.parent.parent &&
 										ts.isModuleDeclaration(declaration.parent.parent) &&
-										declaration.parent.parent.name?.text === "sap/ui/base/ManagedObject"
+										declaration.parent.parent.name?.text === baseClass.module
 									) ||
-									// Declaration via real class (within sap.ui.core project)
+									// Declaration via real class (e.g. within sap.ui.core project)
 									(
 										ts.isSourceFile(declaration.parent) &&
-										declaration.parent.fileName === "/resources/sap/ui/base/ManagedObject.js"
+										(
+											declaration.parent.fileName === `/resources/${baseClass.module}.js` ||
+											declaration.parent.fileName === `/resources/${baseClass.module}.ts`
+										)
 									)
 								)
 							) {
 								return true;
-							} else {
-								return isObjectMetadataAncestor(declaration);
 							}
-						} else {
-							return false;
 						}
+						return isClassUi5Subclass(declaration);
 					});
 				});
 			// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
 			}).reduce((acc, cur) => cur || acc, false) ?? false;
 		};
 
-		const metadataFound = taskStart("isPropertyInMetadata", this.#resourcePath, true);
+		return isClassUi5Subclass(node);
+	}
 
-		if (node.name.getText() !== "metadata") {
-			metadataFound();
-			return false;
-		}
+	analyzeControlRendererDeclaration(node: ts.ClassDeclaration) {
+		const className = node.name?.getText() ?? "<unknown>";
+		const rendererMember = node.members.find((member) => {
+			return (ts.isPropertyDeclaration(member) || ts.isMethodDeclaration(member)) &&
+				member.modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.StaticKeyword) &&
+				member.name.getText() === "renderer";
+		});
 
-		let parentNode: ts.Node = node.parent;
-		while (parentNode && parentNode.kind !== ts.SyntaxKind.ClassDeclaration) {
-			parentNode = parentNode.parent;
+		if (!rendererMember) {
+			// Special cases: Some base classes do not require sub-classes to have a renderer defined:
+			if (this.isUi5ClassDeclaration(node, [
+				"sap/ui/core/mvc/View",
+				// XMLComposite is deprecated, but there still shouldn't be a false-positive about a missing renderer
+				"sap/ui/core/XMLComposite",
+				"sap/ui/core/webc/WebComponent",
+				"sap/uxap/BlockBase",
+			])) {
+				return;
+			}
+			// No definition of renderer causes the runtime to load the corresponding Renderer module synchronously
+			this.#reporter.addMessage(MESSAGE.MISSING_CONTROL_RENDERER_DECLARATION, {className}, node);
+			return;
 		}
 
-		if (!parentNode) {
-			metadataFound();
-			return false;
-		}
+		if (ts.isPropertyDeclaration(rendererMember) && rendererMember.initializer) {
+			const initializerType = this.#checker.getTypeAtLocation(rendererMember.initializer);
 
-		const result = isObjectMetadataAncestor(parentNode as ts.ClassDeclaration);
-		metadataFound();
-		return result;
+			if (initializerType.flags & ts.TypeFlags.Undefined ||
+				initializerType.flags & ts.TypeFlags.Null) {
+				// null / undefined can be used to declare that a control does not have a renderer
+				return;
+			}
+
+			if (initializerType.flags & ts.TypeFlags.StringLiteral) {
+				let rendererName;
+				if (
+					ts.isStringLiteral(rendererMember.initializer) ||
+					ts.isNoSubstitutionTemplateLiteral(rendererMember.initializer)
+				) {
+					rendererName = rendererMember.initializer.text;
+				}
+				// Declaration as string requires sync loading of renderer module
+				this.#reporter.addMessage(MESSAGE.CONTROL_RENDERER_DECLARATION_STRING, {
+					className, rendererName,
+				}, rendererMember.initializer);
+			}
+		}
 	}
 
 	analyzeMetadataProperty(type: string, node: ts.PropertyAssignment) {

src/linter/ui5Types/amdTranspiler/rewriteExtendCall.ts

@@ -70,42 +70,79 @@ function getClassBodyFromArguments(
 		if (ts.isMethodDeclaration(prop)) {
 			// Use method declarations as-is
 			// e.g. "method() {}"
+
+			// Special handling:
+			// - renderer: *static*
+			// This aligns it with how UI5 projects should declare those properties in TypeScript
+
+			if (
+				ts.isIdentifier(prop.name) && prop.name.text === "renderer" &&
+				!prop.modifiers?.find((mod) => mod.kind === ts.SyntaxKind.StaticKeyword)
+			) {
+				// Add static modifier to renderer method
+				const staticModifier = nodeFactory.createToken(ts.SyntaxKind.StaticKeyword);
+				return nodeFactory.updateMethodDeclaration(prop,
+					prop.modifiers ? [...prop.modifiers, staticModifier] : [staticModifier],
+					prop.asteriskToken,
+					prop.name, prop.questionToken, prop.typeParameters, prop.parameters, prop.type, prop.body);
+			}
+
 			return prop;
 		} else if (ts.isPropertyAssignment(prop)) {
 			if (ts.isFunctionExpression(prop.initializer)) {
+				let modifiers: ts.ModifierLike[] | undefined;
+
+				// Special handling:
+				// - renderer: *static*
+				// This aligns it with how UI5 projects should declare those properties in TypeScript
+				if (
+					ts.isIdentifier(prop.name) && prop.name.text === "renderer" &&
+					!prop.initializer.modifiers?.find((mod) => mod.kind === ts.SyntaxKind.StaticKeyword)
+				) {
+					modifiers = [nodeFactory.createToken(ts.SyntaxKind.StaticKeyword)];
+				}
+
 				return nodeFactory.createMethodDeclaration(
-					undefined, undefined,
+					modifiers, undefined,
 					prop.name,
 					undefined, undefined,
 					prop.initializer.parameters,
 					undefined,
 					prop.initializer.body);
-			} else if (ts.isObjectLiteralExpression(prop.initializer) &&
-				ts.isIdentifier(prop.name) && prop.name.text === "metadata") {
-				// Transform to *static* property declaration?
-				// This would align it with how UI5 projects should declare metadata in TypeScript,
-				// however it's unclear whether this helps our static analysis
-				return nodeFactory.createPropertyDeclaration(
-					[nodeFactory.createToken(ts.SyntaxKind.StaticKeyword)],
-					prop.name,
-					undefined, undefined,
-					prop.initializer);
 			} else {
-				// Assign all other properties (including arrow functions) to the class prototype
-				// This transformation does not reflect the runtime behavior where
-				// properties are set on the Class's prototype. However, tsc won't derive *any*
-				// type information from Object.defineProperty(Class.prototype, "prob", ...)
-				// So the current approach works better for the static analysis
-				if (prop.initializer.kind === ts.SyntaxKind.NullKeyword ||
-					prop.initializer.kind === ts.SyntaxKind.UndefinedKeyword) {
+				const modifiers: ts.ModifierLike[] = [];
+
+				// Special handling:
+				// - metadata: *readonly static*
+				// - renderer: *static*
+				// This aligns it with how UI5 projects should declare those properties in TypeScript
+				if (
+					ts.isObjectLiteralExpression(prop.initializer) &&
+					ts.isIdentifier(prop.name) &&
+					prop.name.text === "metadata"
+				) {
+					modifiers.push(nodeFactory.createToken(ts.SyntaxKind.ReadonlyKeyword));
+					modifiers.push(nodeFactory.createToken(ts.SyntaxKind.StaticKeyword));
+				} else if (ts.isIdentifier(prop.name) && prop.name.text === "renderer") {
+					modifiers.push(nodeFactory.createToken(ts.SyntaxKind.StaticKeyword));
+				} else if (prop.initializer.kind === ts.SyntaxKind.NullKeyword ||
+					prop.initializer.kind === ts.SyntaxKind.UndefinedKeyword
+				) {
 					// Skip property assignments that declare a null value, in the hope
 					// that tsc can infer a more useful type based on other assignment
 					// in one of the methods. If we would define a class property, tsc
 					// would not attempt to infer more type information.
 					return;
 				}
+
+				// Assign all properties (including arrow functions) to the class prototype
+				// This transformation does not reflect the runtime behavior where
+				// properties are set on the Class's prototype. However, tsc won't derive *any*
+				// type information from Object.defineProperty(Class.prototype, "prob", ...)
+				// So the current approach works better for the static analysis
+
 				return nodeFactory.createPropertyDeclaration(
-					undefined,
+					modifiers,
 					prop.name,
 					undefined, undefined,
 					prop.initializer);

test/fixtures/linter/projects/sap.ui.core/src/sap/ui/core/FancyText.js

@@ -66,6 +66,7 @@ sap.ui.define(["sap/ui/core/Control", "sap/m/library"],
 				},
 			},
 		},
+		renderer: null // No renderer
 	});
 	return FancyText;
 });

test/fixtures/linter/rules/NoDeprecatedApi/ControlRendererDeclaration.js

@@ -0,0 +1,25 @@
+sap.ui.define(["sap/ui/core/Control", "sap/m/Button"], function(Control, Button) {
+	const Example1 = Control.extend("sap.ui.demo.linter.controls.Example1", {
+		metadata: {},
+		// Declaration of renderer module as string (deprecated)
+		renderer: "sap.ui.demo.linter.controls.Example1Renderer"
+	});
+	const Example2 = Control.extend("sap.ui.demo.linter.controls.Example2", {
+		metadata: {},
+		// Declaration of renderer module as template literal (deprecated)
+		renderer: `sap.ui.demo.linter.controls.Example2Renderer`
+	});
+	const Example3 = Control.extend("sap.ui.demo.linter.controls.Example3", {
+		metadata: {},
+		// Declaration of renderer module as template literal with substitution (deprecated)
+		renderer: `sap.ui.demo.linter.controls.Example${1+2}Renderer`
+	});
+	const Example4 = Control.extend("sap.ui.demo.linter.controls.Example4", {
+		metadata: {},
+		// Missing renderer declaration (deprecated)
+	});
+	const Example5 = Button.extend("sap.ui.demo.linter.controls.Example5", {
+		metadata: {},
+		// Missing renderer declaration (deprecated)
+	});
+});

test/fixtures/linter/rules/NoDeprecatedApi/ControlRendererDeclarationSingleControlExported.js

@@ -0,0 +1,6 @@
+sap.ui.define(["sap/ui/core/Control"], function(Control, Button) {
+	return Control.extend("sap.ui.demo.linter.controls.Example2", {
+		metadata: {},
+		// Missing renderer declaration (deprecated)
+	});
+});