Skip to content

Commit

Permalink
docs(linter): improve docs for eslint-plugin-import rules (#6131)
Browse files Browse the repository at this point in the history
Related to #6050
  • Loading branch information
shulaoda authored Sep 28, 2024
1 parent 375bebe commit 14ba263
Show file tree
Hide file tree
Showing 16 changed files with 291 additions and 62 deletions.
22 changes: 20 additions & 2 deletions crates/oxc_linter/src/rules/import/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,35 @@ pub struct Default;
declare_oxc_lint!(
/// ### What it does
///
/// If a default import is requested, this rule will report if there is no default export in the imported module.
/// If a default import is requested, this rule will report if there is no
/// default export in the imported module.
///
/// ### Example
/// ### Why is this bad?
///
/// Using a default import when there is no default export can lead to
/// confusion and runtime errors. It can make the code harder to understand
/// and maintain, as it may suggest that a module has a default export
/// when it does not, leading to unexpected behavior.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```javascript
/// // ./bar.js
/// export function bar() { return null }
///
/// // ./foo.js
/// import bar from './bar' // no default export found in ./bar
/// ```
///
/// Examples of **correct** code for this rule:
/// ```javascript
/// // ./bar.js
/// export default function bar() { return null }
///
/// // ./foo.js
/// import { bar } from './bar' // correct usage of named import
/// ```
Default,
correctness
);
Expand Down
24 changes: 20 additions & 4 deletions crates/oxc_linter/src/rules/import/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_hash::{FxHashMap, FxHashSet};

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

fn no_named_export(span: Span, module_name: &str) -> OxcDiagnostic {
fn no_named_export(module_name: &str, span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("No named exports found in module '{module_name}'"))
.with_label(span)
}
Expand All @@ -19,13 +19,29 @@ pub struct Export;

declare_oxc_lint!(
/// ### What it does
///
/// Reports funny business with exports, like repeated exports of names or defaults.
///
/// ### Example
/// ### Why is this bad?
///
/// Having multiple exports of the same name can lead to ambiguity and confusion
/// in the codebase. It makes it difficult to track which export is being used
/// and can result in runtime errors if the wrong export is referenced.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```javascript
/// let foo;
/// export { foo }; // Multiple exports of name 'foo'.
/// export * from "./export-all" // export-all.js also export foo
/// export * from "./export-all"; // Conflicts if export-all.js also exports foo
/// ```
///
/// Examples of **correct** code for this rule:
/// ```javascript
/// let foo;
/// export { foo as foo1 }; // Renamed export to avoid conflict
/// export * from "./export-all"; // No conflict if export-all.js also exports foo
/// ```
Export,
nursery
Expand Down Expand Up @@ -58,7 +74,7 @@ impl Rule for Export {
);

if export_names.is_empty() {
ctx.diagnostic(no_named_export(module_request.span(), module_request.name()));
ctx.diagnostic(no_named_export(module_request.name(), module_request.span()));
} else {
all_export_names.insert(star_export_entry.span, export_names);
}
Expand Down
23 changes: 15 additions & 8 deletions crates/oxc_linter/src/rules/import/max_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ use crate::{context::LintContext, rule::Rule};

fn max_dependencies_diagnostic<S: Into<Cow<'static, str>>>(
message: S,
span1: Span,
span: Span,
) -> OxcDiagnostic {
OxcDiagnostic::warn(message)
.with_help("Reduce the number of dependencies in this file")
.with_label(span1)
.with_label(span)
}

/// <https://github.com/import-js/eslint-plugin-import/blob/v2.29.1/docs/rules/max-dependencies.md>
Expand Down Expand Up @@ -47,19 +47,26 @@ declare_oxc_lint!(
///
/// ### Why is this bad?
///
/// This is a useful rule because a module with too many dependencies is a code smell, and
/// usually indicates the module is doing too much and/or should be broken up into smaller
/// modules.
/// This is a useful rule because a module with too many dependencies is a code smell,
/// and usually indicates the module is doing too much and/or should be broken up into
/// smaller modules.
///
/// ### Example
/// ### Examples
///
/// Given `{"max": 2}`
///
/// Examples of **incorrect** code for this rule:
/// ```javascript
/// import a from './a';
/// import b from './b';
/// import c from './c';
/// import c from './c'; // Too many dependencies: 3 (max: 2)
/// ```
///
/// Examples of **correct** code for this rule:
/// ```javascript
/// import a from './a';
/// import b from './b'; // Allowed: 2 dependencies (max: 2)
/// ```
MaxDependencies,
pedantic,
);
Expand Down
37 changes: 21 additions & 16 deletions crates/oxc_linter/src/rules/import/named.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,33 @@ declare_oxc_lint!(
///
/// ### Why is this bad?
///
/// ### Example
/// Importing or exporting names that do not exist in the referenced module
/// can lead to runtime errors and confusion. It may suggest that certain
/// functionality is available when it is not, making the code harder to
/// maintain and understand. This rule helps ensure that your code
/// accurately reflects the available exports, improving reliability.
///
/// ### Examples
///
/// Given
/// ```js
/// // ./foo.js
/// export const foo = "I'm so foo"
/// export const foo = "I'm so foo";
/// ```
///
/// The following is considered valid:
/// Examples of **incorrect** code for this rule:
/// ```js
/// // ./baz.js
/// import { notFoo } from './foo'
///
/// // ES7 proposal
/// export { notFoo as defNotBar } from './foo'
///
/// // will follow 'jsnext:main', if available
/// import { dontCreateStore } from 'redux'
/// ```
///
/// Examples of **correct** code for this rule:
/// ```js
/// // ./bar.js
/// import { foo } from './foo'
Expand All @@ -55,19 +73,6 @@ declare_oxc_lint!(
/// // (import/ignore setting)
/// import { SomeNonsenseThatDoesntExist } from 'react'
/// ```
///
/// ...and the following are reported:
///
/// ```js
/// // ./baz.js
/// import { notFoo } from './foo'
///
/// // ES7 proposal
/// export { notFoo as defNotBar } from './foo'
///
/// // will follow 'jsnext:main', if available
/// import { dontCreateStore } from 'redux'
/// ```
Named,
correctness
);
Expand Down
42 changes: 42 additions & 0 deletions crates/oxc_linter/src/rules/import/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,54 @@ pub struct Namespace {

declare_oxc_lint!(
/// ### What it does
///
/// Enforces names exist at the time they are dereferenced, when imported as
/// a full namespace (i.e. `import * as foo from './foo'; foo.bar();` will
/// report if bar is not exported by `./foo.`). Will report at the import
/// declaration if there are no exported names found. Also, will report for
/// computed references (i.e. `foo["bar"]()`). Reports on assignment to a
/// member of an imported namespace.
///
/// ### Why is this bad?
///
/// Dereferencing a name that does not exist can lead to runtime errors and
/// unexpected behavior in your code. It makes the code less reliable and
/// harder to maintain, as it may not be clear which names are valid. This
/// rule helps ensure that all referenced names are defined, improving
/// the clarity and robustness of your code.
///
/// ### Examples
///
/// Given
/// ```javascript
/// // ./foo.js
/// export const bar = "I'm bar";
/// ```
///
/// Examples of **incorrect** code for this rule:
/// ```javascript
/// // ./qux.js
/// import * as foo from './foo';
/// foo.notExported(); // Error: notExported is not exported
///
/// // Assignment to a member of an imported namespace
/// foo.bar = "new value"; // Error: bar cannot be reassigned
///
/// // Computed reference to a non-existent export
/// const method = "notExported";
/// foo[method](); // Error: notExported does not exist
/// ```
///
/// Examples of **correct** code for this rule:
/// ```javascript
/// // ./baz.js
/// import * as foo from './foo';
/// console.log(foo.bar); // Valid: bar is exported
///
/// // Computed reference
/// const method = "bar";
/// foo[method](); // Valid: method refers to an exported function
/// ```
Namespace,
correctness
);
Expand Down
10 changes: 8 additions & 2 deletions crates/oxc_linter/src/rules/import/no_amd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,20 @@ pub struct NoAmd;
declare_oxc_lint!(
/// ### What it does
///
/// Forbid AMD `require` and `define` calls.
/// Forbids the use of AMD `require` and `define` calls.
///
/// ### Why is this bad?
///
/// AMD (Asynchronous Module Definition) is an older module format
/// that is less common in modern JavaScript development, especially
/// with the widespread use of ES6 modules and CommonJS in Node.js.
/// AMD introduces unnecessary complexity and is often considered outdated.
/// This rule enforces the use of more modern module systems to improve
/// maintainability and consistency across the codebase.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
///
/// ```javascript
/// require([a, b], function() {} );
/// ```
Expand Down
23 changes: 20 additions & 3 deletions crates/oxc_linter/src/rules/import/no_cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,37 @@ declare_oxc_lint!(
/// ### Why is this bad?
///
/// Dependency cycles lead to confusing architectures where bugs become hard to find.
///
/// It is common to import an `undefined` value that is caused by a cyclic dependency.
///
/// ### Example
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```javascript
/// // dep-b.js
/// import './dep-a.js'
/// export function b() { /* ... */ }
/// ```
///
/// ```javascript
/// // dep-a.js
/// import { b } from './dep-b.js' // reported: Dependency cycle detected.
/// export function a() { /* ... */ }
/// ```
///
/// In this example, `dep-a.js` and `dep-b.js` import each other, creating a circular
/// dependency, which is problematic.
///
/// Examples of **correct** code for this rule:
/// ```javascript
/// // dep-b.js
/// export function b() { /* ... */ }
/// ```
/// ```javascript
/// // dep-a.js
/// import { b } from './dep-b.js' // no circular dependency
/// export function a() { /* ... */ }
/// ```
///
/// In this corrected version, `dep-b.js` no longer imports `dep-a.js`, breaking the cycle.
NoCycle,
restriction
);
Expand Down
13 changes: 11 additions & 2 deletions crates/oxc_linter/src/rules/import/no_default_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,15 @@ pub struct NoDefaultExport;
declare_oxc_lint!(
/// ### What it does
///
/// Forbid a module to have a default exports. This help your editor to provide better auto imports.
/// Forbids a module from having default exports. This helps your editor
/// provide better auto-import functionality, as named exports offer more
/// explicit and predictable imports compared to default exports.
///
/// ### Why is this bad?
///
/// Default exports can lead to confusion, as the name of the imported value
/// can vary based on how it's imported. This can make refactoring and
/// auto-imports less reliable.
///
/// ### Examples
///
Expand All @@ -31,7 +39,6 @@ declare_oxc_lint!(
/// export const foo = 'foo';
/// export const bar = 'bar';
/// ```
///
NoDefaultExport,
restriction
);
Expand All @@ -48,6 +55,7 @@ impl Rule for NoDefaultExport {
fn write_diagnostic(ctx: &LintContext<'_>, span: Span) {
ctx.diagnostic(no_default_export_diagnostic(span));
}

fn write_diagnostic_optional(ctx: &LintContext<'_>, span_option: Option<Span>) {
if let Some(span) = span_option {
write_diagnostic(ctx, span);
Expand Down Expand Up @@ -77,6 +85,7 @@ fn test() {
"import {default as foo} from './foo';",
"export type UserId = number;",
];

let fail = vec![
"export default function bar() {};",
"export const foo = 'foo';\nexport default bar;",
Expand Down
20 changes: 18 additions & 2 deletions crates/oxc_linter/src/rules/import/no_deprecated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ use oxc_diagnostics::{LabeledSpan, OxcDiagnostic};
use oxc_macros::declare_oxc_lint;
// use oxc_span::{CompactStr, Span};

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

// #[derive(Debug, Error, Diagnostic)]
// #[error("")]
Expand All @@ -16,7 +19,20 @@ pub struct NoDeprecated;
declare_oxc_lint!(
/// ### What it does
///
/// Reports use of a deprecated name, as indicated by a JSDoc block with a @deprecated tag or TomDoc Deprecated: comment.
/// Reports use of a deprecated name, as indicated by a JSDoc block with
/// a @deprecated tag or TomDoc Deprecated: comment.
///
/// ### Why is this bad?
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```javascript
/// ```
///
/// Examples of **correct** code for this rule:
/// ```javascript
/// ```
NoDeprecated,
nursery
);
Expand Down
Loading

0 comments on commit 14ba263

Please sign in to comment.