Skip to content

Commit 5d2d34c

Browse files
committed
refactor(linter/plugins): move report function and diagnostic types to separate file
1 parent c4abee4 commit 5d2d34c

File tree

5 files changed

+175
-164
lines changed

5 files changed

+175
-164
lines changed

apps/oxlint/src-js/index.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,10 @@ import type { SourceCode } from './plugins/source_code.ts';
44
import type { BeforeHook, Visitor, VisitorWithHooks } from './plugins/types.ts';
55

66
export type * as ESTree from './generated/types.d.ts';
7-
export type {
8-
Context,
9-
Diagnostic,
10-
DiagnosticBase,
11-
DiagnosticWithLoc,
12-
DiagnosticWithNode,
13-
LanguageOptions,
14-
} from './plugins/context.ts';
7+
export type { Context, LanguageOptions } from './plugins/context.ts';
158
export type { Fix, Fixer, FixFn } from './plugins/fix.ts';
169
export type { CreateOnceRule, CreateRule, Plugin, Rule } from './plugins/load.ts';
10+
export type { Diagnostic, DiagnosticBase, DiagnosticWithLoc, DiagnosticWithNode } from './plugins/report.ts';
1711
export type {
1812
Definition,
1913
DefinitionType,

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

Lines changed: 6 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -26,55 +26,23 @@
2626
* and global variables (`filePath`, `settings`, `cwd`).
2727
*/
2828

29-
import { getFixes } from './fix.js';
30-
import { getOffsetFromLineColumn } from './location.js';
3129
import { ast, initAst, SOURCE_CODE } from './source_code.js';
30+
import { report } from './report.js';
3231
import { settings, initSettings } from './settings.js';
3332

34-
import type { Fix, FixFn } from './fix.ts';
3533
import type { RuleDetails } from './load.ts';
34+
import type { Diagnostic } from './report.ts';
3635
import type { SourceCode } from './source_code.ts';
37-
import type { Location, Ranged } from './location.ts';
3836
import type { ModuleKind } from '../generated/types.d.ts';
3937

40-
const { hasOwn, keys: ObjectKeys, freeze, assign: ObjectAssign, create: ObjectCreate } = Object;
41-
42-
// Diagnostic in form passed by user to `Context#report()`
43-
export type Diagnostic = DiagnosticWithNode | DiagnosticWithLoc;
44-
45-
export interface DiagnosticBase {
46-
message?: string | null | undefined;
47-
messageId?: string | null | undefined;
48-
data?: Record<string, string | number> | null | undefined;
49-
fix?: FixFn;
50-
}
51-
52-
export interface DiagnosticWithNode extends DiagnosticBase {
53-
node: Ranged;
54-
}
55-
56-
export interface DiagnosticWithLoc extends DiagnosticBase {
57-
loc: Location;
58-
}
59-
60-
// Diagnostic in form sent to Rust
61-
interface DiagnosticReport {
62-
message: string;
63-
start: number;
64-
end: number;
65-
ruleIndex: number;
66-
fixes: Fix[] | null;
67-
}
68-
69-
// Diagnostics array. Reused for every file.
70-
export const diagnostics: DiagnosticReport[] = [];
38+
const { freeze, assign: ObjectAssign, create: ObjectCreate } = Object;
7139

7240
// Cached current working directory
7341
let cwd: string | null = null;
7442

7543
// Absolute path of file being linted.
7644
// When `null`, indicates that no file is currently being linted (in `createOnce`, or between linting files).
77-
let filePath: string | null = null;
45+
export let filePath: string | null = null;
7846

7947
/**
8048
* Set up context for linting a file.
@@ -391,124 +359,8 @@ export function createContext(fullRuleName: string, ruleDetails: RuleDetails): R
391359
* @throws {TypeError} If `diagnostic` is invalid
392360
*/
393361
report(diagnostic: Diagnostic): void {
394-
// Delegate to `reportImpl`, passing rule-specific details (`RuleDetails`)
395-
reportImpl(diagnostic, ruleDetails);
362+
// Delegate to `report` implementation shared between all rules, passing rule-specific details (`RuleDetails`)
363+
report(diagnostic, ruleDetails);
396364
},
397365
} as unknown as Context); // It seems TS can't understand `__proto__: FILE_CONTEXT`
398366
}
399-
400-
/**
401-
* Report error.
402-
* @param diagnostic - Diagnostic object
403-
* @param ruleDetails - `RuleDetails` object, containing rule-specific details e.g. `isFixable`
404-
* @throws {TypeError} If `diagnostic` is invalid
405-
*/
406-
function reportImpl(diagnostic: Diagnostic, ruleDetails: RuleDetails): void {
407-
if (filePath === null) throw new Error('Cannot report errors in `createOnce`');
408-
409-
// Get message, resolving message from `messageId` if present
410-
let message = getMessage(diagnostic, ruleDetails);
411-
412-
// Interpolate placeholders {{key}} with data values
413-
if (hasOwn(diagnostic, 'data')) {
414-
const { data } = diagnostic;
415-
if (data != null) {
416-
message = message.replace(/\{\{([^}]+)\}\}/g, (match, key) => {
417-
key = key.trim();
418-
const value = data[key];
419-
return value !== undefined ? String(value) : match;
420-
});
421-
}
422-
}
423-
424-
// TODO: Validate `diagnostic`
425-
let start: number, end: number, loc: Location;
426-
427-
if (hasOwn(diagnostic, 'loc') && (loc = (diagnostic as DiagnosticWithLoc).loc) != null) {
428-
// `loc`
429-
if (typeof loc !== 'object') throw new TypeError('`loc` must be an object');
430-
start = getOffsetFromLineColumn(loc.start);
431-
end = getOffsetFromLineColumn(loc.end);
432-
} else {
433-
// `node`
434-
const { node } = diagnostic as DiagnosticWithNode;
435-
if (node == null) throw new TypeError('Either `node` or `loc` is required');
436-
if (typeof node !== 'object') throw new TypeError('`node` must be an object');
437-
438-
// ESLint uses `loc` here instead of `range`.
439-
// We can't do that because AST nodes don't have `loc` property yet. In any case, `range` is preferable,
440-
// as otherwise we have to convert `loc` to `range` which is expensive at present.
441-
// TODO: Revisit this once we have `loc` support in AST, and a fast translation table to convert `loc` to `range`.
442-
const { range } = node;
443-
if (range === null || typeof range !== 'object') throw new TypeError('`node.range` must be present');
444-
start = range[0];
445-
end = range[1];
446-
447-
// Do type validation checks here, to ensure no error in serialization / deserialization.
448-
// Range validation happens on Rust side.
449-
if (
450-
typeof start !== 'number' ||
451-
typeof end !== 'number' ||
452-
start < 0 ||
453-
end < 0 ||
454-
(start | 0) !== start ||
455-
(end | 0) !== end
456-
) {
457-
throw new TypeError('`node.range[0]` and `node.range[1]` must be non-negative integers');
458-
}
459-
}
460-
461-
diagnostics.push({
462-
message,
463-
start,
464-
end,
465-
ruleIndex: ruleDetails.ruleIndex,
466-
fixes: getFixes(diagnostic, ruleDetails),
467-
});
468-
}
469-
470-
/**
471-
* Get message from diagnostic.
472-
* @param diagnostic - Diagnostic object
473-
* @param ruleDetails - `RuleDetails` object, containing rule-specific `messages`
474-
* @returns Message string
475-
* @throws {Error|TypeError} If neither `message` nor `messageId` provided, or of wrong type
476-
*/
477-
function getMessage(diagnostic: Diagnostic, ruleDetails: RuleDetails): string {
478-
if (hasOwn(diagnostic, 'messageId')) {
479-
const { messageId } = diagnostic as { messageId: string | null | undefined };
480-
if (messageId != null) return resolveMessageFromMessageId(messageId, ruleDetails);
481-
}
482-
483-
if (hasOwn(diagnostic, 'message')) {
484-
const { message } = diagnostic;
485-
if (typeof message === 'string') return message;
486-
if (message != null) throw new TypeError('`message` must be a string');
487-
}
488-
489-
throw new Error('Either `message` or `messageId` is required');
490-
}
491-
492-
/**
493-
* Resolve a message ID to its message string, with optional data interpolation.
494-
* @param messageId - The message ID to resolve
495-
* @param ruleDetails - `RuleDetails` object, containing rule-specific `messages`
496-
* @returns Resolved message string
497-
* @throws {Error} If `messageId` is not found in `messages`
498-
*/
499-
function resolveMessageFromMessageId(messageId: string, ruleDetails: RuleDetails): string {
500-
const { messages } = ruleDetails;
501-
if (messages === null) {
502-
throw new Error(`Cannot use messageId '${messageId}' - rule does not define any messages in \`meta.messages\``);
503-
}
504-
505-
if (!hasOwn(messages, messageId)) {
506-
throw new Error(
507-
`Unknown messageId '${messageId}'. Available \`messageIds\`: ${ObjectKeys(messages)
508-
.map((msg) => `'${msg}'`)
509-
.join(', ')}`,
510-
);
511-
}
512-
513-
return messages[messageId];
514-
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { assertIs } from './utils.js';
22

3-
import type { Diagnostic } from './context.ts';
43
import type { RuleDetails } from './load.ts';
54
import type { Range, Ranged } from './location.ts';
5+
import type { Diagnostic } from './report.ts';
66

77
const { prototype: ArrayPrototype, from: ArrayFrom } = Array,
88
{ getPrototypeOf, hasOwn, prototype: ObjectPrototype } = Object,

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { diagnostics, setupFileContext, resetFileContext } from './context.js';
1+
import { setupFileContext, resetFileContext } from './context.js';
22
import { registeredRules } from './load.js';
3+
import { diagnostics } from './report.js';
34
import { setSettingsForFile, resetSettings } from './settings.js';
45
import { ast, initAst, resetSourceAndAst, setupSourceForFile } from './source_code.js';
56
import { assertIs, getErrorMessage } from './utils.js';
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
/*
2+
* `report` function to report errors + diagnostic types.
3+
*/
4+
5+
import { filePath } from './context.js';
6+
import { getFixes } from './fix.js';
7+
import { getOffsetFromLineColumn } from './location.js';
8+
9+
import type { Fix, FixFn } from './fix.ts';
10+
import type { RuleDetails } from './load.ts';
11+
import type { Location, Ranged } from './location.ts';
12+
13+
const { hasOwn, keys: ObjectKeys } = Object;
14+
15+
/**
16+
* Diagnostic object.
17+
* Passed to `Context#report()`.
18+
*/
19+
// This is the type of the value passed to `Context#report()` by user.
20+
// `DiagnosticReport` (see below) is the type of diagnostics sent to Rust.
21+
export type Diagnostic = DiagnosticWithNode | DiagnosticWithLoc;
22+
23+
export interface DiagnosticBase {
24+
message?: string | null | undefined;
25+
messageId?: string | null | undefined;
26+
data?: Record<string, string | number> | null | undefined;
27+
fix?: FixFn;
28+
}
29+
30+
export interface DiagnosticWithNode extends DiagnosticBase {
31+
node: Ranged;
32+
}
33+
34+
export interface DiagnosticWithLoc extends DiagnosticBase {
35+
loc: Location;
36+
}
37+
38+
// Diagnostic in form sent to Rust
39+
interface DiagnosticReport {
40+
message: string;
41+
start: number;
42+
end: number;
43+
ruleIndex: number;
44+
fixes: Fix[] | null;
45+
}
46+
47+
// Diagnostics array. Reused for every file.
48+
export const diagnostics: DiagnosticReport[] = [];
49+
50+
/**
51+
* Report error.
52+
* @param diagnostic - Diagnostic object
53+
* @param ruleDetails - `RuleDetails` object, containing rule-specific details e.g. `isFixable`
54+
* @throws {TypeError} If `diagnostic` is invalid
55+
*/
56+
export function report(diagnostic: Diagnostic, ruleDetails: RuleDetails): void {
57+
if (filePath === null) throw new Error('Cannot report errors in `createOnce`');
58+
59+
// Get message, resolving message from `messageId` if present
60+
let message = getMessage(diagnostic, ruleDetails);
61+
62+
// Interpolate placeholders {{key}} with data values
63+
if (hasOwn(diagnostic, 'data')) {
64+
const { data } = diagnostic;
65+
if (data != null) {
66+
message = message.replace(/\{\{([^}]+)\}\}/g, (match, key) => {
67+
key = key.trim();
68+
const value = data[key];
69+
return value !== undefined ? String(value) : match;
70+
});
71+
}
72+
}
73+
74+
// TODO: Validate `diagnostic`
75+
let start: number, end: number, loc: Location;
76+
77+
if (hasOwn(diagnostic, 'loc') && (loc = (diagnostic as DiagnosticWithLoc).loc) != null) {
78+
// `loc`
79+
if (typeof loc !== 'object') throw new TypeError('`loc` must be an object');
80+
start = getOffsetFromLineColumn(loc.start);
81+
end = getOffsetFromLineColumn(loc.end);
82+
} else {
83+
// `node`
84+
const { node } = diagnostic as DiagnosticWithNode;
85+
if (node == null) throw new TypeError('Either `node` or `loc` is required');
86+
if (typeof node !== 'object') throw new TypeError('`node` must be an object');
87+
88+
// ESLint uses `loc` here instead of `range`.
89+
// We can't do that because AST nodes don't have `loc` property yet. In any case, `range` is preferable,
90+
// as otherwise we have to convert `loc` to `range` which is expensive at present.
91+
// TODO: Revisit this once we have `loc` support in AST, and a fast translation table to convert `loc` to `range`.
92+
const { range } = node;
93+
if (range === null || typeof range !== 'object') throw new TypeError('`node.range` must be present');
94+
start = range[0];
95+
end = range[1];
96+
97+
// Do type validation checks here, to ensure no error in serialization / deserialization.
98+
// Range validation happens on Rust side.
99+
if (
100+
typeof start !== 'number' ||
101+
typeof end !== 'number' ||
102+
start < 0 ||
103+
end < 0 ||
104+
(start | 0) !== start ||
105+
(end | 0) !== end
106+
) {
107+
throw new TypeError('`node.range[0]` and `node.range[1]` must be non-negative integers');
108+
}
109+
}
110+
111+
diagnostics.push({
112+
message,
113+
start,
114+
end,
115+
ruleIndex: ruleDetails.ruleIndex,
116+
fixes: getFixes(diagnostic, ruleDetails),
117+
});
118+
}
119+
120+
/**
121+
* Get message from diagnostic.
122+
* @param diagnostic - Diagnostic object
123+
* @param ruleDetails - `RuleDetails` object, containing rule-specific `messages`
124+
* @returns Message string
125+
* @throws {Error|TypeError} If neither `message` nor `messageId` provided, or of wrong type
126+
*/
127+
function getMessage(diagnostic: Diagnostic, ruleDetails: RuleDetails): string {
128+
if (hasOwn(diagnostic, 'messageId')) {
129+
const { messageId } = diagnostic as { messageId: string | null | undefined };
130+
if (messageId != null) return resolveMessageFromMessageId(messageId, ruleDetails);
131+
}
132+
133+
if (hasOwn(diagnostic, 'message')) {
134+
const { message } = diagnostic;
135+
if (typeof message === 'string') return message;
136+
if (message != null) throw new TypeError('`message` must be a string');
137+
}
138+
139+
throw new Error('Either `message` or `messageId` is required');
140+
}
141+
142+
/**
143+
* Resolve a message ID to its message string, with optional data interpolation.
144+
* @param messageId - The message ID to resolve
145+
* @param ruleDetails - `RuleDetails` object, containing rule-specific `messages`
146+
* @returns Resolved message string
147+
* @throws {Error} If `messageId` is not found in `messages`
148+
*/
149+
function resolveMessageFromMessageId(messageId: string, ruleDetails: RuleDetails): string {
150+
const { messages } = ruleDetails;
151+
if (messages === null) {
152+
throw new Error(`Cannot use messageId '${messageId}' - rule does not define any messages in \`meta.messages\``);
153+
}
154+
155+
if (!hasOwn(messages, messageId)) {
156+
throw new Error(
157+
`Unknown messageId '${messageId}'. Available \`messageIds\`: ${ObjectKeys(messages)
158+
.map((msg) => `'${msg}'`)
159+
.join(', ')}`,
160+
);
161+
}
162+
163+
return messages[messageId];
164+
}

0 commit comments

Comments
 (0)