From fc30c5ccbeba2a102922da497809dd3f812544c4 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 21:09:03 +0300 Subject: internal: refactor incorrect case diagnostics --- crates/hir/src/diagnostics.rs | 29 +---- crates/hir/src/lib.rs | 18 +-- crates/hir_ty/src/diagnostics.rs | 3 - crates/ide/src/diagnostics.rs | 16 +-- crates/ide/src/diagnostics/fixes.rs | 1 - crates/ide/src/diagnostics/fixes/change_case.rs | 155 ---------------------- crates/ide/src/diagnostics/incorrect_case.rs | 166 ++++++++++++++++++++++++ 7 files changed, 179 insertions(+), 209 deletions(-) delete mode 100644 crates/ide/src/diagnostics/fixes/change_case.rs create mode 100644 crates/ide/src/diagnostics/incorrect_case.rs diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index c294a803b..c2d608eb5 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -34,6 +34,7 @@ macro_rules! diagnostics { diagnostics![ BreakOutsideOfLoop, InactiveCode, + IncorrectCase, MacroError, MismatchedArgCount, MissingFields, @@ -195,31 +196,3 @@ impl Diagnostic for InternalBailedOut { } pub use hir_ty::diagnostics::IncorrectCase; - -impl Diagnostic for IncorrectCase { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("incorrect-ident-case") - } - - fn message(&self) -> String { - format!( - "{} `{}` should have {} name, e.g. `{}`", - self.ident_type, - self.ident_text, - self.expected_case.to_string(), - self.suggested_text - ) - } - - fn display_source(&self) -> InFile { - InFile::new(self.file, self.ident.clone().into()) - } - - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } - - fn is_experimental(&self) -> bool { - true - } -} diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index b2731b62f..fc147ade3 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -86,8 +86,8 @@ use crate::{ pub use crate::{ attrs::{HasAttrs, Namespace}, diagnostics::{ - AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError, - MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, + AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase, InternalBailedOut, + MacroError, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, MissingUnsafe, NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, UnresolvedProcMacro, @@ -340,7 +340,7 @@ impl ModuleDef { } } - pub fn diagnostics(self, db: &dyn HirDatabase, sink: &mut DiagnosticSink) { + pub fn diagnostics(self, db: &dyn HirDatabase) -> Vec { let id = match self { ModuleDef::Adt(it) => match it { Adt::Struct(it) => it.id.into(), @@ -353,17 +353,19 @@ impl ModuleDef { ModuleDef::Module(it) => it.id.into(), ModuleDef::Const(it) => it.id.into(), ModuleDef::Static(it) => it.id.into(), - _ => return, + _ => return Vec::new(), }; let module = match self.module(db) { Some(it) => it, - None => return, + None => return Vec::new(), }; + let mut acc = Vec::new(); for diag in hir_ty::diagnostics::validate_module_item(db, module.id.krate(), id) { - sink.push(diag) + acc.push(diag.into()) } + acc } } @@ -624,7 +626,7 @@ impl Module { acc.extend(m.diagnostics(db, sink, internal_diagnostics)) } } - _ => decl.diagnostics(db, sink), + _ => acc.extend(decl.diagnostics(db)), } } @@ -1234,7 +1236,7 @@ impl Function { } for diag in hir_ty::diagnostics::validate_module_item(db, krate, self.id.into()) { - sink.push(diag) + acc.push(diag.into()) } acc } diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs index 407273943..6339c9687 100644 --- a/crates/hir_ty/src/diagnostics.rs +++ b/crates/hir_ty/src/diagnostics.rs @@ -84,9 +84,6 @@ impl fmt::Display for IdentType { } } -// Diagnostic: incorrect-ident-case -// -// This diagnostic is triggered if an item name doesn't follow https://doc.rust-lang.org/1.0.0/style/style/naming/README.html[Rust naming convention]. #[derive(Debug)] pub struct IncorrectCase { pub file: HirFileId, diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 814e64ae4..f084f7b06 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -6,6 +6,7 @@ mod break_outside_of_loop; mod inactive_code; +mod incorrect_case; mod macro_error; mod mismatched_arg_count; mod missing_fields; @@ -135,7 +136,6 @@ pub struct DiagnosticsConfig { struct DiagnosticsContext<'a> { config: &'a DiagnosticsConfig, sema: Semantics<'a, RootDatabase>, - #[allow(unused)] resolve: &'a AssistResolveStrategy, } @@ -165,9 +165,6 @@ pub(crate) fn diagnostics( } let res = RefCell::new(res); let sink_builder = DiagnosticSinkBuilder::new() - .on::(|d| { - res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); - }) .on::(|d| { // 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. @@ -216,6 +213,7 @@ pub(crate) fn diagnostics( #[rustfmt::skip] let d = match diag { AnyDiagnostic::BreakOutsideOfLoop(d) => break_outside_of_loop::break_outside_of_loop(&ctx, &d), + AnyDiagnostic::IncorrectCase(d) => incorrect_case::incorrect_case(&ctx, &d), AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&ctx, &d), AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), @@ -250,16 +248,6 @@ pub(crate) fn diagnostics( res } -fn warning_with_fix( - d: &D, - sema: &Semantics, - resolve: &AssistResolveStrategy, -) -> Diagnostic { - Diagnostic::hint(sema.diagnostics_display_range(d.display_source()).range, d.message()) - .with_fixes(d.fixes(sema, resolve)) - .with_code(Some(d.code())) -} - fn check_unnecessary_braces_in_use_statement( acc: &mut Vec, file_id: FileId, diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index e4bd90c3f..d763dca93 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -1,6 +1,5 @@ //! Provides a way to attach fixes to the diagnostics. //! The same module also has all curret custom fixes for the diagnostics implemented. -mod change_case; use hir::{diagnostics::Diagnostic, Semantics}; use ide_assists::AssistResolveStrategy; diff --git a/crates/ide/src/diagnostics/fixes/change_case.rs b/crates/ide/src/diagnostics/fixes/change_case.rs deleted file mode 100644 index db1a37cd6..000000000 --- a/crates/ide/src/diagnostics/fixes/change_case.rs +++ /dev/null @@ -1,155 +0,0 @@ -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, DiagnosticWithFixes}, - references::rename::rename_with_semantics, -}; - -impl DiagnosticWithFixes for IncorrectCase { - fn fixes( - &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(vec![res]) - } -} - -#[cfg(test)] -mod change_case { - use crate::{ - diagnostics::tests::{check_diagnostics, check_fix}, - 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_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/incorrect_case.rs b/crates/ide/src/diagnostics/incorrect_case.rs new file mode 100644 index 000000000..56283b58b --- /dev/null +++ b/crates/ide/src/diagnostics/incorrect_case.rs @@ -0,0 +1,166 @@ +use hir::{db::AstDatabase, InFile}; +use ide_assists::Assist; +use ide_db::base_db::FilePosition; +use syntax::AstNode; + +use crate::{ + diagnostics::{unresolved_fix, Diagnostic, DiagnosticsContext}, + references::rename::rename_with_semantics, + Severity, +}; + +// Diagnostic: incorrect-ident-case +// +// This diagnostic is triggered if an item name doesn't follow https://doc.rust-lang.org/1.0.0/style/style/naming/README.html[Rust naming convention]. +pub(super) fn incorrect_case(ctx: &DiagnosticsContext<'_>, d: &hir::IncorrectCase) -> Diagnostic { + Diagnostic::new( + "incorrect-ident-case", + format!( + "{} `{}` should have {} name, e.g. `{}`", + d.ident_type, d.ident_text, d.expected_case, d.suggested_text + ), + ctx.sema.diagnostics_display_range(InFile::new(d.file, d.ident.clone().into())).range, + ) + .severity(Severity::WeakWarning) + .with_fixes(fixes(ctx, d)) +} + +fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::IncorrectCase) -> Option> { + let root = ctx.sema.db.parse_or_expand(d.file)?; + let name_node = d.ident.to_node(&root); + + let name_node = InFile::new(d.file, name_node.syntax()); + let frange = name_node.original_file_range(ctx.sema.db); + let file_position = FilePosition { file_id: frange.file_id, offset: frange.range.start() }; + + let label = format!("Rename to {}", d.suggested_text); + let mut res = unresolved_fix("change_case", &label, frange.range); + if ctx.resolve.should_resolve(&res.id) { + let source_change = rename_with_semantics(&ctx.sema, file_position, &d.suggested_text); + res.source_change = Some(source_change.ok().unwrap_or_default()); + } + + Some(vec![res]) +} + +#[cfg(test)] +mod change_case { + use crate::{ + diagnostics::tests::{check_diagnostics, check_fix}, + 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_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); + } +} -- cgit v1.2.3