Skip to content

Commit

Permalink
feat(lint): rule noMisplacedAssertion
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Mar 13, 2024
1 parent 172d906 commit 106ad4b
Show file tree
Hide file tree
Showing 13 changed files with 302 additions and 16 deletions.
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/analyzers/nursery.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
Expand Up @@ -80,10 +80,10 @@ impl Rule for NoExcessiveNestedTestSuites {
"Excessive `describe()` nesting detected."
},
)
.note(markup! {
.note(markup! {
"Excessive nesting of "<Emphasis>"describe()"</Emphasis>" calls can hinder test readability."
})
.note(markup! {
.note(markup! {
"Consider refactoring and "<Emphasis>"reduce the level of nested describe"</Emphasis>" to improve code clarity."
}),
)
Expand Down Expand Up @@ -116,14 +116,10 @@ impl Visitor for NestedTestVisitor {
WalkEvent::Enter(node) => {
if let Some(node) = JsCallExpression::cast_ref(node) {
if let Ok(callee) = node.callee() {
if callee.contains_a_test_pattern() == Ok(true) {
if let Some(function_name) = callee.get_callee_object_name() {
if function_name.text_trimmed() == "describe" {
self.curr_count += 1;
if self.curr_count == self.max_count + 1 {
ctx.match_query(NestedTest(node.clone()));
}
}
if callee.is_test_describe_call() {
self.curr_count += 1;
if self.curr_count == self.max_count + 1 {
ctx.match_query(NestedTest(node.clone()));
}
}
}
Expand All @@ -132,12 +128,8 @@ impl Visitor for NestedTestVisitor {
WalkEvent::Leave(node) => {
if let Some(node) = JsCallExpression::cast_ref(node) {
if let Ok(callee) = node.callee() {
if callee.contains_a_test_pattern() == Ok(true) {
if let Some(function_name) = callee.get_callee_object_name() {
if function_name.text_trimmed() == "describe" {
self.curr_count -= 1;
}
}
if callee.is_test_describe_call() {
self.curr_count -= 1;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
use crate::semantic_services::Semantic;
use biome_analyze::{
context::RuleContext, declare_rule, Rule, RuleDiagnostic, RuleSource, RuleSourceKind,
};
use biome_console::markup;
use biome_deserialize::TextRange;
use biome_js_syntax::{
AnyJsNamedImportSpecifier, JsCallExpression, JsIdentifierBinding, JsImport, JsModule,
};
use biome_rowan::{AstNode, WalkEvent};

declare_rule! {
/// Succinct description of the rule.
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// ```
///
/// ### Valid
///
/// ```js
/// ```
///
pub NoMisplacedAssertion {
version: "next",
name: "noMisplacedAssertion",
recommended: false,
source: RuleSource::EslintJest("no-standalone-expect"),
source_kind: RuleSourceKind::Inspired,
}
}

impl Rule for NoMisplacedAssertion {
type Query = Semantic<JsModule>;
type State = TextRange;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
let model = ctx.model();
let preorder = node.syntax().preorder();
let mut inside_describe_call = false;
let mut inside_it_call = false;
let mut assertion_call = None;
for event in preorder {
match event {
WalkEvent::Enter(node) => {
if let Some(node) = JsCallExpression::cast(node) {
if let Ok(callee) = node.callee() {
if callee.is_test_describe_call() {
inside_describe_call = true
}
if callee.is_test_it_call() {
inside_it_call = true
}
}
}
}
WalkEvent::Leave(node) => {
if let Some(node) = JsCallExpression::cast(node) {
if let Ok(callee) = node.callee() {
if callee.is_test_describe_call() {
inside_describe_call = false
}
if callee.is_test_it_call() {
inside_it_call = false
}
if callee.is_assertion_call() {
if inside_describe_call && !inside_it_call {
assertion_call =
Some(node.callee().ok()?.as_js_reference_identifier()?);
break;
}
}
}
}
}
}
}

if let Some(assertion_call) = assertion_call {
let call_text = assertion_call.value_token().ok()?;
let binding = model.binding(&assertion_call);
if let Some(binding) = binding {
let ident = JsIdentifierBinding::cast_ref(binding.syntax())?;
let import_specifier = ident.parent::<AnyJsNamedImportSpecifier>()?;
let import = import_specifier.import_clause()?.parent::<JsImport>()?;
let source_text = import.source_text().ok()?;
if (matches!(source_text.text(), "chai") && call_text.text_trimmed() == "expect")
|| (matches!(call_text.text_trimmed(), "assert")
&& matches!(source_text.text(), "node:assert" | "node:assert/strict"))
{
return Some(assertion_call.range());
}
} else {
return Some(assertion_call.range());
}
}

None
}

fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
Some(RuleDiagnostic::new(
rule_category!(),
state,
markup! {
"The assertion isn't inside an it call."
},
))
}
}
1 change: 1 addition & 0 deletions crates/biome_js_analyze/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ pub type NoLabelVar =
<semantic_analyzers::suspicious::no_label_var::NoLabelVar as biome_analyze::Rule>::Options;
pub type NoMisleadingCharacterClass = < semantic_analyzers :: suspicious :: no_misleading_character_class :: NoMisleadingCharacterClass as biome_analyze :: Rule > :: Options ;
pub type NoMisleadingInstantiator = < analyzers :: suspicious :: no_misleading_instantiator :: NoMisleadingInstantiator as biome_analyze :: Rule > :: Options ;
pub type NoMisplacedAssertion = < analyzers :: nursery :: no_misplaced_assertion :: NoMisplacedAssertion as biome_analyze :: Rule > :: Options ;
pub type NoMisrefactoredShorthandAssign = < analyzers :: suspicious :: no_misrefactored_shorthand_assign :: NoMisrefactoredShorthandAssign as biome_analyze :: Rule > :: Options ;
pub type NoMultipleSpacesInRegularExpressionLiterals = < analyzers :: complexity :: no_multiple_spaces_in_regular_expression_literals :: NoMultipleSpacesInRegularExpressionLiterals as biome_analyze :: Rule > :: Options ;
pub type NoNamespace =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
describe(() => {
expect("something").toBeTrue()
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: invalid.js
---
# Input
```jsx
describe(() => {
expect("something").toBeTrue()
})
```

# Diagnostics
```
invalid.js:2:2 lint/nursery/noMisplacedAssertion ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The assertion isn't inside an it call.
1 │ describe(() => {
> 2 │ expect("something").toBeTrue()
│ ^^^^^^^^^^^^^^^^^^^
3 │ })
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { expect } from "chai";
describe(() => {
expect("something").toBeTrue()
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: invalidImportedChai.js
---
# Input
```jsx
import { expect } from "chai";
describe(() => {
expect("something").toBeTrue()
})
```

# Diagnostics
```
invalidImportedChai.js:3:2 lint/nursery/noMisplacedAssertion ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The assertion isn't inside an it call.
1 │ import { expect } from "chai";
2 │ describe(() => {
> 3 │ expect("something").toBeTrue()
│ ^^^^^^^^^^^^^^^^^^^
4 │ })
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import assert from "node:assert";
import { describe } from "node:test";

describe(() => {
assert.equal("something", "something")
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
describe("msg", () => {
it("msg", () => {
expect("something").toBeTrue()
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: valid.js
---
# Input
```jsx
describe("msg", () => {
it("msg", () => {
expect("something").toBeTrue()
})
})
```


33 changes: 33 additions & 0 deletions crates/biome_js_syntax/src/expr_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,39 @@ impl AnyJsExpression {
_ => false,
})
}

pub fn is_test_describe_call(&self) -> bool {
if self.contains_a_test_pattern() == Ok(true) {
if let Some(function_name) = self.get_callee_object_name() {
if matches!(function_name.text_trimmed(), "describe") {
return true;
}
}
}
false
}

pub fn is_test_it_call(&self) -> bool {
if self.contains_a_test_pattern() == Ok(true) {
if let Some(function_name) = self.get_callee_object_name() {
if matches!(function_name.text_trimmed(), "it") {
return true;
}
}
}
false
}

pub fn is_assertion_call(&self) -> bool {
let name = self.get_callee_object_name();
if let Some(name) = name {
if matches!(name.text_trimmed(), "expect" | "assert") {
return true;
}
}

false
}
}

/// Iterator that returns the callee names in "top down order".
Expand Down
56 changes: 56 additions & 0 deletions website/src/content/docs/linter/rules/no-misplaced-assertion.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
title: noMisplacedAssertion (not released)
---

**Diagnostic Category: `lint/nursery/noMisplacedAssertion`**

:::danger
This rule hasn't been released yet.
:::

:::caution
This rule is part of the [nursery](/linter/rules/#nursery) group.
:::

Succinct description of the rule.

Put context and details about the rule.
As a starting point, you can take the description of the corresponding _ESLint_ rule (if any).

Try to stay consistent with the descriptions of implemented rules.

Add a link to the corresponding ESLint rule (if any):

## Examples

### Invalid

```jsx
var a = 1;
a = 2;
```

<pre class="language-text"><code class="language-text">nursery/noMisplacedAssertion.js:1:11 <a href="https://biomejs.dev/linter/rules/no-misplaced-assertion">lint/nursery/noMisplacedAssertion</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

<strong><span style="color: Orange;"> </span></strong><strong><span style="color: Orange;">⚠</span></strong> <span style="color: Orange;">Variable is read here.</span>

<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>1 │ </strong>var a = 1;
<strong> │ </strong>
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>2 │ </strong>a = 2;
<strong> │ </strong><strong><span style="color: Tomato;">^</span></strong><strong><span style="color: Tomato;">^</span></strong>
<strong>3 │ </strong>

<strong><span style="color: lightgreen;"> </span></strong><strong><span style="color: lightgreen;">ℹ</span></strong> <span style="color: lightgreen;">This note will give you more information.</span>

</code></pre>

### Valid

```jsx
var a = 1;
```

## Related links

- [Disable a rule](/linter/#disable-a-lint-rule)
- [Rule options](/linter/#rule-options)

0 comments on commit 106ad4b

Please sign in to comment.