From 4039176ec63e5c75d76398f2debe26ac6fa59cbc Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sat, 3 Oct 2020 12:48:02 +0300 Subject: Create basic support for names case checks and implement function name case check --- crates/hir_ty/src/diagnostics/decl_check.rs | 173 ++++++++++++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 crates/hir_ty/src/diagnostics/decl_check.rs (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs new file mode 100644 index 000000000..6c3cd65c5 --- /dev/null +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -0,0 +1,173 @@ +//! Provides validators for the item declarations. +//! This includes the following items: +//! - variable bindings (e.g. `let x = foo();`) +//! - struct fields (e.g. `struct Foo { field: u8 }`) +//! - enum fields (e.g. `enum Foo { Variant { field: u8 } }`) +//! - function/method arguments (e.g. `fn foo(arg: u8)`) + +// TODO: Temporary, to not see warnings until module is somewhat complete. +// If you see these lines in the pull request, feel free to call me stupid :P. +#![allow(dead_code, unused_imports, unused_variables)] + +use std::sync::Arc; + +use hir_def::{ + body::Body, + db::DefDatabase, + expr::{Expr, ExprId, UnaryOp}, + item_tree::ItemTreeNode, + resolver::{resolver_for_expr, ResolveValueResult, ValueNs}, + src::HasSource, + AdtId, FunctionId, Lookup, ModuleDefId, +}; +use hir_expand::{diagnostics::DiagnosticSink, name::Name}; +use syntax::{ast::NameOwner, AstPtr}; + +use crate::{ + db::HirDatabase, + diagnostics::{CaseType, IncorrectCase}, + lower::CallableDefId, + ApplicationTy, InferenceResult, Ty, TypeCtor, +}; + +pub(super) struct DeclValidator<'a, 'b: 'a> { + owner: ModuleDefId, + sink: &'a mut DiagnosticSink<'b>, +} + +#[derive(Debug)] +struct Replacement { + current_name: Name, + suggested_text: String, + expected_case: CaseType, +} + +impl<'a, 'b> DeclValidator<'a, 'b> { + pub(super) fn new( + owner: ModuleDefId, + sink: &'a mut DiagnosticSink<'b>, + ) -> DeclValidator<'a, 'b> { + DeclValidator { owner, sink } + } + + pub(super) fn validate_item(&mut self, db: &dyn HirDatabase) { + // let def = self.owner.into(); + match self.owner { + ModuleDefId::FunctionId(func) => self.validate_func(db, func), + ModuleDefId::AdtId(adt) => self.validate_adt(db, adt), + _ => return, + } + } + + fn validate_func(&mut self, db: &dyn HirDatabase, func: FunctionId) { + let data = db.function_data(func); + + // 1. Check the function name. + let function_name = data.name.to_string(); + let fn_name_replacement = if let Some(new_name) = to_lower_snake_case(&function_name) { + let replacement = Replacement { + current_name: data.name.clone(), + suggested_text: new_name, + expected_case: CaseType::LowerSnakeCase, + }; + Some(replacement) + } else { + None + }; + + // 2. Check the param names. + let mut fn_param_replacements = Vec::new(); + + for param_name in data.param_names.iter().cloned().filter_map(|i| i) { + let name = param_name.to_string(); + if let Some(new_name) = to_lower_snake_case(&name) { + let replacement = Replacement { + current_name: param_name, + suggested_text: new_name, + expected_case: CaseType::LowerSnakeCase, + }; + fn_param_replacements.push(replacement); + } + } + + // 3. If there is at least one element to spawn a warning on, go to the source map and generate a warning. + self.create_incorrect_case_diagnostic_for_func( + func, + db, + fn_name_replacement, + fn_param_replacements, + ) + } + + /// Given the information about incorrect names in the function declaration, looks up into the source code + /// for exact locations and adds diagnostics into the sink. + fn create_incorrect_case_diagnostic_for_func( + &mut self, + func: FunctionId, + db: &dyn HirDatabase, + fn_name_replacement: Option, + fn_param_replacements: Vec, + ) { + // XXX: only look at sources if we do have incorrect names + if fn_name_replacement.is_none() && fn_param_replacements.is_empty() { + return; + } + + let fn_loc = func.lookup(db.upcast()); + let fn_src = fn_loc.source(db.upcast()); + + if let Some(replacement) = fn_name_replacement { + let ast_ptr = if let Some(name) = fn_src.value.name() { + name + } 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: {:?}", + fn_src + ); + return; + }; + + let diagnostic = IncorrectCase { + file: fn_src.file_id, + ident: AstPtr::new(&ast_ptr).into(), + expected_case: replacement.expected_case, + ident_text: replacement.current_name.to_string(), + suggested_text: replacement.suggested_text, + }; + + 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()); + } + + fn validate_adt(&mut self, db: &dyn HirDatabase, adt: AdtId) {} +} + +fn to_lower_snake_case(ident: &str) -> Option { + let lower_snake_case = stdx::to_lower_snake_case(ident); + + if lower_snake_case == ident { + None + } else { + Some(lower_snake_case) + } +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::check_diagnostics; + + #[test] + fn incorrect_function_name() { + check_diagnostics( + r#" +fn NonSnakeCaseName() {} +// ^^^^^^^^^^^^^^^^ Argument `NonSnakeCaseName` should have a snake_case name, e.g. `non_snake_case_name` +"#, + ); + } +} -- cgit v1.2.3 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/decl_check.rs | 97 +++++++++++++++++++++++++++-- 1 file changed, 91 insertions(+), 6 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') 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 From 1773c6d154abe5da00b31bb16139addcaa443bbb Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sat, 3 Oct 2020 14:35:26 +0300 Subject: Extract helper functions into a separate module --- crates/hir_ty/src/diagnostics/decl_check.rs | 49 +++++------- .../src/diagnostics/decl_check/str_helpers.rs | 92 ++++++++++++++++++++++ 2 files changed, 112 insertions(+), 29 deletions(-) create mode 100644 crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index 083df3772..1a0906492 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -9,6 +9,8 @@ // If you see these lines in the pull request, feel free to call me stupid :P. #![allow(dead_code, unused_imports, unused_variables)] +mod str_helpers; + use std::sync::Arc; use hir_def::{ @@ -18,7 +20,7 @@ use hir_def::{ item_tree::ItemTreeNode, resolver::{resolver_for_expr, ResolveValueResult, ValueNs}, src::HasSource, - AdtId, FunctionId, Lookup, ModuleDefId, + AdtId, EnumId, FunctionId, Lookup, ModuleDefId, StructId, }; use hir_expand::{diagnostics::DiagnosticSink, name::Name}; use syntax::{ @@ -28,7 +30,7 @@ use syntax::{ use crate::{ db::HirDatabase, - diagnostics::{CaseType, IncorrectCase}, + diagnostics::{decl_check::str_helpers::*, CaseType, IncorrectCase}, lower::CallableDefId, ApplicationTy, InferenceResult, Ty, TypeCtor, }; @@ -191,41 +193,30 @@ impl<'a, 'b> DeclValidator<'a, 'b> { } } - 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 validate_adt(&mut self, db: &dyn HirDatabase, adt: AdtId) { + match adt { + AdtId::StructId(struct_id) => self.validate_struct(db, struct_id), + AdtId::EnumId(enum_id) => self.validate_enum(db, enum_id), + AdtId::UnionId(_) => { + // Unions aren't yet supported by this validator. + } + } } -} -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); + fn validate_struct(&mut self, db: &dyn HirDatabase, struct_id: StructId) { + let data = db.struct_data(struct_id); } - // Otherwise, assume that it's CamelCase. - let lower_snake_case = stdx::to_lower_snake_case(ident); - - if lower_snake_case == ident { - None - } else { - Some(lower_snake_case) + fn validate_enum(&mut self, db: &dyn HirDatabase, enum_id: EnumId) { + let data = db.enum_data(enum_id); } } -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) +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 { - None + false } } diff --git a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs new file mode 100644 index 000000000..3d8f1b5f2 --- /dev/null +++ b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs @@ -0,0 +1,92 @@ +pub fn to_camel_case(ident: &str) -> Option { + let mut output = String::new(); + + if is_camel_case(ident) { + return None; + } + + let mut capital_added = false; + for chr in ident.chars() { + if chr.is_alphabetic() { + if !capital_added { + output.push(chr.to_ascii_uppercase()); + capital_added = true; + } else { + output.push(chr.to_ascii_lowercase()); + } + } else if chr == '_' { + // Skip this character and make the next one capital. + capital_added = false; + } else { + // Put the characted as-is. + output.push(chr); + } + } + + if output == ident { + None + } else { + Some(output) + } +} + +pub 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 { + None + } else { + Some(lower_snake_case) + } +} + +fn to_lower_snake_case_from_upper_snake_case(ident: &str) -> Option { + if is_upper_snake_case(ident) { + let string = ident.chars().map(|c| c.to_ascii_lowercase()).collect(); + Some(string) + } else { + None + } +} + +fn is_upper_snake_case(ident: &str) -> bool { + ident.chars().all(|c| c.is_ascii_uppercase() || c == '_') +} + +fn is_camel_case(ident: &str) -> bool { + // We assume that the string is either snake case or camel case. + ident.chars().all(|c| c != '_') +} + +#[cfg(test)] +mod tests { + use super::*; + use expect_test::{expect, Expect}; + + fn check Option>(fun: F, input: &str, expect: Expect) { + // `None` is translated to empty string, meaning that there is nothing to fix. + let output = fun(input).unwrap_or_default(); + + expect.assert_eq(&output); + } + + #[test] + fn test_to_lower_snake_case() { + check(to_lower_snake_case, "lower_snake_case", expect![[""]]); + check(to_lower_snake_case, "UPPER_SNAKE_CASE", expect![["upper_snake_case"]]); + check(to_lower_snake_case, "CamelCase", expect![["camel_case"]]); + } + + #[test] + fn test_to_camel_case() { + check(to_camel_case, "CamelCase", expect![[""]]); + check(to_camel_case, "lower_snake_case", expect![["LowerSnakeCase"]]); + check(to_camel_case, "UPPER_SNAKE_CASE", expect![["UpperSnakeCase"]]); + } +} -- cgit v1.2.3 From 329626124f360feadb47e83be5690861c62a4b70 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sat, 3 Oct 2020 14:47:46 +0300 Subject: Add check for structure names to be CamelCase --- crates/hir_ty/src/diagnostics/decl_check.rs | 138 ++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index 1a0906492..b7f511fd8 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -14,6 +14,7 @@ mod str_helpers; use std::sync::Arc; use hir_def::{ + adt::VariantData, body::Body, db::DefDatabase, expr::{Expr, ExprId, UnaryOp}, @@ -205,6 +206,133 @@ impl<'a, 'b> DeclValidator<'a, 'b> { fn validate_struct(&mut self, db: &dyn HirDatabase, struct_id: StructId) { let data = db.struct_data(struct_id); + + // 1. Check the structure name. + let struct_name = data.name.to_string(); + let struct_name_replacement = if let Some(new_name) = to_camel_case(&struct_name) { + let replacement = Replacement { + current_name: data.name.clone(), + suggested_text: new_name, + expected_case: CaseType::UpperCamelCase, + }; + Some(replacement) + } else { + None + }; + + // 2. Check the field names. + let mut struct_fields_replacements = Vec::new(); + + if let VariantData::Record(fields) = data.variant_data.as_ref() { + for (_, field) in fields.iter() { + let field_name = field.name.to_string(); + if let Some(new_name) = to_lower_snake_case(&field_name) { + let replacement = Replacement { + current_name: field.name.clone(), + suggested_text: new_name, + expected_case: CaseType::LowerSnakeCase, + }; + struct_fields_replacements.push(replacement); + } + } + } + + // 3. If there is at least one element to spawn a warning on, go to the source map and generate a warning. + self.create_incorrect_case_diagnostic_for_struct( + struct_id, + db, + struct_name_replacement, + struct_fields_replacements, + ) + } + + /// Given the information about incorrect names in the struct declaration, looks up into the source code + /// for exact locations and adds diagnostics into the sink. + fn create_incorrect_case_diagnostic_for_struct( + &mut self, + struct_id: StructId, + db: &dyn HirDatabase, + struct_name_replacement: Option, + struct_fields_replacements: Vec, + ) { + // XXX: only look at sources if we do have incorrect names + if struct_name_replacement.is_none() && struct_fields_replacements.is_empty() { + return; + } + + let struct_loc = struct_id.lookup(db.upcast()); + let struct_src = struct_loc.source(db.upcast()); + + if let Some(replacement) = struct_name_replacement { + let ast_ptr = if let Some(name) = struct_src.value.name() { + name + } 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 structure without a name: {:?}", + replacement, + struct_src + ); + return; + }; + + let diagnostic = IncorrectCase { + file: struct_src.file_id, + ident_type: "Structure".to_string(), + ident: AstPtr::new(&ast_ptr).into(), + expected_case: replacement.expected_case, + ident_text: replacement.current_name.to_string(), + suggested_text: replacement.suggested_text, + }; + + self.sink.push(diagnostic); + } + + // 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_enum(&mut self, db: &dyn HirDatabase, enum_id: EnumId) { @@ -243,6 +371,16 @@ fn foo(SomeParam: u8) {} fn foo2(ok_param: &str, CAPS_PARAM: u8) {} // ^^^^^^^^^^ Argument `CAPS_PARAM` should have a snake_case name, e.g. `caps_param` +"#, + ); + } + + #[test] + fn incorrect_struct_name() { + check_diagnostics( + r#" +struct non_camel_case_name {} + // ^^^^^^^^^^^^^^^^^^^ Structure `non_camel_case_name` should have a CamelCase name, e.g. `NonCamelCaseName` "#, ); } -- cgit v1.2.3 From 21dd704b6b28374ea7bd2d1e13469be6807c4a8d Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sat, 3 Oct 2020 15:02:46 +0300 Subject: Check structure fields to be snake_case --- crates/hir_ty/src/diagnostics/decl_check.rs | 111 ++++++++++++++++------------ 1 file changed, 65 insertions(+), 46 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index b7f511fd8..260aa9607 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -23,7 +23,10 @@ use hir_def::{ src::HasSource, AdtId, EnumId, FunctionId, Lookup, ModuleDefId, StructId, }; -use hir_expand::{diagnostics::DiagnosticSink, name::Name}; +use hir_expand::{ + diagnostics::DiagnosticSink, + name::{AsName, Name}, +}; use syntax::{ ast::{self, NameOwner}, AstPtr, @@ -288,51 +291,49 @@ impl<'a, 'b> DeclValidator<'a, 'b> { self.sink.push(diagnostic); } - // 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); - // } + let struct_fields_list = match struct_src.value.field_list() { + Some(ast::FieldList::RecordFieldList(fields)) => fields, + _ => { + if !struct_fields_replacements.is_empty() { + log::error!( + "Replacements ({:?}) were generated for a structure fields which had no fields list: {:?}", + struct_fields_replacements, struct_src + ); + } + return; + } + }; + let mut struct_fields_iter = struct_fields_list.fields(); + for field_to_rename in struct_fields_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 struct_fields_iter.next() { + Some(element) if names_equal(element.name(), &field_to_rename.current_name) => { + break element.name().unwrap() + } + Some(_) => {} + None => { + log::error!( + "Replacement ({:?}) was generated for a function parameter which was not found: {:?}", + field_to_rename, struct_src + ); + return; + } + } + }; + + let diagnostic = IncorrectCase { + file: struct_src.file_id, + ident_type: "Field".to_string(), + ident: AstPtr::new(&ast_ptr).into(), + expected_case: field_to_rename.expected_case, + ident_text: field_to_rename.current_name.to_string(), + suggested_text: field_to_rename.suggested_text, + }; + + self.sink.push(diagnostic); + } } fn validate_enum(&mut self, db: &dyn HirDatabase, enum_id: EnumId) { @@ -340,6 +341,14 @@ impl<'a, 'b> DeclValidator<'a, 'b> { } } +fn names_equal(left: Option, right: &Name) -> bool { + if let Some(left) = left { + &left.as_name() == right + } else { + false + } +} + fn pat_equals_to_name(pat: Option, name: &Name) -> bool { if let Some(ast::Pat::IdentPat(ident)) = pat { ident.to_string() == name.to_string() @@ -381,6 +390,16 @@ fn foo2(ok_param: &str, CAPS_PARAM: u8) {} r#" struct non_camel_case_name {} // ^^^^^^^^^^^^^^^^^^^ Structure `non_camel_case_name` should have a CamelCase name, e.g. `NonCamelCaseName` +"#, + ); + } + + #[test] + fn incorrect_struct_field() { + check_diagnostics( + r#" +struct SomeStruct { SomeField: u8 } + // ^^^^^^^^^ Field `SomeField` should have a snake_case name, e.g. `some_field` "#, ); } -- cgit v1.2.3 From 17f1026c46e6e3797caf3c69737f66bd612c58e1 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sat, 3 Oct 2020 16:45:16 +0300 Subject: Improve string helpers functions --- crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs index 3d8f1b5f2..953d0276f 100644 --- a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs +++ b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs @@ -61,7 +61,9 @@ fn is_upper_snake_case(ident: &str) -> bool { fn is_camel_case(ident: &str) -> bool { // We assume that the string is either snake case or camel case. - ident.chars().all(|c| c != '_') + // `_` is allowed only at the beginning or in the end of identifier, not between characters. + ident.trim_matches('_').chars().all(|c| c != '_') + && ident.chars().find(|c| c.is_alphabetic()).map(|c| c.is_ascii_uppercase()).unwrap_or(true) } #[cfg(test)] @@ -80,13 +82,18 @@ mod tests { fn test_to_lower_snake_case() { check(to_lower_snake_case, "lower_snake_case", expect![[""]]); check(to_lower_snake_case, "UPPER_SNAKE_CASE", expect![["upper_snake_case"]]); + check(to_lower_snake_case, "Weird_Case", expect![["weird_case"]]); check(to_lower_snake_case, "CamelCase", expect![["camel_case"]]); } #[test] fn test_to_camel_case() { check(to_camel_case, "CamelCase", expect![[""]]); + check(to_camel_case, "CamelCase_", expect![[""]]); + check(to_camel_case, "_CamelCase", expect![[""]]); check(to_camel_case, "lower_snake_case", expect![["LowerSnakeCase"]]); check(to_camel_case, "UPPER_SNAKE_CASE", expect![["UpperSnakeCase"]]); + check(to_camel_case, "Weird_Case", expect![["WeirdCase"]]); + check(to_camel_case, "name", expect![["Name"]]); } } -- cgit v1.2.3 From fb96bba87895c062a78e6599cea161e461ff607d Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sat, 3 Oct 2020 18:01:25 +0300 Subject: Add diagnostics for enum names and variants --- crates/hir_ty/src/diagnostics/decl_check.rs | 147 +++++++++++++++++++++++++++- 1 file changed, 146 insertions(+), 1 deletion(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index 260aa9607..7fc9c564e 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -315,7 +315,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> { Some(_) => {} None => { log::error!( - "Replacement ({:?}) was generated for a function parameter which was not found: {:?}", + "Replacement ({:?}) was generated for a structure field which was not found: {:?}", field_to_rename, struct_src ); return; @@ -338,6 +338,131 @@ impl<'a, 'b> DeclValidator<'a, 'b> { fn validate_enum(&mut self, db: &dyn HirDatabase, enum_id: EnumId) { let data = db.enum_data(enum_id); + + // 1. Check the enum name. + let enum_name = data.name.to_string(); + let enum_name_replacement = if let Some(new_name) = to_camel_case(&enum_name) { + let replacement = Replacement { + current_name: data.name.clone(), + suggested_text: new_name, + expected_case: CaseType::UpperCamelCase, + }; + Some(replacement) + } else { + None + }; + + // 2. Check the field names. + let mut enum_fields_replacements = Vec::new(); + + for (_, variant) in data.variants.iter() { + let variant_name = variant.name.to_string(); + if let Some(new_name) = to_camel_case(&variant_name) { + let replacement = Replacement { + current_name: variant.name.clone(), + suggested_text: new_name, + expected_case: CaseType::UpperCamelCase, + }; + enum_fields_replacements.push(replacement); + } + } + + // 3. If there is at least one element to spawn a warning on, go to the source map and generate a warning. + self.create_incorrect_case_diagnostic_for_enum( + enum_id, + db, + enum_name_replacement, + enum_fields_replacements, + ) + } + + /// Given the information about incorrect names in the struct declaration, looks up into the source code + /// for exact locations and adds diagnostics into the sink. + fn create_incorrect_case_diagnostic_for_enum( + &mut self, + enum_id: EnumId, + db: &dyn HirDatabase, + enum_name_replacement: Option, + enum_variants_replacements: Vec, + ) { + // XXX: only look at sources if we do have incorrect names + if enum_name_replacement.is_none() && enum_variants_replacements.is_empty() { + return; + } + + let enum_loc = enum_id.lookup(db.upcast()); + let enum_src = enum_loc.source(db.upcast()); + + if let Some(replacement) = enum_name_replacement { + let ast_ptr = if let Some(name) = enum_src.value.name() { + name + } 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 enum without a name: {:?}", + replacement, + enum_src + ); + return; + }; + + let diagnostic = IncorrectCase { + file: enum_src.file_id, + ident_type: "Enum".to_string(), + ident: AstPtr::new(&ast_ptr).into(), + expected_case: replacement.expected_case, + ident_text: replacement.current_name.to_string(), + suggested_text: replacement.suggested_text, + }; + + self.sink.push(diagnostic); + } + + let enum_variants_list = match enum_src.value.variant_list() { + Some(variants) => variants, + _ => { + if !enum_variants_replacements.is_empty() { + log::error!( + "Replacements ({:?}) were generated for a enum variants which had no fields list: {:?}", + enum_variants_replacements, enum_src + ); + } + return; + } + }; + let mut enum_variants_iter = enum_variants_list.variants(); + for variant_to_rename in enum_variants_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 enum_variants_iter.next() { + Some(variant) + if names_equal(variant.name(), &variant_to_rename.current_name) => + { + break variant.name().unwrap() + } + Some(_) => {} + None => { + log::error!( + "Replacement ({:?}) was generated for a enum variant which was not found: {:?}", + variant_to_rename, enum_src + ); + return; + } + } + }; + + let diagnostic = IncorrectCase { + file: enum_src.file_id, + ident_type: "Variant".to_string(), + ident: AstPtr::new(&ast_ptr).into(), + expected_case: variant_to_rename.expected_case, + ident_text: variant_to_rename.current_name.to_string(), + suggested_text: variant_to_rename.suggested_text, + }; + + self.sink.push(diagnostic); + } } } @@ -400,6 +525,26 @@ struct non_camel_case_name {} r#" struct SomeStruct { SomeField: u8 } // ^^^^^^^^^ Field `SomeField` should have a snake_case name, e.g. `some_field` +"#, + ); + } + + #[test] + fn incorrect_enum_name() { + check_diagnostics( + r#" +enum some_enum { Val(u8) } + // ^^^^^^^^^ Enum `some_enum` should have a CamelCase name, e.g. `SomeEnum` +"#, + ); + } + + #[test] + fn incorrect_enum_variant_name() { + check_diagnostics( + r#" +enum SomeEnum { SOME_VARIANT(u8) } + // ^^^^^^^^^^^^ Variant `SOME_VARIANT` should have a CamelCase name, e.g. `SomeVariant` "#, ); } -- cgit v1.2.3 From 9ec1741b651bd13e4e5e6224f2e2c5c503846a6b Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sun, 4 Oct 2020 07:37:43 +0300 Subject: Refactor string helpers for decl_check module --- .../src/diagnostics/decl_check/str_helpers.rs | 129 ++++++++++++++++----- 1 file changed, 97 insertions(+), 32 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs index 953d0276f..e3826909b 100644 --- a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs +++ b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs @@ -1,10 +1,74 @@ +#[derive(Debug)] +enum DetectedCase { + LowerCamelCase, + UpperCamelCase, + LowerSnakeCase, + UpperSnakeCase, + Unknown, +} + +fn detect_case(ident: &str) -> DetectedCase { + let trimmed_ident = ident.trim_matches('_'); + let first_lowercase = + trimmed_ident.chars().next().map(|chr| chr.is_ascii_lowercase()).unwrap_or(false); + let mut has_lowercase = first_lowercase; + let mut has_uppercase = false; + let mut has_underscore = false; + + for chr in trimmed_ident.chars() { + if chr == '_' { + has_underscore = true; + } else if chr.is_ascii_uppercase() { + has_uppercase = true; + } else if chr.is_ascii_lowercase() { + has_lowercase = true; + } + } + + if has_uppercase { + if !has_lowercase { + DetectedCase::UpperSnakeCase + } else if !has_underscore { + if first_lowercase { + DetectedCase::LowerCamelCase + } else { + DetectedCase::UpperCamelCase + } + } else { + // It has uppercase, it has lowercase, it has underscore. + // No assumptions here + DetectedCase::Unknown + } + } else { + DetectedCase::LowerSnakeCase + } +} + pub fn to_camel_case(ident: &str) -> Option { - let mut output = String::new(); + let detected_case = detect_case(ident); - if is_camel_case(ident) { - return None; + match detected_case { + DetectedCase::UpperCamelCase => return None, + DetectedCase::LowerCamelCase => { + let mut first_capitalized = false; + let output = ident + .chars() + .map(|chr| { + if !first_capitalized && chr.is_ascii_lowercase() { + first_capitalized = true; + chr.to_ascii_uppercase() + } else { + chr + } + }) + .collect(); + return Some(output); + } + _ => {} } + let mut output = String::with_capacity(ident.len()); + let mut capital_added = false; for chr in ident.chars() { if chr.is_alphabetic() { @@ -23,47 +87,37 @@ pub fn to_camel_case(ident: &str) -> Option { } } - if output == ident { - None - } else { - Some(output) - } + Some(output) } pub 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); + match detect_case(ident) { + DetectedCase::LowerSnakeCase => return None, + DetectedCase::UpperSnakeCase => { + return Some(ident.chars().map(|chr| chr.to_ascii_lowercase()).collect()) + } + _ => {} } // Otherwise, assume that it's CamelCase. let lower_snake_case = stdx::to_lower_snake_case(ident); - - if lower_snake_case == ident { - None - } else { - Some(lower_snake_case) - } + Some(lower_snake_case) } -fn to_lower_snake_case_from_upper_snake_case(ident: &str) -> Option { - if is_upper_snake_case(ident) { - let string = ident.chars().map(|c| c.to_ascii_lowercase()).collect(); - Some(string) - } else { - None +pub fn to_upper_snake_case(ident: &str) -> Option { + match detect_case(ident) { + DetectedCase::UpperSnakeCase => return None, + DetectedCase::LowerSnakeCase => { + return Some(ident.chars().map(|chr| chr.to_ascii_uppercase()).collect()) + } + _ => {} } -} - -fn is_upper_snake_case(ident: &str) -> bool { - ident.chars().all(|c| c.is_ascii_uppercase() || c == '_') -} -fn is_camel_case(ident: &str) -> bool { - // We assume that the string is either snake case or camel case. - // `_` is allowed only at the beginning or in the end of identifier, not between characters. - ident.trim_matches('_').chars().all(|c| c != '_') - && ident.chars().find(|c| c.is_alphabetic()).map(|c| c.is_ascii_uppercase()).unwrap_or(true) + // Normalize the string from whatever form it's in currently, and then just make it uppercase. + let upper_snake_case = + stdx::to_lower_snake_case(ident).chars().map(|c| c.to_ascii_uppercase()).collect(); + Some(upper_snake_case) } #[cfg(test)] @@ -84,6 +138,7 @@ mod tests { check(to_lower_snake_case, "UPPER_SNAKE_CASE", expect![["upper_snake_case"]]); check(to_lower_snake_case, "Weird_Case", expect![["weird_case"]]); check(to_lower_snake_case, "CamelCase", expect![["camel_case"]]); + check(to_lower_snake_case, "lowerCamelCase", expect![["lower_camel_case"]]); } #[test] @@ -91,9 +146,19 @@ mod tests { check(to_camel_case, "CamelCase", expect![[""]]); check(to_camel_case, "CamelCase_", expect![[""]]); check(to_camel_case, "_CamelCase", expect![[""]]); + check(to_camel_case, "lowerCamelCase", expect![["LowerCamelCase"]]); check(to_camel_case, "lower_snake_case", expect![["LowerSnakeCase"]]); check(to_camel_case, "UPPER_SNAKE_CASE", expect![["UpperSnakeCase"]]); check(to_camel_case, "Weird_Case", expect![["WeirdCase"]]); check(to_camel_case, "name", expect![["Name"]]); } + + #[test] + fn test_to_upper_snake_case() { + check(to_upper_snake_case, "UPPER_SNAKE_CASE", expect![[""]]); + check(to_upper_snake_case, "lower_snake_case", expect![["LOWER_SNAKE_CASE"]]); + check(to_upper_snake_case, "Weird_Case", expect![["WEIRD_CASE"]]); + check(to_upper_snake_case, "CamelCase", expect![["CAMEL_CASE"]]); + check(to_upper_snake_case, "lowerCamelCase", expect![["LOWER_CAMEL_CASE"]]); + } } -- cgit v1.2.3 From b42562b5dee4f4ce23094c7bab6406e3b00f90ad Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sun, 4 Oct 2020 07:39:35 +0300 Subject: Make incorrect case diagnostic work inside of functions --- crates/hir_ty/src/diagnostics/decl_check.rs | 277 ++++++++++++++++++++++++---- 1 file changed, 245 insertions(+), 32 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index 7fc9c564e..d1c51849a 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -5,23 +5,13 @@ //! - enum fields (e.g. `enum Foo { Variant { field: u8 } }`) //! - function/method arguments (e.g. `fn foo(arg: u8)`) -// TODO: Temporary, to not see warnings until module is somewhat complete. -// If you see these lines in the pull request, feel free to call me stupid :P. -#![allow(dead_code, unused_imports, unused_variables)] - mod str_helpers; -use std::sync::Arc; - use hir_def::{ adt::VariantData, - body::Body, - db::DefDatabase, - expr::{Expr, ExprId, UnaryOp}, - item_tree::ItemTreeNode, - resolver::{resolver_for_expr, ResolveValueResult, ValueNs}, + expr::{Pat, PatId}, src::HasSource, - AdtId, EnumId, FunctionId, Lookup, ModuleDefId, StructId, + AdtId, ConstId, EnumId, FunctionId, Lookup, ModuleDefId, StaticId, StructId, }; use hir_expand::{ diagnostics::DiagnosticSink, @@ -35,8 +25,6 @@ use syntax::{ use crate::{ db::HirDatabase, diagnostics::{decl_check::str_helpers::*, CaseType, IncorrectCase}, - lower::CallableDefId, - ApplicationTy, InferenceResult, Ty, TypeCtor, }; pub(super) struct DeclValidator<'a, 'b: 'a> { @@ -64,12 +52,25 @@ impl<'a, 'b> DeclValidator<'a, 'b> { match self.owner { ModuleDefId::FunctionId(func) => self.validate_func(db, func), ModuleDefId::AdtId(adt) => self.validate_adt(db, adt), + ModuleDefId::ConstId(const_id) => self.validate_const(db, const_id), + ModuleDefId::StaticId(static_id) => self.validate_static(db, static_id), _ => return, } } + fn validate_adt(&mut self, db: &dyn HirDatabase, adt: AdtId) { + match adt { + AdtId::StructId(struct_id) => self.validate_struct(db, struct_id), + AdtId::EnumId(enum_id) => self.validate_enum(db, enum_id), + AdtId::UnionId(_) => { + // Unions aren't yet supported by this validator. + } + } + } + fn validate_func(&mut self, db: &dyn HirDatabase, func: FunctionId) { let data = db.function_data(func); + let body = db.body(func.into()); // 1. Check the function name. let function_name = data.name.to_string(); @@ -87,11 +88,18 @@ impl<'a, 'b> DeclValidator<'a, 'b> { // 2. Check the param names. let mut fn_param_replacements = Vec::new(); - for param_name in data.param_names.iter().cloned().filter_map(|i| i) { + for pat_id in body.params.iter().cloned() { + let pat = &body[pat_id]; + + let param_name = match pat { + Pat::Bind { name, .. } => name, + _ => continue, + }; + let name = param_name.to_string(); if let Some(new_name) = to_lower_snake_case(&name) { let replacement = Replacement { - current_name: param_name, + current_name: param_name.clone(), suggested_text: new_name, expected_case: CaseType::LowerSnakeCase, }; @@ -99,13 +107,45 @@ impl<'a, 'b> DeclValidator<'a, 'b> { } } - // 3. If there is at least one element to spawn a warning on, go to the source map and generate a warning. + // 3. Check the patterns inside the function body. + let mut pats_replacements = Vec::new(); + + for (pat_idx, pat) in body.pats.iter() { + if body.params.contains(&pat_idx) { + // We aren't interested in function parameters, we've processed them above. + continue; + } + + let bind_name = match pat { + Pat::Bind { name, .. } => name, + _ => continue, + }; + + let name = bind_name.to_string(); + if let Some(new_name) = to_lower_snake_case(&name) { + let replacement = Replacement { + current_name: bind_name.clone(), + suggested_text: new_name, + expected_case: CaseType::LowerSnakeCase, + }; + pats_replacements.push((pat_idx, replacement)); + } + } + + // 4. If there is at least one element to spawn a warning on, go to the source map and generate a warning. self.create_incorrect_case_diagnostic_for_func( func, db, fn_name_replacement, fn_param_replacements, - ) + ); + self.create_incorrect_case_diagnostic_for_variables(func, db, pats_replacements); + + // 5. Recursively validate inner scope items, such as static variables and constants. + for (item_id, _) in body.item_scope.values() { + let mut validator = DeclValidator::new(item_id, self.sink); + validator.validate_item(db); + } } /// Given the information about incorrect names in the function declaration, looks up into the source code @@ -125,6 +165,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> { let fn_loc = func.lookup(db.upcast()); let fn_src = fn_loc.source(db.upcast()); + // 1. Diagnostic for function name. if let Some(replacement) = fn_name_replacement { let ast_ptr = if let Some(name) = fn_src.value.name() { name @@ -150,6 +191,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> { self.sink.push(diagnostic); } + // 2. Diagnostics for function params. let fn_params_list = match fn_src.value.param_list() { Some(params) => params, None => { @@ -197,12 +239,38 @@ impl<'a, 'b> DeclValidator<'a, 'b> { } } - fn validate_adt(&mut self, db: &dyn HirDatabase, adt: AdtId) { - match adt { - AdtId::StructId(struct_id) => self.validate_struct(db, struct_id), - AdtId::EnumId(enum_id) => self.validate_enum(db, enum_id), - AdtId::UnionId(_) => { - // Unions aren't yet supported by this validator. + /// Given the information about incorrect variable names, looks up into the source code + /// for exact locations and adds diagnostics into the sink. + fn create_incorrect_case_diagnostic_for_variables( + &mut self, + func: FunctionId, + db: &dyn HirDatabase, + pats_replacements: Vec<(PatId, Replacement)>, + ) { + // XXX: only look at source_map if we do have missing fields + if pats_replacements.is_empty() { + return; + } + + let (_, source_map) = db.body_with_source_map(func.into()); + + for (id, replacement) in pats_replacements { + if let Ok(source_ptr) = source_map.pat_syntax(id) { + if let Some(expr) = source_ptr.value.as_ref().left() { + let root = source_ptr.file_syntax(db.upcast()); + if let ast::Pat::IdentPat(ident_pat) = expr.to_node(&root) { + let diagnostic = IncorrectCase { + file: source_ptr.file_id, + ident_type: "Variable".to_string(), + ident: AstPtr::new(&ident_pat).into(), + expected_case: replacement.expected_case, + ident_text: replacement.current_name.to_string(), + suggested_text: replacement.suggested_text, + }; + + self.sink.push(diagnostic); + } + } } } } @@ -246,7 +314,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> { db, struct_name_replacement, struct_fields_replacements, - ) + ); } /// Given the information about incorrect names in the struct declaration, looks up into the source code @@ -464,6 +532,86 @@ impl<'a, 'b> DeclValidator<'a, 'b> { self.sink.push(diagnostic); } } + + fn validate_const(&mut self, db: &dyn HirDatabase, const_id: ConstId) { + let data = db.const_data(const_id); + + let name = match &data.name { + Some(name) => name, + None => return, + }; + + let const_name = name.to_string(); + let replacement = if let Some(new_name) = to_upper_snake_case(&const_name) { + Replacement { + current_name: name.clone(), + suggested_text: new_name, + expected_case: CaseType::UpperSnakeCase, + } + } else { + // Nothing to do here. + return; + }; + + let const_loc = const_id.lookup(db.upcast()); + let const_src = const_loc.source(db.upcast()); + + let ast_ptr = match const_src.value.name() { + Some(name) => name, + None => return, + }; + + let diagnostic = IncorrectCase { + file: const_src.file_id, + ident_type: "Constant".to_string(), + ident: AstPtr::new(&ast_ptr).into(), + expected_case: replacement.expected_case, + ident_text: replacement.current_name.to_string(), + suggested_text: replacement.suggested_text, + }; + + self.sink.push(diagnostic); + } + + fn validate_static(&mut self, db: &dyn HirDatabase, static_id: StaticId) { + let data = db.static_data(static_id); + + let name = match &data.name { + Some(name) => name, + None => return, + }; + + let static_name = name.to_string(); + let replacement = if let Some(new_name) = to_upper_snake_case(&static_name) { + Replacement { + current_name: name.clone(), + suggested_text: new_name, + expected_case: CaseType::UpperSnakeCase, + } + } else { + // Nothing to do here. + return; + }; + + let static_loc = static_id.lookup(db.upcast()); + let static_src = static_loc.source(db.upcast()); + + let ast_ptr = match static_src.value.name() { + Some(name) => name, + None => return, + }; + + let diagnostic = IncorrectCase { + file: static_src.file_id, + ident_type: "Static variable".to_string(), + ident: AstPtr::new(&ast_ptr).into(), + expected_case: replacement.expected_case, + ident_text: replacement.current_name.to_string(), + suggested_text: replacement.suggested_text, + }; + + self.sink.push(diagnostic); + } } fn names_equal(left: Option, right: &Name) -> bool { @@ -491,7 +639,7 @@ mod tests { check_diagnostics( r#" fn NonSnakeCaseName() {} -// ^^^^^^^^^^^^^^^^ Function `NonSnakeCaseName` should have a snake_case name, e.g. `non_snake_case_name` +// ^^^^^^^^^^^^^^^^ Function `NonSnakeCaseName` should have snake_case name, e.g. `non_snake_case_name` "#, ); } @@ -501,10 +649,24 @@ fn NonSnakeCaseName() {} check_diagnostics( r#" fn foo(SomeParam: u8) {} - // ^^^^^^^^^ Argument `SomeParam` should have a snake_case name, e.g. `some_param` + // ^^^^^^^^^ Argument `SomeParam` should have 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` + // ^^^^^^^^^^ Argument `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 a snake_case name, e.g. `some_value` + let AnotherValue = 20; + // ^^^^^^^^^^^^ Variable `AnotherValue` should have snake_case name, e.g. `another_value` +} "#, ); } @@ -514,7 +676,7 @@ fn foo2(ok_param: &str, CAPS_PARAM: u8) {} check_diagnostics( r#" struct non_camel_case_name {} - // ^^^^^^^^^^^^^^^^^^^ Structure `non_camel_case_name` should have a CamelCase name, e.g. `NonCamelCaseName` + // ^^^^^^^^^^^^^^^^^^^ Structure `non_camel_case_name` should have CamelCase name, e.g. `NonCamelCaseName` "#, ); } @@ -524,7 +686,7 @@ struct non_camel_case_name {} check_diagnostics( r#" struct SomeStruct { SomeField: u8 } - // ^^^^^^^^^ Field `SomeField` should have a snake_case name, e.g. `some_field` + // ^^^^^^^^^ Field `SomeField` should have snake_case name, e.g. `some_field` "#, ); } @@ -534,7 +696,7 @@ struct SomeStruct { SomeField: u8 } check_diagnostics( r#" enum some_enum { Val(u8) } - // ^^^^^^^^^ Enum `some_enum` should have a CamelCase name, e.g. `SomeEnum` + // ^^^^^^^^^ Enum `some_enum` should have CamelCase name, e.g. `SomeEnum` "#, ); } @@ -544,7 +706,58 @@ enum some_enum { Val(u8) } check_diagnostics( r#" enum SomeEnum { SOME_VARIANT(u8) } - // ^^^^^^^^^^^^ Variant `SOME_VARIANT` should have a CamelCase name, e.g. `SomeVariant` + // ^^^^^^^^^^^^ 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` + +fn func() { + const someConstInFunc: &str = "hi there"; + // ^^^^^^^^^^^^^^^ Constant `someConstInFunc` should have UPPER_SNAKE_CASE name, e.g. `SOME_CONST_IN_FUNC` + +} +"#, + ); + } + + #[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` + +fn func() { + static someConstInFunc: &str = "hi there"; + // ^^^^^^^^^^^^^^^ Static variable `someConstInFunc` should have UPPER_SNAKE_CASE name, e.g. `SOME_CONST_IN_FUNC` +} +"#, + ); + } + + #[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` + static someConstInFunc: &str = "hi there"; + // ^^^^^^^^^^^^^^^ Static variable `someConstInFunc` should have UPPER_SNAKE_CASE name, e.g. `SOME_CONST_IN_FUNC` + let WHY_VAR_IS_CAPS = 10; + // ^^^^^^^^^^^^^^^ Variable `WHY_VAR_IS_CAPS` should have snake_case name, e.g. `why_var_is_caps` + } +} "#, ); } -- cgit v1.2.3 From 45ac2b2edec05e417124ebfc2e61ec2a5117f4d5 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sun, 4 Oct 2020 08:53:34 +0300 Subject: Code style adjustments --- crates/hir_ty/src/diagnostics/decl_check.rs | 52 +++++++++++++++++++++- .../src/diagnostics/decl_check/str_helpers.rs | 38 ++++++++++++++-- crates/hir_ty/src/diagnostics/unsafe_check.rs | 6 +-- 3 files changed, 88 insertions(+), 8 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index d1c51849a..3a95f1b82 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -19,7 +19,7 @@ use hir_expand::{ }; use syntax::{ ast::{self, NameOwner}, - AstPtr, + AstNode, AstPtr, }; use crate::{ @@ -259,6 +259,21 @@ impl<'a, 'b> DeclValidator<'a, 'b> { if let Some(expr) = source_ptr.value.as_ref().left() { let root = source_ptr.file_syntax(db.upcast()); if let ast::Pat::IdentPat(ident_pat) = expr.to_node(&root) { + let parent = match ident_pat.syntax().parent() { + Some(parent) => parent, + None => continue, + }; + + // We have to check that it's either `let var = ...` or `Variant(_) @ var` statement, + // because e.g. match arms are patterns as well. + // In other words, we check that it's a named variable binding. + if !ast::LetStmt::cast(parent.clone()).is_some() + && !ast::IdentPat::cast(parent).is_some() + { + // This pattern is not an actual variable declaration, e.g. `Some(val) => {..}` match arm. + continue; + } + let diagnostic = IncorrectCase { file: source_ptr.file_id, ident_type: "Variable".to_string(), @@ -663,7 +678,7 @@ fn foo2(ok_param: &str, CAPS_PARAM: u8) {} r#" fn foo() { let SOME_VALUE = 10; - // ^^^^^^^^^^ Variable `SOME_VALUE` should have a snake_case name, e.g. `some_value` + // ^^^^^^^^^^ 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` } @@ -758,6 +773,39 @@ impl someStruct { // ^^^^^^^^^^^^^^^ 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 { + None @ SOME_VAR => (), + // ^^^^^^^^ Variable `SOME_VAR` should have snake_case name, e.g. `some_var` + Some => (), + } +} "#, ); } diff --git a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs index e3826909b..8f70c5e84 100644 --- a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs +++ b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs @@ -1,3 +1,6 @@ +//! Functions for string case manipulation, such as detecting the identifier case, +//! and converting it into appropriate form. + #[derive(Debug)] enum DetectedCase { LowerCamelCase, @@ -44,6 +47,8 @@ fn detect_case(ident: &str) -> DetectedCase { } } +/// Converts an identifier to an UpperCamelCase form. +/// Returns `None` if the string is already is UpperCamelCase. pub fn to_camel_case(ident: &str) -> Option { let detected_case = detect_case(ident); @@ -87,9 +92,17 @@ pub fn to_camel_case(ident: &str) -> Option { } } - Some(output) + if output == ident { + // While we didn't detect the correct case at the beginning, there + // may be special cases: e.g. `A` is both valid CamelCase and UPPER_SNAKE_CASE. + None + } else { + Some(output) + } } +/// Converts an identifier to a lower_snake_case form. +/// Returns `None` if the string is already is lower_snake_case. pub fn to_lower_snake_case(ident: &str) -> Option { // First, assume that it's UPPER_SNAKE_CASE. match detect_case(ident) { @@ -102,9 +115,18 @@ pub fn to_lower_snake_case(ident: &str) -> Option { // Otherwise, assume that it's CamelCase. let lower_snake_case = stdx::to_lower_snake_case(ident); - Some(lower_snake_case) + + if lower_snake_case == ident { + // While we didn't detect the correct case at the beginning, there + // may be special cases: e.g. `a` is both valid camelCase and snake_case. + None + } else { + Some(lower_snake_case) + } } +/// Converts an identifier to an UPPER_SNAKE_CASE form. +/// Returns `None` if the string is already is UPPER_SNAKE_CASE. pub fn to_upper_snake_case(ident: &str) -> Option { match detect_case(ident) { DetectedCase::UpperSnakeCase => return None, @@ -117,7 +139,14 @@ pub fn to_upper_snake_case(ident: &str) -> Option { // Normalize the string from whatever form it's in currently, and then just make it uppercase. let upper_snake_case = stdx::to_lower_snake_case(ident).chars().map(|c| c.to_ascii_uppercase()).collect(); - Some(upper_snake_case) + + if upper_snake_case == ident { + // While we didn't detect the correct case at the beginning, there + // may be special cases: e.g. `A` is both valid CamelCase and UPPER_SNAKE_CASE. + None + } else { + Some(upper_snake_case) + } } #[cfg(test)] @@ -139,6 +168,7 @@ mod tests { check(to_lower_snake_case, "Weird_Case", expect![["weird_case"]]); check(to_lower_snake_case, "CamelCase", expect![["camel_case"]]); check(to_lower_snake_case, "lowerCamelCase", expect![["lower_camel_case"]]); + check(to_lower_snake_case, "a", expect![[""]]); } #[test] @@ -151,6 +181,7 @@ mod tests { check(to_camel_case, "UPPER_SNAKE_CASE", expect![["UpperSnakeCase"]]); check(to_camel_case, "Weird_Case", expect![["WeirdCase"]]); check(to_camel_case, "name", expect![["Name"]]); + check(to_camel_case, "A", expect![[""]]); } #[test] @@ -160,5 +191,6 @@ mod tests { check(to_upper_snake_case, "Weird_Case", expect![["WEIRD_CASE"]]); check(to_upper_snake_case, "CamelCase", expect![["CAMEL_CASE"]]); check(to_upper_snake_case, "lowerCamelCase", expect![["LOWER_CAMEL_CASE"]]); + check(to_upper_snake_case, "A", expect![[""]]); } } diff --git a/crates/hir_ty/src/diagnostics/unsafe_check.rs b/crates/hir_ty/src/diagnostics/unsafe_check.rs index 61ffbf5d1..21a121aad 100644 --- a/crates/hir_ty/src/diagnostics/unsafe_check.rs +++ b/crates/hir_ty/src/diagnostics/unsafe_check.rs @@ -190,13 +190,13 @@ struct Ty { a: u8, } -static mut static_mut: Ty = Ty { a: 0 }; +static mut STATIC_MUT: Ty = Ty { a: 0 }; fn main() { - let x = static_mut.a; + let x = STATIC_MUT.a; //^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block unsafe { - let x = static_mut.a; + let x = STATIC_MUT.a; } } "#, -- cgit v1.2.3 From 2a72f876d655da086e436838fdbc797a2ef71ece Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sun, 4 Oct 2020 09:04:28 +0300 Subject: Fix issues with match arm bindings --- crates/hir_ty/src/diagnostics/decl_check.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index 3a95f1b82..28ce15773 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -263,13 +263,18 @@ impl<'a, 'b> DeclValidator<'a, 'b> { Some(parent) => parent, None => continue, }; + let name_ast = match ident_pat.name() { + Some(name_ast) => name_ast, + None => continue, + }; - // We have to check that it's either `let var = ...` or `Variant(_) @ var` statement, + // We have to check that it's either `let var = ...` or `var @ Variant(_)` statement, // because e.g. match arms are patterns as well. // In other words, we check that it's a named variable binding. - if !ast::LetStmt::cast(parent.clone()).is_some() - && !ast::IdentPat::cast(parent).is_some() - { + let is_binding = ast::LetStmt::cast(parent.clone()).is_some() + || (ast::MatchArm::cast(parent).is_some() + && ident_pat.at_token().is_some()); + if !is_binding { // This pattern is not an actual variable declaration, e.g. `Some(val) => {..}` match arm. continue; } @@ -277,7 +282,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> { let diagnostic = IncorrectCase { file: source_ptr.file_id, ident_type: "Variable".to_string(), - ident: AstPtr::new(&ident_pat).into(), + ident: AstPtr::new(&name_ast).into(), expected_case: replacement.expected_case, ident_text: replacement.current_name.to_string(), suggested_text: replacement.suggested_text, @@ -801,8 +806,8 @@ enum Option { Some, None } fn main() { match Option::None { - None @ SOME_VAR => (), - // ^^^^^^^^ Variable `SOME_VAR` should have snake_case name, e.g. `some_var` + SOME_VAR @ None => (), + // ^^^^^^^^ Variable `SOME_VAR` should have snake_case name, e.g. `some_var` Some => (), } } -- cgit v1.2.3 From 9ea57cac0e9779ac0749ef568eeb3977fe3adacd Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sun, 4 Oct 2020 09:26:39 +0300 Subject: Fix code style issues --- crates/hir_ty/src/diagnostics/decl_check.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index 28ce15773..4c20921e5 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -1,9 +1,14 @@ //! Provides validators for the item declarations. +//! //! This includes the following items: +//! //! - variable bindings (e.g. `let x = foo();`) //! - struct fields (e.g. `struct Foo { field: u8 }`) -//! - enum fields (e.g. `enum Foo { Variant { field: u8 } }`) +//! - enum variants (e.g. `enum Foo { Variant { field: u8 } }`) //! - function/method arguments (e.g. `fn foo(arg: u8)`) +//! - constants (e.g. `const FOO: u8 = 10;`) +//! - static items (e.g. `static FOO: u8 = 10;`) +//! - match arm bindings (e.g. `foo @ Some(_)`) mod str_helpers; @@ -48,7 +53,6 @@ impl<'a, 'b> DeclValidator<'a, 'b> { } pub(super) fn validate_item(&mut self, db: &dyn HirDatabase) { - // let def = self.owner.into(); match self.owner { ModuleDefId::FunctionId(func) => self.validate_func(db, func), ModuleDefId::AdtId(adt) => self.validate_adt(db, adt), -- cgit v1.2.3 From f2c91fc5a8c22b8ac80f100b2b666f2dc9baa67c Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Mon, 5 Oct 2020 19:34:23 +0300 Subject: Apply suggestions from code review Co-authored-by: Lukas Wirth --- crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs index 8f70c5e84..c1ab1a675 100644 --- a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs +++ b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs @@ -13,7 +13,7 @@ enum DetectedCase { fn detect_case(ident: &str) -> DetectedCase { let trimmed_ident = ident.trim_matches('_'); let first_lowercase = - trimmed_ident.chars().next().map(|chr| chr.is_ascii_lowercase()).unwrap_or(false); + trimmed_ident.starts_with(|chr| chr.is_ascii_lowercase()); let mut has_lowercase = first_lowercase; let mut has_uppercase = false; let mut has_underscore = false; @@ -102,7 +102,7 @@ pub fn to_camel_case(ident: &str) -> Option { } /// Converts an identifier to a lower_snake_case form. -/// Returns `None` if the string is already is lower_snake_case. +/// Returns `None` if the string is already in lower_snake_case. pub fn to_lower_snake_case(ident: &str) -> Option { // First, assume that it's UPPER_SNAKE_CASE. match detect_case(ident) { -- cgit v1.2.3 From ebd30033b3743fafe0a0182b5ae34ffb27fe43ff Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Mon, 5 Oct 2020 20:35:52 +0300 Subject: Fix compilation error --- crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs index c1ab1a675..2e1468c4c 100644 --- a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs +++ b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs @@ -12,8 +12,7 @@ enum DetectedCase { fn detect_case(ident: &str) -> DetectedCase { let trimmed_ident = ident.trim_matches('_'); - let first_lowercase = - trimmed_ident.starts_with(|chr| chr.is_ascii_lowercase()); + let first_lowercase = trimmed_ident.starts_with(|chr: char| chr.is_ascii_lowercase()); let mut has_lowercase = first_lowercase; let mut has_uppercase = false; let mut has_underscore = false; -- cgit v1.2.3 From 559cc970732d80e3ec624c20da4f8aac219d6b2e Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Thu, 8 Oct 2020 08:33:35 +0300 Subject: Add to_upper_snake_case function to stdx --- crates/hir_ty/src/diagnostics/decl_check.rs | 4 +- .../hir_ty/src/diagnostics/decl_check/case_conv.rs | 194 ++++++++++++++++++++ .../src/diagnostics/decl_check/str_helpers.rs | 195 --------------------- 3 files changed, 196 insertions(+), 197 deletions(-) create mode 100644 crates/hir_ty/src/diagnostics/decl_check/case_conv.rs delete mode 100644 crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index 4c20921e5..901ccc94f 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -10,7 +10,7 @@ //! - static items (e.g. `static FOO: u8 = 10;`) //! - match arm bindings (e.g. `foo @ Some(_)`) -mod str_helpers; +mod case_conv; use hir_def::{ adt::VariantData, @@ -29,7 +29,7 @@ use syntax::{ use crate::{ db::HirDatabase, - diagnostics::{decl_check::str_helpers::*, CaseType, IncorrectCase}, + diagnostics::{decl_check::case_conv::*, CaseType, IncorrectCase}, }; pub(super) struct DeclValidator<'a, 'b: 'a> { diff --git a/crates/hir_ty/src/diagnostics/decl_check/case_conv.rs b/crates/hir_ty/src/diagnostics/decl_check/case_conv.rs new file mode 100644 index 000000000..3800f2a6b --- /dev/null +++ b/crates/hir_ty/src/diagnostics/decl_check/case_conv.rs @@ -0,0 +1,194 @@ +//! Functions for string case manipulation, such as detecting the identifier case, +//! and converting it into appropriate form. + +#[derive(Debug)] +enum DetectedCase { + LowerCamelCase, + UpperCamelCase, + LowerSnakeCase, + UpperSnakeCase, + Unknown, +} + +fn detect_case(ident: &str) -> DetectedCase { + let trimmed_ident = ident.trim_matches('_'); + let first_lowercase = trimmed_ident.starts_with(|chr: char| chr.is_ascii_lowercase()); + let mut has_lowercase = first_lowercase; + let mut has_uppercase = false; + let mut has_underscore = false; + + for chr in trimmed_ident.chars() { + if chr == '_' { + has_underscore = true; + } else if chr.is_ascii_uppercase() { + has_uppercase = true; + } else if chr.is_ascii_lowercase() { + has_lowercase = true; + } + } + + if has_uppercase { + if !has_lowercase { + DetectedCase::UpperSnakeCase + } else if !has_underscore { + if first_lowercase { + DetectedCase::LowerCamelCase + } else { + DetectedCase::UpperCamelCase + } + } else { + // It has uppercase, it has lowercase, it has underscore. + // No assumptions here + DetectedCase::Unknown + } + } else { + DetectedCase::LowerSnakeCase + } +} + +/// Converts an identifier to an UpperCamelCase form. +/// Returns `None` if the string is already is UpperCamelCase. +pub fn to_camel_case(ident: &str) -> Option { + let detected_case = detect_case(ident); + + match detected_case { + DetectedCase::UpperCamelCase => return None, + DetectedCase::LowerCamelCase => { + let mut first_capitalized = false; + let output = ident + .chars() + .map(|chr| { + if !first_capitalized && chr.is_ascii_lowercase() { + first_capitalized = true; + chr.to_ascii_uppercase() + } else { + chr + } + }) + .collect(); + return Some(output); + } + _ => {} + } + + let mut output = String::with_capacity(ident.len()); + + let mut capital_added = false; + for chr in ident.chars() { + if chr.is_alphabetic() { + if !capital_added { + output.push(chr.to_ascii_uppercase()); + capital_added = true; + } else { + output.push(chr.to_ascii_lowercase()); + } + } else if chr == '_' { + // Skip this character and make the next one capital. + capital_added = false; + } else { + // Put the characted as-is. + output.push(chr); + } + } + + if output == ident { + // While we didn't detect the correct case at the beginning, there + // may be special cases: e.g. `A` is both valid CamelCase and UPPER_SNAKE_CASE. + None + } else { + Some(output) + } +} + +/// Converts an identifier to a lower_snake_case form. +/// Returns `None` if the string is already in lower_snake_case. +pub fn to_lower_snake_case(ident: &str) -> Option { + // First, assume that it's UPPER_SNAKE_CASE. + match detect_case(ident) { + DetectedCase::LowerSnakeCase => return None, + DetectedCase::UpperSnakeCase => { + return Some(ident.chars().map(|chr| chr.to_ascii_lowercase()).collect()) + } + _ => {} + } + + // Otherwise, assume that it's CamelCase. + let lower_snake_case = stdx::to_lower_snake_case(ident); + + if lower_snake_case == ident { + // While we didn't detect the correct case at the beginning, there + // may be special cases: e.g. `a` is both valid camelCase and snake_case. + None + } else { + Some(lower_snake_case) + } +} + +/// Converts an identifier to an UPPER_SNAKE_CASE form. +/// Returns `None` if the string is already is UPPER_SNAKE_CASE. +pub fn to_upper_snake_case(ident: &str) -> Option { + match detect_case(ident) { + DetectedCase::UpperSnakeCase => return None, + DetectedCase::LowerSnakeCase => { + return Some(ident.chars().map(|chr| chr.to_ascii_uppercase()).collect()) + } + _ => {} + } + + // Normalize the string from whatever form it's in currently, and then just make it uppercase. + let upper_snake_case = stdx::to_upper_snake_case(ident); + + if upper_snake_case == ident { + // While we didn't detect the correct case at the beginning, there + // may be special cases: e.g. `A` is both valid CamelCase and UPPER_SNAKE_CASE. + None + } else { + Some(upper_snake_case) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use expect_test::{expect, Expect}; + + fn check Option>(fun: F, input: &str, expect: Expect) { + // `None` is translated to empty string, meaning that there is nothing to fix. + let output = fun(input).unwrap_or_default(); + + expect.assert_eq(&output); + } + + #[test] + fn test_to_lower_snake_case() { + check(to_lower_snake_case, "lower_snake_case", expect![[""]]); + check(to_lower_snake_case, "UPPER_SNAKE_CASE", expect![["upper_snake_case"]]); + check(to_lower_snake_case, "Weird_Case", expect![["weird_case"]]); + check(to_lower_snake_case, "CamelCase", expect![["camel_case"]]); + check(to_lower_snake_case, "lowerCamelCase", expect![["lower_camel_case"]]); + check(to_lower_snake_case, "a", expect![[""]]); + } + + #[test] + fn test_to_camel_case() { + check(to_camel_case, "CamelCase", expect![[""]]); + check(to_camel_case, "CamelCase_", expect![[""]]); + check(to_camel_case, "_CamelCase", expect![[""]]); + check(to_camel_case, "lowerCamelCase", expect![["LowerCamelCase"]]); + check(to_camel_case, "lower_snake_case", expect![["LowerSnakeCase"]]); + check(to_camel_case, "UPPER_SNAKE_CASE", expect![["UpperSnakeCase"]]); + check(to_camel_case, "Weird_Case", expect![["WeirdCase"]]); + check(to_camel_case, "name", expect![["Name"]]); + check(to_camel_case, "A", expect![[""]]); + } + + #[test] + fn test_to_upper_snake_case() { + check(to_upper_snake_case, "UPPER_SNAKE_CASE", expect![[""]]); + check(to_upper_snake_case, "lower_snake_case", expect![["LOWER_SNAKE_CASE"]]); + check(to_upper_snake_case, "Weird_Case", expect![["WEIRD_CASE"]]); + check(to_upper_snake_case, "CamelCase", expect![["CAMEL_CASE"]]); + check(to_upper_snake_case, "lowerCamelCase", expect![["LOWER_CAMEL_CASE"]]); + check(to_upper_snake_case, "A", expect![[""]]); + } +} diff --git a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs deleted file mode 100644 index 2e1468c4c..000000000 --- a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs +++ /dev/null @@ -1,195 +0,0 @@ -//! Functions for string case manipulation, such as detecting the identifier case, -//! and converting it into appropriate form. - -#[derive(Debug)] -enum DetectedCase { - LowerCamelCase, - UpperCamelCase, - LowerSnakeCase, - UpperSnakeCase, - Unknown, -} - -fn detect_case(ident: &str) -> DetectedCase { - let trimmed_ident = ident.trim_matches('_'); - let first_lowercase = trimmed_ident.starts_with(|chr: char| chr.is_ascii_lowercase()); - let mut has_lowercase = first_lowercase; - let mut has_uppercase = false; - let mut has_underscore = false; - - for chr in trimmed_ident.chars() { - if chr == '_' { - has_underscore = true; - } else if chr.is_ascii_uppercase() { - has_uppercase = true; - } else if chr.is_ascii_lowercase() { - has_lowercase = true; - } - } - - if has_uppercase { - if !has_lowercase { - DetectedCase::UpperSnakeCase - } else if !has_underscore { - if first_lowercase { - DetectedCase::LowerCamelCase - } else { - DetectedCase::UpperCamelCase - } - } else { - // It has uppercase, it has lowercase, it has underscore. - // No assumptions here - DetectedCase::Unknown - } - } else { - DetectedCase::LowerSnakeCase - } -} - -/// Converts an identifier to an UpperCamelCase form. -/// Returns `None` if the string is already is UpperCamelCase. -pub fn to_camel_case(ident: &str) -> Option { - let detected_case = detect_case(ident); - - match detected_case { - DetectedCase::UpperCamelCase => return None, - DetectedCase::LowerCamelCase => { - let mut first_capitalized = false; - let output = ident - .chars() - .map(|chr| { - if !first_capitalized && chr.is_ascii_lowercase() { - first_capitalized = true; - chr.to_ascii_uppercase() - } else { - chr - } - }) - .collect(); - return Some(output); - } - _ => {} - } - - let mut output = String::with_capacity(ident.len()); - - let mut capital_added = false; - for chr in ident.chars() { - if chr.is_alphabetic() { - if !capital_added { - output.push(chr.to_ascii_uppercase()); - capital_added = true; - } else { - output.push(chr.to_ascii_lowercase()); - } - } else if chr == '_' { - // Skip this character and make the next one capital. - capital_added = false; - } else { - // Put the characted as-is. - output.push(chr); - } - } - - if output == ident { - // While we didn't detect the correct case at the beginning, there - // may be special cases: e.g. `A` is both valid CamelCase and UPPER_SNAKE_CASE. - None - } else { - Some(output) - } -} - -/// Converts an identifier to a lower_snake_case form. -/// Returns `None` if the string is already in lower_snake_case. -pub fn to_lower_snake_case(ident: &str) -> Option { - // First, assume that it's UPPER_SNAKE_CASE. - match detect_case(ident) { - DetectedCase::LowerSnakeCase => return None, - DetectedCase::UpperSnakeCase => { - return Some(ident.chars().map(|chr| chr.to_ascii_lowercase()).collect()) - } - _ => {} - } - - // Otherwise, assume that it's CamelCase. - let lower_snake_case = stdx::to_lower_snake_case(ident); - - if lower_snake_case == ident { - // While we didn't detect the correct case at the beginning, there - // may be special cases: e.g. `a` is both valid camelCase and snake_case. - None - } else { - Some(lower_snake_case) - } -} - -/// Converts an identifier to an UPPER_SNAKE_CASE form. -/// Returns `None` if the string is already is UPPER_SNAKE_CASE. -pub fn to_upper_snake_case(ident: &str) -> Option { - match detect_case(ident) { - DetectedCase::UpperSnakeCase => return None, - DetectedCase::LowerSnakeCase => { - return Some(ident.chars().map(|chr| chr.to_ascii_uppercase()).collect()) - } - _ => {} - } - - // Normalize the string from whatever form it's in currently, and then just make it uppercase. - let upper_snake_case = - stdx::to_lower_snake_case(ident).chars().map(|c| c.to_ascii_uppercase()).collect(); - - if upper_snake_case == ident { - // While we didn't detect the correct case at the beginning, there - // may be special cases: e.g. `A` is both valid CamelCase and UPPER_SNAKE_CASE. - None - } else { - Some(upper_snake_case) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use expect_test::{expect, Expect}; - - fn check Option>(fun: F, input: &str, expect: Expect) { - // `None` is translated to empty string, meaning that there is nothing to fix. - let output = fun(input).unwrap_or_default(); - - expect.assert_eq(&output); - } - - #[test] - fn test_to_lower_snake_case() { - check(to_lower_snake_case, "lower_snake_case", expect![[""]]); - check(to_lower_snake_case, "UPPER_SNAKE_CASE", expect![["upper_snake_case"]]); - check(to_lower_snake_case, "Weird_Case", expect![["weird_case"]]); - check(to_lower_snake_case, "CamelCase", expect![["camel_case"]]); - check(to_lower_snake_case, "lowerCamelCase", expect![["lower_camel_case"]]); - check(to_lower_snake_case, "a", expect![[""]]); - } - - #[test] - fn test_to_camel_case() { - check(to_camel_case, "CamelCase", expect![[""]]); - check(to_camel_case, "CamelCase_", expect![[""]]); - check(to_camel_case, "_CamelCase", expect![[""]]); - check(to_camel_case, "lowerCamelCase", expect![["LowerCamelCase"]]); - check(to_camel_case, "lower_snake_case", expect![["LowerSnakeCase"]]); - check(to_camel_case, "UPPER_SNAKE_CASE", expect![["UpperSnakeCase"]]); - check(to_camel_case, "Weird_Case", expect![["WeirdCase"]]); - check(to_camel_case, "name", expect![["Name"]]); - check(to_camel_case, "A", expect![[""]]); - } - - #[test] - fn test_to_upper_snake_case() { - check(to_upper_snake_case, "UPPER_SNAKE_CASE", expect![[""]]); - check(to_upper_snake_case, "lower_snake_case", expect![["LOWER_SNAKE_CASE"]]); - check(to_upper_snake_case, "Weird_Case", expect![["WEIRD_CASE"]]); - check(to_upper_snake_case, "CamelCase", expect![["CAMEL_CASE"]]); - check(to_upper_snake_case, "lowerCamelCase", expect![["LOWER_CAMEL_CASE"]]); - check(to_upper_snake_case, "A", expect![[""]]); - } -} -- cgit v1.2.3 From 66cea8cbaa3320653e760e7b4ce839e055976acf Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Thu, 8 Oct 2020 08:40:30 +0300 Subject: Replace 'if let' with 'match' in decl_check.rs --- crates/hir_ty/src/diagnostics/decl_check.rs | 63 +++++++++++++++-------------- 1 file changed, 33 insertions(+), 30 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index 901ccc94f..1f9386b75 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -171,16 +171,17 @@ impl<'a, 'b> DeclValidator<'a, 'b> { // 1. Diagnostic for function name. if let Some(replacement) = fn_name_replacement { - let ast_ptr = if let Some(name) = fn_src.value.name() { - name - } 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, - fn_src - ); - return; + let ast_ptr = match fn_src.value.name() { + Some(name) => name, + None => { + // 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, + fn_src + ); + return; + } }; let diagnostic = IncorrectCase { @@ -359,16 +360,17 @@ impl<'a, 'b> DeclValidator<'a, 'b> { let struct_src = struct_loc.source(db.upcast()); if let Some(replacement) = struct_name_replacement { - let ast_ptr = if let Some(name) = struct_src.value.name() { - name - } 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 structure without a name: {:?}", - replacement, - struct_src - ); - return; + let ast_ptr = match struct_src.value.name() { + Some(name) => name, + None => { + // 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 structure without a name: {:?}", + replacement, + struct_src + ); + return; + } }; let diagnostic = IncorrectCase { @@ -486,16 +488,17 @@ impl<'a, 'b> DeclValidator<'a, 'b> { let enum_src = enum_loc.source(db.upcast()); if let Some(replacement) = enum_name_replacement { - let ast_ptr = if let Some(name) = enum_src.value.name() { - name - } 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 enum without a name: {:?}", - replacement, - enum_src - ); - return; + let ast_ptr = match enum_src.value.name() { + Some(name) => name, + None => { + // 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 enum without a name: {:?}", + replacement, + enum_src + ); + return; + } }; let diagnostic = IncorrectCase { -- cgit v1.2.3 From fb0ab9f7456018ff0bac628e05366f976c5af1a7 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Thu, 8 Oct 2020 09:27:38 +0300 Subject: Keep SyntaxNodePtr::range private --- crates/hir_ty/src/diagnostics/decl_check.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'crates/hir_ty/src/diagnostics') diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index 1f9386b75..f987636fe 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -213,12 +213,21 @@ impl<'a, 'b> DeclValidator<'a, 'b> { 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 { + let ast_ptr: ast::Name = loop { match fn_params_iter.next() { Some(element) if pat_equals_to_name(element.pat(), ¶m_to_rename.current_name) => { - break element.pat().unwrap() + if let ast::Pat::IdentPat(pat) = element.pat().unwrap() { + break pat.name().unwrap(); + } else { + // This is critical. If we consider this parameter the expected one, + // it **must** have a name. + panic!( + "Pattern {:?} equals to expected replacement {:?}, but has no name", + element, param_to_rename + ); + } } Some(_) => {} None => { -- cgit v1.2.3