feat(Formatter): Detect globals and strings for formatters in JS/TS

package.json

@@ -88,7 +88,7 @@
 		"@eslint/js": "^9.15.0",
 		"@istanbuljs/esm-loader-hook": "^0.3.0",
 		"@istanbuljs/nyc-config-typescript": "^1.0.2",
-		"@stylistic/eslint-plugin": "^3.0.0",
+		"@stylistic/eslint-plugin": "^3.0.1",
 		"@types/he": "^1.2.3",
 		"@types/node": "20.11.0",
 		"@types/sinon": "^17.0.3",

src/linter/messages.ts

@@ -411,7 +411,7 @@ export const MESSAGE_INFO = {
 		fatal: true,
 
 		message: ({message}: {message: string}) => message,
-		details: () => `Check the source file for syntax errors`,
+		details: ({details}: {details?: string}) => details ? details : `Check the source file for syntax errors`,
 	},
 
 	[MESSAGE.PARTIALLY_DEPRECATED_CORE_ROUTER]: {

src/linter/ui5Types/SourceFileLinter.ts

@@ -21,6 +21,8 @@ import type {ApiExtract} from "../../utils/ApiExtract.js";
 import {findDirectives} from "./directives.js";
 import BindingLinter from "../binding/BindingLinter.js";
 import {RequireDeclaration} from "../xmlTemplate/Parser.js";
+import BindingParser from "../binding/lib/BindingParser.js";
+import {PropertyBindingInfo} from "sap/ui/base/ManagedObject";
 
 const log = getLogger("linter:ui5Types:SourceFileLinter");
 
@@ -846,7 +848,7 @@ export default class SourceFileLinter {
 			} else if (["bindProperty", "bindAggregation"].includes(symbolName) &&
 				moduleName === "sap/ui/base/ManagedObject" &&
 				node.arguments[1] && ts.isObjectLiteralExpression(node.arguments[1])) {
-				this.#analyzePropertyBindings(node.arguments[1], ["type"]);
+				this.#analyzePropertyBindings(node.arguments[1], ["type", "formatter"]);
 			} else if (symbolName.startsWith("bind") &&
 				nodeType.symbol?.declarations?.some((declaration) =>
 					this.isUi5ClassDeclaration(declaration, "sap/ui/core/Control")) &&
@@ -857,7 +859,7 @@ export default class SourceFileLinter {
 				const alternativePropName = propName.charAt(0).toLowerCase() + propName.slice(1);
 
 				if (this.#isPropertyBinding(node, [propName, alternativePropName])) {
-					this.#analyzePropertyBindings(node.arguments[0], ["type"]);
+					this.#analyzePropertyBindings(node.arguments[0], ["type", "formatter"]);
 				}
 			}
 		}
@@ -1182,7 +1184,7 @@ export default class SourceFileLinter {
 						this.#isPropertyBinding(node, [getPropertyNameText(prop.name) ?? ""]))
 				) {
 					if (ts.isObjectLiteralExpression(prop.initializer)) {
-						this.#analyzePropertyBindings(prop.initializer, ["type"]);
+						this.#analyzePropertyBindings(prop.initializer, ["type", "formatter"]);
 					} else {
 						this.#analyzePropertyStringBindings(prop);
 					}
@@ -1196,7 +1198,7 @@ export default class SourceFileLinter {
 				return;
 			}
 
-			// Get the type property
+			// Get the property inside the binding
 			let propertyField;
 			if (ts.isObjectLiteralExpression(prop.initializer)) {
 				propertyField = getPropertyAssignmentsInObjectLiteralExpression(
@@ -1206,7 +1208,19 @@ export default class SourceFileLinter {
 				// Whether it's a direct property of the Control
 				// or name collision in property binding
 				!ts.isNewExpression(prop.parent.parent)) {
-				propertyField = prop;
+				/* Special Case (JS/TS): If the value of the property 'formatter' is a string,
+				it should be detected since the runtime cannot resolve it
+				even if a 'formatter' variable is imported: */
+				if (prop.name.getText() === "formatter") {
+					this.#reporter.addMessage(MESSAGE.PARSING_ERROR, {
+						message: `Value of property 'formatter' is of type 'string'.`,
+						details:
+							`Do not use strings for 'formatter' values in JavaScript and TypeScript. ` +
+							`{@link topic:28fcd55b04654977b63dacbee0552712 See Best Practices for Developers}`,
+					}, prop.initializer);
+				} else {
+					propertyField = prop;
+				}
 			}
 
 			if (propertyField) {
@@ -1218,6 +1232,26 @@ export default class SourceFileLinter {
 	#analyzePropertyStringBindings(node: ts.PropertyAssignment) {
 		if (ts.isStringLiteralLike(node.initializer) &&
 			node.initializer.text.startsWith("{") && node.initializer.text.endsWith("}")) {
+			/* Special Case (JS/TS): If the value of the property 'formatter' is a string,
+			it should be detected since the runtime cannot resolve it
+			even if a 'formatter' variable is imported: */
+			try {
+				const bindingInfo = BindingParser.complexParser(node.initializer.text, null, true, true, true, true);
+				// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
+				const {formatter} = bindingInfo as PropertyBindingInfo;
+				// Check if formatter is of type string and report a message if so:
+				if (formatter && typeof formatter === "string") {
+					this.#reporter.addMessage(MESSAGE.PARSING_ERROR, {
+						message: `Value of property 'formatter' is of type 'string'.`,
+						details:
+							`Do not use strings for 'formatter' values in JavaScript and TypeScript. ` +
+							`{@link topic:28fcd55b04654977b63dacbee0552712 See Best Practices for Developers}`,
+					}, node.initializer);
+				}
+			} catch {
+				// Ignore errors since they are already reported by the BindingParser
+			}
+
 			const imports = this.sourceFile.statements
 				.filter((stmnt): stmnt is ts.ImportDeclaration =>
 					stmnt.kind === ts.SyntaxKind.ImportDeclaration)

test/fixtures/linter/projects/com.ui5.troublesome.app/ui5lint-custom-broken.config.cjs

@@ -1,4 +1,7 @@
-module.exports = {
-	ignores: [
+// CodeQL code scan complains about this file being broken.
+// This is a case we test for the config manager. So, wrapping it in a JSON.parse to avoid the error there,
+// but keep the test case valid.
+module.exports = JSON.parse(`{
+	"ignores": [
 		"test/**/*",
-		"!test/sap/m/visual/Wizard.spe
+		"!test/sap/m/visual/Wizard.spe`);

test/fixtures/linter/rules/NoGlobals/FormatterGlobalBindingObject.js

@@ -0,0 +1,8 @@
+sap.ui.define(
+	["sap/m/Input"],
+  	(Input) => {
+		const input = new Input({
+			value: { path: 'invoice>Status', formatter: 'formatter.statusText' }
+		});
+	}
+);

test/fixtures/linter/rules/NoGlobals/FormatterGlobalBindingString.js

@@ -0,0 +1,18 @@
+sap.ui.define(
+	["sap/m/Input", "ui5/walkthrough/model/formatter"],
+  	(Input, formatter) => {
+		// The following two cases using global notations should be detected:
+		const input = new Input({
+			value: "{ path: 'invoice>Status', formatter: 'ui5.walkthrough.model.formatter.statusText' }"
+		});
+		input.applySettings({
+			value: "{ path: 'invoice>Status', formatter: 'ui5.walkthrough.model.formatter.statusText' }",
+		});
+
+		// Although the formatter property does not look like a global notation,
+		// it should still be detected if it is a string:
+		const input2 = new Input({
+			value: "{ path: 'invoice>Status', formatter: 'formatter.statusText' }"
+		});
+	}
+);