Merge #6421

6421: Check for allow(..) attributes in case check diagnostic r=popzxc a=popzxc

Resolves #6348

This is not a full-fledged solution, as it doesn't looks up for parent elements (e.g. function -> module -> parent module -> crate root), but it does at least checks attributes of item being checked.
I played a bit with code, and it seems that implementing a proper solution (which will also check for `deny` / `warn` attributes overriding values for `allow`s from above).

So, this solution should fix all the macros which intentionally do "weird" naming and wrap it with `allow`, such as `lazy_static`.

cc @ArifRoktim 


Co-authored-by: Igor Aleksanov <[email protected]>

1 files changed, 96 insertions, 30 deletions

diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs
index f179c62b7..4b3e2fa8f 100644
--- a/crates/hir_ty/src/diagnostics/decl_check.rs
+++ b/crates/hir_ty/src/diagnostics/decl_check.rs
@@ -16,7 +16,7 @@ use hir_def::{
     adt::VariantData,
     expr::{Pat, PatId},
     src::HasSource,
-    AdtId, ConstId, EnumId, FunctionId, Lookup, ModuleDefId, StaticId, StructId,
+    AdtId, AttrDefId, ConstId, EnumId, FunctionId, Lookup, ModuleDefId, StaticId, StructId,
 };
 use hir_expand::{
     diagnostics::DiagnosticSink,
@@ -32,6 +32,12 @@ use crate::{
     diagnostics::{decl_check::case_conv::*, CaseType, IncorrectCase},
 };
 
+mod allow {
+    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> {
     owner: ModuleDefId,
     sink: &'a mut DiagnosticSink<'b>,
@@ -72,11 +78,29 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
         }
     }
 
+    /// 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 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.
+        // 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);
+        }
+
+        // Check whether non-snake case identifiers are allowed for this function.
+        if self.allowed(db, func.into(), allow::NON_SNAKE_CASE) {
+            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 {
@@ -89,7 +113,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
             None
         };
 
-        // 2. Check the param names.
+        // Check the param names.
         let mut fn_param_replacements = Vec::new();
 
         for pat_id in body.params.iter().cloned() {
@@ -111,7 +135,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
             }
         }
 
-        // 3. Check the patterns inside the function body.
+        // Check the patterns inside the function body.
         let mut pats_replacements = Vec::new();
 
         for (pat_idx, pat) in body.pats.iter() {
@@ -136,7 +160,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
             }
         }
 
-        // 4. If there is at least one element to spawn a warning on, go to the source map and generate a warning.
+        // 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,
@@ -144,12 +168,6 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
             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
@@ -169,7 +187,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.
+        // Diagnostic for function name.
         if let Some(replacement) = fn_name_replacement {
             let ast_ptr = match fn_src.value.name() {
                 Some(name) => name,
@@ -196,7 +214,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
             self.sink.push(diagnostic);
         }
 
-        // 2. Diagnostics for function params.
+        // Diagnostics for function params.
         let fn_params_list = match fn_src.value.param_list() {
             Some(params) => params,
             None => {
@@ -312,7 +330,11 @@ 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 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);
+
+        // 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 {
@@ -320,29 +342,35 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
                 suggested_text: new_name,
                 expected_case: CaseType::UpperCamelCase,
             };
-            Some(replacement)
+            if non_camel_case_allowed {
+                None
+            } else {
+                Some(replacement)
+            }
         } else {
             None
         };
 
-        // 2. Check the field names.
+        // 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);
+        if !non_snake_case_allowed {
+            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.
+        // 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,
@@ -442,7 +470,12 @@ 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.
+        // Check whether non-camel case names are allowed for this enum.
+        if self.allowed(db, enum_id.into(), allow::NON_CAMEL_CASE_TYPES) {
+            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 {
@@ -455,7 +488,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
             None
         };
 
-        // 2. Check the field names.
+        // Check the field names.
         let mut enum_fields_replacements = Vec::new();
 
         for (_, variant) in data.variants.iter() {
@@ -470,7 +503,7 @@ 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.
+        // 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,
@@ -572,6 +605,10 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
     fn validate_const(&mut self, db: &dyn HirDatabase, const_id: ConstId) {
         let data = db.const_data(const_id);
 
+        if self.allowed(db, const_id.into(), allow::NON_UPPER_CASE_GLOBAL) {
+            return;
+        }
+
         let name = match &data.name {
             Some(name) => name,
             None => return,
@@ -612,6 +649,10 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
     fn validate_static(&mut self, db: &dyn HirDatabase, static_id: StaticId) {
         let data = db.static_data(static_id);
 
+        if self.allowed(db, static_id.into(), allow::NON_UPPER_CASE_GLOBAL) {
+            return;
+        }
+
         let name = match &data.name {
             Some(name) => name,
             None => return,
@@ -854,4 +895,29 @@ fn main() {
 "#,
         );
     }
+
+    #[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;
+    "#,
+        );
+    }
 }