feat: ESLint plugin compatibility via @rslint/eslint-plugin-runner#1033
feat: ESLint plugin compatibility via @rslint/eslint-plugin-runner#1033fansenze wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a unified IPC CLI entry and a bidirectional IPC service to coordinate ESLint-plugin compatibility between the Go binary and a Node-hosted parent process. It refactors config loading, rule registration, and the linting pipeline to support both native and compat rules, with optimizations like a compat-only fast path that skips TS program creation. The reviewer identified several critical issues: compilation failures on Windows due to the direct use of syscall.SIGHUP in ipc_cli.go and server.go, a thread-safety issue in GetEnabledRules when accessing r.rules without a lock, and a stale configuration bug in handleConfigUpdate where s.jsConfigs is not re-initialized before updating.
| sigChInit := make(chan os.Signal, 1) | ||
| signal.Notify(sigChInit, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) | ||
| defer signal.Stop(sigChInit) | ||
| lintCtx, lintCtxStop := signal.NotifyContext( | ||
| context.Background(), syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP, | ||
| ) |
There was a problem hiding this comment.
Using syscall.SIGHUP directly will cause a compilation failure on Windows, as SIGHUP is not defined in the Windows syscall package. To ensure cross-platform compatibility and successful compilation on Windows, you can cast the numeric value of SIGHUP (which is 1 on Unix) to syscall.Signal(1). This is safe and portable across both Unix and Windows platforms.
| sigChInit := make(chan os.Signal, 1) | |
| signal.Notify(sigChInit, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) | |
| defer signal.Stop(sigChInit) | |
| lintCtx, lintCtxStop := signal.NotifyContext( | |
| context.Background(), syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP, | |
| ) | |
| sigChInit := make(chan os.Signal, 1) | |
| // Use syscall.Signal(1) instead of syscall.SIGHUP to prevent compilation failure on Windows | |
| signal.Notify(sigChInit, syscall.SIGINT, syscall.SIGTERM, syscall.Signal(1)) | |
| defer signal.Stop(sigChInit) | |
| lintCtx, lintCtxStop := signal.NotifyContext( | |
| context.Background(), syscall.SIGINT, syscall.SIGTERM, syscall.Signal(1), | |
| ) |
| // terminal hosts do — we want graceful session teardown). On | ||
| // Windows SIGHUP is a no-op for signal.Notify; the registration | ||
| // is portable. | ||
| sigCtx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM, syscall.SIGHUP) |
There was a problem hiding this comment.
Using syscall.SIGHUP directly will cause a compilation failure on Windows, as SIGHUP is not defined in the Windows syscall package. To ensure cross-platform compatibility and successful compilation on Windows, you can cast the numeric value of SIGHUP (which is 1 on Unix) to syscall.Signal(1).
| sigCtx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM, syscall.SIGHUP) | |
| sigCtx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM, syscall.Signal(1)) |
| @@ -62,12 +85,16 @@ func (r *RuleRegistry) GetEnabledRules(config RslintConfig, filePath string, cwd | |||
| if ruleImpl, exists := r.rules[ruleName]; exists { | |||
There was a problem hiding this comment.
Accessing r.rules directly without a lock in GetEnabledRules is not thread-safe and can cause a concurrent map read/write panic when RegisterEslintPluginRules concurrently mutates the registry during LSP config updates. Use the thread-safe GetRule method instead.
| if ruleImpl, exists := r.rules[ruleName]; exists { | |
| if ruleImpl, exists := r.GetRule(ruleName); exists { |
| for _, cfg := range payload.Configs { | ||
| s.jsConfigs[cfg.ConfigDirectory] = cfg.Entries | ||
| } |
There was a problem hiding this comment.
In handleConfigUpdate, s.jsConfigs is not cleared or re-initialized before the loop. This means that if any JS/TS configs are deleted or updated, the old configs will still linger in s.jsConfigs in memory, and files in those directories will continue to be linted using stale configs. Re-initialize s.jsConfigs at the start of the update to prevent this.
| for _, cfg := range payload.Configs { | |
| s.jsConfigs[cfg.ConfigDirectory] = cfg.Entries | |
| } | |
| s.jsConfigs = make(map[string]config.RslintConfig, len(payload.Configs)) | |
| for _, cfg := range payload.Configs { | |
| s.jsConfigs[cfg.ConfigDirectory] = cfg.Entries | |
| } |
e0f625d to
05bc205
Compare
Wires the CLI (engine.ts) and LSP (CompatPool) hosts plus the Go linter to run ESLint v10 plugin rules through the @rslint/eslint-plugin-runner package (a worker_threads pool over framed IPC), so a config's `eslintPlugins` rules run alongside the native Go rules.
05bc205 to
14ca4f6
Compare
Summary
Adds ESLint-plugin compatibility to rslint: users mount real ESLint plugins (e.g.
eslint-plugin-unicorn) in their config viaeslintPlugins: { prefix: pluginObject }, and those rules run alongside rslint's native rules.@rslint/eslint-plugin-runner— new package: the in-process plugin runtime (loads user plugins, runsrule.createagainst oxc-parser's AST in a Node worker).cli.ts → engine.ts → Go IPC (lintEslintPlugin) → WorkerPool. A compat-only fast path skips ts-go Program construction when every rule is a plugin rule.rslint/lintCompatBatchto the VS Code extension'sCompatPool/WorkerPool, with$/cancelRequestpropagation for per-keystroke supersession.internal/linter/compat_runner.gois the single compat-dispatch entry point shared by the RunLinter (mixed native+plugin) and CLI compat-only ingest paths;internal/lsp/lintcompat_dispatcher.gowires the LSP round-trip.include), nested configs, global ignores, and--fixare all supported and covered by CLI + VS Code e2e tests.Related Links
N/A
Checklist