Skip to content

Enhancement: [no-empty-object-type] Checks for generated empty-object-type #10619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
4 tasks done
yeonjuan opened this issue Jan 6, 2025 · 7 comments
Open
4 tasks done
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look

Comments

@yeonjuan
Copy link
Contributor

yeonjuan commented Jan 6, 2025

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/no-empty-object-type/

Description

I suggest that the no-empty-object-type rule also disallows {} generated by type transformations.
For example, using Omit<V,K> on a type that is nullable will unintentionally create a {} type.

type NullableData = null | { name: string;  num: number };
type ITS_EMPTY_OBJECT_TYPE = Omit<NullableData, 'name'>; // '{}'

Fail

type Data = { name: string; num: number };
type NullableData = null | Data;

function doSomething<T extends Omit<NullableData, 'name'> // lint error
>(param: T) { }

type Unexpected = Omit<NullableData, 'name'>; // lint error

Pass

type Data = { name: string; num: number };

function doSomething<T extends Omit<Data, 'name'>>(param: T) { }

type Expected = Omit<Data, 'name'>;

Additional Info

Perhaps it would be appropriate to handle this in no-unsafe-*?

@yeonjuan yeonjuan added enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jan 6, 2025
@bradzacher
Copy link
Member

IMO this would be a hard no because checking for this requires type information.
The rule is intended to be a fast, syntax-only check for people misusing the {} type -- adding type info to the rule is a breaking change that limits who can use the rule -- so generally we don't do it unless there's a REALLY good reason.

There's potential scope for this in a new rule -- but also this seems like it would be a difficult thing to check.

What does it mean to "generate" a type?
What constructs would we be checking to see if the type is {}?
Realistically any type operation could result in {} and additionally any usage of a generic type could result in {} -- does that mean we should check every single intersection, conditional type, and generic type reference in the codebase? That seems very slow and heavy!

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for team members to take a look labels Jan 7, 2025
@yeonjuan
Copy link
Contributor Author

yeonjuan commented Jan 7, 2025

adding type info to the rule is a breaking change that limits who can use the rule

I see. I agree that it's not appropriate as an option of no-empty-type.

Still, it would be nice if there was some other way to prevent mistakes like the one above.
The example above is actually a simplified representation of the code I found while fixing a bug.
Specifically, if the {} type is used as a parameter to a function, it is similar to if the any type is used as a parameter type, which is less type-safe.

type Data = { name: string; num: number };
type NullableData = null | Data;

function foo(param: Omit<NullableData, 'name'>) { }

foo("aaa"); // no error
foo({}); // no error
foo(1); // no error

That seems very slow and heavy!

What if we only checked when type operations are used and when they are used as generic arguments?
For nested operations, I think we can optimize by only checking the type that is finally created.

type Computed = Omit<NullableData, 'name'>; // check

function foo (arg: Computed) { // Uncheck for the references
}

@bradzacher
Copy link
Member

What if we only checked when type operations are used and when they are used as generic arguments

But how can we tell that Omit<NullableData, 'name'> under the hood uses an &?
We can't without type information!

This is the difficult problem to solve.

At its core you really want to just check every single intersection type across the codebase to look for {} -- which is conceptually simple enough.
However type aliases with generics mask an underlying & operation -- which means we can only assume that every single type reference with a generic might hide an & operation and thus we would need to check it.

@yeonjuan
Copy link
Contributor Author

yeonjuan commented Jan 14, 2025

Okay, so as an alternative, what do you think about adding a rule like no-unsafe-parameters which checks whether {} is used in function & generic parameters or not? This is because it allows almost any type if {} is used in the parameter type, it behaves almost like any.

playground

type Data = { name: string; num: number };
type NullableData = null | Data;

function wrongFunc(param: Omit<NullableData, 'name'> // Unexpected "{}"
) { }

wrongFunc("aaa");
wrongFunc(123);
wrongFunc({});

type WrongFunc< T extends  Omit<NullableData, 'name'> // Unexpected "{}"
> = T;

type A = WrongFunc<1>;
type B = WrongFunc<{}>;

// argument, it is easy to detect with ts error.
function foo(arg: number) {}
declare const arg: Omit<NullableData, 'name'> ;
foo(arg); // ts error 👍

@JoshuaKGoldberg JoshuaKGoldberg added triage Waiting for team members to take a look and removed awaiting response Issues waiting for a reply from the OP or another party labels Mar 2, 2025
@JoshuaKGoldberg
Copy link
Member

This makes me nervous. I don't have hard data, but I suspect {} becomes the default inferred type parameter in more cases than we'd expect.

If someone could make a prototype version of the rule and run it on typescript-eslint & a few other large codebases, that would be helpful. I think we'll want to see real-world proof of whether this is helpful.

@JoshuaKGoldberg JoshuaKGoldberg added evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important and removed triage Waiting for team members to take a look labels Mar 10, 2025
@yeonjuan
Copy link
Contributor Author

If someone could make a prototype version of the rule and run it on typescript-eslint & a few other large codebases, that would be helpful. I think we'll want to see real-world proof of whether this is helpful.

Sounds like a good idea. I'll look into it. Thanks

@yeonjuan
Copy link
Contributor Author

yeonjuan commented Apr 6, 2025

Hi, @JoshuaKGoldberg
I implemented a roughly implemented rule and tested it by setting it up in typescript-eslint, nest, strapi, and trcp projects.

I couldn't find any particular errors in trcp and strapi.
An error occurred in typescript-eslint, but it's not a concern since it was caused intentionally by using the {} type.
(It would probably be better to ignore the operand type when an actual rule is implemented. NonNullable<unknown>)

'PropertyDefinition[value != null]'(
node: { value: NonNullable<unknown> } & TSESTree.PropertyDefinition,
): void {


A meaningful error was found in the nest project. (here)

    //  {} type
    const keepaliveKeys: Record<
      keyof GrpcOptions['options']['keepalive'],
      string
    > = {

As shown in this PR, an incorrect type operation resulted in a {} type. (simplified example on typescript playground)

@JoshuaKGoldberg JoshuaKGoldberg added the triage Waiting for team members to take a look label Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look
Projects
None yet
Development

No branches or pull requests

3 participants