Create basic support for names case checks and implement function name case check

Author: popzxc

crates/hir/src/code_model.rs

@@ -255,6 +255,37 @@ impl ModuleDef {
             ModuleDef::BuiltinType(it) => Some(it.as_name()),
         }
     }
+
+    pub fn diagnostics(self, db: &dyn HirDatabase, sink: &mut DiagnosticSink) {
+        match self {
+            ModuleDef::Adt(it) => match it {
+                Adt::Struct(it) => {
+                    hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink)
+                }
+                Adt::Enum(it) => hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink),
+                Adt::Union(it) => hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink),
+            },
+            ModuleDef::Trait(it) => {
+                hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink)
+            }
+            ModuleDef::Function(it) => {
+                hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink)
+            }
+            ModuleDef::TypeAlias(it) => {
+                hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink)
+            }
+            ModuleDef::Module(it) => {
+                hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink)
+            }
+            ModuleDef::Const(it) => {
+                hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink)
+            }
+            ModuleDef::Static(it) => {
+                hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink)
+            }
+            _ => return,
+        }
+    }
 }
 
 pub use hir_def::{

crates/hir_def/src/data.rs

@@ -19,6 +19,7 @@ use crate::{
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct FunctionData {
     pub name: Name,
+    pub param_names: Vec<Option<Name>>,
     pub params: Vec<TypeRef>,
     pub ret_type: TypeRef,
     pub attrs: Attrs,
@@ -39,6 +40,7 @@ impl FunctionData {
 
         Arc::new(FunctionData {
             name: func.name.clone(),
+            param_names: func.param_names.to_vec(),
             params: func.params.to_vec(),
             ret_type: func.ret_type.clone(),
             attrs: item_tree.attrs(ModItem::from(loc.id.value).into()).clone(),

crates/hir_def/src/item_tree.rs

@@ -507,6 +507,8 @@ pub struct Function {
     pub has_self_param: bool,
     pub has_body: bool,
     pub is_unsafe: bool,
+    /// List of function parameters names. Does not include `self`.
+    pub param_names: Box<[Option<Name>]>,
     pub params: Box<[TypeRef]>,
     pub is_varargs: bool,
     pub ret_type: TypeRef,

crates/hir_def/src/item_tree/lower.rs

@@ -283,6 +283,7 @@ impl Ctx {
         let name = func.name()?.as_name();
 
         let mut params = Vec::new();
+        let mut param_names = Vec::new();
         let mut has_self_param = false;
         if let Some(param_list) = func.param_list() {
             if let Some(self_param) = param_list.self_param() {
@@ -305,6 +306,18 @@ impl Ctx {
                 has_self_param = true;
             }
             for param in param_list.params() {
+                let param_name = param
+                    .pat()
+                    .map(|name| {
+                        if let ast::Pat::IdentPat(ident) = name {
+                            Some(ident.name()?.as_name())
+                        } else {
+                            None
+                        }
+                    })
+                    .flatten();
+                param_names.push(param_name);
+
                 let type_ref = TypeRef::from_ast_opt(&self.body_ctx, param.ty());
                 params.push(type_ref);
             }
@@ -341,6 +354,7 @@ impl Ctx {
             has_body,
             is_unsafe: func.unsafe_token().is_some(),
             params: params.into_boxed_slice(),
+            param_names: param_names.into_boxed_slice(),
             is_varargs,
             ret_type,
             ast_id,

crates/hir_ty/src/diagnostics.rs

@@ -2,10 +2,11 @@
 mod expr;
 mod match_check;
 mod unsafe_check;
+mod decl_check;
 
-use std::any::Any;
+use std::{any::Any, fmt};
 
-use hir_def::DefWithBodyId;
+use hir_def::{DefWithBodyId, ModuleDefId};
 use hir_expand::diagnostics::{Diagnostic, DiagnosticCode, DiagnosticSink};
 use hir_expand::{name::Name, HirFileId, InFile};
 use stdx::format_to;
@@ -15,6 +16,16 @@ use crate::db::HirDatabase;
 
 pub use crate::diagnostics::expr::{record_literal_missing_fields, record_pattern_missing_fields};
 
+pub fn validate_module_item(
+    db: &dyn HirDatabase,
+    owner: ModuleDefId,
+    sink: &mut DiagnosticSink<'_>,
+) {
+    let _p = profile::span("validate_body");
+    let mut validator = decl_check::DeclValidator::new(owner, sink);
+    validator.validate_item(db);
+}
+
 pub fn validate_body(db: &dyn HirDatabase, owner: DefWithBodyId, sink: &mut DiagnosticSink<'_>) {
     let _p = profile::span("validate_body");
     let infer = db.infer(owner);
@@ -231,6 +242,64 @@ impl Diagnostic for MismatchedArgCount {
     }
 }
 
+#[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 => "UpperCamelCase",
+        };
+
+        write!(f, "{}", repr)
+    }
+}
+
+#[derive(Debug)]
+pub struct IncorrectCase {
+    pub file: HirFileId,
+    pub ident: SyntaxNodePtr,
+    pub expected_case: CaseType,
+    pub ident_text: String,
+    pub suggested_text: String,
+}
+
+impl Diagnostic for IncorrectCase {
+    fn code(&self) -> DiagnosticCode {
+        DiagnosticCode("incorrect-ident-case")
+    }
+
+    fn message(&self) -> String {
+        format!(
+            "Argument `{}` should have a {} name, e.g. `{}`",
+            self.ident_text,
+            self.expected_case.to_string(),
+            self.suggested_text
+        )
+    }
+
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
+        InFile::new(self.file, self.ident.clone())
+    }
+
+    fn as_any(&self) -> &(dyn Any + Send + 'static) {
+        self
+    }
+
+    fn is_experimental(&self) -> bool {
+        true
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use base_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt};
@@ -242,7 +311,10 @@ mod tests {
     use rustc_hash::FxHashMap;
     use syntax::{TextRange, TextSize};
 
-    use crate::{diagnostics::validate_body, test_db::TestDB};
+    use crate::{
+        diagnostics::{validate_body, validate_module_item},
+        test_db::TestDB,
+    };
 
     impl TestDB {
         fn diagnostics<F: FnMut(&dyn Diagnostic)>(&self, mut cb: F) {
@@ -253,6 +325,9 @@ mod tests {
                 let mut fns = Vec::new();
                 for (module_id, _) in crate_def_map.modules.iter() {
                     for decl in crate_def_map[module_id].scope.declarations() {
+                        let mut sink = DiagnosticSinkBuilder::new().build(&mut cb);
+                        validate_module_item(self, decl, &mut sink);
+
                         if let ModuleDefId::FunctionId(f) = decl {
                             fns.push(f)
                         }

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<Replacement>,
+        fn_param_replacements: Vec<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());
+
+        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<String> {
+    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`
+"#,
+        );
+    }
+}