diff --git a/.changeset/good-ends-brake.md b/.changeset/good-ends-brake.md new file mode 100644 index 000000000000..e058732bc9dd --- /dev/null +++ b/.changeset/good-ends-brake.md @@ -0,0 +1,19 @@ +--- +"@biomejs/biome": patch +--- + +The documentation & rule sources for [`lint/complexity/noBannedTypes`](https://biomejs.dev/linter/rules/no-banned-types) have been updated to fix a few oversights. + +In addition to some general typo fixes: +- The rule now recommends `Record` instead of `Record` (the latter of which incorrectly allows symbol-keyed properties). +- The rule mentions an alternate method to enforce object emptiness involving `unique symbol`-based guards used by [`type-fest`](https://github.com/sindresorhus/type-fest/blob/main/source/empty-object.d.ts) and [many other packages](https://github.com/search?q=lang%3ATypeScript+%2Ftype%5Cs*%5Cw%2B%5Cs*%3D%5Cs*%5C%7B%5Cs*%5C%5B%5Cw%2B%5C%5D%5C%3F%3A+never%2F&type=code): + ```ts + declare const mySym: unique symbol; + + // Since this type's only property is an unexported `unique symbol`, nothing that imports it can specify any properties directly + // (as far as excess property checks go) + export type EmptyObject = { [mySym]?: never }; + export type IsEmptyObject = T extends EmptyObject ? true : false; + ``` + +The rule's listed sources have been updated as well to reflect the original source rule (`ban-types`) having been [split into 3 separate rules](https://github.com/typescript-eslint/typescript-eslint/pull/8977) circa April 2024. diff --git a/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs b/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs index 705bab27c42a..37258753f237 100644 --- a/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs +++ b/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs @@ -365,6 +365,18 @@ pub(crate) fn migrate_eslint_any_rule( .get_or_insert(Default::default()); rule.set_level(rule.level().max(rule_severity.into())); } + "@typescript-eslint/no-empty-object-type" => { + if !options.include_inspired { + results.add(eslint_name, eslint_to_biome::RuleMigrationResult::Inspired); + return false; + } + let group = rules.complexity.get_or_insert_with(Default::default); + let rule = group + .unwrap_group_as_mut() + .no_banned_types + .get_or_insert(Default::default()); + rule.set_level(rule.level().max(rule_severity.into())); + } "@typescript-eslint/no-explicit-any" => { let group = rules.suspicious.get_or_insert_with(Default::default); let rule = group @@ -569,6 +581,18 @@ pub(crate) fn migrate_eslint_any_rule( .get_or_insert(Default::default()); rule.set_level(rule.level().max(rule_severity.into())); } + "@typescript-eslint/no-unsafe-function-type" => { + if !options.include_inspired { + results.add(eslint_name, eslint_to_biome::RuleMigrationResult::Inspired); + return false; + } + let group = rules.complexity.get_or_insert_with(Default::default); + let rule = group + .unwrap_group_as_mut() + .no_banned_types + .get_or_insert(Default::default()); + rule.set_level(rule.level().max(rule_severity.into())); + } "@typescript-eslint/no-unused-vars" => { let group = rules.correctness.get_or_insert_with(Default::default); let rule = group @@ -601,6 +625,18 @@ pub(crate) fn migrate_eslint_any_rule( .get_or_insert(Default::default()); rule.set_level(rule.level().max(rule_severity.into())); } + "@typescript-eslint/no-wrapper-object-types" => { + if !options.include_inspired { + results.add(eslint_name, eslint_to_biome::RuleMigrationResult::Inspired); + return false; + } + let group = rules.complexity.get_or_insert_with(Default::default); + let rule = group + .unwrap_group_as_mut() + .no_banned_types + .get_or_insert(Default::default()); + rule.set_level(rule.level().max(rule_severity.into())); + } "@typescript-eslint/only-throw-error" => { if !options.include_inspired { results.add(eslint_name, eslint_to_biome::RuleMigrationResult::Inspired); diff --git a/crates/biome_js_analyze/src/lint/complexity/no_banned_types.rs b/crates/biome_js_analyze/src/lint/complexity/no_banned_types.rs index 02981f536fc6..38926feff63a 100644 --- a/crates/biome_js_analyze/src/lint/complexity/no_banned_types.rs +++ b/crates/biome_js_analyze/src/lint/complexity/no_banned_types.rs @@ -36,29 +36,33 @@ declare_lint_rule! { /// ### Disallow the unsafe `Function` type /// /// TypeScript's built-in `Function` type is capable of accepting callbacks of any shape or form, - /// behaving equivalent to `(...rest: any[]) => any` (which uses the unsafe `any` type) when called directly. + /// behaving equivalent to `(...rest: any[]) => any` (which uses the unsafe `any` type) when called directly. \ /// It also accepts classes or plain objects that happen to possess all properties of the `Function` class, /// which is likewise a potential source of confusion. /// /// As such, it is almost always preferable to explicitly specify function parameters and return types where possible. \ /// When a generic "catch-all" callback type is required, one of the following can be used instead: /// - `() => void`: A function that accepts no parameters and whose return value is ignored - /// - `(...args: never) => unknown`: A "top type" for functions that can be assigned any function type, + /// - `(...args: never) => unknown`: A "top type" for functions that can be _assigned_ any function type, /// but can't be called directly /// /// ### Disallow the misleading empty object type `{}` - /// In TypeScript, the type `{}` _doesn't_ represent an empty object (as many new to the language may assume). - /// It actually accepts any non-nullish value, _including non-object primitives_. - /// The following TypeScript example is thus perfectly valid: + /// `{}`, also known as the "empty object" type, _doesn't_ actually represent an empty object (despite what many new to TypeScript may assume). \ + /// Due to TypeScript's type system being _structural_ instead of nominal, it actually accepts _any non-nullish value_, + // including non-object primitives like numbers and strings[^1]. \ + /// The following example is thus perfectly valid TypeScript: /// - /// ```ts,expect_diagnostic + /// ```ts,ignore /// const n: {} = 0; /// ``` /// /// Often, developers writing `{}` actually mean one of the following: /// - `object`: Represents any object value /// - `unknown`: Represents any value at all, including `null` and `undefined` - /// - `{ [k: string]: never }` or `Record`: Represent object types that disallow property access + /// - `{ [k: keyof any]: never }` or `Record`: Represent object types whose properties are all of type `never` (and cannot be used) + /// - `{ [myUniqueInternalSymbol]?: never }`: Represents an object type whose only "property" is an unexported `unique symbol`, thereby forcing external consumers to omit it[^2]. \ + /// This can be used as a type guard for use in `extends` clauses or a type annotation for use in [excess property checks](https://www.typescriptlang.org/docs/handbook/2/objects.html#excess-property-checks), + /// both with their own respective use cases and pitfalls. /// /// To avoid confusion, this rule forbids the use of the type `{}`, except in two situations: /// @@ -79,7 +83,8 @@ declare_lint_rule! { /// In this last case, you can also use the `NonNullable` utility type to the same effect: /// /// ```ts - /// type NonNullableMyType = NonNullable; + /// // equivalent to `{}` + /// type AnythingNotNullish = NonNullable; /// ``` /// /// ## Examples @@ -149,11 +154,21 @@ declare_lint_rule! { /// type notNull = T & {}; /// ``` /// + /// [^1]: This is the exact same mechanism that allows passing `{ foo: number, bar: string }` + /// to a function expecting `{ bar: string }`. + /// Specifying `{}` doesn't restrict compatible types to ones with _exactly_ 0 properties; + /// it simply requires they have _at least_ 0 properties. + /// [^2]: In this case, you'd write `declare const myUniqueInternalSymbol: unique symbol` somewhere in the same file. pub NoBannedTypes { version: "1.0.0", name: "noBannedTypes", language: "ts", - sources: &[RuleSource::EslintTypeScript("ban-types").same()], + sources: &[ + RuleSource::EslintTypeScript("ban-types").same(), + RuleSource::EslintTypeScript("no-empty-object-type").inspired(), + RuleSource::EslintTypeScript("no-wrapper-object-types").inspired(), + RuleSource::EslintTypeScript("no-unsafe-function-type").inspired(), + ], recommended: true, severity: Severity::Warning, fix_kind: FixKind::Safe, @@ -318,11 +333,14 @@ impl BannedType { } Self::Object | Self::EmptyObject => { markup! { - "'"{ self.to_string() }"' accepts ""any"" non-nullable value, including non-object primitives like '123' and 'true'." + "'"{ self.to_string() }"' accepts ""any"" non-nullish value, including non-object primitives like " + "'""123""' and '""true""'." "\n- If you want a type meaning \"any arbitrary object\", use '""object""' instead." "\n- If you want a type meaning \"any value\", use '""unknown""' instead." - "\n- If you want a type meaning \"an object without any properties\", use " - "'""{ [k: string]: never }""' or '""Record""' instead." + "\n- If you want a type meaning \"an object whose properties cannot be used\", use " + "'""{ [k: keyof any]: never }""' or '""Record""' instead." + "\n- If you want a type meaning \"an object that cannot contain any properties whatsoever\", use " + "'""{ [uniqueSymbol]?: never }""' with an unexported ""unique symbol"" in the same file." }.to_owned() } } diff --git a/crates/biome_js_analyze/tests/specs/complexity/noBannedTypes/invalid.ts.snap b/crates/biome_js_analyze/tests/specs/complexity/noBannedTypes/invalid.ts.snap index d535a6f12be0..bff60c50aff2 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/noBannedTypes/invalid.ts.snap +++ b/crates/biome_js_analyze/tests/specs/complexity/noBannedTypes/invalid.ts.snap @@ -91,10 +91,11 @@ invalid.ts:3:8 lint/complexity/noBannedTypes ━━━━━━━━━━━ 4 │ 5 │ let b: { c: String }; - i 'Object' accepts any non-nullable value, including non-object primitives like '123' and 'true'. + i 'Object' accepts any non-nullish value, including non-object primitives like '123' and 'true'. - If you want a type meaning "any arbitrary object", use 'object' instead. - If you want a type meaning "any value", use 'unknown' instead. - - If you want a type meaning "an object without any properties", use '{ [k: string]: never }' or 'Record' instead. + - If you want a type meaning "an object whose properties cannot be used", use '{ [k: keyof any]: never }' or 'Record' instead. + - If you want a type meaning "an object that cannot contain any properties whatsoever", use '{ [uniqueSymbol]?: never }' with an unexported unique symbol in the same file. i If that's really what you want, use an inline disable comment. @@ -263,10 +264,11 @@ invalid.ts:11:58 lint/complexity/noBannedTypes ━━━━━━━━━━━ 12 │ constructor(foo: String | Object) {} 13 │ - i 'Object' accepts any non-nullable value, including non-object primitives like '123' and 'true'. + i 'Object' accepts any non-nullish value, including non-object primitives like '123' and 'true'. - If you want a type meaning "any arbitrary object", use 'object' instead. - If you want a type meaning "any value", use 'unknown' instead. - - If you want a type meaning "an object without any properties", use '{ [k: string]: never }' or 'Record' instead. + - If you want a type meaning "an object whose properties cannot be used", use '{ [k: keyof any]: never }' or 'Record' instead. + - If you want a type meaning "an object that cannot contain any properties whatsoever", use '{ [uniqueSymbol]?: never }' with an unexported unique symbol in the same file. i If that's really what you want, use an inline disable comment. @@ -313,10 +315,11 @@ invalid.ts:12:29 lint/complexity/noBannedTypes ━━━━━━━━━━━ 13 │ 14 │ exit(): Array { - i 'Object' accepts any non-nullable value, including non-object primitives like '123' and 'true'. + i 'Object' accepts any non-nullish value, including non-object primitives like '123' and 'true'. - If you want a type meaning "any arbitrary object", use 'object' instead. - If you want a type meaning "any value", use 'unknown' instead. - - If you want a type meaning "an object without any properties", use '{ [k: string]: never }' or 'Record' instead. + - If you want a type meaning "an object whose properties cannot be used", use '{ [k: keyof any]: never }' or 'Record' instead. + - If you want a type meaning "an object that cannot contain any properties whatsoever", use '{ [uniqueSymbol]?: never }' with an unexported unique symbol in the same file. i If that's really what you want, use an inline disable comment. @@ -483,10 +486,11 @@ invalid.ts:23:14 lint/complexity/noBannedTypes ━━━━━━━━━━━ 24 │ 25 │ let fn: Function = () => true - i '{}' accepts any non-nullable value, including non-object primitives like '123' and 'true'. + i '{}' accepts any non-nullish value, including non-object primitives like '123' and 'true'. - If you want a type meaning "any arbitrary object", use 'object' instead. - If you want a type meaning "any value", use 'unknown' instead. - - If you want a type meaning "an object without any properties", use '{ [k: string]: never }' or 'Record' instead. + - If you want a type meaning "an object whose properties cannot be used", use '{ [k: keyof any]: never }' or 'Record' instead. + - If you want a type meaning "an object that cannot contain any properties whatsoever", use '{ [uniqueSymbol]?: never }' with an unexported unique symbol in the same file. i If that's really what you want, use an inline disable comment. @@ -676,10 +680,11 @@ invalid.ts:37:17 lint/complexity/noBannedTypes ━━━━━━━━━━━ 38 │ 39 │ const capitalObj: Object = { a: 'string' }; - i 'Object' accepts any non-nullable value, including non-object primitives like '123' and 'true'. + i 'Object' accepts any non-nullish value, including non-object primitives like '123' and 'true'. - If you want a type meaning "any arbitrary object", use 'object' instead. - If you want a type meaning "any value", use 'unknown' instead. - - If you want a type meaning "an object without any properties", use '{ [k: string]: never }' or 'Record' instead. + - If you want a type meaning "an object whose properties cannot be used", use '{ [k: keyof any]: never }' or 'Record' instead. + - If you want a type meaning "an object that cannot contain any properties whatsoever", use '{ [uniqueSymbol]?: never }' with an unexported unique symbol in the same file. i If that's really what you want, use an inline disable comment. @@ -698,10 +703,11 @@ invalid.ts:39:19 lint/complexity/noBannedTypes ━━━━━━━━━━━ 40 │ 41 │ const curly1: { - i 'Object' accepts any non-nullable value, including non-object primitives like '123' and 'true'. + i 'Object' accepts any non-nullish value, including non-object primitives like '123' and 'true'. - If you want a type meaning "any arbitrary object", use 'object' instead. - If you want a type meaning "any value", use 'unknown' instead. - - If you want a type meaning "an object without any properties", use '{ [k: string]: never }' or 'Record' instead. + - If you want a type meaning "an object whose properties cannot be used", use '{ [k: keyof any]: never }' or 'Record' instead. + - If you want a type meaning "an object that cannot contain any properties whatsoever", use '{ [uniqueSymbol]?: never }' with an unexported unique symbol in the same file. i If that's really what you want, use an inline disable comment. @@ -723,10 +729,11 @@ invalid.ts:41:15 lint/complexity/noBannedTypes ━━━━━━━━━━━ 44 │ 45 │ const curly2: {} = { a: 'string' }; - i '{}' accepts any non-nullable value, including non-object primitives like '123' and 'true'. + i '{}' accepts any non-nullish value, including non-object primitives like '123' and 'true'. - If you want a type meaning "any arbitrary object", use 'object' instead. - If you want a type meaning "any value", use 'unknown' instead. - - If you want a type meaning "an object without any properties", use '{ [k: string]: never }' or 'Record' instead. + - If you want a type meaning "an object whose properties cannot be used", use '{ [k: keyof any]: never }' or 'Record' instead. + - If you want a type meaning "an object that cannot contain any properties whatsoever", use '{ [uniqueSymbol]?: never }' with an unexported unique symbol in the same file. i If that's really what you want, use an inline disable comment. @@ -744,10 +751,11 @@ invalid.ts:45:15 lint/complexity/noBannedTypes ━━━━━━━━━━━ │ ^^ 46 │ - i '{}' accepts any non-nullable value, including non-object primitives like '123' and 'true'. + i '{}' accepts any non-nullish value, including non-object primitives like '123' and 'true'. - If you want a type meaning "any arbitrary object", use 'object' instead. - If you want a type meaning "any value", use 'unknown' instead. - - If you want a type meaning "an object without any properties", use '{ [k: string]: never }' or 'Record' instead. + - If you want a type meaning "an object whose properties cannot be used", use '{ [k: keyof any]: never }' or 'Record' instead. + - If you want a type meaning "an object that cannot contain any properties whatsoever", use '{ [uniqueSymbol]?: never }' with an unexported unique symbol in the same file. i If that's really what you want, use an inline disable comment.