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

This commit is contained in:
Igor Aleksanov 2020-10-03 12:48:02 +03:00
parent 518f6d7724
commit 4039176ec6
6 changed files with 300 additions and 3 deletions

View File

@ -255,6 +255,37 @@ pub fn name(self, db: &dyn HirDatabase) -> Option<Name> {
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::{

View File

@ -19,6 +19,7 @@
#[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 @@ pub(crate) fn fn_data_query(db: &dyn DefDatabase, func: FunctionId) -> Arc<Funct
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(),

View File

@ -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,

View File

@ -283,6 +283,7 @@ fn lower_function(&mut self, func: &ast::Fn) -> Option<FileItemTreeId<Function>>
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 @@ fn lower_function(&mut self, func: &ast::Fn) -> Option<FileItemTreeId<Function>>
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 @@ fn lower_function(&mut self, func: &ast::Fn) -> Option<FileItemTreeId<Function>>
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,

View File

@ -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 @@
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 @@ fn is_experimental(&self) -> bool {
}
}
#[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 @@ fn diagnostics<F: FnMut(&dyn Diagnostic)>(&self, mut cb: F) {
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)
}

View File

@ -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`
"#,
);
}
}