Skip to content

Commit 1af1091

Browse files
committed
refactor(linter/plugins): store messageId in diagnostics (#16296)
Store `messageId` in `DiagnosticReport` objects. It's not required for sending to Rust, but it is needed for rule tester (#16206). I think it's fine to add a little code that's redundant in the main runtime, because it only has an overhead when a rule produces diagnostics, which should be an uncommon path.
1 parent 9de386d commit 1af1091

File tree

2 files changed

+27
-8
lines changed

2 files changed

+27
-8
lines changed

apps/oxlint/src-js/plugins/lint.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import { walkProgram } from '../generated/walk.js';
2222
// @ts-expect-error we need to generate `.d.ts` file for this module
2323
import { walkProgram } from "../generated/walk.js";
2424

25+
import type { SetOptional } from "type-fest";
26+
import type { DiagnosticReport } from "../plugins/report.ts";
2527
import type { AfterHook, BufferWithArrays } from "./types.ts";
2628

2729
// Buffers cache.
@@ -61,6 +63,12 @@ export function lintFile(
6163

6264
try {
6365
lintFileImpl(filePath, bufferId, buffer, ruleIds, optionsIds, settingsJSON);
66+
67+
// Remove `messageId` field from diagnostics. It's not needed on Rust side.
68+
for (let i = 0, len = diagnostics.length; i < len; i++) {
69+
(diagnostics[i] as SetOptional<DiagnosticReport, "messageId">).messageId = undefined;
70+
}
71+
6472
return JSON.stringify({ Success: diagnostics });
6573
} catch (err) {
6674
return JSON.stringify({ Failure: getErrorMessage(err) });

apps/oxlint/src-js/plugins/report.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ const { hasOwn, keys: ObjectKeys } = Object;
2121
* - Either `node` or `loc` property must be provided.
2222
*/
2323
// This is the type of the value passed to `Context#report()` by user.
24-
// `DiagnosticReport` (see below) is the type of diagnostics sent to Rust.
24+
// `DiagnosticReport` (see below) is the type of diagnostics used internally on JS side, and sent to Rust.
2525
export type Diagnostic = RequireAtLeastOne<
2626
RequireAtLeastOne<DiagnosticBase, "node" | "loc">,
2727
"message" | "messageId"
@@ -55,13 +55,15 @@ interface SuggestionBase {
5555
data?: DiagnosticData | null | undefined;
5656
}
5757

58-
// Diagnostic in form sent to Rust
59-
interface DiagnosticReport {
58+
// Diagnostic in form sent to Rust.
59+
// Actually, the `messageId` field is removed before sending to Rust.
60+
export interface DiagnosticReport {
6061
message: string;
6162
start: number;
6263
end: number;
6364
ruleIndex: number;
6465
fixes: Fix[] | null;
66+
messageId: string | null;
6567
}
6668

6769
// Diagnostics array. Reused for every file.
@@ -81,7 +83,7 @@ export function report(diagnostic: Diagnostic, ruleDetails: RuleDetails): void {
8183
if (filePath === null) throw new Error("Cannot report errors in `createOnce`");
8284

8385
// Get message, resolving message from `messageId` if present
84-
let message = getMessage(diagnostic, ruleDetails);
86+
let { message, messageId } = getMessage(diagnostic, ruleDetails);
8587

8688
// Interpolate placeholders {{key}} with data values
8789
if (hasOwn(diagnostic, "data")) {
@@ -129,6 +131,7 @@ export function report(diagnostic: Diagnostic, ruleDetails: RuleDetails): void {
129131

130132
diagnostics.push({
131133
message,
134+
messageId,
132135
start,
133136
end,
134137
ruleIndex: ruleDetails.ruleIndex,
@@ -140,18 +143,26 @@ export function report(diagnostic: Diagnostic, ruleDetails: RuleDetails): void {
140143
* Get message from diagnostic.
141144
* @param diagnostic - Diagnostic object
142145
* @param ruleDetails - `RuleDetails` object, containing rule-specific `messages`
143-
* @returns Message string
146+
* @returns Message string and `messageId`
144147
* @throws {Error|TypeError} If neither `message` nor `messageId` provided, or of wrong type
145148
*/
146-
function getMessage(diagnostic: Diagnostic, ruleDetails: RuleDetails): string {
149+
function getMessage(
150+
diagnostic: Diagnostic,
151+
ruleDetails: RuleDetails,
152+
): { message: string; messageId: string | null } {
147153
if (hasOwn(diagnostic, "messageId")) {
148154
const { messageId } = diagnostic;
149-
if (messageId != null) return resolveMessageFromMessageId(messageId, ruleDetails);
155+
if (messageId != null) {
156+
return {
157+
message: resolveMessageFromMessageId(messageId, ruleDetails),
158+
messageId,
159+
};
160+
}
150161
}
151162

152163
if (hasOwn(diagnostic, "message")) {
153164
const { message } = diagnostic;
154-
if (typeof message === "string") return message;
165+
if (typeof message === "string") return { message, messageId: null };
155166
if (message != null) throw new TypeError("`message` must be a string");
156167
}
157168

0 commit comments

Comments
 (0)