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.

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

feat(linter): eslint-plugin-unicorn require-array-join-separator(style) #1608

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ mod unicorn {
pub mod prefer_string_starts_ends_with;
pub mod prefer_string_trim_start_end;
pub mod prefer_type_error;
pub mod require_array_join_separator;
pub mod require_number_to_fixed_digits_argument;
pub mod switch_case_braces;
pub mod text_encoding_identifier_case;
Expand Down Expand Up @@ -385,6 +386,7 @@ oxc_macros::declare_all_lint_rules! {
unicorn::prefer_string_starts_ends_with,
unicorn::prefer_string_trim_start_end,
unicorn::prefer_type_error,
unicorn::require_array_join_separator,
unicorn::require_number_to_fixed_digits_argument,
unicorn::switch_case_braces,
unicorn::text_encoding_identifier_case,
Expand Down
182 changes: 182 additions & 0 deletions crates/oxc_linter/src/rules/unicorn/require_array_join_separator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
use oxc_ast::{
ast::{Expression, MemberExpression},
AstKind,
};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};

use crate::{ast_util::is_method_call, context::LintContext, rule::Rule, AstNode};

#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-unicorn(require-array-join-separator): Enforce using the separator argument with Array#join()")]
#[diagnostic(severity(warning), help("Missing the separator argument."))]
struct RequireArrayJoinSeparatorDiagnostic(#[label] pub Span);

#[derive(Debug, Default, Clone)]
pub struct RequireArrayJoinSeparator;

declare_oxc_lint!(
/// ### What it does
///
/// Enforce using the separator argument with Array#join()
///
/// ### Why is this bad?
///
/// It's better to make it clear what the separator is when calling Array#join(),
/// instead of relying on the default comma (',') separator.
///
/// ### Example
/// ```javascript
/// // Bad
/// foo.join()
///
/// // Good
/// foo.join(",")
/// ```
RequireArrayJoinSeparator,
style
);

// ref: https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/rules/utils/array-or-object-prototype-property.js
fn is_prototype_property(
member_expr: &MemberExpression,
property: &str,
object: Option<&str>,
) -> bool {
if !member_expr.static_property_name().is_some_and(|name| name == property)
|| member_expr.optional()
{
return false;
}

// `Object.prototype.method` or `Array.prototype.method`
if let Expression::MemberExpression(member_expr_obj) = member_expr.object() {
if let Expression::Identifier(iden) = member_expr_obj.object() {
if member_expr_obj.static_property_name().is_some_and(|name| name == "prototype")
&& object.is_some_and(|val| val == iden.name)
&& !member_expr.optional()
&& !member_expr_obj.optional()
{
return true;
}
}
};

match object {
// `[].method`
Some("Array") => {
if let Expression::ArrayExpression(array_expr) = member_expr.object() {
array_expr.elements.len() == 0
} else {
false
}
}

// `{}.method`
Some("Object") => {
if let Expression::ObjectExpression(obj_expr) = member_expr.object() {
obj_expr.properties.len() == 0
} else {
false
}
}
_ => false,
}
}

fn is_array_prototype_property(member_expr: &MemberExpression, property: &str) -> bool {
is_prototype_property(member_expr, property, Some("Array"))
}

impl Rule for RequireArrayJoinSeparator {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::CallExpression(call_expr) = node.kind() else {
return;
};

let Some(member_expr) = call_expr.callee.get_member_expr() else {
return;
};

// foo.join()
if is_method_call(call_expr, None, Some(&["join"]), Some(0), Some(0))
&& !call_expr.optional
&& !matches!(member_expr, MemberExpression::ComputedMemberExpression(_))
{
ctx.diagnostic(RequireArrayJoinSeparatorDiagnostic(Span {
start: member_expr.span().end,
end: call_expr.span.end,
}));
}

// `[].join.call(foo)` and `Array.prototype.join.call(foo)`
if let Expression::MemberExpression(member_expr_obj) = member_expr.object() {
if is_method_call(call_expr, None, Some(&["call"]), Some(1), Some(1))
&& !member_expr.optional()
&& !call_expr.optional
&& !call_expr.arguments.iter().any(oxc_ast::ast::Argument::is_spread)
&& is_array_prototype_property(member_expr_obj, "join")
{
ctx.diagnostic(RequireArrayJoinSeparatorDiagnostic(Span {
start: member_expr.span().end,
end: call_expr.span.end,
}));
}
}
}
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
("foo.join(\",\")", None),
(r"join()", None),
(r"foo.join(...[])", None),
(r"foo.join?.()", None),
(r"foo?.join?.()", None),
(r"foo[join]()", None),
("foo[\"join\"]()", None),
("[].join.call(foo, \",\")", None),
(r"[].join.call()", None),
(r"[].join.call(...[foo])", None),
(r"[].join?.call(foo)", None),
(r"[]?.join.call(foo)", None),
(r"[].join[call](foo)", None),
(r"[][join].call(foo)", None),
(r"[,].join.call(foo)", None),
(r"[].join.notCall(foo)", None),
(r"[].notJoin.call(foo)", None),
("Array.prototype.join.call(foo, \"\")", None),
(r"Array.prototype.join.call()", None),
(r"Array.prototype.join.call(...[foo])", None),
(r"Array.prototype.join?.call(foo)", None),
(r"Array.prototype?.join.call(foo)", None),
(r"Array?.prototype.join.call(foo)", None),
("Array.prototype.join[call](foo, \"\")", None),
(r"Array.prototype[join].call(foo)", None),
(r"Array[prototype].join.call(foo)", None),
(r"Array.prototype.join.notCall(foo)", None),
(r"Array.prototype.notJoin.call(foo)", None),
(r"Array.notPrototype.join.call(foo)", None),
(r"NotArray.prototype.join.call(foo)", None),
("path.join(__dirname, \"./foo.js\")", None),
];

let fail = vec![
(r"foo.join()", None),
(r"[].join.call(foo)", None),
(r"[].join.call(foo,)", None),
(r"[].join.call(foo , );", None),
(r"Array.prototype.join.call(foo)", None),
(r"Array.prototype.join.call(foo, )", None),
(r"foo?.join()", None),
];

Tester::new(RequireArrayJoinSeparator::NAME, pass, fail).test_and_snapshot();
}
54 changes: 54 additions & 0 deletions crates/oxc_linter/src/snapshots/require_array_join_separator.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
---
source: crates/oxc_linter/src/tester.rs
expression: require_array_join_separator
---
⚠ eslint-plugin-unicorn(require-array-join-separator): Enforce using the separator argument with Array#join()
╭─[require_array_join_separator.tsx:1:1]
1 │ foo.join()
· ──
╰────
help: Missing the separator argument.

⚠ eslint-plugin-unicorn(require-array-join-separator): Enforce using the separator argument with Array#join()
╭─[require_array_join_separator.tsx:1:1]
1 │ [].join.call(foo)
· ─────
╰────
help: Missing the separator argument.

⚠ eslint-plugin-unicorn(require-array-join-separator): Enforce using the separator argument with Array#join()
╭─[require_array_join_separator.tsx:1:1]
1 │ [].join.call(foo,)
· ──────
╰────
help: Missing the separator argument.

⚠ eslint-plugin-unicorn(require-array-join-separator): Enforce using the separator argument with Array#join()
╭─[require_array_join_separator.tsx:1:1]
1 │ [].join.call(foo , );
· ────────
╰────
help: Missing the separator argument.

⚠ eslint-plugin-unicorn(require-array-join-separator): Enforce using the separator argument with Array#join()
╭─[require_array_join_separator.tsx:1:1]
1 │ Array.prototype.join.call(foo)
· ─────
╰────
help: Missing the separator argument.

⚠ eslint-plugin-unicorn(require-array-join-separator): Enforce using the separator argument with Array#join()
╭─[require_array_join_separator.tsx:1:1]
1 │ Array.prototype.join.call(foo, )
· ───────
╰────
help: Missing the separator argument.

⚠ eslint-plugin-unicorn(require-array-join-separator): Enforce using the separator argument with Array#join()
╭─[require_array_join_separator.tsx:1:1]
1 │ foo?.join()
· ──
╰────
help: Missing the separator argument.


Loading