Skip to content

Commit

Permalink
fix(linter/unicorn): handle type casts and parens in no-null
Browse files Browse the repository at this point in the history
  • Loading branch information
DonIsaac committed Aug 24, 2024
1 parent 79a75f5 commit dc16094
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 40 deletions.
20 changes: 19 additions & 1 deletion crates/oxc_linter/src/ast_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Item = &'ctx AstNode<'a>> + '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,
Expand Down
77 changes: 38 additions & 39 deletions crates/oxc_linter/src/rules/unicorn/no_null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand All @@ -232,13 +227,16 @@ 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),
("Object.create()", None),
("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),
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions crates/oxc_linter/src/snapshots/no_null.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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]
1foo.insertBefore(null as any)
· ────
╰────
help: Replace `null` with `undefined`.

eslint-plugin-unicorn(no-null): Do not use `null` literals
╭─[no_null.tsx:1:23]
1foo.insertBefore(foo, null, bar)
Expand Down

0 comments on commit dc16094

Please sign in to comment.