fix: Event binding in XML Templates

src/linter/binding/BindingLinter.ts

@@ -184,15 +184,24 @@ export default class BindingLinter {
 		return variableName;
 	}
 
-	lintPropertyBinding(bindingDefinition: string, requireDeclarations: RequireDeclaration[], position: PositionInfo) {
+	lintPropertyBinding(
+		bindingDefinition: string, requireDeclarations: RequireDeclaration[],
+		position: PositionInfo, returnParsingError = false
+	) {
 		try {
 			const bindingInfo = this.#parseBinding(bindingDefinition);
 			if (bindingInfo) {
 				this.#lintPropertyBindingInfo(bindingInfo, requireDeclarations, position);
 			}
 		} catch (err) {
 			const message = err instanceof Error ? err.message : String(err);
-			this.#context.addLintingMessage(this.#resourcePath, MESSAGE.PARSING_ERROR, {message}, position);
+			if (returnParsingError) {
+				// Return the parsing error message instead of adding it as a linting message
+				// so that the caller can decided whether to report it or not
+				return message;
+			} else {
+				this.reportParsingError(message, position);
+			}
 		}
 	}
 
@@ -272,4 +281,8 @@ export default class BindingLinter {
 			i += 2; // Skip the next two tokens as we already checked them
 		}
 	}
+
+	reportParsingError(message: string, position: PositionInfo) {
+		this.#context.addLintingMessage(this.#resourcePath, MESSAGE.PARSING_ERROR, {message}, position);
+	}
 }

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

@@ -58,6 +58,13 @@ interface BindingParser {
 		mLocals?: Record<string, string>,
 		bResolveTypesAsync?: boolean
 	) => BindingInfo | string | undefined; // Might return a string when bUnescape is true
+
+	parseExpression: (
+		sInput: string,
+		iStart: number,
+		oEnv: object,
+		mLocals: object
+	) => void;
 }
 
 declare const BindingParser: BindingParser;

src/linter/xmlTemplate/Parser.ts

@@ -13,6 +13,7 @@ import ControllerByIdInfo from "./ControllerByIdInfo.js";
 import BindingLinter from "../binding/BindingLinter.js";
 import {Tag as SaxTag} from "sax-wasm";
 import EventHandlerResolver from "./lib/EventHandlerResolver.js";
+import BindingParser from "../binding/lib/BindingParser.js";
 const log = getLogger("linter:xmlTemplate:Parser");
 
 export type Namespace = string;
@@ -568,18 +569,50 @@ export default class Parser {
 				} else if (this.#apiExtract.isAggregation(symbolName, prop.name)) {
 					this.#bindingLinter.lintAggregationBinding(prop.value, this.#requireDeclarations, position);
 				} else if (this.#apiExtract.isEvent(symbolName, prop.name)) {
+					// In XML templating, it's possible to have bindings in event handlers
+					// We need to parse and lint these as well, but the error should only be reported in case the event
+					// handler is not parsable. This prevents duplicate error messages for the same event handler.
+					const propertyBindingParsingErrorMessage =
+						this.#bindingLinter.lintPropertyBinding(prop.value, this.#requireDeclarations, position, true);
+
+					let isValidEventHandler = true;
+
 					EventHandlerResolver.parse(prop.value).forEach((eventHandler) => {
 						if (eventHandler.startsWith("cmd:")) {
 							// No global usage possible via command execution
 							return;
 						}
-						let functionName;
-						const openBracketIndex = eventHandler.indexOf("(");
-						if (openBracketIndex !== -1) {
-							functionName = eventHandler.slice(0, openBracketIndex);
-						} else {
-							functionName = eventHandler;
+
+						// Try parsing the event handler expression to check whether a property binding parsing error
+						// should be reported or not.
+						// This prevents false-positive errors in case a valid event handler declaration is not parsable
+						// as a binding.
+						// TODO: The parsed expression could also be used to check for global references in the future
+						if (propertyBindingParsingErrorMessage && isValidEventHandler) {
+							try {
+								BindingParser.parseExpression(eventHandler.replace(/^\./, "$controller."), 0, {
+									bTolerateFunctionsNotFound: true,
+								}, {});
+							} catch (_err) {
+								isValidEventHandler = false;
+								// For now we don't report errors here, as it's likely that the input is a valid
+								// binding expression that is processed during XML templating which at runtime
+								// then results into a valid event handler.
+							}
 						}
+
+						// Check for a valid function/identifier name
+						// Currently XML views support the following syntaxes that are covered with this pattern:
+						// - myFunction
+						// - .myFunction
+						// - my.namespace.myFunction
+						// - my.namespace.myFunction(arg1, ${i18n>key}, "test")
+						const validFunctionName = /^(\.?[$_\p{ID_Start}][$\p{ID_Continue}]*(?:\.[$_\p{ID_Start}][$\p{ID_Continue}]*)*)\s*(?:\(|$)/u;
+						const match = validFunctionName.exec(eventHandler);
+						if (!match) {
+							return;
+						}
+						const functionName = match[1];
 						const variableName = this.#bindingLinter.getGlobalReference(
 							functionName, this.#requireDeclarations
 						);
@@ -604,6 +637,10 @@ export default class Parser {
 							}, position);
 						}
 					});
+
+					if (!isValidEventHandler && propertyBindingParsingErrorMessage) {
+						this.#bindingLinter.reportParsingError(propertyBindingParsingErrorMessage, position);
+					}
 				}
 			}
 			// This node declares a control

test/fixtures/linter/rules/NoGlobals/GlobalEventHandlers.view.xml

@@ -18,4 +18,7 @@
 	<!-- Mixed: local, local without leading dot, command execution and global handler -->
 	<Button press="onPressFancyButton(0, ${i18n>TEXT}, $controller.foo.bar, $event); .onPressFancyButton; cmd:Save; global.doSomething($event, $controller)" />
 
+	<!-- Global handler with object argument (not parsable as a PropertyBinding) -->
+	<Button press="global.doSomething($event, { text: ${i18n>TEXT} })" />
+
 </mvc:View>

test/fixtures/linter/rules/NoGlobals/GlobalEventHandlers_Negative.view.xml

@@ -21,4 +21,7 @@
 	<!-- Usage of event handler function imported via core:require -->
 	<Button core:require='{ "doSomething": "ui5/walkthrougth/utils/doSomething" }' press="doSomething" />
 
+	<!-- Event handler with object argument (not parsable as a PropertyBinding) -->
+	<Button press=".onPressFancyButton($event, { text: ${i18n>TEXT} })" />
+
 </mvc:View>

test/fixtures/linter/rules/NoGlobals/Templates.fragment.xml

@@ -0,0 +1,50 @@
+<core:FragmentDefinition xmlns="sap.m" xmlns:core="sap.ui.core"
+    xmlns:template="http://schemas.sap.com/sapui5/extension/sap.ui.core.template/1">
+
+    <template:if test="{= ${dataField>RecordType} === 'com.sap.vocabularies.UI.v1.DataFieldForAction'}">
+        <template:then>
+            <template:if test="{path: 'dataField>', formatter: 'AH.doesActionExist'}">
+                <Button
+                    id="{parts: [{path: 'dataField>'}, {path: 'facet>'}], formatter: 'AH.getStableIdPartForDatafieldActionButton'}{= ${parameter>/settings/quickVariantSelectionX} ? ${path: 'tabItem>', formatter: 'AH.getSuffixFromIconTabFilterKey'} : ''}{= (${chartItem>}) ? '::chart' : ''}"
+                    text="{path: 'dataField>Label', formatter: 'AHModel.format'}"
+                    press="{parts: [{path: 'parameter>/settings'}, {path: 'dataField>'}], formatter: 'AH.getAnnotatedActionPress'}"
+                    enabled="{parts: [{path: 'dataField>'}, {path: 'facet>'}, {path: 'entityType>'}, {path: 'device>/system/phone'}, {path: 'tabItem>'},{path: 'chartItem>'}], formatter: 'AH.buildAnnotatedActionButtonEnablementExpression'}"
+                    class="{parts: [{path: 'dataField>'}, {path: 'facet>'}, {path: 'entityType>'}, {path: 'device>/system/phone'}, {path: 'tabItem>'},{path: 'chartItem>'}], formatter: 'AH.buildAnnotatedActionButtonEnablementExpression'}"
+                    type="Transparent"
+                    visible="{path: 'dataField>', formatter: 'AH.getBindingForHiddenPath'}">
+                    <layoutData>
+                        <OverflowToolbarLayoutData priority="{path: 'dataField>com.sap.vocabularies.UI.v1.Importance/EnumMember', formatter: 'AH.getToolbarButtonPriorityFromImportance'}"/>
+                    </layoutData>
+                    <customData>
+                        <core:CustomData key="Type" value="{dataField>RecordType}" />
+                        <core:CustomData key="Action" value="{path: 'dataField>Action', formatter: 'AHModel.format'}" />
+                        <core:CustomData key="Label" value="{path: 'dataField>Label', formatter: 'AHModel.format'}" />
+                        <core:CustomData key="InvocationGrouping" value="{= ${dataField>InvocationGrouping/EnumMember}}"/>
+                        <template:with path="dataField>Action" helper="AHModel.gotoFunctionImport" var="action">
+                            <core:CustomData key="ActionFor" value="{action>sap:action-for}" />
+                        </template:with>
+                    </customData>
+                </Button>
+            </template:if>
+        </template:then>
+        <template:elseif test="{= ${dataField>RecordType} === 'com.sap.vocabularies.UI.v1.DataFieldForIntentBasedNavigation'}">
+            <Button
+                id="{parts: [{path: 'dataField>'}, {path: 'facet>'}], formatter: 'AH.getStableIdPartForDatafieldActionButton'}{= ${parameter>/settings/quickVariantSelectionX} ? ${path: 'tabItem>', formatter: 'AH.getSuffixFromIconTabFilterKey'} : ''}{= (${chartItem>}) ? '::chart' : ''}"
+                text="{path: 'dataField>Label', formatter: 'AHModel.format'}"
+                press="{parts: [{path: 'parameter>/settings'}, {path: 'dataField>'}, {path: 'parameter>/manifest'}], formatter: 'AH.getAnnotatedActionPress'}"
+                enabled="{parts: [{path: 'dataField>'}, {path: 'facet>'}, {path: 'entityType>'}, {path: 'device>/system/phone'}, {path: 'tabItem>'}, {path: 'chartItem>'}], formatter: 'AH.buildAnnotatedActionButtonEnablementExpression'}"
+                visible="{path: 'dataField>', formatter: 'AH.buildVisibilityExprOfDataFieldForIntentBasedNaviButton'}"
+                type="Transparent">
+                <layoutData>
+                    <OverflowToolbarLayoutData priority="{path: 'dataField>com.sap.vocabularies.UI.v1.Importance/EnumMember', formatter: 'AH.getToolbarButtonPriorityFromImportance'}"/>
+                </layoutData>
+                <customData>
+                    <core:CustomData key="Type" value="{dataField>RecordType}" />
+                    <core:CustomData key="SemanticObject" value="{path: 'dataField>SemanticObject', formatter: 'AHModel.format'}" />
+                    <core:CustomData key="Action" value="{path: 'dataField>Action', formatter: 'AHModel.format'}" />
+                    <core:CustomData key="Label" value="{path: 'dataField>Label', formatter: 'AHModel.format'}" />
+                </customData>
+            </Button>
+        </template:elseif>
+    </template:if>
+</core:FragmentDefinition>

test/fixtures/transpiler/xml/EventHandlerParsingErrors.view.xml

@@ -0,0 +1,22 @@
+<mvc:View xmlns:mvc="sap.ui.core.mvc"
+	xmlns="sap.m"
+	xmlns:template="http://schemas.sap.com/sapui5/extension/sap.ui.core.template/1"
+	template:require="{ Helper: 'com/myapp/Helper' }"
+>
+
+	<!-- Invalid binding (trailing comma) that is resolved during XML templating at runtime -->
+	<Button press="{ parts: [{path: 'facet>Target'}, {path: 'facet>Label'},],formatter: 'Helper.formatEventHandler'}" />
+
+	<!-- Invalid event handler (trailing comma) -->
+	<Button press=".onPressFancyButton($event, { text: ${i18n>TEXT}, })" />
+
+	<!-- Valid binding that is resolved during XML templating at runtime -->
+	<Button press="{ parts: [{path: 'facet>Target'}, {path: 'facet>Label'}],formatter: 'Helper.formatEventHandler'}" />
+
+	<!-- Valid event handler, but not parsable as property binding. This should not lead to any parsing error -->
+	<Button press=".onPressFancyButton($event, { text: ${i18n>TEXT} })" />
+
+	<!-- Invalid event handler, parsable as property binding. This should not lead to any parsing error -->
+	<Button press="{:= '.onPressFancyButton(\'' + ${meta>/@sapui.name} + '\')' }" />
+
+</mvc:View>