feat: Detect override of control "rerender"

JIRA: CPOUI5FOUNDATION-939
SAP · Dec 4, 2024 · 6279338 · 6279338
1 parent 4eb2758
commit 6279338
Show file tree

Hide file tree

Showing 6 changed files with 208 additions and 0 deletions.
diff --git a/src/linter/messages.ts b/src/linter/messages.ts
@@ -47,6 +47,7 @@ export enum MESSAGE {
 	HTML_IN_XML,
 	LIB_INIT_API_VERSION,
 	MISSING_BOOTSTRAP_PARAM,
+	NO_CONTROL_RERENDER_OVERRIDE,
 	NO_DEPRECATED_RENDERER,
 	NO_DIRECT_DATATYPE_ACCESS,
 	NO_DIRECT_ENUM_ACCESS,
@@ -316,6 +317,16 @@ export const MESSAGE_INFO = {
 		details: () => `{@link sap.ui.core.Lib.init Lib.init}`,
 	},
 
+	[MESSAGE.NO_CONTROL_RERENDER_OVERRIDE]: {
+		severity: LintMessageSeverity.Error,
+		ruleId: RULES["no-deprecated-api"],
+		message: ({className}: {className: string}) =>
+			`Override of deprecated method 'rerender' in control '${className}'`,
+		details: () => `Starting from UI5 1.121 the framework no longer calls 'rerender', ` +
+			`so there is no point in overriding it. ` +
+			`Move any rendering related code to the renderer or into onBeforeRendering/onAfterRendering.`,
+	},
+
 	[MESSAGE.NO_DEPRECATED_RENDERER]: {
 		severity: LintMessageSeverity.Error,
 		ruleId: RULES["no-deprecated-api"],

diff --git a/src/linter/ui5Types/SourceFileLinter.ts b/src/linter/ui5Types/SourceFileLinter.ts
@@ -153,6 +153,7 @@ export default class SourceFileLinter {
 			ts.forEachChild(node, visitMetadataNodes);
 		} else if (this.isUi5ClassDeclaration(node, "sap/ui/core/Control")) {
 			this.analyzeControlRendererDeclaration(node);
+			this.analyzeControlRerenderMethod(node);
 		} else if (ts.isPropertyAssignment(node) && removeQuotes(node.name.getText()) === "theme") {
 			this.analyzeTestsuiteThemeProperty(node);
 		}
@@ -539,6 +540,19 @@ export default class SourceFileLinter {
 		}
 	}
 
+	analyzeControlRerenderMethod(node: ts.ClassDeclaration) {
+		const className = node.name?.getText() ?? "<unknown>";
+		const rerenderMethod = node.members.find((member) => {
+			return ts.isMethodDeclaration(member) &&
+				(ts.isIdentifier(member.name) || ts.isStringLiteral(member.name)) &&
+				member.name.text === "rerender";
+		});
+		if (!rerenderMethod) {
+			return;
+		}
+		this.#reporter.addMessage(MESSAGE.NO_CONTROL_RERENDER_OVERRIDE, {className}, rerenderMethod);
+	}
+
 	analyzeMetadataProperty(type: string, node: ts.PropertyAssignment) {
 		const analyzeMetadataDone = taskStart(`analyzeMetadataProperty: ${type}`, this.resourcePath, true);
 		if (type === "interfaces") {

diff --git a/test/fixtures/linter/rules/renderer/ControlRerenderOverride.js b/test/fixtures/linter/rules/renderer/ControlRerenderOverride.js
@@ -0,0 +1,36 @@
+sap.ui.define(["sap/ui/core/Control"], function(Control, Button) {
+	const Example1 = Control.extend("sap.ui.demo.linter.controls.Example1", {
+		metadata: {},
+
+		rerender: function() {
+			console.log("Overriding rerender method");
+			return Control.prototype.rerender.apply(this, arguments);
+		},
+
+		renderer: {
+			apiVersion: 2,
+			render: function(oRm, oControl) {
+				oRm.openStart("div", oControl);
+				oRm.openEnd();
+				oRm.close("div");
+			}
+		}
+	});
+
+	const Example2 = Control.extend("sap.ui.demo.linter.controls.Example2", {
+		metadata: {},
+
+		"rerender": function() {
+			console.log("Overriding rerender method without calling super method");
+		},
+
+		renderer: {
+			apiVersion: 2,
+			render: function(oRm, oControl) {
+				oRm.openStart("div", oControl);
+				oRm.openEnd();
+				oRm.close("div");
+			}
+		}
+	});
+});
diff --git a/test/fixtures/linter/rules/renderer/ControlRerenderOverride.ts b/test/fixtures/linter/rules/renderer/ControlRerenderOverride.ts
@@ -0,0 +1,44 @@
+import Control from "sap/ui/core/Control";
+import type { MetadataOptions } from "sap/ui/core/Element";
+import RenderManager from "sap/ui/core/RenderManager";
+
+/**
+ * @namespace sap.ui.demo.linter.controls
+ */
+class Example1 extends Control {
+	static readonly metadata: MetadataOptions = {}
+
+	rerender() {
+		console.log("Overriding rerender method");
+		return super.rerender();
+	}
+
+	static renderer = {
+		apiVersion: 2,
+		render: function(rm: RenderManager, control: Example1) {
+			rm.openStart("div", control);
+			rm.openEnd();
+			rm.close("div");
+		}
+	}
+}
+
+/**
+ * @namespace sap.ui.demo.linter.controls
+ */
+class Example2 extends Control {
+	static readonly metadata: MetadataOptions = {}
+
+	rerender() {
+		console.log("Overriding rerender method without calling super method");
+	}
+
+	static renderer = {
+		apiVersion: 2,
+		render: function(rm: RenderManager, control: Example1) {
+			rm.openStart("div", control);
+			rm.openEnd();
+			rm.close("div");
+		}
+	}
+}
diff --git a/test/lib/linter/rules/snapshots/renderer.ts.md b/test/lib/linter/rules/snapshots/renderer.ts.md
@@ -475,6 +475,109 @@ Generated by [AVA](https://avajs.dev).
         ],
         warningCount: 0,
       },
+      {
+        coverageInfo: [
+          {
+            category: 1,
+            column: 5,
+            line: 13,
+            message: 'Unable to analyze this method call because the type of identifier "openStart" in "oRm.openStart("div", oControl)"" could not be determined',
+          },
+          {
+            category: 1,
+            column: 5,
+            line: 14,
+            message: 'Unable to analyze this method call because the type of identifier "openEnd" in "oRm.openEnd()"" could not be determined',
+          },
+          {
+            category: 1,
+            column: 5,
+            line: 15,
+            message: 'Unable to analyze this method call because the type of identifier "close" in "oRm.close("div")"" could not be determined',
+          },
+          {
+            category: 1,
+            column: 5,
+            line: 30,
+            message: 'Unable to analyze this method call because the type of identifier "openStart" in "oRm.openStart("div", oControl)"" could not be determined',
+          },
+          {
+            category: 1,
+            column: 5,
+            line: 31,
+            message: 'Unable to analyze this method call because the type of identifier "openEnd" in "oRm.openEnd()"" could not be determined',
+          },
+          {
+            category: 1,
+            column: 5,
+            line: 32,
+            message: 'Unable to analyze this method call because the type of identifier "close" in "oRm.close("div")"" could not be determined',
+          },
+        ],
+        errorCount: 3,
+        fatalErrorCount: 0,
+        filePath: 'ControlRerenderOverride.js',
+        messages: [
+          {
+            column: 3,
+            line: 5,
+            message: 'Override of deprecated method \'rerender\' in control \'Example1\'',
+            messageDetails: 'Starting from UI5 1.121 the framework no longer calls \'rerender\', so there is no point in overriding it. Move any rendering related code to the renderer or into onBeforeRendering/onAfterRendering.',
+            ruleId: 'no-deprecated-api',
+            severity: 2,
+          },
+          {
+            column: 11,
+            line: 7,
+            message: 'Use of deprecated property \'rerender\'',
+            messageDetails: 'Deprecated test message',
+            ruleId: 'no-deprecated-api',
+            severity: 2,
+          },
+          {
+            column: 3,
+            line: 23,
+            message: 'Override of deprecated method \'rerender\' in control \'Example2\'',
+            messageDetails: 'Starting from UI5 1.121 the framework no longer calls \'rerender\', so there is no point in overriding it. Move any rendering related code to the renderer or into onBeforeRendering/onAfterRendering.',
+            ruleId: 'no-deprecated-api',
+            severity: 2,
+          },
+        ],
+        warningCount: 0,
+      },
+      {
+        coverageInfo: [],
+        errorCount: 3,
+        fatalErrorCount: 0,
+        filePath: 'ControlRerenderOverride.ts',
+        messages: [
+          {
+            column: 2,
+            line: 11,
+            message: 'Override of deprecated method \'rerender\' in control \'Example1\'',
+            messageDetails: 'Starting from UI5 1.121 the framework no longer calls \'rerender\', so there is no point in overriding it. Move any rendering related code to the renderer or into onBeforeRendering/onAfterRendering.',
+            ruleId: 'no-deprecated-api',
+            severity: 2,
+          },
+          {
+            column: 16,
+            line: 13,
+            message: 'Call to deprecated function \'rerender\' (super.rerender)',
+            messageDetails: 'Deprecated test message',
+            ruleId: 'no-deprecated-api',
+            severity: 2,
+          },
+          {
+            column: 2,
+            line: 32,
+            message: 'Override of deprecated method \'rerender\' in control \'Example2\'',
+            messageDetails: 'Starting from UI5 1.121 the framework no longer calls \'rerender\', so there is no point in overriding it. Move any rendering related code to the renderer or into onBeforeRendering/onAfterRendering.',
+            ruleId: 'no-deprecated-api',
+            severity: 2,
+          },
+        ],
+        warningCount: 0,
+      },
       {
         coverageInfo: [],
         errorCount: 3,

diff --git a/test/lib/linter/rules/snapshots/renderer.ts.snap b/test/lib/linter/rules/snapshots/renderer.ts.snap