From 58712088ac2114e41d754ec6bcb5cd6bc3625256 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 14 Jun 2021 21:23:59 +0300 Subject: minor: make diagnostics more similar --- .../src/handlers/field_shorthand.rs | 203 +++++++++++++++++++++ .../ide_diagnostics/src/handlers/missing_fields.rs | 29 +++ .../ide_diagnostics/src/handlers/unlinked_file.rs | 23 +-- .../ide_diagnostics/src/handlers/useless_braces.rs | 148 +++++++++++++++ 4 files changed, 390 insertions(+), 13 deletions(-) create mode 100644 crates/ide_diagnostics/src/handlers/field_shorthand.rs create mode 100644 crates/ide_diagnostics/src/handlers/useless_braces.rs (limited to 'crates/ide_diagnostics/src/handlers') diff --git a/crates/ide_diagnostics/src/handlers/field_shorthand.rs b/crates/ide_diagnostics/src/handlers/field_shorthand.rs new file mode 100644 index 000000000..33152e284 --- /dev/null +++ b/crates/ide_diagnostics/src/handlers/field_shorthand.rs @@ -0,0 +1,203 @@ +//! Suggests shortening `Foo { field: field }` to `Foo { field }` in both +//! expressions and patterns. + +use ide_db::{base_db::FileId, source_change::SourceChange}; +use syntax::{ast, match_ast, AstNode, SyntaxNode}; +use text_edit::TextEdit; + +use crate::{fix, Diagnostic, Severity}; + +pub(crate) fn field_shorthand(acc: &mut Vec, file_id: FileId, node: &SyntaxNode) { + match_ast! { + match node { + ast::RecordExpr(it) => check_expr_field_shorthand(acc, file_id, it), + ast::RecordPat(it) => check_pat_field_shorthand(acc, file_id, it), + _ => () + } + }; +} + +fn check_expr_field_shorthand( + acc: &mut Vec, + file_id: FileId, + record_expr: ast::RecordExpr, +) { + let record_field_list = match record_expr.record_expr_field_list() { + Some(it) => it, + None => return, + }; + for record_field in record_field_list.fields() { + let (name_ref, expr) = match record_field.name_ref().zip(record_field.expr()) { + Some(it) => it, + None => continue, + }; + + let field_name = name_ref.syntax().text().to_string(); + let field_expr = expr.syntax().text().to_string(); + let field_name_is_tup_index = name_ref.as_tuple_field().is_some(); + if field_name != field_expr || field_name_is_tup_index { + continue; + } + + let mut edit_builder = TextEdit::builder(); + edit_builder.delete(record_field.syntax().text_range()); + edit_builder.insert(record_field.syntax().text_range().start(), field_name); + let edit = edit_builder.finish(); + + let field_range = record_field.syntax().text_range(); + acc.push( + Diagnostic::new("use-field-shorthand", "Shorthand struct initialization", field_range) + .severity(Severity::WeakWarning) + .with_fixes(Some(vec![fix( + "use_expr_field_shorthand", + "Use struct shorthand initialization", + SourceChange::from_text_edit(file_id, edit), + field_range, + )])), + ); + } +} + +fn check_pat_field_shorthand( + acc: &mut Vec, + file_id: FileId, + record_pat: ast::RecordPat, +) { + let record_pat_field_list = match record_pat.record_pat_field_list() { + Some(it) => it, + None => return, + }; + for record_pat_field in record_pat_field_list.fields() { + let (name_ref, pat) = match record_pat_field.name_ref().zip(record_pat_field.pat()) { + Some(it) => it, + None => continue, + }; + + let field_name = name_ref.syntax().text().to_string(); + let field_pat = pat.syntax().text().to_string(); + let field_name_is_tup_index = name_ref.as_tuple_field().is_some(); + if field_name != field_pat || field_name_is_tup_index { + continue; + } + + let mut edit_builder = TextEdit::builder(); + edit_builder.delete(record_pat_field.syntax().text_range()); + edit_builder.insert(record_pat_field.syntax().text_range().start(), field_name); + let edit = edit_builder.finish(); + + let field_range = record_pat_field.syntax().text_range(); + acc.push( + Diagnostic::new("use-field-shorthand", "Shorthand struct pattern", field_range) + .severity(Severity::WeakWarning) + .with_fixes(Some(vec![fix( + "use_pat_field_shorthand", + "Use struct field shorthand", + SourceChange::from_text_edit(file_id, edit), + field_range, + )])), + ); + } +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_diagnostics, check_fix}; + + #[test] + fn test_check_expr_field_shorthand() { + check_diagnostics( + r#" +struct A { a: &'static str } +fn main() { A { a: "hello" } } +"#, + ); + check_diagnostics( + r#" +struct A(usize); +fn main() { A { 0: 0 } } +"#, + ); + + check_fix( + r#" +struct A { a: &'static str } +fn main() { + let a = "haha"; + A { a$0: a } +} +"#, + r#" +struct A { a: &'static str } +fn main() { + let a = "haha"; + A { a } +} +"#, + ); + + check_fix( + r#" +struct A { a: &'static str, b: &'static str } +fn main() { + let a = "haha"; + let b = "bb"; + A { a$0: a, b } +} +"#, + r#" +struct A { a: &'static str, b: &'static str } +fn main() { + let a = "haha"; + let b = "bb"; + A { a, b } +} +"#, + ); + } + + #[test] + fn test_check_pat_field_shorthand() { + check_diagnostics( + r#" +struct A { a: &'static str } +fn f(a: A) { let A { a: hello } = a; } +"#, + ); + check_diagnostics( + r#" +struct A(usize); +fn f(a: A) { let A { 0: 0 } = a; } +"#, + ); + + check_fix( + r#" +struct A { a: &'static str } +fn f(a: A) { + let A { a$0: a } = a; +} +"#, + r#" +struct A { a: &'static str } +fn f(a: A) { + let A { a } = a; +} +"#, + ); + + check_fix( + r#" +struct A { a: &'static str, b: &'static str } +fn f(a: A) { + let A { a$0: a, b } = a; +} +"#, + r#" +struct A { a: &'static str, b: &'static str } +fn f(a: A) { + let A { a, b } = a; +} +"#, + ); + } +} diff --git a/crates/ide_diagnostics/src/handlers/missing_fields.rs b/crates/ide_diagnostics/src/handlers/missing_fields.rs index bc82c0e4a..0dd36fb23 100644 --- a/crates/ide_diagnostics/src/handlers/missing_fields.rs +++ b/crates/ide_diagnostics/src/handlers/missing_fields.rs @@ -323,4 +323,33 @@ fn f() { "#, ); } + + #[test] + fn import_extern_crate_clash_with_inner_item() { + // This is more of a resolver test, but doesn't really work with the hir_def testsuite. + + check_diagnostics( + r#" +//- /lib.rs crate:lib deps:jwt +mod permissions; + +use permissions::jwt; + +fn f() { + fn inner() {} + jwt::Claims {}; // should resolve to the local one with 0 fields, and not get a diagnostic +} + +//- /permissions.rs +pub mod jwt { + pub struct Claims {} +} + +//- /jwt/lib.rs crate:jwt +pub struct Claims { + field: u8, +} + "#, + ); + } } diff --git a/crates/ide_diagnostics/src/handlers/unlinked_file.rs b/crates/ide_diagnostics/src/handlers/unlinked_file.rs index 8921ddde2..8e601fa48 100644 --- a/crates/ide_diagnostics/src/handlers/unlinked_file.rs +++ b/crates/ide_diagnostics/src/handlers/unlinked_file.rs @@ -14,32 +14,29 @@ use text_edit::TextEdit; use crate::{fix, Assist, Diagnostic, DiagnosticsContext}; -#[derive(Debug)] -pub(crate) struct UnlinkedFile { - pub(crate) file: FileId, -} - // Diagnostic: unlinked-file // // This diagnostic is shown for files that are not included in any crate, or files that are part of // crates rust-analyzer failed to discover. The file will not have IDE features available. -pub(crate) fn unlinked_file(ctx: &DiagnosticsContext, d: &UnlinkedFile) -> Diagnostic { +pub(crate) fn unlinked_file(ctx: &DiagnosticsContext, acc: &mut Vec, file_id: FileId) { // Limit diagnostic to the first few characters in the file. This matches how VS Code // renders it with the full span, but on other editors, and is less invasive. - let range = ctx.sema.db.parse(d.file).syntax_node().text_range(); + let range = ctx.sema.db.parse(file_id).syntax_node().text_range(); // FIXME: This is wrong if one of the first three characters is not ascii: `//Ы`. let range = range.intersect(TextRange::up_to(TextSize::of("..."))).unwrap_or(range); - Diagnostic::new("unlinked-file", "file not included in module tree", range) - .with_fixes(fixes(ctx, d)) + acc.push( + Diagnostic::new("unlinked-file", "file not included in module tree", range) + .with_fixes(fixes(ctx, file_id)), + ); } -fn fixes(ctx: &DiagnosticsContext, d: &UnlinkedFile) -> Option> { +fn fixes(ctx: &DiagnosticsContext, file_id: FileId) -> Option> { // If there's an existing module that could add `mod` or `pub mod` items to include the unlinked file, // suggest that as a fix. - let source_root = ctx.sema.db.source_root(ctx.sema.db.file_source_root(d.file)); - let our_path = source_root.path_for_file(&d.file)?; + let source_root = ctx.sema.db.source_root(ctx.sema.db.file_source_root(file_id)); + let our_path = source_root.path_for_file(&file_id)?; let module_name = our_path.name_and_extension()?.0; // Candidates to look for: @@ -68,7 +65,7 @@ fn fixes(ctx: &DiagnosticsContext, d: &UnlinkedFile) -> Option> { } if module.origin.file_id() == Some(*parent_id) { - return make_fixes(ctx.sema.db, *parent_id, module_name, d.file); + return make_fixes(ctx.sema.db, *parent_id, module_name, file_id); } } } diff --git a/crates/ide_diagnostics/src/handlers/useless_braces.rs b/crates/ide_diagnostics/src/handlers/useless_braces.rs new file mode 100644 index 000000000..8b9330e04 --- /dev/null +++ b/crates/ide_diagnostics/src/handlers/useless_braces.rs @@ -0,0 +1,148 @@ +use ide_db::{base_db::FileId, source_change::SourceChange}; +use itertools::Itertools; +use syntax::{ast, AstNode, SyntaxNode, TextRange}; +use text_edit::TextEdit; + +use crate::{fix, Diagnostic, Severity}; + +// Diagnostic: unnecessary-braces +// +// Diagnostic for unnecessary braces in `use` items. +pub(crate) fn useless_braces( + acc: &mut Vec, + file_id: FileId, + node: &SyntaxNode, +) -> Option<()> { + let use_tree_list = ast::UseTreeList::cast(node.clone())?; + if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() { + // If there is a comment inside the bracketed `use`, + // assume it is a commented out module path and don't show diagnostic. + if use_tree_list.has_inner_comment() { + return Some(()); + } + + let use_range = use_tree_list.syntax().text_range(); + let edit = remove_braces(&single_use_tree).unwrap_or_else(|| { + let to_replace = single_use_tree.syntax().text().to_string(); + let mut edit_builder = TextEdit::builder(); + edit_builder.delete(use_range); + edit_builder.insert(use_range.start(), to_replace); + edit_builder.finish() + }); + + acc.push( + Diagnostic::new( + "unnecessary-braces", + "Unnecessary braces in use statement".to_string(), + use_range, + ) + .severity(Severity::WeakWarning) + .with_fixes(Some(vec![fix( + "remove_braces", + "Remove unnecessary braces", + SourceChange::from_text_edit(file_id, edit), + use_range, + )])), + ); + } + + Some(()) +} + +fn remove_braces(single_use_tree: &ast::UseTree) -> Option { + let use_tree_list_node = single_use_tree.syntax().parent()?; + if single_use_tree.path()?.segment()?.self_token().is_some() { + let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start(); + let end = use_tree_list_node.text_range().end(); + return Some(TextEdit::delete(TextRange::new(start, end))); + } + None +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_diagnostics, check_fix}; + + #[test] + fn test_check_unnecessary_braces_in_use_statement() { + check_diagnostics( + r#" +use a; +use a::{c, d::e}; + +mod a { + mod c {} + mod d { + mod e {} + } +} +"#, + ); + check_diagnostics( + r#" +use a; +use a::{ + c, + // d::e +}; + +mod a { + mod c {} + mod d { + mod e {} + } +} +"#, + ); + check_fix( + r#" +mod b {} +use {$0b}; +"#, + r#" +mod b {} +use b; +"#, + ); + check_fix( + r#" +mod b {} +use {b$0}; +"#, + r#" +mod b {} +use b; +"#, + ); + check_fix( + r#" +mod a { mod c {} } +use a::{c$0}; +"#, + r#" +mod a { mod c {} } +use a::c; +"#, + ); + check_fix( + r#" +mod a {} +use a::{self$0}; +"#, + r#" +mod a {} +use a; +"#, + ); + check_fix( + r#" +mod a { mod c {} mod d { mod e {} } } +use a::{c, d::{e$0}}; +"#, + r#" +mod a { mod c {} mod d { mod e {} } } +use a::{c, d::e}; +"#, + ); + } +} -- cgit v1.2.3