refactor: Improve parsing-error handling

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;
@@ -566,15 +567,38 @@ export default class Parser {
 				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 templates, it's possible to have bindings in event handlers
-					// We need to parse and lint these as well
-					this.#bindingLinter.lintPropertyBinding(prop.value, this.#requireDeclarations, position);
+					// 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;
 						}
+
+						// 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
@@ -611,6 +635,11 @@ export default class Parser {
 							}, position);
 						}
 					});
+
+					if (!isValidEventHandler && propertyBindingParsingErrorMessage) {
+						this.#bindingLinter.reportParsingError(propertyBindingParsingErrorMessage, position);
+					}
+
 				// Treat every other xml attribute as property and check for bindings. XML templates can have such cases
 				} else { // if (this.#apiExtract.isProperty(symbolName, prop.name))
 					this.#bindingLinter.lintPropertyBinding(prop.value, this.#requireDeclarations, position);

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/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>

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

@@ -91,7 +91,7 @@ Generated by [AVA](https://avajs.dev).
     [
       {
         coverageInfo: [],
-        errorCount: 6,
+        errorCount: 7,
         fatalErrorCount: 0,
         filePath: 'GlobalEventHandlers.view.xml',
         messages: [
@@ -167,6 +167,14 @@ Generated by [AVA](https://avajs.dev).
             ruleId: 'no-globals',
             severity: 2,
           },
+          {
+            column: 10,
+            line: 22,
+            message: 'Access of global variable \'global\' (global.doSomething)',
+            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: 3,
       },

test/lib/linter/xmlTemplate/snapshots/transpiler.ts.md

@@ -378,6 +378,88 @@ Generated by [AVA](https://avajs.dev).
 
     Map {}
 
+## Transpile EventHandlerParsingErrors.view.xml
+
+> source
+
+    `import Button from "sap/m/Button";␊
+    import Button2 from "sap/m/Button";␊
+    import Button3 from "sap/m/Button";␊
+    import Button4 from "sap/m/Button";␊
+    import Button5 from "sap/m/Button";␊
+    import Helper from "com/myapp/Helper";␊
+    import View from "sap/ui/core/mvc/View";␊
+    const oButton = new Button({␊
+        press: "{ parts: [{path: 'facet>Target'}, {path: 'facet>Label'},],formatter: 'Helper.formatEventHandler'}",␊
+    });␊
+    ␊
+    const oButton2 = new Button2({␊
+        press: ".onPressFancyButton($event, { text: ${i18n>TEXT}, })",␊
+    });␊
+    ␊
+    const oButton3 = new Button3({␊
+        press: "{ parts: [{path: 'facet>Target'}, {path: 'facet>Label'}],formatter: 'Helper.formatEventHandler'}",␊
+    });␊
+    ␊
+    const oButton4 = new Button4({␊
+        press: ".onPressFancyButton($event, { text: ${i18n>TEXT} })",␊
+    });␊
+    ␊
+    const oButton5 = new Button5({␊
+        press: "{:= '.onPressFancyButton(\\\\'' + ${meta>/@sapui.name} + '\\\\')' }",␊
+    });␊
+    ␊
+    const oView = new View({␊
+        content: [␊
+            oButton,␊
+            oButton2,␊
+            oButton3,␊
+            oButton4,␊
+            oButton5,␊
+        ],␊
+    });␊
+    ␊
+    export default oView;`
+
+> map
+
+    {
+      file: 'EventHandlerParsingErrors.view.js',
+      mappings: 'AAOC,kCAAoH;AAGpH,mCAAuE;AAGvE,mCAAmH;AAGnH,mCAAsE;AAGtE,mCAAgF;AAhBhF,sCAAiD;AAHlD,wCAIC;gBAGA,YAAoH;IAA5G,2GAAyG;;;iBAGjH,aAAuE;IAA/D,8DAA4D;;;iBAGpE,aAAmH;IAA3G,0GAAwG;;;iBAGhH,aAAsE;IAA9D,6DAA2D;;;iBAGnE,aAAgF;IAAxE,yEAAqE;;;cAnB9E,UAIC;IAGA,OAAoH',
+      names: [],
+      sources: [
+        'EventHandlerParsingErrors.view.xml',
+      ],
+      version: 3,
+    }
+
+> messages
+
+    [
+      {
+        column: 10,
+        fatal: true,
+        line: 8,
+        message: 'Unexpected \']\'',
+        messageDetails: 'Check the source file for syntax errors',
+        ruleId: 'parsing-error',
+        severity: 2,
+      },
+      {
+        column: 10,
+        fatal: true,
+        line: 11,
+        message: 'Unexpected \'$\'',
+        messageDetails: 'Check the source file for syntax errors',
+        ruleId: 'parsing-error',
+        severity: 2,
+      },
+    ]
+
+> controllerByIdInfo
+
+    Map {}
+
 ## Transpile IgnoredNamespaces.fragment.xml
 
 > source