rome · Conaclos · Jun 22, 2023 · Jun 21, 2023
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -70,6 +70,30 @@ parameter decorators:
   - inside an export default function
   - for React.use* hooks
 
+- Relax [`noBannedTypes`](https://docs.rome.tools/lint/rules/nobannedtypes/) and improve documentation
+
+  The rule no longer reports a user type that reuses a banned type name.
+  The following code is now allowed:
+
+  ```ts
+  import { Number } from "a-lib";
+  declare const v: Number;
+  ```
+
+  The rule now allows the use of the type `{}` to denote a non-nullable generic type:
+
+  ```ts
+  function f<T extends {}>(x: T) {
+      assert(x != null);
+  }
+  ```
+
+  And in a type intersection for narrowing a type to its non-nullable equivalent type:
+
+  ```ts
+  type NonNullableMyType = MyType & {};
+  ```
+
 - Improve the diagnostic and the code action of [`useDefaultParameterLast`](https://docs.rome.tools/lint/rules/usedefaultparameterlast/).
 
   The diagnostic now reports the last required parameter which should precede optional and default parameters.

diff --git a/crates/rome_js_analyze/src/analyzers/nursery.rs b/crates/rome_js_analyze/src/analyzers/nursery.rs
diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs
diff --git a/.../src/analyzers/nursery/no_banned_types.rs → ...ntic_analyzers/nursery/no_banned_types.rs b/.../src/analyzers/nursery/no_banned_types.rs → ...ntic_analyzers/nursery/no_banned_types.rs
@@ -1,21 +1,65 @@
+use std::fmt::Display;
+
 use rome_analyze::context::RuleContext;
-use rome_analyze::{declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic};
+use rome_analyze::{declare_rule, ActionCategory, Rule, RuleDiagnostic};
 use rome_console::markup;
 use rome_diagnostics::Applicability;
 use rome_js_factory::make;
 use rome_js_syntax::{
-    JsReferenceIdentifier, JsSyntaxKind, TextRange, TsObjectType, TsReferenceType,
+    JsReferenceIdentifier, JsSyntaxKind, TextRange, TsIntersectionTypeElementList, TsObjectType,
+    TsReferenceType, TsTypeConstraintClause,
 };
-use rome_rowan::{declare_node_union, AstNodeList, BatchMutationExt};
+use rome_rowan::{declare_node_union, AstNode, AstNodeList, BatchMutationExt};
 
+use crate::semantic_services::Semantic;
 use crate::JsRuleAction;
 
 declare_rule! {
-    /// Disallow certain types.
+    /// Disallow primitive type aliases and misleading types.
+    ///
+    /// - Enforce consistent names for primitive types
+    ///
+    ///   Primitive types have aliases.
+    ///   For example, `Number` is an alias of `number`.
+    ///   The rule recommends the lowercase primitive type names.
+    ///
+    /// - Disallow the `Function` type
+    ///
+    ///   The `Function` type is loosely typed and is thus considered dangerous or harmful.
+    ///   `Function` is equivalent to the type `(...rest: any[]) => any` that uses the unsafe `any` type.
+    ///
+    /// - Disallow the misleading non-nullable type `{}`
+    ///
+    ///   In TypeScript, the type `{}` doesn't represent an empty object.
+    ///   It represents any value except `null` and `undefined`.
+    ///   The following TypeScript example is perfectly valid:
+    ///
+    ///   ```ts,expect_diagnostic
+    ///   const n: {} = 0
+    ///   ```
+    ///
+    ///   To represent an empty object, you should use `{ [k: string]: never }` or `Record<string, never>`.
+    ///
+    ///   To avoid any confusion, the rule forbids the use of the type `{}`,e except in two situation.
+    ///   In type constraints to restrict a generic type to non-nullable types:
+    ///
+    ///   ```ts
+    ///   function f<T extends {}>(x: T) {
+    ///       assert(x != null);
+    ///   }
+    ///   ```
     ///
-    /// > Some built-in types have aliases, while some types are considered dangerous or harmful. It's often a good idea to ban certain types to help with consistency and safety.
+    ///   And in a type intersection to narrow a type to its non-nullable equivalent type:
     ///
-    /// > This rule bans specific types and can suggest alternatives. Note that it doesn't ban the corresponding runtime objects from being used.
+    ///   ```ts
+    ///   type NonNullableMyType = MyType & {};
+    ///   ```
+    ///
+    ///   In this last case, you can also use the `NonNullable` utility type:
+    ///
+    ///   ```ts
+    ///   type NonNullableMyType = NonNullable<MyType>;
+    ///   ```
     ///
     /// Source: https://typescript-eslint.io/rules/ban-types
     ///
@@ -53,9 +97,109 @@ declare_rule! {
     }
 }
 
+impl Rule for NoBannedTypes {
+    type Query = Semantic<TsBannedType>;
+    type State = State;
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let query = ctx.query();
+        let model = ctx.model();
+        match query {
+            TsBannedType::TsObjectType(ts_object_type) => {
+                // Allow empty object type for type constraint and intersections.
+                // ```js
+                // type AssertNonNullGeneric<T extends {}> = T
+                // type NonNull<T> = T & {}
+                // ```
+                if ts_object_type.members().is_empty()
+                    && (ts_object_type.parent::<TsTypeConstraintClause>().is_none()
+                        && ts_object_type
+                            .parent::<TsIntersectionTypeElementList>()
+                            .is_none())
+                {
+                    return Some(State {
+                        banned_type: BannedType::EmptyObject,
+                        banned_type_range: ts_object_type.range(),
+                        reference_identifier: None,
+                    });
+                }
+            }
+            TsBannedType::TsReferenceType(ts_reference_type) => {
+                let ts_any_name = ts_reference_type.name().ok()?;
+                let reference_identifier = ts_any_name.as_js_reference_identifier()?;
+                if model.binding(reference_identifier).is_none() {
+                    // if the dientifier is global
+                    let identifier_token = reference_identifier.value_token().ok()?;
+                    if let Some(banned_type) = BannedType::from_str(identifier_token.text_trimmed())
+                    {
+                        return Some(State {
+                            banned_type,
+                            banned_type_range: identifier_token.text_trimmed_range(),
+                            reference_identifier: Some(reference_identifier.clone()),
+                        });
+                    }
+                }
+            }
+        }
+
+        None
+    }
+
+    fn diagnostic(
+        _ctx: &RuleContext<Self>,
+        State {
+            banned_type,
+            banned_type_range,
+            ..
+        }: &Self::State,
+    ) -> Option<RuleDiagnostic> {
+        let diagnostic = RuleDiagnostic::new(
+            rule_category!(),
+            banned_type_range,
+            markup! {"Don't use '"{banned_type.to_string()}"' as a type."}.to_owned(),
+        )
+        .note(markup! { {banned_type.message()} }.to_owned());
+        Some(diagnostic)
+    }
+
+    fn action(
+        ctx: &RuleContext<Self>,
+        State {
+            banned_type,
+            reference_identifier,
+            ..
+        }: &Self::State,
+    ) -> Option<JsRuleAction> {
+        let mut mutation = ctx.root().begin();
+        let suggested_type = banned_type.as_js_syntax_kind()?.to_string()?;
+        mutation.replace_node(reference_identifier.clone()?, banned_type.fix_with()?);
+        Some(JsRuleAction {
+            category: ActionCategory::QuickFix,
+            applicability: Applicability::Always,
+            message: markup! { "Use '"{suggested_type}"' instead" }.to_owned(),
+            mutation,
+        })
+    }
+}
+
 declare_node_union! {
     pub(crate) TsBannedType = TsReferenceType | TsObjectType
 }
+
+pub struct State {
+    /// Reference to the enum item containing the banned type.
+    /// Used for both diagnostic and action.
+    banned_type: BannedType,
+    /// Text range used to diagnostic the banned type.
+    banned_type_range: TextRange,
+    /// Reference to the node to be replaced in the action.
+    /// This is optional because we don't replace empty objects references.
+    reference_identifier: Option<JsReferenceIdentifier>,
+}
+
+#[derive(Debug)]
 pub enum BannedType {
     BigInt,
     Boolean,
@@ -84,20 +228,6 @@ impl BannedType {
         })
     }
 
-    /// Convert a [BannedType] to a JS string literal
-    fn as_str(&self) -> &'static str {
-        match self {
-            Self::BigInt => "BigInt",
-            Self::Boolean => "Boolean",
-            Self::Function => "Function",
-            Self::Number => "Number",
-            Self::Object => "Object",
-            Self::String => "String",
-            Self::Symbol => "Symbol",
-            Self::EmptyObject => "{}",
-        }
-    }
-
     /// Retrieves a diagnostic message from a [BannedType]
     fn message(&self) -> &str {
         match *self {
@@ -109,8 +239,8 @@ impl BannedType {
 			Self::Function =>
 				"Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.",
 			Self::Object =>
-				"Prefer explicitly define the object shape. This type means \"any non-nullish value\", which is slightly better than 'unknown', but it's still a broad type.",
-			Self::EmptyObject => "Prefer explicitly define the object shape. '{}' means \"any non-nullish value\".",
+				"Prefer explicitly define the object shape. This type means \"any non-nullable value\", which is slightly better than 'unknown', but it's still a broad type.",
+			Self::EmptyObject => "Prefer explicitly define the object shape. '{}' means \"any non-nullable value\".",
 		}
     }
 
@@ -138,103 +268,18 @@ impl BannedType {
     }
 }
 
-pub struct RuleState {
-    /// Reference to the enum item containing the banned type.
-    /// Used for both diagnostic and action.
-    banned_type: BannedType,
-    /// Text range used to diagnostic the banned type.
-    banned_type_range: TextRange,
-    /// Reference to the node to be replaced in the action.
-    /// This is optional because we don't replace empty objects references.
-    reference_identifier: Option<JsReferenceIdentifier>,
-}
-
-impl Rule for NoBannedTypes {
-    type Query = Ast<TsBannedType>;
-    type State = RuleState;
-    type Signals = Option<Self::State>;
-    type Options = ();
-
-    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
-        let query = ctx.query();
-
-        match query {
-            TsBannedType::TsObjectType(ts_object_type) => {
-                if ts_object_type.members().is_empty() {
-                    let range = TextRange::new(
-                        ts_object_type
-                            .l_curly_token()
-                            .ok()?
-                            .text_trimmed_range()
-                            .start(),
-                        ts_object_type
-                            .r_curly_token()
-                            .ok()?
-                            .text_trimmed_range()
-                            .end(),
-                    );
-
-                    return Some(RuleState {
-                        banned_type: BannedType::EmptyObject,
-                        banned_type_range: range,
-                        reference_identifier: None,
-                    });
-                }
-            }
-            TsBannedType::TsReferenceType(ts_reference_type) => {
-                let ts_any_name = ts_reference_type.name().ok()?;
-                let reference_identifier = ts_any_name.as_js_reference_identifier()?;
-                let identifier_token = reference_identifier.value_token().ok()?;
-
-                if let Some(banned_type) = BannedType::from_str(identifier_token.text_trimmed()) {
-                    return Some(RuleState {
-                        banned_type,
-                        banned_type_range: identifier_token.text_trimmed_range(),
-                        reference_identifier: Some(reference_identifier.clone()),
-                    });
-                }
-            }
-        }
-
-        None
-    }
-
-    fn diagnostic(
-        _ctx: &RuleContext<Self>,
-        RuleState {
-            banned_type,
-            banned_type_range,
-            ..
-        }: &Self::State,
-    ) -> Option<RuleDiagnostic> {
-        let diagnostic = RuleDiagnostic::new(
-            rule_category!(),
-            banned_type_range,
-            markup! {"Don't use '"{banned_type.as_str()}"' as a type."}.to_owned(),
-        )
-        .note(markup! { {banned_type.message()} }.to_owned());
-
-        Some(diagnostic)
-    }
-
-    fn action(
-        ctx: &RuleContext<Self>,
-        RuleState {
-            banned_type,
-            reference_identifier,
-            ..
-        }: &Self::State,
-    ) -> Option<JsRuleAction> {
-        let mut mutation = ctx.root().begin();
-        let suggested_type = banned_type.as_js_syntax_kind()?.to_string()?;
-
-        mutation.replace_node(reference_identifier.clone()?, banned_type.fix_with()?);
-
-        Some(JsRuleAction {
-            category: ActionCategory::QuickFix,
-            applicability: Applicability::Always,
-            message: markup! { "Use '"{suggested_type}"' instead" }.to_owned(),
-            mutation,
-        })
+impl Display for BannedType {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let representation = match self {
+            Self::BigInt => "BigInt",
+            Self::Boolean => "Boolean",
+            Self::Function => "Function",
+            Self::Number => "Number",
+            Self::Object => "Object",
+            Self::String => "String",
+            Self::Symbol => "Symbol",
+            Self::EmptyObject => "{}",
+        };
+        write!(f, "{}", representation)
     }
 }