]> git.lizzy.rs Git - rust.git/blobdiff - crates/hir_ty/src/diagnostics/decl_check.rs
Move incorrect case diagnostic things into their module
[rust.git] / crates / hir_ty / src / diagnostics / decl_check.rs
index 25587e1168630bc026257d619c5ac91452890e18..6a3588dc741a0cff736c4fc2ed2e2f93902f3518 100644 (file)
@@ -1,4 +1,4 @@
-//! Provides validators for the item declarations.
+//! Provides validators for names of declarations.
 //!
 //! This includes the following items:
 //!
@@ -12,6 +12,9 @@
 
 mod case_conv;
 
+use std::fmt;
+
+use base_db::CrateId;
 use hir_def::{
     adt::VariantData,
     expr::{Pat, PatId},
     AdtId, AttrDefId, ConstId, EnumId, FunctionId, Lookup, ModuleDefId, StaticId, StructId,
 };
 use hir_expand::{
-    diagnostics::DiagnosticSink,
     name::{AsName, Name},
+    HirFileId,
 };
+use stdx::{always, never};
 use syntax::{
-    ast::{self, NameOwner},
+    ast::{self, HasName},
     AstNode, AstPtr,
 };
-use test_utils::mark;
 
-use crate::{
-    db::HirDatabase,
-    diagnostics::{decl_check::case_conv::*, CaseType, IncorrectCase},
-};
+use crate::db::HirDatabase;
+
+use self::case_conv::{to_camel_case, to_lower_snake_case, to_upper_snake_case};
 
 mod allow {
+    pub(super) const BAD_STYLE: &str = "bad_style";
+    pub(super) const NONSTANDARD_STYLE: &str = "nonstandard_style";
     pub(super) const NON_SNAKE_CASE: &str = "non_snake_case";
     pub(super) const NON_UPPER_CASE_GLOBAL: &str = "non_upper_case_globals";
     pub(super) const NON_CAMEL_CASE_TYPES: &str = "non_camel_case_types";
 }
 
-pub(super) struct DeclValidator<'a, 'b: 'a> {
+pub fn incorrect_case(
+    db: &dyn HirDatabase,
+    krate: CrateId,
     owner: ModuleDefId,
-    sink: &'a mut DiagnosticSink<'b>,
+) -> Vec<IncorrectCase> {
+    let _p = profile::span("validate_module_item");
+    let mut validator = DeclValidator::new(db, krate);
+    validator.validate_item(owner);
+    validator.sink
+}
+
+#[derive(Debug)]
+pub enum CaseType {
+    // `some_var`
+    LowerSnakeCase,
+    // `SOME_CONST`
+    UpperSnakeCase,
+    // `SomeStruct`
+    UpperCamelCase,
+}
+
+impl fmt::Display for CaseType {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let repr = match self {
+            CaseType::LowerSnakeCase => "snake_case",
+            CaseType::UpperSnakeCase => "UPPER_SNAKE_CASE",
+            CaseType::UpperCamelCase => "CamelCase",
+        };
+
+        write!(f, "{}", repr)
+    }
+}
+
+#[derive(Debug)]
+pub enum IdentType {
+    Constant,
+    Enum,
+    Field,
+    Function,
+    Parameter,
+    StaticVariable,
+    Structure,
+    Variable,
+    Variant,
+}
+
+impl fmt::Display for IdentType {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let repr = match self {
+            IdentType::Constant => "Constant",
+            IdentType::Enum => "Enum",
+            IdentType::Field => "Field",
+            IdentType::Function => "Function",
+            IdentType::Parameter => "Parameter",
+            IdentType::StaticVariable => "Static variable",
+            IdentType::Structure => "Structure",
+            IdentType::Variable => "Variable",
+            IdentType::Variant => "Variant",
+        };
+
+        write!(f, "{}", repr)
+    }
+}
+
+#[derive(Debug)]
+pub struct IncorrectCase {
+    pub file: HirFileId,
+    pub ident: AstPtr<ast::Name>,
+    pub expected_case: CaseType,
+    pub ident_type: IdentType,
+    pub ident_text: String,
+    pub suggested_text: String,
+}
+
+pub(super) struct DeclValidator<'a> {
+    db: &'a dyn HirDatabase,
+    krate: CrateId,
+    pub(super) sink: Vec<IncorrectCase>,
 }
 
 #[derive(Debug)]
@@ -51,129 +130,126 @@ struct Replacement {
     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 }
+impl<'a> DeclValidator<'a> {
+    pub(super) fn new(db: &'a dyn HirDatabase, krate: CrateId) -> DeclValidator<'a> {
+        DeclValidator { db, krate, sink: Vec::new() }
     }
 
-    pub(super) fn validate_item(&mut self, db: &dyn HirDatabase) {
-        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,
+    pub(super) fn validate_item(&mut self, item: ModuleDefId) {
+        match item {
+            ModuleDefId::FunctionId(func) => self.validate_func(func),
+            ModuleDefId::AdtId(adt) => self.validate_adt(adt),
+            ModuleDefId::ConstId(const_id) => self.validate_const(const_id),
+            ModuleDefId::StaticId(static_id) => self.validate_static(static_id),
+            _ => (),
         }
     }
 
-    fn validate_adt(&mut self, db: &dyn HirDatabase, adt: AdtId) {
+    fn validate_adt(&mut self, 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::StructId(struct_id) => self.validate_struct(struct_id),
+            AdtId::EnumId(enum_id) => self.validate_enum(enum_id),
             AdtId::UnionId(_) => {
-                // Unions aren't yet supported by this validator.
+                // FIXME: Unions aren't yet supported by this validator.
             }
         }
     }
 
     /// Checks whether not following the convention is allowed for this item.
-    ///
-    /// Currently this method doesn't check parent attributes.
-    fn allowed(&self, db: &dyn HirDatabase, id: AttrDefId, allow_name: &str) -> bool {
-        db.attrs(id).by_key("allow").tt_values().any(|tt| tt.to_string().contains(allow_name))
+    fn allowed(&self, id: AttrDefId, allow_name: &str, recursing: bool) -> bool {
+        let is_allowed = |def_id| {
+            let attrs = self.db.attrs(def_id);
+            // don't bug the user about directly no_mangle annotated stuff, they can't do anything about it
+            (!recursing && attrs.by_key("no_mangle").exists())
+                || attrs.by_key("allow").tt_values().any(|tt| {
+                    let allows = tt.to_string();
+                    allows.contains(allow_name)
+                        || allows.contains(allow::BAD_STYLE)
+                        || allows.contains(allow::NONSTANDARD_STYLE)
+                })
+        };
+
+        is_allowed(id)
+            // go upwards one step or give up
+            || match id {
+                AttrDefId::ModuleId(m) => m.containing_module(self.db.upcast()).map(|v| v.into()),
+                AttrDefId::FunctionId(f) => Some(f.lookup(self.db.upcast()).container.into()),
+                AttrDefId::StaticId(sid) => Some(sid.lookup(self.db.upcast()).container.into()),
+                AttrDefId::ConstId(cid) => Some(cid.lookup(self.db.upcast()).container.into()),
+                AttrDefId::TraitId(tid) => Some(tid.lookup(self.db.upcast()).container.into()),
+                AttrDefId::ImplId(iid) => Some(iid.lookup(self.db.upcast()).container.into()),
+                // These warnings should not explore macro definitions at all
+                AttrDefId::MacroDefId(_) => None,
+                // Will never occur under an enum/struct/union/type alias
+                AttrDefId::AdtId(_) => None,
+                AttrDefId::FieldId(_) => None,
+                AttrDefId::EnumVariantId(_) => None,
+                AttrDefId::TypeAliasId(_) => None,
+                AttrDefId::GenericParamId(_) => None,
+            }
+            .map(|mid| self.allowed(mid, allow_name, true))
+            .unwrap_or(false)
     }
 
-    fn validate_func(&mut self, db: &dyn HirDatabase, func: FunctionId) {
-        let data = db.function_data(func);
-        if data.is_extern {
-            mark::hit!(extern_func_incorrect_case_ignored);
+    fn validate_func(&mut self, func: FunctionId) {
+        let data = self.db.function_data(func);
+        if data.is_in_extern_block() {
+            cov_mark::hit!(extern_func_incorrect_case_ignored);
             return;
         }
 
-        let body = db.body(func.into());
+        let body = self.db.body(func.into());
 
         // 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);
+        for (_, block_def_map) in body.blocks(self.db.upcast()) {
+            for (_, module) in block_def_map.modules() {
+                for def_id in module.scope.declarations() {
+                    let mut validator = DeclValidator::new(self.db, self.krate);
+                    validator.validate_item(def_id);
+                }
+            }
         }
 
         // Check whether non-snake case identifiers are allowed for this function.
-        if self.allowed(db, func.into(), allow::NON_SNAKE_CASE) {
+        if self.allowed(func.into(), allow::NON_SNAKE_CASE, false) {
             return;
         }
 
         // 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
-        };
-
-        // Check the param names.
-        let mut fn_param_replacements = Vec::new();
-
-        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.clone(),
-                    suggested_text: new_name,
-                    expected_case: CaseType::LowerSnakeCase,
-                };
-                fn_param_replacements.push(replacement);
-            }
-        }
+        let fn_name_replacement = to_lower_snake_case(&function_name).map(|new_name| Replacement {
+            current_name: data.name.clone(),
+            suggested_text: new_name,
+            expected_case: CaseType::LowerSnakeCase,
+        });
 
         // Check the patterns inside the function body.
-        let mut pats_replacements = Vec::new();
+        // This includes function parameters.
+        let pats_replacements = body
+            .pats
+            .iter()
+            .filter_map(|(id, pat)| match pat {
+                Pat::Bind { name, .. } => Some((id, name)),
+                _ => None,
+            })
+            .filter_map(|(id, bind_name)| {
+                Some((
+                    id,
+                    Replacement {
+                        current_name: bind_name.clone(),
+                        suggested_text: to_lower_snake_case(&bind_name.to_string())?,
+                        expected_case: CaseType::LowerSnakeCase,
+                    },
+                ))
+            })
+            .collect();
 
-        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));
-            }
+        // If there is at least one element to spawn a warning on, go to the source map and generate a warning.
+        if let Some(fn_name_replacement) = fn_name_replacement {
+            self.create_incorrect_case_diagnostic_for_func(func, fn_name_replacement);
         }
 
-        // 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);
+        self.create_incorrect_case_diagnostic_for_variables(func, pats_replacements);
     }
 
     /// Given the information about incorrect names in the function declaration, looks up into the source code
@@ -181,100 +257,34 @@ fn validate_func(&mut self, db: &dyn HirDatabase, func: FunctionId) {
     fn create_incorrect_case_diagnostic_for_func(
         &mut self,
         func: FunctionId,
-        db: &dyn HirDatabase,
-        fn_name_replacement: Option<Replacement>,
-        fn_param_replacements: Vec<Replacement>,
+        fn_name_replacement: Replacement,
     ) {
-        // 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());
+        let fn_loc = func.lookup(self.db.upcast());
+        let fn_src = fn_loc.source(self.db.upcast());
 
         // Diagnostic for function name.
-        if let Some(replacement) = fn_name_replacement {
-            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 {
-                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(),
-                suggested_text: replacement.suggested_text,
-            };
-
-            self.sink.push(diagnostic);
-        }
-
-        // Diagnostics for function params.
-        let fn_params_list = match fn_src.value.param_list() {
-            Some(params) => params,
+        let ast_ptr = match fn_src.value.name() {
+            Some(name) => name,
             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
-                    );
-                }
+                never!(
+                    "Replacement ({:?}) was generated for a function without a name: {:?}",
+                    fn_name_replacement,
+                    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: ast::Name = loop {
-                match fn_params_iter.next() {
-                    Some(element)
-                        if pat_equals_to_name(element.pat(), &param_to_rename.current_name) =>
-                    {
-                        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 => {
-                        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,
-            };
+        let diagnostic = IncorrectCase {
+            file: fn_src.file_id,
+            ident_type: IdentType::Function,
+            ident: AstPtr::new(&ast_ptr),
+            expected_case: fn_name_replacement.expected_case,
+            ident_text: fn_name_replacement.current_name.to_string(),
+            suggested_text: fn_name_replacement.suggested_text,
+        };
 
-            self.sink.push(diagnostic);
-        }
+        self.sink.push(diagnostic);
     }
 
     /// Given the information about incorrect variable names, looks up into the source code
@@ -282,7 +292,6 @@ fn create_incorrect_case_diagnostic_for_func(
     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
@@ -290,12 +299,12 @@ fn create_incorrect_case_diagnostic_for_variables(
             return;
         }
 
-        let (_, source_map) = db.body_with_source_map(func.into());
+        let (_, source_map) = self.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());
+                    let root = source_ptr.file_syntax(self.db.upcast());
                     if let ast::Pat::IdentPat(ident_pat) = expr.to_node(&root) {
                         let parent = match ident_pat.syntax().parent() {
                             Some(parent) => parent,
@@ -306,21 +315,26 @@ fn create_incorrect_case_diagnostic_for_variables(
                             None => continue,
                         };
 
+                        let is_param = ast::Param::can_cast(parent.kind());
+
                         // 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.
-                        let is_binding = ast::LetStmt::cast(parent.clone()).is_some()
-                            || (ast::MatchArm::cast(parent).is_some()
+                        let is_binding = ast::LetStmt::can_cast(parent.kind())
+                            || (ast::MatchArm::can_cast(parent.kind())
                                 && ident_pat.at_token().is_some());
-                        if !is_binding {
+                        if !(is_param || is_binding) {
                             // This pattern is not an actual variable declaration, e.g. `Some(val) => {..}` match arm.
                             continue;
                         }
 
+                        let ident_type =
+                            if is_param { IdentType::Parameter } else { IdentType::Variable };
+
                         let diagnostic = IncorrectCase {
                             file: source_ptr.file_id,
-                            ident_type: "Variable".to_string(),
-                            ident: AstPtr::new(&name_ast).into(),
+                            ident_type,
+                            ident: AstPtr::new(&name_ast),
                             expected_case: replacement.expected_case,
                             ident_text: replacement.current_name.to_string(),
                             suggested_text: replacement.suggested_text,
@@ -333,26 +347,21 @@ fn create_incorrect_case_diagnostic_for_variables(
         }
     }
 
-    fn validate_struct(&mut self, db: &dyn HirDatabase, struct_id: StructId) {
-        let data = db.struct_data(struct_id);
+    fn validate_struct(&mut self, struct_id: StructId) {
+        let data = self.db.struct_data(struct_id);
 
         let non_camel_case_allowed =
-            self.allowed(db, struct_id.into(), allow::NON_CAMEL_CASE_TYPES);
-        let non_snake_case_allowed = self.allowed(db, struct_id.into(), allow::NON_SNAKE_CASE);
+            self.allowed(struct_id.into(), allow::NON_CAMEL_CASE_TYPES, false);
+        let non_snake_case_allowed = self.allowed(struct_id.into(), allow::NON_SNAKE_CASE, false);
 
         // 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 {
+        let struct_name_replacement = if !non_camel_case_allowed {
+            to_camel_case(&struct_name).map(|new_name| Replacement {
                 current_name: data.name.clone(),
                 suggested_text: new_name,
                 expected_case: CaseType::UpperCamelCase,
-            };
-            if non_camel_case_allowed {
-                None
-            } else {
-                Some(replacement)
-            }
+            })
         } else {
             None
         };
@@ -379,7 +388,6 @@ fn validate_struct(&mut self, db: &dyn HirDatabase, struct_id: StructId) {
         // 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,
         );
@@ -390,24 +398,22 @@ fn validate_struct(&mut self, db: &dyn HirDatabase, struct_id: StructId) {
     fn create_incorrect_case_diagnostic_for_struct(
         &mut self,
         struct_id: StructId,
-        db: &dyn HirDatabase,
         struct_name_replacement: Option<Replacement>,
         struct_fields_replacements: Vec<Replacement>,
     ) {
-        // XXX: only look at sources if we do have incorrect names
+        // 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());
+        let struct_loc = struct_id.lookup(self.db.upcast());
+        let struct_src = struct_loc.source(self.db.upcast());
 
         if let Some(replacement) = struct_name_replacement {
             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!(
+                    never!(
                         "Replacement ({:?}) was generated for a structure without a name: {:?}",
                         replacement,
                         struct_src
@@ -418,8 +424,8 @@ fn create_incorrect_case_diagnostic_for_struct(
 
             let diagnostic = IncorrectCase {
                 file: struct_src.file_id,
-                ident_type: "Structure".to_string(),
-                ident: AstPtr::new(&ast_ptr).into(),
+                ident_type: IdentType::Structure,
+                ident: AstPtr::new(&ast_ptr),
                 expected_case: replacement.expected_case,
                 ident_text: replacement.current_name.to_string(),
                 suggested_text: replacement.suggested_text,
@@ -431,12 +437,12 @@ fn create_incorrect_case_diagnostic_for_struct(
         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
-                    );
-                }
+                always!(
+                    struct_fields_replacements.is_empty(),
+                    "Replacements ({:?}) were generated for a structure fields which had no fields list: {:?}",
+                    struct_fields_replacements,
+                    struct_src
+                );
                 return;
             }
         };
@@ -445,13 +451,14 @@ fn create_incorrect_case_diagnostic_for_struct(
             // 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()
+                match struct_fields_iter.next().and_then(|field| field.name()) {
+                    Some(field_name) => {
+                        if field_name.as_name() == field_to_rename.current_name {
+                            break field_name;
+                        }
                     }
-                    Some(_) => {}
                     None => {
-                        log::error!(
+                        never!(
                             "Replacement ({:?}) was generated for a structure field which was not found: {:?}",
                             field_to_rename, struct_src
                         );
@@ -462,8 +469,8 @@ fn create_incorrect_case_diagnostic_for_struct(
 
             let diagnostic = IncorrectCase {
                 file: struct_src.file_id,
-                ident_type: "Field".to_string(),
-                ident: AstPtr::new(&ast_ptr).into(),
+                ident_type: IdentType::Field,
+                ident: AstPtr::new(&ast_ptr),
                 expected_case: field_to_rename.expected_case,
                 ident_text: field_to_rename.current_name.to_string(),
                 suggested_text: field_to_rename.suggested_text,
@@ -473,46 +480,38 @@ fn create_incorrect_case_diagnostic_for_struct(
         }
     }
 
-    fn validate_enum(&mut self, db: &dyn HirDatabase, enum_id: EnumId) {
-        let data = db.enum_data(enum_id);
+    fn validate_enum(&mut self, enum_id: EnumId) {
+        let data = self.db.enum_data(enum_id);
 
         // Check whether non-camel case names are allowed for this enum.
-        if self.allowed(db, enum_id.into(), allow::NON_CAMEL_CASE_TYPES) {
+        if self.allowed(enum_id.into(), allow::NON_CAMEL_CASE_TYPES, false) {
             return;
         }
 
         // 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
-        };
+        let enum_name_replacement = to_camel_case(&enum_name).map(|new_name| Replacement {
+            current_name: data.name.clone(),
+            suggested_text: new_name,
+            expected_case: CaseType::UpperCamelCase,
+        });
 
         // 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 {
+        let enum_fields_replacements = data
+            .variants
+            .iter()
+            .filter_map(|(_, variant)| {
+                Some(Replacement {
                     current_name: variant.name.clone(),
-                    suggested_text: new_name,
+                    suggested_text: to_camel_case(&variant.name.to_string())?,
                     expected_case: CaseType::UpperCamelCase,
-                };
-                enum_fields_replacements.push(replacement);
-            }
-        }
+                })
+            })
+            .collect();
 
         // 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,
         )
@@ -523,7 +522,6 @@ fn validate_enum(&mut self, db: &dyn HirDatabase, enum_id: EnumId) {
     fn create_incorrect_case_diagnostic_for_enum(
         &mut self,
         enum_id: EnumId,
-        db: &dyn HirDatabase,
         enum_name_replacement: Option<Replacement>,
         enum_variants_replacements: Vec<Replacement>,
     ) {
@@ -532,15 +530,14 @@ fn create_incorrect_case_diagnostic_for_enum(
             return;
         }
 
-        let enum_loc = enum_id.lookup(db.upcast());
-        let enum_src = enum_loc.source(db.upcast());
+        let enum_loc = enum_id.lookup(self.db.upcast());
+        let enum_src = enum_loc.source(self.db.upcast());
 
         if let Some(replacement) = enum_name_replacement {
             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!(
+                    never!(
                         "Replacement ({:?}) was generated for a enum without a name: {:?}",
                         replacement,
                         enum_src
@@ -551,8 +548,8 @@ fn create_incorrect_case_diagnostic_for_enum(
 
             let diagnostic = IncorrectCase {
                 file: enum_src.file_id,
-                ident_type: "Enum".to_string(),
-                ident: AstPtr::new(&ast_ptr).into(),
+                ident_type: IdentType::Enum,
+                ident: AstPtr::new(&ast_ptr),
                 expected_case: replacement.expected_case,
                 ident_text: replacement.current_name.to_string(),
                 suggested_text: replacement.suggested_text,
@@ -564,12 +561,12 @@ fn create_incorrect_case_diagnostic_for_enum(
         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
-                    );
-                }
+                always!(
+                    enum_variants_replacements.is_empty(),
+                    "Replacements ({:?}) were generated for a enum variants which had no fields list: {:?}",
+                    enum_variants_replacements,
+                    enum_src
+                );
                 return;
             }
         };
@@ -578,15 +575,14 @@ fn create_incorrect_case_diagnostic_for_enum(
             // 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()
+                match enum_variants_iter.next().and_then(|v| v.name()) {
+                    Some(variant_name) => {
+                        if variant_name.as_name() == variant_to_rename.current_name {
+                            break variant_name;
+                        }
                     }
-                    Some(_) => {}
                     None => {
-                        log::error!(
+                        never!(
                             "Replacement ({:?}) was generated for a enum variant which was not found: {:?}",
                             variant_to_rename, enum_src
                         );
@@ -597,8 +593,8 @@ fn create_incorrect_case_diagnostic_for_enum(
 
             let diagnostic = IncorrectCase {
                 file: enum_src.file_id,
-                ident_type: "Variant".to_string(),
-                ident: AstPtr::new(&ast_ptr).into(),
+                ident_type: IdentType::Variant,
+                ident: AstPtr::new(&ast_ptr),
                 expected_case: variant_to_rename.expected_case,
                 ident_text: variant_to_rename.current_name.to_string(),
                 suggested_text: variant_to_rename.suggested_text,
@@ -608,10 +604,10 @@ fn create_incorrect_case_diagnostic_for_enum(
         }
     }
 
-    fn validate_const(&mut self, db: &dyn HirDatabase, const_id: ConstId) {
-        let data = db.const_data(const_id);
+    fn validate_const(&mut self, const_id: ConstId) {
+        let data = self.db.const_data(const_id);
 
-        if self.allowed(db, const_id.into(), allow::NON_UPPER_CASE_GLOBAL) {
+        if self.allowed(const_id.into(), allow::NON_UPPER_CASE_GLOBAL, false) {
             return;
         }
 
@@ -632,8 +628,8 @@ fn validate_const(&mut self, db: &dyn HirDatabase, const_id: ConstId) {
             return;
         };
 
-        let const_loc = const_id.lookup(db.upcast());
-        let const_src = const_loc.source(db.upcast());
+        let const_loc = const_id.lookup(self.db.upcast());
+        let const_src = const_loc.source(self.db.upcast());
 
         let ast_ptr = match const_src.value.name() {
             Some(name) => name,
@@ -642,8 +638,8 @@ fn validate_const(&mut self, db: &dyn HirDatabase, const_id: ConstId) {
 
         let diagnostic = IncorrectCase {
             file: const_src.file_id,
-            ident_type: "Constant".to_string(),
-            ident: AstPtr::new(&ast_ptr).into(),
+            ident_type: IdentType::Constant,
+            ident: AstPtr::new(&ast_ptr),
             expected_case: replacement.expected_case,
             ident_text: replacement.current_name.to_string(),
             suggested_text: replacement.suggested_text,
@@ -652,21 +648,18 @@ fn validate_const(&mut self, db: &dyn HirDatabase, const_id: ConstId) {
         self.sink.push(diagnostic);
     }
 
-    fn validate_static(&mut self, db: &dyn HirDatabase, static_id: StaticId) {
-        let data = db.static_data(static_id);
+    fn validate_static(&mut self, static_id: StaticId) {
+        let data = self.db.static_data(static_id);
         if data.is_extern {
-            mark::hit!(extern_static_incorrect_case_ignored);
+            cov_mark::hit!(extern_static_incorrect_case_ignored);
             return;
         }
 
-        if self.allowed(db, static_id.into(), allow::NON_UPPER_CASE_GLOBAL) {
+        if self.allowed(static_id.into(), allow::NON_UPPER_CASE_GLOBAL, false) {
             return;
         }
 
-        let name = match &data.name {
-            Some(name) => name,
-            None => return,
-        };
+        let name = &data.name;
 
         let static_name = name.to_string();
         let replacement = if let Some(new_name) = to_upper_snake_case(&static_name) {
@@ -680,8 +673,8 @@ fn validate_static(&mut self, db: &dyn HirDatabase, static_id: StaticId) {
             return;
         };
 
-        let static_loc = static_id.lookup(db.upcast());
-        let static_src = static_loc.source(db.upcast());
+        let static_loc = static_id.lookup(self.db.upcast());
+        let static_src = static_loc.source(self.db.upcast());
 
         let ast_ptr = match static_src.value.name() {
             Some(name) => name,
@@ -690,8 +683,8 @@ fn validate_static(&mut self, db: &dyn HirDatabase, static_id: StaticId) {
 
         let diagnostic = IncorrectCase {
             file: static_src.file_id,
-            ident_type: "Static variable".to_string(),
-            ident: AstPtr::new(&ast_ptr).into(),
+            ident_type: IdentType::StaticVariable,
+            ident: AstPtr::new(&ast_ptr),
             expected_case: replacement.expected_case,
             ident_text: replacement.current_name.to_string(),
             suggested_text: replacement.suggested_text,
@@ -700,250 +693,3 @@ fn validate_static(&mut self, db: &dyn HirDatabase, static_id: StaticId) {
         self.sink.push(diagnostic);
     }
 }
-
-fn names_equal(left: Option<ast::Name>, right: &Name) -> bool {
-    if let Some(left) = left {
-        &left.as_name() == right
-    } else {
-        false
-    }
-}
-
-fn pat_equals_to_name(pat: Option<ast::Pat>, name: &Name) -> bool {
-    if let Some(ast::Pat::IdentPat(ident)) = pat {
-        ident.to_string() == name.to_string()
-    } else {
-        false
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use test_utils::mark;
-
-    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) {}
-    // ^^^^^^^^^ Argument `SomeParam` should have snake_case name, e.g. `some_param`
-
-fn foo2(ok_param: &str, CAPS_PARAM: u8) {}
-                     // ^^^^^^^^^^ 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 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`
-
-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`
-    }
-}
-"#,
-        );
-    }
-
-    #[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{
-        let OtherVar = SOME_VAR + 1;
-        OtherVar
-    }
-
-    #[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 ignores_extern_items() {
-        mark::check!(extern_func_incorrect_case_ignored);
-        mark::check!(extern_static_incorrect_case_ignored);
-        check_diagnostics(
-            r#"
-extern {
-    fn NonSnakeCaseName(SOME_VAR: u8) -> u8;
-    pub static SomeStatic: u8 = 10;
-}
-            "#,
-        );
-    }
-}