From 530a61a86c1638aa94f9da2160a7bf5c59f8203d Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Tue, 24 Sep 2024 15:12:57 +0900 Subject: [PATCH 01/31] add rule noUselessLengthCheck --- .../migrate/eslint_any_rule_to_biome.rs | 10 + .../src/analyzer/linter/rules.rs | 78 ++- .../src/categories.rs | 1 + crates/biome_js_analyze/src/lint/nursery.rs | 2 + .../lint/nursery/no_useless_length_check.rs | 363 ++++++++++ crates/biome_js_analyze/src/options.rs | 2 + .../noUselessLengthCheck/invalid.jsonc | 27 + .../noUselessLengthCheck/invalid.jsonc.snap | 630 ++++++++++++++++++ .../nursery/noUselessLengthCheck/valid.jsonc | 34 + .../noUselessLengthCheck/valid.jsonc.snap | 164 +++++ .../@biomejs/backend-jsonrpc/src/workspace.ts | 5 + .../@biomejs/biome/configuration_schema.json | 7 + 12 files changed, 1294 insertions(+), 29 deletions(-) create mode 100644 crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc.snap diff --git a/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs b/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs index 9a39b95c073b..a8aabc788223 100644 --- a/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs +++ b/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs @@ -1485,6 +1485,16 @@ pub(crate) fn migrate_eslint_any_rule( let rule = group.no_then_property.get_or_insert(Default::default()); rule.set_level(rule_severity.into()); } + "unicorn/no-useless-length-check" => { + if !options.include_nursery { + return false; + } + let group = rules.nursery.get_or_insert_with(Default::default); + let rule = group + .no_useless_length_check + .get_or_insert(Default::default()); + rule.set_level(rule_severity.into()); + } "unicorn/no-useless-switch-case" => { let group = rules.complexity.get_or_insert_with(Default::default); let rule = group diff --git a/crates/biome_configuration/src/analyzer/linter/rules.rs b/crates/biome_configuration/src/analyzer/linter/rules.rs index b1f533029a55..286b36dc7f95 100644 --- a/crates/biome_configuration/src/analyzer/linter/rules.rs +++ b/crates/biome_configuration/src/analyzer/linter/rules.rs @@ -3341,6 +3341,10 @@ pub struct Nursery { #[serde(skip_serializing_if = "Option::is_none")] pub no_useless_escape_in_regex: Option>, + #[doc = "Disallow unnecessary length checks within logical expressions."] + #[serde(skip_serializing_if = "Option::is_none")] + pub no_useless_length_check: + Option>, #[doc = "Disallow use of @value rule in css modules."] #[serde(skip_serializing_if = "Option::is_none")] pub no_value_at_rule: Option>, @@ -3428,6 +3432,7 @@ impl Nursery { "noUnknownPseudoClass", "noUnknownPseudoElement", "noUselessEscapeInRegex", + "noUselessLengthCheck", "noValueAtRule", "useAdjacentOverloadSignatures", "useAriaPropsSupportedByRole", @@ -3463,10 +3468,10 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30]), ]; const ALL_RULES_AS_FILTERS: &'static [RuleFilter<'static>] = &[ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]), @@ -3501,6 +3506,7 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32]), ]; #[doc = r" Retrieves the recommended rules"] pub(crate) fn is_recommended_true(&self) -> bool { @@ -3612,71 +3618,76 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } - if let Some(rule) = self.no_value_at_rule.as_ref() { + if let Some(rule) = self.no_useless_length_check.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19])); } } - if let Some(rule) = self.use_adjacent_overload_signatures.as_ref() { + if let Some(rule) = self.no_value_at_rule.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20])); } } - if let Some(rule) = self.use_aria_props_supported_by_role.as_ref() { + if let Some(rule) = self.use_adjacent_overload_signatures.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21])); } } - if let Some(rule) = self.use_component_export_only_modules.as_ref() { + if let Some(rule) = self.use_aria_props_supported_by_role.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22])); } } - if let Some(rule) = self.use_consistent_curly_braces.as_ref() { + if let Some(rule) = self.use_component_export_only_modules.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23])); } } - if let Some(rule) = self.use_consistent_member_accessibility.as_ref() { + if let Some(rule) = self.use_consistent_curly_braces.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24])); } } - if let Some(rule) = self.use_deprecated_reason.as_ref() { + if let Some(rule) = self.use_consistent_member_accessibility.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25])); } } - if let Some(rule) = self.use_explicit_function_return_type.as_ref() { + if let Some(rule) = self.use_deprecated_reason.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26])); } } - if let Some(rule) = self.use_import_restrictions.as_ref() { + if let Some(rule) = self.use_explicit_function_return_type.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27])); } } - if let Some(rule) = self.use_sorted_classes.as_ref() { + if let Some(rule) = self.use_import_restrictions.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[28])); } } - if let Some(rule) = self.use_strict_mode.as_ref() { + if let Some(rule) = self.use_sorted_classes.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29])); } } - if let Some(rule) = self.use_trim_start_end.as_ref() { + if let Some(rule) = self.use_strict_mode.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30])); } } - if let Some(rule) = self.use_valid_autocomplete.as_ref() { + if let Some(rule) = self.use_trim_start_end.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31])); } } + if let Some(rule) = self.use_valid_autocomplete.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32])); + } + } index_set } pub(crate) fn get_disabled_rules(&self) -> FxHashSet> { @@ -3776,71 +3787,76 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } - if let Some(rule) = self.no_value_at_rule.as_ref() { + if let Some(rule) = self.no_useless_length_check.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19])); } } - if let Some(rule) = self.use_adjacent_overload_signatures.as_ref() { + if let Some(rule) = self.no_value_at_rule.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20])); } } - if let Some(rule) = self.use_aria_props_supported_by_role.as_ref() { + if let Some(rule) = self.use_adjacent_overload_signatures.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21])); } } - if let Some(rule) = self.use_component_export_only_modules.as_ref() { + if let Some(rule) = self.use_aria_props_supported_by_role.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22])); } } - if let Some(rule) = self.use_consistent_curly_braces.as_ref() { + if let Some(rule) = self.use_component_export_only_modules.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23])); } } - if let Some(rule) = self.use_consistent_member_accessibility.as_ref() { + if let Some(rule) = self.use_consistent_curly_braces.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24])); } } - if let Some(rule) = self.use_deprecated_reason.as_ref() { + if let Some(rule) = self.use_consistent_member_accessibility.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25])); } } - if let Some(rule) = self.use_explicit_function_return_type.as_ref() { + if let Some(rule) = self.use_deprecated_reason.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26])); } } - if let Some(rule) = self.use_import_restrictions.as_ref() { + if let Some(rule) = self.use_explicit_function_return_type.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27])); } } - if let Some(rule) = self.use_sorted_classes.as_ref() { + if let Some(rule) = self.use_import_restrictions.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[28])); } } - if let Some(rule) = self.use_strict_mode.as_ref() { + if let Some(rule) = self.use_sorted_classes.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29])); } } - if let Some(rule) = self.use_trim_start_end.as_ref() { + if let Some(rule) = self.use_strict_mode.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30])); } } - if let Some(rule) = self.use_valid_autocomplete.as_ref() { + if let Some(rule) = self.use_trim_start_end.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31])); } } + if let Some(rule) = self.use_valid_autocomplete.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32])); + } + } index_set } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] @@ -3953,6 +3969,10 @@ impl Nursery { .no_useless_escape_in_regex .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "noUselessLengthCheck" => self + .no_useless_length_check + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), "noValueAtRule" => self .no_value_at_rule .as_ref() diff --git a/crates/biome_diagnostics_categories/src/categories.rs b/crates/biome_diagnostics_categories/src/categories.rs index 718d5ac6a418..d818c4fcac08 100644 --- a/crates/biome_diagnostics_categories/src/categories.rs +++ b/crates/biome_diagnostics_categories/src/categories.rs @@ -172,6 +172,7 @@ define_categories! { "lint/nursery/noUnmatchableAnbSelector": "https://biomejs.dev/linter/rules/no-unmatchable-anb-selector", "lint/nursery/noUnusedFunctionParameters": "https://biomejs.dev/linter/rules/no-unused-function-parameters", "lint/nursery/noUselessEscapeInRegex": "https://biomejs.dev/linter/rules/no-useless-escape-in-regex", + "lint/nursery/noUselessLengthCheck": "https://biomejs.dev/linter/rules/no-useless-length-check", "lint/nursery/noValueAtRule": "https://biomejs.dev/linter/rules/no-value-at-rule", "lint/nursery/useAdjacentOverloadSignatures": "https://biomejs.dev/linter/rules/use-adjacent-overload-signatures", "lint/nursery/useAriaPropsSupportedByRole": "https://biomejs.dev/linter/rules/use-aria-props-supported-by-role", diff --git a/crates/biome_js_analyze/src/lint/nursery.rs b/crates/biome_js_analyze/src/lint/nursery.rs index e643f3d051ee..d6bdd0d3b5eb 100644 --- a/crates/biome_js_analyze/src/lint/nursery.rs +++ b/crates/biome_js_analyze/src/lint/nursery.rs @@ -16,6 +16,7 @@ pub mod no_secrets; pub mod no_static_element_interactions; pub mod no_substr; pub mod no_useless_escape_in_regex; +pub mod no_useless_length_check; pub mod use_adjacent_overload_signatures; pub mod use_aria_props_supported_by_role; pub mod use_component_export_only_modules; @@ -46,6 +47,7 @@ declare_lint_group! { self :: no_static_element_interactions :: NoStaticElementInteractions , self :: no_substr :: NoSubstr , self :: no_useless_escape_in_regex :: NoUselessEscapeInRegex , + self :: no_useless_length_check :: NoUselessLengthCheck , self :: use_adjacent_overload_signatures :: UseAdjacentOverloadSignatures , self :: use_aria_props_supported_by_role :: UseAriaPropsSupportedByRole , self :: use_component_export_only_modules :: UseComponentExportOnlyModules , diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs new file mode 100644 index 000000000000..ca63efe8daf5 --- /dev/null +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -0,0 +1,363 @@ +use std::collections::{HashMap, HashSet}; + +use biome_analyze::{ + context::RuleContext, declare_lint_rule, ActionCategory, Ast, FixKind, Rule, RuleDiagnostic, + RuleSource, +}; +use biome_console::markup; +use biome_js_syntax::{ + AnyJsExpression, AnyJsLiteralExpression, AnyJsMemberExpression, JsBinaryExpression, + JsLogicalExpression, JsParenthesizedExpression, JsSyntaxKind, T, +}; +use biome_rowan::{AstNode, BatchMutationExt, TextRange}; + +use crate::JsRuleAction; + +declare_lint_rule! { + /// Disallow unnecessary length checks within logical expressions. + /// + /// - `Array#some()` returns `false` for an empty array. There is no need to check if the array is not empty. + /// - `Array#every()` returns `true`` for an empty array. There is no need to check if the array is empty. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// if (array.length === 0 || array.every(Boolean)); + /// ``` + /// + /// ```js,expect_diagnostic + /// if (array.length !== 0 && array.some(Boolean)); + /// ``` + /// + /// ```js,expect_diagnostic + /// if (array.length > 0 && array.some(Boolean)); + /// ``` + /// + /// ```js,expect_diagnostic + /// const isAllTrulyOrEmpty = array.length === 0 || array.every(Boolean); + /// ``` + /// + /// ### Valid + /// + /// ```js + /// if (array.every(Boolean)); + /// ``` + /// + /// ```js + /// if (array.some(Boolean)); + /// ``` + /// + /// ```js + /// const isAllTrulyOrEmpty = array.every(Boolean); + /// ``` + /// + /// ```js + /// const isAllTrulyOrEmpty = array.every(Boolean); + /// ``` + /// + /// ```js + /// if (array.length === 0 || anotherCheck() || array.every(Boolean)); + /// ``` + /// + /// ```js + /// const isNonEmptyAllTrulyArray = array.length > 0 && array.every(Boolean); + /// ``` + /// + /// ```js + /// const isEmptyArrayOrAllTruly = array.length === 0 || array.some(Boolean); + /// ``` + /// + pub NoUselessLengthCheck { + version: "next", + name: "noUselessLengthCheck", + language: "js", + recommended: false, + sources: &[RuleSource::EslintUnicorn("no-useless-length-check")], + fix_kind: FixKind::Unsafe, + } +} + +#[derive(Clone, Debug)] +pub enum ErrorType { + UselessLengthCheckWithSome, + UselessLengthCheckWithEvery, +} + +fn is_logical_exp_child(node: &AnyJsExpression, operator: JsSyntaxKind) -> bool { + let Some(parent) = node.syntax().parent() else { + return false; + }; + // ( hoge ) + if let Some(parenthesized_exp_parent) = JsParenthesizedExpression::cast(parent.clone()) { + return is_logical_exp_child(&AnyJsExpression::from(parenthesized_exp_parent), operator); + } + let Some(parent) = JsLogicalExpression::cast(parent) else { + return false; + }; + let Some(ope) = parent.operator_token().ok() else { + return false; + }; + ope.kind() == operator +} + +fn comparing_length_exp( + binary_exp: &JsBinaryExpression, + expect_error: &ErrorType, +) -> Option { + let left = binary_exp.left().ok()?; + let operator = binary_exp.operator_token().ok()?.kind(); + let right = binary_exp.right().ok()?; + // Check only when the number appears on the right side according to the original rules. + // We assume that you have already complied with useExplicitLengthCheck + comparing_length(&left, operator, &right, expect_error) +} + +fn comparing_length( + compare_exp: &AnyJsExpression, + operator: JsSyntaxKind, + value_exp: &AnyJsExpression, + expect_error: &ErrorType, +) -> Option { + let AnyJsExpression::JsStaticMemberExpression(member_exp) = compare_exp else { + return None; + }; + let target = member_exp.object().ok()?; + let member = member_exp.member().ok()?; + if member.text() != "length" || member_exp.is_optional_chain() { + return None; + } + let AnyJsExpression::AnyJsLiteralExpression(AnyJsLiteralExpression::JsNumberLiteralExpression( + literal, + )) = value_exp + else { + return None; + }; + let number = literal.as_number()?.round() as i64; + // .length === 0 + if matches!(expect_error, ErrorType::UselessLengthCheckWithEvery) + && literal.to_string().trim() == "0" + && (operator == T![===] || operator == T![<]) + { + return Some(target); + } + // .length !== 0 + if matches!(expect_error, ErrorType::UselessLengthCheckWithSome) + && (literal.to_string().trim() == "0" && (operator == T![!==] || operator == T![>]) + || number > 0 && operator == T![===] + || literal.to_string().trim() == "1" && operator == T![>=]) + { + return Some(target); + } + None +} + +pub type Replacer = (JsLogicalExpression, AnyJsExpression, TextRange); + +fn search_logical_exp( + any_exp: &AnyJsExpression, + replacer: Option, + expect_error: &ErrorType, + comparing_zeros: &mut HashMap>, + array_tokens_used_api: &mut HashSet, +) -> Option<()> { + match any_exp { + // || or && + AnyJsExpression::JsLogicalExpression(logical_exp) => { + let operator = match expect_error { + ErrorType::UselessLengthCheckWithEvery => T![||], + ErrorType::UselessLengthCheckWithSome => T![&&], + }; + if logical_exp.operator_token().ok()?.kind() != operator { + return None; + }; + let left = logical_exp.left().ok()?; + let left_replacer = (logical_exp.clone(), logical_exp.right().ok()?, left.range()); + search_logical_exp( + &left, + Some(left_replacer), + expect_error, + comparing_zeros, + array_tokens_used_api, + )?; + let right = logical_exp.right().ok()?; + let right_replacer = (logical_exp.clone(), logical_exp.left().ok()?, right.range()); + search_logical_exp( + &right, + Some(right_replacer), + expect_error, + comparing_zeros, + array_tokens_used_api, + ) + } + // a === 0 ext. + AnyJsExpression::JsBinaryExpression(binary_exp) => { + let comparing_zero = comparing_length_exp(binary_exp, expect_error)?; + let AnyJsExpression::JsIdentifierExpression(array_token) = comparing_zero else { + return None; + }; + let key = array_token.text(); + if let Some(comparing_zero_list) = comparing_zeros.get_mut(&key) { + comparing_zero_list.push(replacer?); + } else { + comparing_zeros.insert(key, vec![replacer?]); + } + Some(()) + } + // .some() or .every() etc. + AnyJsExpression::JsCallExpression(task_exp) => { + if task_exp.is_optional_chain() { + return None; + } + let task_member_exp = + AnyJsMemberExpression::cast(task_exp.callee().ok()?.into_syntax())?; + let task_target = task_member_exp.object().ok()?; + let AnyJsExpression::JsIdentifierExpression(task_target_token) = task_target else { + return None; + }; + + let task_member_name_node = task_member_exp.member_name()?; + let task_member_name = task_member_name_node.text(); + match expect_error { + ErrorType::UselessLengthCheckWithEvery => { + if task_member_name == "every" { + array_tokens_used_api.insert(task_target_token.text()); + Some(()) + } else { + None + } + } + ErrorType::UselessLengthCheckWithSome => { + if task_member_name == "some" { + array_tokens_used_api.insert(task_target_token.text()); + Some(()) + } else { + None + } + } + } + } + // ( hoge ) + AnyJsExpression::JsParenthesizedExpression(parent_exp) => search_logical_exp( + &parent_exp.expression().ok()?, + replacer, + expect_error, + comparing_zeros, + array_tokens_used_api, + ), + // hoge + AnyJsExpression::JsIdentifierExpression(_) => Some(()), + _ => None, + } +} + +fn get_parenthesized_parent(exp: AnyJsExpression) -> AnyJsExpression { + let Some(parent) = exp.syntax().parent() else { + return exp; + }; + let Some(parent) = JsParenthesizedExpression::cast(parent) else { + return exp; + }; + get_parenthesized_parent(AnyJsExpression::from(parent)) +} + +fn get_parenthesized_child(exp: &AnyJsExpression) -> AnyJsExpression { + let AnyJsExpression::JsParenthesizedExpression(parenthesized) = exp else { + return exp.clone(); + }; + let Some(child) = parenthesized.expression().ok() else { + return exp.clone(); + }; + get_parenthesized_child(&child) +} + +impl Rule for NoUselessLengthCheck { + type Query = Ast; + type State = (ErrorType, Replacer); + type Signals = Vec; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let node = ctx.query(); + + let Some(operator) = node.operator_token().ok() else { + return Vec::new(); + }; + // node must not be a child of a logical expression + if is_logical_exp_child(&AnyJsExpression::from(node.clone()), operator.kind()) { + return Vec::new(); + } + + let mut fixable_list: Vec<(ErrorType, Replacer)> = Vec::new(); + + for err_type in [ + ErrorType::UselessLengthCheckWithEvery, + ErrorType::UselessLengthCheckWithSome, + ] { + let mut comparing_zeros: HashMap> = HashMap::new(); + let mut array_tokens_used_api: HashSet = HashSet::new(); + let search_result = search_logical_exp( + &AnyJsExpression::from(node.clone()), + None, + &err_type, + &mut comparing_zeros, + &mut array_tokens_used_api, + ); + if search_result.is_some() { + for array_token in array_tokens_used_api { + if let Some(replacers) = comparing_zeros.get(&array_token) { + for replacer in replacers { + fixable_list.push((err_type.clone(), replacer.clone())); + } + } + } + } + } + fixable_list + } + + fn diagnostic( + _ctx: &RuleContext, + (error_type, (_, _, error_range)): &Self::State, + ) -> Option { + Some( + RuleDiagnostic::new( + rule_category!(), + error_range, + markup! { + "This is a check for unnecessary length." + }, + ) + .note( + match error_type { + ErrorType::UselessLengthCheckWithEvery => markup! { + "The empty check is useless as `Array#every()` returns `true` for an empty array." + }, + ErrorType::UselessLengthCheckWithSome => markup! { + "The non-empty check is useless as `Array#some()` returns `false` for an empty array." + }, + } + ), + ) + } + + fn action( + ctx: &RuleContext, + (_error_type, (prev_node, next_node, _error_range)): &Self::State, + ) -> Option { + let mut mutation = ctx.root().begin(); + + mutation.replace_node( + get_parenthesized_parent(AnyJsExpression::from(prev_node.clone())), + get_parenthesized_child(next_node), + ); + + Some(JsRuleAction::new( + ActionCategory::QuickFix, + ctx.metadata().applicability(), + markup! { "Remove the length check" }.to_owned(), + mutation, + )) + } +} diff --git a/crates/biome_js_analyze/src/options.rs b/crates/biome_js_analyze/src/options.rs index c1d4ab6d04fb..da4eb8e0e073 100644 --- a/crates/biome_js_analyze/src/options.rs +++ b/crates/biome_js_analyze/src/options.rs @@ -243,6 +243,8 @@ pub type NoUselessFragments = ::Options; pub type NoUselessLabel = ::Options; +pub type NoUselessLengthCheck = + ::Options; pub type NoUselessLoneBlockStatements = < lint :: complexity :: no_useless_lone_block_statements :: NoUselessLoneBlockStatements as biome_analyze :: Rule > :: Options ; pub type NoUselessRename = ::Options; diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc new file mode 100644 index 000000000000..605840193da5 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc @@ -0,0 +1,27 @@ +[ + "if (array.length === 0 || array.every(Boolean));", + "if (array.length !== 0 && array.some(Boolean));", + "if (array.length > 0 && array.some(Boolean));", + "const isAllTrulyOrEmpty = array.length === 0 || array.every(Boolean);", + "array.length === 0 || array.every(Boolean)", + "array.length > 0 && array.some(Boolean)", + "array.length !== 0 && array.some(Boolean)", + "if ((( array.length > 0 )) && array.some(Boolean));", + "(array.length === 0 || array.every(Boolean)) || foo", + "foo || (array.length === 0 || array.every(Boolean))", + "(array.length > 0 && array.some(Boolean)) && foo", + "foo && (array.length > 0 && array.some(Boolean))", + "array.every(Boolean) || array.length === 0", + "array.some(Boolean) && array.length !== 0", + "array.some(Boolean) && array.length > 0", + "foo && array.length > 0 && array.some(Boolean)", + "foo || array.length === 0 || array.every(Boolean)", + "(foo || array.length === 0) || array.every(Boolean)", + "array.length === 0 || (array.every(Boolean) || foo)", + "(foo && array.length > 0) && array.some(Boolean)", + "array.length > 0 && (array.some(Boolean) && foo)", + "array.every(Boolean) || array.length === 0 || array.every(Boolean)", + "array.length === 0 || array.every(Boolean) || array.length === 0", + "(array1.length === 0 || array1.every(Boolean)) || (array2.length === 0 || array2.every(Boolean))", + "(array1.length === 0 || array1.every(Boolean)) && (array2.length === 0 || array2.every(Boolean))" +] \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap new file mode 100644 index 000000000000..3aa2e1c7e628 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap @@ -0,0 +1,630 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +assertion_line: 86 +expression: invalid.jsonc +--- +# Input +```cjs +if (array.length === 0 || array.every(Boolean)); +``` + +# Diagnostics +``` +invalid.jsonc:1:5 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ if (array.length === 0 || array.every(Boolean)); + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ if·(array.length·===·0·||·array.every(Boolean)); + │ ---------------------- + +``` + +# Input +```cjs +if (array.length !== 0 && array.some(Boolean)); +``` + +# Diagnostics +``` +invalid.jsonc:1:5 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ if (array.length !== 0 && array.some(Boolean)); + │ ^^^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ if·(array.length·!==·0·&&·array.some(Boolean)); + │ ---------------------- + +``` + +# Input +```cjs +if (array.length > 0 && array.some(Boolean)); +``` + +# Diagnostics +``` +invalid.jsonc:1:5 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ if (array.length > 0 && array.some(Boolean)); + │ ^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ if·(array.length·>·0·&&·array.some(Boolean)); + │ -------------------- + +``` + +# Input +```cjs +const isAllTrulyOrEmpty = array.length === 0 || array.every(Boolean); +``` + +# Diagnostics +``` +invalid.jsonc:1:27 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ const isAllTrulyOrEmpty = array.length === 0 || array.every(Boolean); + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ const·isAllTrulyOrEmpty·=·array.length·===·0·||·array.every(Boolean); + │ ---------------------- + +``` + +# Input +```cjs +array.length === 0 || array.every(Boolean) +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.length === 0 || array.every(Boolean) + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.length·===·0·||·array.every(Boolean) + │ ---------------------- + +``` + +# Input +```cjs +array.length > 0 && array.some(Boolean) +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.length > 0 && array.some(Boolean) + │ ^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.length·>·0·&&·array.some(Boolean) + │ -------------------- + +``` + +# Input +```cjs +array.length !== 0 && array.some(Boolean) +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.length !== 0 && array.some(Boolean) + │ ^^^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.length·!==·0·&&·array.some(Boolean) + │ ---------------------- + +``` + +# Input +```cjs +if ((( array.length > 0 )) && array.some(Boolean)); +``` + +# Diagnostics +``` +invalid.jsonc:1:5 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ if ((( array.length > 0 )) && array.some(Boolean)); + │ ^^^^^^^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ if·(((·array.length·>·0·))·&&·array.some(Boolean)); + │ -------------------------- + +``` + +# Input +```cjs +(array.length === 0 || array.every(Boolean)) || foo +``` + +# Diagnostics +``` +invalid.jsonc:1:2 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ (array.length === 0 || array.every(Boolean)) || foo + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ (array.length·===·0·||·array.every(Boolean))·||·foo + │ ----------------------- - + +``` + +# Input +```cjs +foo || (array.length === 0 || array.every(Boolean)) +``` + +# Diagnostics +``` +invalid.jsonc:1:9 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ foo || (array.length === 0 || array.every(Boolean)) + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ foo·||·(array.length·===·0·||·array.every(Boolean)) + │ ----------------------- - + +``` + +# Input +```cjs +(array.length > 0 && array.some(Boolean)) && foo +``` + +# Diagnostics +``` +invalid.jsonc:1:2 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ (array.length > 0 && array.some(Boolean)) && foo + │ ^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ (array.length·>·0·&&·array.some(Boolean))·&&·foo + │ --------------------- - + +``` + +# Input +```cjs +foo && (array.length > 0 && array.some(Boolean)) +``` + +# Diagnostics +``` +invalid.jsonc:1:9 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ foo && (array.length > 0 && array.some(Boolean)) + │ ^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ foo·&&·(array.length·>·0·&&·array.some(Boolean)) + │ --------------------- - + +``` + +# Input +```cjs +array.every(Boolean) || array.length === 0 +``` + +# Diagnostics +``` +invalid.jsonc:1:25 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.every(Boolean) || array.length === 0 + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.every(Boolean)·||·array.length·===·0 + │ ---------------------- + +``` + +# Input +```cjs +array.some(Boolean) && array.length !== 0 +``` + +# Diagnostics +``` +invalid.jsonc:1:24 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.some(Boolean) && array.length !== 0 + │ ^^^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.some(Boolean)·&&·array.length·!==·0 + │ ---------------------- + +``` + +# Input +```cjs +array.some(Boolean) && array.length > 0 +``` + +# Diagnostics +``` +invalid.jsonc:1:24 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.some(Boolean) && array.length > 0 + │ ^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.some(Boolean)·&&·array.length·>·0 + │ -------------------- + +``` + +# Input +```cjs +foo && array.length > 0 && array.some(Boolean) +``` + +# Diagnostics +``` +invalid.jsonc:1:8 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ foo && array.length > 0 && array.some(Boolean) + │ ^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ foo·&&·array.length·>·0·&&·array.some(Boolean) + │ -------------------- + +``` + +# Input +```cjs +foo || array.length === 0 || array.every(Boolean) +``` + +# Diagnostics +``` +invalid.jsonc:1:8 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ foo || array.length === 0 || array.every(Boolean) + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ foo·||·array.length·===·0·||·array.every(Boolean) + │ ---------------------- + +``` + +# Input +```cjs +(foo || array.length === 0) || array.every(Boolean) +``` + +# Diagnostics +``` +invalid.jsonc:1:9 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ (foo || array.length === 0) || array.every(Boolean) + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ (foo·||·array.length·===·0)·||·array.every(Boolean) + │ - ----------------------- + +``` + +# Input +```cjs +array.length === 0 || (array.every(Boolean) || foo) +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.length === 0 || (array.every(Boolean) || foo) + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.length·===·0·||·(array.every(Boolean)·||·foo) + │ ----------------------- - + +``` + +# Input +```cjs +(foo && array.length > 0) && array.some(Boolean) +``` + +# Diagnostics +``` +invalid.jsonc:1:9 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ (foo && array.length > 0) && array.some(Boolean) + │ ^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ (foo·&&·array.length·>·0)·&&·array.some(Boolean) + │ - --------------------- + +``` + +# Input +```cjs +array.length > 0 && (array.some(Boolean) && foo) +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.length > 0 && (array.some(Boolean) && foo) + │ ^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.length·>·0·&&·(array.some(Boolean)·&&·foo) + │ --------------------- - + +``` + +# Input +```cjs +array.every(Boolean) || array.length === 0 || array.every(Boolean) +``` + +# Diagnostics +``` +invalid.jsonc:1:25 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.every(Boolean) || array.length === 0 || array.every(Boolean) + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.every(Boolean)·||·array.length·===·0·||·array.every(Boolean) + │ ---------------------- + +``` + +# Input +```cjs +array.length === 0 || array.every(Boolean) || array.length === 0 +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.length === 0 || array.every(Boolean) || array.length === 0 + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.length·===·0·||·array.every(Boolean)·||·array.length·===·0 + │ ---------------------- + +``` + +``` +invalid.jsonc:1:47 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ array.length === 0 || array.every(Boolean) || array.length === 0 + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.length·===·0·||·array.every(Boolean)·||·array.length·===·0 + │ ---------------------- + +``` + +# Input +```cjs +(array1.length === 0 || array1.every(Boolean)) || (array2.length === 0 || array2.every(Boolean)) +``` + +# Diagnostics +``` +invalid.jsonc:1:2 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ (array1.length === 0 || array1.every(Boolean)) || (array2.length === 0 || array2.every(Boolean)) + │ ^^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ (array1.length·===·0·||·array1.every(Boolean))·||·(array2.length·===·0·||·array2.every(Boolean)) + │ - ----------------------- - + +``` + +``` +invalid.jsonc:1:52 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ (array1.length === 0 || array1.every(Boolean)) || (array2.length === 0 || array2.every(Boolean)) + │ ^^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ (array1.length·===·0·||·array1.every(Boolean))·||·(array2.length·===·0·||·array2.every(Boolean)) + │ - ----------------------- - + +``` + +# Input +```cjs +(array1.length === 0 || array1.every(Boolean)) && (array2.length === 0 || array2.every(Boolean)) +``` + +# Diagnostics +``` +invalid.jsonc:1:2 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ (array1.length === 0 || array1.every(Boolean)) && (array2.length === 0 || array2.every(Boolean)) + │ ^^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ (array1.length·===·0·||·array1.every(Boolean))·&&·(array2.length·===·0·||·array2.every(Boolean)) + │ - ----------------------- - + +``` + +``` +invalid.jsonc:1:52 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This is a check for unnecessary length. + + > 1 │ (array1.length === 0 || array1.every(Boolean)) && (array2.length === 0 || array2.every(Boolean)) + │ ^^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ (array1.length·===·0·||·array1.every(Boolean))·&&·(array2.length·===·0·||·array2.every(Boolean)) + │ - ----------------------- - + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc new file mode 100644 index 000000000000..547964b5d6f6 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc @@ -0,0 +1,34 @@ +[ + "if (array.every(Boolean));", + "if (array.some(Boolean));", + "const isAllTrulyOrEmpty = array.every(Boolean);", + "if (array.length === 0 || anotherCheck() || array.every(Boolean));", + "const isNonEmptyAllTrulyArray = array.length > 0 && array.every(Boolean);", + "const isEmptyArrayOrAllTruly = array.length === 0 || array.some(Boolean);", + "array.length === 0 ?? array.every(Boolean)", + "array.length === 0 && array.every(Boolean)", + "(array.length === 0) + (array.every(Boolean))", + "array.length === 1 || array.every(Boolean)", + "array.length === '0' || array.every(Boolean)", + "array.length === 0. || array.every(Boolean)", + "array.length === 0x0 || array.every(Boolean)", + "array.length !== 0 || array.every(Boolean)", + "array.length == 0 || array.every(Boolean)", + "0 === array.length || array.every(Boolean)", + "array?.length === 0 || array.every(Boolean)", + "array.notLength === 0 || array.every(Boolean)", + "array[length] === 0 || array.every(Boolean)", + "array.length === 0 || array.every?.(Boolean)", + "array.length === 0 || array?.every(Boolean)", + "array.length === 0 || array.every", + "array.length === 0 || array[every](Boolean)", + "array1.length === 0 || array2.every(Boolean)", + "(foo && array.length === 0) || array.every(Boolean) && foo", + "array.length === 0 || (array.every(Boolean) && foo)", + "(foo || array.length > 0) && array.some(Boolean)", + "array.length > 0 && (array.some(Boolean) || foo)", + "array.length > 0 && array.some(Boolean) && array.other(Boolean)", + "array.length > 0 && array.some(Boolean) && array.every(Boolean)", + "array.length === 0", + "array.every(Boolean)" +] \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc.snap b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc.snap new file mode 100644 index 000000000000..c6431dc41ceb --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc.snap @@ -0,0 +1,164 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +assertion_line: 86 +expression: valid.jsonc +--- +# Input +```cjs +if (array.every(Boolean)); +``` + +# Input +```cjs +if (array.some(Boolean)); +``` + +# Input +```cjs +const isAllTrulyOrEmpty = array.every(Boolean); +``` + +# Input +```cjs +if (array.length === 0 || anotherCheck() || array.every(Boolean)); +``` + +# Input +```cjs +const isNonEmptyAllTrulyArray = array.length > 0 && array.every(Boolean); +``` + +# Input +```cjs +const isEmptyArrayOrAllTruly = array.length === 0 || array.some(Boolean); +``` + +# Input +```cjs +array.length === 0 ?? array.every(Boolean) +``` + +# Input +```cjs +array.length === 0 && array.every(Boolean) +``` + +# Input +```cjs +(array.length === 0) + (array.every(Boolean)) +``` + +# Input +```cjs +array.length === 1 || array.every(Boolean) +``` + +# Input +```cjs +array.length === '0' || array.every(Boolean) +``` + +# Input +```cjs +array.length === 0. || array.every(Boolean) +``` + +# Input +```cjs +array.length === 0x0 || array.every(Boolean) +``` + +# Input +```cjs +array.length !== 0 || array.every(Boolean) +``` + +# Input +```cjs +array.length == 0 || array.every(Boolean) +``` + +# Input +```cjs +0 === array.length || array.every(Boolean) +``` + +# Input +```cjs +array?.length === 0 || array.every(Boolean) +``` + +# Input +```cjs +array.notLength === 0 || array.every(Boolean) +``` + +# Input +```cjs +array[length] === 0 || array.every(Boolean) +``` + +# Input +```cjs +array.length === 0 || array.every?.(Boolean) +``` + +# Input +```cjs +array.length === 0 || array?.every(Boolean) +``` + +# Input +```cjs +array.length === 0 || array.every +``` + +# Input +```cjs +array.length === 0 || array[every](Boolean) +``` + +# Input +```cjs +array1.length === 0 || array2.every(Boolean) +``` + +# Input +```cjs +(foo && array.length === 0) || array.every(Boolean) && foo +``` + +# Input +```cjs +array.length === 0 || (array.every(Boolean) && foo) +``` + +# Input +```cjs +(foo || array.length > 0) && array.some(Boolean) +``` + +# Input +```cjs +array.length > 0 && (array.some(Boolean) || foo) +``` + +# Input +```cjs +array.length > 0 && array.some(Boolean) && array.other(Boolean) +``` + +# Input +```cjs +array.length > 0 && array.some(Boolean) && array.every(Boolean) +``` + +# Input +```cjs +array.length === 0 +``` + +# Input +```cjs +array.every(Boolean) +``` diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index 88cad0f8c634..f837790cd713 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -1294,6 +1294,10 @@ export interface Nursery { * Disallow unnecessary escape sequence in regular expression literals. */ noUselessEscapeInRegex?: RuleFixConfiguration_for_Null; + /** + * Disallow unnecessary length checks within logical expressions. + */ + noUselessLengthCheck?: RuleFixConfiguration_for_Null; /** * Disallow use of @value rule in css modules. */ @@ -2856,6 +2860,7 @@ export type Category = | "lint/nursery/noUnmatchableAnbSelector" | "lint/nursery/noUnusedFunctionParameters" | "lint/nursery/noUselessEscapeInRegex" + | "lint/nursery/noUselessLengthCheck" | "lint/nursery/noValueAtRule" | "lint/nursery/useAdjacentOverloadSignatures" | "lint/nursery/useAriaPropsSupportedByRole" diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index 7c01d4457c7d..85226c348e71 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -2209,6 +2209,13 @@ { "type": "null" } ] }, + "noUselessLengthCheck": { + "description": "Disallow unnecessary length checks within logical expressions.", + "anyOf": [ + { "$ref": "#/definitions/RuleFixConfiguration" }, + { "type": "null" } + ] + }, "noValueAtRule": { "description": "Disallow use of @value rule in css modules.", "anyOf": [ From 38a4d7fa38d0cde4199d14bd5373c579e6f37024 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Fri, 27 Sep 2024 05:09:57 +0900 Subject: [PATCH 02/31] fix: remove the recursive function --- .../lint/nursery/no_useless_length_check.rs | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index ca63efe8daf5..35bb5c8976d0 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -85,41 +85,40 @@ pub enum ErrorType { UselessLengthCheckWithEvery, } -fn is_logical_exp_child(node: &AnyJsExpression, operator: JsSyntaxKind) -> bool { +// Whether the node is a descendant of a logical expression. +fn is_logical_exp_descendant(node: &AnyJsExpression, operator: JsSyntaxKind) -> bool { let Some(parent) = node.syntax().parent() else { return false; }; - // ( hoge ) - if let Some(parenthesized_exp_parent) = JsParenthesizedExpression::cast(parent.clone()) { - return is_logical_exp_child(&AnyJsExpression::from(parenthesized_exp_parent), operator); - } - let Some(parent) = JsLogicalExpression::cast(parent) else { - return false; - }; - let Some(ope) = parent.operator_token().ok() else { - return false; - }; - ope.kind() == operator + parent + .ancestors() + .find_map(|ancestor| { + if let Some(logical_exp) = JsLogicalExpression::cast(ancestor.clone()) { + return logical_exp + .operator_token() + .ok() + .map(|token| token.kind() == operator) + .or(Some(false)); + } + (!JsParenthesizedExpression::can_cast(ancestor.kind())).then_some(false) + }) + .unwrap_or(false) } -fn comparing_length_exp( +// Extract the expressions that perform length comparisons corresponding to the errors you want to check. +fn get_comparing_length_exp( binary_exp: &JsBinaryExpression, expect_error: &ErrorType, ) -> Option { let left = binary_exp.left().ok()?; let operator = binary_exp.operator_token().ok()?.kind(); let right = binary_exp.right().ok()?; + // Check only when the number appears on the right side according to the original rules. // We assume that you have already complied with useExplicitLengthCheck - comparing_length(&left, operator, &right, expect_error) -} + let compare_exp = left; + let value_exp = right; -fn comparing_length( - compare_exp: &AnyJsExpression, - operator: JsSyntaxKind, - value_exp: &AnyJsExpression, - expect_error: &ErrorType, -) -> Option { let AnyJsExpression::JsStaticMemberExpression(member_exp) = compare_exp else { return None; }; @@ -181,6 +180,7 @@ fn search_logical_exp( comparing_zeros, array_tokens_used_api, )?; + let right = logical_exp.right().ok()?; let right_replacer = (logical_exp.clone(), logical_exp.left().ok()?, right.range()); search_logical_exp( @@ -193,7 +193,7 @@ fn search_logical_exp( } // a === 0 ext. AnyJsExpression::JsBinaryExpression(binary_exp) => { - let comparing_zero = comparing_length_exp(binary_exp, expect_error)?; + let comparing_zero = get_comparing_length_exp(binary_exp, expect_error)?; let AnyJsExpression::JsIdentifierExpression(array_token) = comparing_zero else { return None; }; @@ -285,7 +285,7 @@ impl Rule for NoUselessLengthCheck { return Vec::new(); }; // node must not be a child of a logical expression - if is_logical_exp_child(&AnyJsExpression::from(node.clone()), operator.kind()) { + if is_logical_exp_descendant(&AnyJsExpression::from(node.clone()), operator.kind()) { return Vec::new(); } From e400b00306009459c1e2c7eeda8223a1fd1a0cdf Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Tue, 15 Oct 2024 21:31:53 +0900 Subject: [PATCH 03/31] Update crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs Co-authored-by: Carson McManus --- .../src/lint/nursery/no_useless_length_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 35bb5c8976d0..f624ace3da17 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -85,7 +85,7 @@ pub enum ErrorType { UselessLengthCheckWithEvery, } -// Whether the node is a descendant of a logical expression. +/// Whether the node is a descendant of a logical expression. fn is_logical_exp_descendant(node: &AnyJsExpression, operator: JsSyntaxKind) -> bool { let Some(parent) = node.syntax().parent() else { return false; From 292607fd33406667b0d9f83a284caf0b2c1236ba Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Tue, 15 Oct 2024 21:32:02 +0900 Subject: [PATCH 04/31] Update crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs Co-authored-by: Carson McManus --- .../src/lint/nursery/no_useless_length_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index f624ace3da17..870660f555e3 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -105,7 +105,7 @@ fn is_logical_exp_descendant(node: &AnyJsExpression, operator: JsSyntaxKind) -> .unwrap_or(false) } -// Extract the expressions that perform length comparisons corresponding to the errors you want to check. +/// Extract the expressions that perform length comparisons corresponding to the errors you want to check. fn get_comparing_length_exp( binary_exp: &JsBinaryExpression, expect_error: &ErrorType, From 1bbcf839a47d894c71a146dcd0de2688296881ca Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Tue, 15 Oct 2024 21:51:02 +0900 Subject: [PATCH 05/31] doc: add doc comment --- .../biome_js_analyze/src/lint/nursery/no_useless_length_check.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 870660f555e3..6b93635e246c 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -154,6 +154,7 @@ fn get_comparing_length_exp( pub type Replacer = (JsLogicalExpression, AnyJsExpression, TextRange); +/// Search for logical expressions and list expressions that compare to 0 and Array APIs (`.some()`, `.every()`). fn search_logical_exp( any_exp: &AnyJsExpression, replacer: Option, From 6c56b42664e537d5bccea5f09ea7598b5c44bb81 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Fri, 25 Oct 2024 13:25:27 +0900 Subject: [PATCH 06/31] Update crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs Co-authored-by: Carson McManus --- .../src/lint/nursery/no_useless_length_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 6b93635e246c..65c74d546405 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -327,7 +327,7 @@ impl Rule for NoUselessLengthCheck { rule_category!(), error_range, markup! { - "This is a check for unnecessary length." + "This length check is unnecessary." }, ) .note( From 9b00a825fae277d07bf48d8301cd367e9cbc8503 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Fri, 25 Oct 2024 13:36:25 +0900 Subject: [PATCH 07/31] fix: test message --- .../noUselessLengthCheck/invalid.jsonc.snap | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap index 3aa2e1c7e628..f9fef2825ca9 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap @@ -12,7 +12,7 @@ if (array.length === 0 || array.every(Boolean)); ``` invalid.jsonc:1:5 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ if (array.length === 0 || array.every(Boolean)); │ ^^^^^^^^^^^^^^^^^^ @@ -35,7 +35,7 @@ if (array.length !== 0 && array.some(Boolean)); ``` invalid.jsonc:1:5 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ if (array.length !== 0 && array.some(Boolean)); │ ^^^^^^^^^^^^^^^^^^ @@ -58,7 +58,7 @@ if (array.length > 0 && array.some(Boolean)); ``` invalid.jsonc:1:5 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ if (array.length > 0 && array.some(Boolean)); │ ^^^^^^^^^^^^^^^^ @@ -81,7 +81,7 @@ const isAllTrulyOrEmpty = array.length === 0 || array.every(Boolean); ``` invalid.jsonc:1:27 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ const isAllTrulyOrEmpty = array.length === 0 || array.every(Boolean); │ ^^^^^^^^^^^^^^^^^^ @@ -104,7 +104,7 @@ array.length === 0 || array.every(Boolean) ``` invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.length === 0 || array.every(Boolean) │ ^^^^^^^^^^^^^^^^^^ @@ -127,7 +127,7 @@ array.length > 0 && array.some(Boolean) ``` invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.length > 0 && array.some(Boolean) │ ^^^^^^^^^^^^^^^^ @@ -150,7 +150,7 @@ array.length !== 0 && array.some(Boolean) ``` invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.length !== 0 && array.some(Boolean) │ ^^^^^^^^^^^^^^^^^^ @@ -173,7 +173,7 @@ if ((( array.length > 0 )) && array.some(Boolean)); ``` invalid.jsonc:1:5 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ if ((( array.length > 0 )) && array.some(Boolean)); │ ^^^^^^^^^^^^^^^^^^^^^^ @@ -196,7 +196,7 @@ invalid.jsonc:1:5 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━ ``` invalid.jsonc:1:2 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ (array.length === 0 || array.every(Boolean)) || foo │ ^^^^^^^^^^^^^^^^^^ @@ -219,7 +219,7 @@ foo || (array.length === 0 || array.every(Boolean)) ``` invalid.jsonc:1:9 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ foo || (array.length === 0 || array.every(Boolean)) │ ^^^^^^^^^^^^^^^^^^ @@ -242,7 +242,7 @@ invalid.jsonc:1:9 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━ ``` invalid.jsonc:1:2 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ (array.length > 0 && array.some(Boolean)) && foo │ ^^^^^^^^^^^^^^^^ @@ -265,7 +265,7 @@ foo && (array.length > 0 && array.some(Boolean)) ``` invalid.jsonc:1:9 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ foo && (array.length > 0 && array.some(Boolean)) │ ^^^^^^^^^^^^^^^^ @@ -288,7 +288,7 @@ array.every(Boolean) || array.length === 0 ``` invalid.jsonc:1:25 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.every(Boolean) || array.length === 0 │ ^^^^^^^^^^^^^^^^^^ @@ -311,7 +311,7 @@ array.some(Boolean) && array.length !== 0 ``` invalid.jsonc:1:24 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.some(Boolean) && array.length !== 0 │ ^^^^^^^^^^^^^^^^^^ @@ -334,7 +334,7 @@ array.some(Boolean) && array.length > 0 ``` invalid.jsonc:1:24 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.some(Boolean) && array.length > 0 │ ^^^^^^^^^^^^^^^^ @@ -357,7 +357,7 @@ foo && array.length > 0 && array.some(Boolean) ``` invalid.jsonc:1:8 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ foo && array.length > 0 && array.some(Boolean) │ ^^^^^^^^^^^^^^^^ @@ -380,7 +380,7 @@ foo || array.length === 0 || array.every(Boolean) ``` invalid.jsonc:1:8 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ foo || array.length === 0 || array.every(Boolean) │ ^^^^^^^^^^^^^^^^^^ @@ -403,7 +403,7 @@ invalid.jsonc:1:8 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━ ``` invalid.jsonc:1:9 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ (foo || array.length === 0) || array.every(Boolean) │ ^^^^^^^^^^^^^^^^^^ @@ -426,7 +426,7 @@ array.length === 0 || (array.every(Boolean) || foo) ``` invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.length === 0 || (array.every(Boolean) || foo) │ ^^^^^^^^^^^^^^^^^^ @@ -449,7 +449,7 @@ invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━ ``` invalid.jsonc:1:9 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ (foo && array.length > 0) && array.some(Boolean) │ ^^^^^^^^^^^^^^^^ @@ -472,7 +472,7 @@ array.length > 0 && (array.some(Boolean) && foo) ``` invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.length > 0 && (array.some(Boolean) && foo) │ ^^^^^^^^^^^^^^^^ @@ -495,7 +495,7 @@ array.every(Boolean) || array.length === 0 || array.every(Boolean) ``` invalid.jsonc:1:25 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.every(Boolean) || array.length === 0 || array.every(Boolean) │ ^^^^^^^^^^^^^^^^^^ @@ -518,7 +518,7 @@ array.length === 0 || array.every(Boolean) || array.length === 0 ``` invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.length === 0 || array.every(Boolean) || array.length === 0 │ ^^^^^^^^^^^^^^^^^^ @@ -535,7 +535,7 @@ invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━ ``` invalid.jsonc:1:47 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ array.length === 0 || array.every(Boolean) || array.length === 0 │ ^^^^^^^^^^^^^^^^^^ @@ -558,7 +558,7 @@ invalid.jsonc:1:47 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━ ``` invalid.jsonc:1:2 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ (array1.length === 0 || array1.every(Boolean)) || (array2.length === 0 || array2.every(Boolean)) │ ^^^^^^^^^^^^^^^^^^^ @@ -575,7 +575,7 @@ invalid.jsonc:1:2 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━ ``` invalid.jsonc:1:52 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ (array1.length === 0 || array1.every(Boolean)) || (array2.length === 0 || array2.every(Boolean)) │ ^^^^^^^^^^^^^^^^^^^ @@ -598,7 +598,7 @@ invalid.jsonc:1:52 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━ ``` invalid.jsonc:1:2 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ (array1.length === 0 || array1.every(Boolean)) && (array2.length === 0 || array2.every(Boolean)) │ ^^^^^^^^^^^^^^^^^^^ @@ -615,7 +615,7 @@ invalid.jsonc:1:2 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━ ``` invalid.jsonc:1:52 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This is a check for unnecessary length. + ! This length check is unnecessary. > 1 │ (array1.length === 0 || array1.every(Boolean)) && (array2.length === 0 || array2.every(Boolean)) │ ^^^^^^^^^^^^^^^^^^^ From 904b4cb7f0d37917044ddabfa3c0a3ef298bf455 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Fri, 25 Oct 2024 14:26:25 +0900 Subject: [PATCH 08/31] Update crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs Co-authored-by: unvalley <38400669+unvalley@users.noreply.github.com> --- .../src/lint/nursery/no_useless_length_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 65c74d546405..a5e226a2c6b0 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -17,7 +17,7 @@ declare_lint_rule! { /// Disallow unnecessary length checks within logical expressions. /// /// - `Array#some()` returns `false` for an empty array. There is no need to check if the array is not empty. - /// - `Array#every()` returns `true`` for an empty array. There is no need to check if the array is empty. + /// - `Array#every()` returns `true` for an empty array. There is no need to check if the array is empty. /// /// ## Examples /// From 7a5e3ca998538133c4944e0d4de7d62b16176059 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Fri, 25 Oct 2024 14:32:42 +0900 Subject: [PATCH 09/31] Update crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs Co-authored-by: unvalley <38400669+unvalley@users.noreply.github.com> --- .../src/lint/nursery/no_useless_length_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index a5e226a2c6b0..e9d101fe91c3 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -239,7 +239,7 @@ fn search_logical_exp( } } } - // ( hoge ) + // ( foo ) AnyJsExpression::JsParenthesizedExpression(parent_exp) => search_logical_exp( &parent_exp.expression().ok()?, replacer, From 2c9a5d0c7a1c53917856be1b5159ed6bd8c1391b Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Fri, 25 Oct 2024 14:32:53 +0900 Subject: [PATCH 10/31] Update crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs Co-authored-by: unvalley <38400669+unvalley@users.noreply.github.com> --- .../biome_js_analyze/src/lint/nursery/no_useless_length_check.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index e9d101fe91c3..b78ac4fef7b9 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -247,7 +247,6 @@ fn search_logical_exp( comparing_zeros, array_tokens_used_api, ), - // hoge AnyJsExpression::JsIdentifierExpression(_) => Some(()), _ => None, } From 114d6189ce312316d8e9da425c1dc91f63388d8f Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Fri, 25 Oct 2024 14:38:40 +0900 Subject: [PATCH 11/31] fix: use Emphasis --- .../src/lint/nursery/no_useless_length_check.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index b78ac4fef7b9..55f3104f43be 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -332,10 +332,10 @@ impl Rule for NoUselessLengthCheck { .note( match error_type { ErrorType::UselessLengthCheckWithEvery => markup! { - "The empty check is useless as `Array#every()` returns `true` for an empty array." + "The empty check is useless as ""`Array#every()`"" returns ""`true`"" for an empty array." }, ErrorType::UselessLengthCheckWithSome => markup! { - "The non-empty check is useless as `Array#some()` returns `false` for an empty array." + "The non-empty check is useless as ""`Array#some()`"" returns ""`false`"" for an empty array." }, } ), From ee38a15985a1a96d3e57b904f15831872c8d4852 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Fri, 25 Oct 2024 15:09:36 +0900 Subject: [PATCH 12/31] fix: use `text_trimmed` & JsBinaryExpression#operator() --- .../src/lint/nursery/no_useless_length_check.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 55f3104f43be..e16080a32d31 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -7,7 +7,7 @@ use biome_analyze::{ use biome_console::markup; use biome_js_syntax::{ AnyJsExpression, AnyJsLiteralExpression, AnyJsMemberExpression, JsBinaryExpression, - JsLogicalExpression, JsParenthesizedExpression, JsSyntaxKind, T, + JsBinaryOperator, JsLogicalExpression, JsParenthesizedExpression, JsSyntaxKind, T, }; use biome_rowan::{AstNode, BatchMutationExt, TextRange}; @@ -111,7 +111,7 @@ fn get_comparing_length_exp( expect_error: &ErrorType, ) -> Option { let left = binary_exp.left().ok()?; - let operator = binary_exp.operator_token().ok()?.kind(); + let operator = binary_exp.operator().ok()?; let right = binary_exp.right().ok()?; // Check only when the number appears on the right side according to the original rules. @@ -136,16 +136,19 @@ fn get_comparing_length_exp( let number = literal.as_number()?.round() as i64; // .length === 0 if matches!(expect_error, ErrorType::UselessLengthCheckWithEvery) - && literal.to_string().trim() == "0" - && (operator == T![===] || operator == T![<]) + && literal.syntax().text_trimmed() == "0" + && (operator == JsBinaryOperator::StrictEquality || operator == JsBinaryOperator::LessThan) { return Some(target); } // .length !== 0 if matches!(expect_error, ErrorType::UselessLengthCheckWithSome) - && (literal.to_string().trim() == "0" && (operator == T![!==] || operator == T![>]) - || number > 0 && operator == T![===] - || literal.to_string().trim() == "1" && operator == T![>=]) + && (literal.syntax().text_trimmed() == "0" + && (operator == JsBinaryOperator::StrictInequality + || operator == JsBinaryOperator::GreaterThan) + || number > 0 && operator == JsBinaryOperator::StrictEquality + || literal.syntax().text_trimmed() == "1" + && operator == JsBinaryOperator::LessThanOrEqual) { return Some(target); } From 9791ff39d1c1c23c506f595f678f721847aa3bc8 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sat, 26 Oct 2024 02:39:14 +0900 Subject: [PATCH 13/31] Update crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs Co-authored-by: Emanuele Stoppa --- .../src/lint/nursery/no_useless_length_check.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index e16080a32d31..1c6ad343a978 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -16,8 +16,9 @@ use crate::JsRuleAction; declare_lint_rule! { /// Disallow unnecessary length checks within logical expressions. /// - /// - `Array#some()` returns `false` for an empty array. There is no need to check if the array is not empty. - /// - `Array#every()` returns `true` for an empty array. There is no need to check if the array is empty. + /// When using the function [`Array#some()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some), it returns `false` when an array is empty; hence, there isn't need to check if the array is not empty. + /// + /// When using the function [`Array#every()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/every), it returns `true` when an array is empty; hence, there isn't need to check if the array is empty. /// /// ## Examples /// From c101768abe0cc3c7322d47e35fc30e8b536cfdb6 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sat, 26 Oct 2024 02:40:03 +0900 Subject: [PATCH 14/31] Update crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs Co-authored-by: Emanuele Stoppa --- .../src/lint/nursery/no_useless_length_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 1c6ad343a978..033616719f5c 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -94,7 +94,7 @@ fn is_logical_exp_descendant(node: &AnyJsExpression, operator: JsSyntaxKind) -> parent .ancestors() .find_map(|ancestor| { - if let Some(logical_exp) = JsLogicalExpression::cast(ancestor.clone()) { + if let Some(logical_exp) = JsLogicalExpression::cast_ref(ancestor) { return logical_exp .operator_token() .ok() From 9903022533646dcbac0dd920aa126be1374ee921 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sat, 26 Oct 2024 02:48:28 +0900 Subject: [PATCH 15/31] fix: need "&" --- .../src/lint/nursery/no_useless_length_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 033616719f5c..e605b967f371 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -94,7 +94,7 @@ fn is_logical_exp_descendant(node: &AnyJsExpression, operator: JsSyntaxKind) -> parent .ancestors() .find_map(|ancestor| { - if let Some(logical_exp) = JsLogicalExpression::cast_ref(ancestor) { + if let Some(logical_exp) = JsLogicalExpression::cast_ref(&ancestor) { return logical_exp .operator_token() .ok() From a9e0167c56a61de9ba903148fbc3540c0dafbb74 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sat, 26 Oct 2024 03:07:59 +0900 Subject: [PATCH 16/31] test: add Parenthesized case --- .../noUselessLengthCheck/invalid.jsonc | 1 + .../noUselessLengthCheck/invalid.jsonc.snap | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc index 605840193da5..5d392b0ce8fe 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc @@ -2,6 +2,7 @@ "if (array.length === 0 || array.every(Boolean));", "if (array.length !== 0 && array.some(Boolean));", "if (array.length > 0 && array.some(Boolean));", + "if (((((array.length === 0 || array.every(Boolean))))));", "const isAllTrulyOrEmpty = array.length === 0 || array.every(Boolean);", "array.length === 0 || array.every(Boolean)", "array.length > 0 && array.some(Boolean)", diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap index f9fef2825ca9..d3ccaa04dc53 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap @@ -72,6 +72,29 @@ invalid.jsonc:1:5 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━ ``` +# Input +```cjs +if (((((array.length === 0 || array.every(Boolean)))))); +``` + +# Diagnostics +``` +invalid.jsonc:1:9 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This length check is unnecessary. + + > 1 │ if (((((array.length === 0 || array.every(Boolean)))))); + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ if·(((((array.length·===·0·||·array.every(Boolean)))))); + │ -------------------------- ---- + +``` + # Input ```cjs const isAllTrulyOrEmpty = array.length === 0 || array.every(Boolean); From 9c88555dada7f175bb39d372d32ffdc739e7fd22 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sat, 26 Oct 2024 03:16:37 +0900 Subject: [PATCH 17/31] fix: use `omit_parentheses` --- .../lint/nursery/no_useless_length_check.rs | 12 +--------- .../noUselessLengthCheck/invalid.jsonc | 1 + .../noUselessLengthCheck/invalid.jsonc.snap | 23 +++++++++++++++++++ 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index e605b967f371..a9643a7dec2d 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -266,16 +266,6 @@ fn get_parenthesized_parent(exp: AnyJsExpression) -> AnyJsExpression { get_parenthesized_parent(AnyJsExpression::from(parent)) } -fn get_parenthesized_child(exp: &AnyJsExpression) -> AnyJsExpression { - let AnyJsExpression::JsParenthesizedExpression(parenthesized) = exp else { - return exp.clone(); - }; - let Some(child) = parenthesized.expression().ok() else { - return exp.clone(); - }; - get_parenthesized_child(&child) -} - impl Rule for NoUselessLengthCheck { type Query = Ast; type State = (ErrorType, Replacer); @@ -354,7 +344,7 @@ impl Rule for NoUselessLengthCheck { mutation.replace_node( get_parenthesized_parent(AnyJsExpression::from(prev_node.clone())), - get_parenthesized_child(next_node), + next_node.clone().omit_parentheses(), ); Some(JsRuleAction::new( diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc index 5d392b0ce8fe..33abf1cda521 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc @@ -19,6 +19,7 @@ "foo || array.length === 0 || array.every(Boolean)", "(foo || array.length === 0) || array.every(Boolean)", "array.length === 0 || (array.every(Boolean) || foo)", + "array.length === 0 || (((array.every(Boolean) || foo)))", "(foo && array.length > 0) && array.some(Boolean)", "array.length > 0 && (array.some(Boolean) && foo)", "array.every(Boolean) || array.length === 0 || array.every(Boolean)", diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap index d3ccaa04dc53..d8b80c8fc85b 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap @@ -463,6 +463,29 @@ invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━ ``` +# Input +```cjs +array.length === 0 || (((array.every(Boolean) || foo))) +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This length check is unnecessary. + + > 1 │ array.length === 0 || (((array.every(Boolean) || foo))) + │ ^^^^^^^^^^^^^^^^^^ + + i The empty check is useless as `Array#every()` returns `true` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.length·===·0·||·(((array.every(Boolean)·||·foo))) + │ ------------------------- --- + +``` + # Input ```cjs (foo && array.length > 0) && array.some(Boolean) From e21412cc5566407f90cfbd69db8a88358516eb75 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sat, 26 Oct 2024 03:35:53 +0900 Subject: [PATCH 18/31] fix: replace `error_type` => `function_kind` --- .../lint/nursery/no_useless_length_check.rs | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index a9643a7dec2d..6d4371ab3ada 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -81,9 +81,11 @@ declare_lint_rule! { } #[derive(Clone, Debug)] -pub enum ErrorType { - UselessLengthCheckWithSome, - UselessLengthCheckWithEvery, +pub enum FunctionKind { + /// `Array.some()` was used + Some, + /// `Array.every()` was used + Every, } /// Whether the node is a descendant of a logical expression. @@ -109,7 +111,7 @@ fn is_logical_exp_descendant(node: &AnyJsExpression, operator: JsSyntaxKind) -> /// Extract the expressions that perform length comparisons corresponding to the errors you want to check. fn get_comparing_length_exp( binary_exp: &JsBinaryExpression, - expect_error: &ErrorType, + function_kind: &FunctionKind, ) -> Option { let left = binary_exp.left().ok()?; let operator = binary_exp.operator().ok()?; @@ -136,14 +138,14 @@ fn get_comparing_length_exp( }; let number = literal.as_number()?.round() as i64; // .length === 0 - if matches!(expect_error, ErrorType::UselessLengthCheckWithEvery) + if matches!(function_kind, FunctionKind::Every) && literal.syntax().text_trimmed() == "0" && (operator == JsBinaryOperator::StrictEquality || operator == JsBinaryOperator::LessThan) { return Some(target); } // .length !== 0 - if matches!(expect_error, ErrorType::UselessLengthCheckWithSome) + if matches!(function_kind, FunctionKind::Some) && (literal.syntax().text_trimmed() == "0" && (operator == JsBinaryOperator::StrictInequality || operator == JsBinaryOperator::GreaterThan) @@ -162,16 +164,16 @@ pub type Replacer = (JsLogicalExpression, AnyJsExpression, TextRange); fn search_logical_exp( any_exp: &AnyJsExpression, replacer: Option, - expect_error: &ErrorType, + function_kind: &FunctionKind, comparing_zeros: &mut HashMap>, array_tokens_used_api: &mut HashSet, ) -> Option<()> { match any_exp { // || or && AnyJsExpression::JsLogicalExpression(logical_exp) => { - let operator = match expect_error { - ErrorType::UselessLengthCheckWithEvery => T![||], - ErrorType::UselessLengthCheckWithSome => T![&&], + let operator = match function_kind { + FunctionKind::Every => T![||], + FunctionKind::Some => T![&&], }; if logical_exp.operator_token().ok()?.kind() != operator { return None; @@ -181,7 +183,7 @@ fn search_logical_exp( search_logical_exp( &left, Some(left_replacer), - expect_error, + function_kind, comparing_zeros, array_tokens_used_api, )?; @@ -191,14 +193,14 @@ fn search_logical_exp( search_logical_exp( &right, Some(right_replacer), - expect_error, + function_kind, comparing_zeros, array_tokens_used_api, ) } // a === 0 ext. AnyJsExpression::JsBinaryExpression(binary_exp) => { - let comparing_zero = get_comparing_length_exp(binary_exp, expect_error)?; + let comparing_zero = get_comparing_length_exp(binary_exp, function_kind)?; let AnyJsExpression::JsIdentifierExpression(array_token) = comparing_zero else { return None; }; @@ -224,8 +226,8 @@ fn search_logical_exp( let task_member_name_node = task_member_exp.member_name()?; let task_member_name = task_member_name_node.text(); - match expect_error { - ErrorType::UselessLengthCheckWithEvery => { + match function_kind { + FunctionKind::Every => { if task_member_name == "every" { array_tokens_used_api.insert(task_target_token.text()); Some(()) @@ -233,7 +235,7 @@ fn search_logical_exp( None } } - ErrorType::UselessLengthCheckWithSome => { + FunctionKind::Some => { if task_member_name == "some" { array_tokens_used_api.insert(task_target_token.text()); Some(()) @@ -247,7 +249,7 @@ fn search_logical_exp( AnyJsExpression::JsParenthesizedExpression(parent_exp) => search_logical_exp( &parent_exp.expression().ok()?, replacer, - expect_error, + function_kind, comparing_zeros, array_tokens_used_api, ), @@ -268,7 +270,7 @@ fn get_parenthesized_parent(exp: AnyJsExpression) -> AnyJsExpression { impl Rule for NoUselessLengthCheck { type Query = Ast; - type State = (ErrorType, Replacer); + type State = (FunctionKind, Replacer); type Signals = Vec; type Options = (); @@ -283,12 +285,9 @@ impl Rule for NoUselessLengthCheck { return Vec::new(); } - let mut fixable_list: Vec<(ErrorType, Replacer)> = Vec::new(); + let mut fixable_list: Vec<(FunctionKind, Replacer)> = Vec::new(); - for err_type in [ - ErrorType::UselessLengthCheckWithEvery, - ErrorType::UselessLengthCheckWithSome, - ] { + for err_type in [FunctionKind::Every, FunctionKind::Some] { let mut comparing_zeros: HashMap> = HashMap::new(); let mut array_tokens_used_api: HashSet = HashSet::new(); let search_result = search_logical_exp( @@ -313,7 +312,7 @@ impl Rule for NoUselessLengthCheck { fn diagnostic( _ctx: &RuleContext, - (error_type, (_, _, error_range)): &Self::State, + (function_kind, (_, _, error_range)): &Self::State, ) -> Option { Some( RuleDiagnostic::new( @@ -324,11 +323,11 @@ impl Rule for NoUselessLengthCheck { }, ) .note( - match error_type { - ErrorType::UselessLengthCheckWithEvery => markup! { + match function_kind { + FunctionKind::Every => markup! { "The empty check is useless as ""`Array#every()`"" returns ""`true`"" for an empty array." }, - ErrorType::UselessLengthCheckWithSome => markup! { + FunctionKind::Some => markup! { "The non-empty check is useless as ""`Array#some()`"" returns ""`false`"" for an empty array." }, } From f0bf13d6dfa740c2bc9f8f11f38155de1118a061 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sat, 26 Oct 2024 04:00:43 +0900 Subject: [PATCH 19/31] fix: remove `fixable_list` cast --- .../src/lint/nursery/no_useless_length_check.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 6d4371ab3ada..6c37eaa918af 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -277,16 +277,16 @@ impl Rule for NoUselessLengthCheck { fn run(ctx: &RuleContext) -> Self::Signals { let node = ctx.query(); + let mut fixable_list = Vec::new(); + let Some(operator) = node.operator_token().ok() else { - return Vec::new(); + return fixable_list; }; // node must not be a child of a logical expression if is_logical_exp_descendant(&AnyJsExpression::from(node.clone()), operator.kind()) { - return Vec::new(); + return fixable_list; } - let mut fixable_list: Vec<(FunctionKind, Replacer)> = Vec::new(); - for err_type in [FunctionKind::Every, FunctionKind::Some] { let mut comparing_zeros: HashMap> = HashMap::new(); let mut array_tokens_used_api: HashSet = HashSet::new(); From 8326315a3d9a040373b952ae64f70bedb4899ed4 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sat, 26 Oct 2024 04:02:49 +0900 Subject: [PATCH 20/31] fix: remove cast(HashMap & HashSet) --- .../src/lint/nursery/no_useless_length_check.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 6c37eaa918af..8b9675498a3c 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -288,8 +288,8 @@ impl Rule for NoUselessLengthCheck { } for err_type in [FunctionKind::Every, FunctionKind::Some] { - let mut comparing_zeros: HashMap> = HashMap::new(); - let mut array_tokens_used_api: HashSet = HashSet::new(); + let mut comparing_zeros = HashMap::new(); + let mut array_tokens_used_api = HashSet::new(); let search_result = search_logical_exp( &AnyJsExpression::from(node.clone()), None, From f0237ddfd1a31add230b1fe33fcc5ee944935dc5 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sat, 26 Oct 2024 04:13:43 +0900 Subject: [PATCH 21/31] fix: replace `err_type` => `function_kind` --- .../src/lint/nursery/no_useless_length_check.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 8b9675498a3c..744dd1342362 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -287,13 +287,13 @@ impl Rule for NoUselessLengthCheck { return fixable_list; } - for err_type in [FunctionKind::Every, FunctionKind::Some] { + for function_kind in [FunctionKind::Every, FunctionKind::Some] { let mut comparing_zeros = HashMap::new(); let mut array_tokens_used_api = HashSet::new(); let search_result = search_logical_exp( &AnyJsExpression::from(node.clone()), None, - &err_type, + &function_kind, &mut comparing_zeros, &mut array_tokens_used_api, ); @@ -301,7 +301,7 @@ impl Rule for NoUselessLengthCheck { for array_token in array_tokens_used_api { if let Some(replacers) = comparing_zeros.get(&array_token) { for replacer in replacers { - fixable_list.push((err_type.clone(), replacer.clone())); + fixable_list.push((function_kind.clone(), replacer.clone())); } } } From c37e017ccf2078c9763cde906955a8e5af9cf835 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sat, 26 Oct 2024 04:33:45 +0900 Subject: [PATCH 22/31] fix: use struct --- .../lint/nursery/no_useless_length_check.rs | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 744dd1342362..3a68fb00c4d8 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -158,7 +158,16 @@ fn get_comparing_length_exp( None } -pub type Replacer = (JsLogicalExpression, AnyJsExpression, TextRange); +#[derive(Clone)] +/// A struct that manages the form before and after replacement. +pub struct Replacer { + /// The node before the replacement. + prev_node: JsLogicalExpression, + /// The node after the replacement. + next_node: AnyJsExpression, + /// Error occurrence location. + range: TextRange, +} /// Search for logical expressions and list expressions that compare to 0 and Array APIs (`.some()`, `.every()`). fn search_logical_exp( @@ -179,7 +188,11 @@ fn search_logical_exp( return None; }; let left = logical_exp.left().ok()?; - let left_replacer = (logical_exp.clone(), logical_exp.right().ok()?, left.range()); + let left_replacer = Replacer { + prev_node: logical_exp.clone(), + next_node: logical_exp.right().ok()?, + range: left.range(), + }; search_logical_exp( &left, Some(left_replacer), @@ -189,7 +202,11 @@ fn search_logical_exp( )?; let right = logical_exp.right().ok()?; - let right_replacer = (logical_exp.clone(), logical_exp.left().ok()?, right.range()); + let right_replacer = Replacer { + prev_node: logical_exp.clone(), + next_node: logical_exp.left().ok()?, + range: right.range(), + }; search_logical_exp( &right, Some(right_replacer), @@ -312,12 +329,12 @@ impl Rule for NoUselessLengthCheck { fn diagnostic( _ctx: &RuleContext, - (function_kind, (_, _, error_range)): &Self::State, + (function_kind, replacer): &Self::State, ) -> Option { Some( RuleDiagnostic::new( rule_category!(), - error_range, + replacer.range, markup! { "This length check is unnecessary." }, @@ -337,9 +354,14 @@ impl Rule for NoUselessLengthCheck { fn action( ctx: &RuleContext, - (_error_type, (prev_node, next_node, _error_range)): &Self::State, + (_error_type, replacer): &Self::State, ) -> Option { let mut mutation = ctx.root().begin(); + let Replacer { + prev_node, + next_node, + .. + } = replacer; mutation.replace_node( get_parenthesized_parent(AnyJsExpression::from(prev_node.clone())), From f1fa671baec2336c6f35257adafe02755a9aa813 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sun, 27 Oct 2024 00:09:30 +0900 Subject: [PATCH 23/31] fix: rename replacer=>fixable_point & use struct --- .../lint/nursery/no_useless_length_check.rs | 51 ++++++++++++------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 3a68fb00c4d8..e98a1ea74305 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -160,7 +160,7 @@ fn get_comparing_length_exp( #[derive(Clone)] /// A struct that manages the form before and after replacement. -pub struct Replacer { +pub struct FixablePoint { /// The node before the replacement. prev_node: JsLogicalExpression, /// The node after the replacement. @@ -172,9 +172,9 @@ pub struct Replacer { /// Search for logical expressions and list expressions that compare to 0 and Array APIs (`.some()`, `.every()`). fn search_logical_exp( any_exp: &AnyJsExpression, - replacer: Option, + fixable_point: Option, function_kind: &FunctionKind, - comparing_zeros: &mut HashMap>, + comparing_zeros: &mut HashMap>, array_tokens_used_api: &mut HashSet, ) -> Option<()> { match any_exp { @@ -188,28 +188,28 @@ fn search_logical_exp( return None; }; let left = logical_exp.left().ok()?; - let left_replacer = Replacer { + let left_fixable_point = FixablePoint { prev_node: logical_exp.clone(), next_node: logical_exp.right().ok()?, range: left.range(), }; search_logical_exp( &left, - Some(left_replacer), + Some(left_fixable_point), function_kind, comparing_zeros, array_tokens_used_api, )?; let right = logical_exp.right().ok()?; - let right_replacer = Replacer { + let right_fixable_point = FixablePoint { prev_node: logical_exp.clone(), next_node: logical_exp.left().ok()?, range: right.range(), }; search_logical_exp( &right, - Some(right_replacer), + Some(right_fixable_point), function_kind, comparing_zeros, array_tokens_used_api, @@ -223,9 +223,9 @@ fn search_logical_exp( }; let key = array_token.text(); if let Some(comparing_zero_list) = comparing_zeros.get_mut(&key) { - comparing_zero_list.push(replacer?); + comparing_zero_list.push(fixable_point?); } else { - comparing_zeros.insert(key, vec![replacer?]); + comparing_zeros.insert(key, vec![fixable_point?]); } Some(()) } @@ -265,7 +265,7 @@ fn search_logical_exp( // ( foo ) AnyJsExpression::JsParenthesizedExpression(parent_exp) => search_logical_exp( &parent_exp.expression().ok()?, - replacer, + fixable_point, function_kind, comparing_zeros, array_tokens_used_api, @@ -285,9 +285,16 @@ fn get_parenthesized_parent(exp: AnyJsExpression) -> AnyJsExpression { get_parenthesized_parent(AnyJsExpression::from(parent)) } +pub struct NoUselessLengthCheckState { + /// The kind of function used in the logical expression. + function_kind: FunctionKind, + /// The form before and after replacement. + fixable_point: FixablePoint, +} + impl Rule for NoUselessLengthCheck { type Query = Ast; - type State = (FunctionKind, Replacer); + type State = NoUselessLengthCheckState; type Signals = Vec; type Options = (); @@ -316,9 +323,12 @@ impl Rule for NoUselessLengthCheck { ); if search_result.is_some() { for array_token in array_tokens_used_api { - if let Some(replacers) = comparing_zeros.get(&array_token) { - for replacer in replacers { - fixable_list.push((function_kind.clone(), replacer.clone())); + if let Some(fixable_points) = comparing_zeros.get(&array_token) { + for fixable_point in fixable_points { + fixable_list.push(NoUselessLengthCheckState { + function_kind: function_kind.clone(), + fixable_point: fixable_point.clone(), + }); } } } @@ -329,12 +339,15 @@ impl Rule for NoUselessLengthCheck { fn diagnostic( _ctx: &RuleContext, - (function_kind, replacer): &Self::State, + NoUselessLengthCheckState { + function_kind, + fixable_point, + }: &Self::State, ) -> Option { Some( RuleDiagnostic::new( rule_category!(), - replacer.range, + fixable_point.range, markup! { "This length check is unnecessary." }, @@ -354,14 +367,14 @@ impl Rule for NoUselessLengthCheck { fn action( ctx: &RuleContext, - (_error_type, replacer): &Self::State, + NoUselessLengthCheckState { fixable_point, .. }: &Self::State, ) -> Option { let mut mutation = ctx.root().begin(); - let Replacer { + let FixablePoint { prev_node, next_node, .. - } = replacer; + } = fixable_point; mutation.replace_node( get_parenthesized_parent(AnyJsExpression::from(prev_node.clone())), From 2a31e864581b1a47e0da17b592895f69ce27f072 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sun, 27 Oct 2024 00:35:11 +0900 Subject: [PATCH 24/31] doc: add `get_comparing_length_exp` document --- .../src/lint/nursery/no_useless_length_check.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index e98a1ea74305..7289df82f421 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -109,6 +109,14 @@ fn is_logical_exp_descendant(node: &AnyJsExpression, operator: JsSyntaxKind) -> } /// Extract the expressions that perform length comparisons corresponding to the errors you want to check. +/// # Examples +/// ## `foo.every()` +/// `foo.length === 0` -> `Some(foo)` +/// `foo.length !== 0` -> `None` +/// ## `foo.some()` +/// `foo.length !== 0` -> `Some(foo)` +/// `foo.length >= 1` -> `Some(foo)` +/// `foo.length === 0` -> `None` fn get_comparing_length_exp( binary_exp: &JsBinaryExpression, function_kind: &FunctionKind, From a796989e318e461889c490344a0636c2782a21ae Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sun, 27 Oct 2024 00:38:37 +0900 Subject: [PATCH 25/31] fix: remove unused function --- .../src/lint/nursery/no_useless_length_check.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 7289df82f421..897f2a30aeb5 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -121,14 +121,11 @@ fn get_comparing_length_exp( binary_exp: &JsBinaryExpression, function_kind: &FunctionKind, ) -> Option { - let left = binary_exp.left().ok()?; let operator = binary_exp.operator().ok()?; - let right = binary_exp.right().ok()?; - // Check only when the number appears on the right side according to the original rules. // We assume that you have already complied with useExplicitLengthCheck - let compare_exp = left; - let value_exp = right; + let compare_exp = binary_exp.left().ok()?; + let value_exp = binary_exp.right().ok()?; let AnyJsExpression::JsStaticMemberExpression(member_exp) = compare_exp else { return None; From a2fd11f6d9731538803ed29f128cd8726223d49e Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sun, 27 Oct 2024 00:44:49 +0900 Subject: [PATCH 26/31] fix: use `as_js_static_member_expression` --- .../src/lint/nursery/no_useless_length_check.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 897f2a30aeb5..2aa1f8073816 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -127,9 +127,7 @@ fn get_comparing_length_exp( let compare_exp = binary_exp.left().ok()?; let value_exp = binary_exp.right().ok()?; - let AnyJsExpression::JsStaticMemberExpression(member_exp) = compare_exp else { - return None; - }; + let member_exp = compare_exp.as_js_static_member_expression()?; let target = member_exp.object().ok()?; let member = member_exp.member().ok()?; if member.text() != "length" || member_exp.is_optional_chain() { From 30a1a00827b60f4952adb02d024a058238c3e5d7 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sun, 27 Oct 2024 01:10:19 +0900 Subject: [PATCH 27/31] fix: remove i64 cast --- .../src/lint/nursery/no_useless_length_check.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 2aa1f8073816..f79621129598 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -133,13 +133,7 @@ fn get_comparing_length_exp( if member.text() != "length" || member_exp.is_optional_chain() { return None; } - let AnyJsExpression::AnyJsLiteralExpression(AnyJsLiteralExpression::JsNumberLiteralExpression( - literal, - )) = value_exp - else { - return None; - }; - let number = literal.as_number()?.round() as i64; + let number = literal.as_number()?.round(); // .length === 0 if matches!(function_kind, FunctionKind::Every) && literal.syntax().text_trimmed() == "0" @@ -152,7 +146,7 @@ fn get_comparing_length_exp( && (literal.syntax().text_trimmed() == "0" && (operator == JsBinaryOperator::StrictInequality || operator == JsBinaryOperator::GreaterThan) - || number > 0 && operator == JsBinaryOperator::StrictEquality + || number > 0.0 && operator == JsBinaryOperator::StrictEquality || literal.syntax().text_trimmed() == "1" && operator == JsBinaryOperator::LessThanOrEqual) { From d5efa3cc177161222fd101cb054ba9e374ce7bc3 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sun, 27 Oct 2024 01:11:58 +0900 Subject: [PATCH 28/31] fix: use `text_trimmed()` --- .../src/lint/nursery/no_useless_length_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index f79621129598..1adb97f8376c 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -130,7 +130,7 @@ fn get_comparing_length_exp( let member_exp = compare_exp.as_js_static_member_expression()?; let target = member_exp.object().ok()?; let member = member_exp.member().ok()?; - if member.text() != "length" || member_exp.is_optional_chain() { + if member.syntax().text_trimmed() != "length" || member_exp.is_optional_chain() { return None; } let number = literal.as_number()?.round(); From 6b71815da04879d39290af1718f57071230192e0 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sun, 27 Oct 2024 01:12:32 +0900 Subject: [PATCH 29/31] fix: use `as_*` --- .../src/lint/nursery/no_useless_length_check.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 1adb97f8376c..9854c4494020 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -6,8 +6,8 @@ use biome_analyze::{ }; use biome_console::markup; use biome_js_syntax::{ - AnyJsExpression, AnyJsLiteralExpression, AnyJsMemberExpression, JsBinaryExpression, - JsBinaryOperator, JsLogicalExpression, JsParenthesizedExpression, JsSyntaxKind, T, + AnyJsExpression, AnyJsMemberExpression, JsBinaryExpression, JsBinaryOperator, + JsLogicalExpression, JsParenthesizedExpression, JsSyntaxKind, T, }; use biome_rowan::{AstNode, BatchMutationExt, TextRange}; @@ -133,6 +133,8 @@ fn get_comparing_length_exp( if member.syntax().text_trimmed() != "length" || member_exp.is_optional_chain() { return None; } + let literal = value_exp.as_any_js_literal_expression()?; + let literal = literal.as_js_number_literal_expression()?; let number = literal.as_number()?.round(); // .length === 0 if matches!(function_kind, FunctionKind::Every) @@ -234,9 +236,7 @@ fn search_logical_exp( let task_member_exp = AnyJsMemberExpression::cast(task_exp.callee().ok()?.into_syntax())?; let task_target = task_member_exp.object().ok()?; - let AnyJsExpression::JsIdentifierExpression(task_target_token) = task_target else { - return None; - }; + let task_target_token = task_target.as_js_identifier_expression()?; let task_member_name_node = task_member_exp.member_name()?; let task_member_name = task_member_name_node.text(); From edfadf5caebab8aa0e415d24859959f8bce7ca57 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sun, 27 Oct 2024 01:38:20 +0900 Subject: [PATCH 30/31] fix: remove equal length check --- .../lint/nursery/no_useless_length_check.rs | 36 +++++++++---------- .../noUselessLengthCheck/invalid.jsonc | 1 + .../noUselessLengthCheck/invalid.jsonc.snap | 23 ++++++++++++ .../nursery/noUselessLengthCheck/valid.jsonc | 1 + .../noUselessLengthCheck/valid.jsonc.snap | 5 +++ 5 files changed, 47 insertions(+), 19 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 9854c4494020..4effe8d3abb1 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -135,26 +135,24 @@ fn get_comparing_length_exp( } let literal = value_exp.as_any_js_literal_expression()?; let literal = literal.as_js_number_literal_expression()?; - let number = literal.as_number()?.round(); - // .length === 0 - if matches!(function_kind, FunctionKind::Every) - && literal.syntax().text_trimmed() == "0" - && (operator == JsBinaryOperator::StrictEquality || operator == JsBinaryOperator::LessThan) - { - return Some(target); - } - // .length !== 0 - if matches!(function_kind, FunctionKind::Some) - && (literal.syntax().text_trimmed() == "0" - && (operator == JsBinaryOperator::StrictInequality - || operator == JsBinaryOperator::GreaterThan) - || number > 0.0 && operator == JsBinaryOperator::StrictEquality - || literal.syntax().text_trimmed() == "1" - && operator == JsBinaryOperator::LessThanOrEqual) - { - return Some(target); + match function_kind { + FunctionKind::Every => { + // .length === 0 + (literal.syntax().text_trimmed() == "0" + && (operator == JsBinaryOperator::StrictEquality + || operator == JsBinaryOperator::LessThan)) + .then_some(target) + } + FunctionKind::Some => { + // .length !== 0 + (literal.syntax().text_trimmed() == "0" + && (operator == JsBinaryOperator::StrictInequality + || operator == JsBinaryOperator::GreaterThan) + || literal.syntax().text_trimmed() == "1" + && operator == JsBinaryOperator::GreaterThanOrEqual) + .then_some(target) + } } - None } #[derive(Clone)] diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc index 33abf1cda521..2a8b89247646 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc @@ -7,6 +7,7 @@ "array.length === 0 || array.every(Boolean)", "array.length > 0 && array.some(Boolean)", "array.length !== 0 && array.some(Boolean)", + "array.length >= 1 && array.some(Boolean)", "if ((( array.length > 0 )) && array.some(Boolean));", "(array.length === 0 || array.every(Boolean)) || foo", "foo || (array.length === 0 || array.every(Boolean))", diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap index d8b80c8fc85b..89096c535530 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/invalid.jsonc.snap @@ -187,6 +187,29 @@ invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━ ``` +# Input +```cjs +array.length >= 1 && array.some(Boolean) +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/nursery/noUselessLengthCheck FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This length check is unnecessary. + + > 1 │ array.length >= 1 && array.some(Boolean) + │ ^^^^^^^^^^^^^^^^^ + + i The non-empty check is useless as `Array#some()` returns `false` for an empty array. + + i Unsafe fix: Remove the length check + + 1 │ array.length·>=·1·&&·array.some(Boolean) + │ --------------------- + +``` + # Input ```cjs if ((( array.length > 0 )) && array.some(Boolean)); diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc index 547964b5d6f6..1bb14b421c7f 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc @@ -1,6 +1,7 @@ [ "if (array.every(Boolean));", "if (array.some(Boolean));", + "array.length === 5 && array.some(Boolean)", "const isAllTrulyOrEmpty = array.every(Boolean);", "if (array.length === 0 || anotherCheck() || array.every(Boolean));", "const isNonEmptyAllTrulyArray = array.length > 0 && array.every(Boolean);", diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc.snap b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc.snap index c6431dc41ceb..ddad05a6581f 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessLengthCheck/valid.jsonc.snap @@ -13,6 +13,11 @@ if (array.every(Boolean)); if (array.some(Boolean)); ``` +# Input +```cjs +array.length === 5 && array.some(Boolean) +``` + # Input ```cjs const isAllTrulyOrEmpty = array.every(Boolean); From 06b2efcc7dde43cc560312965ed075c5af738787 Mon Sep 17 00:00:00 2001 From: GunseiKPaseri Date: Sun, 27 Oct 2024 02:03:49 +0900 Subject: [PATCH 31/31] doc: add `search_logical_exp` argument document --- .../src/lint/nursery/no_useless_length_check.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs index 4effe8d3abb1..fbca629f8fd5 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_useless_length_check.rs @@ -167,6 +167,11 @@ pub struct FixablePoint { } /// Search for logical expressions and list expressions that compare to 0 and Array APIs (`.some()`, `.every()`). +/// `any_exp` is the expression to be searched. +/// `fixable_point` is the form before and after replacement. +/// `function_kind` is the kind of function used in the logical expression. +/// `comparing_zeros` is a HashMap that holds the names of arrays compared with zero and their corresponding expressions. +/// `array_tokens_used_api` is a HashSet that holds the names of arrays using `some` or `every` corresponding to `function_kind`. fn search_logical_exp( any_exp: &AnyJsExpression, fixable_point: Option,