From 9793c45fa7bfd22877b4bdc95a1df07cd539ec96 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Sat, 24 Aug 2024 14:52:30 -0400 Subject: [PATCH] fix(linter/unicorn): handle type casts and parens in `no-null` --- crates/oxc_linter/src/ast_util.rs | 20 ++++- .../oxc_linter/src/rules/unicorn/no_null.rs | 77 +++++++++---------- crates/oxc_linter/src/snapshots/no_null.snap | 7 ++ 3 files changed, 64 insertions(+), 40 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index c10289a56cca3..86145d312ed8e 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -2,7 +2,7 @@ use std::hash::{Hash, Hasher}; use oxc_ast::ast::BindingIdentifier; use oxc_ast::AstKind; -use oxc_semantic::{AstNode, SymbolId}; +use oxc_semantic::{AstNode, AstNodeId, SymbolId}; use oxc_span::{GetSpan, Span}; use oxc_syntax::operator::{AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator}; use rustc_hash::FxHasher; @@ -251,6 +251,24 @@ pub fn nth_outermost_paren_parent<'a, 'b>( .filter(|parent| !matches!(parent.kind(), AstKind::ParenthesizedExpression(_))) .nth(n) } +/// Iterate over parents of `node`, skipping nodes that are also ignored by +/// [`Expression::get_inner_expression`]. +pub fn iter_outer_expressions<'a, 'ctx>( + ctx: &'ctx LintContext<'a>, + node_id: AstNodeId, +) -> impl Iterator> + 'ctx { + ctx.nodes().iter_parents(node_id).skip(1).filter(|parent| { + !matches!( + parent.kind(), + AstKind::ParenthesizedExpression(_) + | AstKind::TSAsExpression(_) + | AstKind::TSSatisfiesExpression(_) + | AstKind::TSInstantiationExpression(_) + | AstKind::TSNonNullExpression(_) + | AstKind::TSTypeAssertion(_) + ) + }) +} pub fn get_declaration_of_variable<'a, 'b>( ident: &IdentifierReference, diff --git a/crates/oxc_linter/src/rules/unicorn/no_null.rs b/crates/oxc_linter/src/rules/unicorn/no_null.rs index 62e6ff7087a5a..19a4ad85d4a4a 100644 --- a/crates/oxc_linter/src/rules/unicorn/no_null.rs +++ b/crates/oxc_linter/src/rules/unicorn/no_null.rs @@ -11,7 +11,7 @@ use oxc_syntax::operator::BinaryOperator; use serde_json::Value; use crate::{ - ast_util::is_method_call, + ast_util::{is_method_call, iter_outer_expressions}, context::LintContext, fixer::{RuleFix, RuleFixer}, rule::Rule, @@ -53,13 +53,15 @@ declare_oxc_lint!( ); fn match_null_arg(call_expr: &CallExpression, index: usize, span: Span) -> bool { - call_expr.arguments.get(index).map_or(false, |arg| { - if let Argument::NullLiteral(null_lit) = arg { - return null_lit.span == span; - } - - false - }) + match call_expr + .arguments + .get(index) + .and_then(Argument::as_expression) + .map(Expression::get_inner_expression) + { + Some(Expression::NullLiteral(null_lit)) => span.contains_inclusive(null_lit.span), + _ => false, + } } impl NoNull { @@ -173,52 +175,45 @@ impl Rule for NoNull { return; }; - if let Some(parent_node) = ctx.nodes().parent_node(node.id()) { - let grand_parent_kind = ctx.nodes().parent_kind(parent_node.id()); + let mut parents = iter_outer_expressions(ctx, node.id()); + let Some(parent_kind) = parents.next().map(AstNode::kind) else { + ctx.diagnostic_with_fix(no_null_diagnostic(null_literal.span), |fixer| { + fix_null(fixer, null_literal) + }); + return; + }; - if matches!(parent_node.kind(), AstKind::Argument(_)) { - if let Some(AstKind::CallExpression(call_expr)) = grand_parent_kind { - if match_call_expression_pass_case(null_literal, call_expr) { - return; - } - } + let grandparent_kind = parents.next().map(AstNode::kind); + match (parent_kind, grandparent_kind) { + (AstKind::Argument(_), Some(AstKind::CallExpression(call_expr))) + if match_call_expression_pass_case(null_literal, call_expr) => + { + // no violation } - - if let AstKind::BinaryExpression(binary_expr) = parent_node.kind() { + (AstKind::BinaryExpression(binary_expr), _) => { self.diagnose_binary_expression(ctx, null_literal, binary_expr); - return; } - - if let AstKind::VariableDeclarator(variable_declarator) = parent_node.kind() { - diagnose_variable_declarator( - ctx, - null_literal, - variable_declarator, - grand_parent_kind, - ); - - return; + (AstKind::VariableDeclarator(decl), _) => { + diagnose_variable_declarator(ctx, null_literal, decl, grandparent_kind); } - - // `function foo() { return null; }`, - if matches!(parent_node.kind(), AstKind::ReturnStatement(_)) { + (AstKind::ReturnStatement(_), _) => { ctx.diagnostic_with_fix(no_null_diagnostic(null_literal.span), |fixer| { - fixer.delete_range(null_literal.span) + fixer.delete(null_literal) + }); + } + _ => { + ctx.diagnostic_with_fix(no_null_diagnostic(null_literal.span), |fixer| { + fix_null(fixer, null_literal) }); - - return; } } - - ctx.diagnostic_with_fix(no_null_diagnostic(null_literal.span), |fixer| { - fix_null(fixer, null_literal) - }); } } fn fix_null<'a>(fixer: RuleFixer<'_, 'a>, null: &NullLiteral) -> RuleFix<'a> { fixer.replace(null.span, "undefined") } + #[test] fn test() { use crate::tester::Tester; @@ -232,6 +227,7 @@ fn test() { let pass = vec![ ("let foo", None), ("Object.create(null)", None), + ("Object.create(null as any)", None), ("Object.create(null, {foo: {value:1}})", None), ("let insertedNode = parentNode.insertBefore(newNode, null)", None), ("const foo = \"null\";", None), @@ -239,6 +235,8 @@ fn test() { ("Object.create(bar)", None), ("Object.create(\"null\")", None), ("useRef(null)", None), + ("useRef(((((null)))))", None), + ("useRef(null as unknown as HTMLElement)", None), ("React.useRef(null)", None), ("if (foo === null) {}", None), ("if (null === foo) {}", None), @@ -301,6 +299,7 @@ fn test() { ("Object.create(...[null])", None), ("Object.create(null, bar, extraArgument)", None), ("foo.insertBefore(null)", None), + ("foo.insertBefore(null as any)", None), ("foo.insertBefore(foo, null, bar)", None), ("foo.insertBefore(...[foo], null)", None), // Not in right position diff --git a/crates/oxc_linter/src/snapshots/no_null.snap b/crates/oxc_linter/src/snapshots/no_null.snap index 30d1de6314bde..6ecc0f93140f8 100644 --- a/crates/oxc_linter/src/snapshots/no_null.snap +++ b/crates/oxc_linter/src/snapshots/no_null.snap @@ -235,6 +235,13 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Replace `null` with `undefined`. + ⚠ eslint-plugin-unicorn(no-null): Do not use `null` literals + ╭─[no_null.tsx:1:18] + 1 │ foo.insertBefore(null as any) + · ──── + ╰──── + help: Replace `null` with `undefined`. + ⚠ eslint-plugin-unicorn(no-null): Do not use `null` literals ╭─[no_null.tsx:1:23] 1 │ foo.insertBefore(foo, null, bar)