Skip to content

Commit d7f1a53

Browse files
bors[bot]Jonas Schievink
bors[bot]
and
Jonas Schievink
authored
Merge #5524
5524: Allow opting out of experimental diagnostics like MismatchedArgCount r=matklad a=jonas-schievink Closes #5448 Closes #5419 Co-authored-by: Jonas Schievink <[email protected]>
2 parents 75e67ee + 92a4ec8 commit d7f1a53

File tree

10 files changed

+159
-103
lines changed

10 files changed

+159
-103
lines changed

‎crates/ra_hir/src/diagnostics.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
//! FIXME: write short doc here
22
pub use hir_def::diagnostics::UnresolvedModule;
3-
pub use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink};
3+
pub use hir_expand::diagnostics::{
4+
AstDiagnostic, Diagnostic, DiagnosticSink, DiagnosticSinkBuilder,
5+
};
46
pub use hir_ty::diagnostics::{
57
MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, NoSuchField,
68
};

‎crates/ra_hir_expand/src/diagnostics.rs‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static {
2424
fn message(&self) -> String;
2525
fn source(&self) -> InFile<SyntaxNodePtr>;
2626
fn as_any(&self) -> &(dyn Any + Send + 'static);
27+
fn is_experimental(&self) -> bool {
28+
false
29+
}
2730
}
2831

2932
pub trait AstDiagnostic {
@@ -44,16 +47,48 @@ impl dyn Diagnostic {
4447

4548
pub struct DiagnosticSink<'a> {
4649
callbacks: Vec<Box<dyn FnMut(&dyn Diagnostic) -> Result<(), ()> + 'a>>,
50+
filters: Vec<Box<dyn FnMut(&dyn Diagnostic) -> bool + 'a>>,
4751
default_callback: Box<dyn FnMut(&dyn Diagnostic) + 'a>,
4852
}
4953

5054
impl<'a> DiagnosticSink<'a> {
51-
/// FIXME: split `new` and `on` into a separate builder type
52-
pub fn new(cb: impl FnMut(&dyn Diagnostic) + 'a) -> DiagnosticSink<'a> {
53-
DiagnosticSink { callbacks: Vec::new(), default_callback: Box::new(cb) }
55+
pub fn push(&mut self, d: impl Diagnostic) {
56+
let d: &dyn Diagnostic = &d;
57+
self._push(d);
58+
}
59+
60+
fn _push(&mut self, d: &dyn Diagnostic) {
61+
for filter in &mut self.filters {
62+
if !filter(d) {
63+
return;
64+
}
65+
}
66+
for cb in &mut self.callbacks {
67+
match cb(d) {
68+
Ok(()) => return,
69+
Err(()) => (),
70+
}
71+
}
72+
(self.default_callback)(d)
5473
}
74+
}
5575

56-
pub fn on<D: Diagnostic, F: FnMut(&D) + 'a>(mut self, mut cb: F) -> DiagnosticSink<'a> {
76+
pub struct DiagnosticSinkBuilder<'a> {
77+
callbacks: Vec<Box<dyn FnMut(&dyn Diagnostic) -> Result<(), ()> + 'a>>,
78+
filters: Vec<Box<dyn FnMut(&dyn Diagnostic) -> bool + 'a>>,
79+
}
80+
81+
impl<'a> DiagnosticSinkBuilder<'a> {
82+
pub fn new() -> Self {
83+
Self { callbacks: Vec::new(), filters: Vec::new() }
84+
}
85+
86+
pub fn filter<F: FnMut(&dyn Diagnostic) -> bool + 'a>(mut self, cb: F) -> Self {
87+
self.filters.push(Box::new(cb));
88+
self
89+
}
90+
91+
pub fn on<D: Diagnostic, F: FnMut(&D) + 'a>(mut self, mut cb: F) -> Self {
5792
let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::<D>() {
5893
Some(d) => {
5994
cb(d);
@@ -65,18 +100,11 @@ impl<'a> DiagnosticSink<'a> {
65100
self
66101
}
67102

68-
pub fn push(&mut self, d: impl Diagnostic) {
69-
let d: &dyn Diagnostic = &d;
70-
self._push(d);
71-
}
72-
73-
fn _push(&mut self, d: &dyn Diagnostic) {
74-
for cb in self.callbacks.iter_mut() {
75-
match cb(d) {
76-
Ok(()) => return,
77-
Err(()) => (),
78-
}
103+
pub fn build<F: FnMut(&dyn Diagnostic) + 'a>(self, default_callback: F) -> DiagnosticSink<'a> {
104+
DiagnosticSink {
105+
callbacks: self.callbacks,
106+
filters: self.filters,
107+
default_callback: Box::new(default_callback),
79108
}
80-
(self.default_callback)(d)
81109
}
82110
}

‎crates/ra_hir_ty/src/diagnostics.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,9 @@ impl Diagnostic for MismatchedArgCount {
234234
fn as_any(&self) -> &(dyn Any + Send + 'static) {
235235
self
236236
}
237+
fn is_experimental(&self) -> bool {
238+
true
239+
}
237240
}
238241

239242
impl AstDiagnostic for MismatchedArgCount {
@@ -248,7 +251,7 @@ impl AstDiagnostic for MismatchedArgCount {
248251
#[cfg(test)]
249252
mod tests {
250253
use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId};
251-
use hir_expand::diagnostics::{Diagnostic, DiagnosticSink};
254+
use hir_expand::diagnostics::{Diagnostic, DiagnosticSinkBuilder};
252255
use ra_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt};
253256
use ra_syntax::{TextRange, TextSize};
254257
use rustc_hash::FxHashMap;
@@ -280,7 +283,7 @@ mod tests {
280283
}
281284

282285
for f in fns {
283-
let mut sink = DiagnosticSink::new(&mut cb);
286+
let mut sink = DiagnosticSinkBuilder::new().build(&mut cb);
284287
validate_body(self, f.into(), &mut sink);
285288
}
286289
}

‎crates/ra_ide/src/diagnostics.rs‎

Lines changed: 83 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
use std::cell::RefCell;
88

99
use hir::{
10-
diagnostics::{AstDiagnostic, Diagnostic as _, DiagnosticSink},
10+
diagnostics::{AstDiagnostic, Diagnostic as _, DiagnosticSinkBuilder},
1111
HasSource, HirDisplay, Semantics, VariantDef,
1212
};
1313
use itertools::Itertools;
@@ -29,7 +29,11 @@ pub enum Severity {
2929
WeakWarning,
3030
}
3131

32-
pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec<Diagnostic> {
32+
pub(crate) fn diagnostics(
33+
db: &RootDatabase,
34+
file_id: FileId,
35+
enable_experimental: bool,
36+
) -> Vec<Diagnostic> {
3337
let _p = profile("diagnostics");
3438
let sema = Semantics::new(db);
3539
let parse = db.parse(file_id);
@@ -48,79 +52,85 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec<Diagnostic>
4852
check_struct_shorthand_initialization(&mut res, file_id, &node);
4953
}
5054
let res = RefCell::new(res);
51-
let mut sink = DiagnosticSink::new(|d| {
52-
res.borrow_mut().push(Diagnostic {
53-
message: d.message(),
54-
range: sema.diagnostics_range(d).range,
55-
severity: Severity::Error,
56-
fix: None,
57-
})
58-
})
59-
.on::<hir::diagnostics::UnresolvedModule, _>(|d| {
60-
let original_file = d.source().file_id.original_file(db);
61-
let fix = Fix::new(
62-
"Create module",
63-
FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() }.into(),
64-
);
65-
res.borrow_mut().push(Diagnostic {
66-
range: sema.diagnostics_range(d).range,
67-
message: d.message(),
68-
severity: Severity::Error,
69-
fix: Some(fix),
55+
let mut sink = DiagnosticSinkBuilder::new()
56+
.on::<hir::diagnostics::UnresolvedModule, _>(|d| {
57+
let original_file = d.source().file_id.original_file(db);
58+
let fix = Fix::new(
59+
"Create module",
60+
FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() }
61+
.into(),
62+
);
63+
res.borrow_mut().push(Diagnostic {
64+
range: sema.diagnostics_range(d).range,
65+
message: d.message(),
66+
severity: Severity::Error,
67+
fix: Some(fix),
68+
})
7069
})
71-
})
72-
.on::<hir::diagnostics::MissingFields, _>(|d| {
73-
// Note that although we could add a diagnostics to
74-
// fill the missing tuple field, e.g :
75-
// `struct A(usize);`
76-
// `let a = A { 0: () }`
77-
// but it is uncommon usage and it should not be encouraged.
78-
let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) {
79-
None
80-
} else {
81-
let mut field_list = d.ast(db);
82-
for f in d.missed_fields.iter() {
83-
let field =
84-
make::record_field(make::name_ref(&f.to_string()), Some(make::expr_unit()));
85-
field_list = field_list.append_field(&field);
86-
}
87-
88-
let edit = {
89-
let mut builder = TextEditBuilder::default();
90-
algo::diff(&d.ast(db).syntax(), &field_list.syntax()).into_text_edit(&mut builder);
91-
builder.finish()
70+
.on::<hir::diagnostics::MissingFields, _>(|d| {
71+
// Note that although we could add a diagnostics to
72+
// fill the missing tuple field, e.g :
73+
// `struct A(usize);`
74+
// `let a = A { 0: () }`
75+
// but it is uncommon usage and it should not be encouraged.
76+
let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) {
77+
None
78+
} else {
79+
let mut field_list = d.ast(db);
80+
for f in d.missed_fields.iter() {
81+
let field =
82+
make::record_field(make::name_ref(&f.to_string()), Some(make::expr_unit()));
83+
field_list = field_list.append_field(&field);
84+
}
85+
86+
let edit = {
87+
let mut builder = TextEditBuilder::default();
88+
algo::diff(&d.ast(db).syntax(), &field_list.syntax())
89+
.into_text_edit(&mut builder);
90+
builder.finish()
91+
};
92+
Some(Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()))
9293
};
93-
Some(Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()))
94-
};
9594

96-
res.borrow_mut().push(Diagnostic {
97-
range: sema.diagnostics_range(d).range,
98-
message: d.message(),
99-
severity: Severity::Error,
100-
fix,
95+
res.borrow_mut().push(Diagnostic {
96+
range: sema.diagnostics_range(d).range,
97+
message: d.message(),
98+
severity: Severity::Error,
99+
fix,
100+
})
101101
})
102-
})
103-
.on::<hir::diagnostics::MissingOkInTailExpr, _>(|d| {
104-
let node = d.ast(db);
105-
let replacement = format!("Ok({})", node.syntax());
106-
let edit = TextEdit::replace(node.syntax().text_range(), replacement);
107-
let source_change = SourceFileEdit { file_id, edit }.into();
108-
let fix = Fix::new("Wrap with ok", source_change);
109-
res.borrow_mut().push(Diagnostic {
110-
range: sema.diagnostics_range(d).range,
111-
message: d.message(),
112-
severity: Severity::Error,
113-
fix: Some(fix),
102+
.on::<hir::diagnostics::MissingOkInTailExpr, _>(|d| {
103+
let node = d.ast(db);
104+
let replacement = format!("Ok({})", node.syntax());
105+
let edit = TextEdit::replace(node.syntax().text_range(), replacement);
106+
let source_change = SourceFileEdit { file_id, edit }.into();
107+
let fix = Fix::new("Wrap with ok", source_change);
108+
res.borrow_mut().push(Diagnostic {
109+
range: sema.diagnostics_range(d).range,
110+
message: d.message(),
111+
severity: Severity::Error,
112+
fix: Some(fix),
113+
})
114114
})
115-
})
116-
.on::<hir::diagnostics::NoSuchField, _>(|d| {
117-
res.borrow_mut().push(Diagnostic {
118-
range: sema.diagnostics_range(d).range,
119-
message: d.message(),
120-
severity: Severity::Error,
121-
fix: missing_struct_field_fix(&sema, file_id, d),
115+
.on::<hir::diagnostics::NoSuchField, _>(|d| {
116+
res.borrow_mut().push(Diagnostic {
117+
range: sema.diagnostics_range(d).range,
118+
message: d.message(),
119+
severity: Severity::Error,
120+
fix: missing_struct_field_fix(&sema, file_id, d),
121+
})
122122
})
123-
});
123+
// Only collect experimental diagnostics when they're enabled.
124+
.filter(|diag| !diag.is_experimental() || enable_experimental)
125+
// Diagnostics not handled above get no fix and default treatment.
126+
.build(|d| {
127+
res.borrow_mut().push(Diagnostic {
128+
message: d.message(),
129+
range: sema.diagnostics_range(d).range,
130+
severity: Severity::Error,
131+
fix: None,
132+
})
133+
});
124134

125135
if let Some(m) = sema.to_module_def(file_id) {
126136
m.diagnostics(db, &mut sink);
@@ -298,7 +308,7 @@ mod tests {
298308
let after = trim_indent(ra_fixture_after);
299309

300310
let (analysis, file_position) = analysis_and_position(ra_fixture_before);
301-
let diagnostic = analysis.diagnostics(file_position.file_id).unwrap().pop().unwrap();
311+
let diagnostic = analysis.diagnostics(file_position.file_id, true).unwrap().pop().unwrap();
302312
let mut fix = diagnostic.fix.unwrap();
303313
let edit = fix.source_change.source_file_edits.pop().unwrap().edit;
304314
let target_file_contents = analysis.file_text(file_position.file_id).unwrap();
@@ -324,7 +334,7 @@ mod tests {
324334
let ra_fixture_after = &trim_indent(ra_fixture_after);
325335
let (analysis, file_pos) = analysis_and_position(ra_fixture_before);
326336
let current_file_id = file_pos.file_id;
327-
let diagnostic = analysis.diagnostics(current_file_id).unwrap().pop().unwrap();
337+
let diagnostic = analysis.diagnostics(current_file_id, true).unwrap().pop().unwrap();
328338
let mut fix = diagnostic.fix.unwrap();
329339
let edit = fix.source_change.source_file_edits.pop().unwrap();
330340
let changed_file_id = edit.file_id;
@@ -345,14 +355,14 @@ mod tests {
345355
let analysis = mock.analysis();
346356
let diagnostics = files
347357
.into_iter()
348-
.flat_map(|file_id| analysis.diagnostics(file_id).unwrap())
358+
.flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap())
349359
.collect::<Vec<_>>();
350360
assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics);
351361
}
352362

353363
fn check_expect(ra_fixture: &str, expect: Expect) {
354364
let (analysis, file_id) = single_file(ra_fixture);
355-
let diagnostics = analysis.diagnostics(file_id).unwrap();
365+
let diagnostics = analysis.diagnostics(file_id, true).unwrap();
356366
expect.assert_debug_eq(&diagnostics)
357367
}
358368

‎crates/ra_ide/src/lib.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -487,8 +487,12 @@ impl Analysis {
487487
}
488488

489489
/// Computes the set of diagnostics for the given file.
490-
pub fn diagnostics(&self, file_id: FileId) -> Cancelable<Vec<Diagnostic>> {
491-
self.with_db(|db| diagnostics::diagnostics(db, file_id))
490+
pub fn diagnostics(
491+
&self,
492+
file_id: FileId,
493+
enable_experimental: bool,
494+
) -> Cancelable<Vec<Diagnostic>> {
495+
self.with_db(|db| diagnostics::diagnostics(db, file_id, enable_experimental))
492496
}
493497

494498
/// Returns the edit required to rename reference at the position to the new

‎crates/rust-analyzer/src/cli/analysis_bench.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ pub fn analysis_bench(
7070
match &what {
7171
BenchWhat::Highlight { .. } => {
7272
let res = do_work(&mut host, file_id, |analysis| {
73-
analysis.diagnostics(file_id).unwrap();
73+
analysis.diagnostics(file_id, true).unwrap();
7474
analysis.highlight_as_html(file_id, false).unwrap()
7575
});
7676
if verbosity.is_verbose() {

‎crates/rust-analyzer/src/cli/diagnostics.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ pub fn diagnostics(
4747
String::from("unknown")
4848
};
4949
println!("processing crate: {}, module: {}", crate_name, _vfs.file_path(file_id));
50-
for diagnostic in analysis.diagnostics(file_id).unwrap() {
50+
for diagnostic in analysis.diagnostics(file_id, true).unwrap() {
5151
if matches!(diagnostic.severity, Severity::Error) {
5252
found_error = true;
5353
}

‎crates/rust-analyzer/src/config.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub struct Config {
2323
pub client_caps: ClientCapsConfig,
2424

2525
pub publish_diagnostics: bool,
26+
pub experimental_diagnostics: bool,
2627
pub diagnostics: DiagnosticsConfig,
2728
pub lru_capacity: Option<usize>,
2829
pub proc_macro_srv: Option<(PathBuf, Vec<OsString>)>,
@@ -137,6 +138,7 @@ impl Config {
137138

138139
with_sysroot: true,
139140
publish_diagnostics: true,
141+
experimental_diagnostics: true,
140142
diagnostics: DiagnosticsConfig::default(),
141143
lru_capacity: None,
142144
proc_macro_srv: None,
@@ -187,6 +189,7 @@ impl Config {
187189

188190
self.with_sysroot = data.withSysroot;
189191
self.publish_diagnostics = data.diagnostics_enable;
192+
self.experimental_diagnostics = data.diagnostics_enableExperimental;
190193
self.diagnostics = DiagnosticsConfig {
191194
warnings_as_info: data.diagnostics_warningsAsInfo,
192195
warnings_as_hint: data.diagnostics_warningsAsHint,
@@ -405,6 +408,7 @@ config_data! {
405408
completion_postfix_enable: bool = true,
406409

407410
diagnostics_enable: bool = true,
411+
diagnostics_enableExperimental: bool = true,
408412
diagnostics_warningsAsHint: Vec<String> = Vec::new(),
409413
diagnostics_warningsAsInfo: Vec<String> = Vec::new(),
410414

0 commit comments

Comments
 (0)