-
-
Notifications
You must be signed in to change notification settings - Fork 726
feat(linter/plugins): rule options #16215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 11 commits
4c19782
a3f1ba5
839d6af
26513bb
1b4cd57
a20747d
9701596
f56f662
ad6d9fd
88f5f37
4e03bc9
42eee5d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { setOptions } from "./options.js"; | ||
|
|
||
| /** | ||
| * Populates Rust-resolved configuration options on the JS side. | ||
| * Called from Rust side after all configuration options have been resolved. | ||
| * | ||
| * Note: the name `setupConfigs` is currently incorrect, as we only populate rule options. | ||
| * The intention is for this function to transfer all configurations in a multi-config workspace. | ||
| * The configuration relevant to each file would then be resolved on the JS side. | ||
| * | ||
| * @param optionsJSON - JSON serialization of an array containing all rule options across all configurations. | ||
| * @returns "ok" on success, or error message on failure | ||
| */ | ||
| export function setupConfigs(optionsJSON: string): string { | ||
| // TODO: setup settings using this function | ||
| try { | ||
| setOptions(optionsJSON); | ||
| // TODO: flesh out error handling. | ||
| // Currently, the only procedure that may fail is `JSON.parse()` | ||
| // in `setOptions()`, but it won't because the JSON string from | ||
| // the rust side is serde serialized. | ||
| return "ok"; | ||
| } catch (err) { | ||
| return err instanceof Error ? err.message : String(err); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| export { lintFile } from "./lint.js"; | ||
| export { loadPlugin } from "./load.js"; | ||
| export { setupConfigs } from "./config.js"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,7 @@ const PARSER_SERVICES_DEFAULT: Record<string, unknown> = Object.freeze({}); | |
| * @param bufferId - ID of buffer containing file data | ||
| * @param buffer - Buffer containing file data, or `null` if buffer with this ID was previously sent to JS | ||
| * @param ruleIds - IDs of rules to run on this file | ||
| * @param optionsIds - IDs of options to use for rules on this file | ||
| * @param settingsJSON - Settings for file, as JSON | ||
| * @returns Diagnostics or error serialized to JSON string | ||
| */ | ||
|
|
@@ -54,11 +55,9 @@ export function lintFile( | |
| bufferId: number, | ||
| buffer: Uint8Array | null, | ||
| ruleIds: number[], | ||
| optionsIds: number[], | ||
| settingsJSON: string, | ||
| ): string { | ||
| // TODO: Get `optionsIds` from Rust side | ||
| const optionsIds = ruleIds.map((_) => DEFAULT_OPTIONS_ID); | ||
|
|
||
| try { | ||
| lintFileImpl(filePath, bufferId, buffer, ruleIds, optionsIds, settingsJSON); | ||
| return JSON.stringify({ Success: diagnostics }); | ||
|
|
@@ -156,6 +155,9 @@ function lintFileImpl( | |
| // Set `options` for rule | ||
| const optionsId = optionsIds[i]; | ||
| debugAssert(optionsId < allOptions.length, "Options ID out of bounds"); | ||
|
|
||
| // If the rule has no user-provided options, use the plugin-provided default | ||
| // options (which falls back to `DEFAULT_OPTIONS`) | ||
| ruleDetails.options = | ||
| optionsId === DEFAULT_OPTIONS_ID ? ruleDetails.defaultOptions : allOptions[optionsId]; | ||
|
Comment on lines
+158
to
162
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @overlookmotel You mentioned this could be simplified to |
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,10 @@ export type Options = JsonValue[]; | |
| // Default rule options | ||
| export const DEFAULT_OPTIONS: Readonly<Options> = Object.freeze([]); | ||
|
|
||
| // All rule options | ||
| export const allOptions: Readonly<Options>[] = [DEFAULT_OPTIONS]; | ||
| // All rule options. | ||
| // Indexed by options ID sent alongside ruleId for each file. | ||
| // Element 0 is always the default options (empty array). | ||
| export let allOptions: Readonly<Options>[] = [DEFAULT_OPTIONS]; | ||
|
|
||
| // Index into `allOptions` for default options | ||
| export const DEFAULT_OPTIONS_ID = 0; | ||
|
|
@@ -128,3 +130,23 @@ function mergeValues(configValue: JsonValue, defaultValue: JsonValue): JsonValue | |
|
|
||
| return freeze(merged); | ||
| } | ||
|
|
||
| /** | ||
| * Set all external rule options. | ||
| * Called once from Rust after config building, before any linting occurs. | ||
| * @param optionsJson - JSON string of outer array of per-options arrays. | ||
| */ | ||
| export function setOptions(optionsJson: string): void { | ||
| const parsed = JSON.parse(optionsJson); | ||
| if (DEBUG) { | ||
| if (!isArray(parsed)) | ||
| throw new TypeError("Expected optionsJson to decode to an array", { cause: parsed }); | ||
| // Basic shape validation: each element must be an array (options tuple array) | ||
| for (let i = 0; i < parsed.length; i++) { | ||
| const el = parsed[i]; | ||
| if (!isArray(el)) throw new TypeError("Each options entry must be an array", { cause: el }); | ||
| } | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a debug check because I this validation is done probably done on the rust side as well. I need to confirm this. |
||
| deepFreezeArray(parsed); | ||
| allOptions = parsed as Readonly<Options>[]; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "categories": { | ||
| "correctness": "off" | ||
| }, | ||
| "jsPlugins": ["./plugin.ts"], | ||
| "rules": { | ||
| "test-plugin-options/check-options": ["error", true, { "expected": "production" }] | ||
| } | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| debugger; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The comment states "Note: the name
setupConfigsis currently incorrect, as we only populate rule options." While this is acknowledged as a TODO, consider renaming tosetupOptionsorsetupRuleOptionsto accurately reflect the current functionality. This would make the code clearer until multi-config workspace support is implemented.