From 45ac2b2edec05e417124ebfc2e61ec2a5117f4d5 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Sun, 4 Oct 2020 08:53:34 +0300 Subject: [PATCH] Code style adjustments --- crates/hir_ty/src/diagnostics/decl_check.rs | 52 ++++++++++++++++++- .../src/diagnostics/decl_check/str_helpers.rs | 38 ++++++++++++-- crates/hir_ty/src/diagnostics/unsafe_check.rs | 6 +-- 3 files changed, 88 insertions(+), 8 deletions(-) diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index d1c51849a3a..3a95f1b82a1 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -19,7 +19,7 @@ }; use syntax::{ ast::{self, NameOwner}, - AstPtr, + AstNode, AstPtr, }; use crate::{ @@ -259,6 +259,21 @@ fn create_incorrect_case_diagnostic_for_variables( if let Some(expr) = source_ptr.value.as_ref().left() { let root = source_ptr.file_syntax(db.upcast()); if let ast::Pat::IdentPat(ident_pat) = expr.to_node(&root) { + let parent = match ident_pat.syntax().parent() { + Some(parent) => parent, + None => continue, + }; + + // We have to check that it's either `let var = ...` or `Variant(_) @ var` statement, + // because e.g. match arms are patterns as well. + // In other words, we check that it's a named variable binding. + if !ast::LetStmt::cast(parent.clone()).is_some() + && !ast::IdentPat::cast(parent).is_some() + { + // This pattern is not an actual variable declaration, e.g. `Some(val) => {..}` match arm. + continue; + } + let diagnostic = IncorrectCase { file: source_ptr.file_id, ident_type: "Variable".to_string(), @@ -663,7 +678,7 @@ fn incorrect_variable_names() { r#" fn foo() { let SOME_VALUE = 10; - // ^^^^^^^^^^ Variable `SOME_VALUE` should have a snake_case name, e.g. `some_value` + // ^^^^^^^^^^ Variable `SOME_VALUE` should have snake_case name, e.g. `some_value` let AnotherValue = 20; // ^^^^^^^^^^^^ Variable `AnotherValue` should have snake_case name, e.g. `another_value` } @@ -758,6 +773,39 @@ fn SomeFunc(&self) { // ^^^^^^^^^^^^^^^ Variable `WHY_VAR_IS_CAPS` should have snake_case name, e.g. `why_var_is_caps` } } +"#, + ); + } + + #[test] + fn no_diagnostic_for_enum_varinats() { + check_diagnostics( + r#" +enum Option { Some, None } + +fn main() { + match Option::None { + None => (), + Some => (), + } +} +"#, + ); + } + + #[test] + fn non_let_bind() { + check_diagnostics( + r#" +enum Option { Some, None } + +fn main() { + match Option::None { + None @ SOME_VAR => (), + // ^^^^^^^^ Variable `SOME_VAR` should have snake_case name, e.g. `some_var` + Some => (), + } +} "#, ); } diff --git a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs index e3826909bb0..8f70c5e846d 100644 --- a/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs +++ b/crates/hir_ty/src/diagnostics/decl_check/str_helpers.rs @@ -1,3 +1,6 @@ +//! Functions for string case manipulation, such as detecting the identifier case, +//! and converting it into appropriate form. + #[derive(Debug)] enum DetectedCase { LowerCamelCase, @@ -44,6 +47,8 @@ fn detect_case(ident: &str) -> DetectedCase { } } +/// Converts an identifier to an UpperCamelCase form. +/// Returns `None` if the string is already is UpperCamelCase. pub fn to_camel_case(ident: &str) -> Option { let detected_case = detect_case(ident); @@ -87,9 +92,17 @@ pub fn to_camel_case(ident: &str) -> Option { } } - Some(output) + if output == ident { + // While we didn't detect the correct case at the beginning, there + // may be special cases: e.g. `A` is both valid CamelCase and UPPER_SNAKE_CASE. + None + } else { + Some(output) + } } +/// Converts an identifier to a lower_snake_case form. +/// Returns `None` if the string is already is lower_snake_case. pub fn to_lower_snake_case(ident: &str) -> Option { // First, assume that it's UPPER_SNAKE_CASE. match detect_case(ident) { @@ -102,9 +115,18 @@ pub fn to_lower_snake_case(ident: &str) -> Option { // Otherwise, assume that it's CamelCase. let lower_snake_case = stdx::to_lower_snake_case(ident); - Some(lower_snake_case) + + if lower_snake_case == ident { + // While we didn't detect the correct case at the beginning, there + // may be special cases: e.g. `a` is both valid camelCase and snake_case. + None + } else { + Some(lower_snake_case) + } } +/// Converts an identifier to an UPPER_SNAKE_CASE form. +/// Returns `None` if the string is already is UPPER_SNAKE_CASE. pub fn to_upper_snake_case(ident: &str) -> Option { match detect_case(ident) { DetectedCase::UpperSnakeCase => return None, @@ -117,7 +139,14 @@ pub fn to_upper_snake_case(ident: &str) -> Option { // Normalize the string from whatever form it's in currently, and then just make it uppercase. let upper_snake_case = stdx::to_lower_snake_case(ident).chars().map(|c| c.to_ascii_uppercase()).collect(); - Some(upper_snake_case) + + if upper_snake_case == ident { + // While we didn't detect the correct case at the beginning, there + // may be special cases: e.g. `A` is both valid CamelCase and UPPER_SNAKE_CASE. + None + } else { + Some(upper_snake_case) + } } #[cfg(test)] @@ -139,6 +168,7 @@ fn test_to_lower_snake_case() { check(to_lower_snake_case, "Weird_Case", expect![["weird_case"]]); check(to_lower_snake_case, "CamelCase", expect![["camel_case"]]); check(to_lower_snake_case, "lowerCamelCase", expect![["lower_camel_case"]]); + check(to_lower_snake_case, "a", expect![[""]]); } #[test] @@ -151,6 +181,7 @@ fn test_to_camel_case() { check(to_camel_case, "UPPER_SNAKE_CASE", expect![["UpperSnakeCase"]]); check(to_camel_case, "Weird_Case", expect![["WeirdCase"]]); check(to_camel_case, "name", expect![["Name"]]); + check(to_camel_case, "A", expect![[""]]); } #[test] @@ -160,5 +191,6 @@ fn test_to_upper_snake_case() { check(to_upper_snake_case, "Weird_Case", expect![["WEIRD_CASE"]]); check(to_upper_snake_case, "CamelCase", expect![["CAMEL_CASE"]]); check(to_upper_snake_case, "lowerCamelCase", expect![["LOWER_CAMEL_CASE"]]); + check(to_upper_snake_case, "A", expect![[""]]); } } diff --git a/crates/hir_ty/src/diagnostics/unsafe_check.rs b/crates/hir_ty/src/diagnostics/unsafe_check.rs index 61ffbf5d151..21a121aad7e 100644 --- a/crates/hir_ty/src/diagnostics/unsafe_check.rs +++ b/crates/hir_ty/src/diagnostics/unsafe_check.rs @@ -190,13 +190,13 @@ struct Ty { a: u8, } -static mut static_mut: Ty = Ty { a: 0 }; +static mut STATIC_MUT: Ty = Ty { a: 0 }; fn main() { - let x = static_mut.a; + let x = STATIC_MUT.a; //^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block unsafe { - let x = static_mut.a; + let x = STATIC_MUT.a; } } "#,