diff options
author | Igor Aleksanov <[email protected]> | 2020-10-03 10:48:02 +0100 |
---|---|---|
committer | Igor Aleksanov <[email protected]> | 2020-10-12 08:59:54 +0100 |
commit | 4039176ec63e5c75d76398f2debe26ac6fa59cbc (patch) | |
tree | 8f2f2b6d22c57985fc6a8f1b40d84663d40b09f6 | |
parent | 518f6d772482c7c58e59081f340947087a9b4800 (diff) |
Create basic support for names case checks and implement function name case check
-rw-r--r-- | crates/hir/src/code_model.rs | 31 | ||||
-rw-r--r-- | crates/hir_def/src/data.rs | 2 | ||||
-rw-r--r-- | crates/hir_def/src/item_tree.rs | 2 | ||||
-rw-r--r-- | crates/hir_def/src/item_tree/lower.rs | 14 | ||||
-rw-r--r-- | crates/hir_ty/src/diagnostics.rs | 81 | ||||
-rw-r--r-- | crates/hir_ty/src/diagnostics/decl_check.rs | 173 |
6 files changed, 300 insertions, 3 deletions
diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 650b4fa40..19ea26e36 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs | |||
@@ -255,6 +255,37 @@ impl ModuleDef { | |||
255 | ModuleDef::BuiltinType(it) => Some(it.as_name()), | 255 | ModuleDef::BuiltinType(it) => Some(it.as_name()), |
256 | } | 256 | } |
257 | } | 257 | } |
258 | |||
259 | pub fn diagnostics(self, db: &dyn HirDatabase, sink: &mut DiagnosticSink) { | ||
260 | match self { | ||
261 | ModuleDef::Adt(it) => match it { | ||
262 | Adt::Struct(it) => { | ||
263 | hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink) | ||
264 | } | ||
265 | Adt::Enum(it) => hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink), | ||
266 | Adt::Union(it) => hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink), | ||
267 | }, | ||
268 | ModuleDef::Trait(it) => { | ||
269 | hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink) | ||
270 | } | ||
271 | ModuleDef::Function(it) => { | ||
272 | hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink) | ||
273 | } | ||
274 | ModuleDef::TypeAlias(it) => { | ||
275 | hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink) | ||
276 | } | ||
277 | ModuleDef::Module(it) => { | ||
278 | hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink) | ||
279 | } | ||
280 | ModuleDef::Const(it) => { | ||
281 | hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink) | ||
282 | } | ||
283 | ModuleDef::Static(it) => { | ||
284 | hir_ty::diagnostics::validate_module_item(db, it.id.into(), sink) | ||
285 | } | ||
286 | _ => return, | ||
287 | } | ||
288 | } | ||
258 | } | 289 | } |
259 | 290 | ||
260 | pub use hir_def::{ | 291 | pub use hir_def::{ |
diff --git a/crates/hir_def/src/data.rs b/crates/hir_def/src/data.rs index ff1ef0df6..733db2eac 100644 --- a/crates/hir_def/src/data.rs +++ b/crates/hir_def/src/data.rs | |||
@@ -19,6 +19,7 @@ use crate::{ | |||
19 | #[derive(Debug, Clone, PartialEq, Eq)] | 19 | #[derive(Debug, Clone, PartialEq, Eq)] |
20 | pub struct FunctionData { | 20 | pub struct FunctionData { |
21 | pub name: Name, | 21 | pub name: Name, |
22 | pub param_names: Vec<Option<Name>>, | ||
22 | pub params: Vec<TypeRef>, | 23 | pub params: Vec<TypeRef>, |
23 | pub ret_type: TypeRef, | 24 | pub ret_type: TypeRef, |
24 | pub attrs: Attrs, | 25 | pub attrs: Attrs, |
@@ -39,6 +40,7 @@ impl FunctionData { | |||
39 | 40 | ||
40 | Arc::new(FunctionData { | 41 | Arc::new(FunctionData { |
41 | name: func.name.clone(), | 42 | name: func.name.clone(), |
43 | param_names: func.param_names.to_vec(), | ||
42 | params: func.params.to_vec(), | 44 | params: func.params.to_vec(), |
43 | ret_type: func.ret_type.clone(), | 45 | ret_type: func.ret_type.clone(), |
44 | attrs: item_tree.attrs(ModItem::from(loc.id.value).into()).clone(), | 46 | attrs: item_tree.attrs(ModItem::from(loc.id.value).into()).clone(), |
diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs index 8a1121bbd..ca502ce2b 100644 --- a/crates/hir_def/src/item_tree.rs +++ b/crates/hir_def/src/item_tree.rs | |||
@@ -507,6 +507,8 @@ pub struct Function { | |||
507 | pub has_self_param: bool, | 507 | pub has_self_param: bool, |
508 | pub has_body: bool, | 508 | pub has_body: bool, |
509 | pub is_unsafe: bool, | 509 | pub is_unsafe: bool, |
510 | /// List of function parameters names. Does not include `self`. | ||
511 | pub param_names: Box<[Option<Name>]>, | ||
510 | pub params: Box<[TypeRef]>, | 512 | pub params: Box<[TypeRef]>, |
511 | pub is_varargs: bool, | 513 | pub is_varargs: bool, |
512 | pub ret_type: TypeRef, | 514 | pub ret_type: TypeRef, |
diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs index 3328639cf..2ffa46ac0 100644 --- a/crates/hir_def/src/item_tree/lower.rs +++ b/crates/hir_def/src/item_tree/lower.rs | |||
@@ -283,6 +283,7 @@ impl Ctx { | |||
283 | let name = func.name()?.as_name(); | 283 | let name = func.name()?.as_name(); |
284 | 284 | ||
285 | let mut params = Vec::new(); | 285 | let mut params = Vec::new(); |
286 | let mut param_names = Vec::new(); | ||
286 | let mut has_self_param = false; | 287 | let mut has_self_param = false; |
287 | if let Some(param_list) = func.param_list() { | 288 | if let Some(param_list) = func.param_list() { |
288 | if let Some(self_param) = param_list.self_param() { | 289 | if let Some(self_param) = param_list.self_param() { |
@@ -305,6 +306,18 @@ impl Ctx { | |||
305 | has_self_param = true; | 306 | has_self_param = true; |
306 | } | 307 | } |
307 | for param in param_list.params() { | 308 | for param in param_list.params() { |
309 | let param_name = param | ||
310 | .pat() | ||
311 | .map(|name| { | ||
312 | if let ast::Pat::IdentPat(ident) = name { | ||
313 | Some(ident.name()?.as_name()) | ||
314 | } else { | ||
315 | None | ||
316 | } | ||
317 | }) | ||
318 | .flatten(); | ||
319 | param_names.push(param_name); | ||
320 | |||
308 | let type_ref = TypeRef::from_ast_opt(&self.body_ctx, param.ty()); | 321 | let type_ref = TypeRef::from_ast_opt(&self.body_ctx, param.ty()); |
309 | params.push(type_ref); | 322 | params.push(type_ref); |
310 | } | 323 | } |
@@ -341,6 +354,7 @@ impl Ctx { | |||
341 | has_body, | 354 | has_body, |
342 | is_unsafe: func.unsafe_token().is_some(), | 355 | is_unsafe: func.unsafe_token().is_some(), |
343 | params: params.into_boxed_slice(), | 356 | params: params.into_boxed_slice(), |
357 | param_names: param_names.into_boxed_slice(), | ||
344 | is_varargs, | 358 | is_varargs, |
345 | ret_type, | 359 | ret_type, |
346 | ast_id, | 360 | ast_id, |
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 @@ | |||
2 | mod expr; | 2 | mod expr; |
3 | mod match_check; | 3 | mod match_check; |
4 | mod unsafe_check; | 4 | mod unsafe_check; |
5 | mod decl_check; | ||
5 | 6 | ||
6 | use std::any::Any; | 7 | use std::{any::Any, fmt}; |
7 | 8 | ||
8 | use hir_def::DefWithBodyId; | 9 | use hir_def::{DefWithBodyId, ModuleDefId}; |
9 | use hir_expand::diagnostics::{Diagnostic, DiagnosticCode, DiagnosticSink}; | 10 | use hir_expand::diagnostics::{Diagnostic, DiagnosticCode, DiagnosticSink}; |
10 | use hir_expand::{name::Name, HirFileId, InFile}; | 11 | use hir_expand::{name::Name, HirFileId, InFile}; |
11 | use stdx::format_to; | 12 | use stdx::format_to; |
@@ -15,6 +16,16 @@ use crate::db::HirDatabase; | |||
15 | 16 | ||
16 | pub use crate::diagnostics::expr::{record_literal_missing_fields, record_pattern_missing_fields}; | 17 | pub use crate::diagnostics::expr::{record_literal_missing_fields, record_pattern_missing_fields}; |
17 | 18 | ||
19 | pub fn validate_module_item( | ||
20 | db: &dyn HirDatabase, | ||
21 | owner: ModuleDefId, | ||
22 | sink: &mut DiagnosticSink<'_>, | ||
23 | ) { | ||
24 | let _p = profile::span("validate_body"); | ||
25 | let mut validator = decl_check::DeclValidator::new(owner, sink); | ||
26 | validator.validate_item(db); | ||
27 | } | ||
28 | |||
18 | pub fn validate_body(db: &dyn HirDatabase, owner: DefWithBodyId, sink: &mut DiagnosticSink<'_>) { | 29 | pub fn validate_body(db: &dyn HirDatabase, owner: DefWithBodyId, sink: &mut DiagnosticSink<'_>) { |
19 | let _p = profile::span("validate_body"); | 30 | let _p = profile::span("validate_body"); |
20 | let infer = db.infer(owner); | 31 | let infer = db.infer(owner); |
@@ -231,6 +242,64 @@ impl Diagnostic for MismatchedArgCount { | |||
231 | } | 242 | } |
232 | } | 243 | } |
233 | 244 | ||
245 | #[derive(Debug)] | ||
246 | pub enum CaseType { | ||
247 | // `some_var` | ||
248 | LowerSnakeCase, | ||
249 | // `SOME_CONST` | ||
250 | UpperSnakeCase, | ||
251 | // `SomeStruct` | ||
252 | UpperCamelCase, | ||
253 | } | ||
254 | |||
255 | impl fmt::Display for CaseType { | ||
256 | fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
257 | let repr = match self { | ||
258 | CaseType::LowerSnakeCase => "snake_case", | ||
259 | CaseType::UpperSnakeCase => "UPPER_SNAKE_CASE", | ||
260 | CaseType::UpperCamelCase => "UpperCamelCase", | ||
261 | }; | ||
262 | |||
263 | write!(f, "{}", repr) | ||
264 | } | ||
265 | } | ||
266 | |||
267 | #[derive(Debug)] | ||
268 | pub struct IncorrectCase { | ||
269 | pub file: HirFileId, | ||
270 | pub ident: SyntaxNodePtr, | ||
271 | pub expected_case: CaseType, | ||
272 | pub ident_text: String, | ||
273 | pub suggested_text: String, | ||
274 | } | ||
275 | |||
276 | impl Diagnostic for IncorrectCase { | ||
277 | fn code(&self) -> DiagnosticCode { | ||
278 | DiagnosticCode("incorrect-ident-case") | ||
279 | } | ||
280 | |||
281 | fn message(&self) -> String { | ||
282 | format!( | ||
283 | "Argument `{}` should have a {} name, e.g. `{}`", | ||
284 | self.ident_text, | ||
285 | self.expected_case.to_string(), | ||
286 | self.suggested_text | ||
287 | ) | ||
288 | } | ||
289 | |||
290 | fn display_source(&self) -> InFile<SyntaxNodePtr> { | ||
291 | InFile::new(self.file, self.ident.clone()) | ||
292 | } | ||
293 | |||
294 | fn as_any(&self) -> &(dyn Any + Send + 'static) { | ||
295 | self | ||
296 | } | ||
297 | |||
298 | fn is_experimental(&self) -> bool { | ||
299 | true | ||
300 | } | ||
301 | } | ||
302 | |||
234 | #[cfg(test)] | 303 | #[cfg(test)] |
235 | mod tests { | 304 | mod tests { |
236 | use base_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt}; | 305 | use base_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt}; |
@@ -242,7 +311,10 @@ mod tests { | |||
242 | use rustc_hash::FxHashMap; | 311 | use rustc_hash::FxHashMap; |
243 | use syntax::{TextRange, TextSize}; | 312 | use syntax::{TextRange, TextSize}; |
244 | 313 | ||
245 | use crate::{diagnostics::validate_body, test_db::TestDB}; | 314 | use crate::{ |
315 | diagnostics::{validate_body, validate_module_item}, | ||
316 | test_db::TestDB, | ||
317 | }; | ||
246 | 318 | ||
247 | impl TestDB { | 319 | impl TestDB { |
248 | fn diagnostics<F: FnMut(&dyn Diagnostic)>(&self, mut cb: F) { | 320 | fn diagnostics<F: FnMut(&dyn Diagnostic)>(&self, mut cb: F) { |
@@ -253,6 +325,9 @@ mod tests { | |||
253 | let mut fns = Vec::new(); | 325 | let mut fns = Vec::new(); |
254 | for (module_id, _) in crate_def_map.modules.iter() { | 326 | for (module_id, _) in crate_def_map.modules.iter() { |
255 | for decl in crate_def_map[module_id].scope.declarations() { | 327 | for decl in crate_def_map[module_id].scope.declarations() { |
328 | let mut sink = DiagnosticSinkBuilder::new().build(&mut cb); | ||
329 | validate_module_item(self, decl, &mut sink); | ||
330 | |||
256 | if let ModuleDefId::FunctionId(f) = decl { | 331 | if let ModuleDefId::FunctionId(f) = decl { |
257 | fns.push(f) | 332 | fns.push(f) |
258 | } | 333 | } |
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 @@ | |||
1 | //! Provides validators for the item declarations. | ||
2 | //! This includes the following items: | ||
3 | //! - variable bindings (e.g. `let x = foo();`) | ||
4 | //! - struct fields (e.g. `struct Foo { field: u8 }`) | ||
5 | //! - enum fields (e.g. `enum Foo { Variant { field: u8 } }`) | ||
6 | //! - function/method arguments (e.g. `fn foo(arg: u8)`) | ||
7 | |||
8 | // TODO: Temporary, to not see warnings until module is somewhat complete. | ||
9 | // If you see these lines in the pull request, feel free to call me stupid :P. | ||
10 | #![allow(dead_code, unused_imports, unused_variables)] | ||
11 | |||
12 | use std::sync::Arc; | ||
13 | |||
14 | use hir_def::{ | ||
15 | body::Body, | ||
16 | db::DefDatabase, | ||
17 | expr::{Expr, ExprId, UnaryOp}, | ||
18 | item_tree::ItemTreeNode, | ||
19 | resolver::{resolver_for_expr, ResolveValueResult, ValueNs}, | ||
20 | src::HasSource, | ||
21 | AdtId, FunctionId, Lookup, ModuleDefId, | ||
22 | }; | ||
23 | use hir_expand::{diagnostics::DiagnosticSink, name::Name}; | ||
24 | use syntax::{ast::NameOwner, AstPtr}; | ||
25 | |||
26 | use crate::{ | ||
27 | db::HirDatabase, | ||
28 | diagnostics::{CaseType, IncorrectCase}, | ||
29 | lower::CallableDefId, | ||
30 | ApplicationTy, InferenceResult, Ty, TypeCtor, | ||
31 | }; | ||
32 | |||
33 | pub(super) struct DeclValidator<'a, 'b: 'a> { | ||
34 | owner: ModuleDefId, | ||
35 | sink: &'a mut DiagnosticSink<'b>, | ||
36 | } | ||
37 | |||
38 | #[derive(Debug)] | ||
39 | struct Replacement { | ||
40 | current_name: Name, | ||
41 | suggested_text: String, | ||
42 | expected_case: CaseType, | ||
43 | } | ||
44 | |||
45 | impl<'a, 'b> DeclValidator<'a, 'b> { | ||
46 | pub(super) fn new( | ||
47 | owner: ModuleDefId, | ||
48 | sink: &'a mut DiagnosticSink<'b>, | ||
49 | ) -> DeclValidator<'a, 'b> { | ||
50 | DeclValidator { owner, sink } | ||
51 | } | ||
52 | |||
53 | pub(super) fn validate_item(&mut self, db: &dyn HirDatabase) { | ||
54 | // let def = self.owner.into(); | ||
55 | match self.owner { | ||
56 | ModuleDefId::FunctionId(func) => self.validate_func(db, func), | ||
57 | ModuleDefId::AdtId(adt) => self.validate_adt(db, adt), | ||
58 | _ => return, | ||
59 | } | ||
60 | } | ||
61 | |||
62 | fn validate_func(&mut self, db: &dyn HirDatabase, func: FunctionId) { | ||
63 | let data = db.function_data(func); | ||
64 | |||
65 | // 1. Check the function name. | ||
66 | let function_name = data.name.to_string(); | ||
67 | let fn_name_replacement = if let Some(new_name) = to_lower_snake_case(&function_name) { | ||
68 | let replacement = Replacement { | ||
69 | current_name: data.name.clone(), | ||
70 | suggested_text: new_name, | ||
71 | expected_case: CaseType::LowerSnakeCase, | ||
72 | }; | ||
73 | Some(replacement) | ||
74 | } else { | ||
75 | None | ||
76 | }; | ||
77 | |||
78 | // 2. Check the param names. | ||
79 | let mut fn_param_replacements = Vec::new(); | ||
80 | |||
81 | for param_name in data.param_names.iter().cloned().filter_map(|i| i) { | ||
82 | let name = param_name.to_string(); | ||
83 | if let Some(new_name) = to_lower_snake_case(&name) { | ||
84 | let replacement = Replacement { | ||
85 | current_name: param_name, | ||
86 | suggested_text: new_name, | ||
87 | expected_case: CaseType::LowerSnakeCase, | ||
88 | }; | ||
89 | fn_param_replacements.push(replacement); | ||
90 | } | ||
91 | } | ||
92 | |||
93 | // 3. If there is at least one element to spawn a warning on, go to the source map and generate a warning. | ||
94 | self.create_incorrect_case_diagnostic_for_func( | ||
95 | func, | ||
96 | db, | ||
97 | fn_name_replacement, | ||
98 | fn_param_replacements, | ||
99 | ) | ||
100 | } | ||
101 | |||
102 | /// Given the information about incorrect names in the function declaration, looks up into the source code | ||
103 | /// for exact locations and adds diagnostics into the sink. | ||
104 | fn create_incorrect_case_diagnostic_for_func( | ||
105 | &mut self, | ||
106 | func: FunctionId, | ||
107 | db: &dyn HirDatabase, | ||
108 | fn_name_replacement: Option<Replacement>, | ||
109 | fn_param_replacements: Vec<Replacement>, | ||
110 | ) { | ||
111 | // XXX: only look at sources if we do have incorrect names | ||
112 | if fn_name_replacement.is_none() && fn_param_replacements.is_empty() { | ||
113 | return; | ||
114 | } | ||
115 | |||
116 | let fn_loc = func.lookup(db.upcast()); | ||
117 | let fn_src = fn_loc.source(db.upcast()); | ||
118 | |||
119 | if let Some(replacement) = fn_name_replacement { | ||
120 | let ast_ptr = if let Some(name) = fn_src.value.name() { | ||
121 | name | ||
122 | } else { | ||
123 | // We don't want rust-analyzer to panic over this, but it is definitely some kind of error in the logic. | ||
124 | log::error!( | ||
125 | "Replacement was generated for a function without a name: {:?}", | ||
126 | fn_src | ||
127 | ); | ||
128 | return; | ||
129 | }; | ||
130 | |||
131 | let diagnostic = IncorrectCase { | ||
132 | file: fn_src.file_id, | ||
133 | ident: AstPtr::new(&ast_ptr).into(), | ||
134 | expected_case: replacement.expected_case, | ||
135 | ident_text: replacement.current_name.to_string(), | ||
136 | suggested_text: replacement.suggested_text, | ||
137 | }; | ||
138 | |||
139 | self.sink.push(diagnostic); | ||
140 | } | ||
141 | |||
142 | // let item_tree = db.item_tree(loc.id.file_id); | ||
143 | // let fn_def = &item_tree[fn_loc.id.value]; | ||
144 | // let (_, source_map) = db.body_with_source_map(func.into()); | ||
145 | } | ||
146 | |||
147 | fn validate_adt(&mut self, db: &dyn HirDatabase, adt: AdtId) {} | ||
148 | } | ||
149 | |||
150 | fn to_lower_snake_case(ident: &str) -> Option<String> { | ||
151 | let lower_snake_case = stdx::to_lower_snake_case(ident); | ||
152 | |||
153 | if lower_snake_case == ident { | ||
154 | None | ||
155 | } else { | ||
156 | Some(lower_snake_case) | ||
157 | } | ||
158 | } | ||
159 | |||
160 | #[cfg(test)] | ||
161 | mod tests { | ||
162 | use crate::diagnostics::tests::check_diagnostics; | ||
163 | |||
164 | #[test] | ||
165 | fn incorrect_function_name() { | ||
166 | check_diagnostics( | ||
167 | r#" | ||
168 | fn NonSnakeCaseName() {} | ||
169 | // ^^^^^^^^^^^^^^^^ Argument `NonSnakeCaseName` should have a snake_case name, e.g. `non_snake_case_name` | ||
170 | "#, | ||
171 | ); | ||
172 | } | ||
173 | } | ||