diff --git a/crates/oxc_linter/src/config/config_store.rs b/crates/oxc_linter/src/config/config_store.rs index ca686e0040595..cd6425407d071 100644 --- a/crates/oxc_linter/src/config/config_store.rs +++ b/crates/oxc_linter/src/config/config_store.rs @@ -286,7 +286,10 @@ impl ConfigStore { } #[cfg_attr(not(all(feature = "oxlint2", not(feature = "disable_oxlint2"))), expect(dead_code))] - pub(crate) fn resolve_plugin_rule_names(&self, external_rule_id: u32) -> Option<(&str, &str)> { + pub(crate) fn resolve_plugin_rule_names( + &self, + external_rule_id: ExternalRuleId, + ) -> (/* plugin name */ &str, /* rule name */ &str) { self.external_plugin_store.resolve_plugin_rule_names(external_rule_id) } } diff --git a/crates/oxc_linter/src/external_linter.rs b/crates/oxc_linter/src/external_linter.rs index 08d4b3ec01b04..47092e5a41a44 100644 --- a/crates/oxc_linter/src/external_linter.rs +++ b/crates/oxc_linter/src/external_linter.rs @@ -42,7 +42,7 @@ pub enum PluginLoadResult { #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct LintResult { - pub external_rule_id: u32, + pub rule_index: u32, pub message: String, pub loc: Loc, } diff --git a/crates/oxc_linter/src/external_plugin_store.rs b/crates/oxc_linter/src/external_plugin_store.rs index 1e941168c9e12..dbae90ebfdc83 100644 --- a/crates/oxc_linter/src/external_plugin_store.rs +++ b/crates/oxc_linter/src/external_plugin_store.rs @@ -79,11 +79,13 @@ impl ExternalPluginStore { }) } - pub fn resolve_plugin_rule_names(&self, external_rule_id: u32) -> Option<(&str, &str)> { - let external_rule = self.rules.get(ExternalRuleId::from_raw(external_rule_id))?; + pub fn resolve_plugin_rule_names( + &self, + external_rule_id: ExternalRuleId, + ) -> (/* plugin name */ &str, /* rule name */ &str) { + let external_rule = &self.rules[external_rule_id]; let plugin = &self.plugins[external_rule.plugin_id]; - - Some((&plugin.name, &external_rule.name)) + (&plugin.name, &external_rule.name) } } diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 35256a5dfc7b0..13c035fbb01df 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -266,33 +266,18 @@ impl Linter { match result { Ok(diagnostics) => { for diagnostic in diagnostics { - match self.config.resolve_plugin_rule_names(diagnostic.external_rule_id) { - Some((plugin_name, rule_name)) => { - ctx_host.push_diagnostic(Message::new( - // TODO: `error` isn't right, we need to get the severity from `external_rules` - OxcDiagnostic::error(diagnostic.message) - .with_label(Span::new(diagnostic.loc.start, diagnostic.loc.end)) - .with_error_code(plugin_name.to_string(), rule_name.to_string()) - .with_severity( - (*external_rules - .iter() - .find(|(rule_id, _)| { - rule_id.raw() == diagnostic.external_rule_id - }) - .map(|(_, severity)| severity) - .expect( - "external rule must exist when resolving severity", - )) - .into(), - ), - PossibleFixes::None, - )); - } - None => { - // TODO: report diagnostic, this should be unreachable - debug_assert!(false); - } - } + let (external_rule_id, severity) = + external_rules[diagnostic.rule_index as usize]; + let (plugin_name, rule_name) = + self.config.resolve_plugin_rule_names(external_rule_id); + + ctx_host.push_diagnostic(Message::new( + OxcDiagnostic::error(diagnostic.message) + .with_label(Span::new(diagnostic.loc.start, diagnostic.loc.end)) + .with_error_code(plugin_name.to_string(), rule_name.to_string()) + .with_severity(severity.into()), + PossibleFixes::None, + )); } } Err(_err) => { diff --git a/napi/oxlint2/src-js/index.js b/napi/oxlint2/src-js/index.js index fc97945f2ece0..dcf7eb0563fe7 100644 --- a/napi/oxlint2/src-js/index.js +++ b/napi/oxlint2/src-js/index.js @@ -51,39 +51,37 @@ async function loadPluginImpl(path) { // TODO: Use a validation library to assert the shape of the plugin, and of rules const pluginName = plugin.meta.name; - let ruleId = registeredRules.length; + const offset = registeredRules.length; const ruleNames = []; - const ret = { name: pluginName, offset: ruleId, ruleNames }; for (const [ruleName, rule] of Object.entries(plugin.rules)) { ruleNames.push(ruleName); - registeredRules.push({ rule, context: new Context(ruleId, `${pluginName}/${ruleName}`) }); - ruleId++; + registeredRules.push({ rule, context: new Context(`${pluginName}/${ruleName}`) }); } - return JSON.stringify({ Success: ret }); + return JSON.stringify({ Success: { name: pluginName, offset, ruleNames } }); } +let setupContextForFile; + /** * Context class. * * Each rule has its own `Context` object. It is passed to that rule's `create` function. */ class Context { - // Rule ID. Index into `registeredRules` array. - #ruleId; // Full rule name, including plugin name e.g. `my-plugin/my-rule`. id; + // Index into `ruleIds` sent from Rust. Set before calling `rule`'s `create` method. + #ruleIndex; // Absolute path of file being linted. Set before calling `rule`'s `create` method. physicalFilename; /** * @constructor - * @param {number} ruleId - Rule ID * @param {string} fullRuleName - Rule name, in form `/` */ - constructor(ruleId, fullRuleName) { - this.#ruleId = ruleId; + constructor(fullRuleName) { this.id = fullRuleName; } @@ -100,9 +98,28 @@ class Context { diagnostics.push({ message: diagnostic.message, loc: { start: diagnostic.node.start, end: diagnostic.node.end }, - externalRuleId: this.#ruleId, + ruleIndex: this.#ruleIndex, }); } + + static { + /** + * Update a `Context` with file-specific data. + * + * We have to define this function within class body, as it's not possible to set private property + * `#ruleIndex` from outside the class. + * We don't use a normal class method, because we don't want to expose this to user. + * + * @param {Context} context - `Context` object + * @param {number} ruleIndex - Index of this rule within `ruleIds` passed from Rust + * @param {string} filePath - Absolute path of file being linted + * @returns {undefined} + */ + setupContextForFile = (context, ruleIndex, filePath) => { + context.#ruleIndex = ruleIndex; + context.physicalFilename = filePath; + }; + } } // -------------------- @@ -154,9 +171,10 @@ function lintFile([filePath, bufferId, buffer, ruleIds]) { // Get visitors for this file from all rules initCompiledVisitor(); - for (const ruleId of ruleIds) { + for (let i = 0; i < ruleIds.length; i++) { + const ruleId = ruleIds[i]; const { rule: { create }, context } = registeredRules[ruleId]; - context.physicalFilename = filePath; + setupContextForFile(context, i, filePath); const visitor = create(context); addVisitorToCompiled(visitor); }