dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-19 04:08:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

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

Browse source 
Testing will now fail If a rule advertises an auto-fix capability but no fixer test cases are added.

This behavior may be bypassed using `intentionally_allow_no_fix_tests`. This should only be used in certain conditions, and we can check that it's not being abused in PR reviews.

```rs
declare_oxc_lint!(
  /// Some docs
  SomeRule,
  correctness,
  fix
);

#[test]
fn test() {
  use crate::tester::Tester;

  let pass = vec![/* ... */];
  let fail = vec![/* ... */];

  // now fails, since `fix` is reported but no cases are passed to `.expect_fix()`
  Tester::new(SomeRule::NAME, pass, fail).test_and_snapshot();
}
This commit is contained in:
DonIsaac 2024-10-09 01:15:07 +00:00
parent f71c91ed7d
commit a6cae98fbf
12 changed files with 120 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
crates/oxc_linter/src/disable_directives.rs
View file

@ -612,6 +612,6 @@ semi*/
),
];
Tester::new("no-debugger", pass, fail).test();
Tester::new("no-debugger", pass, fail).intentionally_allow_no_fix_tests().test();
}
}

5
crates/oxc_linter/src/rules/eslint/no_unexpected_multiline.rs
View file

@ -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
crates/oxc_linter/src/rules/eslint/no_unsafe_negation.rs
View file

@ -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();
}

8
crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs
View file

@ -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();
}

15
crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs
View file

@ -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();
}

7
crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/typescript_eslint.rs
View file

@ -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();
}

4
crates/oxc_linter/src/rules/eslint/valid_typeof.rs
View file

@ -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
crates/oxc_linter/src/rules/jest/no_test_prefixes.rs
View file

@ -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
crates/oxc_linter/src/rules/typescript/ban_ts_comment.rs
View file

@ -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
crates/oxc_linter/src/rules/typescript/no_explicit_any.rs
View file

@ -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]

11
crates/oxc_linter/src/rules/unicorn/numeric_separators_style.rs
View file

@ -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)]

42
crates/oxc_linter/src/tester.rs
View file

@ -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.
///
@ -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<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
}
@ -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 {