aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIgor Aleksanov <[email protected]>2020-10-04 05:39:35 +0100
committerIgor Aleksanov <[email protected]>2020-10-12 09:05:00 +0100
commitb42562b5dee4f4ce23094c7bab6406e3b00f90ad (patch)
tree8711713d749b19f22a534bb3dd1925063d1baade
parent9ec1741b651bd13e4e5e6224f2e2c5c503846a6b (diff)
Make incorrect case diagnostic work inside of functions
-rw-r--r--crates/hir_def/src/item_scope.rs6
-rw-r--r--crates/hir_ty/src/diagnostics.rs4
-rw-r--r--crates/hir_ty/src/diagnostics/decl_check.rs277
-rw-r--r--crates/ide/src/diagnostics.rs26
4 files changed, 280 insertions, 33 deletions
diff --git a/crates/hir_def/src/item_scope.rs b/crates/hir_def/src/item_scope.rs
index 12c24e1ca..a8b3fe844 100644
--- a/crates/hir_def/src/item_scope.rs
+++ b/crates/hir_def/src/item_scope.rs
@@ -95,6 +95,12 @@ impl ItemScope {
95 self.impls.iter().copied() 95 self.impls.iter().copied()
96 } 96 }
97 97
98 pub fn values(
99 &self,
100 ) -> impl Iterator<Item = (ModuleDefId, Visibility)> + ExactSizeIterator + '_ {
101 self.values.values().copied()
102 }
103
98 pub fn visibility_of(&self, def: ModuleDefId) -> Option<Visibility> { 104 pub fn visibility_of(&self, def: ModuleDefId) -> Option<Visibility> {
99 self.name_of(ItemInNs::Types(def)) 105 self.name_of(ItemInNs::Types(def))
100 .or_else(|| self.name_of(ItemInNs::Values(def))) 106 .or_else(|| self.name_of(ItemInNs::Values(def)))
diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs
index bd370e3b2..40f8c8ba2 100644
--- a/crates/hir_ty/src/diagnostics.rs
+++ b/crates/hir_ty/src/diagnostics.rs
@@ -281,7 +281,7 @@ impl Diagnostic for IncorrectCase {
281 281
282 fn message(&self) -> String { 282 fn message(&self) -> String {
283 format!( 283 format!(
284 "{} `{}` should have a {} name, e.g. `{}`", 284 "{} `{}` should have {} name, e.g. `{}`",
285 self.ident_type, 285 self.ident_type,
286 self.ident_text, 286 self.ident_text,
287 self.expected_case.to_string(), 287 self.expected_case.to_string(),
@@ -339,6 +339,8 @@ mod tests {
339 let impl_data = self.impl_data(impl_id); 339 let impl_data = self.impl_data(impl_id);
340 for item in impl_data.items.iter() { 340 for item in impl_data.items.iter() {
341 if let AssocItemId::FunctionId(f) = item { 341 if let AssocItemId::FunctionId(f) = item {
342 let mut sink = DiagnosticSinkBuilder::new().build(&mut cb);
343 validate_module_item(self, ModuleDefId::FunctionId(*f), &mut sink);
342 fns.push(*f) 344 fns.push(*f)
343 } 345 }
344 } 346 }
diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs
index 7fc9c564e..d1c51849a 100644
--- a/crates/hir_ty/src/diagnostics/decl_check.rs
+++ b/crates/hir_ty/src/diagnostics/decl_check.rs
@@ -5,23 +5,13 @@
5//! - enum fields (e.g. `enum Foo { Variant { field: u8 } }`) 5//! - enum fields (e.g. `enum Foo { Variant { field: u8 } }`)
6//! - function/method arguments (e.g. `fn foo(arg: u8)`) 6//! - function/method arguments (e.g. `fn foo(arg: u8)`)
7 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
12mod str_helpers; 8mod str_helpers;
13 9
14use std::sync::Arc;
15
16use hir_def::{ 10use hir_def::{
17 adt::VariantData, 11 adt::VariantData,
18 body::Body, 12 expr::{Pat, PatId},
19 db::DefDatabase,
20 expr::{Expr, ExprId, UnaryOp},
21 item_tree::ItemTreeNode,
22 resolver::{resolver_for_expr, ResolveValueResult, ValueNs},
23 src::HasSource, 13 src::HasSource,
24 AdtId, EnumId, FunctionId, Lookup, ModuleDefId, StructId, 14 AdtId, ConstId, EnumId, FunctionId, Lookup, ModuleDefId, StaticId, StructId,
25}; 15};
26use hir_expand::{ 16use hir_expand::{
27 diagnostics::DiagnosticSink, 17 diagnostics::DiagnosticSink,
@@ -35,8 +25,6 @@ use syntax::{
35use crate::{ 25use crate::{
36 db::HirDatabase, 26 db::HirDatabase,
37 diagnostics::{decl_check::str_helpers::*, CaseType, IncorrectCase}, 27 diagnostics::{decl_check::str_helpers::*, CaseType, IncorrectCase},
38 lower::CallableDefId,
39 ApplicationTy, InferenceResult, Ty, TypeCtor,
40}; 28};
41 29
42pub(super) struct DeclValidator<'a, 'b: 'a> { 30pub(super) struct DeclValidator<'a, 'b: 'a> {
@@ -64,12 +52,25 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
64 match self.owner { 52 match self.owner {
65 ModuleDefId::FunctionId(func) => self.validate_func(db, func), 53 ModuleDefId::FunctionId(func) => self.validate_func(db, func),
66 ModuleDefId::AdtId(adt) => self.validate_adt(db, adt), 54 ModuleDefId::AdtId(adt) => self.validate_adt(db, adt),
55 ModuleDefId::ConstId(const_id) => self.validate_const(db, const_id),
56 ModuleDefId::StaticId(static_id) => self.validate_static(db, static_id),
67 _ => return, 57 _ => return,
68 } 58 }
69 } 59 }
70 60
61 fn validate_adt(&mut self, db: &dyn HirDatabase, adt: AdtId) {
62 match adt {
63 AdtId::StructId(struct_id) => self.validate_struct(db, struct_id),
64 AdtId::EnumId(enum_id) => self.validate_enum(db, enum_id),
65 AdtId::UnionId(_) => {
66 // Unions aren't yet supported by this validator.
67 }
68 }
69 }
70
71 fn validate_func(&mut self, db: &dyn HirDatabase, func: FunctionId) { 71 fn validate_func(&mut self, db: &dyn HirDatabase, func: FunctionId) {
72 let data = db.function_data(func); 72 let data = db.function_data(func);
73 let body = db.body(func.into());
73 74
74 // 1. Check the function name. 75 // 1. Check the function name.
75 let function_name = data.name.to_string(); 76 let function_name = data.name.to_string();
@@ -87,11 +88,18 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
87 // 2. Check the param names. 88 // 2. Check the param names.
88 let mut fn_param_replacements = Vec::new(); 89 let mut fn_param_replacements = Vec::new();
89 90
90 for param_name in data.param_names.iter().cloned().filter_map(|i| i) { 91 for pat_id in body.params.iter().cloned() {
92 let pat = &body[pat_id];
93
94 let param_name = match pat {
95 Pat::Bind { name, .. } => name,
96 _ => continue,
97 };
98
91 let name = param_name.to_string(); 99 let name = param_name.to_string();
92 if let Some(new_name) = to_lower_snake_case(&name) { 100 if let Some(new_name) = to_lower_snake_case(&name) {
93 let replacement = Replacement { 101 let replacement = Replacement {
94 current_name: param_name, 102 current_name: param_name.clone(),
95 suggested_text: new_name, 103 suggested_text: new_name,
96 expected_case: CaseType::LowerSnakeCase, 104 expected_case: CaseType::LowerSnakeCase,
97 }; 105 };
@@ -99,13 +107,45 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
99 } 107 }
100 } 108 }
101 109
102 // 3. If there is at least one element to spawn a warning on, go to the source map and generate a warning. 110 // 3. Check the patterns inside the function body.
111 let mut pats_replacements = Vec::new();
112
113 for (pat_idx, pat) in body.pats.iter() {
114 if body.params.contains(&pat_idx) {
115 // We aren't interested in function parameters, we've processed them above.
116 continue;
117 }
118
119 let bind_name = match pat {
120 Pat::Bind { name, .. } => name,
121 _ => continue,
122 };
123
124 let name = bind_name.to_string();
125 if let Some(new_name) = to_lower_snake_case(&name) {
126 let replacement = Replacement {
127 current_name: bind_name.clone(),
128 suggested_text: new_name,
129 expected_case: CaseType::LowerSnakeCase,
130 };
131 pats_replacements.push((pat_idx, replacement));
132 }
133 }
134
135 // 4. If there is at least one element to spawn a warning on, go to the source map and generate a warning.
103 self.create_incorrect_case_diagnostic_for_func( 136 self.create_incorrect_case_diagnostic_for_func(
104 func, 137 func,
105 db, 138 db,
106 fn_name_replacement, 139 fn_name_replacement,
107 fn_param_replacements, 140 fn_param_replacements,
108 ) 141 );
142 self.create_incorrect_case_diagnostic_for_variables(func, db, pats_replacements);
143
144 // 5. Recursively validate inner scope items, such as static variables and constants.
145 for (item_id, _) in body.item_scope.values() {
146 let mut validator = DeclValidator::new(item_id, self.sink);
147 validator.validate_item(db);
148 }
109 } 149 }
110 150
111 /// Given the information about incorrect names in the function declaration, looks up into the source code 151 /// Given the information about incorrect names in the function declaration, looks up into the source code
@@ -125,6 +165,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
125 let fn_loc = func.lookup(db.upcast()); 165 let fn_loc = func.lookup(db.upcast());
126 let fn_src = fn_loc.source(db.upcast()); 166 let fn_src = fn_loc.source(db.upcast());
127 167
168 // 1. Diagnostic for function name.
128 if let Some(replacement) = fn_name_replacement { 169 if let Some(replacement) = fn_name_replacement {
129 let ast_ptr = if let Some(name) = fn_src.value.name() { 170 let ast_ptr = if let Some(name) = fn_src.value.name() {
130 name 171 name
@@ -150,6 +191,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
150 self.sink.push(diagnostic); 191 self.sink.push(diagnostic);
151 } 192 }
152 193
194 // 2. Diagnostics for function params.
153 let fn_params_list = match fn_src.value.param_list() { 195 let fn_params_list = match fn_src.value.param_list() {
154 Some(params) => params, 196 Some(params) => params,
155 None => { 197 None => {
@@ -197,12 +239,38 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
197 } 239 }
198 } 240 }
199 241
200 fn validate_adt(&mut self, db: &dyn HirDatabase, adt: AdtId) { 242 /// Given the information about incorrect variable names, looks up into the source code
201 match adt { 243 /// for exact locations and adds diagnostics into the sink.
202 AdtId::StructId(struct_id) => self.validate_struct(db, struct_id), 244 fn create_incorrect_case_diagnostic_for_variables(
203 AdtId::EnumId(enum_id) => self.validate_enum(db, enum_id), 245 &mut self,
204 AdtId::UnionId(_) => { 246 func: FunctionId,
205 // Unions aren't yet supported by this validator. 247 db: &dyn HirDatabase,
248 pats_replacements: Vec<(PatId, Replacement)>,
249 ) {
250 // XXX: only look at source_map if we do have missing fields
251 if pats_replacements.is_empty() {
252 return;
253 }
254
255 let (_, source_map) = db.body_with_source_map(func.into());
256
257 for (id, replacement) in pats_replacements {
258 if let Ok(source_ptr) = source_map.pat_syntax(id) {
259 if let Some(expr) = source_ptr.value.as_ref().left() {
260 let root = source_ptr.file_syntax(db.upcast());
261 if let ast::Pat::IdentPat(ident_pat) = expr.to_node(&root) {
262 let diagnostic = IncorrectCase {
263 file: source_ptr.file_id,
264 ident_type: "Variable".to_string(),
265 ident: AstPtr::new(&ident_pat).into(),
266 expected_case: replacement.expected_case,
267 ident_text: replacement.current_name.to_string(),
268 suggested_text: replacement.suggested_text,
269 };
270
271 self.sink.push(diagnostic);
272 }
273 }
206 } 274 }
207 } 275 }
208 } 276 }
@@ -246,7 +314,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
246 db, 314 db,
247 struct_name_replacement, 315 struct_name_replacement,
248 struct_fields_replacements, 316 struct_fields_replacements,
249 ) 317 );
250 } 318 }
251 319
252 /// Given the information about incorrect names in the struct declaration, looks up into the source code 320 /// Given the information about incorrect names in the struct declaration, looks up into the source code
@@ -464,6 +532,86 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
464 self.sink.push(diagnostic); 532 self.sink.push(diagnostic);
465 } 533 }
466 } 534 }
535
536 fn validate_const(&mut self, db: &dyn HirDatabase, const_id: ConstId) {
537 let data = db.const_data(const_id);
538
539 let name = match &data.name {
540 Some(name) => name,
541 None => return,
542 };
543
544 let const_name = name.to_string();
545 let replacement = if let Some(new_name) = to_upper_snake_case(&const_name) {
546 Replacement {
547 current_name: name.clone(),
548 suggested_text: new_name,
549 expected_case: CaseType::UpperSnakeCase,
550 }
551 } else {
552 // Nothing to do here.
553 return;
554 };
555
556 let const_loc = const_id.lookup(db.upcast());
557 let const_src = const_loc.source(db.upcast());
558
559 let ast_ptr = match const_src.value.name() {
560 Some(name) => name,
561 None => return,
562 };
563
564 let diagnostic = IncorrectCase {
565 file: const_src.file_id,
566 ident_type: "Constant".to_string(),
567 ident: AstPtr::new(&ast_ptr).into(),
568 expected_case: replacement.expected_case,
569 ident_text: replacement.current_name.to_string(),
570 suggested_text: replacement.suggested_text,
571 };
572
573 self.sink.push(diagnostic);
574 }
575
576 fn validate_static(&mut self, db: &dyn HirDatabase, static_id: StaticId) {
577 let data = db.static_data(static_id);
578
579 let name = match &data.name {
580 Some(name) => name,
581 None => return,
582 };
583
584 let static_name = name.to_string();
585 let replacement = if let Some(new_name) = to_upper_snake_case(&static_name) {
586 Replacement {
587 current_name: name.clone(),
588 suggested_text: new_name,
589 expected_case: CaseType::UpperSnakeCase,
590 }
591 } else {
592 // Nothing to do here.
593 return;
594 };
595
596 let static_loc = static_id.lookup(db.upcast());
597 let static_src = static_loc.source(db.upcast());
598
599 let ast_ptr = match static_src.value.name() {
600 Some(name) => name,
601 None => return,
602 };
603
604 let diagnostic = IncorrectCase {
605 file: static_src.file_id,
606 ident_type: "Static variable".to_string(),
607 ident: AstPtr::new(&ast_ptr).into(),
608 expected_case: replacement.expected_case,
609 ident_text: replacement.current_name.to_string(),
610 suggested_text: replacement.suggested_text,
611 };
612
613 self.sink.push(diagnostic);
614 }
467} 615}
468 616
469fn names_equal(left: Option<ast::Name>, right: &Name) -> bool { 617fn names_equal(left: Option<ast::Name>, right: &Name) -> bool {
@@ -491,7 +639,7 @@ mod tests {
491 check_diagnostics( 639 check_diagnostics(
492 r#" 640 r#"
493fn NonSnakeCaseName() {} 641fn NonSnakeCaseName() {}
494// ^^^^^^^^^^^^^^^^ Function `NonSnakeCaseName` should have a snake_case name, e.g. `non_snake_case_name` 642// ^^^^^^^^^^^^^^^^ Function `NonSnakeCaseName` should have snake_case name, e.g. `non_snake_case_name`
495"#, 643"#,
496 ); 644 );
497 } 645 }
@@ -501,10 +649,24 @@ fn NonSnakeCaseName() {}
501 check_diagnostics( 649 check_diagnostics(
502 r#" 650 r#"
503fn foo(SomeParam: u8) {} 651fn foo(SomeParam: u8) {}
504 // ^^^^^^^^^ Argument `SomeParam` should have a snake_case name, e.g. `some_param` 652 // ^^^^^^^^^ Argument `SomeParam` should have snake_case name, e.g. `some_param`
505 653
506fn foo2(ok_param: &str, CAPS_PARAM: u8) {} 654fn foo2(ok_param: &str, CAPS_PARAM: u8) {}
507 // ^^^^^^^^^^ Argument `CAPS_PARAM` should have a snake_case name, e.g. `caps_param` 655 // ^^^^^^^^^^ Argument `CAPS_PARAM` should have snake_case name, e.g. `caps_param`
656"#,
657 );
658 }
659
660 #[test]
661 fn incorrect_variable_names() {
662 check_diagnostics(
663 r#"
664fn foo() {
665 let SOME_VALUE = 10;
666 // ^^^^^^^^^^ Variable `SOME_VALUE` should have a snake_case name, e.g. `some_value`
667 let AnotherValue = 20;
668 // ^^^^^^^^^^^^ Variable `AnotherValue` should have snake_case name, e.g. `another_value`
669}
508"#, 670"#,
509 ); 671 );
510 } 672 }
@@ -514,7 +676,7 @@ fn foo2(ok_param: &str, CAPS_PARAM: u8) {}
514 check_diagnostics( 676 check_diagnostics(
515 r#" 677 r#"
516struct non_camel_case_name {} 678struct non_camel_case_name {}
517 // ^^^^^^^^^^^^^^^^^^^ Structure `non_camel_case_name` should have a CamelCase name, e.g. `NonCamelCaseName` 679 // ^^^^^^^^^^^^^^^^^^^ Structure `non_camel_case_name` should have CamelCase name, e.g. `NonCamelCaseName`
518"#, 680"#,
519 ); 681 );
520 } 682 }
@@ -524,7 +686,7 @@ struct non_camel_case_name {}
524 check_diagnostics( 686 check_diagnostics(
525 r#" 687 r#"
526struct SomeStruct { SomeField: u8 } 688struct SomeStruct { SomeField: u8 }
527 // ^^^^^^^^^ Field `SomeField` should have a snake_case name, e.g. `some_field` 689 // ^^^^^^^^^ Field `SomeField` should have snake_case name, e.g. `some_field`
528"#, 690"#,
529 ); 691 );
530 } 692 }
@@ -534,7 +696,7 @@ struct SomeStruct { SomeField: u8 }
534 check_diagnostics( 696 check_diagnostics(
535 r#" 697 r#"
536enum some_enum { Val(u8) } 698enum some_enum { Val(u8) }
537 // ^^^^^^^^^ Enum `some_enum` should have a CamelCase name, e.g. `SomeEnum` 699 // ^^^^^^^^^ Enum `some_enum` should have CamelCase name, e.g. `SomeEnum`
538"#, 700"#,
539 ); 701 );
540 } 702 }
@@ -544,7 +706,58 @@ enum some_enum { Val(u8) }
544 check_diagnostics( 706 check_diagnostics(
545 r#" 707 r#"
546enum SomeEnum { SOME_VARIANT(u8) } 708enum SomeEnum { SOME_VARIANT(u8) }
547 // ^^^^^^^^^^^^ Variant `SOME_VARIANT` should have a CamelCase name, e.g. `SomeVariant` 709 // ^^^^^^^^^^^^ Variant `SOME_VARIANT` should have CamelCase name, e.g. `SomeVariant`
710"#,
711 );
712 }
713
714 #[test]
715 fn incorrect_const_name() {
716 check_diagnostics(
717 r#"
718const some_weird_const: u8 = 10;
719 // ^^^^^^^^^^^^^^^^ Constant `some_weird_const` should have UPPER_SNAKE_CASE name, e.g. `SOME_WEIRD_CONST`
720
721fn func() {
722 const someConstInFunc: &str = "hi there";
723 // ^^^^^^^^^^^^^^^ Constant `someConstInFunc` should have UPPER_SNAKE_CASE name, e.g. `SOME_CONST_IN_FUNC`
724
725}
726"#,
727 );
728 }
729
730 #[test]
731 fn incorrect_static_name() {
732 check_diagnostics(
733 r#"
734static some_weird_const: u8 = 10;
735 // ^^^^^^^^^^^^^^^^ Static variable `some_weird_const` should have UPPER_SNAKE_CASE name, e.g. `SOME_WEIRD_CONST`
736
737fn func() {
738 static someConstInFunc: &str = "hi there";
739 // ^^^^^^^^^^^^^^^ Static variable `someConstInFunc` should have UPPER_SNAKE_CASE name, e.g. `SOME_CONST_IN_FUNC`
740}
741"#,
742 );
743 }
744
745 #[test]
746 fn fn_inside_impl_struct() {
747 check_diagnostics(
748 r#"
749struct someStruct;
750 // ^^^^^^^^^^ Structure `someStruct` should have CamelCase name, e.g. `SomeStruct`
751
752impl someStruct {
753 fn SomeFunc(&self) {
754 // ^^^^^^^^ Function `SomeFunc` should have snake_case name, e.g. `some_func`
755 static someConstInFunc: &str = "hi there";
756 // ^^^^^^^^^^^^^^^ Static variable `someConstInFunc` should have UPPER_SNAKE_CASE name, e.g. `SOME_CONST_IN_FUNC`
757 let WHY_VAR_IS_CAPS = 10;
758 // ^^^^^^^^^^^^^^^ Variable `WHY_VAR_IS_CAPS` should have snake_case name, e.g. `why_var_is_caps`
759 }
760}
548"#, 761"#,
549 ); 762 );
550 } 763 }
diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs
index ad1b265fd..70d5cbd38 100644
--- a/crates/ide/src/diagnostics.rs
+++ b/crates/ide/src/diagnostics.rs
@@ -879,5 +879,31 @@ pub fn some_fn(val: u8) -> u8 {
879} 879}
880"#, 880"#,
881 ); 881 );
882
883 check_fixes(
884 r#"
885fn some_fn() {
886 let whatAWeird_Formatting<|> = 10;
887 another_func(whatAWeird_Formatting);
888}
889"#,
890 r#"
891fn some_fn() {
892 let what_a_weird_formatting = 10;
893 another_func(what_a_weird_formatting);
894}
895"#,
896 );
897 }
898
899 #[test]
900 fn test_uppercase_const_no_diagnostics() {
901 check_no_diagnostics(
902 r#"
903fn foo() {
904 const ANOTHER_ITEM<|>: &str = "some_item";
905}
906"#,
907 );
882 } 908 }
883} 909}