refactor(autofix): Validate XML before and after autofix

Similar to the JS-autofix, we should validate XML for syntax errors
before and after applying fixes.

Add new dependency 'fast-xml-parser' which seems to have good reputation
and existing usage across SAP. It has only one transitive dependency and
overall reasonable footprint with ~600 KB install size. License is MIT.

JIRA: CPOUI5FOUNDATION-994

Author: RandomByte

npm-shrinkwrap.json

@@ -78,6 +78,7 @@
 		"@ui5/project": "^4.0.4",
 		"chalk": "^5.4.1",
 		"data-with-position": "^0.5.0",
+		"fast-xml-parser": "^5.2.5",
 		"figures": "^6.1.0",
 		"globals": "^16.2.0",
 		"he": "^1.2.0",

package.json

@@ -155,6 +155,15 @@ function getJsErrors(code: string, resourcePath: string) {
 	return getJsErrorsForSourceFile(sourceFile, program, host);
 }
 
+async function getXmlErrorForFile(content: string) {
+	const {XMLValidator} = await import("fast-xml-parser");
+	const validOrError = XMLValidator.validate(content);
+	if (validOrError === true) {
+		return; // No errors
+	}
+	return validOrError.err;
+}
+
 export function getFactoryPosition(moduleDeclaration: ExistingModuleDeclarationInfo): {start: number; end: number} {
 	const {moduleDeclaration: declaration} = moduleDeclaration;
 	const factory = "factory" in declaration ? declaration.factory : declaration.callback;
@@ -190,6 +199,23 @@ export default async function ({
 		}
 	}
 
+	const res: AutofixResult = new Map();
+	if (jsResources.length) {
+		log.verbose(`Applying autofixes for ${jsResources.length} JS resources`);
+		await autofixJs(jsResources, messages, context, res);
+	}
+	if (xmlResources.length) {
+		log.verbose(`Applying autofixes for ${xmlResources.length} XML resources`);
+		await autofixXml(xmlResources, messages, context, res);
+	}
+
+	return res;
+}
+
+async function autofixJs(
+	jsResources: Resource[], messages: Map<ResourcePath, RawLintMessage[]>, context: LinterContext,
+	res: AutofixResult
+): Promise<void> {
 	const sourceFiles: SourceFiles = new Map();
 	const jsResourcePaths = [];
 	for (const resource of jsResources) {
@@ -210,7 +236,6 @@ export default async function ({
 	const program = createProgram(jsResourcePaths, compilerHost);
 
 	const checker = program.getTypeChecker();
-	const res: AutofixResult = new Map();
 	for (const [resourcePath, sourceFile] of sourceFiles) {
 		// Checking for syntax errors in the original source file.
 		// We should not apply autofixes to files with syntax errors
@@ -246,26 +271,41 @@ export default async function ({
 			const jsErrors = getJsErrors(newContent, resourcePath);
 			if (jsErrors.length) {
 				const contentWithMarkers = newContent.split("\n");
-				const message = `Syntax error after applying autofix for '${resourcePath}':\n - ` +
-					jsErrors
-						.sort((a, b) => {
-							if (a.start === undefined || b.start === undefined) {
-								return 0;
-							}
-							return a.start - b.start;
-						}).map((d) => {
-							if (d.line !== undefined && d.character !== undefined) {
-								// Insert line below the finding and mark the character
-								const line = d.line + 1;
-								contentWithMarkers.splice(line, 0, " ".repeat(d.character) + "^");
-							}
-							let res = d.messageText;
-							if (d.codeSnippet) {
-								res += ` (\`${d.codeSnippet}\`)`;
-							}
-							return res;
-						}).join("\n - ");
+				let lineOffset = 0;
+				const message = `Syntax error after applying autofix for '${resourcePath}'. ` +
+					`This is likely a UI5 linter internal issue. Please check the verbose log. ` +
+					`Please report this using the bug report template: ` +
+					`https://github.com/UI5/linter/issues/new?template=bug-report.md`;
+				const errors = jsErrors
+					.sort((a, b) => {
+						if (a.start === undefined || b.start === undefined) {
+							return 0;
+						}
+						return a.start - b.start;
+					}).map((d) => {
+						let res = d.messageText;
+						if (d.line !== undefined && d.character !== undefined) {
+							// Prepend line and character
+							res = `${d.line + 1}:${d.character + 1} ${res}`;
+
+							// Fill contentWithMarkers array for debug logging
+							// Insert line below the finding and mark the character
+							const lineContent = contentWithMarkers[d.line + lineOffset];
+							// Count number of tabs until the character
+							const tabCount = lineContent.slice(0, d.character).split("\t").length - 1;
+							const leadingTabs = "\t".repeat(tabCount);
+							const markerLine = d.line + lineOffset + 1;
+							contentWithMarkers.splice(markerLine, 0,
+								leadingTabs + " ".repeat(d.character - tabCount) + "^");
+							lineOffset++;
+						}
+						if (d.codeSnippet) {
+							res += ` (\`${d.codeSnippet}\`)`;
+						}
+						return res;
+					}).join("\n - ");
 				log.verbose(message);
+				log.verbose(errors);
 				log.verbose(resourcePath + ":\n" + contentWithMarkers.join("\n"));
 				context.addLintingMessage(resourcePath, MESSAGE.AUTOFIX_ERROR, {message});
 			} else {
@@ -274,16 +314,51 @@ export default async function ({
 			}
 		}
 	}
+}
 
+async function autofixXml(
+	xmlResources: Resource[], messages: Map<ResourcePath, RawLintMessage[]>, context: LinterContext,
+	res: AutofixResult
+): Promise<void> {
 	for (const resource of xmlResources) {
 		const resourcePath = resource.getPath();
+		const existingXmlError = await getXmlErrorForFile(await resource.getString());
+		if (existingXmlError) {
+			log.verbose(`Skipping autofix for '${resourcePath}'. Syntax error reported in original source file:\n` +
+				`[${existingXmlError.line}:${existingXmlError.col}] ${existingXmlError.msg}`);
+			continue;
+		}
 		const newContent = await applyFixesXml(resource, messages.get(resourcePath)!);
-		if (newContent) {
-			res.set(resourcePath, newContent);
+		if (!newContent) {
+			continue;
 		}
-	}
 
-	return res;
+		const newXmlError = await getXmlErrorForFile(newContent);
+		if (newXmlError) {
+			const message = `Syntax error after applying autofix for '${resourcePath}'. ` +
+				`This is likely a UI5 linter internal issue. Please check the verbose log. ` +
+				`Please report this using the bug report template: ` +
+				`https://github.com/UI5/linter/issues/new?template=bug-report.md`;
+			const error = `Reported error (${newXmlError.line}:${newXmlError.col}): ${newXmlError.msg}`;
+			log.verbose(message);
+			log.verbose(error);
+			const contentWithMarkers = newContent.split("\n");
+			if (newXmlError.line !== undefined && newXmlError.col !== undefined) {
+				const line = newXmlError.line - 1;
+				// Insert line below the finding and mark the character
+				const lineContent = contentWithMarkers[line];
+				// Count number of tabs until the character
+				const tabCount = lineContent.slice(0, newXmlError.col).split("\t").length - 1;
+				const leadingTabs = "\t".repeat(tabCount);
+				const markerLine = line + 1;
+				contentWithMarkers.splice(markerLine, 0, leadingTabs + " ".repeat(newXmlError.col - tabCount) + "^");
+			}
+			log.verbose(resourcePath + ":\n" + contentWithMarkers.join("\n"));
+			context.addLintingMessage(resourcePath, MESSAGE.AUTOFIX_ERROR, {message});
+			continue;
+		}
+		res.set(resourcePath, newContent);
+	}
 }
 
 function applyFixesJs(

src/autofix/autofix.ts

@@ -0,0 +1,8 @@
+<mvc:View xmlns:mvc="sap.ui.core.mvc"
+	xmlns="sap.m"
+	controllerName="com.myapp.controller.Main"
+>
+	<Button id="segmented-button-inner" tap=".onButtonTap"/>
+	<Button id="segmented-button-inner" tap=".onButtonTap />
+
+</mvc:View>

test/fixtures/autofix/ParsingError.view.xml

@@ -3,11 +3,12 @@ import anyTest, {TestFn} from "ava";
 import sinonGlobal from "sinon";
 import esmock from "esmock";
 import autofix, {AutofixResource, ChangeAction, InsertChange} from "../../../src/autofix/autofix.js";
-import LinterContext from "../../../src/linter/LinterContext.js";
+import LinterContext, {PositionInfo} from "../../../src/linter/LinterContext.js";
 import {MESSAGE} from "../../../src/linter/messages.js";
 import type {addDependencies} from "../../../src/autofix/amdImports.js";
 import {createResource} from "@ui5/fs/resourceFactory";
-import Fix from "../../../src/linter/ui5Types/fix/Fix.js";
+import XmlEnabledFix from "../../../src/linter/ui5Types/fix/XmlEnabledFix.js";
+import {SaxEventType} from "sax-wasm";
 
 const test = anyTest as TestFn<{
 	sinon: sinonGlobal.SinonSandbox;
@@ -37,25 +38,31 @@ test.afterEach.always((t) => {
 	t.context.sinon.restore();
 });
 
-class TestFix extends Fix {
+class TestFix extends XmlEnabledFix {
+	constructor(private position: PositionInfo) {
+		super();
+	}
+
 	visitLinterNode() {
 		return true;
 	}
 
 	getNodeSearchParameters() {
 		return {
 			nodeTypes: [ts.SyntaxKind.PropertyAccessExpression, ts.SyntaxKind.ElementAccessExpression],
-			position: {
-				line: 1,
-				column: 1,
-			},
+			xmlEventTypes: [SaxEventType.Attribute],
+			position: this.position,
 		};
 	}
 
 	visitAutofixNode() {
 		return true;
 	}
 
+	visitAutofixXmlNode() {
+		return true;
+	}
+
 	getAffectedSourceCodeRange() {
 		return [];
 	}
@@ -89,7 +96,7 @@ class TestFix extends Fix {
 	}
 }
 
-test("autofix: Parsing error after applying fixes", async (t) => {
+test("autofix (JS): Parsing error after applying fixes", async (t) => {
 	const {autofix, linterContext} = t.context;
 
 	const resources = new Map<string, AutofixResource>();
@@ -108,7 +115,10 @@ test("autofix: Parsing error after applying fixes", async (t) => {
 					column: 1,
 				},
 				args: {},
-				fix: new TestFix(),
+				fix: new TestFix({
+					line: 1,
+					column: 1,
+				}),
 			},
 		],
 	});
@@ -133,10 +143,68 @@ test("autofix: Parsing error after applying fixes", async (t) => {
 				{
 					column: undefined,
 					line: undefined,
-					message: `Syntax error after applying autofix for '/resources/file.js':
- - Unexpected keyword or identifier. (\`s\`)
- - Declaration or statement expected. (\`)\`)
- - ')' expected.`,
+					message: `Syntax error after applying autofix for '/resources/file.js'. ` +
+						`This is likely a UI5 linter internal issue. Please check the verbose log. ` +
+						`Please report this using the bug report template: ` +
+						`https://github.com/UI5/linter/issues/new?template=bug-report.md`,
+					ruleId: "autofix-error",
+					severity: 1,
+				},
+			],
+		},
+	]);
+});
+
+test("autofix (XML): Parsing error after applying fixes", async (t) => {
+	const {autofix, linterContext} = t.context;
+
+	const resources = new Map<string, AutofixResource>();
+	resources.set("/resources/file.view.xml", {
+		resource: createResource({
+			path: "/resources/file.view.xml",
+			string: "<mvc:View xmlns:mvc='sap.ui.core.mvc' xmlns='sap.m'><Button text='Test'/></mvc:View>",
+		}),
+		messages: [
+			// Note: Message details don't need to be correct in this test case
+			// as we stub the addDependencies function
+			{
+				id: MESSAGE.DEPRECATED_PROPERTY,
+				position: {
+					line: 1,
+					column: 1,
+				},
+				args: {},
+				fix: new TestFix({
+					line: 1,
+					column: 11,
+				}),
+			},
+		],
+	});
+	const result = await autofix({
+		rootDir: "/",
+		context: linterContext,
+		resources,
+	});
+
+	t.truthy(result);
+	t.deepEqual(Array.from(result.keys()), []);
+
+	t.deepEqual(linterContext.generateLintResults(), [
+		{
+			coverageInfo: [],
+			errorCount: 0,
+			fatalErrorCount: 0,
+			warningCount: 1,
+			filePath: "/resources/file.view.xml",
+			messages: [
+				{
+					column: undefined,
+					line: undefined,
+					message: `Syntax error after applying autofix for '/resources/file.view.xml'. ` +
+						`This is likely a UI5 linter internal issue. Please check the verbose log. ` +
+						`Please report this using the bug report template: ` +
+						`https://github.com/UI5/linter/issues/new?template=bug-report.md`,
 					ruleId: "autofix-error",
 					severity: 1,
 				},