Skip to content

Commit c551b8f

Browse files
committed
feat(linter): report diagnostics from custom plugins (#12219)
1 parent d387729 commit c551b8f

File tree

14 files changed

+125
-25
lines changed

14 files changed

+125
-25
lines changed

apps/oxlint/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ pub mod cli {
1111
}
1212

1313
pub use oxc_linter::{
14-
ExternalLinter, ExternalLinterCb, ExternalLinterLoadPluginCb, PluginLoadResult,
14+
ExternalLinter, ExternalLinterCb, ExternalLinterLoadPluginCb, LintResult, PluginLoadResult,
1515
};
1616

1717
#[cfg(all(feature = "oxlint2", not(feature = "disable_oxlint2")))]

crates/oxc_linter/src/config/config_store.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,10 @@ impl ConfigStore {
286286
}
287287
None
288288
}
289+
290+
pub(crate) fn resolve_plugin_rule_names(&self, external_rule_id: u32) -> Option<(&str, &str)> {
291+
self.external_plugin_store.resolve_plugin_rule_names(external_rule_id)
292+
}
289293
}
290294

291295
#[cfg(test)]

crates/oxc_linter/src/context/host.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ impl<'a> ContextHost<'a> {
138138
/// Add a diagnostic message to the end of the list of diagnostics. Can be used
139139
/// by any rule to report issues.
140140
#[inline]
141-
pub(super) fn push_diagnostic(&self, diagnostic: Message<'a>) {
141+
pub(crate) fn push_diagnostic(&self, diagnostic: Message<'a>) {
142142
self.diagnostics.borrow_mut().push(diagnostic);
143143
}
144144

crates/oxc_linter/src/external_linter.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ pub type ExternalLinterLoadPluginCb = Arc<
1717
>;
1818

1919
pub type ExternalLinterCb = Arc<
20-
dyn Fn(String, Vec<u32>) -> Result<(), Box<dyn std::error::Error + Send + Sync>> + Sync + Send,
20+
dyn Fn(String, Vec<u32>) -> Result<Vec<LintResult>, Box<dyn std::error::Error + Send + Sync>>
21+
+ Sync
22+
+ Send,
2123
>;
2224

2325
#[derive(Clone, Debug, Deserialize, Serialize)]
@@ -26,6 +28,20 @@ pub enum PluginLoadResult {
2628
Failure(String),
2729
}
2830

31+
#[derive(Clone, Debug, Deserialize, Serialize)]
32+
#[serde(rename_all = "camelCase")]
33+
pub struct LintResult {
34+
pub external_rule_id: u32,
35+
pub message: String,
36+
pub loc: Loc,
37+
}
38+
39+
#[derive(Clone, Debug, Deserialize, Serialize)]
40+
pub struct Loc {
41+
pub start: u32,
42+
pub end: u32,
43+
}
44+
2945
#[derive(Clone)]
3046
#[cfg_attr(any(not(feature = "oxlint2"), feature = "disable_oxlint2"), expect(dead_code))]
3147
pub struct ExternalLinter {

crates/oxc_linter/src/external_plugin_store.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,14 @@ impl ExternalPluginStore {
107107
}
108108
})
109109
}
110+
111+
pub fn resolve_plugin_rule_names(&self, external_rule_id: u32) -> Option<(&str, &str)> {
112+
let external_rule =
113+
self.rules.get(NonMaxU32::new(external_rule_id).map(ExternalRuleId)?)?;
114+
let plugin = &self.plugins[external_rule.plugin_id];
115+
116+
Some((&plugin.name, &external_rule.name))
117+
}
110118
}
111119

112120
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -131,7 +139,6 @@ impl fmt::Display for ExternalRuleLookupError {
131139
impl std::error::Error for ExternalRuleLookupError {}
132140

133141
#[derive(Debug, Default)]
134-
#[expect(dead_code)]
135142
struct ExternalPlugin {
136143
name: String,
137144
rules: FxHashMap<String, ExternalRuleId>,

crates/oxc_linter/src/lib.rs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ pub mod table;
2626

2727
use std::{path::Path, rc::Rc, sync::Arc};
2828

29+
use oxc_diagnostics::OxcDiagnostic;
2930
use oxc_semantic::{AstNode, Semantic};
31+
use oxc_span::Span;
3032

3133
pub use crate::{
3234
config::{
@@ -35,7 +37,7 @@ pub use crate::{
3537
},
3638
context::LintContext,
3739
external_linter::{
38-
ExternalLinter, ExternalLinterCb, ExternalLinterLoadPluginCb, PluginLoadResult,
40+
ExternalLinter, ExternalLinterCb, ExternalLinterLoadPluginCb, LintResult, PluginLoadResult,
3941
},
4042
external_plugin_store::ExternalPluginStore,
4143
fixer::FixKind,
@@ -52,7 +54,7 @@ pub use crate::{
5254
use crate::{
5355
config::{LintConfig, OxlintEnv, OxlintGlobals, OxlintSettings, ResolvedLinterState},
5456
context::ContextHost,
55-
fixer::{Fixer, Message},
57+
fixer::{Fixer, Message, PossibleFixes},
5658
rules::RuleEnum,
5759
utils::iter_possible_jest_call_node,
5860
};
@@ -204,8 +206,31 @@ impl Linter {
204206
external_rules.iter().map(|(rule_id, _)| rule_id.as_u32()).collect(),
205207
);
206208
match result {
207-
Ok(()) => {
208-
// TODO: report diagnostics
209+
Ok(diagnostics) => {
210+
for diagnostic in diagnostics {
211+
match self.config.resolve_plugin_rule_names(diagnostic.external_rule_id)
212+
{
213+
Some((plugin_name, rule_name)) => {
214+
ctx_host.push_diagnostic(Message::new(
215+
// TODO: `error` isn't right, we need to get the severity from `external_rules`
216+
OxcDiagnostic::error(diagnostic.message)
217+
.with_label(Span::new(
218+
diagnostic.loc.start,
219+
diagnostic.loc.end,
220+
))
221+
.with_error_code(
222+
plugin_name.to_string(),
223+
rule_name.to_string(),
224+
),
225+
PossibleFixes::None,
226+
));
227+
}
228+
None => {
229+
// TODO: report diagnostic, this should be unreachable
230+
debug_assert!(false);
231+
}
232+
}
233+
}
209234
}
210235
Err(_err) => {
211236
// TODO: report diagnostic

napi/oxlint2/src/bindings.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ export type JsLoadPluginCb =
44
((arg: string) => Promise<string>)
55

66
export type JsRunCb =
7-
((arg0: string, arg1: Array<number>) => void)
7+
((arg0: string, arg1: Array<number>) => string)
88

99
export declare function lint(loadPlugin: JsLoadPluginCb, run: JsRunCb): Promise<boolean>

napi/oxlint2/src/index.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class PluginRegistry {
2828

2929
*getRules(ruleIds) {
3030
for (const ruleId of ruleIds) {
31-
yield this.registeredRules[ruleId];
31+
yield { rule: this.registeredRules[ruleId], ruleId };
3232
}
3333
}
3434
}
@@ -71,6 +71,28 @@ class Linter {
7171
if (!Array.isArray(ruleIds) || ruleIds.length === 0) {
7272
throw new Error('Expected `ruleIds` to be a non-zero len array');
7373
}
74+
75+
const diagnostics = [];
76+
77+
const createContext = (ruleId) => ({
78+
physicalFilename: filePath,
79+
report: (diagnostic) => {
80+
diagnostics.push({
81+
message: diagnostic.message,
82+
loc: { start: diagnostic.node.start, end: diagnostic.node.end },
83+
externalRuleId: ruleId,
84+
});
85+
},
86+
});
87+
88+
const rules = [];
89+
for (const { rule, ruleId } of this.pluginRegistry.getRules(ruleIds)) {
90+
rules.push(rule(createContext(ruleId)));
91+
}
92+
93+
// TODO: walk the AST
94+
95+
return JSON.stringify(diagnostics);
7496
};
7597
}
7698

napi/oxlint2/src/lib.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,18 @@ use napi::{
1111
use napi_derive::napi;
1212

1313
use oxlint::{
14-
ExternalLinter, ExternalLinterCb, ExternalLinterLoadPluginCb, PluginLoadResult,
14+
ExternalLinter, ExternalLinterCb, ExternalLinterLoadPluginCb, LintResult, PluginLoadResult,
1515
lint as oxlint_lint,
1616
};
1717

1818
#[napi]
19-
pub type JsRunCb = ThreadsafeFunction<(String, Vec<u32>), (), (String, Vec<u32>), Status, false>;
19+
pub type JsRunCb = ThreadsafeFunction<
20+
(String, Vec<u32>),
21+
String, /* Vec<LintResult> */
22+
(String, Vec<u32>),
23+
Status,
24+
false,
25+
>;
2026

2127
#[napi]
2228
pub type JsLoadPluginCb = ThreadsafeFunction<
@@ -39,11 +45,14 @@ fn wrap_run(cb: JsRunCb) -> ExternalLinterCb {
3945
ThreadsafeFunctionCallMode::NonBlocking,
4046
move |result, _env| {
4147
let _ = match &result {
42-
Ok(()) => tx.send(Ok(())),
48+
Ok(r) => match serde_json::from_str::<Vec<LintResult>>(r) {
49+
Ok(v) => tx.send(Ok(v)),
50+
Err(_e) => tx.send(Err("Failed to deserialize lint result".to_string())),
51+
},
4352
Err(e) => tx.send(Err(e.to_string())),
4453
};
4554

46-
result
55+
result.map(|_| ())
4756
},
4857
);
4958

@@ -52,7 +61,7 @@ fn wrap_run(cb: JsRunCb) -> ExternalLinterCb {
5261
}
5362

5463
match rx.recv() {
55-
Ok(Ok(())) => Ok(()),
64+
Ok(Ok(x)) => Ok(x),
5665
Ok(Err(e)) => Err(format!("Callback reported error: {e}").into()),
5766
Err(e) => Err(format!("Callback did not respond: {e}").into()),
5867
}

napi/oxlint2/test/__snapshots__/e2e.test.ts.snap

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,21 @@ Finished in Xms on 1 file using X threads."
1919
`;
2020

2121
exports[`cli options for bundling > should load a custom plugin 1`] = `
22-
"Found 0 warnings and 0 errors.
22+
"
23+
! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed
24+
,-[index.js:1:1]
25+
1 | debugger;
26+
: ^^^^^^^^^
27+
\`----
28+
help: Remove the debugger statement
29+
30+
x basic-custom-plugin(no-debugger): Unexpected Debugger Statement
31+
,-[index.js:1:1]
32+
1 | debugger;
33+
: ^
34+
\`----
35+
36+
Found 1 warning and 1 error.
2337
Finished in Xms on 1 file using X threads."
2438
`;
2539

0 commit comments

Comments
 (0)