Skip to content

Commit

Permalink
feat(linter/eslint-plugin-promise): implement catch-or-return
Browse files Browse the repository at this point in the history
  • Loading branch information
jelly committed Jul 16, 2024
1 parent 1ffd302 commit c01a85d
Show file tree
Hide file tree
Showing 3 changed files with 441 additions and 0 deletions.
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ mod tree_shaking {

mod promise {
pub mod avoid_new;
pub mod catch_or_return;
pub mod no_callback_in_promise;
pub mod no_new_statics;
pub mod no_promise_in_callback;
Expand Down Expand Up @@ -843,4 +844,5 @@ oxc_macros::declare_all_lint_rules! {
promise::no_promise_in_callback,
promise::prefer_await_to_then,
promise::no_callback_in_promise,
promise::catch_or_return,
}
293 changes: 293 additions & 0 deletions crates/oxc_linter/src/rules/promise/catch_or_return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,293 @@
use oxc_ast::{
ast::{CallExpression, Expression},
AstKind,
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

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

fn catch_or_return_diagnostic(x0: &str, span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("eslint-plugin-promise(catch-or-return): Expected {x0} or return"))
.with_label(span0)
}

#[derive(Debug, Default, Clone)]
pub struct CatchOrReturn(Box<CatchOrReturnConfig>);

#[derive(Debug, Clone)]
pub struct CatchOrReturnConfig {
allow_finally: bool,
allow_then: bool,
termination_method: Vec<String>,
}

impl Default for CatchOrReturnConfig {
fn default() -> Self {
Self {
allow_finally: false,
allow_then: false,
termination_method: vec!["catch".to_string()],
}
}
}

impl std::ops::Deref for CatchOrReturn {
type Target = CatchOrReturnConfig;

fn deref(&self) -> &Self::Target {
&self.0
}
}

declare_oxc_lint!(
/// ### What it does
///
/// Ensure that each time a then() is applied to a promise, a catch() is applied as well.
/// Exceptions are made if you are returning that promise.
///
/// ### Why is this bad?
///
/// Not catching errors in a promise can cause hard to debug problems or missing handling of
/// error conditions.
///
/// ### Example
/// ```javascript
/// myPromise.then(doSomething)
/// myPromise.then(doSomething, catchErrors) // catch() may be a little better
/// function doSomethingElse() {
/// return myPromise.then(doSomething)
/// }
/// ```
CatchOrReturn,
restriction,
);

impl Rule for CatchOrReturn {
fn from_configuration(value: serde_json::Value) -> Self {
let mut config = CatchOrReturnConfig::default();

if let Some(termination_array_config) = value
.get(0)
.and_then(|v| v.get("terminationMethod"))
.and_then(serde_json::Value::as_array)
{
config.termination_method = termination_array_config
.iter()
.filter_map(serde_json::Value::as_str)
.map(ToString::to_string)
.collect();
}

if let Some(termination_string_config) = value
.get(0)
.and_then(|v| v.get("terminationMethod"))
.and_then(serde_json::Value::as_str)
{
config.termination_method = vec![termination_string_config.to_string()];
}

if let Some(allow_finally_config) =
value.get(0).and_then(|v| v.get("allowFinally")).and_then(serde_json::Value::as_bool)
{
config.allow_finally = allow_finally_config;
}

if let Some(allow_then_config) =
value.get(0).and_then(|v| v.get("allowThen")).and_then(serde_json::Value::as_bool)
{
config.allow_then = allow_then_config;
}

Self(Box::new(config))
}

fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::ExpressionStatement(expr_stmt) = node.kind() else {
return;
};

let Expression::CallExpression(call_expr) = &expr_stmt.expression else {
return;
};

if !is_promise(call_expr) {
return;
}

if self.is_allowed_promise_termination(call_expr) {
return;
}

let termination_method = &self.termination_method[0];
ctx.diagnostic(catch_or_return_diagnostic(termination_method, call_expr.span));
}
}

impl CatchOrReturn {
fn is_allowed_promise_termination(&self, call_expr: &CallExpression) -> bool {
let Some(member_expr) = call_expr.callee.get_member_expr() else {
return false;
};

let Some(prop_name) = member_expr.static_property_name() else {
return false;
};

// somePromise.then(a, b)
if self.allow_then && prop_name == "then" && call_expr.arguments.len() == 2 {
return true;
}

// somePromise.catch().finally(fn)
if self.allow_finally && prop_name == "finally" {
let Expression::CallExpression(object_call_expr) = member_expr.object() else {
return false;
};

if is_promise(object_call_expr) && self.is_allowed_promise_termination(object_call_expr)
{
return true;
}
}

// somePromise.catch()
if self.termination_method.contains(&prop_name.to_string()) {
return true;
}

// somePromise['catch']()
if prop_name == "catch"
&& matches!(call_expr.callee.get_inner_expression(), Expression::StringLiteral(_))
{
return true;
}

false
}
}

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

let pass = vec![
("frank().then(go).catch(doIt)", None),
("frank().then(go).then().then().then().catch(doIt)", None),
("frank().then(go).then().catch(function() { /* why bother */ })", None),
("frank.then(go).then(to).catch(jail)", None),
("Promise.resolve(frank).catch(jail)", None),
(r#"Promise.resolve(frank)["catch"](jail)"#, None),
("frank.then(to).finally(fn).catch(jail)", None),
(
r#"postJSON("/smajobber/api/reportJob.json")
.then(()=>this.setState())
.catch(()=>this.setState())"#,
None,
),
("function a() { return frank().then(go) }", None),
("function a() { return frank().then(go).then().then().then() }", None),
("function a() { return frank().then(go).then()}", None),
("function a() { return frank.then(go).then(to) }", None),
("frank().then(go).then(null, doIt)", Some(serde_json::json!([{ "allowThen": true }]))),
(
"frank().then(go).then().then().then().then(null, doIt)",
Some(serde_json::json!([{ "allowThen": true }])),
),
(
"frank().then(go).then().then(null, function() { /* why bother */ })",
Some(serde_json::json!([{ "allowThen": true }])),
),
(
"frank.then(go).then(to).then(null, jail)",
Some(serde_json::json!([{ "allowThen": true }])),
),
("frank().then(a, b)", Some(serde_json::json!([{ "allowThen": true }]))),
("frank().then(a).then(b).then(null, c)", Some(serde_json::json!([{ "allowThen": true }]))),
("frank().then(a).then(b).then(c, d)", Some(serde_json::json!([{ "allowThen": true }]))),
(
"frank().then(a).then(b).then().then().then(null, doIt)",
Some(serde_json::json!([{ "allowThen": true }])),
),
(
"frank().then(a).then(b).then(null, function() { /* why bother */ })",
Some(serde_json::json!([{ "allowThen": true }])),
),
("frank().then(go).then(zam, doIt)", Some(serde_json::json!([{ "allowThen": true }]))),
(
"frank().then(go).then().then().then().then(wham, doIt)",
Some(serde_json::json!([{ "allowThen": true }])),
),
(
"frank().then(go).then().then(function() {}, function() { /* why bother */ })",
Some(serde_json::json!([{ "allowThen": true }])),
),
(
"frank.then(go).then(to).then(pewPew, jail)",
Some(serde_json::json!([{ "allowThen": true }])),
),
(
"frank().then(go).catch(doIt).finally(fn)",
Some(serde_json::json!([{ "allowFinally": true }])),
),
(
"frank().then(go).then().then().then().catch(doIt).finally(fn)",
Some(serde_json::json!([{ "allowFinally": true }])),
),
(
"frank().then(go).then().catch(function() { /* why bother */ }).finally(fn)",
Some(serde_json::json!([{ "allowFinally": true }])),
),
("frank().then(goo).done()", Some(serde_json::json!([{ "terminationMethod": "done" }]))),
(
"frank().then(go).catch()",
Some(serde_json::json!([{ "terminationMethod": ["catch", "done"] }])),
),
(
"frank().then(go).done()",
Some(serde_json::json!([{ "terminationMethod": ["catch", "done"] }])),
),
(
"frank().then(go).finally()",
Some(serde_json::json!([{ "terminationMethod": ["catch", "finally"] }])),
),
("nonPromiseExpressionStatement();", None),
("frank().then(go)['catch']", None),
];

let fail = vec![
("function callPromise(promise, cb) { promise.then(cb) }", None),
(r#"fetch("http://www.yahoo.com").then(console.log.bind(console))"#, None),
(r#"a.then(function() { return "x"; }).then(function(y) { throw y; })"#, None),
("Promise.resolve(frank)", None),
("Promise.all([])", None),
("Promise.allSettled([])", None),
("Promise.any([])", None),
("Promise.race([])", None),
("frank().then(to).catch(fn).then(foo)", None),
("frank().finally(fn)", None),
("frank().then(to).finally(fn)", None),
("frank().then(go).catch(doIt).finally(fn)", None),
("frank().then(go).then().then().then().catch(doIt).finally(fn)", None),
("frank().then(go).then().catch(function() { /* why bother */ }).finally(fn)", None),
("function a() { frank().then(go) }", None),
("function a() { frank().then(go).then().then().then() }", None),
("function a() { frank().then(go).then()}", None),
("function a() { frank.then(go).then(to) }", None),
(
"frank().then(go).catch(doIt).finally(fn).then(foo)",
Some(serde_json::json!([{ "allowFinally": true }])),
),
(
"frank().then(go).catch(doIt).finally(fn).foobar(foo)",
Some(serde_json::json!([{ "allowFinally": true }])),
),
("frank().then(go)", Some(serde_json::json!([{ "terminationMethod": "done" }]))),
("frank().catch(go)", Some(serde_json::json!([{ "terminationMethod": "done" }]))),
("frank().catch(go).someOtherMethod()", None),
("frank()['catch'](go).someOtherMethod()", None),
];

Tester::new(CatchOrReturn::NAME, pass, fail).test_and_snapshot();
}
Loading

0 comments on commit c01a85d

Please sign in to comment.