Skip to content

Commit

Permalink
feat(linter): implement noCommonJs (#3821)
Browse files Browse the repository at this point in the history
  • Loading branch information
minht11 committed Sep 10, 2024
1 parent a66e450 commit 37cf1e0
Show file tree
Hide file tree
Showing 13 changed files with 471 additions and 143 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
Contributed by @Conaclos

- Add [nursery/useConsistentMemberAccessibility](https://biomejs.dev/linter/rules/use-consistent-member-accessibility/). Contributed by @seitarof
- Add [nursery/noCommonJs](https://biomejs.dev/linter/rules/no-common-js/). Contributed by @minht11

#### Enhancements

Expand Down
16 changes: 16 additions & 0 deletions crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

305 changes: 162 additions & 143 deletions crates/biome_configuration/src/analyzer/linter/rules.rs

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions crates/biome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ define_categories! {
"lint/correctness/useYield": "https://biomejs.dev/linter/rules/use-yield",
"lint/nursery/colorNoInvalidHex": "https://biomejs.dev/linter/rules/color-no-invalid-hex",
"lint/nursery/noColorInvalidHex": "https://biomejs.dev/linter/rules/no-color-invalid-hex",
"lint/nursery/noCommonJs": "https://biomejs.dev/linter/rules/no-common-js",
"lint/nursery/noConsole": "https://biomejs.dev/linter/rules/no-console",
"lint/nursery/noDoneCallback": "https://biomejs.dev/linter/rules/no-done-callback",
"lint/nursery/noDuplicateAtImportRules": "https://biomejs.dev/linter/rules/no-duplicate-at-import-rules",
Expand Down
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use biome_analyze::declare_lint_group;

pub mod no_common_js;
pub mod no_console;
pub mod no_done_callback;
pub mod no_duplicate_else_if;
Expand Down Expand Up @@ -50,6 +51,7 @@ declare_lint_group! {
pub Nursery {
name : "nursery" ,
rules : [
self :: no_common_js :: NoCommonJs ,
self :: no_console :: NoConsole ,
self :: no_done_callback :: NoDoneCallback ,
self :: no_duplicate_else_if :: NoDuplicateElseIf ,
Expand Down
153 changes: 153 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery/no_common_js.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
use biome_analyze::{context::RuleContext, declare_lint_rule, Rule, RuleDiagnostic, RuleSource};
use biome_console::markup;
use biome_js_syntax::{
global_identifier, JsCallExpression, JsReferenceIdentifier, JsStaticMemberAssignment,
};
use biome_rowan::{declare_node_union, AstNode};

use crate::services::semantic::Semantic;

declare_lint_rule! {
/// Disallow use of CommonJs module system in favor of ESM style imports.
///
/// ESM-style `import`s are modern alternative to CommonJS `require` imports. Supported by all modern browsers and Node.js versions.
/// Tooling can more easily statically analyze and tree-shake ESM `import`s compared to CommonJs.
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// require('node:fs');
/// ```
/// ```js,expect_diagnostic
/// module.exports = { a: 'b' }
/// ```
/// ```js,expect_diagnostic
/// exports.a = 'b';
/// ```
///
/// ### Valid
///
/// ```js
/// import fs from 'node:fs';
/// ```
/// ```js
/// import('node:fs')
/// ```
/// ```js
/// export const a = 'b';
/// ```
/// ```js
/// export default { a: 'b' };
/// ```
///
/// ## Caveats
///
/// Rule is automatically disabled inside `.cjs` and `.cts` files, because they are explicitly CommonJs files.
///
/// This rule could be helpful if you are migrating from CommonJs to ESM,
/// but if you wish to continue using CommonJs, you can safely disable it.
///
pub NoCommonJs {
version: "1.9.0",
name: "noCommonJs",
language: "js",
sources: &[
RuleSource::EslintTypeScript("no-require-imports"),
RuleSource::EslintImport("no-commonjs"),
],
recommended: false,
}
}

impl Rule for NoCommonJs {
type Query = Semantic<CommonJsImportExport>;
type State = bool;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let file_ext = ctx.file_path().extension();
// cjs and cts files can only use CommonJs modules
if file_ext.is_some_and(|file_ext| matches!(file_ext.as_encoded_bytes(), b"cjs" | b"cts")) {
return None;
}

let expression = ctx.query();
let (reference, is_export) = match expression {
CommonJsImportExport::JsCallExpression(node) => {
Some((is_require_call_expression(node)?, false))
}
CommonJsImportExport::JsStaticMemberAssignment(node) => {
Some((is_common_js_exports(node)?, true))
}
}?;

if ctx.model().binding(&reference).is_none() {
return Some(is_export);
}

None
}

fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let node = ctx.query();
let (es_name, common_js_name) = match state {
true => ("export", "module.exports"),
false => ("import", "require"),
};

Some(
RuleDiagnostic::new(
rule_category!(),
node.range(),
markup! {
"Use ESM "<Emphasis>{es_name}</Emphasis>"s instead of "<Emphasis>{common_js_name}</Emphasis>"."
},
)
.note(markup! {
"ESM-style "<Emphasis>{es_name}</Emphasis>" statements are more easily statically analyzable and tree-shakable compared to CommonJs "<Emphasis>{common_js_name}</Emphasis>"."
}),
)
}
}

declare_node_union! {
pub CommonJsImportExport = JsStaticMemberAssignment | JsCallExpression
}

fn is_require_call_expression(node: &JsCallExpression) -> Option<JsReferenceIdentifier> {
let callee = node.callee().ok()?;
let (reference, name) = global_identifier(&callee.omit_parentheses())?;

if name.text() == "require" {
return Some(reference);
}

None
}

fn is_common_js_exports(node: &JsStaticMemberAssignment) -> Option<JsReferenceIdentifier> {
let object = node.object().ok()?;
let (reference, name) = global_identifier(&object.omit_parentheses())?;
let object_name = name.text();

// exports.*
if object_name == "exports" {
return Some(reference);
}

// module.exports.*
if object_name != "module" {
return None;
}

let value_token = node.member().ok()?.value_token().ok()?;
let member_name = value_token.text_trimmed();
if member_name == "exports" {
return Some(reference);
}

None
}
1 change: 1 addition & 0 deletions crates/biome_js_analyze/src/options.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
require('node:fs')

if (true) {
require('node:fs')
}

module.exports = 'path'
exports.path = 'path'
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: invalid.js
---
# Input
```jsx
require('node:fs')

if (true) {
require('node:fs')
}

module.exports = 'path'
exports.path = 'path'

```

# Diagnostics
```
invalid.js:1:1 lint/nursery/noCommonJs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use ESM imports instead of require.
> 1 │ require('node:fs')
│ ^^^^^^^^^^^^^^^^^^
2 │
3 │ if (true) {
i ESM-style import statements are more easily statically analyzable and tree-shakable compared to CommonJs require.
```

```
invalid.js:4:5 lint/nursery/noCommonJs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use ESM imports instead of require.
3 │ if (true) {
> 4 │ require('node:fs')
│ ^^^^^^^^^^^^^^^^^^
5 │ }
6 │
i ESM-style import statements are more easily statically analyzable and tree-shakable compared to CommonJs require.
```

```
invalid.js:7:1 lint/nursery/noCommonJs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use ESM exports instead of module.exports.
5 │ }
6 │
> 7 │ module.exports = 'path'
│ ^^^^^^^^^^^^^^
8 │ exports.path = 'path'
9 │
i ESM-style export statements are more easily statically analyzable and tree-shakable compared to CommonJs module.exports.
```

```
invalid.js:8:1 lint/nursery/noCommonJs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Use ESM exports instead of module.exports.
7 │ module.exports = 'path'
> 8 │ exports.path = 'path'
│ ^^^^^^^^^^^^
9 │
i ESM-style export statements are more easily statically analyzable and tree-shakable compared to CommonJs module.exports.
```
14 changes: 14 additions & 0 deletions crates/biome_js_analyze/tests/specs/nursery/noCommonJs/valid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import 'node:fs'
import { join } from 'node:path'

import('node:fs/promises')

const require = () => {}

require('node:fs')

const module = {}
module.exports = 'path'

const exports = {}
exports.path = 'path'
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: valid.js
---
# Input
```jsx
import 'node:fs'
import { join } from 'node:path'

import('node:fs/promises')

const require = () => {}

require('node:fs')

const module = {}
module.exports = 'path'

const exports = {}
exports.path = 'path'
```
5 changes: 5 additions & 0 deletions packages/@biomejs/backend-jsonrpc/src/workspace.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions packages/@biomejs/biome/configuration_schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 37cf1e0

Please sign in to comment.