Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(camelcase): allow for non-declaration object destructuring and named import when has no alias #1187

Merged
merged 2 commits into from
Aug 31, 2023
Merged

feat(camelcase): allow for non-declaration object destructuring and named import when has no alias #1187

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12fc2eb
feat(camelcase): allow for object destructuring and named import when…
dsherret Aug 16, 2023
540f51f
Improve detection of destructuring or not.
dsherret Aug 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 92 additions & 87 deletions src/rules/camelcase.rs
View file
Open in desktop
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