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 From 3478897f86cc1b3e3f83e9d4e7cedff41721fb04 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 21:33:54 +0300 Subject: internal: remove DiagnosticWithFix infra --- crates/ide/src/diagnostics.rs | 514 ++------------------------- crates/ide/src/diagnostics/fixes.rs | 24 -- crates/ide/src/diagnostics/incorrect_case.rs | 322 +++++++++++++++++ crates/ide/src/diagnostics/unlinked_file.rs | 259 ++++++++++---- 4 files changed, 534 insertions(+), 585 deletions(-) delete mode 100644 crates/ide/src/diagnostics/fixes.rs diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index f084f7b06..7978c1fc2 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -16,20 +16,19 @@ mod no_such_field; mod remove_this_semicolon; mod replace_filter_map_next_with_find_map; mod unimplemented_builtin_macro; +mod unlinked_file; mod unresolved_extern_crate; mod unresolved_import; mod unresolved_macro_call; mod unresolved_module; mod unresolved_proc_macro; -mod fixes; mod field_shorthand; -mod unlinked_file; use std::cell::RefCell; use hir::{ - diagnostics::{AnyDiagnostic, Diagnostic as _, DiagnosticCode, DiagnosticSinkBuilder}, + diagnostics::{AnyDiagnostic, DiagnosticCode, DiagnosticSinkBuilder}, Semantics, }; use ide_assists::AssistResolveStrategy; @@ -38,15 +37,13 @@ use itertools::Itertools; use rustc_hash::FxHashSet; use syntax::{ ast::{self, AstNode}, - SyntaxNode, SyntaxNodePtr, TextRange, TextSize, + SyntaxNode, TextRange, }; use text_edit::TextEdit; use unlinked_file::UnlinkedFile; use crate::{Assist, AssistId, AssistKind, FileId, Label, SourceChange}; -use self::fixes::DiagnosticWithFixes; - #[derive(Debug)] pub struct Diagnostic { // pub name: Option, @@ -165,19 +162,6 @@ pub(crate) fn diagnostics( } let res = RefCell::new(res); let sink_builder = DiagnosticSinkBuilder::new() - .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. - let range = sema.diagnostics_display_range(d.display_source()).range; - let range = range.intersect(TextRange::up_to(TextSize::of("..."))).unwrap_or(range); - - // Override severity and mark as unused. - res.borrow_mut().push( - Diagnostic::hint(range, d.message()) - .with_fixes(d.fixes(&sema, resolve)) - .with_code(Some(d.code())), - ); - }) // Only collect experimental diagnostics when they're enabled. .filter(|diag| !(diag.is_experimental() && config.disable_experimental)) .filter(|diag| !config.disabled.contains(diag.code().as_str())); @@ -197,11 +181,9 @@ pub(crate) fn diagnostics( let mut diags = Vec::new(); let internal_diagnostics = cfg!(test); - match sema.to_module_def(file_id) { - Some(m) => diags = m.diagnostics(db, &mut sink, internal_diagnostics), - None => { - sink.push(UnlinkedFile { file_id, node: SyntaxNodePtr::new(parse.tree().syntax()) }); - } + let module = sema.to_module_def(file_id); + if let Some(m) = module { + diags = m.diagnostics(db, &mut sink, internal_diagnostics) } drop(sink); @@ -209,6 +191,12 @@ pub(crate) fn diagnostics( let mut res = res.into_inner(); let ctx = DiagnosticsContext { config, sema, resolve }; + if module.is_none() { + let d = UnlinkedFile { file: file_id }; + let d = unlinked_file::unlinked_file(&ctx, &d); + res.push(d) + } + for diag in diags { #[rustfmt::skip] let d = match diag { @@ -234,16 +222,20 @@ pub(crate) fn diagnostics( None => continue, } }; + res.push(d) + } + + res.retain(|d| { if let Some(code) = d.code { if ctx.config.disabled.contains(code.as_str()) { - continue; + return false; } } if ctx.config.disable_experimental && d.experimental { - continue; + return false; } - res.push(d) - } + true + }); res } @@ -378,8 +370,9 @@ mod tests { file_position.offset ); } + /// Checks that there's a diagnostic *without* fix at `$0`. - fn check_no_fix(ra_fixture: &str) { + pub(crate) fn check_no_fix(ra_fixture: &str) { let (analysis, file_position) = fixture::position(ra_fixture); let diagnostic = analysis .diagnostics( @@ -523,142 +516,6 @@ mod a { assert!(!diagnostics.is_empty()); } - #[test] - fn unlinked_file_prepend_first_item() { - cov_mark::check!(unlinked_file_prepend_before_first_item); - // Only tests the first one for `pub mod` since the rest are the same - check_fixes( - r#" -//- /main.rs -fn f() {} -//- /foo.rs -$0 -"#, - vec![ - r#" -mod foo; - -fn f() {} -"#, - r#" -pub mod foo; - -fn f() {} -"#, - ], - ); - } - - #[test] - fn unlinked_file_append_mod() { - cov_mark::check!(unlinked_file_append_to_existing_mods); - check_fix( - r#" -//- /main.rs -//! Comment on top - -mod preexisting; - -mod preexisting2; - -struct S; - -mod preexisting_bottom;) -//- /foo.rs -$0 -"#, - r#" -//! Comment on top - -mod preexisting; - -mod preexisting2; -mod foo; - -struct S; - -mod preexisting_bottom;) -"#, - ); - } - - #[test] - fn unlinked_file_insert_in_empty_file() { - cov_mark::check!(unlinked_file_empty_file); - check_fix( - r#" -//- /main.rs -//- /foo.rs -$0 -"#, - r#" -mod foo; -"#, - ); - } - - #[test] - fn unlinked_file_old_style_modrs() { - check_fix( - r#" -//- /main.rs -mod submod; -//- /submod/mod.rs -// in mod.rs -//- /submod/foo.rs -$0 -"#, - r#" -// in mod.rs -mod foo; -"#, - ); - } - - #[test] - fn unlinked_file_new_style_mod() { - check_fix( - r#" -//- /main.rs -mod submod; -//- /submod.rs -//- /submod/foo.rs -$0 -"#, - r#" -mod foo; -"#, - ); - } - - #[test] - fn unlinked_file_with_cfg_off() { - cov_mark::check!(unlinked_file_skip_fix_when_mod_already_exists); - check_no_fix( - r#" -//- /main.rs -#[cfg(never)] -mod foo; - -//- /foo.rs -$0 -"#, - ); - } - - #[test] - fn unlinked_file_with_cfg_on() { - check_diagnostics( - r#" -//- /main.rs -#[cfg(not(never))] -mod foo; - -//- /foo.rs -"#, - ); - } - #[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. @@ -1595,330 +1452,3 @@ fn main() { } } } - -#[cfg(test)] -mod decl_check_tests { - use crate::diagnostics::tests::check_diagnostics; - - #[test] - fn incorrect_function_name() { - check_diagnostics( - r#" -fn NonSnakeCaseName() {} -// ^^^^^^^^^^^^^^^^ Function `NonSnakeCaseName` should have snake_case name, e.g. `non_snake_case_name` -"#, - ); - } - - #[test] - fn incorrect_function_params() { - check_diagnostics( - r#" -fn foo(SomeParam: u8) {} - // ^^^^^^^^^ Parameter `SomeParam` should have snake_case name, e.g. `some_param` - -fn foo2(ok_param: &str, CAPS_PARAM: u8) {} - // ^^^^^^^^^^ Parameter `CAPS_PARAM` should have snake_case name, e.g. `caps_param` -"#, - ); - } - - #[test] - fn incorrect_variable_names() { - check_diagnostics( - r#" -fn foo() { - let SOME_VALUE = 10; - // ^^^^^^^^^^ Variable `SOME_VALUE` should have snake_case name, e.g. `some_value` - let AnotherValue = 20; - // ^^^^^^^^^^^^ Variable `AnotherValue` should have snake_case name, e.g. `another_value` -} -"#, - ); - } - - #[test] - fn incorrect_struct_names() { - check_diagnostics( - r#" -struct non_camel_case_name {} - // ^^^^^^^^^^^^^^^^^^^ Structure `non_camel_case_name` should have CamelCase name, e.g. `NonCamelCaseName` - -struct SCREAMING_CASE {} - // ^^^^^^^^^^^^^^ Structure `SCREAMING_CASE` should have CamelCase name, e.g. `ScreamingCase` -"#, - ); - } - - #[test] - fn no_diagnostic_for_camel_cased_acronyms_in_struct_name() { - check_diagnostics( - r#" -struct AABB {} -"#, - ); - } - - #[test] - fn incorrect_struct_field() { - check_diagnostics( - r#" -struct SomeStruct { SomeField: u8 } - // ^^^^^^^^^ Field `SomeField` should have snake_case name, e.g. `some_field` -"#, - ); - } - - #[test] - fn incorrect_enum_names() { - check_diagnostics( - r#" -enum some_enum { Val(u8) } - // ^^^^^^^^^ Enum `some_enum` should have CamelCase name, e.g. `SomeEnum` - -enum SOME_ENUM {} - // ^^^^^^^^^ Enum `SOME_ENUM` should have CamelCase name, e.g. `SomeEnum` -"#, - ); - } - - #[test] - fn no_diagnostic_for_camel_cased_acronyms_in_enum_name() { - check_diagnostics( - r#" -enum AABB {} -"#, - ); - } - - #[test] - fn incorrect_enum_variant_name() { - check_diagnostics( - r#" -enum SomeEnum { SOME_VARIANT(u8) } - // ^^^^^^^^^^^^ Variant `SOME_VARIANT` should have CamelCase name, e.g. `SomeVariant` -"#, - ); - } - - #[test] - fn incorrect_const_name() { - check_diagnostics( - r#" -const some_weird_const: u8 = 10; - // ^^^^^^^^^^^^^^^^ Constant `some_weird_const` should have UPPER_SNAKE_CASE name, e.g. `SOME_WEIRD_CONST` -"#, - ); - } - - #[test] - fn incorrect_static_name() { - check_diagnostics( - r#" -static some_weird_const: u8 = 10; - // ^^^^^^^^^^^^^^^^ Static variable `some_weird_const` should have UPPER_SNAKE_CASE name, e.g. `SOME_WEIRD_CONST` -"#, - ); - } - - #[test] - fn fn_inside_impl_struct() { - check_diagnostics( - r#" -struct someStruct; - // ^^^^^^^^^^ Structure `someStruct` should have CamelCase name, e.g. `SomeStruct` - -impl someStruct { - fn SomeFunc(&self) { - // ^^^^^^^^ Function `SomeFunc` should have snake_case name, e.g. `some_func` - let WHY_VAR_IS_CAPS = 10; - // ^^^^^^^^^^^^^^^ Variable `WHY_VAR_IS_CAPS` should have snake_case name, e.g. `why_var_is_caps` - } -} -"#, - ); - } - - #[test] - fn no_diagnostic_for_enum_varinats() { - check_diagnostics( - r#" -enum Option { Some, None } - -fn main() { - match Option::None { - None => (), - Some => (), - } -} -"#, - ); - } - - #[test] - fn non_let_bind() { - check_diagnostics( - r#" -enum Option { Some, None } - -fn main() { - match Option::None { - SOME_VAR @ None => (), - // ^^^^^^^^ Variable `SOME_VAR` should have snake_case name, e.g. `some_var` - Some => (), - } -} -"#, - ); - } - - #[test] - fn allow_attributes() { - check_diagnostics( - r#" -#[allow(non_snake_case)] -fn NonSnakeCaseName(SOME_VAR: u8) -> u8{ - // cov_flags generated output from elsewhere in this file - extern "C" { - #[no_mangle] - static lower_case: u8; - } - - let OtherVar = SOME_VAR + 1; - OtherVar -} - -#[allow(nonstandard_style)] -mod CheckNonstandardStyle { - fn HiImABadFnName() {} -} - -#[allow(bad_style)] -mod CheckBadStyle { - fn HiImABadFnName() {} -} - -mod F { - #![allow(non_snake_case)] - fn CheckItWorksWithModAttr(BAD_NAME_HI: u8) {} -} - -#[allow(non_snake_case, non_camel_case_types)] -pub struct some_type { - SOME_FIELD: u8, - SomeField: u16, -} - -#[allow(non_upper_case_globals)] -pub const some_const: u8 = 10; - -#[allow(non_upper_case_globals)] -pub static SomeStatic: u8 = 10; - "#, - ); - } - - #[test] - fn allow_attributes_crate_attr() { - check_diagnostics( - r#" -#![allow(non_snake_case)] - -mod F { - fn CheckItWorksWithCrateAttr(BAD_NAME_HI: u8) {} -} - "#, - ); - } - - #[test] - #[ignore] - fn bug_trait_inside_fn() { - // FIXME: - // This is broken, and in fact, should not even be looked at by this - // lint in the first place. There's weird stuff going on in the - // collection phase. - // It's currently being brought in by: - // * validate_func on `a` recursing into modules - // * then it finds the trait and then the function while iterating - // through modules - // * then validate_func is called on Dirty - // * ... which then proceeds to look at some unknown module taking no - // attrs from either the impl or the fn a, and then finally to the root - // module - // - // It should find the attribute on the trait, but it *doesn't even see - // the trait* as far as I can tell. - - check_diagnostics( - r#" -trait T { fn a(); } -struct U {} -impl T for U { - fn a() { - // this comes out of bitflags, mostly - #[allow(non_snake_case)] - trait __BitFlags { - const HiImAlsoBad: u8 = 2; - #[inline] - fn Dirty(&self) -> bool { - false - } - } - - } -} - "#, - ); - } - - #[test] - #[ignore] - fn bug_traits_arent_checked() { - // FIXME: Traits and functions in traits aren't currently checked by - // r-a, even though rustc will complain about them. - check_diagnostics( - r#" -trait BAD_TRAIT { - // ^^^^^^^^^ Trait `BAD_TRAIT` should have CamelCase name, e.g. `BadTrait` - fn BAD_FUNCTION(); - // ^^^^^^^^^^^^ Function `BAD_FUNCTION` should have snake_case name, e.g. `bad_function` - fn BadFunction(); - // ^^^^^^^^^^^^ Function `BadFunction` should have snake_case name, e.g. `bad_function` -} - "#, - ); - } - - #[test] - fn ignores_extern_items() { - cov_mark::check!(extern_func_incorrect_case_ignored); - cov_mark::check!(extern_static_incorrect_case_ignored); - check_diagnostics( - r#" -extern { - fn NonSnakeCaseName(SOME_VAR: u8) -> u8; - pub static SomeStatic: u8 = 10; -} - "#, - ); - } - - #[test] - fn infinite_loop_inner_items() { - check_diagnostics( - r#" -fn qualify() { - mod foo { - use super::*; - } -} - "#, - ) - } - - #[test] // Issue #8809. - fn parenthesized_parameter() { - check_diagnostics(r#"fn f((O): _) {}"#) - } -} diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs deleted file mode 100644 index d763dca93..000000000 --- a/crates/ide/src/diagnostics/fixes.rs +++ /dev/null @@ -1,24 +0,0 @@ -//! Provides a way to attach fixes to the diagnostics. -//! The same module also has all curret custom fixes for the diagnostics implemented. - -use hir::{diagnostics::Diagnostic, Semantics}; -use ide_assists::AssistResolveStrategy; -use ide_db::RootDatabase; - -use crate::Assist; - -/// A [Diagnostic] that potentially has some fixes available. -/// -/// [Diagnostic]: hir::diagnostics::Diagnostic -pub(crate) trait DiagnosticWithFixes: Diagnostic { - /// `resolve` determines if the diagnostic should fill in the `edit` field - /// of the assist. - /// - /// If `resolve` is false, the edit will be computed later, on demand, and - /// can be omitted. - fn fixes( - &self, - sema: &Semantics, - _resolve: &AssistResolveStrategy, - ) -> Option>; -} diff --git a/crates/ide/src/diagnostics/incorrect_case.rs b/crates/ide/src/diagnostics/incorrect_case.rs index 56283b58b..832394400 100644 --- a/crates/ide/src/diagnostics/incorrect_case.rs +++ b/crates/ide/src/diagnostics/incorrect_case.rs @@ -163,4 +163,326 @@ impl TestStruct { check_fix(input, expected); } + + #[test] + fn incorrect_function_name() { + check_diagnostics( + r#" +fn NonSnakeCaseName() {} +// ^^^^^^^^^^^^^^^^ Function `NonSnakeCaseName` should have snake_case name, e.g. `non_snake_case_name` +"#, + ); + } + + #[test] + fn incorrect_function_params() { + check_diagnostics( + r#" +fn foo(SomeParam: u8) {} + // ^^^^^^^^^ Parameter `SomeParam` should have snake_case name, e.g. `some_param` + +fn foo2(ok_param: &str, CAPS_PARAM: u8) {} + // ^^^^^^^^^^ Parameter `CAPS_PARAM` should have snake_case name, e.g. `caps_param` +"#, + ); + } + + #[test] + fn incorrect_variable_names() { + check_diagnostics( + r#" +fn foo() { + let SOME_VALUE = 10; + // ^^^^^^^^^^ Variable `SOME_VALUE` should have snake_case name, e.g. `some_value` + let AnotherValue = 20; + // ^^^^^^^^^^^^ Variable `AnotherValue` should have snake_case name, e.g. `another_value` +} +"#, + ); + } + + #[test] + fn incorrect_struct_names() { + check_diagnostics( + r#" +struct non_camel_case_name {} + // ^^^^^^^^^^^^^^^^^^^ Structure `non_camel_case_name` should have CamelCase name, e.g. `NonCamelCaseName` + +struct SCREAMING_CASE {} + // ^^^^^^^^^^^^^^ Structure `SCREAMING_CASE` should have CamelCase name, e.g. `ScreamingCase` +"#, + ); + } + + #[test] + fn no_diagnostic_for_camel_cased_acronyms_in_struct_name() { + check_diagnostics( + r#" +struct AABB {} +"#, + ); + } + + #[test] + fn incorrect_struct_field() { + check_diagnostics( + r#" +struct SomeStruct { SomeField: u8 } + // ^^^^^^^^^ Field `SomeField` should have snake_case name, e.g. `some_field` +"#, + ); + } + + #[test] + fn incorrect_enum_names() { + check_diagnostics( + r#" +enum some_enum { Val(u8) } + // ^^^^^^^^^ Enum `some_enum` should have CamelCase name, e.g. `SomeEnum` + +enum SOME_ENUM {} + // ^^^^^^^^^ Enum `SOME_ENUM` should have CamelCase name, e.g. `SomeEnum` +"#, + ); + } + + #[test] + fn no_diagnostic_for_camel_cased_acronyms_in_enum_name() { + check_diagnostics( + r#" +enum AABB {} +"#, + ); + } + + #[test] + fn incorrect_enum_variant_name() { + check_diagnostics( + r#" +enum SomeEnum { SOME_VARIANT(u8) } + // ^^^^^^^^^^^^ Variant `SOME_VARIANT` should have CamelCase name, e.g. `SomeVariant` +"#, + ); + } + + #[test] + fn incorrect_const_name() { + check_diagnostics( + r#" +const some_weird_const: u8 = 10; + // ^^^^^^^^^^^^^^^^ Constant `some_weird_const` should have UPPER_SNAKE_CASE name, e.g. `SOME_WEIRD_CONST` +"#, + ); + } + + #[test] + fn incorrect_static_name() { + check_diagnostics( + r#" +static some_weird_const: u8 = 10; + // ^^^^^^^^^^^^^^^^ Static variable `some_weird_const` should have UPPER_SNAKE_CASE name, e.g. `SOME_WEIRD_CONST` +"#, + ); + } + + #[test] + fn fn_inside_impl_struct() { + check_diagnostics( + r#" +struct someStruct; + // ^^^^^^^^^^ Structure `someStruct` should have CamelCase name, e.g. `SomeStruct` + +impl someStruct { + fn SomeFunc(&self) { + // ^^^^^^^^ Function `SomeFunc` should have snake_case name, e.g. `some_func` + let WHY_VAR_IS_CAPS = 10; + // ^^^^^^^^^^^^^^^ Variable `WHY_VAR_IS_CAPS` should have snake_case name, e.g. `why_var_is_caps` + } +} +"#, + ); + } + + #[test] + fn no_diagnostic_for_enum_varinats() { + check_diagnostics( + r#" +enum Option { Some, None } + +fn main() { + match Option::None { + None => (), + Some => (), + } +} +"#, + ); + } + + #[test] + fn non_let_bind() { + check_diagnostics( + r#" +enum Option { Some, None } + +fn main() { + match Option::None { + SOME_VAR @ None => (), + // ^^^^^^^^ Variable `SOME_VAR` should have snake_case name, e.g. `some_var` + Some => (), + } +} +"#, + ); + } + + #[test] + fn allow_attributes_crate_attr() { + check_diagnostics( + r#" +#![allow(non_snake_case)] + +mod F { + fn CheckItWorksWithCrateAttr(BAD_NAME_HI: u8) {} +} + "#, + ); + } + + #[test] + #[ignore] + fn bug_trait_inside_fn() { + // FIXME: + // This is broken, and in fact, should not even be looked at by this + // lint in the first place. There's weird stuff going on in the + // collection phase. + // It's currently being brought in by: + // * validate_func on `a` recursing into modules + // * then it finds the trait and then the function while iterating + // through modules + // * then validate_func is called on Dirty + // * ... which then proceeds to look at some unknown module taking no + // attrs from either the impl or the fn a, and then finally to the root + // module + // + // It should find the attribute on the trait, but it *doesn't even see + // the trait* as far as I can tell. + + check_diagnostics( + r#" +trait T { fn a(); } +struct U {} +impl T for U { + fn a() { + // this comes out of bitflags, mostly + #[allow(non_snake_case)] + trait __BitFlags { + const HiImAlsoBad: u8 = 2; + #[inline] + fn Dirty(&self) -> bool { + false + } + } + + } +} + "#, + ); + } + + #[test] + fn infinite_loop_inner_items() { + check_diagnostics( + r#" +fn qualify() { + mod foo { + use super::*; + } +} + "#, + ) + } + + #[test] // Issue #8809. + fn parenthesized_parameter() { + check_diagnostics(r#"fn f((O): _) {}"#) + } + + #[test] + fn ignores_extern_items() { + cov_mark::check!(extern_func_incorrect_case_ignored); + cov_mark::check!(extern_static_incorrect_case_ignored); + check_diagnostics( + r#" +extern { + fn NonSnakeCaseName(SOME_VAR: u8) -> u8; + pub static SomeStatic: u8 = 10; +} + "#, + ); + } + + #[test] + #[ignore] + fn bug_traits_arent_checked() { + // FIXME: Traits and functions in traits aren't currently checked by + // r-a, even though rustc will complain about them. + check_diagnostics( + r#" +trait BAD_TRAIT { + // ^^^^^^^^^ Trait `BAD_TRAIT` should have CamelCase name, e.g. `BadTrait` + fn BAD_FUNCTION(); + // ^^^^^^^^^^^^ Function `BAD_FUNCTION` should have snake_case name, e.g. `bad_function` + fn BadFunction(); + // ^^^^^^^^^^^^ Function `BadFunction` should have snake_case name, e.g. `bad_function` +} + "#, + ); + } + + #[test] + fn allow_attributes() { + check_diagnostics( + r#" +#[allow(non_snake_case)] +fn NonSnakeCaseName(SOME_VAR: u8) -> u8{ + // cov_flags generated output from elsewhere in this file + extern "C" { + #[no_mangle] + static lower_case: u8; + } + + let OtherVar = SOME_VAR + 1; + OtherVar +} + +#[allow(nonstandard_style)] +mod CheckNonstandardStyle { + fn HiImABadFnName() {} +} + +#[allow(bad_style)] +mod CheckBadStyle { + fn HiImABadFnName() {} +} + +mod F { + #![allow(non_snake_case)] + fn CheckItWorksWithModAttr(BAD_NAME_HI: u8) {} +} + +#[allow(non_snake_case, non_camel_case_types)] +pub struct some_type { + SOME_FIELD: u8, + SomeField: u16, +} + +#[allow(non_upper_case_globals)] +pub const some_const: u8 = 10; + +#[allow(non_upper_case_globals)] +pub static SomeStatic: u8 = 10; + "#, + ); + } } diff --git a/crates/ide/src/diagnostics/unlinked_file.rs b/crates/ide/src/diagnostics/unlinked_file.rs index 51fe0f360..a5b2e3399 100644 --- a/crates/ide/src/diagnostics/unlinked_file.rs +++ b/crates/ide/src/diagnostics/unlinked_file.rs @@ -1,11 +1,6 @@ //! Diagnostic emitted for files that aren't part of any crate. -use hir::{ - db::DefDatabase, - diagnostics::{Diagnostic, DiagnosticCode}, - InFile, -}; -use ide_assists::AssistResolveStrategy; +use hir::db::DefDatabase; use ide_db::{ base_db::{FileId, FileLoader, SourceDatabase, SourceDatabaseExt}, source_change::SourceChange, @@ -13,92 +8,77 @@ use ide_db::{ }; use syntax::{ ast::{self, ModuleItemOwner, NameOwner}, - AstNode, SyntaxNodePtr, + AstNode, TextRange, TextSize, }; use text_edit::TextEdit; use crate::{ - diagnostics::{fix, fixes::DiagnosticWithFixes}, - Assist, + diagnostics::{fix, DiagnosticsContext}, + Assist, Diagnostic, }; +#[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. -#[derive(Debug)] -pub(crate) struct UnlinkedFile { - pub(crate) file_id: FileId, - pub(crate) node: SyntaxNodePtr, +pub(super) fn unlinked_file(ctx: &DiagnosticsContext, d: &UnlinkedFile) -> Diagnostic { + // 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(); + // 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)) } -impl Diagnostic for UnlinkedFile { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("unlinked-file") - } +fn fixes(ctx: &DiagnosticsContext, d: &UnlinkedFile) -> 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. - fn message(&self) -> String { - "file not included in module tree".to_string() - } + 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 module_name = our_path.name_and_extension()?.0; - fn display_source(&self) -> InFile { - InFile::new(self.file_id.into(), self.node.clone()) - } + // Candidates to look for: + // - `mod.rs` in the same folder + // - we also check `main.rs` and `lib.rs` + // - `$dir.rs` in the parent folder, where `$dir` is the directory containing `self.file_id` + let parent = our_path.parent()?; + let mut paths = vec![parent.join("mod.rs")?, parent.join("lib.rs")?, parent.join("main.rs")?]; - fn as_any(&self) -> &(dyn std::any::Any + Send + 'static) { - self + // `submod/bla.rs` -> `submod.rs` + if let Some(newmod) = (|| { + let name = parent.name_and_extension()?.0; + parent.parent()?.join(&format!("{}.rs", name)) + })() { + paths.push(newmod); } -} -impl DiagnosticWithFixes for UnlinkedFile { - fn fixes( - &self, - sema: &hir::Semantics, - _resolve: &AssistResolveStrategy, - ) -> 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 = sema.db.source_root(sema.db.file_source_root(self.file_id)); - let our_path = source_root.path_for_file(&self.file_id)?; - let module_name = our_path.name_and_extension()?.0; - - // Candidates to look for: - // - `mod.rs` in the same folder - // - we also check `main.rs` and `lib.rs` - // - `$dir.rs` in the parent folder, where `$dir` is the directory containing `self.file_id` - let parent = our_path.parent()?; - let mut paths = - vec![parent.join("mod.rs")?, parent.join("lib.rs")?, parent.join("main.rs")?]; - - // `submod/bla.rs` -> `submod.rs` - if let Some(newmod) = (|| { - let name = parent.name_and_extension()?.0; - parent.parent()?.join(&format!("{}.rs", name)) - })() { - paths.push(newmod); - } + for path in &paths { + if let Some(parent_id) = source_root.file_for_path(path) { + for krate in ctx.sema.db.relevant_crates(*parent_id).iter() { + let crate_def_map = ctx.sema.db.crate_def_map(*krate); + for (_, module) in crate_def_map.modules() { + if module.origin.is_inline() { + // We don't handle inline `mod parent {}`s, they use different paths. + continue; + } - for path in &paths { - if let Some(parent_id) = source_root.file_for_path(path) { - for krate in sema.db.relevant_crates(*parent_id).iter() { - let crate_def_map = sema.db.crate_def_map(*krate); - for (_, module) in crate_def_map.modules() { - if module.origin.is_inline() { - // We don't handle inline `mod parent {}`s, they use different paths. - continue; - } - - if module.origin.file_id() == Some(*parent_id) { - return make_fixes(sema.db, *parent_id, module_name, self.file_id); - } + if module.origin.file_id() == Some(*parent_id) { + return make_fixes(ctx.sema.db, *parent_id, module_name, d.file); } } } } - - None } + + None } fn make_fixes( @@ -181,3 +161,144 @@ fn make_fixes( ), ]) } + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::{check_diagnostics, check_fix, check_fixes, check_no_fix}; + + #[test] + fn unlinked_file_prepend_first_item() { + cov_mark::check!(unlinked_file_prepend_before_first_item); + // Only tests the first one for `pub mod` since the rest are the same + check_fixes( + r#" +//- /main.rs +fn f() {} +//- /foo.rs +$0 +"#, + vec![ + r#" +mod foo; + +fn f() {} +"#, + r#" +pub mod foo; + +fn f() {} +"#, + ], + ); + } + + #[test] + fn unlinked_file_append_mod() { + cov_mark::check!(unlinked_file_append_to_existing_mods); + check_fix( + r#" +//- /main.rs +//! Comment on top + +mod preexisting; + +mod preexisting2; + +struct S; + +mod preexisting_bottom;) +//- /foo.rs +$0 +"#, + r#" +//! Comment on top + +mod preexisting; + +mod preexisting2; +mod foo; + +struct S; + +mod preexisting_bottom;) +"#, + ); + } + + #[test] + fn unlinked_file_insert_in_empty_file() { + cov_mark::check!(unlinked_file_empty_file); + check_fix( + r#" +//- /main.rs +//- /foo.rs +$0 +"#, + r#" +mod foo; +"#, + ); + } + + #[test] + fn unlinked_file_old_style_modrs() { + check_fix( + r#" +//- /main.rs +mod submod; +//- /submod/mod.rs +// in mod.rs +//- /submod/foo.rs +$0 +"#, + r#" +// in mod.rs +mod foo; +"#, + ); + } + + #[test] + fn unlinked_file_new_style_mod() { + check_fix( + r#" +//- /main.rs +mod submod; +//- /submod.rs +//- /submod/foo.rs +$0 +"#, + r#" +mod foo; +"#, + ); + } + + #[test] + fn unlinked_file_with_cfg_off() { + cov_mark::check!(unlinked_file_skip_fix_when_mod_already_exists); + check_no_fix( + r#" +//- /main.rs +#[cfg(never)] +mod foo; + +//- /foo.rs +$0 +"#, + ); + } + + #[test] + fn unlinked_file_with_cfg_on() { + check_diagnostics( + r#" +//- /main.rs +#[cfg(not(never))] +mod foo; + +//- /foo.rs +"#, + ); + } +} -- cgit v1.2.3