Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(linter): make sure all auto-fixing rules have fixer test #6378

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/disable_directives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,6 @@ semi*/
),
];

Tester::new("no-debugger", pass, fail).test();
Tester::new("no-debugger", pass, fail).intentionally_allow_no_fix_tests().test();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -387,5 +387,8 @@ fn test() {
"class C { field1 = function() {}\n[field2]; }", // { "ecmaVersion": 2022 }
];

Tester::new(NoUnexpectedMultiline::NAME, pass, fail).test_and_snapshot();
// TODO: add more fixer tests
let fix = vec![("var a = b\n(x || y).doSomething()", "var a = b\n;(x || y).doSomething()")];

Tester::new(NoUnexpectedMultiline::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
}
19 changes: 18 additions & 1 deletion crates/oxc_linter/src/rules/eslint/no_unsafe_negation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,5 +140,22 @@ fn test() {
("! a <= b", Some(serde_json::json!([{ "enforceForOrderingRelations": true }]))),
];

Tester::new(NoUnsafeNegation::NAME, pass, fail).test_and_snapshot();
let fix = vec![
("!a in b", "!(a in b)"),
("(!a in b)", "(!(a in b))"),
("!(a) in b", "!(a in b)"),
("!a instanceof b", "!(a instanceof b)"),
("(!a instanceof b)", "(!(a instanceof b))"),
("!(a) instanceof b", "!(a instanceof b)"),
// FIXME: I think these should be failing. I encountered these while
// making sure all fix-reporting rules have fix test cases. Debugging +
// fixing this is out of scope for this PR.
// ("if (! a < b) {}", "if (!(a < b)) {}"),
// ("while (! a > b) {}", "while (!(a > b)) {}"),
// ("foo = ! a <= b;", "foo = !(a <= b);"),
// ("foo = ! a >= b;", "foo = !(a >= b);"),
// ("!a <= b", "!(a <= b)"),
];

Tester::new(NoUnsafeNegation::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ fn fixme() {
), // { "ecmaVersion": 2015 },
];
let fail = vec![];
Tester::new(NoUnusedVars::NAME, pass, fail).test();
Tester::new(NoUnusedVars::NAME, pass, fail).intentionally_allow_no_fix_tests().test();
}

#[test]
fn test() {
let pass = vec![
Expand Down Expand Up @@ -992,5 +993,8 @@ fn test() {
), // { "ecmaVersion": 2015 }
];

Tester::new(NoUnusedVars::NAME, pass, fail).with_snapshot_suffix("eslint").test_and_snapshot();
Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("eslint")
.test_and_snapshot();
}
15 changes: 13 additions & 2 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ fn test_vars_self_use() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-vars-self-use")
.test_and_snapshot();
}
Expand Down Expand Up @@ -201,6 +202,7 @@ fn test_vars_discarded_reads() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-vars-discarded-read")
.test_and_snapshot();
}
Expand Down Expand Up @@ -288,6 +290,7 @@ fn test_vars_reassignment() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-vars-reassignment")
.test_and_snapshot();
}
Expand Down Expand Up @@ -408,6 +411,7 @@ fn test_vars_catch() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-vars-catch")
.test_and_snapshot();
}
Expand All @@ -419,6 +423,7 @@ fn test_vars_using() {
let fail = vec![("using a = 1;", None)];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-vars-using")
.test_and_snapshot();
}
Expand Down Expand Up @@ -706,7 +711,7 @@ fn test_exports() {
let fail = vec!["import { a as b } from 'a'; export { a }"];

// these are mostly pass[] cases, so do not snapshot
Tester::new(NoUnusedVars::NAME, pass, fail).test();
Tester::new(NoUnusedVars::NAME, pass, fail).intentionally_allow_no_fix_tests().test();
}

#[test]
Expand Down Expand Up @@ -752,7 +757,7 @@ fn test_react() {
",
];

Tester::new(NoUnusedVars::NAME, pass, fail).test();
Tester::new(NoUnusedVars::NAME, pass, fail).intentionally_allow_no_fix_tests().test();
}

#[test]
Expand Down Expand Up @@ -783,6 +788,7 @@ fn test_arguments() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-arguments")
.test_and_snapshot();
}
Expand All @@ -798,6 +804,7 @@ fn test_enums() {
let fail = vec!["enum Foo { A }"];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-enums")
.test_and_snapshot();
}
Expand Down Expand Up @@ -863,6 +870,7 @@ fn test_classes() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-classes")
.test_and_snapshot();
}
Expand Down Expand Up @@ -915,6 +923,7 @@ fn test_namespaces() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-namespaces")
.test_and_snapshot();
}
Expand All @@ -931,6 +940,7 @@ fn test_type_aliases() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-type-aliases")
.test_and_snapshot();
}
Expand Down Expand Up @@ -990,6 +1000,7 @@ fn test_type_references() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("oxc-type-references")
.test_and_snapshot();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,7 @@ fn test() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.change_rule_path_extension("ts")
.with_snapshot_suffix("typescript-eslint")
.test_and_snapshot();
Expand Down Expand Up @@ -1867,6 +1868,7 @@ fn test_tsx() {
];

Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.with_snapshot_suffix("typescript-eslint-tsx")
.test_and_snapshot();
}
Expand Down Expand Up @@ -1906,5 +1908,8 @@ fn test_d_ts() {
];
let fail = vec![];

Tester::new(NoUnusedVars::NAME, pass, fail).change_rule_path_extension("d.ts").test();
Tester::new(NoUnusedVars::NAME, pass, fail)
.intentionally_allow_no_fix_tests()
.change_rule_path_extension("d.ts")
.test();
}
4 changes: 3 additions & 1 deletion crates/oxc_linter/src/rules/eslint/valid_typeof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,5 +222,7 @@ fn test() {
),
];

Tester::new(ValidTypeof::NAME, pass, fail).test_and_snapshot();
let fix = vec![("typeof foo === undefined", r#"typeof foo === "undefined""#)];

Tester::new(ValidTypeof::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
}
11 changes: 11 additions & 0 deletions crates/oxc_linter/src/rules/jest/no_test_prefixes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,19 @@ fn test() {
pass.extend(pass_vitest);
fail.extend(fail_vitest);

let fix = vec![
("xdescribe('foo', () => {})", "describe.skip('foo', () => {})"),
("fdescribe('foo', () => {})", "describe.only('foo', () => {})"),
("xtest('foo', () => {})", "test.skip('foo', () => {})"),
// NOTE(@DonIsaac): is this intentional?
// ("ftest('foo', () => {})", "test.only('foo', () => {})"),
("xit('foo', () => {})", "it.skip('foo', () => {})"),
("fit('foo', () => {})", "it.only('foo', () => {})"),
];

Tester::new(NoTestPrefixes::NAME, pass, fail)
.with_jest_plugin(true)
.with_vitest_plugin(true)
.expect_fix(fix)
.test_and_snapshot();
}
9 changes: 8 additions & 1 deletion crates/oxc_linter/src/rules/typescript/ban_ts_comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -988,5 +988,12 @@ if (false) {
),
];

Tester::new(BanTsComment::NAME, pass, fail).test_and_snapshot();
let fix = vec![
("// @ts-ignore", r"// @ts-expect-error"),
("// @ts-ignore: TS1234 because xyz", r"// @ts-expect-error: TS1234 because xyz"),
("// @ts-ignore: TS1234", r"// @ts-expect-error: TS1234"),
("// @ts-ignore : TS1234 because xyz", r"// @ts-expect-error : TS1234 because xyz"),
];

Tester::new(BanTsComment::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
}
6 changes: 5 additions & 1 deletion crates/oxc_linter/src/rules/typescript/no_explicit_any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,11 @@ mod tests {
fn test_simple() {
let pass = vec!["let x: number = 1"];
let fail = vec!["let x: any = 1"];
Tester::new(NoExplicitAny::NAME, pass, fail).test();
let fix = vec![
("let x: any = 1", "let x: unknown = 1", Some(json!([{ "fixToUnknown": true }]))),
("let x: any = 1", "let x: any = 1", None),
];
Tester::new(NoExplicitAny::NAME, pass, fail).expect_fix(fix).test();
}

#[test]
Expand Down
11 changes: 7 additions & 4 deletions crates/oxc_linter/src/rules/unicorn/numeric_separators_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,9 @@ fn test_with_snapshot() {
"const foo = -100000_1",
];

Tester::new(NumericSeparatorsStyle::NAME, vec![], fail).test_and_snapshot();
let fix = vec![("const foo = 0b10_10_0001", "const foo = 0b1010_0001")];

Tester::new(NumericSeparatorsStyle::NAME, vec![], fail).expect_fix(fix).test_and_snapshot();
}

#[test]
Expand Down Expand Up @@ -627,7 +629,7 @@ fn test_with_config() {

let fail = vec![];

Tester::new(NumericSeparatorsStyle::NAME, pass, fail).test();
Tester::new(NumericSeparatorsStyle::NAME, pass, fail).intentionally_allow_no_fix_tests().test();
}

#[test]
Expand All @@ -642,9 +644,10 @@ fn test_misc() {
"const foo = '1234567n'",
];

let fail = vec![];
let fail = vec!["1_23_4444"];
let fix = vec![("1_23_4444", "1_234_444")];

Tester::new(NumericSeparatorsStyle::NAME, pass, fail).test();
Tester::new(NumericSeparatorsStyle::NAME, pass, fail).expect_fix(fix).test();
}

#[cfg(test)]
Expand Down
42 changes: 38 additions & 4 deletions crates/oxc_linter/src/tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,13 @@ pub struct Tester {
rule_path: PathBuf,
expect_pass: Vec<TestCase>,
expect_fail: Vec<TestCase>,
expect_fix: Vec<ExpectFix>,
/// Intentionally not an empty array when no fix test cases are provided.
/// We check that rules that report a fix capability have fix test cases.
/// Providing `Some(vec![])` allows for intentional disabling of this behavior.
///
/// Note that disabling this check should be done as little as possible, and
/// never in bad faith (e.g. no `#[test]` functions have fixer cases at all).
expect_fix: Option<Vec<ExpectFix>>,
snapshot: String,
/// Suffix added to end of snapshot name.
///
Expand All @@ -191,7 +197,7 @@ impl Tester {
rule_path,
expect_pass,
expect_fail,
expect_fix: vec![],
expect_fix: None,
snapshot: String::new(),
snapshot_suffix: None,
current_working_directory,
Expand Down Expand Up @@ -256,6 +262,9 @@ impl Tester {
/// These cases will fail if no fixes are produced or if the fixed source
/// code does not match the expected result.
///
/// Additionally, if your rule reports a fix capability but no fix cases are
/// provided, the test will fail.
///
/// ```
/// use oxc_linter::tester::Tester;
///
Expand All @@ -273,8 +282,26 @@ impl Tester {
/// // the first argument is normally `MyRuleStruct::NAME`.
/// Tester::new("no-undef", pass, fail).expect_fix(fix).test();
/// ```
#[must_use]
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<_>>();
// prevent `expect_fix` abuse
assert!(
!expect_fix.is_empty(),
"You must provide at least one fixer test case to `expect_fix`"
);

self.expect_fix =
Some(expect_fix.into_iter().map(std::convert::Into::into).collect::<Vec<_>>());
self
}

/// Intentionally allow testing to pass if no fix test cases are provided.
///
/// This should only be used when testing is broken up into multiple
/// test functions, and only some of them are testing fixes.
#[must_use]
pub fn intentionally_allow_no_fix_tests(mut self) -> Self {
self.expect_fix = Some(vec![]);
self
}

Expand Down Expand Up @@ -321,7 +348,14 @@ impl Tester {
}

fn test_fix(&mut self) {
for fix in self.expect_fix.clone() {
// If auto-fixes are reported, make sure some fix test cases are provided
let rule = self.find_rule();
let Some(fix_test_cases) = self.expect_fix.clone() else {
assert!(!rule.fix().has_fix(), "'{}/{}' reports that it can auto-fix violations, but no fix cases were provided. Please add fixer test cases with `tester.expect_fix()`", rule.plugin_name(), rule.name());
return;
};

for fix in fix_test_cases {
let ExpectFix { source, expected, kind, rule_config: config } = fix;
let result = self.run(&source, config, &None, None, kind);
match result {
Expand Down
Loading