From fa7fc0e5cb5343e2c59220fb91370f005c13be3a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 17 May 2021 12:04:17 +0300 Subject: internal: scalable module structure for fixits --- crates/ide/src/diagnostics/fixes/change_case.rs | 155 +++++++++++++++ crates/ide/src/diagnostics/fixes/create_field.rs | 157 +++++++++++++++ .../ide/src/diagnostics/fixes/remove_semicolon.rs | 31 +++ .../src/diagnostics/fixes/replace_with_find_map.rs | 42 ++++ .../ide/src/diagnostics/fixes/unresolved_module.rs | 87 +++++++++ crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs | 211 +++++++++++++++++++++ 6 files changed, 683 insertions(+) create mode 100644 crates/ide/src/diagnostics/fixes/change_case.rs create mode 100644 crates/ide/src/diagnostics/fixes/create_field.rs create mode 100644 crates/ide/src/diagnostics/fixes/remove_semicolon.rs create mode 100644 crates/ide/src/diagnostics/fixes/replace_with_find_map.rs create mode 100644 crates/ide/src/diagnostics/fixes/unresolved_module.rs create mode 100644 crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs (limited to 'crates/ide/src/diagnostics/fixes') diff --git a/crates/ide/src/diagnostics/fixes/change_case.rs b/crates/ide/src/diagnostics/fixes/change_case.rs new file mode 100644 index 000000000..80aca58a1 --- /dev/null +++ b/crates/ide/src/diagnostics/fixes/change_case.rs @@ -0,0 +1,155 @@ +use hir::{db::AstDatabase, diagnostics::IncorrectCase, InFile, Semantics}; +use ide_assists::{Assist, AssistResolveStrategy}; +use ide_db::{base_db::FilePosition, RootDatabase}; +use syntax::AstNode; + +use crate::{ + diagnostics::{unresolved_fix, DiagnosticWithFix}, + references::rename::rename_with_semantics, +}; + +impl DiagnosticWithFix for IncorrectCase { + fn fix( + &self, + sema: &Semantics, + resolve: &AssistResolveStrategy, + ) -> Option { + let root = sema.db.parse_or_expand(self.file)?; + let name_node = self.ident.to_node(&root); + + let name_node = InFile::new(self.file, name_node.syntax()); + let frange = name_node.original_file_range(sema.db); + let file_position = FilePosition { file_id: frange.file_id, offset: frange.range.start() }; + + let label = format!("Rename to {}", self.suggested_text); + let mut res = unresolved_fix("change_case", &label, frange.range); + if resolve.should_resolve(&res.id) { + let source_change = rename_with_semantics(sema, file_position, &self.suggested_text); + res.source_change = Some(source_change.ok().unwrap_or_default()); + } + + Some(res) + } +} + +#[cfg(test)] +mod change_case { + use crate::{ + diagnostics::tests::{check_fix, check_no_diagnostics}, + fixture, AssistResolveStrategy, DiagnosticsConfig, + }; + + #[test] + fn test_rename_incorrect_case() { + check_fix( + r#" +pub struct test_struct$0 { one: i32 } + +pub fn some_fn(val: test_struct) -> test_struct { + test_struct { one: val.one + 1 } +} +"#, + r#" +pub struct TestStruct { one: i32 } + +pub fn some_fn(val: TestStruct) -> TestStruct { + TestStruct { one: val.one + 1 } +} +"#, + ); + + check_fix( + r#" +pub fn some_fn(NonSnakeCase$0: u8) -> u8 { + NonSnakeCase +} +"#, + r#" +pub fn some_fn(non_snake_case: u8) -> u8 { + non_snake_case +} +"#, + ); + + check_fix( + r#" +pub fn SomeFn$0(val: u8) -> u8 { + if val != 0 { SomeFn(val - 1) } else { val } +} +"#, + r#" +pub fn some_fn(val: u8) -> u8 { + if val != 0 { some_fn(val - 1) } else { val } +} +"#, + ); + + check_fix( + r#" +fn some_fn() { + let whatAWeird_Formatting$0 = 10; + another_func(whatAWeird_Formatting); +} +"#, + r#" +fn some_fn() { + let what_a_weird_formatting = 10; + another_func(what_a_weird_formatting); +} +"#, + ); + } + + #[test] + fn test_uppercase_const_no_diagnostics() { + check_no_diagnostics( + r#" +fn foo() { + const ANOTHER_ITEM$0: &str = "some_item"; +} +"#, + ); + } + + #[test] + fn test_rename_incorrect_case_struct_method() { + check_fix( + r#" +pub struct TestStruct; + +impl TestStruct { + pub fn SomeFn$0() -> TestStruct { + TestStruct + } +} +"#, + r#" +pub struct TestStruct; + +impl TestStruct { + pub fn some_fn() -> TestStruct { + TestStruct + } +} +"#, + ); + } + + #[test] + fn test_single_incorrect_case_diagnostic_in_function_name_issue_6970() { + let input = r#"fn FOO$0() {}"#; + let expected = r#"fn foo() {}"#; + + let (analysis, file_position) = fixture::position(input); + let diagnostics = analysis + .diagnostics( + &DiagnosticsConfig::default(), + AssistResolveStrategy::All, + file_position.file_id, + ) + .unwrap(); + assert_eq!(diagnostics.len(), 1); + + check_fix(input, expected); + } +} diff --git a/crates/ide/src/diagnostics/fixes/create_field.rs b/crates/ide/src/diagnostics/fixes/create_field.rs new file mode 100644 index 000000000..24e0fda52 --- /dev/null +++ b/crates/ide/src/diagnostics/fixes/create_field.rs @@ -0,0 +1,157 @@ +use hir::{db::AstDatabase, diagnostics::NoSuchField, HasSource, HirDisplay, Semantics}; +use ide_db::{base_db::FileId, source_change::SourceChange, RootDatabase}; +use syntax::{ + ast::{self, edit::IndentLevel, make}, + AstNode, +}; +use text_edit::TextEdit; + +use crate::{ + diagnostics::{fix, DiagnosticWithFix}, + Assist, AssistResolveStrategy, +}; + +impl DiagnosticWithFix for NoSuchField { + fn fix( + &self, + sema: &Semantics, + _resolve: &AssistResolveStrategy, + ) -> Option { + 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), + ) + } +} + +fn missing_record_expr_field_fix( + sema: &Semantics, + usage_file_id: FileId, + record_expr_field: &ast::RecordExprField, +) -> Option { + 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 def_id { + hir::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)? + } + hir::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()? + } + hir::VariantDef::Variant(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( + None, + make::name(&record_expr_field.field_name()?.text()), + 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 = SourceChange::from_text_edit( + def_file_id, + TextEdit::insert(last_field_syntax.text_range().end(), new_field), + ); + + return Some(fix( + "create_field", + "Create field", + source_change, + record_expr_field.syntax().text_range(), + )); + + fn record_field_list(field_def_list: ast::FieldList) -> Option { + match field_def_list { + ast::FieldList::RecordFieldList(it) => Some(it), + ast::FieldList::TupleFieldList(_) => None, + } + } +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::check_fix; + + #[test] + fn test_add_field_from_usage() { + check_fix( + r" +fn main() { + Foo { bar: 3, baz$0: false}; +} +struct Foo { + bar: i32 +} +", + r" +fn main() { + Foo { bar: 3, baz: false}; +} +struct Foo { + bar: i32, + baz: bool +} +", + ) + } + + #[test] + fn test_add_field_in_other_file_from_usage() { + check_fix( + r#" +//- /main.rs +mod foo; + +fn main() { + foo::Foo { bar: 3, $0baz: false}; +} +//- /foo.rs +struct Foo { + bar: i32 +} +"#, + r#" +struct Foo { + bar: i32, + pub(crate) baz: bool +} +"#, + ) + } +} diff --git a/crates/ide/src/diagnostics/fixes/remove_semicolon.rs b/crates/ide/src/diagnostics/fixes/remove_semicolon.rs new file mode 100644 index 000000000..058002c69 --- /dev/null +++ b/crates/ide/src/diagnostics/fixes/remove_semicolon.rs @@ -0,0 +1,31 @@ +use hir::{db::AstDatabase, diagnostics::RemoveThisSemicolon, Semantics}; +use ide_assists::{Assist, AssistResolveStrategy}; +use ide_db::{source_change::SourceChange, RootDatabase}; +use syntax::{ast, AstNode}; +use text_edit::TextEdit; + +use crate::diagnostics::{fix, DiagnosticWithFix}; + +impl DiagnosticWithFix for RemoveThisSemicolon { + fn fix( + &self, + sema: &Semantics, + _resolve: &AssistResolveStrategy, + ) -> Option { + let root = sema.db.parse_or_expand(self.file)?; + + let semicolon = self + .expr + .to_node(&root) + .syntax() + .parent() + .and_then(ast::ExprStmt::cast) + .and_then(|expr| expr.semicolon_token())? + .text_range(); + + let edit = TextEdit::delete(semicolon); + let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); + + Some(fix("remove_semicolon", "Remove this semicolon", source_change, semicolon)) + } +} diff --git a/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs b/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs new file mode 100644 index 000000000..5ddfd2064 --- /dev/null +++ b/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs @@ -0,0 +1,42 @@ +use hir::{db::AstDatabase, diagnostics::ReplaceFilterMapNextWithFindMap, Semantics}; +use ide_assists::{Assist, AssistResolveStrategy}; +use ide_db::{source_change::SourceChange, RootDatabase}; +use syntax::{ + ast::{self, ArgListOwner}, + AstNode, TextRange, +}; +use text_edit::TextEdit; + +use crate::diagnostics::{fix, DiagnosticWithFix}; + +impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap { + fn fix( + &self, + sema: &Semantics, + _resolve: &AssistResolveStrategy, + ) -> Option { + let root = sema.db.parse_or_expand(self.file)?; + let next_expr = self.next_expr.to_node(&root); + let next_call = ast::MethodCallExpr::cast(next_expr.syntax().clone())?; + + let filter_map_call = ast::MethodCallExpr::cast(next_call.receiver()?.syntax().clone())?; + let filter_map_name_range = filter_map_call.name_ref()?.ident_token()?.text_range(); + let filter_map_args = filter_map_call.arg_list()?; + + let range_to_replace = + TextRange::new(filter_map_name_range.start(), next_expr.syntax().text_range().end()); + let replacement = format!("find_map{}", filter_map_args.syntax().text()); + let trigger_range = next_expr.syntax().text_range(); + + let edit = TextEdit::replace(range_to_replace, replacement); + + let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); + + Some(fix( + "replace_with_find_map", + "Replace filter_map(..).next() with find_map()", + source_change, + trigger_range, + )) + } +} diff --git a/crates/ide/src/diagnostics/fixes/unresolved_module.rs b/crates/ide/src/diagnostics/fixes/unresolved_module.rs new file mode 100644 index 000000000..81244b293 --- /dev/null +++ b/crates/ide/src/diagnostics/fixes/unresolved_module.rs @@ -0,0 +1,87 @@ +use hir::{db::AstDatabase, diagnostics::UnresolvedModule, Semantics}; +use ide_assists::{Assist, AssistResolveStrategy}; +use ide_db::{base_db::AnchoredPathBuf, source_change::FileSystemEdit, RootDatabase}; +use syntax::AstNode; + +use crate::diagnostics::{fix, DiagnosticWithFix}; + +impl DiagnosticWithFix for UnresolvedModule { + fn fix( + &self, + sema: &Semantics, + _resolve: &AssistResolveStrategy, + ) -> Option { + let root = sema.db.parse_or_expand(self.file)?; + let unresolved_module = self.decl.to_node(&root); + Some(fix( + "create_module", + "Create module", + FileSystemEdit::CreateFile { + dst: AnchoredPathBuf { + anchor: self.file.original_file(sema.db), + path: self.candidate.clone(), + }, + initial_contents: "".to_string(), + } + .into(), + unresolved_module.syntax().text_range(), + )) + } +} + +#[cfg(test)] +mod tests { + use expect_test::expect; + + use crate::diagnostics::tests::check_expect; + + #[test] + fn test_unresolved_module_diagnostic() { + check_expect( + r#"mod foo;"#, + expect![[r#" + [ + Diagnostic { + message: "unresolved module", + range: 0..8, + severity: Error, + fix: Some( + Assist { + id: AssistId( + "create_module", + QuickFix, + ), + label: "Create module", + group: None, + target: 0..8, + source_change: Some( + SourceChange { + source_file_edits: {}, + file_system_edits: [ + CreateFile { + dst: AnchoredPathBuf { + anchor: FileId( + 0, + ), + path: "foo.rs", + }, + initial_contents: "", + }, + ], + is_snippet: false, + }, + ), + }, + ), + unused: false, + code: Some( + DiagnosticCode( + "unresolved-module", + ), + ), + }, + ] + "#]], + ); + } +} diff --git a/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs b/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs new file mode 100644 index 000000000..66676064a --- /dev/null +++ b/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs @@ -0,0 +1,211 @@ +use hir::{db::AstDatabase, diagnostics::MissingOkOrSomeInTailExpr, Semantics}; +use ide_assists::{Assist, AssistResolveStrategy}; +use ide_db::{source_change::SourceChange, RootDatabase}; +use syntax::AstNode; +use text_edit::TextEdit; + +use crate::diagnostics::{fix, DiagnosticWithFix}; + +impl DiagnosticWithFix for MissingOkOrSomeInTailExpr { + fn fix( + &self, + sema: &Semantics, + _resolve: &AssistResolveStrategy, + ) -> Option { + 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 replacement = format!("{}({})", self.required, tail_expr.syntax()); + let edit = TextEdit::replace(tail_expr_range, replacement); + let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); + let name = if self.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" }; + Some(fix("wrap_tail_expr", name, source_change, tail_expr_range)) + } +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::{check_fix, check_no_diagnostics}; + + #[test] + fn test_wrap_return_type_option() { + check_fix( + r#" +//- /main.rs crate:main deps:core +use core::option::Option::{self, Some, None}; + +fn div(x: i32, y: i32) -> Option { + if y == 0 { + return None; + } + x / y$0 +} +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + r#" +use core::option::Option::{self, Some, None}; + +fn div(x: i32, y: i32) -> Option { + if y == 0 { + return None; + } + Some(x / y) +} +"#, + ); + } + + #[test] + fn test_wrap_return_type() { + check_fix( + r#" +//- /main.rs crate:main deps:core +use core::result::Result::{self, Ok, Err}; + +fn div(x: i32, y: i32) -> Result { + if y == 0 { + return Err(()); + } + x / y$0 +} +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + r#" +use core::result::Result::{self, Ok, Err}; + +fn div(x: i32, y: i32) -> Result { + if y == 0 { + return Err(()); + } + Ok(x / y) +} +"#, + ); + } + + #[test] + fn test_wrap_return_type_handles_generic_functions() { + check_fix( + r#" +//- /main.rs crate:main deps:core +use core::result::Result::{self, Ok, Err}; + +fn div(x: T) -> Result { + if x == 0 { + return Err(7); + } + $0x +} +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + r#" +use core::result::Result::{self, Ok, Err}; + +fn div(x: T) -> Result { + if x == 0 { + return Err(7); + } + Ok(x) +} +"#, + ); + } + + #[test] + fn test_wrap_return_type_handles_type_aliases() { + check_fix( + r#" +//- /main.rs crate:main deps:core +use core::result::Result::{self, Ok, Err}; + +type MyResult = Result; + +fn div(x: i32, y: i32) -> MyResult { + if y == 0 { + return Err(()); + } + x $0/ y +} +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + r#" +use core::result::Result::{self, Ok, Err}; + +type MyResult = Result; + +fn div(x: i32, y: i32) -> MyResult { + if y == 0 { + return Err(()); + } + Ok(x / y) +} +"#, + ); + } + + #[test] + fn test_wrap_return_type_not_applicable_when_expr_type_does_not_match_ok_type() { + check_no_diagnostics( + r#" +//- /main.rs crate:main deps:core +use core::result::Result::{self, Ok, Err}; + +fn foo() -> Result<(), i32> { 0 } + +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + ); + } + + #[test] + fn test_wrap_return_type_not_applicable_when_return_type_is_not_result_or_option() { + check_no_diagnostics( + r#" +//- /main.rs crate:main deps:core +use core::result::Result::{self, Ok, Err}; + +enum SomeOtherEnum { Ok(i32), Err(String) } + +fn foo() -> SomeOtherEnum { 0 } + +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + ); + } +} -- cgit v1.2.3