Skip to content

Commit

Permalink
feat(noExportedImports): add lint rule
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos committed Jun 6, 2024
1 parent eb8de09 commit 83565f9
Show file tree
Hide file tree
Showing 20 changed files with 343 additions and 119 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

### Linter

#### New features

- Add [nursery/noExportedImports](https://biomejs.dev/linter/rules/no-exported-imports/). Contributed by @Conaclos

#### Bug fixes

- The `no-empty-block` css lint rule now treats empty blocks containing comments as valid ones. Contributed by @Sec-ant
Expand Down
161 changes: 90 additions & 71 deletions crates/biome_configuration/src/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 @@ -124,6 +124,7 @@ define_categories! {
"lint/nursery/noDuplicateSelectorsKeyframeBlock": "https://biomejs.dev/linter/rules/no-duplicate-selectors-keyframe-block",
"lint/nursery/noEmptyBlock": "https://biomejs.dev/linter/rules/no-empty-block",
"lint/nursery/noEvolvingTypes": "https://biomejs.dev/linter/rules/no-evolving-any",
"lint/nursery/noExportedImports": "https://biomejs.dev/linter/rules/no-exported-imports",
"lint/nursery/noImportantInKeyframe": "https://biomejs.dev/linter/rules/no-important-in-keyframe",
"lint/nursery/noInvalidPositionAtImportRule": "https://biomejs.dev/linter/rules/no-invalid-position-at-import-rule",
"lint/nursery/noLabelWithoutControl": "https://biomejs.dev/linter/rules/no-label-without-control",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::globals::is_node_builtin_module;
use biome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic, RuleSource};
use biome_console::markup;
use biome_js_syntax::{inner_string_text, AnyJsImportSpecifierLike};
use biome_js_syntax::{inner_string_text, AnyJsImportLike};
use biome_rowan::TextRange;

declare_rule! {
Expand Down Expand Up @@ -36,7 +36,7 @@ declare_rule! {
}

impl Rule for NoNodejsModules {
type Query = Ast<AnyJsImportSpecifierLike>;
type Query = Ast<AnyJsImportLike>;
type State = TextRange;
type Signals = Option<Self::State>;
type Options = ();
Expand Down
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery.rs

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

85 changes: 85 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery/no_exported_imports.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
use biome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
use biome_console::markup;
use biome_js_semantic::CanBeImportedExported;
use biome_js_syntax::AnyJsImportSpecifier;
use biome_rowan::AstNode;

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

declare_rule! {
/// Disallow exporting an imported variable.
///
/// In JavaScript, you can re-export a variable either by using `export from` or
/// by first importing the variable and then exporting it with a regular `export`.
///
/// You may prefer to use the first approach, as it clearly communicates the intention
/// to re-export an import, and can make static analysis easier.
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// import { A } from "mod";
/// export { A };
/// ```
///
/// ```js,expect_diagnostic
/// import * as ns from "mod";
/// export { ns };
/// ```
///
/// ```js,expect_diagnostic
/// import D from "mod";
/// export { D };
/// ```
///
/// ### Valid
///
/// ```js
/// export { A } from "mod";
/// export * as ns from "mod";
/// export { default as D } from "mod";
/// ```
///
pub NoExportedImports {
version: "next",
name: "noExportedImports",
language: "js",
recommended: false,
}
}

impl Rule for NoExportedImports {
type Query = Semantic<AnyJsImportSpecifier>;
type State = ();
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let specifier = ctx.query();
let local_name = specifier.local_name().ok()?;
let local_name = local_name.as_js_identifier_binding()?;
if local_name.is_exported(ctx.model()) {
Some(())
} else {
None
}
}

fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
let specifier = ctx.query();
Some(
RuleDiagnostic::new(
rule_category!(),
specifier.range(),
markup! {
"An import should not be exported. Use "<Emphasis>"export from"</Emphasis>"instead."
},
)
.note(markup! {
<Emphasis>"export from"</Emphasis>" makes it clearer that the intention is to re-export a variable."
}),
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use biome_analyze::context::RuleContext;
use biome_analyze::{declare_rule, Ast, Rule, RuleDiagnostic, RuleSource};
use biome_console::markup;
use biome_deserialize_macros::Deserializable;
use biome_js_syntax::{inner_string_text, AnyJsImportSpecifierLike};
use biome_js_syntax::{inner_string_text, AnyJsImportLike};
use biome_rowan::TextRange;
use rustc_hash::FxHashMap;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -47,7 +47,7 @@ pub struct RestrictedImportsOptions {
}

impl Rule for NoRestrictedImports {
type Query = Ast<AnyJsImportSpecifierLike>;
type Query = Ast<AnyJsImportLike>;
type State = (TextRange, String);
type Signals = Option<Self::State>;
type Options = Box<RestrictedImportsOptions>;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::services::manifest::Manifest;
use biome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
use biome_console::markup;
use biome_js_syntax::AnyJsImportSpecifierLike;
use biome_js_syntax::AnyJsImportLike;
use biome_rowan::AstNode;

declare_rule! {
Expand Down Expand Up @@ -38,7 +38,7 @@ declare_rule! {
}

impl Rule for NoUndeclaredDependencies {
type Query = Manifest<AnyJsImportSpecifierLike>;
type Query = Manifest<AnyJsImportLike>;
type State = ();
type Signals = Option<Self::State>;
type Options = ();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use biome_analyze::{
};
use biome_console::markup;
use biome_js_factory::make;
use biome_js_syntax::{inner_string_text, AnyJsImportSpecifierLike, JsLanguage};
use biome_js_syntax::{inner_string_text, AnyJsImportLike, JsLanguage};
use biome_rowan::{BatchMutationExt, SyntaxToken};

use crate::JsRuleAction;
Expand Down Expand Up @@ -80,7 +80,7 @@ declare_rule! {
}

impl Rule for UseImportExtensions {
type Query = Ast<AnyJsImportSpecifierLike>;
type Query = Ast<AnyJsImportLike>;
type State = UseImportExtensionsState;
type Signals = Option<Self::State>;
type Options = ();
Expand Down Expand Up @@ -142,7 +142,7 @@ pub struct UseImportExtensionsState {

fn get_extensionless_import(
file_ext: &str,
node: &AnyJsImportSpecifierLike,
node: &AnyJsImportLike,
) -> Option<UseImportExtensionsState> {
let module_name_token = node.module_name_token()?;
let module_path = inner_string_text(&module_name_token);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use biome_analyze::{
context::RuleContext, declare_rule, ActionCategory, Ast, FixKind, Rule, RuleDiagnostic,
};
use biome_console::markup;
use biome_js_syntax::{inner_string_text, AnyJsImportSpecifierLike, JsSyntaxKind, JsSyntaxToken};
use biome_js_syntax::{inner_string_text, AnyJsImportLike, JsSyntaxKind, JsSyntaxToken};
use biome_rowan::BatchMutationExt;

declare_rule! {
Expand Down Expand Up @@ -35,7 +35,7 @@ declare_rule! {
}

impl Rule for UseNodeAssertStrict {
type Query = Ast<AnyJsImportSpecifierLike>;
type Query = Ast<AnyJsImportLike>;
type State = JsSyntaxToken;
type Signals = Option<Self::State>;
type Options = ();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use biome_analyze::{
RuleSource,
};
use biome_console::markup;
use biome_js_syntax::{inner_string_text, AnyJsImportSpecifierLike, JsSyntaxKind, JsSyntaxToken};
use biome_js_syntax::{inner_string_text, AnyJsImportLike, JsSyntaxKind, JsSyntaxToken};
use biome_rowan::BatchMutationExt;

use crate::{globals::is_node_builtin_module, JsRuleAction};
Expand Down Expand Up @@ -51,7 +51,7 @@ declare_rule! {
}

impl Rule for UseNodejsImportProtocol {
type Query = Ast<AnyJsImportSpecifierLike>;
type Query = Ast<AnyJsImportLike>;
type State = JsSyntaxToken;
type Signals = Option<Self::State>;
type Options = ();
Expand Down
25 changes: 11 additions & 14 deletions crates/biome_js_analyze/src/lint/suspicious/no_import_assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@ use crate::services::semantic::Semantic;
use biome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic, RuleSource};
use biome_console::markup;
use biome_js_semantic::ReferencesExtensions;
use biome_js_syntax::{
JsDefaultImportSpecifier, JsIdentifierAssignment, JsIdentifierBinding, JsNamedImportSpecifier,
JsNamespaceImportSpecifier, JsShorthandNamedImportSpecifier,
};
use biome_js_syntax::{AnyJsImportSpecifier, JsIdentifierAssignment, JsIdentifierBinding};

use biome_rowan::{declare_node_union, AstNode};
use biome_rowan::AstNode;

declare_rule! {
/// Disallow assigning to imported bindings
Expand Down Expand Up @@ -59,7 +56,7 @@ declare_rule! {
}

impl Rule for NoImportAssign {
type Query = Semantic<AnyJsImportLike>;
type Query = Semantic<AnyJsImportSpecifier>;
/// The first element of the tuple is the invalid `JsIdentifierAssignment`, the second element of the tuple is the imported `JsIdentifierBinding`.
type State = (JsIdentifierAssignment, JsIdentifierBinding);
type Signals = Vec<Self::State>;
Expand All @@ -71,22 +68,26 @@ impl Rule for NoImportAssign {
let local_name_binding = match label_statement {
// `import {x as xx} from 'y'`
// ^^^^^^^
AnyJsImportLike::JsNamedImportSpecifier(specifier) => specifier.local_name().ok(),
AnyJsImportSpecifier::JsNamedImportSpecifier(specifier) => specifier.local_name().ok(),
// `import {x} from 'y'`
// ^
AnyJsImportLike::JsShorthandNamedImportSpecifier(specifier) => {
AnyJsImportSpecifier::JsShorthandNamedImportSpecifier(specifier) => {
specifier.local_name().ok()
}
// `import * as xxx from 'y'`
// ^^^^^^^^
// `import a, * as b from 'y'`
// ^^^^^^
AnyJsImportLike::JsNamespaceImportSpecifier(specifier) => specifier.local_name().ok(),
AnyJsImportSpecifier::JsNamespaceImportSpecifier(specifier) => {
specifier.local_name().ok()
}
// `import xx from 'y'`
// ^^
// `import a, * as b from 'y'`
// ^
AnyJsImportLike::JsDefaultImportSpecifier(specifier) => specifier.local_name().ok(),
AnyJsImportSpecifier::JsDefaultImportSpecifier(specifier) => {
specifier.local_name().ok()
}
};
local_name_binding
.and_then(|binding| {
Expand Down Expand Up @@ -125,7 +126,3 @@ impl Rule for NoImportAssign {
)
}
}

declare_node_union! {
pub AnyJsImportLike = JsNamedImportSpecifier | JsShorthandNamedImportSpecifier | JsNamespaceImportSpecifier | JsDefaultImportSpecifier
}
2 changes: 2 additions & 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 @@
import { A } from "mod";
export { A };

import * as ns from "mod";
export { ns };

import D from "mod";
export { D };
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: invalid.js
---
# Input
```jsx
import { A } from "mod";
export { A };

import * as ns from "mod";
export { ns };

import D from "mod";
export { D };
```

# Diagnostics
```
invalid.js:1:10 lint/nursery/noExportedImports ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! An import should not be exported. Use export frominstead.
> 1 │ import { A } from "mod";
│ ^
2 │ export { A };
3 │
i export from makes it clearer that the intention is to re-export a variable.
```

```
invalid.js:4:8 lint/nursery/noExportedImports ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! An import should not be exported. Use export frominstead.
2 │ export { A };
3 │
> 4 │ import * as ns from "mod";
│ ^^^^^^^
5 │ export { ns };
6 │
i export from makes it clearer that the intention is to re-export a variable.
```

```
invalid.js:7:8 lint/nursery/noExportedImports ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! An import should not be exported. Use export frominstead.
5 │ export { ns };
6 │
> 7 │ import D from "mod";
│ ^
8 │ export { D };
i export from makes it clearer that the intention is to re-export a variable.
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export { A } from "mod";
export * as ns from "mod";
export { default as D } from "mod";
Loading

0 comments on commit 83565f9

Please sign in to comment.