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 660
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(rome_js_analyze): implement
noNonNullAssertion rule (#3891)
- Loading branch information
Showing
13 changed files
with
744 additions
and
7 deletions.
There are no files selected for viewing
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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
160 changes: 160 additions & 0 deletions
160
crates/rome_js_analyze/src/analyzers/nursery/no_non_null_assertion.rs
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,160 @@ | ||
| use crate::JsRuleAction; | ||
| use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic}; | ||
| use rome_console::markup; | ||
| use rome_diagnostics::Applicability; | ||
| use rome_js_factory::make; | ||
| use rome_js_syntax::{ | ||
| AnyJsExpression, JsSyntaxKind, TsNonNullAssertionAssignment, TsNonNullAssertionExpression, | ||
| }; | ||
| use rome_rowan::{declare_node_union, AstNode, BatchMutationExt}; | ||
|
|
||
| declare_rule! { | ||
| /// Disallow non-null assertions using the `!` postfix operator. | ||
| /// | ||
| /// TypeScript's `!` non-null assertion operator asserts to the type system that an expression is non-nullable, as | ||
| /// in not `null` or `undefined`. Using assertions to tell the type system new information is often a sign that | ||
| /// code is not fully type-safe. It's generally better to structure program logic so that TypeScript understands | ||
| /// when values may be nullable. | ||
| /// | ||
| /// ## Examples | ||
| /// | ||
| /// ### Invalid | ||
| /// | ||
| /// ```ts,expect_diagnostic | ||
| /// interface Example { | ||
| /// property?: string; | ||
| /// } | ||
| /// declare const example: Example; | ||
| /// const includesBaz = foo.property!.includes('baz'); | ||
| /// ``` | ||
| /// ```ts,expect_diagnostic | ||
| /// (b!! as number) = "test"; | ||
| /// ``` | ||
| /// | ||
| /// ### Valid | ||
| /// | ||
| /// ```ts | ||
| /// interface Example { | ||
| /// property?: string; | ||
| /// } | ||
| /// | ||
| /// declare const example: Example; | ||
| /// const includesBaz = foo.property?.includes('baz') ?? false; | ||
| /// ``` | ||
| /// | ||
| pub(crate) NoNonNullAssertion { | ||
| version: "11.0.0", | ||
| name: "noNonNullAssertion", | ||
| recommended: false, | ||
| } | ||
| } | ||
|
|
||
| declare_node_union! { | ||
| pub(crate) AnyTsNonNullAssertion = TsNonNullAssertionExpression | TsNonNullAssertionAssignment | ||
| } | ||
|
|
||
| impl Rule for NoNonNullAssertion { | ||
| type Query = Ast<AnyTsNonNullAssertion>; | ||
| type State = (); | ||
| type Signals = Option<Self::State>; | ||
| type Options = (); | ||
|
|
||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
| match ctx.query() { | ||
| AnyTsNonNullAssertion::TsNonNullAssertionExpression(node) => node | ||
| .parent::<TsNonNullAssertionExpression>() | ||
| .map_or(Some(()), |_| None), | ||
| AnyTsNonNullAssertion::TsNonNullAssertionAssignment(node) => node | ||
| .parent::<TsNonNullAssertionAssignment>() | ||
| .map_or(Some(()), |_| None), | ||
| } | ||
| } | ||
|
|
||
| fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> { | ||
| Some(RuleDiagnostic::new( | ||
| rule_category!(), | ||
| ctx.query().range(), | ||
| markup! { | ||
| "Forbidden non-null assertion." | ||
| }, | ||
| )) | ||
| } | ||
|
|
||
| fn action(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<JsRuleAction> { | ||
| let node = ctx.query(); | ||
| match node { | ||
| AnyTsNonNullAssertion::TsNonNullAssertionAssignment(_) => None, | ||
| AnyTsNonNullAssertion::TsNonNullAssertionExpression(node) => { | ||
| if !can_replace_with_optional_chain(node) { | ||
| return None; | ||
| } | ||
| let mut mutation = ctx.root().begin(); | ||
|
|
||
| let assertions = | ||
| std::iter::successors(node.expression().ok(), |expression| match expression { | ||
| AnyJsExpression::TsNonNullAssertionExpression(assertion) => { | ||
| assertion.expression().ok() | ||
| } | ||
| _ => None, | ||
| }); | ||
|
|
||
| for assertion in assertions { | ||
| if let Some(non_null_expr) = assertion.as_ts_non_null_assertion_expression() { | ||
| mutation.remove_token(non_null_expr.excl_token().ok()?); | ||
| } | ||
| } | ||
|
|
||
| match node.parent::<AnyJsExpression>()? { | ||
| AnyJsExpression::JsComputedMemberExpression(parent) => { | ||
| if parent.is_optional() { | ||
| mutation.remove_token(node.excl_token().ok()?); | ||
| } else { | ||
| mutation.replace_token( | ||
| node.excl_token().ok()?, | ||
| make::token(JsSyntaxKind::QUESTIONDOT), | ||
| ); | ||
| } | ||
| } | ||
| AnyJsExpression::JsCallExpression(parent) => { | ||
| if parent.is_optional() { | ||
| mutation.remove_token(node.excl_token().ok()?); | ||
| } else { | ||
| mutation.replace_token( | ||
| node.excl_token().ok()?, | ||
| make::token(JsSyntaxKind::QUESTIONDOT), | ||
| ); | ||
| } | ||
| } | ||
| AnyJsExpression::JsStaticMemberExpression(parent) => { | ||
| if parent.is_optional() { | ||
| mutation.remove_token(node.excl_token().ok()?); | ||
| } else { | ||
| mutation.replace_token( | ||
| node.excl_token().ok()?, | ||
| make::token(JsSyntaxKind::QUESTION), | ||
| ); | ||
| } | ||
| } | ||
| _ => {} | ||
| }; | ||
|
|
||
| Some(JsRuleAction { | ||
| category: ActionCategory::QuickFix, | ||
| applicability: Applicability::MaybeIncorrect, | ||
| message: markup! { "Replace with optional chain operator "<Emphasis>"?."</Emphasis>" This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator" } | ||
| .to_owned(), | ||
| mutation, | ||
| }) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn can_replace_with_optional_chain(node: &TsNonNullAssertionExpression) -> bool { | ||
| use AnyJsExpression::*; | ||
|
|
||
| matches!( | ||
| node.parent::<AnyJsExpression>(), | ||
| Some(JsStaticMemberExpression(_) | JsComputedMemberExpression(_) | JsCallExpression(_)) | ||
| ) | ||
| } |
20 changes: 20 additions & 0 deletions
20
crates/rome_js_analyze/tests/specs/nursery/noNonNullAssertion/invalid.ts
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,20 @@ | ||
| x!; | ||
| x!.y; | ||
| x.y!; | ||
| !x!.y; | ||
| x!.y?.z; | ||
| x![y]; | ||
| x![y]?.z; | ||
| x.y.z!(); | ||
| x.y?.z!(); | ||
| x!!!; | ||
| x!!.y; | ||
| x.y!!; | ||
| x.y.z!!(); | ||
| x!?.[y].z; | ||
| x!?.y.z; | ||
| x!!!?.y.z; | ||
| x.y.z!?.(); | ||
| x.y.z!!!?.(); | ||
| (b! as number) = "test"; | ||
| (b!! as number) = "test"; |
Oops, something went wrong.