Skip to content

Commit

Permalink
feat(noForEach): add validIdentifiers option
Browse files Browse the repository at this point in the history
  • Loading branch information
lucasweng committed Nov 14, 2024
1 parent 8e8164b commit 2367015
Show file tree
Hide file tree
Showing 10 changed files with 234 additions and 4 deletions.
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,29 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

- Add [noGlobalDirnameFilename](https://biomejs.dev/linter/rules/no-global-dirname-filename/). Contributed by @unvalley

- [noForEach](https://biomejs.dev/linter/rules/no-for-each/) now provides a new option `validIdentifiers` ([#3351](https://github.com/biomejs/biome/issues/3351)) to specify which variable names are allowed to call `forEach`.

Identifiers containing dots (e.g., "lib._") or empty strings are not allowed. Invalid configurations will produce a diagnostic warning.

```json
{
"linter": {
"rules": {
"complexity": {
"noForEach": {
"level": "error",
"options": {
"validIdentifiers": ["Effect", "_"]
}
}
}
}
}
}
```

Contributed by @lucasweng

#### Enhancements

- `useExportType` and `useImportType` now ignore TypeScript declaration files ([#4416](https://github.com/biomejs/biome/pull/4416)). Contributed by @Conaclos
Expand Down
75 changes: 73 additions & 2 deletions crates/biome_js_analyze/src/lint/complexity/no_for_each.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ use biome_analyze::{
context::RuleContext, declare_lint_rule, Ast, Rule, RuleDiagnostic, RuleSource,
};
use biome_console::markup;
use biome_deserialize::{DeserializableValidator, DeserializationDiagnostic};
use biome_deserialize_macros::Deserializable;
use biome_js_syntax::{AnyJsExpression, AnyJsMemberExpression, JsCallExpression};
use biome_rowan::{AstNode, AstSeparatedList};
use biome_rowan::{AstNode, AstSeparatedList, TextRange};
use serde::{Deserialize, Serialize};

declare_lint_rule! {
/// Prefer `for...of` statement instead of `Array.forEach`.
Expand Down Expand Up @@ -60,6 +63,20 @@ declare_lint_rule! {
/// }
/// ```
///
/// ## Options
///
/// The rule provides a `validIdentifiers` option that allows specific variable names to call `forEach`.
/// In the following configuration, it's allowed to call `forEach` with expressions that match `Effect` or `_`:
///
/// ```json
/// {
/// "options": {
/// "validIdentifiers": ["Effect", "_"]
/// }
/// }
/// ```
///
/// Values with dots (e.g., "lib._") will not be accepted.
pub NoForEach {
version: "1.0.0",
name: "noForEach",
Expand All @@ -76,7 +93,7 @@ impl Rule for NoForEach {
type Query = Ast<JsCallExpression>;
type State = ();
type Signals = Option<Self::State>;
type Options = ();
type Options = NoForEachOptions;

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
Expand All @@ -85,6 +102,24 @@ impl Rule for NoForEach {
if member_expression.member_name()?.text() != "forEach" {
return None;
}

let options = ctx.options();
// Check if `forEach` is called by a valid identifier.
if !options.valid_identifiers.is_empty() {
let object = member_expression.object().ok()?;
if let Some(reference) = object.as_js_reference_identifier() {
let value_token = reference.value_token().ok()?;
let name = value_token.text_trimmed();
if options
.valid_identifiers
.iter()
.any(|identifier| identifier.as_ref() == name)
{
return None;
}
}
}

// Extract first parameter and ensure we have no more than 2 parameters.
let [Some(first), _, None] = node.arguments().ok()?.get_arguments_by_index([0, 1, 2])
else {
Expand Down Expand Up @@ -116,3 +151,39 @@ impl Rule for NoForEach {
}))
}
}

#[derive(Clone, Debug, Default, Deserialize, Deserializable, Eq, PartialEq, Serialize)]
#[deserializable(with_validator)]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[serde(rename_all = "camelCase", deny_unknown_fields, default)]
pub struct NoForEachOptions {
#[serde(skip_serializing_if = "<[_]>::is_empty")]
/// A list of variable names allowed for `forEach` calls.
pub valid_identifiers: Box<[Box<str>]>,
}

impl DeserializableValidator for NoForEachOptions {
fn validate(
&mut self,
_name: &str,
range: TextRange,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> bool {
if self
.valid_identifiers
.iter()
.any(|identifier| identifier.is_empty() || identifier.contains('.'))
{
diagnostics
.push(
DeserializationDiagnostic::new(markup!(
<Emphasis>"'validIdentifiers'"</Emphasis>" does not accept empty values or values with dots."
))
.with_range(range)
);
return false;
}

true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
lib._.forEach([1, 2], function (value) {
console.log(value);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: invalidConfig.js
---
# Input
```jsx
lib._.forEach([1, 2], function (value) {
console.log(value);
});

```

# Diagnostics
```
invalidConfig.options:8:17 deserialize ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× 'validIdentifiers' does not accept empty values or values with dots.
6 │ "noForEach": {
7 │ "level": "error",
> 8 │ "options": {
│ ^
> 9 │ "validIdentifiers": ["lib._"]
> 10 │ }
│ ^
11 │ }
12 │ }
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
"linter": {
"rules": {
"complexity": {
"noForEach": {
"level": "error",
"options": {
"validIdentifiers": ["lib._"]
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Effect.forEach([1, 2, 3, 4, 5], (n) =>
Console.log(`Current element: ${n}`).pipe(Effect.as(n * 2))
);

_.forEach([1, 2], function (value) {
console.log(value);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: validIdentifiers.js
---
# Input
```jsx
Effect.forEach([1, 2, 3, 4, 5], (n) =>
Console.log(`Current element: ${n}`).pipe(Effect.as(n * 2))
);

_.forEach([1, 2], function (value) {
console.log(value);
});

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
"linter": {
"rules": {
"complexity": {
"noForEach": {
"level": "error",
"options": {
"validIdentifiers": ["Effect", "_"]
}
}
}
}
}
}
21 changes: 20 additions & 1 deletion packages/@biomejs/backend-jsonrpc/src/workspace.ts

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

34 changes: 33 additions & 1 deletion 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 2367015

Please sign in to comment.