feat(Formatter): Detect globals and strings of formatters in bindings…

… (JS/TS) (#499)

src/linter/messages.ts

@@ -13,6 +13,7 @@ const RULES = {
 	"no-pseudo-modules": "no-pseudo-modules",
 	"parsing-error": "parsing-error",
 	"ui5-class-declaration": "ui5-class-declaration",
+	"unsupported-api-usage": "unsupported-api-usage",
 	"prefer-test-starter": "prefer-test-starter",
 } as const;
 
@@ -76,6 +77,7 @@ export enum MESSAGE {
 	REDUNDANT_VIEW_CONFIG_PROPERTY,
 	REPLACED_BOOTSTRAP_PARAM,
 	SPELLING_BOOTSTRAP_PARAM,
+	STRING_FOR_FORMATTER_VALUE_IN_JS,
 	SVG_IN_XML,
 	MISSING_CONTROL_RENDERER_DECLARATION,
 	CONTROL_RENDERER_DECLARATION_STRING,
@@ -553,6 +555,14 @@ export const MESSAGE_INFO = {
 		details: () => undefined,
 	},
 
+	[MESSAGE.STRING_FOR_FORMATTER_VALUE_IN_JS]: {
+		severity: LintMessageSeverity.Error,
+		ruleId: RULES["unsupported-api-usage"],
+
+		message: () => `Do not use strings for 'formatter' values in JavaScript and TypeScript.`,
+		details: () => `{@link topic:28fcd55b04654977b63dacbee0552712 See Best Practices for Developers}`,
+	},
+
 	[MESSAGE.SVG_IN_XML]: {
 		severity: LintMessageSeverity.Error,
 		ruleId: RULES["no-deprecated-api"],

src/linter/ui5Types/SourceFileLinter.ts

@@ -21,6 +21,7 @@ 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, {PropertyBindingInfo} from "../binding/lib/BindingParser.js";
 
 const log = getLogger("linter:ui5Types:SourceFileLinter");
 
@@ -846,7 +847,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 +858,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 +1183,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 +1197,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 +1207,14 @@ 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.STRING_FOR_FORMATTER_VALUE_IN_JS, prop.initializer);
+				} else {
+					propertyField = prop;
+				}
 			}
 
 			if (propertyField) {
@@ -1218,6 +1226,23 @@ 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);
+				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.NO_GLOBALS, {
+						variableName: formatter.split(".")[0],
+						namespace: formatter,
+					}, 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/rules/NoGlobals/FormatterGlobalBindingObject.js

@@ -0,0 +1,14 @@
+sap.ui.define(
+	["sap/m/Input", "ui5/walkthrough/model/formatter"],
+  	(Input, formatter) => {
+		// Should be detected as "unsupported-api-usage" (formatter is a string):
+		const input = new Input({
+			value: { path: 'invoice>Status', formatter: 'formatter.statusText' }
+		});
+
+		// Should NOT be detected:
+		const input2 = 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' }"
+		});
+	}
+);

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

@@ -4,6 +4,86 @@ The actual snapshot is saved in `NoGlobals.ts.snap`.
 
 Generated by [AVA](https://avajs.dev).
 
+## General: FormatterGlobalBindingObject.js
+
+> Snapshot 1
+
+    [
+      {
+        coverageInfo: [],
+        errorCount: 1,
+        fatalErrorCount: 0,
+        filePath: 'FormatterGlobalBindingObject.js',
+        messages: [
+          {
+            column: 48,
+            line: 6,
+            message: 'Do not use strings for \'formatter\' values in JavaScript and TypeScript.',
+            messageDetails: 'See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
+            ruleId: 'unsupported-api-usage',
+            severity: 2,
+          },
+        ],
+        warningCount: 0,
+      },
+    ]
+
+## General: FormatterGlobalBindingString.js
+
+> Snapshot 1
+
+    [
+      {
+        coverageInfo: [],
+        errorCount: 5,
+        fatalErrorCount: 0,
+        filePath: 'FormatterGlobalBindingString.js',
+        messages: [
+          {
+            column: 4,
+            line: 6,
+            message: 'Access of global variable \'ui5\' (ui5.walkthrough.model.formatter.statusText)',
+            messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
+            ruleId: 'no-globals',
+            severity: 2,
+          },
+          {
+            column: 4,
+            line: 9,
+            message: 'Access of global variable \'ui5\' (ui5.walkthrough.model.formatter.statusText)',
+            messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
+            ruleId: 'no-globals',
+            severity: 2,
+          },
+          {
+            column: 11,
+            line: 6,
+            message: 'Access of global variable \'ui5\' (ui5.walkthrough.model.formatter.statusText)',
+            messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
+            ruleId: 'no-globals',
+            severity: 2,
+          },
+          {
+            column: 11,
+            line: 9,
+            message: 'Access of global variable \'ui5\' (ui5.walkthrough.model.formatter.statusText)',
+            messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
+            ruleId: 'no-globals',
+            severity: 2,
+          },
+          {
+            column: 11,
+            line: 15,
+            message: 'Access of global variable \'formatter\' (formatter.statusText)',
+            messageDetails: 'Do not use global variables to access UI5 modules or APIs. See Best Practices for Developers (https://ui5.sap.com/#/topic/28fcd55b04654977b63dacbee0552712)',
+            ruleId: 'no-globals',
+            severity: 2,
+          },
+        ],
+        warningCount: 0,
+      },
+    ]
+
 ## General: ModelDataTypes.view.xml
 
 > Snapshot 1