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 Feb 28, 2024
1 parent 29f9227 commit 45047e7
Show file tree
Hide file tree
Showing 20 changed files with 400 additions and 79 deletions.
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 @@ -114,6 +114,7 @@ define_categories! {
"lint/nursery/noGlobalEval": "https://biomejs.dev/linter/rules/no-global-eval",
"lint/nursery/noInvalidUseBeforeDeclaration": "https://biomejs.dev/linter/rules/no-invalid-use-before-declaration",
"lint/nursery/noMisleadingCharacterClass": "https://biomejs.dev/linter/rules/no-misleading-character-class",
"lint/nursery/noMisplacedAssertion": "https://biomejs.dev/linter/rules/no-misplaced-assertion",
"lint/nursery/noNamespaceImport": "https://biomejs.dev/linter/rules/no-namespace-import",
"lint/nursery/noNodejsModules": "https://biomejs.dev/linter/rules/no-nodejs-modules",
"lint/nursery/noReExportAll": "https://biomejs.dev/linter/rules/no-re-export-all",
Expand Down
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."
},
))
}
}
6 changes: 4 additions & 2 deletions crates/biome_js_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,9 @@ mod tests {
String::from_utf8(buffer).unwrap()
}

const SOURCE: &str = r#"<a href="class/html-css1/navigation/links#" onclick="window.location.href=index.html"> Home </a>
const SOURCE: &str = r#"describe(() => {
assert.equal("something", "something")
})
"#;
// const SOURCE: &str = r#"document.querySelector("foo").value = document.querySelector("foo").value
//
Expand All @@ -273,7 +275,7 @@ mod tests {
closure_index: Some(0),
dependencies_index: Some(1),
};
let rule_filter = RuleFilter::Rule("a11y", "useValidAnchor");
let rule_filter = RuleFilter::Rule("nursery", "noMisplacedAssertion");

options.configuration.rules.push_rule(
RuleKey::new("nursery", "useHookAtTopLevel"),
Expand Down
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 @@ -104,6 +104,7 @@ pub type NoLabelVar =
<semantic_analyzers::suspicious::no_label_var::NoLabelVar as biome_analyze::Rule>::Options;
pub type NoMisleadingCharacterClass = < semantic_analyzers :: nursery :: 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 @@ -1055,6 +1055,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
Loading

0 comments on commit 45047e7

Please sign in to comment.