From f5cea35986a0c8182ca427f10e20bc97ec564315 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sat, 3 Oct 2020 13:39:10 +0300 Subject: Add checks for function parameters --- crates/hir_ty/src/diagnostics.rs | 4 +- crates/hir_ty/src/diagnostics/decl_check.rs | 97 +++++++++++++++++++++++++++-- 2 files changed, 94 insertions(+), 7 deletions(-) diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs index 7227e7010..24fff690a 100644 --- a/crates/hir_ty/src/diagnostics.rs +++ b/crates/hir_ty/src/diagnostics.rs @@ -269,6 +269,7 @@ pub struct IncorrectCase { pub file: HirFileId, pub ident: SyntaxNodePtr, pub expected_case: CaseType, + pub ident_type: String, pub ident_text: String, pub suggested_text: String, } @@ -280,7 +281,8 @@ impl Diagnostic for IncorrectCase { fn message(&self) -> String { format!( - "Argument `{}` should have a {} name, e.g. `{}`", + "{} `{}` should have a {} name, e.g. `{}`", + self.ident_type, self.ident_text, self.expected_case.to_string(), self.suggested_text diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index 6c3cd65c5..083df3772 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -21,7 +21,10 @@ use hir_def::{ AdtId, FunctionId, Lookup, ModuleDefId, }; use hir_expand::{diagnostics::DiagnosticSink, name::Name}; -use syntax::{ast::NameOwner, AstPtr}; +use syntax::{ + ast::{self, NameOwner}, + AstPtr, +}; use crate::{ db::HirDatabase, @@ -122,7 +125,8 @@ impl<'a, 'b> DeclValidator<'a, 'b> { } else { // We don't want rust-analyzer to panic over this, but it is definitely some kind of error in the logic. log::error!( - "Replacement was generated for a function without a name: {:?}", + "Replacement ({:?}) was generated for a function without a name: {:?}", + replacement, fn_src ); return; @@ -130,6 +134,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> { let diagnostic = IncorrectCase { file: fn_src.file_id, + ident_type: "Function".to_string(), ident: AstPtr::new(&ast_ptr).into(), expected_case: replacement.expected_case, ident_text: replacement.current_name.to_string(), @@ -139,15 +144,71 @@ impl<'a, 'b> DeclValidator<'a, 'b> { self.sink.push(diagnostic); } - // let item_tree = db.item_tree(loc.id.file_id); - // let fn_def = &item_tree[fn_loc.id.value]; - // let (_, source_map) = db.body_with_source_map(func.into()); + let fn_params_list = match fn_src.value.param_list() { + Some(params) => params, + None => { + if !fn_param_replacements.is_empty() { + log::error!( + "Replacements ({:?}) were generated for a function parameters which had no parameters list: {:?}", + fn_param_replacements, fn_src + ); + } + return; + } + }; + let mut fn_params_iter = fn_params_list.params(); + for param_to_rename in fn_param_replacements { + // We assume that parameters in replacement are in the same order as in the + // actual params list, but just some of them (ones that named correctly) are skipped. + let ast_ptr = loop { + match fn_params_iter.next() { + Some(element) + if pat_equals_to_name(element.pat(), ¶m_to_rename.current_name) => + { + break element.pat().unwrap() + } + Some(_) => {} + None => { + log::error!( + "Replacement ({:?}) was generated for a function parameter which was not found: {:?}", + param_to_rename, fn_src + ); + return; + } + } + }; + + let diagnostic = IncorrectCase { + file: fn_src.file_id, + ident_type: "Argument".to_string(), + ident: AstPtr::new(&ast_ptr).into(), + expected_case: param_to_rename.expected_case, + ident_text: param_to_rename.current_name.to_string(), + suggested_text: param_to_rename.suggested_text, + }; + + self.sink.push(diagnostic); + } } fn validate_adt(&mut self, db: &dyn HirDatabase, adt: AdtId) {} } +fn pat_equals_to_name(pat: Option, name: &Name) -> bool { + if let Some(ast::Pat::IdentPat(ident)) = pat { + ident.to_string() == name.to_string() + } else { + false + } +} + fn to_lower_snake_case(ident: &str) -> Option { + // First, assume that it's UPPER_SNAKE_CASE. + if let Some(normalized) = to_lower_snake_case_from_upper_snake_case(ident) { + return Some(normalized); + } + + // Otherwise, assume that it's CamelCase. let lower_snake_case = stdx::to_lower_snake_case(ident); if lower_snake_case == ident { @@ -157,6 +218,17 @@ fn to_lower_snake_case(ident: &str) -> Option { } } +fn to_lower_snake_case_from_upper_snake_case(ident: &str) -> Option { + let is_upper_snake_case = ident.chars().all(|c| c.is_ascii_uppercase() || c == '_'); + + if is_upper_snake_case { + let string = ident.chars().map(|c| c.to_ascii_lowercase()).collect(); + Some(string) + } else { + None + } +} + #[cfg(test)] mod tests { use crate::diagnostics::tests::check_diagnostics; @@ -166,7 +238,20 @@ mod tests { check_diagnostics( r#" fn NonSnakeCaseName() {} -// ^^^^^^^^^^^^^^^^ Argument `NonSnakeCaseName` should have a snake_case name, e.g. `non_snake_case_name` +// ^^^^^^^^^^^^^^^^ Function `NonSnakeCaseName` should have a snake_case name, e.g. `non_snake_case_name` +"#, + ); + } + + #[test] + fn incorrect_function_params() { + check_diagnostics( + r#" +fn foo(SomeParam: u8) {} + // ^^^^^^^^^ Argument `SomeParam` should have a snake_case name, e.g. `some_param` + +fn foo2(ok_param: &str, CAPS_PARAM: u8) {} + // ^^^^^^^^^^ Argument `CAPS_PARAM` should have a snake_case name, e.g. `caps_param` "#, ); } -- cgit v1.2.3