From 2ec2f7d829247f10d2c72bac77292ddc0c9ffc7a Mon Sep 17 00:00:00 2001 From: Edwin Lim <68587082+edwinlzs@users.noreply.github.com> Date: Fri, 6 Sep 2024 20:40:20 +0800 Subject: [PATCH] feat(linter/eslint): implement no-alert (#5535) Contributing to #479 ! Rule Details: [link](https://eslint.org/docs/latest/rules/no-alert) Am new to the internals of parsers and linters so I kept the implementation similar to ESLint's source code Commented out the `globalThis` test cases in the pass array meant for pre-ES2020 before `globalThis` was introduced, having clarified in the Discord that it's ok to treat the built-in `globalThis` as always present. --------- Co-authored-by: Don Isaac --- crates/oxc_linter/src/rules.rs | 2 + .../oxc_linter/src/rules/eslint/no_alert.rs | 179 ++++++++++++++++++ crates/oxc_linter/src/snapshots/no_alert.snap | 160 ++++++++++++++++ 3 files changed, 341 insertions(+) create mode 100644 crates/oxc_linter/src/rules/eslint/no_alert.rs create mode 100644 crates/oxc_linter/src/snapshots/no_alert.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 11ef4f45ae353..bdaf4d8b6ba8e 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -39,6 +39,7 @@ mod eslint { pub mod max_classes_per_file; pub mod max_lines; pub mod max_params; + pub mod no_alert; pub mod no_array_constructor; pub mod no_async_promise_executor; pub mod no_await_in_loop; @@ -493,6 +494,7 @@ oxc_macros::declare_all_lint_rules! { eslint::no_ternary, eslint::no_this_before_super, eslint::no_template_curly_in_string, + eslint::no_alert, eslint::no_array_constructor, eslint::no_async_promise_executor, eslint::no_bitwise, diff --git a/crates/oxc_linter/src/rules/eslint/no_alert.rs b/crates/oxc_linter/src/rules/eslint/no_alert.rs new file mode 100644 index 0000000000000..3377c0de9f9b7 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_alert.rs @@ -0,0 +1,179 @@ +use oxc_ast::{ast::Expression, AstKind}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::ScopeId; +use oxc_span::{GetSpan, Span}; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +fn no_alert_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("`alert`, `confirm` and `prompt` functions are not allowed") + .with_help("Use a custom UI instead") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoAlert; + +declare_oxc_lint!( + /// ### What it does + /// Disallow the use of alert, confirm, and prompt + /// + /// ### Why is this bad? + /// + /// JavaScript’s alert, confirm, and prompt functions are widely considered to be obtrusive as UI elements and should be replaced by a more appropriate custom UI implementation. + /// Furthermore, alert is often used while debugging code, which should be removed before deployment to production. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// alert("here!"); + /// + /// confirm("Are you sure?"); + /// + /// prompt("What's your name?", "John Doe"); + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// customAlert("Something happened!"); + /// + /// customConfirm("Are you sure?"); + /// + /// customPrompt("Who are you?"); + /// + /// function foo() { + /// var alert = myCustomLib.customAlert; + /// alert(); + /// } + /// ``` + NoAlert, + restriction, +); + +const GLOBAL_THIS: &str = "globalThis"; +const GLOBAL_WINDOW: &str = "window"; + +fn is_prohibited_identifier(value: &str) -> bool { + matches!(value, "alert" | "confirm" | "prompt") +} + +fn is_global_this_ref_or_global_window<'a>( + scope_id: ScopeId, + ctx: &LintContext<'a>, + expr: &Expression<'a>, +) -> bool { + if let Expression::ThisExpression(_) = expr { + if ctx.scopes().get_flags(scope_id).is_top() { + return true; + } + } + + let Some(ident) = expr.get_identifier_reference() else { + return false; + }; + + if ctx.semantic().is_reference_to_global_variable(ident) + && (expr.is_specific_id(GLOBAL_WINDOW) || (expr.is_specific_id(GLOBAL_THIS))) + { + return !is_shadowed(scope_id, ident.name.as_str(), ctx); + } + + false +} + +fn is_shadowed<'a>(scope_id: ScopeId, name: &'a str, ctx: &LintContext<'a>) -> bool { + ctx.scopes().find_binding(scope_id, name).is_some() +} + +impl Rule for NoAlert { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::CallExpression(call_expr) = node.kind() else { + return; + }; + + let scope_id = node.scope_id(); + let callee = &call_expr.callee; + + if let Expression::Identifier(ident) = callee { + let name = ident.name.as_str(); + if !is_shadowed(scope_id, name, ctx) && is_prohibited_identifier(name) { + return ctx.diagnostic(no_alert_diagnostic(ident.span)); + } + + return; + } + + let Some(member_expr) = callee.get_member_expr() else { return }; + if !is_global_this_ref_or_global_window(scope_id, ctx, member_expr.object()) { + return; + } + + let Some(property_name) = member_expr.static_property_name() else { + return; + }; + if is_prohibited_identifier(property_name) { + ctx.diagnostic(no_alert_diagnostic(member_expr.span())); + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "a[o.k](1)", + "foo.alert(foo)", + "foo.confirm(foo)", + "foo.prompt(foo)", + "function alert() {} alert();", + "var alert = function() {}; alert();", + "function foo() { var alert = bar; alert(); }", + "function foo(alert) { alert(); }", + "var alert = function() {}; function test() { alert(); }", + "function foo() { var alert = function() {}; function test() { alert(); } }", + "function confirm() {} confirm();", + "function prompt() {} prompt();", + "window[alert]();", + "function foo() { this.alert(); }", + "function foo() { var window = bar; window.alert(); }", + // "globalThis.alert();", + // "globalThis['alert']();", // { "ecmaVersion": 6 }, + // "globalThis.alert();", // { "ecmaVersion": 2017 }, + "var globalThis = foo; globalThis.alert();", // { "ecmaVersion": 2020 }, + "function foo() { var globalThis = foo; globalThis.alert(); }", // { "ecmaVersion": 2020 } + ]; + + let fail = vec![ + "alert(foo)", + "window.alert(foo)", + "window['alert'](foo)", + "confirm(foo)", + "window.confirm(foo)", + "window['confirm'](foo)", + "prompt(foo)", + "window.prompt(foo)", + "window['prompt'](foo)", + "function alert() {} window.alert(foo)", + "var alert = function() {}; + window.alert(foo)", + "function foo(alert) { window.alert(); }", + "function foo() { alert(); }", + "function foo() { var alert = function() {}; } + alert();", + "this.alert(foo)", + "this['alert'](foo)", + "function foo() { var window = bar; window.alert(); } + window.alert();", + "globalThis['alert'](foo)", // { "ecmaVersion": 2020 }, + "globalThis.alert();", // { "ecmaVersion": 2020 }, + "function foo() { var globalThis = bar; globalThis.alert(); } + globalThis.alert();", // { "ecmaVersion": 2020 }, + "window?.alert(foo)", // { "ecmaVersion": 2020 }, + "(window?.alert)(foo)", // { "ecmaVersion": 2020 } + ]; + + Tester::new(NoAlert::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_alert.snap b/crates/oxc_linter/src/snapshots/no_alert.snap new file mode 100644 index 0000000000000..fccca6c47b87c --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_alert.snap @@ -0,0 +1,160 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ alert(foo) + · ───── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ window.alert(foo) + · ──────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ window['alert'](foo) + · ─────────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ confirm(foo) + · ─────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ window.confirm(foo) + · ────────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ window['confirm'](foo) + · ───────────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ prompt(foo) + · ────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ window.prompt(foo) + · ───────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ window['prompt'](foo) + · ──────────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:21] + 1 │ function alert() {} window.alert(foo) + · ──────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:2:10] + 1 │ var alert = function() {}; + 2 │ window.alert(foo) + · ──────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:23] + 1 │ function foo(alert) { window.alert(); } + · ──────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:18] + 1 │ function foo() { alert(); } + · ───── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:2:10] + 1 │ function foo() { var alert = function() {}; } + 2 │ alert(); + · ───── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ this.alert(foo) + · ────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ this['alert'](foo) + · ───────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:2:10] + 1 │ function foo() { var window = bar; window.alert(); } + 2 │ window.alert(); + · ──────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ globalThis['alert'](foo) + · ─────────────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ globalThis.alert(); + · ──────────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:2:10] + 1 │ function foo() { var globalThis = bar; globalThis.alert(); } + 2 │ globalThis.alert(); + · ──────────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:1] + 1 │ window?.alert(foo) + · ───────────── + ╰──── + help: Use a custom UI instead + + ⚠ eslint(no-alert): `alert`, `confirm` and `prompt` functions are not allowed + ╭─[no_alert.tsx:1:2] + 1 │ (window?.alert)(foo) + · ───────────── + ╰──── + help: Use a custom UI instead