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

fix(linter) improve span of bad-remove-event-listener, merge rules #1734

Merged
merged 1 commit into from
Dec 18, 2023
Merged

fix(linter) improve span of bad-remove-event-listener, merge rules #1734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 0 additions & 2 deletions crates/oxc_linter/src/rules.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ mod deepscan {
pub mod bad_bitwise_operator;
pub mod bad_comparison_sequence;
pub mod bad_min_max_func;
pub mod bad_remove_event_listener;
pub mod missing_throw;
pub mod number_arg_out_of_range;
pub mod uninvoked_array_callback;
Expand Down Expand Up @@ -249,7 +248,6 @@ oxc_macros::declare_all_lint_rules! {
deepscan::bad_bitwise_operator,
deepscan::bad_comparison_sequence,
deepscan::bad_min_max_func,
deepscan::bad_remove_event_listener,
deepscan::missing_throw,
deepscan::number_arg_out_of_range,
deepscan::uninvoked_array_callback,
Expand Down
65 changes: 0 additions & 65 deletions crates/oxc_linter/src/rules/deepscan/bad_remove_event_listener.rs
View file
Open in desktop

This file was deleted.

68 changes: 64 additions & 4 deletions crates/oxc_linter/src/rules/unicorn/no_invalid_remove_event_listener.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ use crate::{context::LintContext, rule::Rule, AstNode};
"eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call."
)]
#[diagnostic(severity(warning), help("The listener argument should be a function reference."))]
struct NoInvalidRemoveEventListenerDiagnostic(#[label] pub Span);
struct NoInvalidRemoveEventListenerDiagnostic(
#[label("`removeEventListener` called here.")] pub Span,
#[label("Invalid argument here")] pub Span,
);

#[derive(Debug, Default, Clone)]
pub struct NoInvalidRemoveEventListener;
Expand Down Expand Up @@ -55,14 +58,15 @@ impl Rule for NoInvalidRemoveEventListener {

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

match member_expr {
let remove_event_listener_ident_span = match member_expr {
MemberExpression::StaticMemberExpression(v) => {
if v.property.name != "removeEventListener" {
return;
}
v.property.span
}
_ => return,
}
};

if member_expr.optional() {
return;
Expand Down Expand Up @@ -96,7 +100,26 @@ impl Rule for NoInvalidRemoveEventListener {
}
}

ctx.diagnostic(NoInvalidRemoveEventListenerDiagnostic(listener.span()));
let listener_span = listener.span();
let listener_span = if listener_span.size() > 20 {
match listener {
Expression::FunctionExpression(func_expr) => {
Span::new(func_expr.span.start, func_expr.params.span.end)
}
Expression::ArrowExpression(arrow_expr) => {
Span::new(arrow_expr.span.start, arrow_expr.body.span.start)
}
Expression::CallExpression(_) => listener_span,
_ => unreachable!(),
}
} else {
listener_span
};

ctx.diagnostic(NoInvalidRemoveEventListenerDiagnostic(
remove_event_listener_ident_span,
listener_span,
));
}
}

Expand Down Expand Up @@ -126,6 +149,8 @@ fn test() {
r#"el.removeEventListener("keydown", obj.listener)"#,
r#"removeEventListener("keyup", () => {})"#,
r#"removeEventListener("keydown", function () {})"#,
"document.removeEventListener('keydown', keydownHandler)",
"document.removeEventListener('keydown', this.keydownHandler)",
];

let fail = vec![
Expand All @@ -139,6 +164,41 @@ fn test() {
r#"el.removeEventListener("mouseout", function (e) {}, true)"#,
r#"el.removeEventListener("click", function (e) {}, ...moreArguments)"#,
r"el.removeEventListener(() => {}, () => {}, () => {})",
"document.removeEventListener('keydown', () => foo())",
"document.removeEventListener('keydown', function () {})",
// make sure that if the listener is a big one, we shorten the span.
r#"
element.removeEventListener("glider-refresh", event => {
// $ExpectType GliderEvent<undefined>
event;

// $ExpectType boolean
event.bubbles;

event.target;

if (event.target) {
// $ExpectType Glider<HTMLElement> | undefined
event.target._glider;
}
});
"#,
r#"
element.removeEventListener("glider-refresh", function (event) {
// $ExpectType GliderEvent<undefined>
event;

// $ExpectType boolean
event.bubbles;

event.target;

if (event.target) {
// $ExpectType Glider<HTMLElement> | undefined
event.target._glider;
}
});
"#,
];

Tester::new_without_config(NoInvalidRemoveEventListener::NAME, pass, fail).test_and_snapshot();
Expand Down
19 changes: 0 additions & 19 deletions crates/oxc_linter/src/snapshots/bad_remove_event_listener.snap
View file
Open in desktop

This file was deleted.

80 changes: 70 additions & 10 deletions crates/oxc_linter/src/snapshots/no_invalid_remove_event_listener.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,70 +5,130 @@ expression: no_invalid_remove_event_listener
⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ window.removeEventListener("scroll", handler.bind(abc))
· ─────────────────
· ─────────┬───────── ────────┬────────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ window.removeEventListener("scroll", this.handler.bind(abc))
· ──────────────────────
· ─────────┬───────── ───────────┬──────────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ window.removeEventListener("click", () => {})
· ────────
· ─────────┬───────── ────┬───
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ window.removeEventListener("keydown", function () {})
· ──────────────
· ─────────┬───────── ───────┬──────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ el.removeEventListener("click", (e) => { e.preventDefault(); })
· ──────────────────────────────
· ─────────┬───────── ───┬───
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ el.removeEventListener("mouseover", fn.bind(abc))
· ────────────
· ─────────┬───────── ──────┬─────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ el.removeEventListener("mouseout", function (e) {})
· ───────────────
· ─────────┬───────── ───────┬───────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ el.removeEventListener("mouseout", function (e) {}, true)
· ───────────────
· ─────────┬───────── ───────┬───────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ el.removeEventListener("click", function (e) {}, ...moreArguments)
· ───────────────
· ─────────┬───────── ───────┬───────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ el.removeEventListener(() => {}, () => {}, () => {})
· ────────
· ─────────┬───────── ────┬───
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ document.removeEventListener('keydown', () => foo())
· ─────────┬───────── ─────┬─────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │ document.removeEventListener('keydown', function () {})
· ─────────┬───────── ───────┬──────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │
2 │ element.removeEventListener("glider-refresh", event => {
· ─────────┬───────── ────┬────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
3 │ // $ExpectType GliderEvent<undefined>
╰────
help: The listener argument should be a function reference.

⚠ eslint-plugin-unicorn(no-invalid-remove-event-listener): Invalid `removeEventListener` call.
╭─[no_invalid_remove_event_listener.tsx:1:1]
1 │
2 │ element.removeEventListener("glider-refresh", function (event) {
· ─────────┬───────── ────────┬───────
· │ ╰── Invalid argument here
· ╰── `removeEventListener` called here.
3 │ // $ExpectType GliderEvent<undefined>
╰────
help: The listener argument should be a function reference.

Expand Down