From 57c0efaa5514ca9e7c0bc960e5c82d46de2c11b4 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 31 Aug 2023 09:14:01 -0500 Subject: [PATCH] feat(camelcase): allow for non-declaration object destructuring and named import when has no alias (#1187) --- src/rules/camelcase.rs | 179 +++++++++++++++++++++-------------------- 1 file changed, 92 insertions(+), 87 deletions(-) diff --git a/src/rules/camelcase.rs b/src/rules/camelcase.rs index eb5f56c53..ee32c78f9 100644 --- a/src/rules/camelcase.rs +++ b/src/rules/camelcase.rs @@ -3,6 +3,7 @@ use super::{Context, LintRule}; use crate::handler::{Handler, Traverse}; use crate::swc_util::StringRepr; +use deno_ast::view::{Node, NodeKind, NodeTrait}; use deno_ast::{view as ast_view, SourceRange, SourceRanged}; use once_cell::sync::Lazy; use regex::{Captures, Regex}; @@ -115,6 +116,7 @@ enum IdentToCheck { key_name: String, value_name: Option, has_default: bool, + is_destructuring: bool, }, /// Local name and imported name in named import, for example: /// @@ -177,6 +179,7 @@ impl IdentToCheck { key_name: &K, value_name: Option<&V>, has_default: bool, + is_destructuring: bool, ) -> Self where K: AsRef, @@ -186,6 +189,7 @@ impl IdentToCheck { key_name: key_name.as_ref().to_string(), value_name: value_name.map(|v| v.as_ref().to_string()), has_default, + is_destructuring, } } @@ -286,12 +290,20 @@ impl IdentToCheck { key_name, value_name, has_default, + is_destructuring: in_var_decl, } => { - if let Some(value_name) = value_name { + let rename_name = if let Some(value_name) = value_name { + Some(value_name) + } else if *in_var_decl { + None + } else { + Some(key_name) + }; + if let Some(name) = rename_name { return format!( "Consider renaming `{}` to `{}`", - value_name, - to_camelcase(value_name), + name, + to_camelcase(name), ); } @@ -427,6 +439,7 @@ impl CamelcaseHandler { &key.string_repr().unwrap_or_else(|| "[KEY]".to_string()), Some(&value_ident.id.inner), false, + pat_in_var_declarator(pat.into()), ), ); } @@ -440,6 +453,7 @@ impl CamelcaseHandler { &key.string_repr().unwrap_or_else(|| "[KEY]".to_string()), Some(&value_ident.id.inner), true, + pat_in_var_declarator(pat.into()), ), ); } @@ -453,14 +467,18 @@ impl CamelcaseHandler { .. }) => { let has_default = value.is_some(); - self.check_ident( - key, - IdentToCheck::object_pat::<&str, &str>( - &key.inner.as_ref(), - None, - has_default, - ), - ); + let in_var_declarator = pat_in_var_declarator(pat.into()); + if !in_var_declarator { + self.check_ident( + key, + IdentToCheck::object_pat::<&str, &str>( + &key.inner.as_ref(), + None, + has_default, + in_var_declarator, + ), + ); + } } ast_view::ObjectPatProp::Rest(ast_view::RestPat { ref arg, @@ -587,16 +605,18 @@ impl Handler for CamelcaseHandler { let ast_view::ImportNamedSpecifier { local, imported, .. } = import_named_specifier; - self.check_ident( - local, - IdentToCheck::named_import( - local.inner, - imported.as_ref().map(|i| match i { - ast_view::ModuleExportName::Ident(ident) => ident.sym(), - ast_view::ModuleExportName::Str(str) => str.value(), - }), - ), - ); + if let Some(imported) = &imported { + self.check_ident( + local, + IdentToCheck::named_import( + local.inner, + Some(match imported { + ast_view::ModuleExportName::Ident(ident) => ident.sym(), + ast_view::ModuleExportName::Str(str) => str.value(), + }), + ), + ); + } } fn import_default_specifier( @@ -719,6 +739,28 @@ impl Handler for CamelcaseHandler { } } +fn pat_in_var_declarator(pat: Node) -> bool { + for ancestor in pat.ancestors() { + match ancestor.kind() { + NodeKind::VarDeclarator => { + return true; + } + NodeKind::ArrayPat + | NodeKind::ObjectPat + | NodeKind::AssignPat + | NodeKind::AssignPatProp + | NodeKind::RestPat + | NodeKind::KeyValuePatProp => { + // keep going + } + _ => { + return false; + } + } + } + false +} + #[cfg(test)] mod tests { use super::*; @@ -788,6 +830,7 @@ mod tests { key_name: s("foo_bar"), value_name: None, has_default: false, + is_destructuring: true, }, "Consider replacing `{ foo_bar }` with `{ foo_bar: fooBar }`", ), @@ -796,6 +839,7 @@ mod tests { key_name: s("foo_bar"), value_name: Some(s("snake_case")), has_default: false, + is_destructuring: true, }, "Consider renaming `snake_case` to `snakeCase`", ), @@ -804,14 +848,26 @@ mod tests { key_name: s("foo_bar"), value_name: None, has_default: true, + is_destructuring: true, }, "Consider replacing `{ foo_bar = .. }` with `{ foo_bar: fooBar = .. }`", ), + ( + IdentToCheck::ObjectPat { + key_name: s("foo_bar"), + value_name: None, + has_default: true, + is_destructuring: false, + }, + // not destructuring, so suggest a rename + "Consider renaming `foo_bar` to `fooBar`", + ), ( IdentToCheck::ObjectPat { key_name: s("foo_bar"), value_name: Some(s("snake_case")), has_default: true, + is_destructuring: true, }, "Consider renaming `snake_case` to `snakeCase`", ), @@ -869,9 +925,14 @@ mod tests { r#"var { category_id: category } = query;"#, r#"var { _leading } = query;"#, r#"var { trailing_ } = query;"#, + r#"var { or_middle } = query;"#, + r#"var { category_id = 1 } = query;"#, + r#"var { category_id: { property_test } } = query;"#, + r#"const { no_camelcased = false } = bar;"#, r#"import { camelCased } from "external module";"#, r#"import { _leading } from "external module";"#, r#"import { trailing_ } from "external module";"#, + r#"import { or_middle } from "external module";"#, r#"import { no_camelcased as camelCased } from "external-module";"#, r#"import { no_camelcased as _leading } from "external-module";"#, r#"import { no_camelcased as trailing_ } from "external-module";"#, @@ -989,13 +1050,6 @@ mod tests { hint: "Consider renaming `category_alias` to `categoryAlias`", } ], - r#"var { category_id } = query;"#: [ - { - col: 6, - message: "Identifier 'category_id' is not in camel case.", - hint: "Consider replacing `{ category_id }` with `{ category_id: categoryId }`", - } - ], r#"var { category_id: category_id } = query;"#: [ { col: 19, @@ -1003,20 +1057,6 @@ mod tests { hint: "Consider renaming `category_id` to `categoryId`", } ], - r#"var { category_id = 1 } = query;"#: [ - { - col: 6, - message: "Identifier 'category_id' is not in camel case.", - hint: "Consider replacing `{ category_id = .. }` with `{ category_id: categoryId = .. }`", - } - ], - r#"import no_camelcased from "external-module";"#: [ - { - col: 7, - message: "Identifier 'no_camelcased' is not in camel case.", - hint: "Consider renaming `no_camelcased` to `noCamelcased`", - } - ], r#"import * as no_camelcased from "external-module";"#: [ { col: 12, @@ -1024,13 +1064,6 @@ mod tests { hint: "Consider renaming `no_camelcased` to `noCamelcased`", } ], - r#"import { no_camelcased } from "external-module";"#: [ - { - col: 9, - message: "Identifier 'no_camelcased' is not in camel case.", - hint: "Consider replacing `{ no_camelcased }` with `{ no_camelcased as noCamelcased }`", - } - ], r#"import { no_camelcased as no_camel_cased } from "external module";"#: [ { col: 26, @@ -1045,27 +1078,6 @@ mod tests { hint: "Consider renaming `no_camel_cased` to `noCamelCased`", } ], - r#"import { camelCased, no_camelcased } from "external-module";"#: [ - { - col: 21, - message: "Identifier 'no_camelcased' is not in camel case.", - hint: "Consider replacing `{ no_camelcased }` with `{ no_camelcased as noCamelcased }`", - } - ], - r#"import { no_camelcased as camelCased, another_no_camelcased } from "external-module";"#: [ - { - col: 38, - message: "Identifier 'another_no_camelcased' is not in camel case.", - hint: "Consider replacing `{ another_no_camelcased }` with `{ another_no_camelcased as anotherNoCamelcased }`", - } - ], - r#"import camelCased, { no_camelcased } from "external-module";"#: [ - { - col: 21, - message: "Identifier 'no_camelcased' is not in camel case.", - hint: "Consider replacing `{ no_camelcased }` with `{ no_camelcased as noCamelcased }`", - } - ], r#"import no_camelcased, { another_no_camelcased as camelCased } from "external-module";"#: [ { col: 7, @@ -1098,14 +1110,14 @@ mod tests { { col: 15, message: "Identifier 'no_camelcased' is not in camel case.", - hint: "Consider replacing `{ no_camelcased }` with `{ no_camelcased: noCamelcased }`", + hint: "Consider renaming `no_camelcased` to `noCamelcased`", } ], r#"function foo({ no_camelcased = 'default value' }) {};"#: [ { col: 15, message: "Identifier 'no_camelcased' is not in camel case.", - hint: "Consider replacing `{ no_camelcased = .. }` with `{ no_camelcased: noCamelcased = .. }`", + hint: "Consider renaming `no_camelcased` to `noCamelcased`", } ], r#"const no_camelcased = 0; function foo({ camelcased_value = no_camelcased }) {}"#: [ @@ -1117,7 +1129,7 @@ mod tests { { col: 40, message: "Identifier 'camelcased_value' is not in camel case.", - hint: "Consider replacing `{ camelcased_value = .. }` with `{ camelcased_value: camelcasedValue = .. }`", + hint: "Consider renaming `camelcased_value` to `camelcasedValue`", } ], r#"const { bar: no_camelcased } = foo;"#: [ @@ -1141,25 +1153,18 @@ mod tests { hint: "Consider renaming `no_camelcased` to `noCamelcased`", } ], - r#"var { foo: bar_baz = 1 } = quz;"#: [ - { - col: 11, - message: "Identifier 'bar_baz' is not in camel case.", - hint: "Consider renaming `bar_baz` to `barBaz`", - } - ], - r#"const { no_camelcased = false } = bar;"#: [ + r#"function foo({ isCamelcased: { no_camelcased } }) {};"#: [ { - col: 8, + col: 31, message: "Identifier 'no_camelcased' is not in camel case.", - hint: "Consider replacing `{ no_camelcased = .. }` with `{ no_camelcased: noCamelcased = .. }`", + hint: "Consider renaming `no_camelcased` to `noCamelcased`", } ], - r#"const { no_camelcased = foo_bar } = bar;"#: [ + r#"var { foo: bar_baz = 1 } = quz;"#: [ { - col: 8, - message: "Identifier 'no_camelcased' is not in camel case.", - hint: "Consider replacing `{ no_camelcased = .. }` with `{ no_camelcased: noCamelcased = .. }`", + col: 11, + message: "Identifier 'bar_baz' is not in camel case.", + hint: "Consider renaming `bar_baz` to `barBaz`", } ], r#"const f = function no_camelcased() {};"#: [