Skip to content

Commit

Permalink
feat(linter): add fixer for no-useless-fallback-in-spread rule (#3544)
Browse files Browse the repository at this point in the history
  • Loading branch information
DonIsaac authored Jun 6, 2024
1 parent ff3f37d commit 1fb9d23
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@ use oxc_ast::{ast::Expression, AstKind};
use oxc_diagnostics::OxcDiagnostic;

use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use oxc_span::{GetSpan, Span};
use oxc_syntax::operator::LogicalOperator;

use crate::{ast_util::outermost_paren_parent, context::LintContext, rule::Rule, AstNode};
use crate::{
ast_util::outermost_paren_parent, context::LintContext, fixer::Fix, rule::Rule, AstNode,
};

fn no_useless_fallback_in_spread_diagnostic(span0: Span) -> OxcDiagnostic {
fn no_useless_fallback_in_spread_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals")
.with_help("Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback.")
.with_labels([span0.into()])
.with_labels([span.into()])
}

#[derive(Debug, Default, Clone)]
Expand Down Expand Up @@ -75,7 +77,29 @@ impl Rule for NoUselessFallbackInSpread {
return;
}

ctx.diagnostic(no_useless_fallback_in_spread_diagnostic(spread_element.span));
let diagnostic = no_useless_fallback_in_spread_diagnostic(spread_element.span);

if can_fix(&logical_expression.left) {
ctx.diagnostic_with_fix(diagnostic, || {
let left_text = logical_expression.left.span().source_text(ctx.source_text());
Fix::new(format!("...{left_text}"), spread_element.span)
});
} else {
ctx.diagnostic(diagnostic);
}
}
}

fn can_fix(left: &Expression<'_>) -> bool {
const BANNED_IDENTIFIERS: [&str; 3] = ["undefined", "NaN", "Infinity"];
match left.without_parenthesized() {
Expression::Identifier(ident) => !BANNED_IDENTIFIERS.contains(&ident.name.as_str()),
Expression::LogicalExpression(expr) => can_fix(&expr.left),
Expression::ObjectExpression(_)
| Expression::CallExpression(_)
| Expression::StaticMemberExpression(_)
| Expression::ComputedMemberExpression(_) => true,
_ => false,
}
}

Expand Down Expand Up @@ -131,5 +155,15 @@ fn test() {
r"const object = {...(document.all || {})}",
];

Tester::new(NoUselessFallbackInSpread::NAME, pass, fail).test_and_snapshot();
let fix = vec![
//
(r"const object = {...(foo || {})}", r"const object = {...foo}"),
(r"const object = {...(foo() || {})}", r"const object = {...foo()}"),
(r"const object = {...((foo && {}) || {})}", "const object = {...(foo && {})}"),
(r"const object = {...(0 || {})}", r"const object = {...(0 || {})}"),
(r"const object = {...(NaN || {})}", r"const object = {...(NaN || {})}"),
(r"const object = {...(Infinity || {})}", r"const object = {...(Infinity || {})}"),
];

Tester::new(NoUselessFallbackInSpread::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
}
30 changes: 24 additions & 6 deletions crates/oxc_linter/src/tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,30 @@ impl From<(&str, Option<Value>, Option<Value>, Option<PathBuf>)> for TestCase {
}
}

#[derive(Debug, Clone)]
pub struct ExpectFix {
/// Source code being tested
source: String,
/// Expected source code after fix has been applied
expected: String,
rule_config: Option<Value>,
}
impl<S: Into<String>> From<(S, S, Option<Value>)> for ExpectFix {
fn from(value: (S, S, Option<Value>)) -> Self {
Self { source: value.0.into(), expected: value.1.into(), rule_config: value.2 }
}
}
impl<S: Into<String>> From<(S, S)> for ExpectFix {
fn from(value: (S, S)) -> Self {
Self { source: value.0.into(), expected: value.1.into(), rule_config: None }
}
}
pub struct Tester {
rule_name: &'static str,
rule_path: PathBuf,
expect_pass: Vec<TestCase>,
expect_fail: Vec<TestCase>,
expect_fix: Vec<(String, String, Option<Value>)>,
expect_fix: Vec<ExpectFix>,
snapshot: String,
current_working_directory: Box<Path>,
import_plugin: bool,
Expand Down Expand Up @@ -138,9 +156,8 @@ impl Tester {
self
}

pub fn expect_fix<S: Into<String>>(mut self, expect_fix: Vec<(S, S, Option<Value>)>) -> Self {
self.expect_fix =
expect_fix.into_iter().map(|(s1, s2, r)| (s1.into(), s2.into(), r)).collect::<Vec<_>>();
pub fn expect_fix<F: Into<ExpectFix>>(mut self, expect_fix: Vec<F>) -> Self {
self.expect_fix = expect_fix.into_iter().map(std::convert::Into::into).collect::<Vec<_>>();
self
}

Expand Down Expand Up @@ -179,8 +196,9 @@ impl Tester {
}

fn test_fix(&mut self) {
for (test, expected, config) in self.expect_fix.clone() {
let result = self.run(&test, config, &None, None, true);
for fix in self.expect_fix.clone() {
let ExpectFix { source, expected, rule_config: config } = fix;
let result = self.run(&source, config, &None, None, true);
if let TestResult::Fixed(fixed_str) = result {
assert_eq!(expected, fixed_str);
} else {
Expand Down

0 comments on commit 1fb9d23

Please sign in to comment.