refactor(linter/plugins): simplify return value of load_plugin#16459
Conversation
github-actions
bot
added
A-linter
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
CodSpeed Performance ReportMerging #16459 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the load_plugin return value handling to simplify the API by flattening a nested Result<Result<Data>> structure. Previously, LoadPluginResult was an enum with Success and Failure variants, creating redundancy with the outer Result type. The refactoring converts LoadPluginResult to a struct representing only the success case, while failures become Err variants of the outer Result.
Key Changes
LoadPluginResultchanged from enum to struct containing only success case fields (name,offset,rule_names)- New
LoadPluginReturnValueenum introduced for JS deserialization boundary withSuccess/Failurevariants - Simplified error handling in
config_builder.rsby removing nested match statement
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/oxc_linter/src/external_linter.rs | Converted LoadPluginResult from enum to struct, removing the Failure variant and flattening fields from the former Success variant |
| crates/oxc_linter/src/config/config_builder.rs | Simplified plugin loading logic by removing nested match on LoadPluginResult variants; error handling now occurs at the Result level via .map_err() |
| apps/oxlint/src/js_plugins/external_linter.rs | Added LoadPluginReturnValue enum for JS callback deserialization, mapping Success to Ok(LoadPluginResult) and Failure to Err(String) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
overlookmotel
force-pushed
the
12-03-refactor_linter_plugins_simplify_return_value_of_load_plugin_
branch
from
2f8d234 to
0544977
Compare
overlookmotel
force-pushed
the
12-03-refactor_linter_plugins_rename_loadpluginresult_
branch
from
6ccb6a4 to
08187e7
Compare
Merge activity
|
) Previously `load_plugin` returned a `Result<LoadPluginResult>`, where `LoadPluginResult` was an enum with one variant being an error case - so it was essentially `Result<Result<Data>>`. Flatten it so that `LoadPluginResult` is a now struct containing details of success case, and the error case that `LoadPluginResult::Failure` previously covered becomes `Err`. This simplifies the logic, and matches the pattern used by `lint_file` + `LintFileResult`.
graphite-app
bot
force-pushed
the
12-03-refactor_linter_plugins_rename_loadpluginresult_
branch
from
08187e7 to
0df6e73
Compare
graphite-app
bot
force-pushed
the
12-03-refactor_linter_plugins_simplify_return_value_of_load_plugin_
branch
from
0544977 to
3cb25e3
Compare
graphite-app
bot
deleted the
12-03-refactor_linter_plugins_simplify_return_value_of_load_plugin_
branch
…-project#16459) Previously `load_plugin` returned a `Result<LoadPluginResult>`, where `LoadPluginResult` was an enum with one variant being an error case - so it was essentially `Result<Result<Data>>`. Flatten it so that `LoadPluginResult` is a now struct containing details of success case, and the error case that `LoadPluginResult::Failure` previously covered becomes `Err`. This simplifies the logic, and matches the pattern used by `lint_file` + `LintFileResult`.
1 participant
Previously
load_pluginreturned aResult<LoadPluginResult>, whereLoadPluginResultwas an enum with one variant being an error case - so it was essentiallyResult<Result<Data>>.Flatten it so that
LoadPluginResultis a now struct containing details of success case, and the error case thatLoadPluginResult::Failurepreviously covered becomesErr.This simplifies the logic, and matches the pattern used by
lint_file+LintFileResult.