feat(directives): Add ui5lint-disable directives for XML, YAML and HTML (#623)

Follow up of #327
Resolves #419

JIRA: CPOUI5FOUNDATION-959

Author: RandomByte

README.md

@@ -331,6 +331,8 @@ UI5 linter supports directives similar to ESLint's configuration comments, allow
 
 ### Specifying Rules
 
+**JavaScript / TypeScript**
+
 You can disable specific rules by listing them after the directive. Rules must be separated by commas if several are given:
 
 * `/* ui5lint-disable no-deprecated-api */`
@@ -341,9 +343,23 @@ An explanation why a rule is disabled can be added after the rule name; it must
 
 * `// ui5lint-disable-next-line no-deprecated-api -- explanation`
 
+**XML / HTML**
+
+* `<!-- ui5lint-disable no-deprecated-api -->`
+* `<!-- ui5lint-disable-next-line -->`
+
+Note that in XML and HTML files, depending on your browser runtime, the use of double-hyphen `--` is usually not allowed, making it impossible to add explanations as part of the directive. Please use separate comment blocks for this purpose.
+
+Also, since comments can't be placed inside XML and HTML tags, you might need to use `ui5lint-disable` and `ui5lint-enable` directives outside of the tags instead of `ui5lint-disable-line` or `ui5lint-disable-next-line`.
+
+**YAML**
+
+* `# ui5lint-disable no-deprecated-api`
+* `# ui5lint-disable-next-line`
+
 ### Scope
 
-Directives are currently supported in JavaScript and TypeScript files only; they are **not** supported in XML, YAML, HTML, or any other type of file.
+Directives are currently supported in JavaScript and TypeScript files as well as XML, HTML and YAML files.
 
 ## Node.js API
 

src/linter/LinterContext.ts

@@ -3,7 +3,6 @@ import {createReader} from "@ui5/fs/resourceFactory";
 import {resolveLinks} from "../formatter/lib/resolveLinks.js";
 import {LintMessageSeverity, MESSAGE, MESSAGE_INFO} from "./messages.js";
 import {MessageArgs} from "./MessageArgs.js";
-import {Directive} from "./ui5Types/directives.js";
 import ts from "typescript";
 
 export type FilePattern = string; // glob patterns
@@ -100,6 +99,16 @@ export interface PositionRange {
 	end?: PositionInfo;
 }
 
+export type DirectiveAction = "enable" | "disable";
+export type DirectiveScope = "line" | "next-line" | undefined;
+export interface Directive {
+	action: DirectiveAction;
+	scope: DirectiveScope;
+	ruleNames: string[];
+	line: number;
+	column: number;
+}
+
 export interface LintMetadata {
 	// The metadata holds information to be shared across linters
 	directives: Set<Directive>;
@@ -216,6 +225,14 @@ export default class LinterContext {
 		this.getCoverageInfo(resourcePath).push(coverageInfo);
 	}
 
+	addDirective(resourcePath: ResourcePath, directive: Directive) {
+		const metadata = this.getMetadata(resourcePath);
+		if (!metadata.directives) {
+			metadata.directives = new Set<Directive>();
+		}
+		metadata.directives.add(directive);
+	}
+
 	#getMessageFromRawMessage<M extends MESSAGE>(rawMessage: RawLintMessage<M>): LintMessage {
 		const messageInfo = MESSAGE_INFO[rawMessage.id];
 		if (!messageInfo) {

src/linter/html/parser.ts

@@ -1,50 +1,62 @@
 import type {ReadStream} from "node:fs";
-import {SaxEventType, Tag as SaxTag} from "sax-wasm";
-import {parseXML} from "../../utils/xmlParser.js";
+import {SaxEventType, Tag as SaxTag, Text as SaxText} from "sax-wasm";
+import {extractDirective, parseXML} from "../../utils/xmlParser.js";
+import {Directive} from "../LinterContext.js";
 
 interface ExtractedTags {
 	scriptTags: SaxTag[];
 	stylesheetLinkTags: SaxTag[];
 }
+function parseTag(event: SaxEventType, tag: SaxTag, extractedTags: ExtractedTags) {
+	if (event === SaxEventType.OpenTag &&
+		tag.name === "link") {
+		if (tag.attributes.some((attr) => {
+			return (attr.name.value === "rel" &&
+				attr.value.value === "stylesheet");
+		})) {
+			extractedTags.stylesheetLinkTags.push(tag);
+		};
+	} else if (event === SaxEventType.CloseTag &&
+		tag.name === "script") {
+		const isJSScriptTag = tag.attributes.every((attr) => {
+			// The "type" attribute of the script tag should be
+			// 1. not set (default),
+			// 2. an empty string,
+			// 3. or a JavaScript MIME type (text/javascript)
+			// https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script/type#attribute_is_not_set_default_an_empty_string_or_a_javascript_mime_type
+			return attr.name.value !== "type" ||
+				(attr.name.value === "type" &&
+					["",
+						"module",
+						"text/javascript",
+						"application/javascript", /* legacy */
+					].includes(attr.value.value.toLowerCase()));
+		});
+		if (isJSScriptTag) {
+			extractedTags.scriptTags.push(tag);
+		}
+	}
+}
+
+function parseComment(comment: SaxText, directives: Set<Directive>) {
+	const directive = extractDirective(comment);
+	if (directive) {
+		directives.add(directive);
+	}
+}
 
 export async function extractHTMLTags(contentStream: ReadStream) {
 	const extractedTags: ExtractedTags = {
 		scriptTags: [],
 		stylesheetLinkTags: [],
 	};
+	const directives = new Set<Directive>();
 	await parseXML(contentStream, (event, tag) => {
-		if (!(tag instanceof SaxTag)) {
-			return;
-		}
-		const serializedTag = tag.toJSON() as SaxTag;
-		if (event === SaxEventType.OpenTag &&
-			serializedTag.name === "link") {
-			if (serializedTag.attributes.some((attr) => {
-				return (attr.name.value === "rel" &&
-					attr.value.value === "stylesheet");
-			})) {
-				extractedTags.stylesheetLinkTags.push(serializedTag);
-			};
-		} else if (event === SaxEventType.CloseTag &&
-			serializedTag.name === "script") {
-			const isJSScriptTag = serializedTag.attributes.every((attr) => {
-				// The "type" attribute of the script tag should be
-				// 1. not set (default),
-				// 2. an empty string,
-				// 3. or a JavaScript MIME type (text/javascript)
-				// https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script/type#attribute_is_not_set_default_an_empty_string_or_a_javascript_mime_type
-				return attr.name.value !== "type" ||
-					(attr.name.value === "type" &&
-						["",
-							"module",
-							"text/javascript",
-							"application/javascript", /* legacy */
-						].includes(attr.value.value.toLowerCase()));
-			});
-			if (isJSScriptTag) {
-				extractedTags.scriptTags.push(serializedTag);
-			}
+		if (tag instanceof SaxTag) {
+			parseTag(event, tag.toJSON() as SaxTag, extractedTags);
+		} else if (tag instanceof SaxText && event === SaxEventType.Comment) {
+			parseComment(tag.toJSON() as SaxText, directives);
 		}
 	});
-	return extractedTags;
+	return {extractedTags, directives};
 }

src/linter/html/transpiler.ts

@@ -13,8 +13,13 @@ export default async function transpileHtml(
 	try {
 		const taskEnd = taskStart("Transpile HTML", resourcePath, true);
 		const report = new HtmlReporter(resourcePath, context);
-		const {scriptTags, stylesheetLinkTags} = await extractHTMLTags(contentStream);
+		const {extractedTags, directives} = await extractHTMLTags(contentStream);
 
+		if (directives.size) {
+			context.getMetadata(resourcePath).directives = directives;
+		}
+
+		const {scriptTags, stylesheetLinkTags} = extractedTags;
 		const bootstrapTag = findBootstrapTag(scriptTags);
 
 		if (bootstrapTag) {

src/linter/ui5Types/directives.ts

@@ -1,8 +1,13 @@
 import ts from "typescript";
-import {LintMetadata} from "../LinterContext.js";
+import {Directive, DirectiveAction, DirectiveScope, LintMetadata} from "../LinterContext.js";
+
+interface PossibleDirective extends Directive {
+	pos: number;
+	length: number;
+}
 
 export function findDirectives(sourceFile: ts.SourceFile, metadata: LintMetadata) {
-	metadata.directives = new Set<Directive>();
+	metadata.directives ??= new Set<Directive>();
 
 	const possibleDirectives = collectPossibleDirectives(sourceFile);
 	if (possibleDirectives.size === 0) {
@@ -14,7 +19,7 @@ export function findDirectives(sourceFile: ts.SourceFile, metadata: LintMetadata
 }
 
 function traverseAndFindDirectives(
-	node: ts.Node, sourceText: string, possibleDirectives: Set<Directive>, confirmedDirectives: Set<Directive>
+	node: ts.Node, sourceText: string, possibleDirectives: Set<PossibleDirective>, confirmedDirectives: Set<Directive>
 ) {
 	findDirectivesAroundNode(node, sourceText, possibleDirectives, confirmedDirectives);
 	node.getChildren().forEach((child) => {
@@ -23,7 +28,7 @@ function traverseAndFindDirectives(
 }
 
 function findDirectivesAroundNode(
-	node: ts.Node, sourceText: string, possibleDirectives: Set<Directive>, confirmedDirectives: Set<Directive>
+	node: ts.Node, sourceText: string, possibleDirectives: Set<PossibleDirective>, confirmedDirectives: Set<Directive>
 ) {
 	/*
 		// This is a comment
@@ -87,29 +92,17 @@ function findDirectivesAroundNode(
 	it would be impossible to know whether the code in the second line is part of the directive's description or not.
 */
 /* eslint-disable max-len */
-const directiveRegex =
-/*  | ----------------------------------------------- Multi-line comments -------------------------------------------- | ------------------------------------------ Single-line comments ------------------------------------| */
+const DIRECTIVE_REGEX =
+/*  | ----------------------------------------------- Multi-line comments ----------------------------------------------- | ------------------------------------------ Single-line comments ----------------------------------------------------------------- | */
 	/\/\*\s*ui5lint-(enable|disable)(?:-((?:next-)?line))?(\s+(?:[\w-]+\s*,\s*)*(?:\s*[\w-]+))?\s*,?\s*(?:--[\s\S]*?)?\*\/|\/\/\s*ui5lint-(enable|disable)(?:-((?:next-)?line))?([ \t]+(?:[\w-]+[ \t]*,[ \t]*)*(?:[ \t]*[\w-]+))?[ \t]*,?[ \t]*(?:--.*)?$/mg;
-/*                  |CG #1: action |    | CG #2: scope    |  CG #3: rules                      |Dangling,| Description      |               |CG #4: action |    | CG #5: scope    | CG #6: rules                                       |Dangling,| Description | */
+/*                  |CG #1: action |    | CG #2: scope    |  CG #3: rules                      |Dangling,| Description    |               |CG #4: action |    | CG #5: scope    | CG #6: rules                                        |Dangling,| Description | */
 /* eslint-enable max-len */
 
-export type DirectiveAction = "enable" | "disable";
-export type DirectiveScope = "line" | "next-line" | undefined;
-export interface Directive {
-	action: DirectiveAction;
-	scope: DirectiveScope;
-	ruleNames: string[];
-	pos: number;
-	length: number;
-	line: number;
-	column: number;
-}
-
 export function collectPossibleDirectives(sourceFile: ts.SourceFile) {
 	const text = sourceFile.getFullText();
-	let match;
-	const comments = new Set<Directive>();
-	while ((match = directiveRegex.exec(text)) !== null) {
+	const comments = new Set<PossibleDirective>();
+	const matches = text.matchAll(DIRECTIVE_REGEX);
+	for (const match of matches) {
 		const action = (match[1] ?? match[4]) as DirectiveAction;
 		const scope = (match[2] ?? match[5]) as DirectiveScope;
 		const rules = match[3] ?? match[6];

src/linter/xmlTemplate/Parser.ts

@@ -3,17 +3,18 @@ import he from "he";
 import ViewGenerator from "./generator/ViewGenerator.js";
 import FragmentGenerator from "./generator/FragmentGenerator.js";
 import JSTokenizer from "./lib/JSTokenizer.js";
-import LinterContext, {PositionInfo} from "../LinterContext.js";
+import LinterContext, {Directive, PositionInfo} from "../LinterContext.js";
 import {TranspileResult} from "../LinterContext.js";
 import AbstractGenerator from "./generator/AbstractGenerator.js";
 import {getLogger} from "@ui5/logger";
 import {MESSAGE} from "../messages.js";
 import {ApiExtract} from "../../utils/ApiExtract.js";
 import ControllerByIdInfo from "./ControllerByIdInfo.js";
 import BindingLinter from "../binding/BindingLinter.js";
-import {Tag as SaxTag} from "sax-wasm";
+import {Tag as SaxTag, Text as SaxText} from "sax-wasm";
 import EventHandlerResolver from "./lib/EventHandlerResolver.js";
 import BindingParser from "../binding/lib/BindingParser.js";
+import {extractDirective} from "../../utils/xmlParser.js";
 const log = getLogger("linter:xmlTemplate:Parser");
 
 export type Namespace = string;
@@ -132,6 +133,7 @@ export default class Parser {
 	#xmlDocumentKind: DocumentKind;
 
 	#context: LinterContext;
+	#directives = new Set<Directive>();
 	#namespaceStack: NamespaceStackEntry[] = [];
 	#nodeStack: NodeDeclaration[] = [];
 
@@ -187,7 +189,18 @@ export default class Parser {
 		this._removeNamespacesForLevel(level);
 	}
 
+	parseComment(comment: SaxText) {
+		const directive = extractDirective(comment);
+		if (directive) {
+			this.#directives.add(directive);
+		}
+	}
+
 	generate(): TranspileResult {
+		if (this.#directives.size) {
+			// Add directives to the context
+			this.#context.getMetadata(this.#resourcePath).directives = this.#directives;
+		}
 		const {source, map} = this.#generator.getModuleContent();
 		return {
 			source,

src/linter/xmlTemplate/transpiler.ts

@@ -1,4 +1,4 @@
-import {SaxEventType, SAXParser, Tag as SaxTag} from "sax-wasm";
+import {SaxEventType, SAXParser, Tag as SaxTag, Text as SaxText} from "sax-wasm";
 import {ReadStream} from "node:fs";
 import fs from "node:fs/promises";
 import {finished} from "node:stream/promises";
@@ -62,15 +62,17 @@ async function transpileXmlToJs(
 	const parser = new Parser(resourcePath, apiExtract, context, controllerByIdInfo);
 
 	// Initialize parser
-	const saxParser = new SAXParser(SaxEventType.OpenTag | SaxEventType.CloseTag);
+	const saxParser = new SAXParser(SaxEventType.OpenTag | SaxEventType.CloseTag | SaxEventType.Comment);
 
-	saxParser.eventHandler = (event, tag) => {
-		if (tag instanceof SaxTag) {
+	saxParser.eventHandler = (event, data) => {
+		if (data instanceof SaxTag) {
 			if (event === SaxEventType.OpenTag) {
-				parser.pushTag(tag.toJSON() as SaxTag);
+				parser.pushTag(data.toJSON() as SaxTag);
 			} else if (event === SaxEventType.CloseTag) {
-				parser.popTag(tag.toJSON() as SaxTag);
+				parser.popTag(data.toJSON() as SaxTag);
 			}
+		} else if (data instanceof SaxText && event === SaxEventType.Comment) {
+			parser.parseComment(data.toJSON() as SaxText);
 		}
 	};
 

src/linter/yaml/YamlLinter.ts

@@ -1,4 +1,4 @@
-import LinterContext from "../LinterContext.js";
+import LinterContext, {DirectiveAction, DirectiveScope} from "../LinterContext.js";
 import {deprecatedLibraries, deprecatedThemeLibraries} from "../../utils/deprecations.js";
 import {DataWithPosition, fromYaml, getPosition} from "data-with-position";
 import {MESSAGE} from "../messages.js";
@@ -20,6 +20,8 @@ interface YamlWithPosInfo extends DataWithPosition {
 		};
 	};
 }
+// This regex is derived from the single-line variant defined in ui5types/directives.ts
+const DIRECTIVE_REGEX = /#\s*ui5lint-(enable|disable)(?:-((?:next-)?line))?([ \t]+(?:[\w-]+[ \t]*,[ \t]*)*(?:[ \t]*[\w-]+))?[ \t]*,?[ \t]*(?:--.*)?$/mg;
 
 export default class YamlLinter {
 	#content;
@@ -45,6 +47,8 @@ export default class YamlLinter {
 				const parsedYamlWithPosInfo: YamlWithPosInfo = this.#parseYaml(document);
 				// Analyze part content with line number offset
 				this.#analyzeYaml(parsedYamlWithPosInfo, lineNumberOffset);
+				this.#collectDirectives(document, lineNumberOffset);
+
 				// Update line number offset for next part
 				lineNumberOffset += document.split(/\r\n|\r|\n/g).length;
 			});
@@ -79,4 +83,29 @@ export default class YamlLinter {
 			}
 		});
 	}
+
+	#collectDirectives(content: string, offset: number) {
+		const matches = content.matchAll(DIRECTIVE_REGEX);
+		for (const match of matches) {
+			const action = (match[1] ?? match[4]) as DirectiveAction;
+			const scope = (match[2] ?? match[5]) as DirectiveScope;
+			const rules = match[3] ?? match[6];
+
+			let ruleNames = rules?.split(",") ?? [];
+			ruleNames = ruleNames.map((rule) => rule.trim());
+
+			// Determine line and column of match
+			const left = content.slice(0, match.index);
+			const line = (left.match(/\n/g) ?? []).length + 1 + offset;
+			const lastIndexOf = left.lastIndexOf("\n") + 1;
+			const column = match.index - lastIndexOf + 1;
+
+			this.#context.addDirective(this.#resourcePath, {
+				action,
+				scope, ruleNames,
+				line,
+				column,
+			});
+		}
+	}
 }