Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_analyze): implement the validTypeof rule #2649

Merged
merged 4 commits into from
Jun 9, 2022
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
2 changes: 2 additions & 0 deletions crates/rome_analyze/src/analyzers.rs

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

201 changes: 201 additions & 0 deletions crates/rome_analyze/src/analyzers/use_valid_typeof.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
use rome_console::markup;
use rome_diagnostics::{Applicability, Severity};
use rome_js_factory::make;
use rome_js_syntax::{
JsAnyExpression, JsAnyLiteralExpression, JsAnyRoot, JsBinaryExpression,
JsBinaryExpressionFields, JsBinaryOperator, JsUnaryOperator,
};
use rome_rowan::{AstNode, AstNodeExt};

use crate::{
registry::{JsRuleAction, Rule, RuleDiagnostic},
ActionCategory, RuleCategory,
};

/// This rule verifies the result of `typeof $expr` unary expressions is being
/// compared to valid values, either string literals containing valid type
/// names or other `typeof` expressions
pub(crate) enum UseValidTypeof {}

impl Rule for UseValidTypeof {
const NAME: &'static str = "useValidTypeof";
const CATEGORY: RuleCategory = RuleCategory::Lint;

type Query = JsBinaryExpression;
type State = Option<(JsAnyExpression, JsTypeName)>;

fn run(n: &Self::Query) -> Option<Self::State> {
let JsBinaryExpressionFields {
left,
operator_token: _,
right,
} = n.as_fields();

if !matches!(
n.operator().ok()?,
JsBinaryOperator::Equality
| JsBinaryOperator::StrictEquality
| JsBinaryOperator::Inequality
| JsBinaryOperator::StrictInequality
) {
return None;
}

let left = left.ok()?;
let right = right.ok()?;

match (&left, &right) {
// Check for `typeof $expr == $lit` and `$lit == typeof $expr`
(
JsAnyExpression::JsUnaryExpression(unary),
lit @ JsAnyExpression::JsAnyLiteralExpression(literal),
)
| (
lit @ JsAnyExpression::JsAnyLiteralExpression(literal),
JsAnyExpression::JsUnaryExpression(unary),
) => {
if unary.operator().ok()? != JsUnaryOperator::Typeof {
return None;
}

if let JsAnyLiteralExpression::JsStringLiteralExpression(literal) = literal {
let literal = literal.value_token().ok()?;
let literal = literal
.text_trimmed()
.trim_start_matches(['"', '\''])
.trim_end_matches(['"', '\''])
.to_lowercase();

if JsTypeName::from_str(&literal).is_some() {
return None;
}

// Try to fix the casing of the literal eg. "String" -> "string"
let literal = literal.to_lowercase();
return Some(
JsTypeName::from_str(&literal).map(|type_name| (lit.clone(), type_name)),
);
}
}

// Check for `typeof $expr == typeof $expr`
(
JsAnyExpression::JsUnaryExpression(left),
JsAnyExpression::JsUnaryExpression(right),
) => {
let is_typeof_left = left.operator().ok()? != JsUnaryOperator::Typeof;
let is_typeof_right = right.operator().ok()? != JsUnaryOperator::Typeof;
if (is_typeof_left && is_typeof_right) || (!is_typeof_left && !is_typeof_right) {
return None;
}
}

// Check for `typeof $expr == $ident`
(
JsAnyExpression::JsUnaryExpression(unary),
id @ JsAnyExpression::JsIdentifierExpression(ident),
)
| (
JsAnyExpression::JsIdentifierExpression(ident),
id @ JsAnyExpression::JsUnaryExpression(unary),
) => {
if unary.operator().ok()? != JsUnaryOperator::Typeof {
return None;
}

// Try to convert the identifier to a string literal eg. String -> "string"
return Some(ident.name().ok().and_then(|name| {
let value = name.value_token().ok()?;

let to_lower = value.text_trimmed().to_lowercase();
let as_type = JsTypeName::from_str(&to_lower)?;

Some((id.clone(), as_type))
}));
}

// Check for `typeof $expr == $expr`
(JsAnyExpression::JsUnaryExpression(unary), _)
| (_, JsAnyExpression::JsUnaryExpression(unary)) => {
if unary.operator().ok()? != JsUnaryOperator::Typeof {
return None;
}
}

_ => return None,
}

Some(None)
}

fn diagnostic(node: &Self::Query, _: &Self::State) -> Option<RuleDiagnostic> {
Some(RuleDiagnostic {
severity: Severity::Warning,
message: markup! {
"Invalid typeof comparison value"
}
.to_owned(),
range: node.range(),
})
}

fn action(root: JsAnyRoot, _node: &Self::Query, state: &Self::State) -> Option<JsRuleAction> {
let (expr, type_name) = state.as_ref()?;

let root = root.replace_node(
expr.clone(),
JsAnyExpression::JsAnyLiteralExpression(JsAnyLiteralExpression::from(
make::js_string_literal_expression(make::js_string_literal(type_name.as_str())),
)),
)?;

Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
message: markup! { "Compare the result of `typeof` with a valid type name" }.to_owned(),
root,
})
}
}

pub enum JsTypeName {
Undefined,
Object,
Boolean,
Number,
String,
Function,
Symbol,
BigInt,
}

impl JsTypeName {
/// construct a [JsTypeName] from the textual name of a JavaScript type
fn from_str(s: &str) -> Option<Self> {
Some(match s {
"undefined" => Self::Undefined,
"object" => Self::Object,
"boolean" => Self::Boolean,
"number" => Self::Number,
"string" => Self::String,
"function" => Self::Function,
"symbol" => Self::Symbol,
"bigint" => Self::BigInt,
_ => return None,
})
}

/// Convert a [JsTypeName] to a JS string literal
const fn as_str(&self) -> &'static str {
match self {
Self::Undefined => "undefined",
Self::Object => "object",
Self::Boolean => "boolean",
Self::Number => "number",
Self::String => "string",
Self::Function => "function",
Self::Symbol => "symbol",
Self::BigInt => "bigint",
}
}
}
1 change: 1 addition & 0 deletions crates/rome_analyze/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ impl_registry_builders!(
NoDelete,
NoDoubleEquals,
UseSingleVarDeclarator,
UseValidTypeof,
UseWhile,
// Assists
FlipBinExp,
Expand Down
17 changes: 17 additions & 0 deletions crates/rome_analyze/tests/specs/useValidTypeof.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Valid Cases
typeof foo === "string"
typeof bar == "undefined"
typeof bar === typeof qux

// Invalid literals
typeof foo === "strnig"
typeof foo == "undefimed"
typeof bar != "nunber"
typeof bar !== "fucntion"

// Invalid expressions
typeof foo === undefined
typeof bar == Object
typeof foo === baz
typeof foo == 5
typeof foo == -5
140 changes: 140 additions & 0 deletions crates/rome_analyze/tests/specs/useValidTypeof.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
---
source: crates/rome_analyze/tests/spec_tests.rs
expression: useValidTypeof.js
---
# Input
```js
// Valid Cases
typeof foo === "string"
typeof bar == "undefined"
typeof bar === typeof qux

// Invalid literals
typeof foo === "strnig"
typeof foo == "undefimed"
typeof bar != "nunber"
typeof bar !== "fucntion"

// Invalid expressions
typeof foo === undefined
typeof bar == Object
typeof foo === baz
typeof foo == 5
typeof foo == -5

```

# Diagnostics
```
warning[useValidTypeof]: Invalid typeof comparison value
┌─ useValidTypeof.js:7:1
7 │ typeof foo === "strnig"
│ ----------------------- Invalid typeof comparison value


```

```
warning[useValidTypeof]: Invalid typeof comparison value
┌─ useValidTypeof.js:8:1
8 │ typeof foo == "undefimed"
│ ------------------------- Invalid typeof comparison value


```

```
warning[useValidTypeof]: Invalid typeof comparison value
┌─ useValidTypeof.js:9:1
9 │ typeof bar != "nunber"
│ ---------------------- Invalid typeof comparison value


```

```
warning[useValidTypeof]: Invalid typeof comparison value
┌─ useValidTypeof.js:10:1
10 │ typeof bar !== "fucntion"
│ ------------------------- Invalid typeof comparison value


```

```
warning[useValidTypeof]: Invalid typeof comparison value
┌─ useValidTypeof.js:13:1
13 │ typeof foo === undefined
│ ------------------------ Invalid typeof comparison value

Compare the result of `typeof` with a valid type name
| @@ -10,7 +10,7 @@
9 9 | typeof bar !== "fucntion"
10 10 |
11 11 | // Invalid expressions
12 | - typeof foo === undefined
12 | + typeof foo === "undefined"
13 13 | typeof bar == Object
14 14 | typeof foo === baz
15 15 | typeof foo == 5


```

```
warning[useValidTypeof]: Invalid typeof comparison value
┌─ useValidTypeof.js:14:1
14 │ typeof bar == Object
│ -------------------- Invalid typeof comparison value

Compare the result of `typeof` with a valid type name
| @@ -11,7 +11,7 @@
10 10 |
11 11 | // Invalid expressions
12 12 | typeof foo === undefined
13 | - typeof bar == Object
13 | + typeof bar == "object"
14 14 | typeof foo === baz
15 15 | typeof foo == 5
16 16 | typeof foo == -5


```

```
warning[useValidTypeof]: Invalid typeof comparison value
┌─ useValidTypeof.js:15:1
15 │ typeof foo === baz
│ ------------------ Invalid typeof comparison value


```

```
warning[useValidTypeof]: Invalid typeof comparison value
┌─ useValidTypeof.js:16:1
16 │ typeof foo == 5
│ --------------- Invalid typeof comparison value


```

```
warning[useValidTypeof]: Invalid typeof comparison value
┌─ useValidTypeof.js:17:1
17 │ typeof foo == -5
│ ---------------- Invalid typeof comparison value


```


10 changes: 10 additions & 0 deletions crates/rome_js_factory/src/make.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ pub fn ident(text: &str) -> JsSyntaxToken {
JsSyntaxToken::new_detached(JsSyntaxKind::IDENT, text, [], [])
}

/// Create a new string literal token with no attached trivia
pub fn js_string_literal(text: &str) -> JsSyntaxToken {
JsSyntaxToken::new_detached(
JsSyntaxKind::JS_STRING_LITERAL,
&format!("\"{text}\""),
[],
[],
)
}

/// Create a new token with the specified syntax kind and no attached trivia
pub fn token(kind: JsSyntaxKind) -> JsSyntaxToken {
if let Some(text) = kind.to_string() {
Expand Down