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

feat(linter): Implement node/no-new-require rule #6615

Closed
Closed
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: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,7 @@ mod vitest {

mod node {
pub mod no_exports_assign;
pub mod no_new_require;
}

oxc_macros::declare_all_lint_rules! {
Expand Down Expand Up @@ -738,6 +739,7 @@ oxc_macros::declare_all_lint_rules! {
nextjs::no_typos,
nextjs::no_unwanted_polyfillio,
node::no_exports_assign,
node::no_new_require,
oxc::approx_constant,
oxc::bad_array_method_on_arguments,
oxc::bad_bitwise_operator,
Expand Down
86 changes: 86 additions & 0 deletions crates/oxc_linter/src/rules/node/no_new_require.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use oxc_ast::ast::Expression;
use oxc_ast::AstKind;
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{
context::LintContext,
rule::Rule,
AstNode,
};

fn no_new_require_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("`const myInstance = new require('some-module')` is likely unintended as it does not invoke the constructor exported from `'some-module'`.")
.with_help("Replace with one of the correct alternatives instead. `const myInstance = new (require('some-module'))`, `const myInstance = new (require('some-module'))`, or `const MyConstructor = require('some-module'); const myInstance = new MyConstructor();`")
.with_label(span)
}

#[derive(Debug, Default, Clone)]
pub struct NoNewRequire;

declare_oxc_lint!(
/// ### What it does
///
/// Disallows the likely incorrect code of `new require('some-module')`.
///
/// ### Why is this bad?
///
/// The code author likely intended to use `new (require('some-module'))` instead to invoke the constructor returned by `require('some-module')`.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```js
/// const myInstance = new require('some-module');
/// ```
///
/// Examples of **correct** code for this rule:
/// ```js
/// const SomeModule = require('some-module');
/// const myInstance = new SomeModule();
///
/// const myInstance = new (require('some-module'))();
/// ```
NoNewRequire,
suspicious,

pending // TODO: describe fix capabilities. Remove if no fix can be done,
// keep at 'pending' if you think one could be added but don't know how.
// Options are 'fix', 'fix_dangerous', 'suggestion', and 'conditional_fix_suggestion'
);

impl Rule for NoNewRequire {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::NewExpression(new_expr) = node.kind() else {
return;
};

let Expression::Identifier(ident) = &new_expr.callee.without_parentheses() else {
return;
};
if ident.name != "require" {
return;
}
ctx.diagnostic(no_new_require_diagnostic(ident.span))
}
}

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

let pass = vec![
"var appHeader = require('app-header')",
"var AppHeader = new (require('app-header'))",
"var AppHeader = new (require('app-header'))()",
"var AppHeader = new (require('headers').appHeader)",
];

let fail = vec![
"var appHeader = new require('app-header')",
"var appHeader = new require('headers').appHeader",
];

Tester::new(NoNewRequire::NAME, pass, fail).test_and_snapshot();
}
16 changes: 16 additions & 0 deletions crates/oxc_linter/src/snapshots/no_new_require.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint-plugin-node(no-new-require): `const myInstance = new require('some-module')` is likely unintended as it does not invoke the constructor exported from `'some-module'`.
╭─[no_new_require.tsx:1:21]
1 │ var appHeader = new require('app-header')
· ───────
╰────
help: Replace with one of the correct alternatives instead. `const myInstance = new (require('some-module'))`, `const myInstance = new (require('some-module'))`, or `const MyConstructor = require('some-module'); const myInstance = new MyConstructor();`

⚠ eslint-plugin-node(no-new-require): `const myInstance = new require('some-module')` is likely unintended as it does not invoke the constructor exported from `'some-module'`.
╭─[no_new_require.tsx:1:21]
1 │ var appHeader = new require('headers').appHeader
· ───────
╰────
help: Replace with one of the correct alternatives instead. `const myInstance = new (require('some-module'))`, `const myInstance = new (require('some-module'))`, or `const MyConstructor = require('some-module'); const myInstance = new MyConstructor();`
Loading