Skip to content

Commit 1aead13

Browse files
committed
refactor(linter/plugins): improve handling Context method calls in createOnce (#14032)
Refactor detection of when `Context` has not been set up for a file yet. * Use `ruleIndex: -1` as sentinel, instead of `filePath: ''`. SMIs are cheaper to check, and it's more reliable. * Move logic for checking this into a function, to avoid repeated code.
1 parent 8083740 commit 1aead13

File tree

1 file changed

+30
-16
lines changed

1 file changed

+30
-16
lines changed

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

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,23 @@ export let setupContextForFile: (
3535
filePath: string,
3636
) => void;
3737

38+
/**
39+
* Get internal data from `Context`.
40+
*
41+
* Throws an `Error` if `Context` has not been set up for a file (in body of `createOnce`).
42+
*
43+
* We have to define this function within class body, as it's not possible to access private property
44+
* `#internal` from outside the class.
45+
* We don't use a normal class method, because we don't want to expose this to user.
46+
* We don't use a private class method, because private property/method accesses are somewhat expensive.
47+
*
48+
* @param context - `Context` object
49+
* @param actionDescription - Description of the action being attempted. Used in error message if context is not set up.
50+
* @returns `InternalContext` object
51+
* @throws {Error} If context has not been set up
52+
*/
53+
let getInternal: (context: Context, actionDescription: string) => InternalContext;
54+
3855
// Internal data within `Context` that don't want to expose to plugins.
3956
// Stored as `#internal` property of `Context`.
4057
interface InternalContext {
@@ -66,51 +83,42 @@ export class Context {
6683
this.#internal = {
6784
id: fullRuleName,
6885
filePath: '',
69-
ruleIndex: 0,
86+
ruleIndex: -1,
7087
options: [],
7188
};
7289
}
7390

7491
// Getter for full rule name, in form `<plugin>/<rule>`
7592
get id() {
76-
const internal = this.#internal;
77-
if (internal.filePath === '') throw new Error('Cannot access `context.id` in `createOnce`');
78-
return internal.id;
93+
return getInternal(this, 'access `context.id`').id;
7994
}
8095

8196
// Getter for absolute path of file being linted.
8297
get filename() {
83-
const { filePath } = this.#internal;
84-
if (filePath === '') throw new Error('Cannot access `context.filename` in `createOnce`');
85-
return filePath;
98+
return getInternal(this, 'access `context.filename`').filePath;
8699
}
87100

88101
// Getter for absolute path of file being linted.
89102
// TODO: Unclear how this differs from `filename`.
90103
get physicalFilename() {
91-
const { filePath } = this.#internal;
92-
if (filePath === '') throw new Error('Cannot access `context.physicalFilename` in `createOnce`');
93-
return filePath;
104+
return getInternal(this, 'access `context.physicalFilename`').filePath;
94105
}
95106

96107
// Getter for options for file being linted.
97108
get options() {
98-
const internal = this.#internal;
99-
if (internal.filePath === '') throw new Error('Cannot access `context.options` in `createOnce`');
100-
return internal.options;
109+
return getInternal(this, 'access `context.options`').options;
101110
}
102111

103112
/**
104113
* Report error.
105114
* @param diagnostic - Diagnostic object
106115
*/
107116
report(diagnostic: Diagnostic): void {
108-
const internal = this.#internal;
109-
if (internal.filePath === '') throw new Error('Cannot report errors in `createOnce`');
117+
const { ruleIndex } = getInternal(this, 'report errors');
110118
diagnostics.push({
111119
message: diagnostic.message,
112120
loc: { start: diagnostic.node.start, end: diagnostic.node.end },
113-
ruleIndex: internal.ruleIndex,
121+
ruleIndex,
114122
});
115123
}
116124

@@ -121,5 +129,11 @@ export class Context {
121129
internal.ruleIndex = ruleIndex;
122130
internal.filePath = filePath;
123131
};
132+
133+
getInternal = (context, actionDescription) => {
134+
const internal = context.#internal;
135+
if (internal.ruleIndex === -1) throw new Error(`Cannot ${actionDescription} in \`createOnce\``);
136+
return internal;
137+
};
124138
}
125139
}

0 commit comments

Comments
 (0)