squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Merge #6421

Browse source 
6421: Check for allow(..) attributes in case check diagnostic r=popzxc a=popzxc

Resolves #6348

This is not a full-fledged solution, as it doesn't looks up for parent elements (e.g. function -> module -> parent module -> crate root), but it does at least checks attributes of item being checked.
I played a bit with code, and it seems that implementing a proper solution (which will also check for `deny` / `warn` attributes overriding values for `allow`s from above).

So, this solution should fix all the macros which intentionally do "weird" naming and wrap it with `allow`, such as `lazy_static`.

cc @ArifRoktim 


Co-authored-by: Igor Aleksanov <popzxc@yandex.ru>
This commit is contained in:
bors[bot] 2020-11-03 07:36:49 +00:00 committed by GitHub
commit 65b44d2ba5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 96 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

126
crates/hir_ty/src/diagnostics/decl_check.rs
View file

@ -16,7 +16,7 @@ use hir_def::{
adt::VariantData,
expr::{Pat, PatId},
src::HasSource,
AdtId, ConstId, EnumId, FunctionId, Lookup, ModuleDefId, StaticId, StructId,
AdtId, AttrDefId, ConstId, EnumId, FunctionId, Lookup, ModuleDefId, StaticId, StructId,
};
use hir_expand::{
diagnostics::DiagnosticSink,
@ -32,6 +32,12 @@ use crate::{
diagnostics::{decl_check::case_conv::*, CaseType, IncorrectCase},
};
mod allow {
pub(super) const NON_SNAKE_CASE: &str = "non_snake_case";
pub(super) const NON_UPPER_CASE_GLOBAL: &str = "non_upper_case_globals";
pub(super) const NON_CAMEL_CASE_TYPES: &str = "non_camel_case_types";
}
pub(super) struct DeclValidator<'a, 'b: 'a> {
owner: ModuleDefId,
sink: &'a mut DiagnosticSink<'b>,
@ -72,11 +78,29 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
}
}
/// Checks whether not following the convention is allowed for this item.
///
/// Currently this method doesn't check parent attributes.
fn allowed(&self, db: &dyn HirDatabase, id: AttrDefId, allow_name: &str) -> bool {
db.attrs(id).by_key("allow").tt_values().any(|tt| tt.to_string().contains(allow_name))
}
fn validate_func(&mut self, db: &dyn HirDatabase, func: FunctionId) {
let data = db.function_data(func);
let body = db.body(func.into());
// 1. Check the function name.
// Recursively validate inner scope items, such as static variables and constants.
for (item_id, _) in body.item_scope.values() {
let mut validator = DeclValidator::new(item_id, self.sink);
validator.validate_item(db);
}
// Check whether non-snake case identifiers are allowed for this function.
if self.allowed(db, func.into(), allow::NON_SNAKE_CASE) {
return;
}
// 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 {
@ -89,7 +113,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
None
};
// 2. Check the param names.
// Check the param names.
let mut fn_param_replacements = Vec::new();
for pat_id in body.params.iter().cloned() {
@ -111,7 +135,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
}
}
// 3. Check the patterns inside the function body.
// Check the patterns inside the function body.
let mut pats_replacements = Vec::new();
for (pat_idx, pat) in body.pats.iter() {
@ -136,7 +160,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
}
}
// 4. If there is at least one element to spawn a warning on, go to the source map and generate a warning.
// 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,
@ -144,12 +168,6 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
fn_param_replacements,
);
self.create_incorrect_case_diagnostic_for_variables(func, db, pats_replacements);
// 5. Recursively validate inner scope items, such as static variables and constants.
for (item_id, _) in body.item_scope.values() {
let mut validator = DeclValidator::new(item_id, self.sink);
validator.validate_item(db);
}
}
/// Given the information about incorrect names in the function declaration, looks up into the source code
@ -169,7 +187,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
let fn_loc = func.lookup(db.upcast());
let fn_src = fn_loc.source(db.upcast());
// 1. Diagnostic for function name.
// Diagnostic for function name.
if let Some(replacement) = fn_name_replacement {
let ast_ptr = match fn_src.value.name() {
Some(name) => name,
@ -196,7 +214,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
self.sink.push(diagnostic);
}
// 2. Diagnostics for function params.
// Diagnostics for function params.
let fn_params_list = match fn_src.value.param_list() {
Some(params) => params,
None => {
@ -312,7 +330,11 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
fn validate_struct(&mut self, db: &dyn HirDatabase, struct_id: StructId) {
let data = db.struct_data(struct_id);
// 1. Check the structure name.
let non_camel_case_allowed =
self.allowed(db, struct_id.into(), allow::NON_CAMEL_CASE_TYPES);
let non_snake_case_allowed = self.allowed(db, struct_id.into(), allow::NON_SNAKE_CASE);
// Check the structure name.
let struct_name = data.name.to_string();
let struct_name_replacement = if let Some(new_name) = to_camel_case(&struct_name) {
let replacement = Replacement {
@ -320,29 +342,35 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
suggested_text: new_name,
expected_case: CaseType::UpperCamelCase,
};
Some(replacement)
if non_camel_case_allowed {
None
} else {
Some(replacement)
}
} else {
None
};
// 2. Check the field names.
// Check the field names.
let mut struct_fields_replacements = Vec::new();
if let VariantData::Record(fields) = data.variant_data.as_ref() {
for (_, field) in fields.iter() {
let field_name = field.name.to_string();
if let Some(new_name) = to_lower_snake_case(&field_name) {
let replacement = Replacement {
current_name: field.name.clone(),
suggested_text: new_name,
expected_case: CaseType::LowerSnakeCase,
};
struct_fields_replacements.push(replacement);
if !non_snake_case_allowed {
if let VariantData::Record(fields) = data.variant_data.as_ref() {
for (_, field) in fields.iter() {
let field_name = field.name.to_string();
if let Some(new_name) = to_lower_snake_case(&field_name) {
let replacement = Replacement {
current_name: field.name.clone(),
suggested_text: new_name,
expected_case: CaseType::LowerSnakeCase,
};
struct_fields_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.
// 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_struct(
struct_id,
db,
@ -442,7 +470,12 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
fn validate_enum(&mut self, db: &dyn HirDatabase, enum_id: EnumId) {
let data = db.enum_data(enum_id);
// 1. Check the enum name.
// Check whether non-camel case names are allowed for this enum.
if self.allowed(db, enum_id.into(), allow::NON_CAMEL_CASE_TYPES) {
return;
}
// Check the enum name.
let enum_name = data.name.to_string();
let enum_name_replacement = if let Some(new_name) = to_camel_case(&enum_name) {
let replacement = Replacement {
@ -455,7 +488,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
None
};
// 2. Check the field names.
// Check the field names.
let mut enum_fields_replacements = Vec::new();
for (_, variant) in data.variants.iter() {
@ -470,7 +503,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
}
}
// 3. If there is at least one element to spawn a warning on, go to the source map and generate a warning.
// 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_enum(
enum_id,
db,
@ -572,6 +605,10 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
fn validate_const(&mut self, db: &dyn HirDatabase, const_id: ConstId) {
let data = db.const_data(const_id);
if self.allowed(db, const_id.into(), allow::NON_UPPER_CASE_GLOBAL) {
return;
}
let name = match &data.name {
Some(name) => name,
None => return,
@ -612,6 +649,10 @@ impl<'a, 'b> DeclValidator<'a, 'b> {
fn validate_static(&mut self, db: &dyn HirDatabase, static_id: StaticId) {
let data = db.static_data(static_id);
if self.allowed(db, static_id.into(), allow::NON_UPPER_CASE_GLOBAL) {
return;
}
let name = match &data.name {
Some(name) => name,
None => return,
@ -854,4 +895,29 @@ fn main() {
"#,
);
}
#[test]
fn allow_attributes() {
check_diagnostics(
r#"
#[allow(non_snake_case)]
fn NonSnakeCaseName(SOME_VAR: u8) -> u8{
let OtherVar = SOME_VAR + 1;
OtherVar
}
#[allow(non_snake_case, non_camel_case_types)]
pub struct some_type {
SOME_FIELD: u8,
SomeField: u16,
}
#[allow(non_upper_case_globals)]
pub const some_const: u8 = 10;
#[allow(non_upper_case_globals)]
pub static SomeStatic: u8 = 10;
"#,
);
}
}