Mirrors/rust-analyzer
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-analyzer synced 2024-12-26 13:03:31 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge #5553

Browse source 
5553: Add fix ranges for diagnostics r=matklad a=SomeoneToIgnore

A follow-up of https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Less.20red.20in.20the.20code

Now diagnostics can apply fixes in a range that's different from the range used to highlight the diagnostics.
Previous logic did not consider the fix range, having both ranges equal, which could cause a lot of red noise in the editor.
Now, the fix range gets used with the fix, the diagnostics range is used for everything else which allows to improve the error highlighting.

before:
<img width="191" alt="image" src="https://user-images.githubusercontent.com/2690773/88590727-df9a6a00-d063-11ea-97ed-9809c1c5e6e6.png">
after:
<img width="222" alt="image" src="https://user-images.githubusercontent.com/2690773/88590734-e1fcc400-d063-11ea-9b7c-25701cbd5352.png">

`MissingFields` and `MissingPatFields` diagnostics now have the fix range as `ast::RecordFieldList` of the expression with an error (as it was before this PR), and the diagnostics range as a `ast::Path` of the expression, if it's present (do you have any example of `ast::Expr::RecordLit` that has no path btw?).
The rest of the diagnostics have both ranges equal, same as it was before this PR.

Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
This commit is contained in:
bors[bot] 2020-08-12 13:44:13 +00:00 committed by GitHub
commit 5b8fdfe231
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 288 additions and 287 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
crates/ra_assists/src/handlers/fix_visibility.rs
View file

@ -121,7 +121,7 @@ fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) ->
Some(cap) => match current_visibility { Some(cap) => match current_visibility {
Some(current_visibility) => builder.replace_snippet( Some(current_visibility) => builder.replace_snippet(
cap, cap,
dbg!(current_visibility.syntax()).text_range(), current_visibility.syntax().text_range(),
format!("$0{}", missing_visibility), format!("$0{}", missing_visibility),
), ),
None => builder.insert_snippet(cap, offset, format!("$0{} ", missing_visibility)), None => builder.insert_snippet(cap, offset, format!("$0{} ", missing_visibility)),

4
crates/ra_hir/src/diagnostics.rs
View file

@ -1,8 +1,6 @@
//! FIXME: write short doc here //! FIXME: write short doc here
pub use hir_def::diagnostics::UnresolvedModule; pub use hir_def::diagnostics::UnresolvedModule;
pub use hir_expand::diagnostics::{ pub use hir_expand::diagnostics::{Diagnostic, DiagnosticSink, DiagnosticSinkBuilder};
AstDiagnostic, Diagnostic, DiagnosticSink, DiagnosticSinkBuilder,
};
pub use hir_ty::diagnostics::{ pub use hir_ty::diagnostics::{
MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, NoSuchField, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, NoSuchField,
}; };

18
crates/ra_hir/src/semantics.rs
View file

@ -8,7 +8,7 @@ use hir_def::{
resolver::{self, HasResolver, Resolver}, resolver::{self, HasResolver, Resolver},
AsMacroCall, FunctionId, TraitId, VariantId, AsMacroCall, FunctionId, TraitId, VariantId,
}; };
use hir_expand::{diagnostics::AstDiagnostic, hygiene::Hygiene, name::AsName, ExpansionInfo}; use hir_expand::{hygiene::Hygiene, name::AsName, ExpansionInfo};
use hir_ty::associated_type_shorthand_candidates; use hir_ty::associated_type_shorthand_candidates;
use itertools::Itertools; use itertools::Itertools;
use ra_db::{FileId, FileRange}; use ra_db::{FileId, FileRange};
@ -110,13 +110,6 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
self.imp.parse(file_id) self.imp.parse(file_id)
} }
pub fn ast<T: AstDiagnostic + Diagnostic>(&self, d: &T) -> <T as AstDiagnostic>::AST {
let file_id = d.source().file_id;
let root = self.db.parse_or_expand(file_id).unwrap();
self.imp.cache(root, file_id);
d.ast(self.db.upcast())
}
pub fn expand(&self, macro_call: &ast::MacroCall) -> Option<SyntaxNode> { pub fn expand(&self, macro_call: &ast::MacroCall) -> Option<SyntaxNode> {
self.imp.expand(macro_call) self.imp.expand(macro_call)
} }
@ -146,8 +139,8 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
self.imp.original_range(node) self.imp.original_range(node)
} }
pub fn diagnostics_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { pub fn diagnostics_display_range(&self, diagnostics: &dyn Diagnostic) -> FileRange {
self.imp.diagnostics_range(diagnostics) self.imp.diagnostics_display_range(diagnostics)
} }
pub fn ancestors_with_macros(&self, node: SyntaxNode) -> impl Iterator<Item = SyntaxNode> + '_ { pub fn ancestors_with_macros(&self, node: SyntaxNode) -> impl Iterator<Item = SyntaxNode> + '_ {
@ -389,10 +382,11 @@ impl<'db> SemanticsImpl<'db> {
original_range(self.db, node.as_ref()) original_range(self.db, node.as_ref())
} }
fn diagnostics_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { fn diagnostics_display_range(&self, diagnostics: &dyn Diagnostic) -> FileRange {
let src = diagnostics.source(); let src = diagnostics.display_source();
let root = self.db.parse_or_expand(src.file_id).unwrap(); let root = self.db.parse_or_expand(src.file_id).unwrap();
let node = src.value.to_node(&root); let node = src.value.to_node(&root);
self.cache(root, src.file_id);
original_range(self.db, src.with_value(&node)) original_range(self.db, src.with_value(&node))
} }

2
crates/ra_hir_def/src/diagnostics.rs
View file

@ -18,7 +18,7 @@ impl Diagnostic for UnresolvedModule {
fn message(&self) -> String { fn message(&self) -> String {
"unresolved module".to_string() "unresolved module".to_string()
} }
fn source(&self) -> InFile<SyntaxNodePtr> { fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile::new(self.file, self.decl.clone().into()) InFile::new(self.file, self.decl.clone().into())
} }
fn as_any(&self) -> &(dyn Any + Send + 'static) { fn as_any(&self) -> &(dyn Any + Send + 'static) {

25
crates/ra_hir_expand/src/diagnostics.rs
View file

@ -16,35 +16,20 @@
use std::{any::Any, fmt}; use std::{any::Any, fmt};
use ra_syntax::{SyntaxNode, SyntaxNodePtr}; use ra_syntax::SyntaxNodePtr;
use crate::{db::AstDatabase, InFile}; use crate::InFile;
pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static {
fn message(&self) -> String; fn message(&self) -> String;
fn source(&self) -> InFile<SyntaxNodePtr>; /// Used in highlighting and related purposes
fn display_source(&self) -> InFile<SyntaxNodePtr>;
fn as_any(&self) -> &(dyn Any + Send + 'static); fn as_any(&self) -> &(dyn Any + Send + 'static);
fn is_experimental(&self) -> bool { fn is_experimental(&self) -> bool {
false false
} }
} }
pub trait AstDiagnostic {
type AST;
fn ast(&self, db: &dyn AstDatabase) -> Self::AST;
}
impl dyn Diagnostic {
pub fn syntax_node(&self, db: &impl AstDatabase) -> SyntaxNode {
let node = db.parse_or_expand(self.source().file_id).unwrap();
self.source().value.to_node(&node)
}
pub fn downcast_ref<D: Diagnostic>(&self) -> Option<&D> {
self.as_any().downcast_ref()
}
}
pub struct DiagnosticSink<'a> { pub struct DiagnosticSink<'a> {
callbacks: Vec<Box<dyn FnMut(&dyn Diagnostic) -> Result<(), ()> + 'a>>, callbacks: Vec<Box<dyn FnMut(&dyn Diagnostic) -> Result<(), ()> + 'a>>,
filters: Vec<Box<dyn FnMut(&dyn Diagnostic) -> bool + 'a>>, filters: Vec<Box<dyn FnMut(&dyn Diagnostic) -> bool + 'a>>,
@ -89,7 +74,7 @@ impl<'a> DiagnosticSinkBuilder<'a> {
} }
pub fn on<D: Diagnostic, F: FnMut(&D) + 'a>(mut self, mut cb: F) -> Self { pub fn on<D: Diagnostic, F: FnMut(&D) + 'a>(mut self, mut cb: F) -> Self {
let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::<D>() { let cb = move |diag: &dyn Diagnostic| match diag.as_any().downcast_ref::<D>() {
Some(d) => { Some(d) => {
cb(d); cb(d);
Ok(()) Ok(())

124
crates/ra_hir_ty/src/diagnostics.rs
View file

@ -6,10 +6,10 @@ mod unsafe_check;
use std::any::Any; use std::any::Any;
use hir_def::DefWithBodyId; use hir_def::DefWithBodyId;
use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink}; use hir_expand::diagnostics::{Diagnostic, DiagnosticSink};
use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile}; use hir_expand::{name::Name, HirFileId, InFile};
use ra_prof::profile; use ra_prof::profile;
use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr}; use ra_syntax::{ast, AstPtr, SyntaxNodePtr};
use stdx::format_to; use stdx::format_to;
use crate::db::HirDatabase; use crate::db::HirDatabase;
@ -37,7 +37,7 @@ impl Diagnostic for NoSuchField {
"no such field".to_string() "no such field".to_string()
} }
fn source(&self) -> InFile<SyntaxNodePtr> { fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile::new(self.file, self.field.clone().into()) InFile::new(self.file, self.field.clone().into())
} }
@ -46,20 +46,11 @@ impl Diagnostic for NoSuchField {
} }
} }
impl AstDiagnostic for NoSuchField {
type AST = ast::RecordExprField;
fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
let root = db.parse_or_expand(self.source().file_id).unwrap();
let node = self.source().value.to_node(&root);
ast::RecordExprField::cast(node).unwrap()
}
}
#[derive(Debug)] #[derive(Debug)]
pub struct MissingFields { pub struct MissingFields {
pub file: HirFileId, pub file: HirFileId,
pub field_list: AstPtr<ast::RecordExprFieldList>, pub field_list_parent: AstPtr<ast::RecordExpr>,
pub field_list_parent_path: Option<AstPtr<ast::Path>>,
pub missed_fields: Vec<Name>, pub missed_fields: Vec<Name>,
} }
@ -71,28 +62,28 @@ impl Diagnostic for MissingFields {
} }
buf buf
} }
fn source(&self) -> InFile<SyntaxNodePtr> {
InFile { file_id: self.file, value: self.field_list.clone().into() } fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile {
file_id: self.file,
value: self
.field_list_parent_path
.clone()
.map(SyntaxNodePtr::from)
.unwrap_or_else(|| self.field_list_parent.clone().into()),
} }
}
fn as_any(&self) -> &(dyn Any + Send + 'static) { fn as_any(&self) -> &(dyn Any + Send + 'static) {
self self
} }
} }
impl AstDiagnostic for MissingFields {
type AST = ast::RecordExprFieldList;
fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
let root = db.parse_or_expand(self.source().file_id).unwrap();
let node = self.source().value.to_node(&root);
ast::RecordExprFieldList::cast(node).unwrap()
}
}
#[derive(Debug)] #[derive(Debug)]
pub struct MissingPatFields { pub struct MissingPatFields {
pub file: HirFileId, pub file: HirFileId,
pub field_list: AstPtr<ast::RecordPatFieldList>, pub field_list_parent: AstPtr<ast::RecordPat>,
pub field_list_parent_path: Option<AstPtr<ast::Path>>,
pub missed_fields: Vec<Name>, pub missed_fields: Vec<Name>,
} }
@ -104,8 +95,15 @@ impl Diagnostic for MissingPatFields {
} }
buf buf
} }
fn source(&self) -> InFile<SyntaxNodePtr> { fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile { file_id: self.file, value: self.field_list.clone().into() } InFile {
file_id: self.file,
value: self
.field_list_parent_path
.clone()
.map(SyntaxNodePtr::from)
.unwrap_or_else(|| self.field_list_parent.clone().into()),
}
} }
fn as_any(&self) -> &(dyn Any + Send + 'static) { fn as_any(&self) -> &(dyn Any + Send + 'static) {
self self
@ -123,7 +121,7 @@ impl Diagnostic for MissingMatchArms {
fn message(&self) -> String { fn message(&self) -> String {
String::from("Missing match arm") String::from("Missing match arm")
} }
fn source(&self) -> InFile<SyntaxNodePtr> { fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile { file_id: self.file, value: self.match_expr.clone().into() } InFile { file_id: self.file, value: self.match_expr.clone().into() }
} }
fn as_any(&self) -> &(dyn Any + Send + 'static) { fn as_any(&self) -> &(dyn Any + Send + 'static) {
@ -141,7 +139,7 @@ impl Diagnostic for MissingOkInTailExpr {
fn message(&self) -> String { fn message(&self) -> String {
"wrap return expression in Ok".to_string() "wrap return expression in Ok".to_string()
} }
fn source(&self) -> InFile<SyntaxNodePtr> { fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile { file_id: self.file, value: self.expr.clone().into() } InFile { file_id: self.file, value: self.expr.clone().into() }
} }
fn as_any(&self) -> &(dyn Any + Send + 'static) { fn as_any(&self) -> &(dyn Any + Send + 'static) {
@ -149,16 +147,6 @@ impl Diagnostic for MissingOkInTailExpr {
} }
} }
impl AstDiagnostic for MissingOkInTailExpr {
type AST = ast::Expr;
fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
let root = db.parse_or_expand(self.file).unwrap();
let node = self.source().value.to_node(&root);
ast::Expr::cast(node).unwrap()
}
}
#[derive(Debug)] #[derive(Debug)]
pub struct BreakOutsideOfLoop { pub struct BreakOutsideOfLoop {
pub file: HirFileId, pub file: HirFileId,
@ -169,7 +157,7 @@ impl Diagnostic for BreakOutsideOfLoop {
fn message(&self) -> String { fn message(&self) -> String {
"break outside of loop".to_string() "break outside of loop".to_string()
} }
fn source(&self) -> InFile<SyntaxNodePtr> { fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile { file_id: self.file, value: self.expr.clone().into() } InFile { file_id: self.file, value: self.expr.clone().into() }
} }
fn as_any(&self) -> &(dyn Any + Send + 'static) { fn as_any(&self) -> &(dyn Any + Send + 'static) {
@ -177,16 +165,6 @@ impl Diagnostic for BreakOutsideOfLoop {
} }
} }
impl AstDiagnostic for BreakOutsideOfLoop {
type AST = ast::Expr;
fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
let root = db.parse_or_expand(self.file).unwrap();
let node = self.source().value.to_node(&root);
ast::Expr::cast(node).unwrap()
}
}
#[derive(Debug)] #[derive(Debug)]
pub struct MissingUnsafe { pub struct MissingUnsafe {
pub file: HirFileId, pub file: HirFileId,
@ -197,7 +175,7 @@ impl Diagnostic for MissingUnsafe {
fn message(&self) -> String { fn message(&self) -> String {
format!("This operation is unsafe and requires an unsafe function or block") format!("This operation is unsafe and requires an unsafe function or block")
} }
fn source(&self) -> InFile<SyntaxNodePtr> { fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile { file_id: self.file, value: self.expr.clone().into() } InFile { file_id: self.file, value: self.expr.clone().into() }
} }
fn as_any(&self) -> &(dyn Any + Send + 'static) { fn as_any(&self) -> &(dyn Any + Send + 'static) {
@ -205,16 +183,6 @@ impl Diagnostic for MissingUnsafe {
} }
} }
impl AstDiagnostic for MissingUnsafe {
type AST = ast::Expr;
fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
let root = db.parse_or_expand(self.source().file_id).unwrap();
let node = self.source().value.to_node(&root);
ast::Expr::cast(node).unwrap()
}
}
#[derive(Debug)] #[derive(Debug)]
pub struct MismatchedArgCount { pub struct MismatchedArgCount {
pub file: HirFileId, pub file: HirFileId,
@ -228,7 +196,7 @@ impl Diagnostic for MismatchedArgCount {
let s = if self.expected == 1 { "" } else { "s" }; let s = if self.expected == 1 { "" } else { "s" };
format!("Expected {} argument{}, found {}", self.expected, s, self.found) format!("Expected {} argument{}, found {}", self.expected, s, self.found)
} }
fn source(&self) -> InFile<SyntaxNodePtr> { fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile { file_id: self.file, value: self.call_expr.clone().into() } InFile { file_id: self.file, value: self.call_expr.clone().into() }
} }
fn as_any(&self) -> &(dyn Any + Send + 'static) { fn as_any(&self) -> &(dyn Any + Send + 'static) {
@ -239,19 +207,13 @@ impl Diagnostic for MismatchedArgCount {
} }
} }
impl AstDiagnostic for MismatchedArgCount {
type AST = ast::CallExpr;
fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
let root = db.parse_or_expand(self.source().file_id).unwrap();
let node = self.source().value.to_node(&root);
ast::CallExpr::cast(node).unwrap()
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId}; use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId};
use hir_expand::diagnostics::{Diagnostic, DiagnosticSinkBuilder}; use hir_expand::{
db::AstDatabase,
diagnostics::{Diagnostic, DiagnosticSinkBuilder},
};
use ra_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt}; use ra_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt};
use ra_syntax::{TextRange, TextSize}; use ra_syntax::{TextRange, TextSize};
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
@ -296,9 +258,11 @@ mod tests {
let mut actual: FxHashMap<FileId, Vec<(TextRange, String)>> = FxHashMap::default(); let mut actual: FxHashMap<FileId, Vec<(TextRange, String)>> = FxHashMap::default();
db.diagnostics(|d| { db.diagnostics(|d| {
// FXIME: macros... let src = d.display_source();
let file_id = d.source().file_id.original_file(&db); let root = db.parse_or_expand(src.file_id).unwrap();
let range = d.syntax_node(&db).text_range(); // FIXME: macros...
let file_id = src.file_id.original_file(&db);
let range = src.value.to_node(&root).text_range();
let message = d.message().to_owned(); let message = d.message().to_owned();
actual.entry(file_id).or_default().push((range, message)); actual.entry(file_id).or_default().push((range, message));
}); });
@ -326,7 +290,7 @@ struct S { foo: i32, bar: () }
impl S { impl S {
fn new() -> S { fn new() -> S {
S { S {
//^... Missing structure fields: //^ Missing structure fields:
//| - bar //| - bar
foo: 92, foo: 92,
baz: 62, baz: 62,
@ -448,7 +412,7 @@ impl Foo {
struct S { foo: i32, bar: () } struct S { foo: i32, bar: () }
fn baz(s: S) { fn baz(s: S) {
let S { foo: _ } = s; let S { foo: _ } = s;
//^^^^^^^^^^ Missing structure fields: //^ Missing structure fields:
//| - bar //| - bar
} }
"#, "#,

14
crates/ra_hir_ty/src/diagnostics/expr.rs
View file

@ -100,8 +100,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
if let Ok(source_ptr) = source_map.expr_syntax(id) { if let Ok(source_ptr) = source_map.expr_syntax(id) {
let root = source_ptr.file_syntax(db.upcast()); let root = source_ptr.file_syntax(db.upcast());
if let ast::Expr::RecordExpr(record_lit) = &source_ptr.value.to_node(&root) { if let ast::Expr::RecordExpr(record_expr) = &source_ptr.value.to_node(&root) {
if let Some(field_list) = record_lit.record_expr_field_list() { if let Some(_) = record_expr.record_expr_field_list() {
let variant_data = variant_data(db.upcast(), variant_def); let variant_data = variant_data(db.upcast(), variant_def);
let missed_fields = missed_fields let missed_fields = missed_fields
.into_iter() .into_iter()
@ -109,7 +109,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
.collect(); .collect();
self.sink.push(MissingFields { self.sink.push(MissingFields {
file: source_ptr.file_id, file: source_ptr.file_id,
field_list: AstPtr::new(&field_list), field_list_parent: AstPtr::new(&record_expr),
field_list_parent_path: record_expr.path().map(|path| AstPtr::new(&path)),
missed_fields, missed_fields,
}) })
} }
@ -131,7 +132,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
if let Some(expr) = source_ptr.value.as_ref().left() { if let Some(expr) = source_ptr.value.as_ref().left() {
let root = source_ptr.file_syntax(db.upcast()); let root = source_ptr.file_syntax(db.upcast());
if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) { if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) {
if let Some(field_list) = record_pat.record_pat_field_list() { if let Some(_) = record_pat.record_pat_field_list() {
let variant_data = variant_data(db.upcast(), variant_def); let variant_data = variant_data(db.upcast(), variant_def);
let missed_fields = missed_fields let missed_fields = missed_fields
.into_iter() .into_iter()
@ -139,7 +140,10 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
.collect(); .collect();
self.sink.push(MissingPatFields { self.sink.push(MissingPatFields {
file: source_ptr.file_id, file: source_ptr.file_id,
field_list: AstPtr::new(&field_list), field_list_parent: AstPtr::new(&record_pat),
field_list_parent_path: record_pat
.path()
.map(|path| AstPtr::new(&path)),
missed_fields, missed_fields,
}) })
} }

4
crates/ra_hir_ty/src/diagnostics/match_check.rs
View file

@ -1161,14 +1161,14 @@ fn main() {
//^ Missing match arm //^ Missing match arm
match a { match a {
Either::A { } => (), Either::A { } => (),
//^^^ Missing structure fields: //^^^^^^^^^ Missing structure fields:
// | - foo // | - foo
Either::B => (), Either::B => (),
} }
match a { match a {
//^ Missing match arm //^ Missing match arm
Either::A { } => (), Either::A { } => (),
} //^^^ Missing structure fields: } //^^^^^^^^^ Missing structure fields:
// | - foo // | - foo
match a { match a {

184
crates/ra_ide/src/diagnostics.rs
View file

@ -6,22 +6,21 @@
use std::cell::RefCell; use std::cell::RefCell;
use hir::{ use hir::{diagnostics::DiagnosticSinkBuilder, Semantics};
diagnostics::{AstDiagnostic, Diagnostic as _, DiagnosticSinkBuilder},
HasSource, HirDisplay, Semantics, VariantDef,
};
use itertools::Itertools; use itertools::Itertools;
use ra_db::SourceDatabase; use ra_db::SourceDatabase;
use ra_ide_db::RootDatabase; use ra_ide_db::RootDatabase;
use ra_prof::profile; use ra_prof::profile;
use ra_syntax::{ use ra_syntax::{
algo, ast::{self, AstNode},
ast::{self, edit::IndentLevel, make, AstNode},
SyntaxNode, TextRange, T, SyntaxNode, TextRange, T,
}; };
use ra_text_edit::{TextEdit, TextEditBuilder}; use ra_text_edit::{TextEdit, TextEditBuilder};
use crate::{Diagnostic, FileId, FileSystemEdit, Fix, SourceFileEdit}; use crate::{Diagnostic, FileId, Fix, SourceFileEdit};
mod diagnostics_with_fix;
use diagnostics_with_fix::DiagnosticWithFix;
#[derive(Debug, Copy, Clone)] #[derive(Debug, Copy, Clone)]
pub enum Severity { pub enum Severity {
@ -54,73 +53,16 @@ pub(crate) fn diagnostics(
let res = RefCell::new(res); let res = RefCell::new(res);
let mut sink = DiagnosticSinkBuilder::new() let mut sink = DiagnosticSinkBuilder::new()
.on::<hir::diagnostics::UnresolvedModule, _>(|d| { .on::<hir::diagnostics::UnresolvedModule, _>(|d| {
let original_file = d.source().file_id.original_file(db); res.borrow_mut().push(diagnostic_with_fix(d, &sema));
let fix = Fix::new(
"Create module",
FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() }
.into(),
);
res.borrow_mut().push(Diagnostic {
range: sema.diagnostics_range(d).range,
message: d.message(),
severity: Severity::Error,
fix: Some(fix),
})
}) })
.on::<hir::diagnostics::MissingFields, _>(|d| { .on::<hir::diagnostics::MissingFields, _>(|d| {
// Note that although we could add a diagnostics to res.borrow_mut().push(diagnostic_with_fix(d, &sema));
// fill the missing tuple field, e.g :
// `struct A(usize);`
// `let a = A { 0: () }`
// but it is uncommon usage and it should not be encouraged.
let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) {
None
} else {
let mut field_list = d.ast(db);
for f in d.missed_fields.iter() {
let field = make::record_expr_field(
make::name_ref(&f.to_string()),
Some(make::expr_unit()),
);
field_list = field_list.append_field(&field);
}
let edit = {
let mut builder = TextEditBuilder::default();
algo::diff(&d.ast(db).syntax(), &field_list.syntax())
.into_text_edit(&mut builder);
builder.finish()
};
Some(Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()))
};
res.borrow_mut().push(Diagnostic {
range: sema.diagnostics_range(d).range,
message: d.message(),
severity: Severity::Error,
fix,
})
}) })
.on::<hir::diagnostics::MissingOkInTailExpr, _>(|d| { .on::<hir::diagnostics::MissingOkInTailExpr, _>(|d| {
let node = d.ast(db); res.borrow_mut().push(diagnostic_with_fix(d, &sema));
let replacement = format!("Ok({})", node.syntax());
let edit = TextEdit::replace(node.syntax().text_range(), replacement);
let source_change = SourceFileEdit { file_id, edit }.into();
let fix = Fix::new("Wrap with ok", source_change);
res.borrow_mut().push(Diagnostic {
range: sema.diagnostics_range(d).range,
message: d.message(),
severity: Severity::Error,
fix: Some(fix),
})
}) })
.on::<hir::diagnostics::NoSuchField, _>(|d| { .on::<hir::diagnostics::NoSuchField, _>(|d| {
res.borrow_mut().push(Diagnostic { res.borrow_mut().push(diagnostic_with_fix(d, &sema));
range: sema.diagnostics_range(d).range,
message: d.message(),
severity: Severity::Error,
fix: missing_struct_field_fix(&sema, file_id, d),
})
}) })
// Only collect experimental diagnostics when they're enabled. // Only collect experimental diagnostics when they're enabled.
.filter(|diag| !diag.is_experimental() || enable_experimental) .filter(|diag| !diag.is_experimental() || enable_experimental)
@ -128,7 +70,7 @@ pub(crate) fn diagnostics(
.build(|d| { .build(|d| {
res.borrow_mut().push(Diagnostic { res.borrow_mut().push(Diagnostic {
message: d.message(), message: d.message(),
range: sema.diagnostics_range(d).range, range: sema.diagnostics_display_range(d).range,
severity: Severity::Error, severity: Severity::Error,
fix: None, fix: None,
}) })
@ -141,77 +83,12 @@ pub(crate) fn diagnostics(
res.into_inner() res.into_inner()
} }
fn missing_struct_field_fix( fn diagnostic_with_fix<D: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> Diagnostic {
sema: &Semantics<RootDatabase>, Diagnostic {
usage_file_id: FileId, range: sema.diagnostics_display_range(d).range,
d: &hir::diagnostics::NoSuchField, message: d.message(),
) -> Option<Fix> { severity: Severity::Error,
let record_expr = sema.ast(d); fix: d.fix(&sema),
let record_lit = ast::RecordExpr::cast(record_expr.syntax().parent()?.parent()?)?;
let def_id = sema.resolve_variant(record_lit)?;
let module;
let def_file_id;
let record_fields = match VariantDef::from(def_id) {
VariantDef::Struct(s) => {
module = s.module(sema.db);
let source = s.source(sema.db);
def_file_id = source.file_id;
let fields = source.value.field_list()?;
record_field_list(fields)?
}
VariantDef::Union(u) => {
module = u.module(sema.db);
let source = u.source(sema.db);
def_file_id = source.file_id;
source.value.record_field_list()?
}
VariantDef::EnumVariant(e) => {
module = e.module(sema.db);
let source = e.source(sema.db);
def_file_id = source.file_id;
let fields = source.value.field_list()?;
record_field_list(fields)?
}
};
let def_file_id = def_file_id.original_file(sema.db);
let new_field_type = sema.type_of_expr(&record_expr.expr()?)?;
if new_field_type.is_unknown() {
return None;
}
let new_field = make::record_field(
record_expr.field_name()?,
make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?),
);
let last_field = record_fields.fields().last()?;
let last_field_syntax = last_field.syntax();
let indent = IndentLevel::from_node(last_field_syntax);
let mut new_field = new_field.to_string();
if usage_file_id != def_file_id {
new_field = format!("pub(crate) {}", new_field);
}
new_field = format!("\n{}{}", indent, new_field);
let needs_comma = !last_field_syntax.to_string().ends_with(',');
if needs_comma {
new_field = format!(",{}", new_field);
}
let source_change = SourceFileEdit {
file_id: def_file_id,
edit: TextEdit::insert(last_field_syntax.text_range().end(), new_field),
};
let fix = Fix::new("Create field", source_change.into());
return Some(fix);
fn record_field_list(field_def_list: ast::FieldList) -> Option<ast::RecordFieldList> {
match field_def_list {
ast::FieldList::RecordFieldList(it) => Some(it),
ast::FieldList::TupleFieldList(_) => None,
}
} }
} }
@ -222,24 +99,25 @@ fn check_unnecessary_braces_in_use_statement(
) -> Option<()> { ) -> Option<()> {
let use_tree_list = ast::UseTreeList::cast(node.clone())?; let use_tree_list = ast::UseTreeList::cast(node.clone())?;
if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() { if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() {
let range = use_tree_list.syntax().text_range(); let use_range = use_tree_list.syntax().text_range();
let edit = let edit =
text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(&single_use_tree) text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(&single_use_tree)
.unwrap_or_else(|| { .unwrap_or_else(|| {
let to_replace = single_use_tree.syntax().text().to_string(); let to_replace = single_use_tree.syntax().text().to_string();
let mut edit_builder = TextEditBuilder::default(); let mut edit_builder = TextEditBuilder::default();
edit_builder.delete(range); edit_builder.delete(use_range);
edit_builder.insert(range.start(), to_replace); edit_builder.insert(use_range.start(), to_replace);
edit_builder.finish() edit_builder.finish()
}); });
acc.push(Diagnostic { acc.push(Diagnostic {
range, range: use_range,
message: "Unnecessary braces in use statement".to_string(), message: "Unnecessary braces in use statement".to_string(),
severity: Severity::WeakWarning, severity: Severity::WeakWarning,
fix: Some(Fix::new( fix: Some(Fix::new(
"Remove unnecessary braces", "Remove unnecessary braces",
SourceFileEdit { file_id, edit }.into(), SourceFileEdit { file_id, edit }.into(),
use_range,
)), )),
}); });
} }
@ -254,8 +132,7 @@ fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(
if single_use_tree.path()?.segment()?.syntax().first_child_or_token()?.kind() == T![self] { if single_use_tree.path()?.segment()?.syntax().first_child_or_token()?.kind() == T![self] {
let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start(); let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start();
let end = use_tree_list_node.text_range().end(); let end = use_tree_list_node.text_range().end();
let range = TextRange::new(start, end); return Some(TextEdit::delete(TextRange::new(start, end)));
return Some(TextEdit::delete(range));
} }
None None
} }
@ -278,13 +155,15 @@ fn check_struct_shorthand_initialization(
edit_builder.insert(record_field.syntax().text_range().start(), field_name); edit_builder.insert(record_field.syntax().text_range().start(), field_name);
let edit = edit_builder.finish(); let edit = edit_builder.finish();
let field_range = record_field.syntax().text_range();
acc.push(Diagnostic { acc.push(Diagnostic {
range: record_field.syntax().text_range(), range: field_range,
message: "Shorthand struct initialization".to_string(), message: "Shorthand struct initialization".to_string(),
severity: Severity::WeakWarning, severity: Severity::WeakWarning,
fix: Some(Fix::new( fix: Some(Fix::new(
"Use struct shorthand initialization", "Use struct shorthand initialization",
SourceFileEdit { file_id, edit }.into(), SourceFileEdit { file_id, edit }.into(),
field_range,
)), )),
}); });
} }
@ -304,7 +183,7 @@ mod tests {
/// Takes a multi-file input fixture with annotated cursor positions, /// Takes a multi-file input fixture with annotated cursor positions,
/// and checks that: /// and checks that:
/// * a diagnostic is produced /// * a diagnostic is produced
/// * this diagnostic touches the input cursor position /// * this diagnostic fix trigger range touches the input cursor position
/// * that the contents of the file containing the cursor match `after` after the diagnostic fix is applied /// * that the contents of the file containing the cursor match `after` after the diagnostic fix is applied
fn check_fix(ra_fixture_before: &str, ra_fixture_after: &str) { fn check_fix(ra_fixture_before: &str, ra_fixture_after: &str) {
let after = trim_indent(ra_fixture_after); let after = trim_indent(ra_fixture_after);
@ -322,10 +201,10 @@ mod tests {
assert_eq_text!(&after, &actual); assert_eq_text!(&after, &actual);
assert!( assert!(
diagnostic.range.start() <= file_position.offset fix.fix_trigger_range.start() <= file_position.offset
&& diagnostic.range.end() >= file_position.offset, && fix.fix_trigger_range.end() >= file_position.offset,
"diagnostic range {:?} does not touch cursor position {:?}", "diagnostic fix range {:?} does not touch cursor position {:?}",
diagnostic.range, fix.fix_trigger_range,
file_position.offset file_position.offset
); );
} }
@ -642,6 +521,7 @@ fn test_fn() {
], ],
is_snippet: false, is_snippet: false,
}, },
fix_trigger_range: 0..8,
}, },
), ),
}, },

171
crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs Normal file
View file

@ -0,0 +1,171 @@
//! Provides a way to attach fixes to the diagnostics.
//! The same module also has all curret custom fixes for the diagnostics implemented.
use crate::Fix;
use ast::{edit::IndentLevel, make};
use hir::{
db::AstDatabase,
diagnostics::{Diagnostic, MissingFields, MissingOkInTailExpr, NoSuchField, UnresolvedModule},
HasSource, HirDisplay, Semantics, VariantDef,
};
use ra_db::FileId;
use ra_ide_db::{
source_change::{FileSystemEdit, SourceFileEdit},
RootDatabase,
};
use ra_syntax::{algo, ast, AstNode};
use ra_text_edit::{TextEdit, TextEditBuilder};
/// A [Diagnostic] that potentially has a fix available.
///
/// [Diagnostic]: hir::diagnostics::Diagnostic
pub trait DiagnosticWithFix: Diagnostic {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix>;
}
impl DiagnosticWithFix for UnresolvedModule {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
let root = sema.db.parse_or_expand(self.file)?;
let unresolved_module = self.decl.to_node(&root);
Some(Fix::new(
"Create module",
FileSystemEdit::CreateFile {
anchor: self.file.original_file(sema.db),
dst: self.candidate.clone(),
}
.into(),
unresolved_module.syntax().text_range(),
))
}
}
impl DiagnosticWithFix for NoSuchField {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
let root = sema.db.parse_or_expand(self.file)?;
missing_record_expr_field_fix(
&sema,
self.file.original_file(sema.db),
&self.field.to_node(&root),
)
}
}
impl DiagnosticWithFix for MissingFields {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
// Note that although we could add a diagnostics to
// fill the missing tuple field, e.g :
// `struct A(usize);`
// `let a = A { 0: () }`
// but it is uncommon usage and it should not be encouraged.
if self.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) {
return None;
}
let root = sema.db.parse_or_expand(self.file)?;
let old_field_list = self.field_list_parent.to_node(&root).record_expr_field_list()?;
let mut new_field_list = old_field_list.clone();
for f in self.missed_fields.iter() {
let field =
make::record_expr_field(make::name_ref(&f.to_string()), Some(make::expr_unit()));
new_field_list = new_field_list.append_field(&field);
}
let edit = {
let mut builder = TextEditBuilder::default();
algo::diff(&old_field_list.syntax(), &new_field_list.syntax())
.into_text_edit(&mut builder);
builder.finish()
};
Some(Fix::new(
"Fill struct fields",
SourceFileEdit { file_id: self.file.original_file(sema.db), edit }.into(),
sema.original_range(&old_field_list.syntax()).range,
))
}
}
impl DiagnosticWithFix for MissingOkInTailExpr {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
let root = sema.db.parse_or_expand(self.file)?;
let tail_expr = self.expr.to_node(&root);
let tail_expr_range = tail_expr.syntax().text_range();
let edit = TextEdit::replace(tail_expr_range, format!("Ok({})", tail_expr.syntax()));
let source_change =
SourceFileEdit { file_id: self.file.original_file(sema.db), edit }.into();
Some(Fix::new("Wrap with ok", source_change, tail_expr_range))
}
}
fn missing_record_expr_field_fix(
sema: &Semantics<RootDatabase>,
usage_file_id: FileId,
record_expr_field: &ast::RecordExprField,
) -> Option<Fix> {
let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?;
let def_id = sema.resolve_variant(record_lit)?;
let module;
let def_file_id;
let record_fields = match VariantDef::from(def_id) {
VariantDef::Struct(s) => {
module = s.module(sema.db);
let source = s.source(sema.db);
def_file_id = source.file_id;
let fields = source.value.field_list()?;
record_field_list(fields)?
}
VariantDef::Union(u) => {
module = u.module(sema.db);
let source = u.source(sema.db);
def_file_id = source.file_id;
source.value.record_field_list()?
}
VariantDef::EnumVariant(e) => {
module = e.module(sema.db);
let source = e.source(sema.db);
def_file_id = source.file_id;
let fields = source.value.field_list()?;
record_field_list(fields)?
}
};
let def_file_id = def_file_id.original_file(sema.db);
let new_field_type = sema.type_of_expr(&record_expr_field.expr()?)?;
if new_field_type.is_unknown() {
return None;
}
let new_field = make::record_field(
record_expr_field.field_name()?,
make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?),
);
let last_field = record_fields.fields().last()?;
let last_field_syntax = last_field.syntax();
let indent = IndentLevel::from_node(last_field_syntax);
let mut new_field = new_field.to_string();
if usage_file_id != def_file_id {
new_field = format!("pub(crate) {}", new_field);
}
new_field = format!("\n{}{}", indent, new_field);
let needs_comma = !last_field_syntax.to_string().ends_with(',');
if needs_comma {
new_field = format!(",{}", new_field);
}
let source_change = SourceFileEdit {
file_id: def_file_id,
edit: TextEdit::insert(last_field_syntax.text_range().end(), new_field),
};
return Some(Fix::new(
"Create field",
source_change.into(),
record_expr_field.syntax().text_range(),
));
fn record_field_list(field_def_list: ast::FieldList) -> Option<ast::RecordFieldList> {
match field_def_list {
ast::FieldList::RecordFieldList(it) => Some(it),
ast::FieldList::TupleFieldList(_) => None,
}
}
}

10
crates/ra_ide/src/lib.rs
View file

@ -112,13 +112,19 @@ pub struct Diagnostic {
pub struct Fix { pub struct Fix {
pub label: String, pub label: String,
pub source_change: SourceChange, pub source_change: SourceChange,
/// Allows to trigger the fix only when the caret is in the range given
pub fix_trigger_range: TextRange,
} }
impl Fix { impl Fix {
pub fn new(label: impl Into<String>, source_change: SourceChange) -> Self { pub fn new(
label: impl Into<String>,
source_change: SourceChange,
fix_trigger_range: TextRange,
) -> Self {
let label = label.into(); let label = label.into();
assert!(label.starts_with(char::is_uppercase) && !label.ends_with('.')); assert!(label.starts_with(char::is_uppercase) && !label.ends_with('.'));
Self { label, source_change } Self { label, source_change, fix_trigger_range }
} }
} }

9
crates/rust-analyzer/src/handlers.rs
View file

@ -773,12 +773,11 @@ fn handle_fixes(
let diagnostics = snap.analysis.diagnostics(file_id, snap.config.experimental_diagnostics)?; let diagnostics = snap.analysis.diagnostics(file_id, snap.config.experimental_diagnostics)?;
let fixes_from_diagnostics = diagnostics for fix in diagnostics
.into_iter() .into_iter()
.filter_map(|d| Some((d.range, d.fix?))) .filter_map(|d| d.fix)
.filter(|(diag_range, _fix)| diag_range.intersect(range).is_some()) .filter(|fix| fix.fix_trigger_range.intersect(range).is_some())
.map(|(_range, fix)| fix); {
for fix in fixes_from_diagnostics {
let title = fix.label; let title = fix.label;
let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?; let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?;
let action = lsp_ext::CodeAction { let action = lsp_ext::CodeAction {