Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linter): implement no-nested-ternary #4067

Merged
merged 2 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

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

119 changes: 69 additions & 50 deletions crates/biome_configuration/src/analyzer/linter/rules.rs

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions crates/biome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ,
Expand Down
91 changes: 91 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery/no_nested_ternary.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
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, TextRange};

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<JsConditionalExpression>;
type State = TextRange;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
let alternate = node.alternate().ok()?;
let consequent = node.consequent().ok()?;

if let AnyJsExpression::JsConditionalExpression(expr) = consequent {
return Some(expr.range());
}

if let AnyJsExpression::JsConditionalExpression(expr) = alternate {
return Some(expr.range());
}

None
}

fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
Some(
RuleDiagnostic::new(
rule_category!(),
state,
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."
}),
)
}
}
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/options.rs

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
var thing = foo ? bar : baz === qux ? quxx : foobar;

foo ? baz === qux ? quxx() : foobar() : bar();
Original file line number Diff line number Diff line change
@@ -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: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();

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: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.

i Convert nested ternary expression into if-else statements or separate the conditions to make the logic easier to understand.


```
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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;
}
```
5 changes: 5 additions & 0 deletions packages/@biomejs/backend-jsonrpc/src/workspace.ts

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

7 changes: 7 additions & 0 deletions packages/@biomejs/biome/configuration_schema.json

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

Loading