aboutsummaryrefslogtreecommitdiff
path: root/crates/hir_ty
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2021-04-13 13:02:26 +0100
committerGitHub <[email protected]>2021-04-13 13:02:26 +0100
commit9beed98f2ab15f6d62283d09b6b96fa5f57a78d1 (patch)
treeb9b28d597d793ce7c12ee0c347ffe0edf0e5327b /crates/hir_ty
parent03e0bf7f555ad4f3c8e127e009897b6fa83a6194 (diff)
parent26d2653dd64f139c89449cf4bab8bac737e930a5 (diff)
Merge #8432
8432: decl_check: consider outer scopes' allows r=jonas-schievink a=lf- Fix #8417. Also makes it less noisy about no_mangle annotated stuff the user can do nothing about. Note: this still is broken with bitfield! macros. A repro in an ignore test is included here. I believe this bug is elsewhere, and I don't think I can work around it here. I would like help filing the remaining bug, as it does actually affect users, but I don't know how to describe the behaviour (or even if it is unintended). Co-authored-by: Jade <[email protected]>
Diffstat (limited to 'crates/hir_ty')
-rw-r--r--crates/hir_ty/src/diagnostics/decl_check.rs154
1 files changed, 140 insertions, 14 deletions
diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs
index 1c9f9ede7..fb0fc4c91 100644
--- a/crates/hir_ty/src/diagnostics/decl_check.rs
+++ b/crates/hir_ty/src/diagnostics/decl_check.rs
@@ -35,6 +35,8 @@ use crate::{
35}; 35};
36 36
37mod allow { 37mod allow {
38 pub(super) const BAD_STYLE: &str = "bad_style";
39 pub(super) const NONSTANDARD_STYLE: &str = "nonstandard_style";
38 pub(super) const NON_SNAKE_CASE: &str = "non_snake_case"; 40 pub(super) const NON_SNAKE_CASE: &str = "non_snake_case";
39 pub(super) const NON_UPPER_CASE_GLOBAL: &str = "non_upper_case_globals"; 41 pub(super) const NON_UPPER_CASE_GLOBAL: &str = "non_upper_case_globals";
40 pub(super) const NON_CAMEL_CASE_TYPES: &str = "non_camel_case_types"; 42 pub(super) const NON_CAMEL_CASE_TYPES: &str = "non_camel_case_types";
@@ -83,10 +85,39 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
83 } 85 }
84 86
85 /// Checks whether not following the convention is allowed for this item. 87 /// Checks whether not following the convention is allowed for this item.
86 /// 88 fn allowed(&self, id: AttrDefId, allow_name: &str, recursing: bool) -> bool {
87 /// Currently this method doesn't check parent attributes. 89 let is_allowed = |def_id| {
88 fn allowed(&self, id: AttrDefId, allow_name: &str) -> bool { 90 let attrs = self.db.attrs(def_id);
89 self.db.attrs(id).by_key("allow").tt_values().any(|tt| tt.to_string().contains(allow_name)) 91 // don't bug the user about directly no_mangle annotated stuff, they can't do anything about it
92 (!recursing && attrs.by_key("no_mangle").exists())
93 || attrs.by_key("allow").tt_values().any(|tt| {
94 let allows = tt.to_string();
95 allows.contains(allow_name)
96 || allows.contains(allow::BAD_STYLE)
97 || allows.contains(allow::NONSTANDARD_STYLE)
98 })
99 };
100
101 is_allowed(id)
102 // go upwards one step or give up
103 || match id {
104 AttrDefId::ModuleId(m) => m.containing_module(self.db.upcast()).map(|v| v.into()),
105 AttrDefId::FunctionId(f) => Some(f.lookup(self.db.upcast()).container.into()),
106 AttrDefId::StaticId(sid) => Some(sid.lookup(self.db.upcast()).container.into()),
107 AttrDefId::ConstId(cid) => Some(cid.lookup(self.db.upcast()).container.into()),
108 AttrDefId::TraitId(tid) => Some(tid.lookup(self.db.upcast()).container.into()),
109 AttrDefId::ImplId(iid) => Some(iid.lookup(self.db.upcast()).container.into()),
110 // These warnings should not explore macro definitions at all
111 AttrDefId::MacroDefId(_) => None,
112 // Will never occur under an enum/struct/union/type alias
113 AttrDefId::AdtId(_) => None,
114 AttrDefId::FieldId(_) => None,
115 AttrDefId::EnumVariantId(_) => None,
116 AttrDefId::TypeAliasId(_) => None,
117 AttrDefId::GenericParamId(_) => None,
118 }
119 .map(|mid| self.allowed(mid, allow_name, true))
120 .unwrap_or(false)
90 } 121 }
91 122
92 fn validate_func(&mut self, func: FunctionId) { 123 fn validate_func(&mut self, func: FunctionId) {
@@ -109,7 +140,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
109 } 140 }
110 141
111 // Check whether non-snake case identifiers are allowed for this function. 142 // Check whether non-snake case identifiers are allowed for this function.
112 if self.allowed(func.into(), allow::NON_SNAKE_CASE) { 143 if self.allowed(func.into(), allow::NON_SNAKE_CASE, false) {
113 return; 144 return;
114 } 145 }
115 146
@@ -328,8 +359,9 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
328 fn validate_struct(&mut self, struct_id: StructId) { 359 fn validate_struct(&mut self, struct_id: StructId) {
329 let data = self.db.struct_data(struct_id); 360 let data = self.db.struct_data(struct_id);
330 361
331 let non_camel_case_allowed = self.allowed(struct_id.into(), allow::NON_CAMEL_CASE_TYPES); 362 let non_camel_case_allowed =
332 let non_snake_case_allowed = self.allowed(struct_id.into(), allow::NON_SNAKE_CASE); 363 self.allowed(struct_id.into(), allow::NON_CAMEL_CASE_TYPES, false);
364 let non_snake_case_allowed = self.allowed(struct_id.into(), allow::NON_SNAKE_CASE, false);
333 365
334 // Check the structure name. 366 // Check the structure name.
335 let struct_name = data.name.to_string(); 367 let struct_name = data.name.to_string();
@@ -461,7 +493,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
461 let data = self.db.enum_data(enum_id); 493 let data = self.db.enum_data(enum_id);
462 494
463 // Check whether non-camel case names are allowed for this enum. 495 // Check whether non-camel case names are allowed for this enum.
464 if self.allowed(enum_id.into(), allow::NON_CAMEL_CASE_TYPES) { 496 if self.allowed(enum_id.into(), allow::NON_CAMEL_CASE_TYPES, false) {
465 return; 497 return;
466 } 498 }
467 499
@@ -584,7 +616,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
584 fn validate_const(&mut self, const_id: ConstId) { 616 fn validate_const(&mut self, const_id: ConstId) {
585 let data = self.db.const_data(const_id); 617 let data = self.db.const_data(const_id);
586 618
587 if self.allowed(const_id.into(), allow::NON_UPPER_CASE_GLOBAL) { 619 if self.allowed(const_id.into(), allow::NON_UPPER_CASE_GLOBAL, false) {
588 return; 620 return;
589 } 621 }
590 622
@@ -632,7 +664,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
632 return; 664 return;
633 } 665 }
634 666
635 if self.allowed(static_id.into(), allow::NON_UPPER_CASE_GLOBAL) { 667 if self.allowed(static_id.into(), allow::NON_UPPER_CASE_GLOBAL, false) {
636 return; 668 return;
637 } 669 }
638 670
@@ -869,10 +901,31 @@ fn main() {
869 r#" 901 r#"
870 #[allow(non_snake_case)] 902 #[allow(non_snake_case)]
871 fn NonSnakeCaseName(SOME_VAR: u8) -> u8{ 903 fn NonSnakeCaseName(SOME_VAR: u8) -> u8{
904 // cov_flags generated output from elsewhere in this file
905 extern "C" {
906 #[no_mangle]
907 static lower_case: u8;
908 }
909
872 let OtherVar = SOME_VAR + 1; 910 let OtherVar = SOME_VAR + 1;
873 OtherVar 911 OtherVar
874 } 912 }
875 913
914 #[allow(nonstandard_style)]
915 mod CheckNonstandardStyle {
916 fn HiImABadFnName() {}
917 }
918
919 #[allow(bad_style)]
920 mod CheckBadStyle {
921 fn HiImABadFnName() {}
922 }
923
924 mod F {
925 #![allow(non_snake_case)]
926 fn CheckItWorksWithModAttr(BAD_NAME_HI: u8) {}
927 }
928
876 #[allow(non_snake_case, non_camel_case_types)] 929 #[allow(non_snake_case, non_camel_case_types)]
877 pub struct some_type { 930 pub struct some_type {
878 SOME_FIELD: u8, 931 SOME_FIELD: u8,
@@ -889,15 +942,88 @@ fn main() {
889 } 942 }
890 943
891 #[test] 944 #[test]
945 fn allow_attributes_crate_attr() {
946 check_diagnostics(
947 r#"
948 #![allow(non_snake_case)]
949
950 mod F {
951 fn CheckItWorksWithCrateAttr(BAD_NAME_HI: u8) {}
952 }
953
954 "#,
955 );
956 }
957
958 #[test]
959 #[ignore]
960 fn bug_trait_inside_fn() {
961 // FIXME:
962 // This is broken, and in fact, should not even be looked at by this
963 // lint in the first place. There's weird stuff going on in the
964 // collection phase.
965 // It's currently being brought in by:
966 // * validate_func on `a` recursing into modules
967 // * then it finds the trait and then the function while iterating
968 // through modules
969 // * then validate_func is called on Dirty
970 // * ... which then proceeds to look at some unknown module taking no
971 // attrs from either the impl or the fn a, and then finally to the root
972 // module
973 //
974 // It should find the attribute on the trait, but it *doesn't even see
975 // the trait* as far as I can tell.
976
977 check_diagnostics(
978 r#"
979 trait T { fn a(); }
980 struct U {}
981 impl T for U {
982 fn a() {
983 // this comes out of bitflags, mostly
984 #[allow(non_snake_case)]
985 trait __BitFlags {
986 const HiImAlsoBad: u8 = 2;
987 #[inline]
988 fn Dirty(&self) -> bool {
989 false
990 }
991 }
992
993 }
994 }
995 "#,
996 );
997 }
998
999 #[test]
1000 #[ignore]
1001 fn bug_traits_arent_checked() {
1002 // FIXME: Traits and functions in traits aren't currently checked by
1003 // r-a, even though rustc will complain about them.
1004 check_diagnostics(
1005 r#"
1006 trait BAD_TRAIT {
1007 // ^^^^^^^^^ Trait `BAD_TRAIT` should have CamelCase name, e.g. `BadTrait`
1008 fn BAD_FUNCTION();
1009 // ^^^^^^^^^^^^ Function `BAD_FUNCTION` should have snake_case name, e.g. `bad_function`
1010 fn BadFunction();
1011 // ^^^^^^^^^^^^ Function `BadFunction` should have snake_case name, e.g. `bad_function`
1012 }
1013 "#,
1014 );
1015 }
1016
1017 #[test]
892 fn ignores_extern_items() { 1018 fn ignores_extern_items() {
893 cov_mark::check!(extern_func_incorrect_case_ignored); 1019 cov_mark::check!(extern_func_incorrect_case_ignored);
894 cov_mark::check!(extern_static_incorrect_case_ignored); 1020 cov_mark::check!(extern_static_incorrect_case_ignored);
895 check_diagnostics( 1021 check_diagnostics(
896 r#" 1022 r#"
897extern { 1023 extern {
898 fn NonSnakeCaseName(SOME_VAR: u8) -> u8; 1024 fn NonSnakeCaseName(SOME_VAR: u8) -> u8;
899 pub static SomeStatic: u8 = 10; 1025 pub static SomeStatic: u8 = 10;
900} 1026 }
901 "#, 1027 "#,
902 ); 1028 );
903 } 1029 }