feat: OData implicit globals detection (JS/TS) (#533)

Also improves the existing detection in XML which now also detects
multiple function calls within one binding.

JIRA: CPOUI5FOUNDATION-1008

src/linter/binding/BindingLinter.ts

@@ -2,9 +2,16 @@ import LinterContext, {PositionInfo, ResourcePath} from "../LinterContext.js";
 import {MESSAGE} from "../messages.js";
 import {RequireDeclaration} from "../xmlTemplate/Parser.js";
 import BindingParser, {
-	AggregationBindingInfo, BindingInfo, FilterInfo, PropertyBindingInfo, SorterInfo,
+	AggregationBindingInfo, BindingInfo, ExpressionBinding, FilterInfo, PropertyBindingInfo, SorterInfo,
 } from "./lib/BindingParser.js";
 
+const ODATA_EXPRESSION_ADDONS_MODULE = "sap/ui/model/odata/ODataExpressionAddons";
+const ODATA_FUNCTION_MODULE_MAP: Record<string, string> = {
+	compare: "sap/ui/model/odata/v4/ODataUtils",
+	fillUriTemplate: "sap/ui/thirdparty/URITemplate",
+	uriEncode: "sap/ui/model/odata/ODataUtils",
+} as const;
+
 export default class BindingLinter {
 	#resourcePath: string;
 	#context: LinterContext;
@@ -14,8 +21,9 @@ export default class BindingLinter {
 		this.#context = context;
 	}
 
-	#parseBinding(binding: string): BindingInfo {
-		const bindingInfo = BindingParser.complexParser(binding, null, true, true, true, true);
+	#parseBinding(binding: string): BindingInfo | string | undefined {
+		// Do not unescape (3rd argument), as we're only interested in the binding
+		const bindingInfo = BindingParser.complexParser(binding, null, false, true, true, true);
 		return bindingInfo;
 	}
 
@@ -131,8 +139,11 @@ export default class BindingLinter {
 	lintPropertyBinding(bindingDefinition: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
 		try {
 			const bindingInfo = this.#parseBinding(bindingDefinition);
-			if (bindingInfo) {
+			if (typeof bindingInfo === "object") {
 				this.#analyzePropertyBinding(bindingInfo as PropertyBindingInfo, requireDeclarations, position);
+				if ("tokens" in bindingInfo) {
+					this.#lintExpressionBindingTokens(bindingInfo, requireDeclarations, position);
+				}
 			}
 		} catch (err) {
 			const message = err instanceof Error ? err.message : String(err);
@@ -144,7 +155,7 @@ export default class BindingLinter {
 		bindingDefinition: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
 		try {
 			const bindingInfo = this.#parseBinding(bindingDefinition);
-			if (bindingInfo) {
+			if (typeof bindingInfo === "object") {
 				this.#analyzeAggregationBinding(bindingInfo as AggregationBindingInfo, requireDeclarations, position);
 			}
 		} catch (err) {
@@ -153,27 +164,35 @@ export default class BindingLinter {
 		}
 	}
 
-	#isExpressionBinding(bindingDefinition: string): boolean {
-		return /^\{:?=/.test(bindingDefinition) && bindingDefinition.endsWith("}");
-	}
-
-	lintPropertyExpression(
-		bindingDefinition: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
-		if (!this.#isExpressionBinding(bindingDefinition)) {
-			return;
-		}
-		const allFunctionsModule = "sap/ui/model/odata/ODataExpressionAddons";
-		const varModuleMap = {
-			"odata.compare": "sap/ui/model/odata/v4/ODataUtils",
-			"odata.fillUriTemplate": "sap/ui/thirdparty/URITemplate",
-			"odata.uriEncode": "sap/ui/model/odata/ODataUtils",
-		};
-
-		for (const [key, value] of Object.entries(varModuleMap)) {
-			if (bindingDefinition.includes(`${key}(`) &&
-				!requireDeclarations.some((decl) => decl.moduleName === value || value === allFunctionsModule)) {
-				this.#context.addLintingMessage(this.#resourcePath, MESSAGE.NO_ODATA_GLOBALS, {} as never, position);
+	#lintExpressionBindingTokens(
+		expressionBinding: ExpressionBinding, requireDeclarations: RequireDeclaration[], position: PositionInfo
+	) {
+		const {tokens} = expressionBinding;
+		for (let i = 0; i < tokens.length; i++) {
+			const token = tokens[i];
+			if (token.id !== "IDENTIFIER" || token.value !== "odata" || tokens[i + 1]?.id !== ".") {
+				continue;
+			}
+			const functionToken = tokens[i + 2];
+			if (functionToken?.id !== "IDENTIFIER") {
+				// Can't happen. A "." must be followed by an IDENTIFIER
+				continue;
+			}
+			const functionName = functionToken.value;
+			if (functionName in ODATA_FUNCTION_MODULE_MAP) {
+				const expectedModuleName = ODATA_FUNCTION_MODULE_MAP[functionName];
+				if (
+					// There must be either an import for the corresponding module
+					// or for the ODataExpressionAddons module
+					!requireDeclarations.some((decl) =>
+						decl.moduleName === expectedModuleName || decl.moduleName === ODATA_EXPRESSION_ADDONS_MODULE)
+				) {
+					this.#context.addLintingMessage(
+						this.#resourcePath, MESSAGE.NO_ODATA_GLOBALS, {} as never, position
+					);
+				}
 			}
+			i += 2; // Skip the next two tokens as we already checked them
 		}
 	}
 }

src/linter/binding/lib/BindingParser.d.ts

@@ -33,7 +33,16 @@ export interface AggregationBindingInfo extends BindingInfoBase {
 	groupHeaderFactory?: FunctionReference;
 }
 
-export type BindingInfo = PropertyBindingInfo | AggregationBindingInfo;
+interface ExpressionBindingToken {
+	id: "IDENTIFIER" | "."; // Only the ones required for UI5 linter
+	value: string;
+}
+
+export interface ExpressionBinding extends BindingInfoBase {
+	tokens: ExpressionBindingToken[];
+}
+
+export type BindingInfo = PropertyBindingInfo | AggregationBindingInfo | ExpressionBinding;
 
 interface BindingParser {
 	complexParser: (
@@ -45,7 +54,7 @@ interface BindingParser {
 		bPreferContext?: boolean,
 		mLocals?: Record<string, string>,
 		bResolveTypesAsync?: boolean
-	) => BindingInfo;
+	) => BindingInfo | string | undefined; // Might return a string when bUnescape is true
 }
 
 declare const BindingParser: BindingParser;

src/linter/binding/lib/ExpressionParser.js

@@ -929,7 +929,9 @@ export default {
 
 		/* UI5 LINTER MODIFICATION:
 			Disabled the next few formatter-related lines.
-			The ExpressionParser formatter function is not required for linting purposes
+			The ExpressionParser formatter function is not required for linting purposes.
+
+			Instead, the tokens are returned to allow linting the individual parts of the expression.
 		*/
 		// function formatter() {
 		// 	//turn separate parameters for parts into one (array like) parameter
@@ -939,7 +941,8 @@ export default {
 		return {
 			result: {
 				// formatter: formatter,
-				parts: oTokens.parts
+				parts: oTokens.parts,
+				tokens: oTokens.tokens
 			},
 			at: oResult.at || oTokens.at
 		};

src/linter/xmlTemplate/Parser.ts

@@ -561,10 +561,6 @@ export default class Parser {
 						line: prop.start.line + 1, // Add one to align with IDEs
 						column: prop.start.column + 1,
 					});
-					this.#bindingLinter.lintPropertyExpression(prop.value, this.#requireDeclarations, {
-						line: prop.start.line + 1, // Add one to align with IDEs
-						column: prop.start.column + 1,
-					});
 				} else if (this.#apiExtract.isAggregation(`${namespace}.${moduleName}`, prop.name)) {
 					this.#bindingLinter.lintAggregationBinding(prop.value, this.#requireDeclarations, {
 						line: prop.start.line + 1, // Add one to align with IDEs

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

@@ -0,0 +1,11 @@
+sap.ui.define(["sap/m/Text"], (Text) => {
+	const oText1 = new Text({
+		text: "{= odata.compare(%{myvalue1},%{myvalue2})}"
+	});
+	const oText2 = new Text({
+		text: "{= odata.fillUriTemplate(${myvalue1},${myvalue2})}"
+	});
+	oText2.applySettings({
+		text: "{= odata.uriEncode(%{myvalue1},'Edm.String') + ' - ' + odata.uriEncode(%{myvalue2},'Edm.String') }"
+	});
+});

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

@@ -0,0 +1,14 @@
+sap.ui.define(["sap/m/Text", "sap/ui/model/odata/ODataExpressionAddons"], (Text) => {
+	const oText1 = new Text({
+		text: "{= odata.compare(%{myvalue1},%{myvalue2})}"
+	});
+	const oText2 = new Text({
+		text: "{= odata.fillUriTemplate(${myvalue1},${myvalue2})}"
+	});
+	oText2.applySettings({
+		text: "{= odata.uriEncode(%{myvalue1},'Edm.String') + ' - ' + odata.uriEncode(%{myvalue2},'Edm.String') }"
+	});
+	oText2.applySettings({
+		text: `\\{"foo": "bar"}` // Escaped JSON string should not cause issues when checking for odata globals within a binding
+	});
+});

test/lib/linter/binding/BindingLinter.ts

@@ -347,6 +347,36 @@ test("XML Expression Binding with encoded ampersand", (t) => {
 	t.snapshot(linterContext.generateLintResult("/test.js"));
 });
 
+test("XML Expression Binding with odata function calls", (t) => {
+	const {bindingLinter, linterContext} = t.context;
+
+	bindingLinter.lintPropertyBinding(
+		"{= odata.uriEncode(%{myvalue1},'Edm.String') + ' - ' + odata.uriEncode(%{myvalue2},'Edm.String') }",
+		[], {line: 1, column: 1});
+
+	t.snapshot(linterContext.generateLintResult("/test.js"));
+});
+
+test("XML Expression Binding with unknown odata function call", (t) => {
+	const {bindingLinter, linterContext} = t.context;
+
+	bindingLinter.lintPropertyBinding(
+		"{= odata.foo(%{myvalue1},'Edm.String') }",
+		[], {line: 1, column: 1});
+
+	t.snapshot(linterContext.generateLintResult("/test.js"));
+});
+
+test("XML Expression Binding with nested unknown odata function call", (t) => {
+	const {bindingLinter, linterContext} = t.context;
+
+	bindingLinter.lintPropertyBinding(
+		"{= odata.foo.bar(%{myvalue1},'Edm.String') }",
+		[], {line: 1, column: 1});
+
+	t.snapshot(linterContext.generateLintResult("/test.js"));
+});
+
 test("Error Testing: XML Property Binding missing closing bracket", (t) => {
 	const {bindingLinter, linterContext} = t.context;
 
@@ -356,3 +386,23 @@ test("Error Testing: XML Property Binding missing closing bracket", (t) => {
 
 	t.snapshot(linterContext.generateLintResult("/test.js"));
 });
+
+test("Error Testing: Escaped JSON (escaped opening bracket)", (t) => {
+	const {bindingLinter, linterContext} = t.context;
+
+	bindingLinter.lintPropertyBinding(
+		`\\{"foo": "bar"}`,
+		[], {line: 1, column: 1});
+
+	t.snapshot(linterContext.generateLintResult("/test.js"));
+});
+
+test("Error Testing: Escaped JSON (escaped opening/closing brackets)", (t) => {
+	const {bindingLinter, linterContext} = t.context;
+
+	bindingLinter.lintPropertyBinding(
+		`\\{"foo": "bar"\\}`,
+		[], {line: 1, column: 1});
+
+	t.snapshot(linterContext.generateLintResult("/test.js"));
+});

test/lib/linter/binding/lib/BindingParser.ts

@@ -2,12 +2,12 @@ import anyTest, {TestFn} from "ava";
 import BindingParser, {BindingInfo} from "../../../../../src/linter/binding/lib/BindingParser.js";
 
 const test = anyTest as TestFn<{
-	parse: (string: string) => BindingInfo;
+	parse: (string: string) => BindingInfo | string | undefined;
 }>;
 
 test.before((t) => {
 	t.context.parse = function (string: string) {
-		return BindingParser.complexParser(string, null, true, true, true, true);
+		return BindingParser.complexParser(string, null, false, true, true, true);
 	};
 });
 
@@ -108,11 +108,37 @@ test("XML Binding: Expression Binding with an embedded composite binding", (t) =
 	const {parse} = t.context;
 
 	const res = parse(`{= %{/data/message}.length < 20
-      ? %{i18n>errorMsg} 
+      ? %{i18n>errorMsg}
       : %{parts: [
          {path: 'i18n>successMsg'},
          {path: '/data/today', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}},
          {path: '/data/tomorrow', type:'sap.ui.model.type.Date', constraints:{displayFormat:'Date'}}
       ], formatter: 'my.globalFormatter'}}`);
 	t.snapshot(res);
 });
+
+test("No binding: Just text", (t) => {
+	const {parse} = t.context;
+
+	const res = parse(`foo bar`);
+	t.snapshot(res);
+});
+
+test("No binding: Escaped JSON (escaped opening bracket)", (t) => {
+	const {parse} = t.context;
+
+	const res = parse(`\\{"foo": "bar"}`);
+	t.snapshot(res);
+});
+
+test("No binding: Escaped JSON (escaped opening/closing brackets)", (t) => {
+	const {parse} = t.context;
+
+	const res = parse(`\\{"foo": "bar"\\}`);
+	t.snapshot(res);
+});
+
+test("No binding: Escaped JSON (escaped opening/closing brackets), bUnescape=true", (t) => {
+	const res = BindingParser.complexParser(`\\{"foo": "bar"\\}`, null, /* bUnescape */ true, true, true, true);
+	t.snapshot(res);
+});