Skip to content

Commit

Permalink
refactor: Finalize message handling refactoring
Browse files Browse the repository at this point in the history
- Use new addMessage signature in all reporters
- Reduce duplicate code for addMessage by moving logic into
  LinterContext
- Only produce actual messages at the end of linting
  (Might slightly reduce peak memory usage)
  • Loading branch information
matz3 committed Sep 12, 2024
1 parent 18d80af commit e5d3f32
Show file tree
Hide file tree
Showing 29 changed files with 337 additions and 366 deletions.
2 changes: 1 addition & 1 deletion src/formatter/coverage.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import path from "node:path";
import {
LintResult,
LintMessageSeverity,
CoverageInfo,
CoverageCategory,
} from "../linter/LinterContext.js";
import {readFile} from "fs/promises";
import {LintMessageSeverity} from "../linter/messages.js";

const visualizedSpace = "\u00b7";
const visualizedTab = "\u00bb";
Expand Down
3 changes: 2 additions & 1 deletion src/formatter/markdown.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {LintMessageSeverity, LintResult, LintMessage} from "../linter/LinterContext.js";
import {LintResult, LintMessage} from "../linter/LinterContext.js";
import {LintMessageSeverity} from "../linter/messages.js";

export class Markdown {
format(lintResults: LintResult[], showDetails: boolean): string {
Expand Down
3 changes: 2 additions & 1 deletion src/formatter/text.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import chalk from "chalk";
import path from "node:path";
import {LintMessageSeverity, LintResult, LintMessage} from "../linter/LinterContext.js";
import {LintResult, LintMessage} from "../linter/LinterContext.js";
import {LintMessageSeverity} from "../linter/messages.js";

const detailsHeader = chalk.white.bold("Details:");

Expand Down
91 changes: 71 additions & 20 deletions src/linter/LinterContext.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {AbstractAdapter, AbstractReader} from "@ui5/fs";
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";

export type FilePath = string; // Platform-dependent path
export type ResourcePath = string; // Always POSIX
Expand All @@ -16,9 +18,10 @@ export interface LintResult {
warningCount: number;
}

export enum LintMessageSeverity {
Warning = 1,
Error = 2,
export interface RawLintMessage<M extends MESSAGE = MESSAGE> {
id: M;
args: MessageArgs[M];
position?: PositionInfo;
}

export interface LintMessage {
Expand Down Expand Up @@ -85,7 +88,7 @@ export interface LintMetadata {
export default class LinterContext {
#rootDir: string;
#namespace: string | undefined;
#messages = new Map<ResourcePath, LintMessage[]>();
#rawMessages = new Map<ResourcePath, RawLintMessage[]>();
#coverageInfo = new Map<ResourcePath, CoverageInfo[]>();
#metadata = new Map<ResourcePath, LintMetadata>();
#rootReader: AbstractReader | undefined;
Expand Down Expand Up @@ -147,21 +150,31 @@ export default class LinterContext {
this.#resourcePathsToLint?.push(resourcePath);
}

getLintingMessages(resourcePath: ResourcePath): LintMessage[] {
let messages = this.#messages.get(resourcePath);
if (!messages) {
messages = [];
this.#messages.set(resourcePath, messages);
getRawLintingMessages(resourcePath: ResourcePath): RawLintMessage[] {
let rawMessages = this.#rawMessages.get(resourcePath);
if (!rawMessages) {
rawMessages = [];
this.#rawMessages.set(resourcePath, rawMessages);
}
return messages;
return rawMessages;
}

addLintingMessage(resourcePath: ResourcePath, message: LintMessage) {
if (message.messageDetails) {
message.messageDetails = resolveLinks(message.messageDetails);
}
addLintingMessage<M extends MESSAGE>(
resourcePath: ResourcePath, id: M, args: MessageArgs[M]
): void;
addLintingMessage<M extends MESSAGE>(
resourcePath: ResourcePath, id: M, args: MessageArgs[M], position: PositionInfo
): void;
addLintingMessage<M extends MESSAGE>(
resourcePath: ResourcePath, id: M, args: MessageArgs[M], position?: PositionInfo
) {
this.getRawLintingMessages(resourcePath).push({id, args, position});
}

this.getLintingMessages(resourcePath).push(message);
addLintingMessages<M extends MESSAGE>(
resourcePath: ResourcePath, rawMessages: RawLintMessage<M>[]
) {
this.getRawLintingMessages(resourcePath).push(...rawMessages);
}

getCoverageInfo(resourcePath: ResourcePath): CoverageInfo[] {
Expand All @@ -177,13 +190,50 @@ export default class LinterContext {
this.getCoverageInfo(resourcePath).push(coverageInfo);
}

#getMessageFromRawMessage<M extends MESSAGE>(rawMessage: RawLintMessage<M>): LintMessage {
const messageInfo = MESSAGE_INFO[rawMessage.id];
if (!messageInfo) {
throw new Error(`Invalid message id '${rawMessage.id}'`);
}

const messageFunc = messageInfo.message as (args: MessageArgs[M]) => string;

const message: LintMessage = {
ruleId: messageInfo.ruleId,
severity: messageInfo.severity,
line: rawMessage.position ? rawMessage.position.line : undefined,
column: rawMessage.position ? rawMessage.position.column : undefined,
message: messageFunc(rawMessage.args),
};

if (this.#includeMessageDetails) {
const detailsFunc = messageInfo.details as (args: MessageArgs[M]) => string | undefined;
const messageDetails = detailsFunc(rawMessage.args);
if (messageDetails) {
message.messageDetails = resolveLinks(messageDetails);
}
}

if ("fatal" in messageInfo && typeof messageInfo.fatal === "boolean") {
message.fatal = messageInfo.fatal;
}

if (message.fatal && message.severity !== LintMessageSeverity.Error) {
throw new Error(`Reports flagged as "fatal" must be of severity "Error"`);
}

return message;
}

generateLintResult(resourcePath: ResourcePath): LintResult {
const messages = this.#messages.get(resourcePath) ?? [];
const rawMessages = this.#rawMessages.get(resourcePath) ?? [];
const coverageInfo = this.#coverageInfo.get(resourcePath) ?? [];
let errorCount = 0;
let warningCount = 0;
let fatalErrorCount = 0;
for (const message of messages) {

const messages: LintMessage[] = rawMessages.map((rawMessage) => {
const message = this.#getMessageFromRawMessage(rawMessage);
if (message.severity === LintMessageSeverity.Error) {
errorCount++;
if (message.fatal) {
Expand All @@ -192,7 +242,8 @@ export default class LinterContext {
} else {
warningCount++;
}
}
return message;
});

return {
filePath: resourcePath,
Expand All @@ -208,9 +259,9 @@ export default class LinterContext {
const lintResults: LintResult[] = [];
let resourcePaths;
if (this.#reportCoverage) {
resourcePaths = new Set([...this.#messages.keys(), ...this.#coverageInfo.keys()]).values();
resourcePaths = new Set([...this.#rawMessages.keys(), ...this.#coverageInfo.keys()]).values();
} else {
resourcePaths = this.#messages.keys();
resourcePaths = this.#rawMessages.keys();
}

for (const resourcePath of resourcePaths) {
Expand Down
11 changes: 11 additions & 0 deletions src/linter/MessageArgs.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import type {MESSAGE_INFO} from "./messages.js";

type ExtractArgs<F> = F extends (args: infer P) => unknown ? P : never;
type CombineArgs<M, D> = M & D extends object ? M & D : never;

export type MessageArgs = {
[K in keyof typeof MESSAGE_INFO]:
CombineArgs<
ExtractArgs<typeof MESSAGE_INFO[K]["message"]>, ExtractArgs<typeof MESSAGE_INFO[K]["details"]>
>;
};
32 changes: 13 additions & 19 deletions src/linter/dotLibrary/DotLibraryLinter.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import {LintMessageSeverity} from "../LinterContext.js";
import LinterContext from "../LinterContext.js";
import {deprecatedLibraries} from "../../utils/deprecations.js";
import {SaxEventType, Tag as SaxTag} from "sax-wasm";
import {parseXML} from "../../utils/xmlParser.js";
import {ReadStream} from "node:fs";
import {RULES, MESSAGES, formatMessage} from "../linterReporting.js";
import {MESSAGE} from "../messages.js";

export default class DotLibraryLinter {
#contentStream;
Expand All @@ -23,12 +22,7 @@ export default class DotLibraryLinter {
this.#analyzeDeprecatedLibs(dotLibraryDependencyTags);
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
this.#context.addLintingMessage(this.#resourcePath, {
severity: LintMessageSeverity.Error,
message,
ruleId: RULES["ui5-linter-parsing-error"],
fatal: true,
});
this.#context.addLintingMessage(this.#resourcePath, MESSAGE.PARSING_ERROR, {message});
}
}

Expand Down Expand Up @@ -64,19 +58,19 @@ export default class DotLibraryLinter {
#analyzeDeprecatedLibs(libs: SaxTag[]) {
// Check for deprecated libraries
libs.forEach((lib) => {
const {line, character: column} = lib.openStart;
// textNodes is always an array, but it might be empty
const libName = lib.textNodes[0]?.value;
const libraryName = lib.textNodes[0]?.value;

if (deprecatedLibraries.includes(libName)) {
this.#context.addLintingMessage(this.#resourcePath, {
ruleId: RULES["ui5-linter-no-deprecated-library"],
severity: LintMessageSeverity.Error,
fatal: undefined,
line: line + 1,
column: column + 1,
message: formatMessage(MESSAGES.SHORT__DEPRECATED_LIBRARY, libName),
});
if (deprecatedLibraries.includes(libraryName)) {
this.#context.addLintingMessage(
this.#resourcePath,
MESSAGE.DEPRECATED_LIBRARY,
{libraryName},
{
line: lib.openStart.line + 1,
column: lib.openStart.character + 1,
}
);
}
});
}
Expand Down
42 changes: 21 additions & 21 deletions src/linter/html/HtmlReporter.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import {Tag as SaxTag} from "sax-wasm";
import LinterContext, {CoverageInfo, LintMessage, LintMessageSeverity, ResourcePath} from "../LinterContext.js";
import {resolveLinks} from "../../formatter/lib/resolveLinks.js";

interface ReporterMessage extends LintMessage {
node: SaxTag;
}
import LinterContext, {CoverageInfo, ResourcePath} from "../LinterContext.js";
import {MESSAGE} from "../messages.js";
import {MessageArgs} from "../MessageArgs.js";

interface ReporterCoverageInfo extends CoverageInfo {
node: SaxTag;
Expand All @@ -19,24 +16,27 @@ export default class HtmlReporter {
this.#context = context;
}

addMessage({node, message, messageDetails, severity, ruleId, fatal = undefined}: ReporterMessage) {
if (fatal && severity !== LintMessageSeverity.Error) {
throw new Error(`Reports flagged as "fatal" must be of severity "Error"`);
addMessage<M extends MESSAGE>(id: M, args: MessageArgs[M], node: SaxTag): void;
addMessage<M extends MESSAGE>(id: M, node: SaxTag): void;
addMessage<M extends MESSAGE>(
id: M, argsOrNode?: MessageArgs[M] | SaxTag, node?: SaxTag
) {
if (!argsOrNode) {
throw new Error("Invalid arguments: Missing second argument");
}

let line = 0, column = 0;
if (node instanceof SaxTag) {
({line, character: column} = node.openStart);
let args: MessageArgs[M];
if (argsOrNode instanceof SaxTag) {
node = argsOrNode;
args = null as unknown as MessageArgs[M];
} else if (!node) {
throw new Error("Invalid arguments: Missing 'node'");
} else {
args = argsOrNode;
}

this.#context.addLintingMessage(this.#resourcePath, {
ruleId,
severity,
fatal,
line: line + 1,
column: column + 1,
message,
messageDetails: messageDetails ? resolveLinks(messageDetails) : undefined,
this.#context.addLintingMessage(this.#resourcePath, id, args, {
line: node.openStart.line + 1,
column: node.openStart.character + 1,
});
}

Expand Down
18 changes: 4 additions & 14 deletions src/linter/html/transpiler.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {ReadStream} from "node:fs";
import {extractJSScriptTags} from "./parser.js";
import HtmlReporter from "./HtmlReporter.js";
import LinterContext, {LintMessageSeverity, ResourcePath, TranspileResult} from "../LinterContext.js";
import LinterContext, {ResourcePath, TranspileResult} from "../LinterContext.js";
import {taskStart} from "../../utils/perf.js";
import {MESSAGE} from "../messages.js";

export default async function transpileHtml(
resourcePath: ResourcePath, contentStream: ReadStream, context: LinterContext
Expand All @@ -19,26 +20,15 @@ export default async function transpileHtml(
});

if (!hasSrc && tag.textNodes?.length > 0) {
report.addMessage({
node: tag,
severity: LintMessageSeverity.Warning,
ruleId: "ui5-linter-csp-unsafe-inline-script",
message: `Use of unsafe inline script`,
messageDetails: "{@link topic:fe1a6dba940e479fb7c3bc753f92b28c Content Security Policy}",
});
report.addMessage(MESSAGE.CSP_UNSAFE_INLINE_SCRIPT, tag);
}
});

taskEnd();
return {source: "", map: ""};
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
context.addLintingMessage(resourcePath, {
severity: LintMessageSeverity.Error,
message,
ruleId: "ui5-linter-parsing-error",
fatal: true,
});
context.addLintingMessage(resourcePath, MESSAGE.PARSING_ERROR, {message});
return;
}
}
49 changes: 0 additions & 49 deletions src/linter/linterReporting.ts

This file was deleted.

Loading

0 comments on commit e5d3f32

Please sign in to comment.