Linter: Make rule selection an explicit option, not positional rule lists

[marcoroth]

Rules reach the Linter through parameters that were added one at a time, and the shape has stopped describing what the class actually does:

static from(herb: HerbBackend, config?: Config, customRules?: RuleClass[], options?: FilterRulesOptions): Linter
constructor(herb: HerbBackend, rules?: RuleClass[], config?: Config, allAvailableRules?: RuleClass[])
static filterRulesByConfig(allRules: RuleClass[], userRulesConfig?: Record<string, RuleConfig>, configVersion?: string, options?: FilterRulesOptions): FilterRulesResult

There are really only three inputs here, the universe of rules that exist, how to select from it, and the config that informs both, but they're spread across seven positional parameters and two overlapping options types.

The universe and the selection are two positional lists

rules is meant to be the filtered subset of allAvailableRules, which is the full list used for herb:disable validation. Nothing enforces that relationship, and it's easy to supply one and not the other:

constructor(herb: HerbBackend, rules?: RuleClass[], config?: Config, allAvailableRules?: RuleClass[]) {
  this.rules = rules !== undefined ? rules : this.getDefaultRules()
  this.allAvailableRules = allAvailableRules !== undefined ? allAvailableRules : this.rules
}

StimulusLinter passes only rules, so allAvailableRules silently collapses to the enabled subset, and herb-disable-comment-valid-rule-name there validates against a narrower list than the one that exists. A disable comment naming a rule that is real but not currently enabled gets reported as unknown.

The same constructor also carries a dead extension point. StimulusLinter overrides getDefaultRules(), but that only fires when rules is undefined, and it always passes rules, then re-assigns the same value right after super():

constructor(herb: HerbBackend, rules: RuleClass[], stimulusProject?: Project) {
  super(herb, rules)
  this.stimulusProject = stimulusProject
  this.rules = rules
}

LinterOptions is dead code

linter.ts exports an options interface that nothing references:

export interface LinterOptions {
  rules?: RuleClass[]
  loadCustomRules?: boolean
  customRulesBaseDir?: string
  customRulesPatterns?: string[]
  silentCustomRules?: boolean
}

An options-object API was intended once and never landed, and the constructor implements none of it. Because it's inert, adding --only meant introducing a second options type (FilterRulesOptions) next to it rather than extending the one that was already there. The next option added will face the same choice.

Construction-time facts are patched after construction

Linter.from() builds the instance and then assigns four public fields, all of which are decided by the same filtering call that just ran:

const linter = new Linter(herb, filterResult.enabled, config, allRules)
linter.rulesSkippedByVersion = filterResult.skippedByVersion
linter.rulesDisabledByConfig = filterResult.disabledByConfig
linter.rulesNotEnabledByDefault = filterResult.notEnabledByDefault
linter.onlyRules = Linter.normalizeOnlyRules(options?.only)

The language server patches a fifth from the outside, because mode is a mutable public field rather than an input:

const { enabled: filteredRules } = Linter.filterRulesByConfig(this.allRules, config.linter?.rules, config.configVersion)

this.linter = new Linter(Herb, filteredRules, config, this.allRules)
this.linter.mode = "editor"

That block is also the clearest symptom: the language server can't use Linter.from() at all, because there's no way to hand it a rule universe or a mode, so it repeats the filter-then-construct dance by hand and has to keep it in sync with from().

Custom rules are loaded in three places

FileProcessor, lint-worker.ts and LinterService each glob, import, merge and warn on their own, and each then combines the result with the built-in rules differently, the language server keeps its own allRules field, the CLI passes customRules into from(), the worker does the same with different silencing rules. Any behaviour that should apply to custom rules has to be wired into all three. The --only work hit this directly: validating rule names needed the custom rules, and herb-stimulus-lint needed its own rule names on top, which is why additionalRuleNames() exists on the CLI at all.

How it could work

Make the selection a named concept instead of an options field, so the universe, the selection and the config are the three inputs:

export type RuleSelection =
  | "config"            
  | { only: string[] }  

For "config" it would be " herb.yml + version gating + per-rule defaults". For only: "exactly these rules, config ignored".

Everything the class derives from those inputs, the enabled rules, the skipped/disabled counts, the selection itself, becomes readonly state computed in the constructor, and the post-construction patching goes away. Future selections drop into the union without another parameter or another options type.

The dead LinterOptions is the natural place to put the rest, rather than adding a third options bag:

export interface LinterOptions {
  rules?: RuleClass[]   
  customRules?: RuleClass[]  
  selection?: RuleSelection
  mode?: LinterMode       
}

constructor(herb: HerbBackend, config?: Config, options?: LinterOptions)

With mode as an input and the universe explicit, the language server's hand-rolled block becomes a single Linter.from(Herb, config, { rules: this.allRules, mode: "editor" }), and StimulusLinter passes { rules: defaultRules } instead of relying on a getDefaultRules() override that never runs.

Custom rule loading should stay out of the core, it needs fs and it's async, while the Linter has to keep working in the browser and WASM builds, where the playground constructs it with no config at all:

const linter = new Linter(herb)

A single async factory in the existing Node-only @herb-tools/linter/loader entry point would give the three call sites one path, returning the linter alongside the rule info and warnings they each want to print. Custom rules would then join the universe before selection is applied, which is what makes --only my-custom-rule work without per-CLI rule-name lists and lets additionalRuleNames() and the Stimulus CLI's manual rule filtering go away.

Linter.from(), the constructor and filterRulesByConfig() are all exported from a published package, so reshaping them is breaking for anyone outside the monorepo. In practice the consumers are this repository, stimulus-lint and the playground, so the migration is mechanical, but it wants a release-note entry.