Skip to content

Commit

Permalink
feat(linter): implement eslint/prefer-promise-reject-errors (#8254)
Browse files Browse the repository at this point in the history
  • Loading branch information
tbashiyy authored Jan 13, 2025
1 parent 25d4bf9 commit d178360
Show file tree
Hide file tree
Showing 5 changed files with 597 additions and 100 deletions.
88 changes: 88 additions & 0 deletions crates/oxc_linter/src/ast_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use oxc_semantic::{AstNode, IsGlobalReference, NodeId, ReferenceId, Semantic, Sy
use oxc_span::{GetSpan, Span};
use oxc_syntax::operator::{AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator};

use crate::LintContext;

/// Test if an AST node is a boolean value that never changes. Specifically we
/// test for:
/// 1. Literal booleans (`true` or `false`)
Expand Down Expand Up @@ -469,3 +471,89 @@ pub fn leftmost_identifier_reference<'a, 'b: 'a>(
_ => Err(expr),
}
}

fn is_definitely_non_error_type(ty: &TSType) -> bool {
match ty {
TSType::TSNumberKeyword(_)
| TSType::TSStringKeyword(_)
| TSType::TSBooleanKeyword(_)
| TSType::TSNullKeyword(_)
| TSType::TSUndefinedKeyword(_) => true,
TSType::TSUnionType(union) => union.types.iter().all(is_definitely_non_error_type),
TSType::TSIntersectionType(intersect) => {
intersect.types.iter().all(is_definitely_non_error_type)
}
_ => false,
}
}

pub fn could_be_error(ctx: &LintContext, expr: &Expression) -> bool {
match expr.get_inner_expression() {
Expression::NewExpression(_)
| Expression::AwaitExpression(_)
| Expression::CallExpression(_)
| Expression::ChainExpression(_)
| Expression::YieldExpression(_)
| Expression::PrivateFieldExpression(_)
| Expression::StaticMemberExpression(_)
| Expression::ComputedMemberExpression(_)
| Expression::TaggedTemplateExpression(_) => true,
Expression::AssignmentExpression(expr) => {
if matches!(expr.operator, AssignmentOperator::Assign | AssignmentOperator::LogicalAnd)
{
return could_be_error(ctx, &expr.right);
}

if matches!(
expr.operator,
AssignmentOperator::LogicalOr | AssignmentOperator::LogicalNullish
) {
return expr.left.get_expression().map_or(true, |expr| could_be_error(ctx, expr))
|| could_be_error(ctx, &expr.right);
}

false
}
Expression::SequenceExpression(expr) => {
expr.expressions.last().is_some_and(|expr| could_be_error(ctx, expr))
}
Expression::LogicalExpression(expr) => {
if matches!(expr.operator, LogicalOperator::And) {
return could_be_error(ctx, &expr.right);
}

could_be_error(ctx, &expr.left) || could_be_error(ctx, &expr.right)
}
Expression::ConditionalExpression(expr) => {
could_be_error(ctx, &expr.consequent) || could_be_error(ctx, &expr.alternate)
}
Expression::Identifier(ident) => {
let reference = ctx.symbols().get_reference(ident.reference_id());
let Some(symbol_id) = reference.symbol_id() else {
return true;
};
let decl = ctx.nodes().get_node(ctx.symbols().get_declaration(symbol_id));
match decl.kind() {
AstKind::VariableDeclarator(decl) => {
if let Some(init) = &decl.init {
could_be_error(ctx, init)
} else {
// TODO: warn about throwing undefined
false
}
}
AstKind::Function(_)
| AstKind::Class(_)
| AstKind::TSModuleDeclaration(_)
| AstKind::TSEnumDeclaration(_) => false,
AstKind::FormalParameter(param) => !param
.pattern
.type_annotation
.as_ref()
.is_some_and(|annot| is_definitely_non_error_type(&annot.type_annotation)),
_ => true,
}
}
_ => false,
}
}
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ mod eslint {
pub mod prefer_exponentiation_operator;
pub mod prefer_numeric_literals;
pub mod prefer_object_has_own;
pub mod prefer_promise_reject_errors;
pub mod prefer_rest_params;
pub mod prefer_spread;
pub mod radix;
Expand Down Expand Up @@ -643,6 +644,7 @@ oxc_macros::declare_all_lint_rules! {
eslint::no_var,
eslint::no_void,
eslint::no_with,
eslint::prefer_promise_reject_errors,
eslint::prefer_rest_params,
eslint::prefer_exponentiation_operator,
eslint::prefer_numeric_literals,
Expand Down
103 changes: 3 additions & 100 deletions crates/oxc_linter/src/rules/eslint/no_throw_literal.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
use oxc_ast::{
ast::{AssignmentOperator, Expression, LogicalOperator, TSType},
AstKind,
};
use oxc_ast::{ast::Expression, AstKind};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};

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

fn no_throw_literal_diagnostic(span: Span, is_undef: bool) -> OxcDiagnostic {
let message =
Expand Down Expand Up @@ -92,108 +89,14 @@ impl Rule for NoThrowLiteral {
Expression::Identifier(id) if SPECIAL_IDENTIFIERS.contains(&id.name.as_str()) => {
ctx.diagnostic(no_throw_literal_diagnostic(expr.span(), true));
}
expr if !Self::could_be_error(ctx, expr) => {
expr if !could_be_error(ctx, expr) => {
ctx.diagnostic(no_throw_literal_diagnostic(expr.span(), false));
}
_ => {}
}
}
}

impl NoThrowLiteral {
fn could_be_error(ctx: &LintContext, expr: &Expression) -> bool {
match expr.get_inner_expression() {
Expression::NewExpression(_)
| Expression::AwaitExpression(_)
| Expression::CallExpression(_)
| Expression::ChainExpression(_)
| Expression::YieldExpression(_)
| Expression::PrivateFieldExpression(_)
| Expression::StaticMemberExpression(_)
| Expression::ComputedMemberExpression(_)
| Expression::TaggedTemplateExpression(_) => true,
Expression::AssignmentExpression(expr) => {
if matches!(
expr.operator,
AssignmentOperator::Assign | AssignmentOperator::LogicalAnd
) {
return Self::could_be_error(ctx, &expr.right);
}

if matches!(
expr.operator,
AssignmentOperator::LogicalOr | AssignmentOperator::LogicalNullish
) {
return expr
.left
.get_expression()
.map_or(true, |expr| Self::could_be_error(ctx, expr))
|| Self::could_be_error(ctx, &expr.right);
}

false
}
Expression::SequenceExpression(expr) => {
expr.expressions.last().is_some_and(|expr| Self::could_be_error(ctx, expr))
}
Expression::LogicalExpression(expr) => {
if matches!(expr.operator, LogicalOperator::And) {
return Self::could_be_error(ctx, &expr.right);
}

Self::could_be_error(ctx, &expr.left) || Self::could_be_error(ctx, &expr.right)
}
Expression::ConditionalExpression(expr) => {
Self::could_be_error(ctx, &expr.consequent)
|| Self::could_be_error(ctx, &expr.alternate)
}
Expression::Identifier(ident) => {
let reference = ctx.symbols().get_reference(ident.reference_id());
let Some(symbol_id) = reference.symbol_id() else {
return true;
};
let decl = ctx.nodes().get_node(ctx.symbols().get_declaration(symbol_id));
match decl.kind() {
AstKind::VariableDeclarator(decl) => {
if let Some(init) = &decl.init {
Self::could_be_error(ctx, init)
} else {
// TODO: warn about throwing undefined
false
}
}
AstKind::Function(_)
| AstKind::Class(_)
| AstKind::TSModuleDeclaration(_)
| AstKind::TSEnumDeclaration(_) => false,
AstKind::FormalParameter(param) => {
!param.pattern.type_annotation.as_ref().is_some_and(|annot| {
is_definitely_non_error_type(&annot.type_annotation)
})
}
_ => true,
}
}
_ => false,
}
}
}

fn is_definitely_non_error_type(ty: &TSType) -> bool {
match ty {
TSType::TSNumberKeyword(_)
| TSType::TSStringKeyword(_)
| TSType::TSBooleanKeyword(_)
| TSType::TSNullKeyword(_)
| TSType::TSUndefinedKeyword(_) => true,
TSType::TSUnionType(union) => union.types.iter().all(is_definitely_non_error_type),
TSType::TSIntersectionType(intersect) => {
intersect.types.iter().all(is_definitely_non_error_type)
}
_ => false,
}
}

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

0 comments on commit d178360

Please sign in to comment.