From 84075b4d7562e0c2d79314b1a3b4700ad6d2f75e Mon Sep 17 00:00:00 2001 From: kaykdm Date: Tue, 24 Sep 2024 21:22:32 +0900 Subject: [PATCH 1/2] feat(linter): implement no-nested-ternary --- .../migrate/eslint_any_rule_to_biome.rs | 8 ++ .../src/analyzer/linter/rules.rs | 119 ++++++++++-------- .../src/categories.rs | 1 + crates/biome_js_analyze/src/lint/nursery.rs | 2 + .../src/lint/nursery/no_nested_ternary.rs | 89 +++++++++++++ crates/biome_js_analyze/src/options.rs | 2 + .../specs/nursery/noNestedTernary/invalid.js | 3 + .../nursery/noNestedTernary/invalid.js.snap | 45 +++++++ .../specs/nursery/noNestedTernary/valid.js | 12 ++ .../nursery/noNestedTernary/valid.js.snap | 19 +++ .../@biomejs/backend-jsonrpc/src/workspace.ts | 5 + .../@biomejs/biome/configuration_schema.json | 7 ++ 12 files changed, 262 insertions(+), 50 deletions(-) create mode 100644 crates/biome_js_analyze/src/lint/nursery/no_nested_ternary.rs create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js.snap create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/valid.js create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/valid.js.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..1502eae7abc9 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 @@ -973,6 +973,14 @@ pub(crate) fn migrate_eslint_any_rule( let rule = group.no_negation_else.get_or_insert(Default::default()); rule.set_level(rule_severity.into()); } + "no-nested-ternary" => { + if !options.include_nursery { + return false; + } + let group = rules.nursery.get_or_insert_with(Default::default); + let rule = group.no_nested_ternary.get_or_insert(Default::default()); + rule.set_level(rule_severity.into()); + } "no-new-native-nonconstructor" => { let group = rules.correctness.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..a1eb9bb0c6fd 100644 --- a/crates/biome_configuration/src/analyzer/linter/rules.rs +++ b/crates/biome_configuration/src/analyzer/linter/rules.rs @@ -3305,6 +3305,9 @@ pub struct Nursery { #[serde(skip_serializing_if = "Option::is_none")] pub no_missing_var_function: Option>, + #[doc = "Disallow nested ternary expressions."] + #[serde(skip_serializing_if = "Option::is_none")] + pub no_nested_ternary: Option>, #[doc = "Disallow octal escape sequences in string literals"] #[serde(skip_serializing_if = "Option::is_none")] pub no_octal_escape: Option>, @@ -3418,6 +3421,7 @@ impl Nursery { "noExportedImports", "noIrregularWhitespace", "noMissingVarFunction", + "noNestedTernary", "noOctalEscape", "noProcessEnv", "noRestrictedImports", @@ -3460,13 +3464,13 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[3]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8]), - 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[19]), + 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 +3505,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 { @@ -3562,121 +3567,126 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.no_octal_escape.as_ref() { + if let Some(rule) = self.no_nested_ternary.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.no_process_env.as_ref() { + if let Some(rule) = self.no_octal_escape.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.no_restricted_imports.as_ref() { + if let Some(rule) = self.no_process_env.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.no_restricted_types.as_ref() { + if let Some(rule) = self.no_restricted_imports.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.no_secrets.as_ref() { + if let Some(rule) = self.no_restricted_types.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13])); } } - if let Some(rule) = self.no_static_element_interactions.as_ref() { + if let Some(rule) = self.no_secrets.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.no_substr.as_ref() { + if let Some(rule) = self.no_static_element_interactions.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15])); } } - if let Some(rule) = self.no_unknown_pseudo_class.as_ref() { + if let Some(rule) = self.no_substr.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16])); } } - if let Some(rule) = self.no_unknown_pseudo_element.as_ref() { + if let Some(rule) = self.no_unknown_pseudo_class.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); } } - if let Some(rule) = self.no_useless_escape_in_regex.as_ref() { + if let Some(rule) = self.no_unknown_pseudo_element.as_ref() { if rule.is_enabled() { 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_escape_in_regex.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> { @@ -3726,121 +3736,126 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.no_octal_escape.as_ref() { + if let Some(rule) = self.no_nested_ternary.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.no_process_env.as_ref() { + if let Some(rule) = self.no_octal_escape.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.no_restricted_imports.as_ref() { + if let Some(rule) = self.no_process_env.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.no_restricted_types.as_ref() { + if let Some(rule) = self.no_restricted_imports.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.no_secrets.as_ref() { + if let Some(rule) = self.no_restricted_types.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13])); } } - if let Some(rule) = self.no_static_element_interactions.as_ref() { + if let Some(rule) = self.no_secrets.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.no_substr.as_ref() { + if let Some(rule) = self.no_static_element_interactions.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15])); } } - if let Some(rule) = self.no_unknown_pseudo_class.as_ref() { + if let Some(rule) = self.no_substr.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16])); } } - if let Some(rule) = self.no_unknown_pseudo_element.as_ref() { + if let Some(rule) = self.no_unknown_pseudo_class.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); } } - if let Some(rule) = self.no_useless_escape_in_regex.as_ref() { + if let Some(rule) = self.no_unknown_pseudo_element.as_ref() { if rule.is_disabled() { 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_escape_in_regex.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"] @@ -3913,6 +3928,10 @@ impl Nursery { .no_missing_var_function .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "noNestedTernary" => self + .no_nested_ternary + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), "noOctalEscape" => self .no_octal_escape .as_ref() diff --git a/crates/biome_diagnostics_categories/src/categories.rs b/crates/biome_diagnostics_categories/src/categories.rs index 718d5ac6a418..b1301407ee2e 100644 --- a/crates/biome_diagnostics_categories/src/categories.rs +++ b/crates/biome_diagnostics_categories/src/categories.rs @@ -151,6 +151,7 @@ define_categories! { "lint/nursery/noIrregularWhitespace": "https://biomejs.dev/linter/rules/no-irregular-whitespace", "lint/nursery/noMissingGenericFamilyKeyword": "https://biomejs.dev/linter/rules/no-missing-generic-family-keyword", "lint/nursery/noMissingVarFunction": "https://biomejs.dev/linter/rules/no-missing-var-function", + "lint/nursery/noNestedTernary": "https://biomejs.dev/linter/rules/no-nested-ternary", "lint/nursery/noOctalEscape": "https://biomejs.dev/linter/rules/no-octal-escape", "lint/nursery/noProcessEnv": "https://biomejs.dev/linter/rules/no-process-env", "lint/nursery/noReactSpecificProps": "https://biomejs.dev/linter/rules/no-react-specific-props", diff --git a/crates/biome_js_analyze/src/lint/nursery.rs b/crates/biome_js_analyze/src/lint/nursery.rs index e643f3d051ee..7969f987f502 100644 --- a/crates/biome_js_analyze/src/lint/nursery.rs +++ b/crates/biome_js_analyze/src/lint/nursery.rs @@ -8,6 +8,7 @@ pub mod no_dynamic_namespace_import_access; pub mod no_enum; pub mod no_exported_imports; pub mod no_irregular_whitespace; +pub mod no_nested_ternary; pub mod no_octal_escape; pub mod no_process_env; pub mod no_restricted_imports; @@ -38,6 +39,7 @@ declare_lint_group! { self :: no_enum :: NoEnum , self :: no_exported_imports :: NoExportedImports , self :: no_irregular_whitespace :: NoIrregularWhitespace , + self :: no_nested_ternary :: NoNestedTernary , self :: no_octal_escape :: NoOctalEscape , self :: no_process_env :: NoProcessEnv , self :: no_restricted_imports :: NoRestrictedImports , diff --git a/crates/biome_js_analyze/src/lint/nursery/no_nested_ternary.rs b/crates/biome_js_analyze/src/lint/nursery/no_nested_ternary.rs new file mode 100644 index 000000000000..98efb5d053f4 --- /dev/null +++ b/crates/biome_js_analyze/src/lint/nursery/no_nested_ternary.rs @@ -0,0 +1,89 @@ +use biome_analyze::{ + context::RuleContext, declare_lint_rule, Ast, Rule, RuleDiagnostic, RuleSource, +}; +use biome_console::markup; +use biome_js_syntax::{AnyJsExpression, JsConditionalExpression}; +use biome_rowan::AstNode; + +declare_lint_rule! { + /// Disallow nested ternary expressions. + /// + /// Nesting ternary expressions can make code more difficult to understand. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// const thing = foo ? bar : baz === qux ? quxx : foobar; + /// ``` + /// + /// ```js,expect_diagnostic + /// foo ? baz === qux ? quxx() : foobar() : bar(); + /// ``` + /// + /// ### Valid + /// + /// ```js + /// const thing = foo ? bar : foobar; + /// ``` + /// + /// ```js + /// let thing; + /// + /// if (foo) { + /// thing = bar; + /// } else if (baz === qux) { + /// thing = quxx; + /// } else { + /// thing = foobar; + /// } + /// ``` + /// + pub NoNestedTernary { + version: "next", + name: "noNestedTernary", + language: "js", + recommended: false, + sources: &[RuleSource::Eslint("no-nested-ternary")], + } +} + +impl Rule for NoNestedTernary { + type Query = Ast; + type State = (); + type Signals = Option; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let node = ctx.query(); + let alternate = node.alternate().ok()?; + let consequent = node.consequent().ok()?; + if matches!(alternate, AnyJsExpression::JsConditionalExpression(_)) + || matches!(consequent, AnyJsExpression::JsConditionalExpression(_)) + { + return Some(()); + } + + None + } + + fn diagnostic(ctx: &RuleContext, _state: &Self::State) -> Option { + let node = ctx.query(); + Some( + RuleDiagnostic::new( + rule_category!(), + node.range(), + markup! { + "Do not nest ternary expressions." + }, + ) + .note(markup! { + "Nesting ternary expressions can make code more difficult to understand." + }) + .note(markup! { + "Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand." + }), + ) + } +} diff --git a/crates/biome_js_analyze/src/options.rs b/crates/biome_js_analyze/src/options.rs index c1d4ab6d04fb..3f02fe6b57db 100644 --- a/crates/biome_js_analyze/src/options.rs +++ b/crates/biome_js_analyze/src/options.rs @@ -143,6 +143,8 @@ pub type NoNamespaceImport = ::Options; pub type NoNegationElse = ::Options; +pub type NoNestedTernary = + ::Options; pub type NoNewSymbol = ::Options; pub type NoNodejsModules = diff --git a/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js b/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js new file mode 100644 index 000000000000..c10203d5dd11 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js @@ -0,0 +1,3 @@ +var thing = foo ? bar : baz === qux ? quxx : foobar; + +foo ? baz === qux ? quxx() : foobar() : bar(); \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js.snap new file mode 100644 index 000000000000..dbbb9b1bf352 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js.snap @@ -0,0 +1,45 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: invalid.js +--- +# Input +```jsx +var thing = foo ? bar : baz === qux ? quxx : foobar; + +foo ? baz === qux ? quxx() : foobar() : bar(); +``` + +# Diagnostics +``` +invalid.js:1:13 lint/nursery/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Do not nest ternary expressions. + + > 1 │ var thing = foo ? bar : baz === qux ? quxx : foobar; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + 3 │ foo ? baz === qux ? quxx() : foobar() : bar(); + + i Nesting ternary expressions can make code more difficult to understand. + + i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand. + + +``` + +``` +invalid.js:3:1 lint/nursery/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Do not nest ternary expressions. + + 1 │ var thing = foo ? bar : baz === qux ? quxx : foobar; + 2 │ + > 3 │ foo ? baz === qux ? quxx() : foobar() : bar(); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + i Nesting ternary expressions can make code more difficult to understand. + + i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/valid.js b/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/valid.js new file mode 100644 index 000000000000..4c25576b0041 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/valid.js @@ -0,0 +1,12 @@ +/* should not generate diagnostics */ +const thing = foo ? bar : foobar; + +let thing; + +if (foo) { + thing = bar; +} else if (baz === qux) { + thing = quxx; +} else { + thing = foobar; +} \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/valid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/valid.js.snap new file mode 100644 index 000000000000..686dfe866681 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/valid.js.snap @@ -0,0 +1,19 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: valid.js +--- +# Input +```jsx +/* should not generate diagnostics */ +const thing = foo ? bar : foobar; + +let thing; + +if (foo) { + thing = bar; +} else if (baz === qux) { + thing = quxx; +} else { + thing = foobar; +} +``` diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index 5f6212d128fa..d1d37f06a061 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -1254,6 +1254,10 @@ export interface Nursery { * Disallow missing var function for css variables. */ noMissingVarFunction?: RuleConfiguration_for_Null; + /** + * Disallow nested ternary expressions. + */ + noNestedTernary?: RuleConfiguration_for_Null; /** * Disallow octal escape sequences in string literals */ @@ -2840,6 +2844,7 @@ export type Category = | "lint/nursery/noIrregularWhitespace" | "lint/nursery/noMissingGenericFamilyKeyword" | "lint/nursery/noMissingVarFunction" + | "lint/nursery/noNestedTernary" | "lint/nursery/noOctalEscape" | "lint/nursery/noProcessEnv" | "lint/nursery/noReactSpecificProps" diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index 7c01d4457c7d..93f122967de7 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -2139,6 +2139,13 @@ { "type": "null" } ] }, + "noNestedTernary": { + "description": "Disallow nested ternary expressions.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noOctalEscape": { "description": "Disallow octal escape sequences in string literals", "anyOf": [ From 5bf0f081864d63ea488d7a35a4b34c0a697319db Mon Sep 17 00:00:00 2001 From: kaykdm Date: Wed, 25 Sep 2024 14:09:52 +0900 Subject: [PATCH 2/2] Fix: Flag only the offending child ternary instead of the entire parent ternary --- .../src/lint/nursery/no_nested_ternary.rs | 20 ++++++++++--------- .../nursery/noNestedTernary/invalid.js.snap | 8 ++++---- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_nested_ternary.rs b/crates/biome_js_analyze/src/lint/nursery/no_nested_ternary.rs index 98efb5d053f4..7587478829f4 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_nested_ternary.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_nested_ternary.rs @@ -3,7 +3,7 @@ use biome_analyze::{ }; use biome_console::markup; use biome_js_syntax::{AnyJsExpression, JsConditionalExpression}; -use biome_rowan::AstNode; +use biome_rowan::{AstNode, TextRange}; declare_lint_rule! { /// Disallow nested ternary expressions. @@ -51,7 +51,7 @@ declare_lint_rule! { impl Rule for NoNestedTernary { type Query = Ast; - type State = (); + type State = TextRange; type Signals = Option; type Options = (); @@ -59,21 +59,23 @@ impl Rule for NoNestedTernary { let node = ctx.query(); let alternate = node.alternate().ok()?; let consequent = node.consequent().ok()?; - if matches!(alternate, AnyJsExpression::JsConditionalExpression(_)) - || matches!(consequent, AnyJsExpression::JsConditionalExpression(_)) - { - return Some(()); + + if let AnyJsExpression::JsConditionalExpression(expr) = consequent { + return Some(expr.range()); + } + + if let AnyJsExpression::JsConditionalExpression(expr) = alternate { + return Some(expr.range()); } None } - fn diagnostic(ctx: &RuleContext, _state: &Self::State) -> Option { - let node = ctx.query(); + fn diagnostic(_: &RuleContext, state: &Self::State) -> Option { Some( RuleDiagnostic::new( rule_category!(), - node.range(), + state, markup! { "Do not nest ternary expressions." }, diff --git a/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js.snap index dbbb9b1bf352..b087df4a7799 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/noNestedTernary/invalid.js.snap @@ -11,12 +11,12 @@ foo ? baz === qux ? quxx() : foobar() : bar(); # Diagnostics ``` -invalid.js:1:13 lint/nursery/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.js:1:25 lint/nursery/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Do not nest ternary expressions. > 1 │ var thing = foo ? bar : baz === qux ? quxx : foobar; - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 2 │ 3 │ foo ? baz === qux ? quxx() : foobar() : bar(); @@ -28,14 +28,14 @@ invalid.js:1:13 lint/nursery/noNestedTernary ━━━━━━━━━━━ ``` ``` -invalid.js:3:1 lint/nursery/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.js:3:7 lint/nursery/noNestedTernary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Do not nest ternary expressions. 1 │ var thing = foo ? bar : baz === qux ? quxx : foobar; 2 │ > 3 │ foo ? baz === qux ? quxx() : foobar() : bar(); - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ i Nesting ternary expressions can make code more difficult to understand.