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): noDynamicNamespaceImportAccess #3241

Merged
merged 5 commits into from
Jul 4, 2024
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
#### New features

- Add [nursery/useValidAutocomplete](https://biomejs.dev/linter/rules/use-valid-autocomplete/). Contributed by @unvalley
- Add [nursery/noDynamicNamespaceImportAccess](https://biomejs.dev/linter/no-dynamic-namespace-import-access/). Contributed by @minht11

#### Bug fixes

Expand Down
206 changes: 113 additions & 93 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 @@ -122,6 +122,7 @@ define_categories! {
"lint/nursery/noDuplicateFontNames": "https://biomejs.dev/linter/rules/no-font-family-duplicate-names",
"lint/nursery/noDuplicateJsonKeys": "https://biomejs.dev/linter/rules/no-duplicate-json-keys",
"lint/nursery/noDuplicateSelectorsKeyframeBlock": "https://biomejs.dev/linter/rules/no-duplicate-selectors-keyframe-block",
"lint/nursery/noDynamicNamespaceImportAccess": "https://biomejs.dev/linter/rules/no-dynamic-namespace-import-access",
"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",
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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
use biome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
use biome_console::markup;
use biome_js_semantic::ReferencesExtensions;
use biome_js_syntax::{JsComputedMemberExpression, JsImportNamespaceClause};
use biome_rowan::{AstNode, TextRange};

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

declare_rule! {
/// Disallow accessing namespace imports dynamically.
///
/// Accessing namespace imports dynamically can prevent efficient tree shaking and increase bundle size.
/// This happens because the bundler cannot determine which parts of the namespace are used at compile time,
/// so it must include the entire namespace in the bundle.
///
/// Instead, consider using named imports or if that is not possible
/// access the namespaced import properties statically.
///
/// If you want to completely disallow namespace imports, consider using the [noNamespaceImport](https://biomejs.dev/linter/rules/no-namespace-import/) rule.
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// import * as foo from "foo"
/// foo["bar"]
/// ```
///
/// ```js,expect_diagnostic
/// import * as foo from "foo"
/// const key = "bar"
/// foo[key]
/// ```
///
/// ### Valid
///
/// ```js
/// import * as foo from "foo"
/// foo.bar
/// ```
///
/// ```js
/// import { bar } from "foo"
/// bar
/// ```
///
/// ```js
/// import messages from "i18n"
/// const knownMessagesMap = {
/// hello: messages.hello,
/// goodbye: messages.goodbye
/// }
///
/// const dynamicKey = "hello"
/// knownMessagesMap[dynamicKey]
/// ```
///
pub NoDynamicNamespaceImportAccess {
version: "next",
name: "noDynamicNamespaceImportAccess",
language: "js",
recommended: false,
}
}

impl Rule for NoDynamicNamespaceImportAccess {
type Query = Semantic<JsImportNamespaceClause>;
type State = TextRange;
type Signals = Vec<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
find_dynamic_namespace_import_accesses(ctx).map_or(vec![], |range| range)
}

fn diagnostic(_: &RuleContext<Self>, range: &Self::State) -> Option<RuleDiagnostic> {
let diagnostic = RuleDiagnostic::new(
rule_category!(),
range,
markup! {
"Avoid accessing namespace imports dynamically, it can prevent efficient tree shaking and increase bundle size."
},
)
.note(markup! {
"Prefer static property access or use named imports instead."
});

Some(diagnostic)
}
}

fn find_dynamic_namespace_import_accesses(
ctx: &RuleContext<NoDynamicNamespaceImportAccess>,
) -> Option<Vec<TextRange>> {
let import_namespace_clause: &JsImportNamespaceClause = ctx.query();

// Allow type import e.g. `import type * as foo from "foo"`
if import_namespace_clause.type_token().is_some() {
return None;
}

let specifier = import_namespace_clause.namespace_specifier().ok()?;
let any_binding = specifier.local_name().ok()?;
let identifier = any_binding.as_js_identifier_binding()?;
let reads = identifier.all_reads(ctx.model());

let ranges = reads
.into_iter()
.filter_map(|read| {
let syntax = read.syntax().parent()?.parent()?;
let node = JsComputedMemberExpression::cast(syntax)?;

Some(node.range())
})
.collect::<Vec<_>>();

Some(ranges)
}
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,6 @@
import * as foo from "foo"

foo["bar"]
foo[1]
const key = "bar"
foo[key]
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: invalid.js
---
# Input
```jsx
import * as foo from "foo"

foo["bar"]
foo[1]
const key = "bar"
foo[key]
```

# Diagnostics
```
invalid.js:3:1 lint/nursery/noDynamicNamespaceImportAccess ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid accessing namespace imports dynamically, it can prevent efficient tree shaking and increase bundle size.

1 │ import * as foo from "foo"
2 │
> 3 │ foo["bar"]
│ ^^^^^^^^^^
4 │ foo[1]
5 │ const key = "bar"

i Prefer static property access or use named imports instead.


```

```
invalid.js:4:1 lint/nursery/noDynamicNamespaceImportAccess ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid accessing namespace imports dynamically, it can prevent efficient tree shaking and increase bundle size.

3 │ foo["bar"]
> 4 │ foo[1]
│ ^^^^^^
5 │ const key = "bar"
6 │ foo[key]

i Prefer static property access or use named imports instead.


```

```
invalid.js:6:1 lint/nursery/noDynamicNamespaceImportAccess ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid accessing namespace imports dynamically, it can prevent efficient tree shaking and increase bundle size.

4 │ foo[1]
5 │ const key = "bar"
> 6 │ foo[key]
│ ^^^^^^^^

i Prefer static property access or use named imports instead.


```
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import * as foo from "foo"

foo.bar
const key = "bar"
foo[key] = "bar"
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: valid.js
---
# Input
```jsx
import * as foo from "foo"

foo.bar
const key = "bar"
foo[key] = "bar"
```
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.