Skip to content

Commit

Permalink
feat(camelcase): allow for non-declaration object destructuring and n…
Browse files Browse the repository at this point in the history 
…amed import when has no alias (#1187)
  • Loading branch information
@dsherret
dsherret authored Aug 31, 2023
1 parent cbc93b7 commit 57c0efa
Showing 1 changed file with 92 additions and 87 deletions.
179 changes: 92 additions & 87 deletions src/rules/camelcase.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -115,6 +116,7 @@ enum IdentToCheck {
key_name: String,
value_name: Option<String>,
has_default: bool,
is_destructuring: bool,
},
/// Local name and imported name in named import, for example:
///
Expand Down Expand Up @@ -177,6 +179,7 @@ impl IdentToCheck {
key_name: &K,
value_name: Option<&V>,
has_default: bool,
is_destructuring: bool,
) -> Self
where
K: AsRef<str>,
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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),
);
}

Expand Down Expand Up @@ -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()),
),
);
}
Expand All @@ -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()),
),
);
}
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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::*;
Expand Down Expand Up @@ -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 }`",
),
Expand All @@ -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`",
),
Expand All @@ -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`",
),
Expand Down Expand Up @@ -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";"#,
Expand Down Expand Up @@ -989,48 +1050,20 @@ 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,
message: "Identifier 'category_id' is not in camel case.",
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,
message: "Identifier 'no_camelcased' is not in camel case.",
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,
Expand All @@ -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,
Expand Down Expand Up @@ -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 }) {}"#: [
Expand All @@ -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;"#: [
Expand All @@ -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() {};"#: [
Expand Down

0 comments on commit 57c0efa

Please sign in to comment.