fix: Improve Control Renderer lint (#392)

- Detect non static renderer objects in ES6/TS code
- Detect apiVersion property that is not part of the variable
initialization
- Properly analyze SAPUI5 raw renderers that might be ambient modules
and present in `@sapui5/types`

src/linter/messages.ts

@@ -11,6 +11,7 @@ const RULES = {
 	"no-globals": "no-globals",
 	"no-pseudo-modules": "no-pseudo-modules",
 	"parsing-error": "parsing-error",
+	"ui5-class-declaration": "ui5-class-declaration",
 } as const;
 
 export enum LintMessageSeverity {
@@ -51,6 +52,7 @@ export enum MESSAGE {
 	NO_DIRECT_ENUM_ACCESS,
 	NO_GLOBALS,
 	NO_ICON_POOL_RENDERER,
+	NOT_STATIC_CONTROL_RENDERER,
 	PARSING_ERROR,
 	PARTIALLY_DEPRECATED_CORE_ROUTER,
 	PARTIALLY_DEPRECATED_CREATE_COMPONENT,
@@ -333,6 +335,15 @@ export const MESSAGE_INFO = {
 		details: () => `"{@link sap.ui.core.RenderManager#methods/icon RenderManager}",`,
 	},
 
+	[MESSAGE.NOT_STATIC_CONTROL_RENDERER]: {
+		severity: LintMessageSeverity.Warning,
+		ruleId: RULES["ui5-class-declaration"],
+
+		message: ({className}: {className?: string}) =>
+			`The control renderer${className ? (" of '" + className + "'") : ""} must be a static property`,
+		details: () => undefined,
+	},
+
 	[MESSAGE.NO_DIRECT_DATATYPE_ACCESS]: {
 		severity: LintMessageSeverity.Error,
 		ruleId: RULES["no-pseudo-modules"],

src/linter/ui5Types/TypeLinter.ts

@@ -95,7 +95,8 @@ export default class TypeChecker {
 		const host = await createVirtualCompilerHost(this.#compilerOptions, files, sourceMaps);
 
 		const createProgramDone = taskStart("ts.createProgram", undefined, true);
-		const program = ts.createProgram(pathsToLint, this.#compilerOptions, host);
+		const program = ts.createProgram(
+			allResources.map((resource) => resource.getPath()), this.#compilerOptions, host);
 		createProgramDone();
 
 		const getTypeCheckerDone = taskStart("program.getTypeChecker", undefined, true);
@@ -126,7 +127,7 @@ export default class TypeChecker {
 				const linterDone = taskStart("Type-check resource", sourceFile.fileName, true);
 				const linter = new SourceFileLinter(
 					this.#context, sourceFile.fileName,
-					sourceFile, sourceMaps,
+					sourceFile, sourceMaps, program,
 					checker, reportCoverage, messageDetails,
 					apiExtract, manifestContent
 				);

test/fixtures/linter/rules/renderer/2ControlRenderer.js

@@ -1,7 +1,7 @@
 sap.ui.define([], function () {
-	var myControlRenderer = {
-		apiVersion: 1, // Deprecated
-	};
+	var myControlRenderer = {};
+
+	myControlRenderer.apiVersion = 1; // Deprecated
 
 	myControlRenderer.render = function (oRm, oMyControl) {};
 

test/fixtures/linter/rules/renderer/ControlRendererDeclaration_negative.js

@@ -1,8 +1,8 @@
 sap.ui.define([
 	"sap/ui/core/Control", "sap/m/Button", "sap/ui/core/webc/WebComponent",
 	"sap/uxap/BlockBase", "./NegativeExample1Renderer", "sap/ui/core/mvc/View",
-	"sap/ui/core/XMLComposite"
-], function(Control, Button, WebComponent, BlockBase, NegativeExample1Renderer, View, XMLComposite) {
+	"sap/ui/core/XMLComposite", "sap/f/cards/loading/PlaceholderBaseRenderer"
+], function(Control, Button, WebComponent, BlockBase, NegativeExample1Renderer, View, XMLComposite, PlaceholderBaseRenderer) {
 
 	const NegativeExample1 = Control.extend("sap.ui.demo.linter.controls.NegativeExample1", {
 		metadata: {},
@@ -76,5 +76,13 @@ sap.ui.define([
 		// No deprecation: Uses inline renderer of XMLComposite if no renderer is specified
 		// Note: XMLComposite itself is deprecated, but there should not be a finding for a missing renderer
 	});
+
+	const NegativeExample13 = Control.extend("sap.ui.demo.linter.controls.NegativeExample13", {
+		metadata: {},
+		// No deprecation: sap/f/cards/loading/PlaceholderBaseRenderer
+		// is an ambient module, so it should be present in the types,
+		// despite that Renderers are usually not present in the ts definitions.
+		renderer: PlaceholderBaseRenderer,
+	});
 
 });

test/fixtures/linter/rules/renderer/TSControl.ts

@@ -0,0 +1,104 @@
+import Control from "sap/ui/core/Control";
+import RenderManager from "sap/ui/core/RenderManager";
+import type {MetadataOptions} from "sap/ui/core/Element";
+import ImportedRenderer from "./TSControlRenderer.js";
+
+export class Test1 extends Control {
+	static readonly metadata: MetadataOptions = {
+		properties: {
+			text: "string",
+			tag: "string",
+		},
+		aggregations: {
+			children: { type: "sap.ui.core.Control", multiple: true },
+		},
+		defaultAggregation: "children",
+	};
+
+	static renderer = {
+		apiVersion: 1,
+		render: (rm: RenderManager, control: HTMLElement) => {},
+	};
+}
+
+export class Test2 extends Control {
+	static readonly metadata: MetadataOptions = {
+		properties: {
+			text: "string",
+			tag: "string",
+		},
+		aggregations: {
+			children: { type: "sap.ui.core.Control", multiple: true },
+		},
+		defaultAggregation: "children",
+	};
+
+	static renderer = {
+		render: (rm: RenderManager, control: HTMLElement) => {},
+	};
+}
+
+export class Test3 extends Control {
+	static readonly metadata: MetadataOptions = {
+		properties: {
+			text: "string",
+			tag: "string",
+		},
+		aggregations: {
+			children: { type: "sap.ui.core.Control", multiple: true },
+		},
+		defaultAggregation: "children",
+	};
+
+	static renderer = (rm: RenderManager, control: HTMLElement) => {};
+}
+
+export class Test4 extends Control {
+	static readonly metadata: MetadataOptions = {
+		properties: {
+			text: "string",
+			tag: "string",
+		},
+		aggregations: {
+			children: { type: "sap.ui.core.Control", multiple: true },
+		},
+		defaultAggregation: "children",
+	};
+
+	renderer = {
+		apiVersion: 2,
+		render: (rm: RenderManager, control: HTMLElement) => {},
+	};
+}
+
+export class Test5 extends Control {
+	static readonly metadata: MetadataOptions = {
+		properties: {
+			text: "string",
+			tag: "string",
+		},
+		aggregations: {
+			children: { type: "sap.ui.core.Control", multiple: true },
+		},
+		defaultAggregation: "children",
+	};
+
+	// Missing static keyword. Renderer won't be analyzed as such
+	// until its defined explicitly as static property.
+	renderer = ImportedRenderer;
+}
+
+export class Test6 extends Control {
+	static readonly metadata: MetadataOptions = {
+		properties: {
+			text: "string",
+			tag: "string",
+		},
+		aggregations: {
+			children: { type: "sap.ui.core.Control", multiple: true },
+		},
+		defaultAggregation: "children",
+	};
+
+	static renderer = ImportedRenderer;
+}

test/fixtures/linter/rules/renderer/TSControlRenderer.ts

@@ -0,0 +1,4 @@
+var myControlRenderer = {};
+myControlRenderer.apiVersion = 1;
+myControlRenderer.render = (oRm, oMyControl) => { };
+export default myControlRenderer;

test/lib/linter/_linterHelper.ts

@@ -5,7 +5,7 @@ import {readdirSync} from "node:fs";
 import path from "node:path";
 import esmock from "esmock";
 import SourceFileLinter from "../../../src/linter/ui5Types/SourceFileLinter.js";
-import {SourceFile, TypeChecker} from "typescript";
+import {Program, SourceFile, TypeChecker} from "typescript";
 import LinterContext, {LinterOptions, LintResult} from "../../../src/linter/LinterContext.js";
 import {ApiExtract} from "../../../src/utils/ApiExtract.js";
 
@@ -28,15 +28,16 @@ export async function esmockDeprecationText() {
 		"../../../src/linter/ui5Types/SourceFileLinter.js":
 			function (
 				context: LinterContext, filePath: string, sourceFile: SourceFile,
-				sourceMaps: Map<string, string> | undefined, checker: TypeChecker,
+				sourceMaps: Map<string, string> | undefined, program: Program,
+				checker: TypeChecker,
 				reportCoverage: boolean | undefined = false,
 				messageDetails: boolean | undefined = false,
 				apiExtract: ApiExtract,
 				manifestContent?: string
 			) {
 				// Don't use sinon's stubs as it's hard to clean after them in this case and it leaks memory.
 				const linter = new SourceFileLinter(
-					context, filePath, sourceFile, sourceMaps, checker, reportCoverage,
+					context, filePath, sourceFile, sourceMaps, program, checker, reportCoverage,
 					messageDetails, apiExtract, manifestContent
 				);
 				linter.getDeprecationText = () => "Deprecated test message";

test/lib/linter/rules/snapshots/renderer.ts.md

@@ -72,8 +72,8 @@ Generated by [AVA](https://avajs.dev).
         filePath: '2ControlRenderer.js',
         messages: [
           {
-            column: 3,
-            line: 3,
+            column: 6,
+            line: 2,
             message: 'Use of deprecated renderer detected. Define explicitly the {apiVersion: 2} parameter in the renderer object',
             messageDetails: '"Renderer Object (https://ui5.sap.com/#/topic/c9ab34570cc14ea5ab72a6d1a4a03e3f)",',
             ruleId: 'no-deprecated-api',
@@ -475,4 +475,68 @@ Generated by [AVA](https://avajs.dev).
         ],
         warningCount: 0,
       },
+      {
+        coverageInfo: [],
+        errorCount: 3,
+        fatalErrorCount: 0,
+        filePath: 'TSControl.ts',
+        messages: [
+          {
+            column: 3,
+            line: 19,
+            message: 'Use of deprecated renderer detected. Define explicitly the {apiVersion: 2} parameter in the renderer object',
+            messageDetails: '"Renderer Object (https://ui5.sap.com/#/topic/c9ab34570cc14ea5ab72a6d1a4a03e3f)",',
+            ruleId: 'no-deprecated-api',
+            severity: 2,
+          },
+          {
+            column: 20,
+            line: 36,
+            message: 'Use of deprecated renderer detected. Define explicitly the {apiVersion: 2} parameter in the renderer object',
+            messageDetails: '"Renderer Object (https://ui5.sap.com/#/topic/c9ab34570cc14ea5ab72a6d1a4a03e3f)",',
+            ruleId: 'no-deprecated-api',
+            severity: 2,
+          },
+          {
+            column: 20,
+            line: 53,
+            message: 'Use of deprecated renderer detected. Define explicitly the {apiVersion: 2} parameter in the renderer object',
+            messageDetails: '"Renderer Object (https://ui5.sap.com/#/topic/c9ab34570cc14ea5ab72a6d1a4a03e3f)",',
+            ruleId: 'no-deprecated-api',
+            severity: 2,
+          },
+          {
+            column: 2,
+            line: 68,
+            message: 'The control renderer of \'Test4\' must be a static property',
+            ruleId: 'ui5-class-declaration',
+            severity: 1,
+          },
+          {
+            column: 2,
+            line: 88,
+            message: 'The control renderer of \'Test5\' must be a static property',
+            ruleId: 'ui5-class-declaration',
+            severity: 1,
+          },
+        ],
+        warningCount: 2,
+      },
+      {
+        coverageInfo: [],
+        errorCount: 1,
+        fatalErrorCount: 0,
+        filePath: 'TSControlRenderer.ts',
+        messages: [
+          {
+            column: 32,
+            line: 2,
+            message: 'Use of deprecated renderer detected. Define explicitly the {apiVersion: 2} parameter in the renderer object',
+            messageDetails: '"Renderer Object (https://ui5.sap.com/#/topic/c9ab34570cc14ea5ab72a6d1a4a03e3f)",',
+            ruleId: 'no-deprecated-api',
+            severity: 2,
+          },
+        ],
+        warningCount: 0,
+      },
     ]