Enhancement: no-unused-vars option for allowing type-only `declare class` expressions

[LukeAbby]

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

• I have searched for related issues and found none that match my proposal.
• I have searched the current rule list and found no rules that match my proposal.
• I have read the FAQ and my problem is not listed.

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://eslint.org/docs/latest/rules/no-unused-vars

Description

While no-unused-var triggering on most values that are only used in types makes sense, this breaks down when doing advanced things with classes. I propose an option to allow declare class expressions to be counted as used by types.

I run into this frequently when writing declaration files, specifically because I write type-only subclasses to widen the class (the most frequent example is to create a bound that allows abstract classes and changed constructors). I recognize this is niche, a more universal example would be typing JS mixins or in general functions that return internally-scoped classes.

For example if you encounter a JS function like:

function Mixin(BaseClass) {
    return class Mixed extends BaseClass { ... }
}

It is most nicely typed as:

// Not really defined at the top level of this file but hoisted because there's no way to inline and it'd be really hideous if it did work. Module visibility means that this fake value existing doesn't matter in practice.
declare class Mixed { ... }

type AnyClass = new (arg0: never, ...args: never[]) => object;
function Mixin<BaseClass extends AnyClass>(BaseClass: BaseClass): BaseClass & Mixed;

For those who want to comply with this rule, some fake-value classes can be successfully turned into types, though it is more verbose:

declare class SomeClass {
    static staticProp: number;
    instanceProp: number;
}

Can be converted to:

interface SomeClassConstructor {
    staticProp: number;
    new (): SomeClass;
}

interface SomeClass {
    instanceProp: number;
}

This already is a mild sacrifice because it's more verbose, forces you to separate instance and static props (even if they make more intuitive sense to order them interspersed), and you lose the ability to write typeof SomeClass to mean the class, which to me feels more intuitive if you're used to regular classes.

However there's no valid transformations when:

• The class has an accessibility modifier on a property, i.e. private, protected, readonly, and internal
• The class has a truly private property. This may seem unimportant but they have implications towards the variance of the class so stripping them is not a true 1:1 mapping.
• The class has a getter or setter. While in common cases you could emulate them by creating a property this breaks down when the getter/setter can't be unified (e.g. set number, get string) and it also breaks subclassing as you can't substitute a getter/setter pair for a property.
• The class is abstract.
• The class has an internal, private, or protected constructor. You could simply leave out the constructor in SomeClassConstructor but this gives worse diagnostics.

While I'm quite comfortable with classes and translating them to regular interfaces, I also imagine most people will find the syntax for generic classes annoying. It also becomes more awkward when the base class expression is complex like mixins, especially when generic passthrough is involved:

type AnyClass = new (...args: any[]) => object;
function Mixin<BaseClass extends AnyClass>(BaseClass: BaseClass) {
    return class extends BaseClass { ... }
}

declare class Generic<T> {
    prop: T;
}

class Mixed<T> extends Mixin(Generic)<T> { ... }

You can transform Mixed to:

interface MixedConstructor {
    new <T>(): Mixed<T>;
}

type Mixed<T> = typeof Mixin<typeof Generic<T>> & { ... };

This code is obviously ugly and it doesn't generalize well to more complex mixins, especially where the generic parameters and parameters differ significantly.

Fail

declare abstract class AnyError extends Error {
    constructor(arg0: never, ...args: never[]);
}

Pass

declare abstract class AnyError extends Error {
    constructor(arg0: never, ...args: never[]);
}

type ErrorClasses = Array<typeof AnyError>;

Additional Info

No response

[bradzacher]

My question would be "how often are you doing this in your codebase?"

Are we talking every file? One in 10 files? One in a hundred? One in a thousand?

Anecdotally I've very rarely seen this pattern of dynamic class creation. And quickly searching public github repos shows just 3460 cases.

I just don't think that this passes the "broadly applicable" acceptance test. But I'm looking for more info to convince me otherwise.

[LukeAbby]

Personally I have 237 classes of the form:

declare abstract class AnyCombatTracker extends CombatTracker<ApplicationOptions> {
  constructor(arg0: never, ...args: never[]);
}

This is for 647 files and a total of 463 classes that match ^(declare )?(abstract )?class. Merely searching for class isn't precise enough because there's a lot of the word "class" mentioned in documentation or non-definition contexts.

This is of course a bit on the extreme end and I don't expect most libraries would standardize something like this. I've done this in my library to help users side-step common subclassing issues they run into where they want a bound like CombatTracker.Any that permits subclasses, basically what passes the instanceof CombatTracker test. The JS library I'm typing does instanceof and prototype checks rather than requiring a specific constructor in most cases.

To focus on mixins, I have 16 mixin functions in the JS repo I've been typing that use this hoisted class trick. It makes maintaining them much easier because I can keep them in the same form as the original JS (property order) etc. I do think mixins are somewhat niche because, frankly, they're a bit of an anti-pattern and given the rough edges in TypeScript's support for mixins it can scare people away.

Though for what it's worth searching for return class is going to miss a lot of mixins. From searching extends AnyConstructor alone I see people doing:

declare const Events : <T extends AnyConstructor<Base>>(base : T) => AnyConstructor<InstanceType<T> & EventsMixin>
type MixinHelperFunc = <A extends AnyConstructor, T>(required: [A], arg: T) => T extends AnyFunction<infer M> ? M : never
export const Mixin1Func = <T extends AnyConstructor<Base>>(base: T) =>
    /**
     * Internal class of the Mixin1
     */
    class Mixin1Class extends base {
        property1 = "init";

        method1(arg: Mixin1Type[]): Mixin1Type[] {
            return [...arg, this];
        }
    };

The last one is an example of a .ts file successfully using an unnameable class return value, if you were to instead be using isolatedDeclarations (it'd error with Inference from class expressions is not supported with --isolatedDeclarations.) or creating a .d.ts file for this you would need to use this declare class approach.

A more accurate survey of cases where you might want to hoist classes to the top level when writing .d.ts files would be looking for /^[ \t]+class.*extends/; the indentation at the beginning of the line excludes most "regular" classes, leaving mostly classes that would need to be hoisted out of functions etc., though it's obviously not perfect.

I'm unfortunately not sure there is a good regex because if you want to look for mixins the best approach would be looking for a function that contains a class that extends one of the parameters. Maybe /^[ \t]+class [A-Z].* extends [a-z]([^.]| .*)$/ is decent? It looks for a class that uses a pascal cased class name that then extends a lower cased class. This helps indicate that it's class Mixed extends baseClass but obviously this misses class Mixed extends BaseClass

[bradzacher]

If you include \bextends AnyConstructor\b in the search it only ups the count to 3519 results (so +59 cases).

/^[ \t]+class.*extends/ has too many false positives as it matches cases like:

namespace Foo {
  class C1 extends Bar { ... }
}
function Baz() {
  class C2 extends Bar { ... }
  return new C2();
}

Doing a quick search to also include cases of declare class in non-declaration files only reveals an additional 1940 cases.

There's ultimately no perfect regex to match mixins and there's no easy way to syntactically search all of GH. But we don't want the exact numbers -- a rough heuristic of scale is what I'm interested in. All in all we're sitting at a rough heuristic showing there's possibly single-digit-thousands declarations of mixins with that number maybe being in the "low teens".

In the scope of TS patterns and TS code that scale is a grain of sand on a beach, really.

I'm personally unconvinced that this pattern is common enough for us to build and maintain an entire option to support.

---

cc @typescript-eslint/triage-team -- do we have any other thoughts or views?
To summarise my view: I'm a -1 here as I don't believe this meets the "broadly applicable" requirement. Would love to hear your thoughts.

[JoshuaKGoldberg]

I'm also not convinced. Maybe this is a good case for evaluating community engagement?