This repository has been archived by the owner on Aug 31, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 664
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(rome_analyze): implement the validTypeof rule (#2649)
* feat(rome_analyze): implement the validTypeof rule * rename rule to `useValidTypeof` * add documentation and fix merge issues * update snapshots
- Loading branch information
Showing
6 changed files
with
371 additions
and
0 deletions.
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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", | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
``` | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters