From 2367015320f3b2ac39d266226ee3825b52784a1a Mon Sep 17 00:00:00 2001 From: Lucas Weng Date: Mon, 11 Nov 2024 04:12:18 +0800 Subject: [PATCH] feat(noForEach): add `validIdentifiers` option --- CHANGELOG.md | 23 ++++++ .../src/lint/complexity/no_for_each.rs | 75 ++++++++++++++++++- .../complexity/noForEach/invalidConfig.js | 3 + .../noForEach/invalidConfig.js.snap | 30 ++++++++ .../noForEach/invalidConfig.options.json | 15 ++++ .../complexity/noForEach/validIdentifiers.js | 7 ++ .../noForEach/validIdentifiers.js.snap | 15 ++++ .../noForEach/validIdentifiers.options.json | 15 ++++ .../@biomejs/backend-jsonrpc/src/workspace.ts | 21 +++++- .../@biomejs/biome/configuration_schema.json | 34 ++++++++- 10 files changed, 234 insertions(+), 4 deletions(-) create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.js create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.js.snap create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.options.json create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.js create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.js.snap create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.options.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 6798bf61c065..c7c35438351f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/crates/biome_js_analyze/src/lint/complexity/no_for_each.rs b/crates/biome_js_analyze/src/lint/complexity/no_for_each.rs index 5a68f872ce93..5ccdce60a79c 100644 --- a/crates/biome_js_analyze/src/lint/complexity/no_for_each.rs +++ b/crates/biome_js_analyze/src/lint/complexity/no_for_each.rs @@ -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`. @@ -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", @@ -76,7 +93,7 @@ impl Rule for NoForEach { type Query = Ast; type State = (); type Signals = Option; - type Options = (); + type Options = NoForEachOptions; fn run(ctx: &RuleContext) -> Self::Signals { let node = ctx.query(); @@ -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 { @@ -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]>, +} + +impl DeserializableValidator for NoForEachOptions { + fn validate( + &mut self, + _name: &str, + range: TextRange, + diagnostics: &mut Vec, + ) -> bool { + if self + .valid_identifiers + .iter() + .any(|identifier| identifier.is_empty() || identifier.contains('.')) + { + diagnostics + .push( + DeserializationDiagnostic::new(markup!( + "'validIdentifiers'"" does not accept empty values or values with dots." + )) + .with_range(range) + ); + return false; + } + + true + } +} diff --git a/crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.js b/crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.js new file mode 100644 index 000000000000..8fb3903f7ead --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.js @@ -0,0 +1,3 @@ +lib._.forEach([1, 2], function (value) { + console.log(value); +}); diff --git a/crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.js.snap b/crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.js.snap new file mode 100644 index 000000000000..a1fab1c31f5c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.js.snap @@ -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 │ } + + +``` diff --git a/crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.options.json b/crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.options.json new file mode 100644 index 000000000000..96043d5c4c00 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noForEach/invalidConfig.options.json @@ -0,0 +1,15 @@ +{ + "$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json", + "linter": { + "rules": { + "complexity": { + "noForEach": { + "level": "error", + "options": { + "validIdentifiers": ["lib._"] + } + } + } + } + } +} diff --git a/crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.js b/crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.js new file mode 100644 index 000000000000..be20c42f08fa --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.js @@ -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); +}); diff --git a/crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.js.snap b/crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.js.snap new file mode 100644 index 000000000000..468cea1ea7fc --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.js.snap @@ -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); +}); + +``` diff --git a/crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.options.json b/crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.options.json new file mode 100644 index 000000000000..0f7b7ee552c7 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noForEach/validIdentifiers.options.json @@ -0,0 +1,15 @@ +{ + "$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json", + "linter": { + "rules": { + "complexity": { + "noForEach": { + "level": "error", + "options": { + "validIdentifiers": ["Effect", "_"] + } + } + } + } + } +} diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index c25fc70014d4..afa50e1ae00b 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -883,7 +883,7 @@ export interface Complexity { /** * Prefer for...of statement instead of Array.forEach. */ - noForEach?: RuleConfiguration_for_Null; + noForEach?: RuleConfiguration_for_NoForEachOptions; /** * Disallow unclear usage of consecutive space characters in regular expression literals */ @@ -2062,6 +2062,9 @@ export type RuleFixConfiguration_for_ValidAriaRoleOptions = export type RuleConfiguration_for_ComplexityOptions = | RulePlainConfiguration | RuleWithOptions_for_ComplexityOptions; +export type RuleConfiguration_for_NoForEachOptions = + | RulePlainConfiguration + | RuleWithOptions_for_NoForEachOptions; export type RuleConfiguration_for_NoUndeclaredDependenciesOptions = | RulePlainConfiguration | RuleWithOptions_for_NoUndeclaredDependenciesOptions; @@ -2213,6 +2216,16 @@ export interface RuleWithOptions_for_ComplexityOptions { */ options: ComplexityOptions; } +export interface RuleWithOptions_for_NoForEachOptions { + /** + * The severity of the emitted diagnostics by the rule + */ + level: RulePlainConfiguration; + /** + * Rule's options + */ + options: NoForEachOptions; +} export interface RuleWithOptions_for_NoUndeclaredDependenciesOptions { /** * The severity of the emitted diagnostics by the rule @@ -2476,6 +2489,12 @@ export interface ComplexityOptions { */ maxAllowedComplexity?: number; } +export interface NoForEachOptions { + /** + * A list of variable names allowed for `forEach` calls. + */ + validIdentifiers: string[]; +} /** * Rule's options */ diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index fcf9f1c1f679..0d66e48a5e8b 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -440,7 +440,7 @@ "noForEach": { "description": "Prefer for...of statement instead of Array.forEach.", "anyOf": [ - { "$ref": "#/definitions/RuleConfiguration" }, + { "$ref": "#/definitions/NoForEachConfiguration" }, { "type": "null" } ] }, @@ -2038,6 +2038,23 @@ }, "additionalProperties": false }, + "NoForEachConfiguration": { + "anyOf": [ + { "$ref": "#/definitions/RulePlainConfiguration" }, + { "$ref": "#/definitions/RuleWithNoForEachOptions" } + ] + }, + "NoForEachOptions": { + "type": "object", + "properties": { + "validIdentifiers": { + "description": "A list of variable names allowed for `forEach` calls.", + "type": "array", + "items": { "type": "string" } + } + }, + "additionalProperties": false + }, "NoLabelWithoutControlConfiguration": { "anyOf": [ { "$ref": "#/definitions/RulePlainConfiguration" }, @@ -3000,6 +3017,21 @@ }, "additionalProperties": false }, + "RuleWithNoForEachOptions": { + "type": "object", + "required": ["level"], + "properties": { + "level": { + "description": "The severity of the emitted diagnostics by the rule", + "allOf": [{ "$ref": "#/definitions/RulePlainConfiguration" }] + }, + "options": { + "description": "Rule's options", + "allOf": [{ "$ref": "#/definitions/NoForEachOptions" }] + } + }, + "additionalProperties": false + }, "RuleWithNoLabelWithoutControlOptions": { "type": "object", "required": ["level"],