-
-
Notifications
You must be signed in to change notification settings - Fork 501
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(linter): implement noCommonJs (#3821)
- Loading branch information
Showing
13 changed files
with
471 additions
and
143 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16 changes: 16 additions & 0 deletions
16
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.
Oops, something went wrong.
305 changes: 162 additions & 143 deletions
305
crates/biome_configuration/src/analyzer/linter/rules.rs
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
153 changes: 153 additions & 0 deletions
153
crates/biome_js_analyze/src/lint/nursery/no_common_js.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
8 changes: 8 additions & 0 deletions
8
crates/biome_js_analyze/tests/specs/nursery/noCommonJs/invalid.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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' |
80 changes: 80 additions & 0 deletions
80
crates/biome_js_analyze/tests/specs/nursery/noCommonJs/invalid.js.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
14
crates/biome_js_analyze/tests/specs/nursery/noCommonJs/valid.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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' |
21 changes: 21 additions & 0 deletions
21
crates/biome_js_analyze/tests/specs/nursery/noCommonJs/valid.js.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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' | ||
``` |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.