mirror of
https://github.com/danbulant/oxc
synced 2026-05-24 12:21:58 +00:00
feat(linter/node): implement no-exports-assign (#5370)
This commit is contained in:
parent
0a5780d328
commit
4473779074
11 changed files with 175 additions and 3 deletions
|
|
@ -261,6 +261,10 @@ pub struct EnablePlugins {
|
||||||
/// Enable the promise plugin and detect promise usage problems
|
/// Enable the promise plugin and detect promise usage problems
|
||||||
#[bpaf(switch, hide_usage)]
|
#[bpaf(switch, hide_usage)]
|
||||||
pub promise_plugin: bool,
|
pub promise_plugin: bool,
|
||||||
|
|
||||||
|
/// Enable the node plugin and detect node usage problems
|
||||||
|
#[bpaf(switch, hide_usage)]
|
||||||
|
pub node_plugin: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
|
|
|
||||||
|
|
@ -105,7 +105,8 @@ impl Runner for LintRunner {
|
||||||
.with_jsx_a11y_plugin(enable_plugins.jsx_a11y_plugin)
|
.with_jsx_a11y_plugin(enable_plugins.jsx_a11y_plugin)
|
||||||
.with_nextjs_plugin(enable_plugins.nextjs_plugin)
|
.with_nextjs_plugin(enable_plugins.nextjs_plugin)
|
||||||
.with_react_perf_plugin(enable_plugins.react_perf_plugin)
|
.with_react_perf_plugin(enable_plugins.react_perf_plugin)
|
||||||
.with_promise_plugin(enable_plugins.promise_plugin);
|
.with_promise_plugin(enable_plugins.promise_plugin)
|
||||||
|
.with_node_plugin(enable_plugins.node_plugin);
|
||||||
|
|
||||||
let linter = match Linter::from_options(lint_options) {
|
let linter = match Linter::from_options(lint_options) {
|
||||||
Ok(lint_service) => lint_service,
|
Ok(lint_service) => lint_service,
|
||||||
|
|
|
||||||
|
|
@ -144,6 +144,12 @@ impl LintOptions {
|
||||||
self.plugins.promise = yes;
|
self.plugins.promise = yes;
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[must_use]
|
||||||
|
pub fn with_node_plugin(mut self, yes: bool) -> Self {
|
||||||
|
self.plugins.node = yes;
|
||||||
|
self
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl LintOptions {
|
impl LintOptions {
|
||||||
|
|
@ -240,6 +246,7 @@ impl LintOptions {
|
||||||
"oxc" => self.plugins.oxc,
|
"oxc" => self.plugins.oxc,
|
||||||
"eslint" | "tree_shaking" => true,
|
"eslint" | "tree_shaking" => true,
|
||||||
"promise" => self.plugins.promise,
|
"promise" => self.plugins.promise,
|
||||||
|
"node" => self.plugins.node,
|
||||||
name => panic!("Unhandled plugin: {name}"),
|
name => panic!("Unhandled plugin: {name}"),
|
||||||
})
|
})
|
||||||
.cloned()
|
.cloned()
|
||||||
|
|
|
||||||
|
|
@ -17,6 +17,7 @@ pub struct LintPluginOptions {
|
||||||
pub nextjs: bool,
|
pub nextjs: bool,
|
||||||
pub react_perf: bool,
|
pub react_perf: bool,
|
||||||
pub promise: bool,
|
pub promise: bool,
|
||||||
|
pub node: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Default for LintPluginOptions {
|
impl Default for LintPluginOptions {
|
||||||
|
|
@ -34,6 +35,7 @@ impl Default for LintPluginOptions {
|
||||||
nextjs: false,
|
nextjs: false,
|
||||||
react_perf: false,
|
react_perf: false,
|
||||||
promise: false,
|
promise: false,
|
||||||
|
node: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -55,6 +57,7 @@ impl LintPluginOptions {
|
||||||
nextjs: false,
|
nextjs: false,
|
||||||
react_perf: false,
|
react_perf: false,
|
||||||
promise: false,
|
promise: false,
|
||||||
|
node: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -74,6 +77,7 @@ impl LintPluginOptions {
|
||||||
nextjs: true,
|
nextjs: true,
|
||||||
react_perf: true,
|
react_perf: true,
|
||||||
promise: true,
|
promise: true,
|
||||||
|
node: true,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -99,6 +103,7 @@ impl<S: AsRef<str>> FromIterator<(S, bool)> for LintPluginOptions {
|
||||||
"nextjs" => options.nextjs = enabled,
|
"nextjs" => options.nextjs = enabled,
|
||||||
"react-perf" => options.react_perf = enabled,
|
"react-perf" => options.react_perf = enabled,
|
||||||
"promise" => options.promise = enabled,
|
"promise" => options.promise = enabled,
|
||||||
|
"node" => options.node = enabled,
|
||||||
_ => { /* ignored */ }
|
_ => { /* ignored */ }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -129,6 +134,7 @@ mod test {
|
||||||
&& self.nextjs == other.nextjs
|
&& self.nextjs == other.nextjs
|
||||||
&& self.react_perf == other.react_perf
|
&& self.react_perf == other.react_perf
|
||||||
&& self.promise == other.promise
|
&& self.promise == other.promise
|
||||||
|
&& self.node == other.node
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -160,6 +166,7 @@ mod test {
|
||||||
nextjs: false,
|
nextjs: false,
|
||||||
react_perf: false,
|
react_perf: false,
|
||||||
promise: false,
|
promise: false,
|
||||||
|
node: false,
|
||||||
};
|
};
|
||||||
assert_eq!(plugins, expected);
|
assert_eq!(plugins, expected);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -470,6 +470,10 @@ mod vitest {
|
||||||
pub mod require_local_test_context_for_concurrent_snapshots;
|
pub mod require_local_test_context_for_concurrent_snapshots;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
mod node {
|
||||||
|
pub mod no_exports_assign;
|
||||||
|
}
|
||||||
|
|
||||||
oxc_macros::declare_all_lint_rules! {
|
oxc_macros::declare_all_lint_rules! {
|
||||||
eslint::array_callback_return,
|
eslint::array_callback_return,
|
||||||
eslint::constructor_super,
|
eslint::constructor_super,
|
||||||
|
|
@ -892,4 +896,5 @@ oxc_macros::declare_all_lint_rules! {
|
||||||
vitest::prefer_to_be_truthy,
|
vitest::prefer_to_be_truthy,
|
||||||
vitest::no_conditional_tests,
|
vitest::no_conditional_tests,
|
||||||
vitest::require_local_test_context_for_concurrent_snapshots,
|
vitest::require_local_test_context_for_concurrent_snapshots,
|
||||||
|
node::no_exports_assign,
|
||||||
}
|
}
|
||||||
|
|
|
||||||
129
crates/oxc_linter/src/rules/node/no_exports_assign.rs
Normal file
129
crates/oxc_linter/src/rules/node/no_exports_assign.rs
Normal file
|
|
@ -0,0 +1,129 @@
|
||||||
|
use oxc_ast::{
|
||||||
|
ast::{AssignmentTarget, Expression, IdentifierReference, MemberExpression},
|
||||||
|
AstKind,
|
||||||
|
};
|
||||||
|
use oxc_diagnostics::OxcDiagnostic;
|
||||||
|
use oxc_macros::declare_oxc_lint;
|
||||||
|
use oxc_span::{GetSpan, Span};
|
||||||
|
|
||||||
|
use crate::{context::LintContext, rule::Rule, AstNode};
|
||||||
|
|
||||||
|
fn no_exports_assign(span: Span) -> OxcDiagnostic {
|
||||||
|
OxcDiagnostic::warn("Disallow the assignment to `exports`.")
|
||||||
|
.with_label(span)
|
||||||
|
.with_help("Unexpected assignment to 'exports' variable. Use 'module.exports' instead.")
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_global_reference(ctx: &LintContext, id: &IdentifierReference, name: &str) -> bool {
|
||||||
|
if let Some(reference_id) = id.reference_id() {
|
||||||
|
return id.name == name && ctx.symbols().is_global_reference(reference_id);
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_exports(node: &AssignmentTarget, ctx: &LintContext) -> bool {
|
||||||
|
let AssignmentTarget::AssignmentTargetIdentifier(id) = node else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
|
||||||
|
is_global_reference(ctx, id, "exports")
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_module_exports(expr: Option<&MemberExpression>, ctx: &LintContext) -> bool {
|
||||||
|
let Some(mem_expr) = expr else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
|
||||||
|
let Some(obj_id) = mem_expr.object().get_identifier_reference() else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
|
||||||
|
return mem_expr.static_property_name() == Some("exports")
|
||||||
|
&& is_global_reference(ctx, obj_id, "module");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Default, Clone)]
|
||||||
|
pub struct NoExportsAssign;
|
||||||
|
|
||||||
|
declare_oxc_lint!(
|
||||||
|
/// ### What it does
|
||||||
|
///
|
||||||
|
/// This rule is aimed at disallowing `exports = {}`, but allows `module.exports = exports = {}` to avoid conflict with `n/exports-style` rule's `allowBatchAssign` option.
|
||||||
|
///
|
||||||
|
/// ### Why is this bad?
|
||||||
|
///
|
||||||
|
/// Directly using `exports = {}` can lead to confusion and potential bugs because it reassigns the `exports` object, which may break module exports. It is more predictable and clearer to use `module.exports` directly or in conjunction with `exports`.
|
||||||
|
///
|
||||||
|
/// ### Examples
|
||||||
|
///
|
||||||
|
/// Examples of **incorrect** code for this rule:
|
||||||
|
/// ```js
|
||||||
|
/// exports = {}
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Examples of **correct** code for this rule:
|
||||||
|
/// ```js
|
||||||
|
/// module.exports.foo = 1
|
||||||
|
/// exports.bar = 2
|
||||||
|
/// module.exports = {}
|
||||||
|
///
|
||||||
|
/// // allows `exports = {}` if along with `module.exports =`
|
||||||
|
/// module.exports = exports = {}
|
||||||
|
/// exports = module.exports = {}
|
||||||
|
/// ```
|
||||||
|
NoExportsAssign,
|
||||||
|
style,
|
||||||
|
fix
|
||||||
|
);
|
||||||
|
|
||||||
|
impl Rule for NoExportsAssign {
|
||||||
|
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||||
|
let AstKind::AssignmentExpression(assign_expr) = node.kind() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
if !is_exports(&assign_expr.left, ctx) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if let Expression::AssignmentExpression(assign_expr) = &assign_expr.right {
|
||||||
|
if is_module_exports(assign_expr.left.as_member_expression(), ctx) {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
let parent = ctx.nodes().parent_node(node.id());
|
||||||
|
if let Some(AstKind::AssignmentExpression(assign_expr)) = parent.map(AstNode::kind) {
|
||||||
|
if is_module_exports(assign_expr.left.as_member_expression(), ctx) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
ctx.diagnostic_with_fix(no_exports_assign(assign_expr.left.span()), |fixer| {
|
||||||
|
fixer.replace(assign_expr.left.span(), "module.exports")
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test() {
|
||||||
|
use crate::tester::Tester;
|
||||||
|
|
||||||
|
let pass = vec![
|
||||||
|
"module.exports.foo = 1",
|
||||||
|
"exports.bar = 1",
|
||||||
|
"module.exports = exports = {}",
|
||||||
|
"exports = module.exports = {}",
|
||||||
|
"function f(exports) { exports = {} }",
|
||||||
|
"let exports; exports = {}",
|
||||||
|
];
|
||||||
|
|
||||||
|
let fail = vec!["exports = {}"];
|
||||||
|
|
||||||
|
let fix = vec![("exports = {}", "module.exports = {}")];
|
||||||
|
|
||||||
|
Tester::new(NoExportsAssign::NAME, pass, fail)
|
||||||
|
.expect_fix(fix)
|
||||||
|
.with_node_plugin(true)
|
||||||
|
.test_and_snapshot();
|
||||||
|
}
|
||||||
9
crates/oxc_linter/src/snapshots/no_exports_assign.snap
Normal file
9
crates/oxc_linter/src/snapshots/no_exports_assign.snap
Normal file
|
|
@ -0,0 +1,9 @@
|
||||||
|
---
|
||||||
|
source: crates/oxc_linter/src/tester.rs
|
||||||
|
---
|
||||||
|
⚠ node(no-exports-assign): Disallow the assignment to `exports`.
|
||||||
|
╭─[no_exports_assign.tsx:1:1]
|
||||||
|
1 │ exports = {}
|
||||||
|
· ───────
|
||||||
|
╰────
|
||||||
|
help: Unexpected assignment to 'exports' variable. Use 'module.exports' instead.
|
||||||
|
|
@ -250,6 +250,11 @@ impl Tester {
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn with_node_plugin(mut self, yes: bool) -> Self {
|
||||||
|
self.plugins.node = yes;
|
||||||
|
self
|
||||||
|
}
|
||||||
|
|
||||||
/// Add cases that should fix problems found in the source code.
|
/// Add cases that should fix problems found in the source code.
|
||||||
///
|
///
|
||||||
/// These cases will fail if no fixes are produced or if the fixed source
|
/// These cases will fail if no fixes are produced or if the fixed source
|
||||||
|
|
@ -351,7 +356,8 @@ impl Tester {
|
||||||
.with_vitest_plugin(self.plugins.vitest)
|
.with_vitest_plugin(self.plugins.vitest)
|
||||||
.with_jsx_a11y_plugin(self.plugins.jsx_a11y)
|
.with_jsx_a11y_plugin(self.plugins.jsx_a11y)
|
||||||
.with_nextjs_plugin(self.plugins.nextjs)
|
.with_nextjs_plugin(self.plugins.nextjs)
|
||||||
.with_react_perf_plugin(self.plugins.react_perf);
|
.with_react_perf_plugin(self.plugins.react_perf)
|
||||||
|
.with_node_plugin(self.plugins.node);
|
||||||
let eslint_config = eslint_config
|
let eslint_config = eslint_config
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.map_or_else(OxlintConfig::default, |v| OxlintConfig::deserialize(v).unwrap());
|
.map_or_else(OxlintConfig::default, |v| OxlintConfig::deserialize(v).unwrap());
|
||||||
|
|
|
||||||
|
|
@ -46,7 +46,8 @@ fn bench_linter(criterion: &mut Criterion) {
|
||||||
.with_jsx_a11y_plugin(true)
|
.with_jsx_a11y_plugin(true)
|
||||||
.with_nextjs_plugin(true)
|
.with_nextjs_plugin(true)
|
||||||
.with_react_perf_plugin(true)
|
.with_react_perf_plugin(true)
|
||||||
.with_vitest_plugin(true);
|
.with_vitest_plugin(true)
|
||||||
|
.with_node_plugin(true);
|
||||||
let linter = Linter::from_options(lint_options).unwrap();
|
let linter = Linter::from_options(lint_options).unwrap();
|
||||||
let semantic = Rc::new(semantic_ret.semantic);
|
let semantic = Rc::new(semantic_ret.semantic);
|
||||||
b.iter(|| linter.run(Path::new(std::ffi::OsStr::new("")), Rc::clone(&semantic)));
|
b.iter(|| linter.run(Path::new(std::ffi::OsStr::new("")), Rc::clone(&semantic)));
|
||||||
|
|
|
||||||
|
|
@ -65,6 +65,8 @@ Arguments:
|
||||||
Enable the React performance plugin and detect rendering performance problems
|
Enable the React performance plugin and detect rendering performance problems
|
||||||
- **` --promise-plugin`** —
|
- **` --promise-plugin`** —
|
||||||
Enable the promise plugin and detect promise usage problems
|
Enable the promise plugin and detect promise usage problems
|
||||||
|
- **` --node-plugin`** —
|
||||||
|
Enable the node plugin and detect node usage problems
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -42,6 +42,7 @@ Enable Plugins
|
||||||
--react-perf-plugin Enable the React performance plugin and detect rendering performance
|
--react-perf-plugin Enable the React performance plugin and detect rendering performance
|
||||||
problems
|
problems
|
||||||
--promise-plugin Enable the promise plugin and detect promise usage problems
|
--promise-plugin Enable the promise plugin and detect promise usage problems
|
||||||
|
--node-plugin Enable the node plugin and detect node usage problems
|
||||||
|
|
||||||
Fix Problems
|
Fix Problems
|
||||||
--fix Fix as many issues as possible. Only unfixed issues are reported in
|
--fix Fix as many issues as possible. Only unfixed issues are reported in
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue