diff --git a/crates/oxc_linter/src/disable_directives.rs b/crates/oxc_linter/src/disable_directives.rs index 510a4053a4cfa..eada7a89aceb7 100644 --- a/crates/oxc_linter/src/disable_directives.rs +++ b/crates/oxc_linter/src/disable_directives.rs @@ -612,6 +612,6 @@ semi*/ ), ]; - Tester::new("no-debugger", pass, fail).test(); + Tester::new("no-debugger", pass, fail).intentionally_allow_no_fix_tests().test(); } } diff --git a/crates/oxc_linter/src/rules/eslint/no_unexpected_multiline.rs b/crates/oxc_linter/src/rules/eslint/no_unexpected_multiline.rs index 573517aa5a197..424654c3a1920 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unexpected_multiline.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unexpected_multiline.rs @@ -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(); } diff --git a/crates/oxc_linter/src/rules/eslint/no_unsafe_negation.rs b/crates/oxc_linter/src/rules/eslint/no_unsafe_negation.rs index d5fb528178eea..54773918e7ddc 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unsafe_negation.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unsafe_negation.rs @@ -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(); } diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs index 61302856e6dca..5177e18d61d96 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs @@ -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![ @@ -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(); } diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs index 332b2edd5718d..3094c4d7927ea 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs @@ -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(); } @@ -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(); } @@ -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(); } @@ -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(); } @@ -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(); } @@ -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] @@ -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] @@ -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(); } @@ -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(); } @@ -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(); } @@ -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(); } @@ -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(); } @@ -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(); } diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/typescript_eslint.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/typescript_eslint.rs index bc3f15eb2eb68..46078aa089d47 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/typescript_eslint.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/typescript_eslint.rs @@ -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(); @@ -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(); } @@ -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(); } diff --git a/crates/oxc_linter/src/rules/eslint/valid_typeof.rs b/crates/oxc_linter/src/rules/eslint/valid_typeof.rs index b1e768a2b8165..64957d6c994eb 100644 --- a/crates/oxc_linter/src/rules/eslint/valid_typeof.rs +++ b/crates/oxc_linter/src/rules/eslint/valid_typeof.rs @@ -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(); } diff --git a/crates/oxc_linter/src/rules/jest/no_test_prefixes.rs b/crates/oxc_linter/src/rules/jest/no_test_prefixes.rs index dcf49906ae113..9b13d8d819444 100644 --- a/crates/oxc_linter/src/rules/jest/no_test_prefixes.rs +++ b/crates/oxc_linter/src/rules/jest/no_test_prefixes.rs @@ -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(); } diff --git a/crates/oxc_linter/src/rules/typescript/ban_ts_comment.rs b/crates/oxc_linter/src/rules/typescript/ban_ts_comment.rs index 46fb4b00c79fc..8bfc9b8c918e8 100644 --- a/crates/oxc_linter/src/rules/typescript/ban_ts_comment.rs +++ b/crates/oxc_linter/src/rules/typescript/ban_ts_comment.rs @@ -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(); } diff --git a/crates/oxc_linter/src/rules/typescript/no_explicit_any.rs b/crates/oxc_linter/src/rules/typescript/no_explicit_any.rs index b79539fcbee7e..b8da9f402d300 100644 --- a/crates/oxc_linter/src/rules/typescript/no_explicit_any.rs +++ b/crates/oxc_linter/src/rules/typescript/no_explicit_any.rs @@ -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] diff --git a/crates/oxc_linter/src/rules/unicorn/numeric_separators_style.rs b/crates/oxc_linter/src/rules/unicorn/numeric_separators_style.rs index d03d3a17f70ed..d01d0d1bf10d6 100644 --- a/crates/oxc_linter/src/rules/unicorn/numeric_separators_style.rs +++ b/crates/oxc_linter/src/rules/unicorn/numeric_separators_style.rs @@ -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] @@ -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] @@ -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)] diff --git a/crates/oxc_linter/src/tester.rs b/crates/oxc_linter/src/tester.rs index bea5a0035e29f..72bc3c7a69ff7 100644 --- a/crates/oxc_linter/src/tester.rs +++ b/crates/oxc_linter/src/tester.rs @@ -164,7 +164,13 @@ pub struct Tester { rule_path: PathBuf, expect_pass: Vec, expect_fail: Vec, - expect_fix: Vec, + /// 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>, snapshot: String, /// Suffix added to end of snapshot name. /// @@ -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, @@ -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; /// @@ -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>(mut self, expect_fix: Vec) -> Self { - self.expect_fix = expect_fix.into_iter().map(std::convert::Into::into).collect::>(); + // 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::>()); + 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 } @@ -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 {