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.rs | 81 ++++++++++++- crates/hir_ty/src/diagnostics/decl_check.rs | 173 ++++++++++++++++++++++++++++ 2 files changed, 251 insertions(+), 3 deletions(-) create mode 100644 crates/hir_ty/src/diagnostics/decl_check.rs (limited to 'crates/hir_ty') diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs index 9ba005fab..7227e7010 100644 --- a/crates/hir_ty/src/diagnostics.rs +++ b/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 { + 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(&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) } 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