Adopt t3code's lint rules: ban-ts-comment and six anti-slop rules

.github/workflows/bench.yml

@@ -7,6 +7,9 @@ on:
         branches:
             - main
 
+permissions:
+    contents: read
+
 env:
     FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true'
     CARGO_TERM_COLOR: always
@@ -38,10 +41,12 @@ jobs:
                   go-version-file: go/helper/go.mod
                   cache-dependency-path: go/helper/go.sum
 
-            - uses: taiki-e/install-action@v2
+            # hyperfine 2 runs commands without a shell and changes the JSON
+            # export, and scripts/bench.sh relies on both.
+            - uses: taiki-e/install-action@8cca0918f8471c33515202b73f56ab27f2bed283 # v2.87.29
               name: Install hyperfine
               with:
-                  tool: hyperfine
+                  tool: hyperfine@1
 
             # Each side is built in its own checkout into storage/bench/<side>.
             # A base that predates the Rust workspace has nothing to compare.

README.md

@@ -70,7 +70,7 @@ fmtkit changes layout and applies safe lint fixes; it does not change behaviour
 
 **Vue, HTML, Markdown:** formatted as whole documents. Scripts inside them go through the script pipeline; styles and CSS fences through the oxc CSS formatter. Markdown prose is not rewrapped.
 
-**Lint:** Oxlint (pinned to `oxlint_v1.86.0`) with a bundled policy of 197 rules, including native `perfectionist/*`, `@nkzw/*`, and `no-only-tests/*` rules. The `anti-slop/*` rules are available but off by default. `error` fails the run; `warn` does not.
+**Lint:** Oxlint (pinned to `oxlint_v1.86.0`) with a bundled policy of 198 rules, including native `perfectionist/*`, `@nkzw/*`, and `no-only-tests/*` rules. The `anti-slop/*` rules are available but off by default. `error` fails the run; `warn` does not.
 
 ### What it leaves alone
 

crates/lint/policy/oxlintrc.json

@@ -237,6 +237,14 @@
 				"default": "generic"
 			}
 		],
+		"typescript/ban-ts-comment": [
+			"error",
+			{
+				"ts-expect-error": "allow-with-description",
+				"ts-ignore": "allow-with-description",
+				"ts-nocheck": "allow-with-description"
+			}
+		],
 		"typescript/consistent-type-imports": "error",
 		"typescript/no-duplicate-enum-values": "error",
 		"typescript/no-empty-object-type": "error",

crates/lint/src/rules/anti_slop/mod.rs

@@ -1,23 +1,31 @@
-//! fmtkit's own `anti-slop` rules, ported from the v1 oxlint JS plugin.
+//! fmtkit's own `anti-slop` rules: the v1 oxlint JS plugin, ported, and
+//! rules adapted from t3code's oxlint plugin
+//! (<https://github.com/pingdotgg/t3code/tree/main/oxlint-plugin-t3code>).
 //!
 //! The rules read oxc's AST through [`shared`], which hides the nodes ESTree
-//! does not have, so they report what v1 reported at the same spans.
+//! does not have, so the v1 rules report what v1 reported at the same spans.
 
 pub mod shared;
 
 mod no_ambient_nondeterminism;
 mod no_chained_type_assertions;
 mod no_conditional_empty_object_spread;
+mod no_hookless_use_function;
 mod no_known_value_widening;
+mod no_low_value_comments;
 mod no_module_mocking;
 mod no_object_parameters;
+mod no_polling_loop_in_tests;
+mod no_real_time_wait_in_tests;
 mod no_reflect_apply;
 mod no_reflect_get;
 mod no_runtime_typeof;
 mod no_shape_in_symbol_names;
+mod no_test_in_loop;
 mod no_unknown_parameters;
 mod no_unknown_returns;
 mod no_unknown_type_aliases;
+mod no_unreasoned_test_skip;
 mod no_unsafe_dictionary_type;
 mod no_widen_then_assert;
 mod require_safety_comment_for_type_assertion;
@@ -29,22 +37,28 @@ mod tests;
 
 use super::Factory;
 
-/// Every v1 rule, by full name. Only `no-runtime-typeof` takes options; the
+/// Every rule, by full name. Only `no-runtime-typeof` takes options; the
 /// others ignore any they are given, as v1 did.
 pub const RULES: &[(&str, Factory)] = &[
     (no_ambient_nondeterminism::NAME, |_| Ok(Box::new(no_ambient_nondeterminism::NoAmbientNondeterminism))),
     (no_chained_type_assertions::NAME, |_| Ok(Box::new(no_chained_type_assertions::NoChainedTypeAssertions))),
     (no_conditional_empty_object_spread::NAME, |_| Ok(Box::new(no_conditional_empty_object_spread::NoConditionalEmptyObjectSpread))),
+    (no_hookless_use_function::NAME, |_| Ok(Box::new(no_hookless_use_function::NoHooklessUseFunction))),
     (no_known_value_widening::NAME, |_| Ok(Box::new(no_known_value_widening::NoKnownValueWidening))),
+    (no_low_value_comments::NAME, |_| Ok(Box::new(no_low_value_comments::NoLowValueComments))),
     (no_module_mocking::NAME, |_| Ok(Box::new(no_module_mocking::NoModuleMocking))),
     (no_object_parameters::NAME, |_| Ok(Box::new(no_object_parameters::NoObjectParameters))),
+    (no_polling_loop_in_tests::NAME, |_| Ok(Box::new(no_polling_loop_in_tests::NoPollingLoopInTests))),
+    (no_real_time_wait_in_tests::NAME, |_| Ok(Box::new(no_real_time_wait_in_tests::NoRealTimeWaitInTests))),
     (no_reflect_apply::NAME, |_| Ok(Box::new(no_reflect_apply::NoReflectApply))),
     (no_reflect_get::NAME, |_| Ok(Box::new(no_reflect_get::NoReflectGet))),
     (no_runtime_typeof::NAME, no_runtime_typeof::build),
     (no_shape_in_symbol_names::NAME, |_| Ok(Box::new(no_shape_in_symbol_names::NoShapeInSymbolNames))),
+    (no_test_in_loop::NAME, |_| Ok(Box::new(no_test_in_loop::NoTestInLoop))),
     (no_unknown_parameters::NAME, |_| Ok(Box::new(no_unknown_parameters::NoUnknownParameters))),
     (no_unknown_returns::NAME, |_| Ok(Box::new(no_unknown_returns::NoUnknownReturns))),
     (no_unknown_type_aliases::NAME, |_| Ok(Box::new(no_unknown_type_aliases::NoUnknownTypeAliases))),
+    (no_unreasoned_test_skip::NAME, |_| Ok(Box::new(no_unreasoned_test_skip::NoUnreasonedTestSkip))),
     (no_unsafe_dictionary_type::NAME, |_| Ok(Box::new(no_unsafe_dictionary_type::NoUnsafeDictionaryType))),
     (no_widen_then_assert::NAME, |_| Ok(Box::new(no_widen_then_assert::NoWidenThenAssert))),
     (require_safety_comment_for_type_assertion::NAME, |_| Ok(Box::new(require_safety_comment_for_type_assertion::RequireSafetyCommentForTypeAssertion))),

crates/lint/src/rules/anti_slop/no_ambient_nondeterminism.rs

@@ -1,12 +1,18 @@
 //! `anti-slop/no-ambient-nondeterminism`: no reading the clock or the entropy
 //! source; deterministic code takes time and randomness from its caller.
+//!
+//! A reading passed to a mock (`vi.setSystemTime(Date.now())`, or inside a
+//! `mockReturnValue` or `vi.mock` factory) seeds fake time rather than measures
+//! the run, so it is allowed, as in t3code's `t3code/no-wall-clock-in-tests`.
 
 use oxc_ast::AstKind;
 use oxc_ast::ast::Expression;
 use oxc_semantic::AstNode;
+use oxc_span::GetSpan;
 
 use super::super::{Context, Rule};
-use super::shared::{is_global, is_global_member};
+use super::shared::testing::is_test_framework_object;
+use super::shared::{ancestors, is_global, is_global_member, member};
 
 pub const NAME: &str = "anti-slop/no-ambient-nondeterminism";
 
@@ -20,6 +26,13 @@ const AMBIENT_MEMBERS: [(&str, &str, &str); 6] = [
     ("process", "hrtime", "process.hrtime()"),
 ];
 
+/// Methods of a mock function that set what it returns.
+const MOCK_METHODS: [&str; 6] =
+    ["mockImplementation", "mockImplementationOnce", "mockResolvedValue", "mockResolvedValueOnce", "mockReturnValue", "mockReturnValueOnce"];
+
+/// Members of `vi` and `jest` that build a mock or set fake time.
+const FRAMEWORK_MOCKS: [&str; 5] = ["doMock", "fn", "mock", "setSystemTime", "spyOn"];
+
 pub struct NoAmbientNondeterminism;
 
 impl Rule for NoAmbientNondeterminism {
@@ -34,17 +47,46 @@ impl Rule for NoAmbientNondeterminism {
                     return;
                 }
 
-                if let Some((_, _, source)) = AMBIENT_MEMBERS.iter().find(|(owner, property, _)| is_global_member(ctx, &call.callee, owner, property)) {
+                let source = AMBIENT_MEMBERS
+                    .iter()
+                    .find(|(owner, property, _)| is_global_member(ctx, &call.callee, owner, property))
+                    .map(|(_, _, source)| *source)
+                    .or_else(|| is_hrtime_bigint(ctx, &call.callee).then_some("process.hrtime.bigint()"));
+
+                if let Some(source) = source
+                    && !feeds_mock(ctx, node)
+                {
                     ctx.report(
                         call.span,
                         format!("`{source}` makes this output depend on when and where it ran. Take the value as a parameter so the caller owns it."),
                     );
                 }
             }
-            AstKind::NewExpression(new) if new.arguments.is_empty() && is_global(ctx, &new.callee, "Date") => {
+            AstKind::NewExpression(new) if new.arguments.is_empty() && is_global(ctx, &new.callee, "Date") && !feeds_mock(ctx, node) => {
                 ctx.report(new.span, "`new Date()` without an argument reads the ambient clock. Take the instant as a parameter so the caller owns it.");
             }
             _ => {}
         }
     }
 }
+
+/// `process.hrtime.bigint`, off the unshadowed global `process`.
+fn is_hrtime_bigint(ctx: &Context<'_, '_>, callee: &Expression<'_>) -> bool {
+    member(callee).is_some_and(|(object, property)| property == "bigint" && is_global_member(ctx, object, "process", "hrtime"))
+}
+
+/// Whether `node` is inside the arguments of a call that builds a mock or sets fake time.
+fn feeds_mock(ctx: &Context<'_, '_>, node: &AstNode<'_>) -> bool {
+    let span = node.kind().span();
+
+    ancestors(ctx.semantic.nodes(), node.id()).any(|ancestor| {
+        let AstKind::CallExpression(call) = ancestor.kind() else {
+            return false;
+        };
+
+        !call.callee.span().contains_inclusive(span)
+            && member(&call.callee).is_some_and(|(object, property)| {
+                MOCK_METHODS.contains(&property) || (FRAMEWORK_MOCKS.contains(&property) && is_test_framework_object(ctx, object))
+            })
+    })
+}

crates/lint/src/rules/anti_slop/no_hookless_use_function.rs

@@ -0,0 +1,132 @@
+//! `anti-slop/no-hookless-use-function`: a `use*` function calls a hook. The
+//! React Compiler only compiles functions that do, so a hookless `useX` is
+//! skipped and rebuilds what it returns on every render. Adapted from
+//! t3code's `t3code/no-hookless-use-function`, after eslint-react's
+//! `no-unnecessary-use-prefix`.
+//!
+//! Vue and other frameworks name plain composables `useX` too, so the rule
+//! only reads React files: `.jsx` and `.tsx` files, and files importing
+//! `react`. A `.vue` file never is one.
+
+use std::ffi::OsStr;
+use std::path::Path;
+
+use oxc_ast::AstKind;
+use oxc_ast::ast::{ArrowFunctionExpression, BindingPattern, CallExpression, Expression, Function};
+use oxc_ast_visit::{Visit, walk};
+use oxc_semantic::{AstNode, Semantic};
+use oxc_syntax::scope::ScopeFlags;
+
+use super::super::{Context, Rule};
+use super::shared::strip_parens;
+
+pub const NAME: &str = "anti-slop/no-hookless-use-function";
+
+pub struct NoHooklessUseFunction;
+
+impl Rule for NoHooklessUseFunction {
+    fn name(&self) -> &'static str {
+        NAME
+    }
+
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &mut Context<'_, 'a>) {
+        let (name, span, calls_hook) = match node.kind() {
+            AstKind::Function(function) if function.is_declaration() => {
+                // An overload or `declare function` has no body to check.
+                let (Some(id), Some(body)) = (&function.id, function.body.as_deref()) else {
+                    return;
+                };
+
+                (id.name.as_str(), id.span, HookCall::find(|finder| finder.visit_function_body(body)))
+            }
+            AstKind::VariableDeclarator(declarator) => {
+                let (BindingPattern::BindingIdentifier(id), Some(init)) = (&declarator.id, &declarator.init) else {
+                    return;
+                };
+
+                let calls_hook = match init {
+                    Expression::ArrowFunctionExpression(arrow) => HookCall::find(|finder| finder.visit_arrow_function_body(&arrow.body)),
+                    Expression::FunctionExpression(function) => {
+                        function.body.as_deref().is_some_and(|body| HookCall::find(|finder| finder.visit_function_body(body)))
+                    }
+                    _ => return,
+                };
+
+                (id.name.as_str(), id.span, calls_hook)
+            }
+            _ => return,
+        };
+
+        if is_hook_name(name) && !calls_hook && is_react_file(ctx) {
+            ctx.report(
+                span,
+                format!(
+                    "`{name}` calls no hooks, so the React Compiler skips it and whatever it returns is rebuilt on every render. Rename it to a plain function, or call the hook it depends on inside it."
+                ),
+            );
+        }
+    }
+}
+
+/// Whether the file is React code; see the module docs.
+fn is_react_file(ctx: &mut Context<'_, '_>) -> bool {
+    match Path::new(ctx.rel).extension().and_then(OsStr::to_str) {
+        Some("vue") => false,
+        Some("jsx" | "tsx") => true,
+        _ => ctx.shared(ImportsReact::detect).0,
+    }
+}
+
+/// Whether the file imports `react` or one of its subpaths.
+struct ImportsReact(bool);
+
+impl ImportsReact {
+    fn detect(semantic: &Semantic<'_>) -> Self {
+        Self(semantic.nodes().iter().any(
+            |node| matches!(node.kind(), AstKind::ImportDeclaration(import) if import.source.value == "react" || import.source.value.starts_with("react/")),
+        ))
+    }
+}
+
+/// `useX` as a hook name: `use` then an uppercase letter.
+fn is_hook_name(name: &str) -> bool {
+    name.strip_prefix("use").is_some_and(|rest| rest.starts_with(|c: char| c.is_ascii_uppercase()))
+}
+
+/// Whether `callee` calls a hook: `useX(…)`, React 19's bare `use(…)`, or `x.useX(…)`.
+fn is_hook_callee(callee: &Expression<'_>) -> bool {
+    let name = match strip_parens(callee) {
+        Expression::Identifier(identifier) => identifier.name.as_str(),
+        Expression::StaticMemberExpression(member) => member.property.name.as_str(),
+        _ => return false,
+    };
+
+    name == "use" || is_hook_name(name)
+}
+
+/// Finds a hook call outside nested functions: a hook called in a callback
+/// runs when the callback does, not while rendering.
+struct HookCall(bool);
+
+impl HookCall {
+    fn find(visit: impl FnOnce(&mut Self)) -> bool {
+        let mut finder = Self(false);
+
+        visit(&mut finder);
+        finder.0
+    }
+}
+
+impl<'a> Visit<'a> for HookCall {
+    fn visit_call_expression(&mut self, call: &CallExpression<'a>) {
+        if is_hook_callee(&call.callee) {
+            self.0 = true;
+        } else {
+            walk::walk_call_expression(self, call);
+        }
+    }
+
+    fn visit_function(&mut self, _: &Function<'a>, _: ScopeFlags) {}
+
+    fn visit_arrow_function_expression(&mut self, _: &ArrowFunctionExpression<'a>) {}
+}